diff mbox series

[2/2] hwmon: (pmbus/adp1050): add support for adp1051, adp1055 and ltp8800

Message ID 20241120035826.3920-3-cedricjustine.encarnacion@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for ADP1051/ADP1055 and LTP8800-1A/-2/-4A | expand

Commit Message

Cedric Encarnacion Nov. 20, 2024, 3:58 a.m. UTC
ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
    ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
    LTP8800-1A/-2/-4A: 150A/135A/200A DC/DC µModule Regulator

The LTP8800 is a family of step-down μModule regulators that provides
microprocessor core voltage from 54V power distribution architecture.
LTP8800 features telemetry monitoring of input/output voltage, input
current, output power, and temperature over PMBus.

Co-developed-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
---
 Documentation/hwmon/adp1050.rst | 63 +++++++++++++++++++++++++++--
 drivers/hwmon/pmbus/Kconfig     |  9 +++++
 drivers/hwmon/pmbus/adp1050.c   | 72 +++++++++++++++++++++++++++++++--
 3 files changed, 137 insertions(+), 7 deletions(-)

Comments

Randy Dunlap Nov. 20, 2024, 5 a.m. UTC | #1
On 11/19/24 7:58 PM, Cedric Encarnacion wrote:
>     ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
>     ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
>     LTP8800-1A/-2/-4A: 150A/135A/200A DC/DC µModule Regulator
> 
> The LTP8800 is a family of step-down μModule regulators that provides
> microprocessor core voltage from 54V power distribution architecture.
> LTP8800 features telemetry monitoring of input/output voltage, input
> current, output power, and temperature over PMBus.
> 
> Co-developed-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> ---
>  Documentation/hwmon/adp1050.rst | 63 +++++++++++++++++++++++++++--
>  drivers/hwmon/pmbus/Kconfig     |  9 +++++
>  drivers/hwmon/pmbus/adp1050.c   | 72 +++++++++++++++++++++++++++++++--
>  3 files changed, 137 insertions(+), 7 deletions(-)
> 

> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index f6d352841953..5d03a307824e 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -67,6 +67,15 @@ config SENSORS_ADP1050
>  	  This driver can also be built as a module. If so, the module will
>  	  be called adp1050.
>  
> +config SENSORS_ADP1050_REGULATOR
> +	bool "Regulator support for ADP1050 and compatibles"
> +	depends on SENSORS_ADP1050 && REGULATOR
> +	help
> +	  If you say yes here you get regulator support for Analog Devices
> +	  LTP8800-1A, LTP8800-4A, and LTP8800-2. LTP8800 is a family of DC/DC
> +	  µModule regulators that can provide microprocessor power from 54V
> +	  power distribution architecture.
> +
>  config SENSORS_BEL_PFE
>  	tristate "Bel PFE Compatible Power Supplies"
>  	help

FYI:

The 'micro' symbol displays as a blank space in 'menuconfig' or 'nconfig'.
(It shows up correctly in gconfig and xconfig.)

This problem is not unique to this driver entry.
See https://lore.kernel.org/all/20231006202942.GA865945@bhelgaas/ from 2023.

AFAIK no one is working on this issue.
Feel free to change the help text or leave it...
Andy Shevchenko Nov. 20, 2024, 1:45 p.m. UTC | #2
On Tue, Nov 19, 2024 at 09:00:28PM -0800, Randy Dunlap wrote:
> On 11/19/24 7:58 PM, Cedric Encarnacion wrote:

...

> > +config SENSORS_ADP1050_REGULATOR
> > +	bool "Regulator support for ADP1050 and compatibles"
> > +	depends on SENSORS_ADP1050 && REGULATOR
> > +	help
> > +	  If you say yes here you get regulator support for Analog Devices
> > +	  LTP8800-1A, LTP8800-4A, and LTP8800-2. LTP8800 is a family of DC/DC
> > +	  µModule regulators that can provide microprocessor power from 54V
> > +	  power distribution architecture.

> FYI:
> 
> The 'micro' symbol displays as a blank space in 'menuconfig' or 'nconfig'.
> (It shows up correctly in gconfig and xconfig.)
> 
> This problem is not unique to this driver entry.
> See https://lore.kernel.org/all/20231006202942.GA865945@bhelgaas/ from 2023.
> 
> AFAIK no one is working on this issue.
> Feel free to change the help text or leave it...

If it's part of the commercial / official name, I would leave it.
The bug is not in the help text anyway.
Andy Shevchenko Nov. 20, 2024, 1:52 p.m. UTC | #3
On Wed, Nov 20, 2024 at 11:58:26AM +0800, Cedric Encarnacion wrote:

I would start the commit message with the plain English sentence that describes
the list given below. E.g., "Introduce support for the following components:".

>     ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
>     ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
>     LTP8800-1A/-2/-4A: 150A/135A/200A DC/DC µModule Regulator
> 
> The LTP8800 is a family of step-down μModule regulators that provides
> microprocessor core voltage from 54V power distribution architecture.
> LTP8800 features telemetry monitoring of input/output voltage, input
> current, output power, and temperature over PMBus.

...

>    - Radu Sabau <radu.sabau@analog.com>
>  
> -
>  Description
>  -----------

Stray change.

...

> -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™

Can we use small x to make it more visible that it's _not_ the part of the
name, but a glob-like placeholder?

>  interface targeting high density, high efficiency dc-to-dc power
>  conversion used to monitor system temperatures, voltages and currents.

...

> +#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)

Why? Is the data type undefined without this?

> +static const struct regulator_desc adp1050_reg_desc[] = {
> +	PMBUS_REGULATOR_ONE("vout"),
> +};
> +#endif /* CONFIG_SENSORS_ADP1050_REGULATOR */

Note, this can be dropped anyway in order to use PTR_IF() below, if required.

...

> +#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)
> +	.num_regulators = 1,
> +	.reg_desc = adp1050_reg_desc,
> +#endif

Ditto, are the fields not defined without the symbol?

...

>  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);

Why not i2c_get_match_data()?

> +	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},

Please, split this patch to at least two:
1) Introduce chip_info;
2) add new devices.

> +	{ .name = "adp1051", .driver_data = (kernel_ulong_t)&adp1051_info},
> +	{ .name = "adp1055", .driver_data = (kernel_ulong_t)&adp1055_info},
> +	{ .name = "ltp8800", .driver_data = (kernel_ulong_t)&ltp8800_info},
>  	{}
>  };
Guenter Roeck Nov. 20, 2024, 2:39 p.m. UTC | #4
On 11/19/24 19:58, Cedric Encarnacion wrote:
>      ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
>      ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
>      LTP8800-1A/-2/-4A: 150A/135A/200A DC/DC µModule Regulator
> 
> The LTP8800 is a family of step-down μModule regulators that provides
> microprocessor core voltage from 54V power distribution architecture.
> LTP8800 features telemetry monitoring of input/output voltage, input
> current, output power, and temperature over PMBus.
> 
> Co-developed-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> ---
>   Documentation/hwmon/adp1050.rst | 63 +++++++++++++++++++++++++++--
>   drivers/hwmon/pmbus/Kconfig     |  9 +++++
>   drivers/hwmon/pmbus/adp1050.c   | 72 +++++++++++++++++++++++++++++++--
>   3 files changed, 137 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/hwmon/adp1050.rst b/Documentation/hwmon/adp1050.rst
> index 8fa937064886..1692373ee5af 100644
> --- a/Documentation/hwmon/adp1050.rst
> +++ b/Documentation/hwmon/adp1050.rst
> @@ -13,18 +13,43 @@ 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
> +
> +  * Analog Devices LTP8800-1A/-2/-4A
> +
> +    Prefix: 'ltp8800'
> +
> +    Addresses scanned: -
> +
> +    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/LTP8800-1A.pdf
> +         https://www.analog.com/media/en/technical-documentation/data-sheets/LTP8800-2.pdf
> +         https://www.analog.com/media/en/technical-documentation/data-sheets/LTP8800-4A.pdf
> +
>   Authors:
>   
>     - Radu Sabau <radu.sabau@analog.com>
>   
> -

Unrelated

>   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™

Please refrain from using device name wildcards, There is no guarantee that
all chips in the name range of ADP105[0-9] will have the same functionality.
Either name the chips, or say something like "The supported chips are ...".

Guenter
Guenter Roeck Nov. 20, 2024, 2:53 p.m. UTC | #5
On 11/20/24 05:52, Andy Shevchenko wrote:
> On Wed, Nov 20, 2024 at 11:58:26AM +0800, Cedric Encarnacion wrote:
> 
> I would start the commit message with the plain English sentence that describes
> the list given below. E.g., "Introduce support for the following components:".
> 
>>      ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
>>      ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
>>      LTP8800-1A/-2/-4A: 150A/135A/200A DC/DC µModule Regulator
>>
>> The LTP8800 is a family of step-down μModule regulators that provides
>> microprocessor core voltage from 54V power distribution architecture.
>> LTP8800 features telemetry monitoring of input/output voltage, input
>> current, output power, and temperature over PMBus.
> 
> ...
> 
>>     - Radu Sabau <radu.sabau@analog.com>
>>   
>> -
>>   Description
>>   -----------
> 
> Stray change.
> 
> ...
> 
>> -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™
> 
> Can we use small x to make it more visible that it's _not_ the part of the
> name, but a glob-like placeholder?
> 

As mentioned in my other reply, I'd rather not have a placeholder in the first
place.

>>   interface targeting high density, high efficiency dc-to-dc power
>>   conversion used to monitor system temperatures, voltages and currents.
> 
> ...
> 
>> +#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)
> 
> Why? Is the data type undefined without this?
> 

Look into other drivers. That is how it is implemented there,
and not really the point. One has to know about an alternative to use it.

>> +static const struct regulator_desc adp1050_reg_desc[] = {
>> +	PMBUS_REGULATOR_ONE("vout"),
>> +};
>> +#endif /* CONFIG_SENSORS_ADP1050_REGULATOR */
> 
> Note, this can be dropped anyway in order to use PTR_IF() below, if required.
> 

FWIW, PTR_IF() isn't widely used, and I for my part was not aware that
it exists.

> ...
> 
>> +#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)
>> +	.num_regulators = 1,
>> +	.reg_desc = adp1050_reg_desc,
>> +#endif
> 
> Ditto, are the fields not defined without the symbol?
> 

They are, but they must be 0/NULL. PTR_IF() would be an alternative.
It is a bit odd to use it for a non-pointer, but it is type-agnostic,
so using it should be ok to avoid the #ifdefs. We should maybe adopt
that mechanism for other PMBus drivers.

> ...
> 
>>   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);
> 
> Why not i2c_get_match_data()?
> 
>> +	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},
> 
> Please, split this patch to at least two:
> 1) Introduce chip_info;

That would really be "Use driver data to point to chip info".

> 2) add new devices.
> 

I don't really care much about separating those two (after all, they are
related), but adding regulator support to the driver is a major change
and should be a separate patch. On top of that, it isn't even mentioned
in the patch description.

Thanks,
Guenter
Andy Shevchenko Nov. 20, 2024, 4:29 p.m. UTC | #6
On Wed, Nov 20, 2024 at 06:53:58AM -0800, Guenter Roeck wrote:
> On 11/20/24 05:52, Andy Shevchenko wrote:
> > On Wed, Nov 20, 2024 at 11:58:26AM +0800, Cedric Encarnacion wrote:

...

> > > +#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)
> > 
> > Why? Is the data type undefined without this?
> 
> Look into other drivers. That is how it is implemented there,
> and not really the point. One has to know about an alternative to use it.
> 
> > > +static const struct regulator_desc adp1050_reg_desc[] = {
> > > +	PMBUS_REGULATOR_ONE("vout"),
> > > +};
> > > +#endif /* CONFIG_SENSORS_ADP1050_REGULATOR */
> > 
> > Note, this can be dropped anyway in order to use PTR_IF() below, if required.
> 
> FWIW, PTR_IF() isn't widely used, and I for my part was not aware that
> it exists.

Yeah, it's a relatively new one...

...

> > > +#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)
> > > +	.num_regulators = 1,
> > > +	.reg_desc = adp1050_reg_desc,
> > > +#endif
> > 
> > Ditto, are the fields not defined without the symbol?
> 
> They are, but they must be 0/NULL. PTR_IF() would be an alternative.
> It is a bit odd to use it for a non-pointer, but it is type-agnostic,
> so using it should be ok to avoid the #ifdefs. We should maybe adopt
> that mechanism for other PMBus drivers.

I see, thanks for elaboration on all of this.

...

> > Please, split this patch to at least two:
> > 1) Introduce chip_info;
> 
> That would really be "Use driver data to point to chip info".

I agree on the title, what I meant is the rough description of what
should be done in the change.

> > 2) add new devices.
> 
> I don't really care much about separating those two (after all, they are
> related), but adding regulator support to the driver is a major change
> and should be a separate patch. On top of that, it isn't even mentioned
> in the patch description.

Indeed, that's why I mentioned "at least" in the reply.
kernel test robot Nov. 21, 2024, 9:54 p.m. UTC | #7
Hi Cedric,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on linus/master v6.12 next-20241121]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cedric-Encarnacion/dt-bindings-hwmon-pmbus-adp1050-Add-bindings-for-adp1051-adp1055-and-ltp8800/20241121-144856
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20241120035826.3920-3-cedricjustine.encarnacion%40analog.com
patch subject: [PATCH 2/2] hwmon: (pmbus/adp1050): add support for adp1051, adp1055 and ltp8800
config: loongarch-randconfig-r064-20241122 (https://download.01.org/0day-ci/archive/20241122/202411220500.414mHL27-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241122/202411220500.414mHL27-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/202411220500.414mHL27-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/hwmon/pmbus/adp1050.c: In function 'adp1050_probe':
>> drivers/hwmon/pmbus/adp1050.c:88:39: warning: passing argument 2 of 'pmbus_do_probe' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      88 |         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 +88 drivers/hwmon/pmbus/adp1050.c

    79	
    80	static int adp1050_probe(struct i2c_client *client)
    81	{
    82		const struct pmbus_driver_info *info;
    83	
    84		info = device_get_match_data(&client->dev);
    85		if (!info)
    86			return -ENODEV;
    87	
  > 88		return pmbus_do_probe(client, info);
    89	}
    90
kernel test robot Nov. 22, 2024, 1:46 p.m. UTC | #8
Hi Cedric,

kernel test robot noticed the following build errors:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on linus/master v6.12 next-20241122]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cedric-Encarnacion/dt-bindings-hwmon-pmbus-adp1050-Add-bindings-for-adp1051-adp1055-and-ltp8800/20241121-144856
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20241120035826.3920-3-cedricjustine.encarnacion%40analog.com
patch subject: [PATCH 2/2] hwmon: (pmbus/adp1050): add support for adp1051, adp1055 and ltp8800
config: i386-buildonly-randconfig-003-20241122 (https://download.01.org/0day-ci/archive/20241122/202411222109.6PmpUvSa-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/20241122/202411222109.6PmpUvSa-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/202411222109.6PmpUvSa-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:88:32: error: passing 'const struct pmbus_driver_info *' to parameter of type 'struct pmbus_driver_info *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
      88 |         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 +88 drivers/hwmon/pmbus/adp1050.c

    79	
    80	static int adp1050_probe(struct i2c_client *client)
    81	{
    82		const struct pmbus_driver_info *info;
    83	
    84		info = device_get_match_data(&client->dev);
    85		if (!info)
    86			return -ENODEV;
    87	
  > 88		return pmbus_do_probe(client, info);
    89	}
    90
kernel test robot Nov. 23, 2024, 3:11 a.m. UTC | #9
Hi Cedric,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on linus/master v6.12 next-20241122]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cedric-Encarnacion/dt-bindings-hwmon-pmbus-adp1050-Add-bindings-for-adp1051-adp1055-and-ltp8800/20241121-144856
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20241120035826.3920-3-cedricjustine.encarnacion%40analog.com
patch subject: [PATCH 2/2] hwmon: (pmbus/adp1050): add support for adp1051, adp1055 and ltp8800
config: loongarch-randconfig-r111-20241122 (https://download.01.org/0day-ci/archive/20241123/202411231030.hhgCtzrl-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20241123/202411231030.hhgCtzrl-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/202411231030.hhgCtzrl-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/hwmon/pmbus/adp1050.c:88: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:88:39: sparse:     expected struct pmbus_driver_info *info
   drivers/hwmon/pmbus/adp1050.c:88:39: sparse:     got struct pmbus_driver_info const *[assigned] info

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

    79	
    80	static int adp1050_probe(struct i2c_client *client)
    81	{
    82		const struct pmbus_driver_info *info;
    83	
    84		info = device_get_match_data(&client->dev);
    85		if (!info)
    86			return -ENODEV;
    87	
  > 88		return pmbus_do_probe(client, info);
    89	}
    90
diff mbox series

Patch

diff --git a/Documentation/hwmon/adp1050.rst b/Documentation/hwmon/adp1050.rst
index 8fa937064886..1692373ee5af 100644
--- a/Documentation/hwmon/adp1050.rst
+++ b/Documentation/hwmon/adp1050.rst
@@ -13,18 +13,43 @@  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
+
+  * Analog Devices LTP8800-1A/-2/-4A
+
+    Prefix: 'ltp8800'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/LTP8800-1A.pdf
+         https://www.analog.com/media/en/technical-documentation/data-sheets/LTP8800-2.pdf
+         https://www.analog.com/media/en/technical-documentation/data-sheets/LTP8800-4A.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 +74,46 @@  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_alarm	  Output current alarm
+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/Kconfig b/drivers/hwmon/pmbus/Kconfig
index f6d352841953..5d03a307824e 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -67,6 +67,15 @@  config SENSORS_ADP1050
 	  This driver can also be built as a module. If so, the module will
 	  be called adp1050.
 
+config SENSORS_ADP1050_REGULATOR
+	bool "Regulator support for ADP1050 and compatibles"
+	depends on SENSORS_ADP1050 && REGULATOR
+	help
+	  If you say yes here you get regulator support for Analog Devices
+	  LTP8800-1A, LTP8800-4A, and LTP8800-2. LTP8800 is a family of DC/DC
+	  µModule regulators that can provide microprocessor power from 54V
+	  power distribution architecture.
+
 config SENSORS_BEL_PFE
 	tristate "Bel PFE Compatible Power Supplies"
 	help
diff --git a/drivers/hwmon/pmbus/adp1050.c b/drivers/hwmon/pmbus/adp1050.c
index 20f22730fc01..46f2dda8adbb 100644
--- a/drivers/hwmon/pmbus/adp1050.c
+++ b/drivers/hwmon/pmbus/adp1050.c
@@ -11,6 +11,12 @@ 
 
 #include "pmbus.h"
 
+#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)
+static const struct regulator_desc adp1050_reg_desc[] = {
+	PMBUS_REGULATOR_ONE("vout"),
+};
+#endif /* CONFIG_SENSORS_ADP1050_REGULATOR */
+
 static struct pmbus_driver_info adp1050_info = {
 	.pages = 1,
 	.format[PSC_VOLTAGE_IN] = linear,
@@ -23,19 +29,79 @@  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 struct pmbus_driver_info ltp8800_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_POUT
+		| PMBUS_HAVE_STATUS_VOUT
+		| PMBUS_HAVE_STATUS_INPUT
+		| PMBUS_HAVE_STATUS_TEMP,
+#if IS_ENABLED(CONFIG_SENSORS_ADP1050_REGULATOR)
+	.num_regulators = 1,
+	.reg_desc = adp1050_reg_desc,
+#endif
+};
+
 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},
+	{ .name = "ltp8800", .driver_data = (kernel_ulong_t)&ltp8800_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},
+	{ .compatible = "adi,ltp8800", .data = &ltp8800_info},
 	{}
 };
 MODULE_DEVICE_TABLE(of, adp1050_of_match);