[PATCH 0/2] Add support for ADP1051 and ADP1055

[PATCH 0/2] Add support for ADP1051 and ADP1055

[Alexis Cezar Torreno]

The ADP1051, and ADP1055 have 6 PWM for individual monitoring. ADP1051
can monitor input/output voltages, input/output currents, and temperature.
ADP1055 is similar and can also monitor power.

Alexis Cezar Torreno (2):
  dt-bindings: hwmon: (pmbus/adp1050): Support adp1051 and adp1055: add
    bindings.
  hwmon: (pmbus/adp1050): Support adp1051 and adp1055

 .../bindings/hwmon/pmbus/adi,adp1050.yaml     | 12 ++++-
 Documentation/hwmon/adp1050.rst               | 52 +++++++++++++++++--
 drivers/hwmon/pmbus/adp1050.c                 | 44 ++++++++++++++--
 3 files changed, 98 insertions(+), 10 deletions(-)


base-commit: aa8cbc0898902070f1ad093a6e036cf57f0d47bc
-- 
2.34.1

[PATCH 1/2] dt-bindings: hwmon: (pmbus/adp1050): Support adp1051 and adp1055: add bindings.

[Alexis Cezar Torreno]

Add dt-bindings for adp1051 and adp1055 pmbus.
ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature

Signed-off-by: Alexis Cezar Torreno <alexisczezar.torreno@analog.com>
---
 .../devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
index 10c2204bc3df..88aaa29b3bd1 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
@@ -10,16 +10,24 @@ maintainers:
   - Radu Sabau <radu.sabau@analog.com>
 
 description: |
-   The ADP1050 is used to monitor system voltages, currents and temperatures.
+   The ADP1050 and similar devices are used to monitor system voltages,
+   currents, power, and temperatures.
+
    Through the PMBus interface, the ADP1050 targets isolated power supplies
    and has four individual monitors for input/output voltage, input current
    and temperature.
    Datasheet:
      https://www.analog.com/en/products/adp1050.html
+     https://www.analog.com/en/products/adp1051.html
+     https://www.analog.com/en/products/adp1055.html
 
 properties:
+
   compatible:
-    const: adi,adp1050
+    enum:
+      - adi,adp1050
+      - adi,adp1051
+      - adi,adp1055
 
   reg:
     maxItems: 1
-- 
2.34.1

[PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Alexis Cezar Torreno]

ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
Signed-off-by: Alexis Cezar Torreno <alexisczezar.torreno@analog.com>
---
 Documentation/hwmon/adp1050.rst | 52 ++++++++++++++++++++++++++++++---
 drivers/hwmon/pmbus/adp1050.c   | 44 +++++++++++++++++++++++++---
 2 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/Documentation/hwmon/adp1050.rst b/Documentation/hwmon/adp1050.rst
index 8fa937064886..5e2edcbe0c59 100644
--- a/Documentation/hwmon/adp1050.rst
+++ b/Documentation/hwmon/adp1050.rst
@@ -13,18 +13,33 @@ Supported chips:
 
     Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADP1050.pdf
 
+  * Analog Devices ADP1051
+
+    Prefix: 'adp1051'
+
+    Addresses scanned: I2C 0x70 - 0x77
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADP1051.pdf
+
+  * Analog Devices ADP1055
+
+    Prefix: 'adp1055'
+
+    Addresses scanned: I2C 0x4B - 0x77
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADP1055.pdf
+
 Authors:
 
   - Radu Sabau <radu.sabau@analog.com>
 
-
 Description
 -----------
 
-This driver supprts hardware monitoring for Analog Devices ADP1050 Digital
-Controller for Isolated Power Supply with PMBus interface.
+This driver supports hardware monitoring for Analog Devices ADP1050, ADP1051, and
+ADP1055 Digital Controller for Isolated Power Supply with PMBus interface.
 
-The ADP1050 is an advanced digital controller with a PMBus™
+The ADP105X is an advanced digital controller with a PMBus™
 interface targeting high density, high efficiency dc-to-dc power
 conversion used to monitor system temperatures, voltages and currents.
 Through the PMBus interface, the device can monitor input/output voltages,
@@ -49,16 +64,45 @@ Sysfs Attributes
 in1_label         "vin"
 in1_input         Measured input voltage
 in1_alarm	  Input voltage alarm
+in1_crit          Critical maximum input voltage
+in1_crit_alarm    Input voltage high alarm
+in1_lcrit         Critical minimum input voltage
+in1_lcrit_alarm   Input voltage critical low alarm
 in2_label	  "vout1"
 in2_input	  Measured output voltage
 in2_crit	  Critical maximum output voltage
 in2_crit_alarm    Output voltage high alarm
 in2_lcrit	  Critical minimum output voltage
 in2_lcrit_alarm	  Output voltage critical low alarm
+in2_max           Critical maximum output voltage
+in2_max_alarm     Output voltage critical max alarm
+in2_min           Critical minimum output voltage
+in2_min_alarm     Output voltage critical min alarm
 curr1_label	  "iin"
 curr1_input	  Measured input current.
 curr1_alarm	  Input current alarm
+curr1_crit        Critical maximum input current
+curr1_crit_alarm  Input current high alarm
+curr2_label       "iout1"
+curr2_input       Measured output current
+curr2_crit        Critical maximum output current
+curr2_crit_alarm  Output current high alarm
+curr2_lcrit       Critical minimum output current
+curr2_lcrit_alarm Output current critical low alarm
+curr2_max         Critical maximum output current
+curr2_max_alarm   Output current critical max alarm
+power1_label      "pout1"
+power1_input      Measured output power
+power1_crit       Critical maximum output power
+power1_crit_alarm Output power high alarm
 temp1_input       Measured temperature
 temp1_crit	  Critical high temperature
 temp1_crit_alarm  Chip temperature critical high alarm
+temp1_max         Critical maximum temperature
+temp1_max_alarm   Temperature critical max alarm
+temp2_input       Measured temperature
+temp2_crit        Critical high temperature
+temp2_crit_alarm  Chip temperature critical high alarm
+temp2_max         Critical maximum temperature
+temp2_max_alarm   Temperature critical max alarm
 ================= ========================================
diff --git a/drivers/hwmon/pmbus/adp1050.c b/drivers/hwmon/pmbus/adp1050.c
index 20f22730fc01..db2054181d14 100644
--- a/drivers/hwmon/pmbus/adp1050.c
+++ b/drivers/hwmon/pmbus/adp1050.c
@@ -6,8 +6,8 @@
  */
 #include <linux/bits.h>
 #include <linux/i2c.h>
-#include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 
 #include "pmbus.h"
 
@@ -23,19 +23,55 @@ static struct pmbus_driver_info adp1050_info = {
 		| PMBUS_HAVE_STATUS_TEMP,
 };
 
+static struct pmbus_driver_info adp1051_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = linear,
+	.format[PSC_VOLTAGE_OUT] = linear,
+	.format[PSC_CURRENT_IN] = linear,
+	.format[PSC_TEMPERATURE] = linear,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT
+		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT
+		   | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT
+		   | PMBUS_HAVE_STATUS_TEMP,
+};
+
+static struct pmbus_driver_info adp1055_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = linear,
+	.format[PSC_VOLTAGE_OUT] = linear,
+	.format[PSC_CURRENT_IN] = linear,
+	.format[PSC_TEMPERATURE] = linear,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT
+		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3
+		   | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_VOUT
+		   | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT
+		   | PMBUS_HAVE_STATUS_TEMP,
+};
+
 static int adp1050_probe(struct i2c_client *client)
 {
-	return pmbus_do_probe(client, &adp1050_info);
+	const struct pmbus_driver_info *info;
+
+	info = device_get_match_data(&client->dev);
+	if (!info)
+		return -ENODEV;
+
+	return pmbus_do_probe(client, info);
 }
 
 static const struct i2c_device_id adp1050_id[] = {
-	{"adp1050"},
+	{ .name = "adp1050", .driver_data = (kernel_ulong_t)&adp1050_info},
+	{ .name = "adp1051", .driver_data = (kernel_ulong_t)&adp1051_info},
+	{ .name = "adp1055", .driver_data = (kernel_ulong_t)&adp1055_info},
 	{}
 };
+
 MODULE_DEVICE_TABLE(i2c, adp1050_id);
 
 static const struct of_device_id adp1050_of_match[] = {
-	{ .compatible = "adi,adp1050"},
+	{ .compatible = "adi,adp1050", .data = &adp1050_info},
+	{ .compatible = "adi,adp1051", .data = &adp1051_info},
+	{ .compatible = "adi,adp1055", .data = &adp1055_info},
 	{}
 };
 MODULE_DEVICE_TABLE(of, adp1050_of_match);
-- 
2.34.1

Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Andy Shevchenko]

On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno wrote:
> ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
> ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature

Missing blank line and perhaps you can add Datasheet: tag(s) for these HW?
(see `git log --no-merges --grep Datasheet:` for the example)

> Signed-off-by: Alexis Cezar Torreno <alexisczezar.torreno@analog.com>

...

> --- a/drivers/hwmon/pmbus/adp1050.c
> +++ b/drivers/hwmon/pmbus/adp1050.c
> @@ -6,8 +6,8 @@
>   */
>  #include <linux/bits.h>
>  #include <linux/i2c.h>
> -#include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  
>  #include "pmbus.h"

Stray change. This pure depends on the your `locale` settings.
The original one seems using en_US.UTF-8 and it's perfectly fine.

...

> +static struct pmbus_driver_info adp1051_info = {
> +	.pages = 1,
> +	.format[PSC_VOLTAGE_IN] = linear,
> +	.format[PSC_VOLTAGE_OUT] = linear,
> +	.format[PSC_CURRENT_IN] = linear,
> +	.format[PSC_TEMPERATURE] = linear,
> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT
> +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT
> +		   | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT
> +		   | PMBUS_HAVE_STATUS_TEMP,

I dunno if the other entries in the file are written in the same style, but
usual one is

	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT |
		   PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT |
		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT |
		   PMBUS_HAVE_STATUS_TEMP,

Or even more logically

	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
		   PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
		   PMBUS_HAVE_TEMP |
		   PMBUS_HAVE_STATUS_INPUT |
		   PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
		   PMBUS_HAVE_STATUS_TEMP,

> +};
> +
> +static struct pmbus_driver_info adp1055_info = {
> +	.pages = 1,
> +	.format[PSC_VOLTAGE_IN] = linear,
> +	.format[PSC_VOLTAGE_OUT] = linear,
> +	.format[PSC_CURRENT_IN] = linear,
> +	.format[PSC_TEMPERATURE] = linear,
> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT
> +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3
> +		   | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_VOUT
> +		   | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT
> +		   | PMBUS_HAVE_STATUS_TEMP,

Ditto.

> +};

...

>  static const struct i2c_device_id adp1050_id[] = {
> -	{"adp1050"},
> +	{ .name = "adp1050", .driver_data = (kernel_ulong_t)&adp1050_info},
> +	{ .name = "adp1051", .driver_data = (kernel_ulong_t)&adp1051_info},
> +	{ .name = "adp1055", .driver_data = (kernel_ulong_t)&adp1055_info},
>  	{}
>  };

> +

Stray blank line.

>  MODULE_DEVICE_TABLE(i2c, adp1050_id);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] dt-bindings: hwmon: (pmbus/adp1050): Support adp1051 and adp1055: add bindings.

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]

On Wed, Nov 06, 2024 at 05:03:10PM +0800, Alexis Cezar Torreno wrote:
> Add dt-bindings for adp1051 and adp1055 pmbus.
> ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
> ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
> 
> Signed-off-by: Alexis Cezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  .../devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> index 10c2204bc3df..88aaa29b3bd1 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> @@ -10,16 +10,24 @@ maintainers:
>    - Radu Sabau <radu.sabau@analog.com>
>  
>  description: |
> -   The ADP1050 is used to monitor system voltages, currents and temperatures.
> +   The ADP1050 and similar devices are used to monitor system voltages,
> +   currents, power, and temperatures.
> +
>     Through the PMBus interface, the ADP1050 targets isolated power supplies
>     and has four individual monitors for input/output voltage, input current
>     and temperature.
>     Datasheet:
>       https://www.analog.com/en/products/adp1050.html
> +     https://www.analog.com/en/products/adp1051.html
> +     https://www.analog.com/en/products/adp1055.html
>  
>  properties:
> +

That's an abnormal newline, leave it alone if you respin.

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

>    compatible:
> -    const: adi,adp1050
> +    enum:
> +      - adi,adp1050
> +      - adi,adp1051
> +      - adi,adp1055
>  
>    reg:
>      maxItems: 1
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Guenter Roeck]

On 11/6/24 03:24, Andy Shevchenko wrote:
> On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno wrote:
>> ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
>> ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
> 
> Missing blank line and perhaps you can add Datasheet: tag(s) for these HW?
> (see `git log --no-merges --grep Datasheet:` for the example)
> 

Is that an official tag ? Frankly, if so, I think it is quite useless
in the patch description because datasheet locations keep changing.
I think it is much better to provide a link in the driver documentation.

>> Signed-off-by: Alexis Cezar Torreno <alexisczezar.torreno@analog.com>
> 
> ...
> 
>> --- a/drivers/hwmon/pmbus/adp1050.c
>> +++ b/drivers/hwmon/pmbus/adp1050.c
>> @@ -6,8 +6,8 @@
>>    */
>>   #include <linux/bits.h>
>>   #include <linux/i2c.h>
>> -#include <linux/mod_devicetable.h>
>>   #include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>>   
>>   #include "pmbus.h"
> 
> Stray change. This pure depends on the your `locale` settings.
> The original one seems using en_US.UTF-8 and it's perfectly fine.
> 

Agreed.

> ...
> 
>> +static struct pmbus_driver_info adp1051_info = {
>> +	.pages = 1,
>> +	.format[PSC_VOLTAGE_IN] = linear,
>> +	.format[PSC_VOLTAGE_OUT] = linear,
>> +	.format[PSC_CURRENT_IN] = linear,
>> +	.format[PSC_TEMPERATURE] = linear,
>> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT
>> +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT
>> +		   | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT
>> +		   | PMBUS_HAVE_STATUS_TEMP,
> 
> I dunno if the other entries in the file are written in the same style, but
> usual one is
> 
> 	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT |
> 		   PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT |
> 		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT |
> 		   PMBUS_HAVE_STATUS_TEMP,
> 
> Or even more logically
> 
> 	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> 		   PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
> 		   PMBUS_HAVE_TEMP |
> 		   PMBUS_HAVE_STATUS_INPUT |
> 		   PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
> 		   PMBUS_HAVE_STATUS_TEMP,
> 
>> +};
>> +
>> +static struct pmbus_driver_info adp1055_info = {
>> +	.pages = 1,
>> +	.format[PSC_VOLTAGE_IN] = linear,
>> +	.format[PSC_VOLTAGE_OUT] = linear,
>> +	.format[PSC_CURRENT_IN] = linear,
>> +	.format[PSC_TEMPERATURE] = linear,
>> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT
>> +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3
>> +		   | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_VOUT
>> +		   | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT
>> +		   | PMBUS_HAVE_STATUS_TEMP,
> 
> Ditto.
> 

That one slipped through with the original driver submission.
I thought that checkpatch complains about that, but it turns out that
it doesn't. I agree, though, that the usual style should be used.

Guenter

>> +};
> 
> ...
> 
>>   static const struct i2c_device_id adp1050_id[] = {
>> -	{"adp1050"},
>> +	{ .name = "adp1050", .driver_data = (kernel_ulong_t)&adp1050_info},
>> +	{ .name = "adp1051", .driver_data = (kernel_ulong_t)&adp1051_info},
>> +	{ .name = "adp1055", .driver_data = (kernel_ulong_t)&adp1055_info},
>>   	{}
>>   };
> 
>> +
> 
> Stray blank line.
> 
>>   MODULE_DEVICE_TABLE(i2c, adp1050_id);
> 

Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Andy Shevchenko]

On Wed, Nov 06, 2024 at 07:55:30AM -0800, Guenter Roeck wrote:
> On 11/6/24 03:24, Andy Shevchenko wrote:
> > On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno wrote:
> > > ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
> > > ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
> > 
> > Missing blank line and perhaps you can add Datasheet: tag(s) for these HW?
> > (see `git log --no-merges --grep Datasheet:` for the example)
> 
> Is that an official tag ? Frankly, if so, I think it is quite useless
> in the patch description because datasheet locations keep changing.
> I think it is much better to provide a link in the driver documentation.

I believe it's semi-official, meaning that people use it from time to time.
I'm fine with the Link in the documentation. Actually with any solution that
saves the respective link in the kernel source tree (either in form of commit
message or documentation / comments in the code).

...

> > > +static struct pmbus_driver_info adp1055_info = {
> > > +	.pages = 1,
> > > +	.format[PSC_VOLTAGE_IN] = linear,
> > > +	.format[PSC_VOLTAGE_OUT] = linear,
> > > +	.format[PSC_CURRENT_IN] = linear,
> > > +	.format[PSC_TEMPERATURE] = linear,
> > > +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_VOUT
> > > +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3
> > > +		   | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_VOUT
> > > +		   | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT
> > > +		   | PMBUS_HAVE_STATUS_TEMP,
> > 
> > Ditto.
> 
> That one slipped through with the original driver submission.
> I thought that checkpatch complains about that, but it turns out that
> it doesn't. I agree, though, that the usual style should be used.

Oh, okay, that's up to you then.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[kernel test robot]

Hi Alexis,

kernel test robot noticed the following build errors:

[auto build test ERROR on aa8cbc0898902070f1ad093a6e036cf57f0d47bc]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexis-Cezar-Torreno/dt-bindings-hwmon-pmbus-adp1050-Support-adp1051-and-adp1055-add-bindings/20241106-170853
base:   aa8cbc0898902070f1ad093a6e036cf57f0d47bc
patch link:    https://lore.kernel.org/r/20241106090311.17536-3-alexisczezar.torreno%40analog.com
patch subject: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055
config: i386-randconfig-141-20241106 (https://download.01.org/0day-ci/archive/20241107/202411070008.3X7zgKXO-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241107/202411070008.3X7zgKXO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411070008.3X7zgKXO-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/hwmon/pmbus/adp1050.c:8:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/hwmon/pmbus/adp1050.c:59:32: error: passing 'const struct pmbus_driver_info *' to parameter of type 'struct pmbus_driver_info *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
      59 |         return pmbus_do_probe(client, info);
         |                                       ^~~~
   drivers/hwmon/pmbus/pmbus.h:541:73: note: passing argument to parameter 'info' here
     541 | int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info);
         |                                                                         ^
   1 warning and 1 error generated.


vim +59 drivers/hwmon/pmbus/adp1050.c

    50	
    51	static int adp1050_probe(struct i2c_client *client)
    52	{
    53		const struct pmbus_driver_info *info;
    54	
    55		info = device_get_match_data(&client->dev);
    56		if (!info)
    57			return -ENODEV;
    58	
  > 59		return pmbus_do_probe(client, info);
    60	}
    61	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[kernel test robot]

Hi Alexis,

kernel test robot noticed the following build warnings:

[auto build test WARNING on aa8cbc0898902070f1ad093a6e036cf57f0d47bc]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexis-Cezar-Torreno/dt-bindings-hwmon-pmbus-adp1050-Support-adp1051-and-adp1055-add-bindings/20241106-170853
base:   aa8cbc0898902070f1ad093a6e036cf57f0d47bc
patch link:    https://lore.kernel.org/r/20241106090311.17536-3-alexisczezar.torreno%40analog.com
patch subject: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20241107/202411070017.nDsv8lMO-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241107/202411070017.nDsv8lMO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411070017.nDsv8lMO-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/hwmon/pmbus/adp1050.c: In function 'adp1050_probe':
>> drivers/hwmon/pmbus/adp1050.c:59:39: warning: passing argument 2 of 'pmbus_do_probe' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      59 |         return pmbus_do_probe(client, info);
         |                                       ^~~~
   In file included from drivers/hwmon/pmbus/adp1050.c:12:
   drivers/hwmon/pmbus/pmbus.h:541:73: note: expected 'struct pmbus_driver_info *' but argument is of type 'const struct pmbus_driver_info *'
     541 | int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info);
         |                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~


vim +59 drivers/hwmon/pmbus/adp1050.c

    50	
    51	static int adp1050_probe(struct i2c_client *client)
    52	{
    53		const struct pmbus_driver_info *info;
    54	
    55		info = device_get_match_data(&client->dev);
    56		if (!info)
    57			return -ENODEV;
    58	
  > 59		return pmbus_do_probe(client, info);
    60	}
    61	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[kernel test robot]

Hi Alexis,

kernel test robot noticed the following build warnings:

[auto build test WARNING on aa8cbc0898902070f1ad093a6e036cf57f0d47bc]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexis-Cezar-Torreno/dt-bindings-hwmon-pmbus-adp1050-Support-adp1051-and-adp1055-add-bindings/20241106-170853
base:   aa8cbc0898902070f1ad093a6e036cf57f0d47bc
patch link:    https://lore.kernel.org/r/20241106090311.17536-3-alexisczezar.torreno%40analog.com
patch subject: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055
config: x86_64-randconfig-r123-20241106 (https://download.01.org/0day-ci/archive/20241107/202411070427.lkz6nVFX-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241107/202411070427.lkz6nVFX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411070427.lkz6nVFX-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/hwmon/pmbus/adp1050.c:59:39: sparse: sparse: incorrect type in argument 2 (different modifiers) @@     expected struct pmbus_driver_info *info @@     got struct pmbus_driver_info const *[assigned] info @@
   drivers/hwmon/pmbus/adp1050.c:59:39: sparse:     expected struct pmbus_driver_info *info
   drivers/hwmon/pmbus/adp1050.c:59:39: sparse:     got struct pmbus_driver_info const *[assigned] info

vim +59 drivers/hwmon/pmbus/adp1050.c

    50	
    51	static int adp1050_probe(struct i2c_client *client)
    52	{
    53		const struct pmbus_driver_info *info;
    54	
    55		info = device_get_match_data(&client->dev);
    56		if (!info)
    57			return -ENODEV;
    58	
  > 59		return pmbus_do_probe(client, info);
    60	}
    61	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH 1/2] dt-bindings: hwmon: (pmbus/adp1050): Support adp1051 and adp1055: add bindings.

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Wednesday, November 6, 2024 11:49 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-hwmon@vger.kernel.org; Sabau, Radu
> bogdan <Radu.Sabau@analog.com>; Jean Delvare <jdelvare@suse.com>;
> Guenter Roeck <linux@roeck-us.net>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Andy
> Shevchenko <andriy.shevchenko@linux.intel.com>; Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de>
> Subject: Re: [PATCH 1/2] dt-bindings: hwmon: (pmbus/adp1050): Support
> adp1051 and adp1055: add bindings.
> 
> [External]
> 
> On Wed, Nov 06, 2024 at 05:03:10PM +0800, Alexis Cezar Torreno wrote:
> > Add dt-bindings for adp1051 and adp1055 pmbus.
> > ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
> > ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
> >
> > Signed-off-by: Alexis Cezar Torreno <alexisczezar.torreno@analog.com>
> > ---
> >  .../devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml | 12 ++++++++++-
> -
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git
> a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> > index 10c2204bc3df..88aaa29b3bd1 100644
> > --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> > +++
> b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> > @@ -10,16 +10,24 @@ maintainers:
> >    - Radu Sabau <radu.sabau@analog.com>
> >
> >  description: |
> > -   The ADP1050 is used to monitor system voltages, currents and
> temperatures.
> > +   The ADP1050 and similar devices are used to monitor system voltages,
> > +   currents, power, and temperatures.
> > +
> >     Through the PMBus interface, the ADP1050 targets isolated power supplies
> >     and has four individual monitors for input/output voltage, input current
> >     and temperature.
> >     Datasheet:
> >       https://www.analog.com/en/products/adp1050.html
> > +     https://www.analog.com/en/products/adp1051.html
> > +     https://www.analog.com/en/products/adp1055.html
> >
> >  properties:
> > +
> 
> That's an abnormal newline, leave it alone if you respin.
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Cheers,
> Conor.

Will remove/revert the newline. Thanks!

Regards,
Alexis

> 
> >    compatible:
> > -    const: adi,adp1050
> > +    enum:
> > +      - adi,adp1050
> > +      - adi,adp1051
> > +      - adi,adp1055
> >
> >    reg:
> >      maxItems: 1
> > --
> > 2.34.1
> >

RE: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Thursday, November 7, 2024 12:01 AM
> To: Guenter Roeck <linux@roeck-us.net>
> Cc: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-hwmon@vger.kernel.org; Sabau, Radu
> bogdan <Radu.Sabau@analog.com>; Jean Delvare <jdelvare@suse.com>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Uwe
> Kleine-König <u.kleine-koenig@pengutronix.de>
> Subject: Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and
> adp1055
> 
> [External]
> 
> On Wed, Nov 06, 2024 at 07:55:30AM -0800, Guenter Roeck wrote:
> > On 11/6/24 03:24, Andy Shevchenko wrote:
> > > On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno wrote:
> > > > ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
> > > > ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
> > >
> > > Missing blank line and perhaps you can add Datasheet: tag(s) for these HW?
> > > (see `git log --no-merges --grep Datasheet:` for the example)
> >
> > Is that an official tag ? Frankly, if so, I think it is quite useless
> > in the patch description because datasheet locations keep changing.
> > I think it is much better to provide a link in the driver documentation.
> 
> I believe it's semi-official, meaning that people use it from time to time.
> I'm fine with the Link in the documentation. Actually with any solution that
> saves the respective link in the kernel source tree (either in form of commit
> message or documentation / comments in the code).
> 

Will add the blank line after description. 
Am I right to understand that we leave this as is? No need to add driver link
in patch description since it is in driver documentation?

> ...
> 
> > > > +static struct pmbus_driver_info adp1055_info = {
> > > > +	.pages = 1,
> > > > +	.format[PSC_VOLTAGE_IN] = linear,
> > > > +	.format[PSC_VOLTAGE_OUT] = linear,
> > > > +	.format[PSC_CURRENT_IN] = linear,
> > > > +	.format[PSC_TEMPERATURE] = linear,
> > > > +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> PMBUS_HAVE_VOUT
> > > > +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 |
> PMBUS_HAVE_TEMP3
> > > > +		   | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_VOUT
> > > > +		   | PMBUS_HAVE_STATUS_IOUT |
> PMBUS_HAVE_STATUS_INPUT
> > > > +		   | PMBUS_HAVE_STATUS_TEMP,
> > >
> > > Ditto.
> >
> > That one slipped through with the original driver submission.
> > I thought that checkpatch complains about that, but it turns out that
> > it doesn't. I agree, though, that the usual style should be used.
> 
> Oh, okay, that's up to you then.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

I based my code style on the original, but I agree that the usual style
should be followed.  
I will change it to follow the usual style.
Should I leave the original untouched or should I format it too?

Regards,
Alexis

RE: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Wednesday, November 6, 2024 11:56 PM
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Torreno, Alexis
> Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-hwmon@vger.kernel.org; Sabau, Radu
> bogdan <Radu.Sabau@analog.com>; Jean Delvare <jdelvare@suse.com>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Uwe
> Kleine-König <u.kleine-koenig@pengutronix.de>
> Subject: Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and
> adp1055
> 
> [External]
> 
> On 11/6/24 03:24, Andy Shevchenko wrote:
> > On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno wrote:
> >> ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
> >> ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
> >
> > Missing blank line and perhaps you can add Datasheet: tag(s) for these HW?
> > (see `git log --no-merges --grep Datasheet:` for the example)
> >
> 
> Is that an official tag ? Frankly, if so, I think it is quite useless in the patch
> description because datasheet locations keep changing.
> I think it is much better to provide a link in the driver documentation.
> 
> >> Signed-off-by: Alexis Cezar Torreno <alexisczezar.torreno@analog.com>
> >
> > ...
> >
> >> --- a/drivers/hwmon/pmbus/adp1050.c
> >> +++ b/drivers/hwmon/pmbus/adp1050.c
> >> @@ -6,8 +6,8 @@
> >>    */
> >>   #include <linux/bits.h>
> >>   #include <linux/i2c.h>
> >> -#include <linux/mod_devicetable.h>
> >>   #include <linux/module.h>
> >> +#include <linux/mod_devicetable.h>
> >>
> >>   #include "pmbus.h"
> >
> > Stray change. This pure depends on the your `locale` settings.
> > The original one seems using en_US.UTF-8 and it's perfectly fine.
> >
> 
> Agreed.

Will revert, I surprisingly don't remember touching this.
Thanks!

> 
> > ...
> >
> >> +static struct pmbus_driver_info adp1051_info = {
> >> +	.pages = 1,
> >> +	.format[PSC_VOLTAGE_IN] = linear,
> >> +	.format[PSC_VOLTAGE_OUT] = linear,
> >> +	.format[PSC_CURRENT_IN] = linear,
> >> +	.format[PSC_TEMPERATURE] = linear,
> >> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> PMBUS_HAVE_VOUT
> >> +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP |
> PMBUS_HAVE_STATUS_VOUT
> >> +		   | PMBUS_HAVE_STATUS_IOUT |
> PMBUS_HAVE_STATUS_INPUT
> >> +		   | PMBUS_HAVE_STATUS_TEMP,
> >
> > I dunno if the other entries in the file are written in the same
> > style, but usual one is
> >
> > 	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> PMBUS_HAVE_VOUT |
> > 		   PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP |
> PMBUS_HAVE_STATUS_VOUT |
> > 		   PMBUS_HAVE_STATUS_IOUT |
> PMBUS_HAVE_STATUS_INPUT |
> > 		   PMBUS_HAVE_STATUS_TEMP,
> >
> > Or even more logically
> >
> > 	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> > 		   PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
> > 		   PMBUS_HAVE_TEMP |
> > 		   PMBUS_HAVE_STATUS_INPUT |
> > 		   PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
> |
> > 		   PMBUS_HAVE_STATUS_TEMP,
> >
> >> +};
> >> +
> >> +static struct pmbus_driver_info adp1055_info = {
> >> +	.pages = 1,
> >> +	.format[PSC_VOLTAGE_IN] = linear,
> >> +	.format[PSC_VOLTAGE_OUT] = linear,
> >> +	.format[PSC_CURRENT_IN] = linear,
> >> +	.format[PSC_TEMPERATURE] = linear,
> >> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> PMBUS_HAVE_VOUT
> >> +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 |
> PMBUS_HAVE_TEMP3
> >> +		   | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_VOUT
> >> +		   | PMBUS_HAVE_STATUS_IOUT |
> PMBUS_HAVE_STATUS_INPUT
> >> +		   | PMBUS_HAVE_STATUS_TEMP,
> >
> > Ditto.
> >
> 
> That one slipped through with the original driver submission.
> I thought that checkpatch complains about that, but it turns out that it doesn't.
> I agree, though, that the usual style should be used.
> 
> Guenter
> 
> >> +};
> >
> > ...
> >
> >>   static const struct i2c_device_id adp1050_id[] = {
> >> -	{"adp1050"},
> >> +	{ .name = "adp1050", .driver_data = (kernel_ulong_t)&adp1050_info},
> >> +	{ .name = "adp1051", .driver_data = (kernel_ulong_t)&adp1051_info},
> >> +	{ .name = "adp1055", .driver_data = (kernel_ulong_t)&adp1055_info},
> >>   	{}
> >>   };
> >
> >> +
> >
> > Stray blank line.

Will remove/revert. 

Regards,
Alexis

> >
> >>   MODULE_DEVICE_TABLE(i2c, adp1050_id);
> >

Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Andy Shevchenko]

On Thu, Nov 07, 2024 at 01:17:04AM +0000, Torreno, Alexis Czezar wrote:
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: Thursday, November 7, 2024 12:01 AM
> > On Wed, Nov 06, 2024 at 07:55:30AM -0800, Guenter Roeck wrote:
> > > On 11/6/24 03:24, Andy Shevchenko wrote:
> > > > On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno wrote:

...

> > > Is that an official tag ? Frankly, if so, I think it is quite useless
> > > in the patch description because datasheet locations keep changing.
> > > I think it is much better to provide a link in the driver documentation.
> > 
> > I believe it's semi-official, meaning that people use it from time to time.
> > I'm fine with the Link in the documentation. Actually with any solution that
> > saves the respective link in the kernel source tree (either in form of commit
> > message or documentation / comments in the code).
> 
> Will add the blank line after description. 
> Am I right to understand that we leave this as is? No need to add driver link
> in patch description since it is in driver documentation?

Add it to the documentation.

...

> > > > > +static struct pmbus_driver_info adp1055_info = {
> > > > > +	.pages = 1,
> > > > > +	.format[PSC_VOLTAGE_IN] = linear,
> > > > > +	.format[PSC_VOLTAGE_OUT] = linear,
> > > > > +	.format[PSC_CURRENT_IN] = linear,
> > > > > +	.format[PSC_TEMPERATURE] = linear,
> > > > > +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> > PMBUS_HAVE_VOUT
> > > > > +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 |
> > PMBUS_HAVE_TEMP3
> > > > > +		   | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_VOUT
> > > > > +		   | PMBUS_HAVE_STATUS_IOUT |
> > PMBUS_HAVE_STATUS_INPUT
> > > > > +		   | PMBUS_HAVE_STATUS_TEMP,
> > > >
> > > > Ditto.
> > >
> > > That one slipped through with the original driver submission.
> > > I thought that checkpatch complains about that, but it turns out that
> > > it doesn't. I agree, though, that the usual style should be used.
> > 
> > Oh, okay, that's up to you then.

> I based my code style on the original, but I agree that the usual style
> should be followed.  

> I will change it to follow the usual style.

No, please be consistent with the existing style. If you want to change it,
add an additional patch to do that for the _existing_ code first and base your
patch on top of that.

> Should I leave the original untouched or should I format it too?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Thursday, November 7, 2024 4:05 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Guenter Roeck <linux@roeck-us.net>; linux-doc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
> hwmon@vger.kernel.org; Sabau, Radu bogdan <Radu.Sabau@analog.com>;
> Jean Delvare <jdelvare@suse.com>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> Jonathan Corbet <corbet@lwn.net>; Uwe Kleine-König <u.kleine-
> koenig@pengutronix.de>
> Subject: Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and
> adp1055
> 
> [External]
> 
> On Thu, Nov 07, 2024 at 01:17:04AM +0000, Torreno, Alexis Czezar wrote:
> > > -----Original Message-----
> > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Sent: Thursday, November 7, 2024 12:01 AM On Wed, Nov 06, 2024 at
> > > 07:55:30AM -0800, Guenter Roeck wrote:
> > > > On 11/6/24 03:24, Andy Shevchenko wrote:
> > > > > On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno
> wrote:
> 
> ...
> 
> > > > Is that an official tag ? Frankly, if so, I think it is quite
> > > > useless in the patch description because datasheet locations keep
> changing.
> > > > I think it is much better to provide a link in the driver documentation.
> > >
> > > I believe it's semi-official, meaning that people use it from time to time.
> > > I'm fine with the Link in the documentation. Actually with any
> > > solution that saves the respective link in the kernel source tree
> > > (either in form of commit message or documentation / comments in the
> code).
> >
> > Will add the blank line after description.
> > Am I right to understand that we leave this as is? No need to add
> > driver link in patch description since it is in driver documentation?
> 
> Add it to the documentation.
> 

Already added the links for the datasheets in the documentation.

> ...
> 
> > > > > > +static struct pmbus_driver_info adp1055_info = {
> > > > > > +	.pages = 1,
> > > > > > +	.format[PSC_VOLTAGE_IN] = linear,
> > > > > > +	.format[PSC_VOLTAGE_OUT] = linear,
> > > > > > +	.format[PSC_CURRENT_IN] = linear,
> > > > > > +	.format[PSC_TEMPERATURE] = linear,
> > > > > > +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |
> > > PMBUS_HAVE_VOUT
> > > > > > +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 |
> > > PMBUS_HAVE_TEMP3
> > > > > > +		   | PMBUS_HAVE_POUT |
> PMBUS_HAVE_STATUS_VOUT
> > > > > > +		   | PMBUS_HAVE_STATUS_IOUT |
> > > PMBUS_HAVE_STATUS_INPUT
> > > > > > +		   | PMBUS_HAVE_STATUS_TEMP,
> > > > >
> > > > > Ditto.
> > > >
> > > > That one slipped through with the original driver submission.
> > > > I thought that checkpatch complains about that, but it turns out
> > > > that it doesn't. I agree, though, that the usual style should be used.
> > >
> > > Oh, okay, that's up to you then.
> 
> > I based my code style on the original, but I agree that the usual
> > style should be followed.
> 
> > I will change it to follow the usual style.
> 
> No, please be consistent with the existing style. If you want to change it, add an
> additional patch to do that for the _existing_ code first and base your patch on
> top of that.
> 

I see, I'll keep it consistent with the existing style.

Thank you, will send an updated patch soon.

Regards,
Alexis

> > Should I leave the original untouched or should I format it too?
> 
> --
> With Best Regards,
> Andy Shevchenko
>