[PATCH] hwmon: (pmbus/bel-pfe) Enable PMBUS_SKIP_STATUS_CHECK for pfe1100

[PATCH] hwmon: (pmbus/bel-pfe) Enable PMBUS_SKIP_STATUS_CHECK for pfe1100

[rentao.bupt]

From: Tao Ren <rentao.bupt@gmail.com>

Enable PMBUS_SKIP_STATUS_CHECK flag for both pfe1100 and pfe3000 because
the similar communication error is observed on pfe1100 devices.

Signed-off-by: Tao Ren <rentao.bupt@gmail.com>
---
 drivers/hwmon/pmbus/bel-pfe.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
index fa5070ae26bc..8280d274da3f 100644
--- a/drivers/hwmon/pmbus/bel-pfe.c
+++ b/drivers/hwmon/pmbus/bel-pfe.c
@@ -17,12 +17,12 @@
 enum chips {pfe1100, pfe3000};
 
 /*
- * Disable status check for pfe3000 devices, because some devices report
+ * Disable status check for pfexxxx devices, because some devices report
  * communication error (invalid command) for VOUT_MODE command (0x20)
  * although correct VOUT_MODE (0x16) is returned: it leads to incorrect
  * exponent in linear mode.
  */
-static struct pmbus_platform_data pfe3000_plat_data = {
+static struct pmbus_platform_data pfe_plat_data = {
 	.flags = PMBUS_SKIP_STATUS_CHECK,
 };
 
@@ -94,6 +94,7 @@ static int pfe_pmbus_probe(struct i2c_client *client)
 	int model;
 
 	model = (int)i2c_match_id(pfe_device_id, client)->driver_data;
+	client->dev.platform_data = &pfe_plat_data;
 
 	/*
 	 * PFE3000-12-069RA devices may not stay in page 0 during device
@@ -101,7 +102,6 @@ static int pfe_pmbus_probe(struct i2c_client *client)
 	 * So let's set the device to page 0 at the beginning.
 	 */
 	if (model == pfe3000) {
-		client->dev.platform_data = &pfe3000_plat_data;
 		i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
 	}
 
-- 
2.40.1

Re: [PATCH] hwmon: (pmbus/bel-pfe) Enable PMBUS_SKIP_STATUS_CHECK for pfe1100

[Guenter Roeck]

On 8/3/23 16:55, rentao.bupt@gmail.com wrote:
> From: Tao Ren <rentao.bupt@gmail.com>
> 
> Enable PMBUS_SKIP_STATUS_CHECK flag for both pfe1100 and pfe3000 because
> the similar communication error is observed on pfe1100 devices.
> 
> Signed-off-by: Tao Ren <rentao.bupt@gmail.com>
> ---
>   drivers/hwmon/pmbus/bel-pfe.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
> index fa5070ae26bc..8280d274da3f 100644
> --- a/drivers/hwmon/pmbus/bel-pfe.c
> +++ b/drivers/hwmon/pmbus/bel-pfe.c
> @@ -17,12 +17,12 @@
>   enum chips {pfe1100, pfe3000};
>   
>   /*
> - * Disable status check for pfe3000 devices, because some devices report
> + * Disable status check for pfexxxx devices, because some devices report
>    * communication error (invalid command) for VOUT_MODE command (0x20)
>    * although correct VOUT_MODE (0x16) is returned: it leads to incorrect
>    * exponent in linear mode.
>    */

Rephrase to something like

  Disable status check because some devices ... linear mode.
  This affects both pfe3000 and pfe1100.

We don't know if other pfe devices will be supported by the driver in the
future, and we don't know if those will be affected, so we should not make
any claims about such devices.

> -static struct pmbus_platform_data pfe3000_plat_data = {
> +static struct pmbus_platform_data pfe_plat_data = {
>   	.flags = PMBUS_SKIP_STATUS_CHECK,
>   };
>   
> @@ -94,6 +94,7 @@ static int pfe_pmbus_probe(struct i2c_client *client)
>   	int model;
>   
>   	model = (int)i2c_match_id(pfe_device_id, client)->driver_data;
> +	client->dev.platform_data = &pfe_plat_data;
>   
>   	/*
>   	 * PFE3000-12-069RA devices may not stay in page 0 during device
> @@ -101,7 +102,6 @@ static int pfe_pmbus_probe(struct i2c_client *client)
>   	 * So let's set the device to page 0 at the beginning.
>   	 */
>   	if (model == pfe3000) {
> -		client->dev.platform_data = &pfe3000_plat_data;
>   		i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
>   	}
>   

{ } is no longer needed.

Thanks,
Guenter

Re: [PATCH] hwmon: (pmbus/bel-pfe) Enable PMBUS_SKIP_STATUS_CHECK for pfe1100

[Tao Ren]

Hi Guenter,

Thank you for the quick review, and I've addressed your comments in v2.


Cheers,
Tao

On Fri, Aug 04, 2023 at 08:19:40AM -0700, Guenter Roeck wrote:
> On 8/3/23 16:55, rentao.bupt@gmail.com wrote:
> > From: Tao Ren <rentao.bupt@gmail.com>
> > 
> > Enable PMBUS_SKIP_STATUS_CHECK flag for both pfe1100 and pfe3000 because
> > the similar communication error is observed on pfe1100 devices.
> > 
> > Signed-off-by: Tao Ren <rentao.bupt@gmail.com>
> > ---
> >   drivers/hwmon/pmbus/bel-pfe.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
> > index fa5070ae26bc..8280d274da3f 100644
> > --- a/drivers/hwmon/pmbus/bel-pfe.c
> > +++ b/drivers/hwmon/pmbus/bel-pfe.c
> > @@ -17,12 +17,12 @@
> >   enum chips {pfe1100, pfe3000};
> >   /*
> > - * Disable status check for pfe3000 devices, because some devices report
> > + * Disable status check for pfexxxx devices, because some devices report
> >    * communication error (invalid command) for VOUT_MODE command (0x20)
> >    * although correct VOUT_MODE (0x16) is returned: it leads to incorrect
> >    * exponent in linear mode.
> >    */
> 
> Rephrase to something like
> 
>  Disable status check because some devices ... linear mode.
>  This affects both pfe3000 and pfe1100.
> 
> We don't know if other pfe devices will be supported by the driver in the
> future, and we don't know if those will be affected, so we should not make
> any claims about such devices.
> 
> > -static struct pmbus_platform_data pfe3000_plat_data = {
> > +static struct pmbus_platform_data pfe_plat_data = {
> >   	.flags = PMBUS_SKIP_STATUS_CHECK,
> >   };
> > @@ -94,6 +94,7 @@ static int pfe_pmbus_probe(struct i2c_client *client)
> >   	int model;
> >   	model = (int)i2c_match_id(pfe_device_id, client)->driver_data;
> > +	client->dev.platform_data = &pfe_plat_data;
> >   	/*
> >   	 * PFE3000-12-069RA devices may not stay in page 0 during device
> > @@ -101,7 +102,6 @@ static int pfe_pmbus_probe(struct i2c_client *client)
> >   	 * So let's set the device to page 0 at the beginning.
> >   	 */
> >   	if (model == pfe3000) {
> > -		client->dev.platform_data = &pfe3000_plat_data;
> >   		i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
> >   	}
> 
> { } is no longer needed.
> 
> Thanks,
> Guenter
>