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@xxxxxxxxxx>

...

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