Re: [PATCH 3/3] hwmon: (pmbus) Add support for additional Flex BMR converters to pmbus

[Guenter Roeck]

On 5/5/21 11:32 AM, Erik Rosen wrote:
> Add support for Flex BMR310, BMR456, BMR457, BMR458, BMR480, BMR490,
> BMR491 and BMR492 to the pmbus driver
> 
> Signed-off-by: Erik Rosen <erik.rosen@xxxxxxxxxxxxx>
> ---
>  Documentation/hwmon/pmbus.rst | 11 +++++++----
>  drivers/hwmon/pmbus/Kconfig   |  7 ++++---
>  drivers/hwmon/pmbus/pmbus.c   | 24 ++++++++++++++++++++++--
>  3 files changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/hwmon/pmbus.rst b/Documentation/hwmon/pmbus.rst
> index c44f14115413..0514c3052d4a 100644
> --- a/Documentation/hwmon/pmbus.rst
> +++ b/Documentation/hwmon/pmbus.rst
> @@ -3,15 +3,18 @@ Kernel driver pmbus
>  
>  Supported chips:
>  
> -  * Ericsson BMR453, BMR454
> +  * Flex BMR453, BMR454, BMR456, BMR457, BMR458, BMR480,
> +    BMR490, BMR491, BMR310, BMR492
>  
> -    Prefixes: 'bmr453', 'bmr454'
> +    Prefixes: 'bmr453', 'bmr454', 'bmr456', 'bmr457', 'bmr458', 'bmr480',
> +    'bmr490', 'bmr491', 'bmr310', 'bmr492'
>  
>      Addresses scanned: -
>  
> -    Datasheet:
> +    Datasheets:
> +
> + 	https://flexpowermodules.com/products
>  
> - http://archive.ericsson.net/service/internet/picov/get?DocNo=28701-EN/LZT146395
>  
>    * ON Semiconductor ADP4000, NCP4200, NCP4208
>  
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 32d2fc850621..59080d142bf7 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -19,9 +19,10 @@ config SENSORS_PMBUS
>  	default y
>  	help
>  	  If you say yes here you get hardware monitoring support for generic
> -	  PMBus devices, including but not limited to ADP4000, BMR453, BMR454,
> -	  MAX20796, MDT040, NCP4200, NCP4208, PDT003, PDT006, PDT012, TPS40400,
> -	  TPS544B20, TPS544B25, TPS544C20, TPS544C25, and UDT020.
> +	  PMBus devices, including but not limited to ADP4000, BMR310, BMR453,
> +	  BMR454, BMR456, BMR457, BMR458, BMR480, BMR490, BMR491, BMR492,
> +	  MAX20796, MDT040, NCP4200, NCP4208, PDT003, PDT006, PDT012,
> +	  TPS40400, TPS544B20, TPS544B25, TPS544C20, TPS544C25, and UDT020.
>  
>  	  This driver can also be built as a module. If so, the module will
>  	  be called pmbus.
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index a1b4260e75b2..688c4a3a87e0 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -173,13 +173,18 @@ static int pmbus_probe(struct i2c_client *client)
>  		return -ENOMEM;
>  
>  	device_info = (struct pmbus_device_info *)i2c_match_id(pmbus_id, client)->driver_data;
> -	if (device_info->flags & PMBUS_SKIP_STATUS_CHECK) {
> +	if (device_info->flags & PMBUS_SKIP_STATUS_CHECK ||
> +	    device_info->flags & PMBUS_READ_STATUS_AFTER_FAILED_CHECK) {

I don't think it makes sense to skip WRITE_PROTECT here.
Just make this
	if (device_info->flags) {

>  		pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
>  				     GFP_KERNEL);
>  		if (!pdata)
>  			return -ENOMEM;
>  
> -		pdata->flags = PMBUS_SKIP_STATUS_CHECK;
> +		pdata->flags = 0;
> +		if (device_info->flags & PMBUS_SKIP_STATUS_CHECK)
> +			pdata->flags |= PMBUS_SKIP_STATUS_CHECK;
> +		if (device_info->flags & PMBUS_READ_STATUS_AFTER_FAILED_CHECK)
> +			pdata->flags |= PMBUS_READ_STATUS_AFTER_FAILED_CHECK;

and this
		pdata->flags = device_info->flags;

Guenter

>  	}
>  
>  	info->pages = device_info->pages;
> @@ -193,22 +198,37 @@ static const struct pmbus_device_info pmbus_info_one = {
>  	.pages = 1,
>  	.flags = 0
>  };
> +
>  static const struct pmbus_device_info pmbus_info_zero = {
>  	.pages = 0,
>  	.flags = 0
>  };
> +
>  static const struct pmbus_device_info pmbus_info_one_skip = {
>  	.pages = 1,
>  	.flags = PMBUS_SKIP_STATUS_CHECK
>  };
>  
> +static const struct pmbus_device_info pmbus_info_one_status = {
> +	.pages = 1,
> +	.flags = PMBUS_READ_STATUS_AFTER_FAILED_CHECK
> +};
> +
>  /*
>   * Use driver_data to set the number of pages supported by the chip.
>   */
>  static const struct i2c_device_id pmbus_id[] = {
>  	{"adp4000", (kernel_ulong_t)&pmbus_info_one},
> +	{"bmr310", (kernel_ulong_t)&pmbus_info_one_status},
>  	{"bmr453", (kernel_ulong_t)&pmbus_info_one},
>  	{"bmr454", (kernel_ulong_t)&pmbus_info_one},
> +	{"bmr456", (kernel_ulong_t)&pmbus_info_one},
> +	{"bmr457", (kernel_ulong_t)&pmbus_info_one},
> +	{"bmr458", (kernel_ulong_t)&pmbus_info_one_status},
> +	{"bmr480", (kernel_ulong_t)&pmbus_info_one_status},
> +	{"bmr490", (kernel_ulong_t)&pmbus_info_one_status},
> +	{"bmr491", (kernel_ulong_t)&pmbus_info_one_status},
> +	{"bmr492", (kernel_ulong_t)&pmbus_info_one},
>  	{"dps460", (kernel_ulong_t)&pmbus_info_one_skip},
>  	{"dps650ab", (kernel_ulong_t)&pmbus_info_one_skip},
>  	{"dps800", (kernel_ulong_t)&pmbus_info_one_skip},
>