drivers: power: adp1050: add support for adp1051 #2381

ivangilmercano · 2024-11-29T06:55:56Z

Pull Request Description

The ADP1051 is an advanced digital controller with a PMBusTM interface targeting high density, high efficiency dc-to-dc power
conversion. This controller implements voltage mode control with high speed, input line feedforward for enhanced transient and improved noise performance. Some of the added support to the adp1050 driver to support adp1051 include extra pwm output, light load mode, and deep light load mode.

PR Type

Bug fix (change that fixes an issue)
New feature (change that adds new functionality)
Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

I have followed the Coding style guidelines
I have performed a self-review of the changes
I have commented my code, at least hard-to-understand parts
I have build all projects affected by the changes in this PR
I have tested in hardware affected projects, at the relevant boards
I have signed off all commits from this PR
I have updated the documentation (wiki pages, ReadMe etc), if applies

CLAassistant · 2024-11-29T06:56:02Z

All committers have signed the CLA.

kister-jimenez · 2024-11-29T06:58:59Z

/AzurePipelines run

azure-pipelines · 2024-11-29T06:59:11Z

Azure Pipelines successfully started running 2 pipeline(s).

jemfgeronimo · 2024-11-29T08:42:32Z

drivers/power/adp1050/README.rst


-ADP1050 IIO Driver Initialization Example
+ADP105x IIO Driver Initialization Example


changes from ADP1050 to ADP105x (this and some above) are unnecessary

jemfgeronimo · 2024-11-29T08:43:18Z

drivers/power/adp1050/adp1050.c


 		reg_fedge = ADP1050_OUTA_FALLING_EDGE_TIMING;
 		reg_redge = ADP1050_OUTA_RISING_EDGE_TIMING;
 		reg_lsb = ADP1050_OUTA_RISING_FALLING_TIMING_LSB;
-


remove these kinds of changes

dont include these changes: removed/added empty lines

jemfgeronimo · 2024-11-29T08:44:19Z

drivers/power/adp1050/adp1050.c

 		break;
+


ditto on added empty lines

jemfgeronimo · 2024-11-29T08:45:25Z

drivers/power/adp1050/adp1050.c

 	case ADP1050_OUTB:
 		fedge_msb = no_os_field_get(ADP1050_EDGE_MSB_MASK, pulse_width + pulse_start);
 		redge_msb = no_os_field_get(ADP1050_EDGE_MSB_MASK, pulse_start);
 		lsb = no_os_field_get(ADP1050_FALLING_EDGE_LSB_MASK,
 				      pulse_width + pulse_start) | no_os_field_get(ADP1050_RISING_EDGE_LSB_MASK,
-						      pulse_start);
+					      pulse_start);


does this change only involve indention? if so, i think this is unnecessary

jemfgeronimo · 2024-11-29T08:45:50Z

drivers/power/adp1050/adp1050.c

 	case ADP1050_SR1:
 		redge_msb = no_os_field_get(ADP1050_EDGE_MSB_MASK, pulse_width + pulse_start);
 		fedge_msb = no_os_field_get(ADP1050_EDGE_MSB_MASK, pulse_start);
 		lsb = no_os_field_get(ADP1050_RISING_EDGE_LSB_MASK,
 				      pulse_width + pulse_start) | no_os_field_get(ADP1050_FALLING_EDGE_LSB_MASK,
-						      pulse_start);
+					      pulse_start);


ditto on indention

jemfgeronimo · 2024-11-29T08:46:26Z

drivers/power/adp1050/adp1050.c

@@ -610,6 +750,7 @@ int adp1050_set_open_loop(struct adp1050_desc *desc, uint8_t rising_edge,
 	switch (chan) {
 	case ADP1050_OUTA:
 	case ADP1050_OUTB:
+		/** notice this*/


i think this comment is unnecessary

jemfgeronimo · 2024-11-29T08:47:25Z

drivers/power/adp1050/adp1050.c

-int adp1050_filter(struct adp1050_desc *desc, uint8_t zero, uint8_t pole,
-		   uint8_t lf, uint8_t hf)
+int adp1050_nfilter(struct adp1050_desc *desc, uint8_t zero, uint8_t pole,
+		    uint8_t lf, uint8_t hf)


what is the reason for this renaming?
also, ditto on indention

the renaming was done due to the addition of a new filter from the adp1051 which includes normal filter and light filter

jemfgeronimo · 2024-11-29T08:48:55Z

drivers/power/adp1050/adp1050.c

@@ -1184,4 +1354,4 @@ int adp1050_remove(struct adp1050_desc *desc)
 	no_os_free(desc);

 	return 0;
-}
+}


don't remove empty line on EOF

jemfgeronimo · 2024-11-29T08:49:36Z

drivers/power/adp1050/adp1050.h

@@ -205,15 +240,20 @@
 #define ADP1050_VOUT_MARGIN_HIGH		0x25
 #define ADP1050_VOUT_MARGIN_LOW			0x26
 #define ADP1050_VOUT_TRANSITION_RATE		0x27
+#define ADP1051_VOUT_DROOP			0x28 //ADP1051


unnecessary comment

jemfgeronimo · 2024-11-29T08:49:49Z

drivers/power/adp1050/adp1050.h

 #define ADP1050_VOUT_SCALE_LOOP			0x29
 #define ADP1050_VOUT_SCALE_MONITOR		0x2A
 #define ADP1050_FREQUENCY_SWITCH		0x33
 #define ADP1050_VIN_ON				0x35
 #define ADP1050_VIN_OFF				0x36
+#define ADP1051_IOUT_CAL_GAIN			0x38 //ADP1051


ditto on unnecessary comment

jemfgeronimo · 2024-11-29T08:50:47Z

drivers/power/adp1050/adp1050.h

@@ -328,6 +374,11 @@
 #define ADP1050_PWM_180_PHASE_SHIFT_SETTINGS	0xFE3B
 #define ADP1050_MODULATION_LIMIT		0xFE3C
 #define ADP1050_FEEDFORWARD_SS_FILTER_GAIN	0xFE3D
+/***********Added Support for ADP1051******************/


i think ADP1051_ prefix should suffice. These comments are unnecessary.

jemfgeronimo · 2024-11-29T08:52:43Z

drivers/power/adp1050/adp1050.h

 	struct no_os_pwm_init_param *syni_param;
+	struct no_os_gpio_init_param *pg_alt_param;


This struct member already exists, you just moved it.

jemfgeronimo · 2024-11-29T08:53:26Z

projects/adp1050/Makefile

-BASIC_EXAMPLE = n
-IIO_EXAMPLE = y
+BASIC_EXAMPLE = y
+IIO_EXAMPLE = n


these changes are unnecessary

jemfgeronimo · 2024-11-29T08:53:53Z

projects/adp1050/README.rst

-ADP1050 no-OS Example Project
-=============================
+Evaluating the ADP105x
+======================


You can just keep the existing texts.

jemfgeronimo · 2024-11-29T08:55:58Z

projects/adp1050/src/examples/basic/basic_example.c


 exit:
 	if (ret)
 		pr_info("Error!\n");
 	adp1050_remove(adp1050_desc);
 	return ret;
-}
+}


Ditto on empty line on EOF.
Also, If the existing basic_example also works on your device, you can leave it as it is.

jemfgeronimo · 2024-11-29T08:57:59Z

projects/adp1050/src/platform/maxim/main.c

@@ -65,8 +71,8 @@ int main()
 #error At least one example has to be selected using y value in Makefile.
 #elif (BASIC_EXAMPLE + IIO_EXAMPLE > 1)
 #error Selected example projects cannot be enabled at the same time. \
-Please enable only one example and rebuild the project.
+Please enable only one example and re-build the project.


I think this is one of the reasons of the astyle check error. Remove this change.

CiprianRegus · 2024-11-29T09:00:42Z

drivers/power/adp1050/adp1050.c

@@ -5,30 +5,36 @@
 ********************************************************************************
 * Copyright 2024(c) Analog Devices, Inc.
 *
+ * All rights reserved.


The current license for all No-OS code is BSD 3 clause. Please leave it as it currently is in the adpd1050.c file.

Signed-off-by: ivangilmercano <[email protected]>

buha · 2024-12-02T08:07:45Z

/AzurePipelines run

azure-pipelines · 2024-12-02T08:07:58Z

Azure Pipelines successfully started running 2 pipeline(s).

Signed-off-by: ivangilmercano <[email protected]>

kister-jimenez · 2024-12-02T12:27:28Z

/AzurePipelines run

azure-pipelines · 2024-12-02T12:27:40Z

Azure Pipelines successfully started running 2 pipeline(s).

ivangilmercano requested review from buha, CiprianRegus, dbogdan, amiclaus, rbolboac and danmois as code owners November 29, 2024 06:55

kister-jimenez self-requested a review November 29, 2024 06:57

jemfgeronimo reviewed Nov 29, 2024

View reviewed changes

CiprianRegus reviewed Nov 29, 2024

View reviewed changes

kister-jimenez marked this pull request as draft November 29, 2024 09:12

ivangilmercano force-pushed the dev/adp1051 branch 2 times, most recently from 41af4a3 to 4db75de Compare December 2, 2024 05:37

ivangilmercano force-pushed the dev/adp1051 branch 9 times, most recently from e3adef1 to df8eab2 Compare December 2, 2024 07:46

Adding driver support for ADP105x - ADP1051

43c4356

Signed-off-by: ivangilmercano <[email protected]>

ivangilmercano force-pushed the dev/adp1051 branch from df8eab2 to d5fe978 Compare December 2, 2024 07:55

ivangilmercano added 3 commits December 2, 2024 16:12

Adding project support for ADP105x - ADP1051

568b6dd

Signed-off-by: ivangilmercano <[email protected]>

Add README documentation for ADP105x - ADP1051

6c7f85f

Signed-off-by: ivangilmercano <[email protected]>

Add README project documentation for ADP105x - ADP1051

1d6a400

Signed-off-by: ivangilmercano <[email protected]>

ivangilmercano force-pushed the dev/adp1051 branch from 7b7cb22 to 1d6a400 Compare December 2, 2024 08:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: power: adp1050: add support for adp1051 #2381

drivers: power: adp1050: add support for adp1051 #2381

ivangilmercano commented Nov 29, 2024 •

edited

Loading

CLAassistant commented Nov 29, 2024 •

edited

Loading

kister-jimenez commented Nov 29, 2024

azure-pipelines bot commented Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

ivangilmercano Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

jemfgeronimo Nov 29, 2024

CiprianRegus Nov 29, 2024 •

edited

Loading

buha commented Dec 2, 2024

azure-pipelines bot commented Dec 2, 2024

kister-jimenez commented Dec 2, 2024

azure-pipelines bot commented Dec 2, 2024


		ADP1050 IIO Driver Initialization Example
		ADP105x IIO Driver Initialization Example

		struct no_os_pwm_init_param *syni_param;
		struct no_os_gpio_init_param *pg_alt_param;

drivers: power: adp1050: add support for adp1051 #2381

Are you sure you want to change the base?

drivers: power: adp1050: add support for adp1051 #2381

Conversation

ivangilmercano commented Nov 29, 2024 • edited Loading

Pull Request Description

PR Type

PR Checklist

CLAassistant commented Nov 29, 2024 • edited Loading

kister-jimenez commented Nov 29, 2024

azure-pipelines bot commented Nov 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CiprianRegus Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

buha commented Dec 2, 2024

azure-pipelines bot commented Dec 2, 2024

kister-jimenez commented Dec 2, 2024

azure-pipelines bot commented Dec 2, 2024

ivangilmercano commented Nov 29, 2024 •

edited

Loading

CLAassistant commented Nov 29, 2024 •

edited

Loading

CiprianRegus Nov 29, 2024 •

edited

Loading