hwmon: pmbus: Add support for MPQ8786

[PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786

Posted by Carl Lee via B4 Relay 5 days, 12 hours ago

From: Carl Lee <carl.lee@amd.com>

According to MPQ8785/MPQ8786 datasheet, VID mode configuration is
the same as direct mode configuration. Therefore, when VOUT is
reported in VID mode, it must be forced to use direct format.

Signed-off-by: Carl Lee <carl.lee@amd.com>
---
 drivers/hwmon/pmbus/mpq8785.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
index f35534836cb8..d6624af076c3 100644
--- a/drivers/hwmon/pmbus/mpq8785.c
+++ b/drivers/hwmon/pmbus/mpq8785.c
@@ -48,6 +48,25 @@ static int mpq8785_identify(struct i2c_client *client,
 	return 0;
 };
 
+static int mpq8785_read_byte_data(struct i2c_client *client, int page, int reg)
+{
+	int ret;
+
+	switch (reg) {
+	case PMBUS_VOUT_MODE:
+		ret = pmbus_read_byte_data(client, page, reg);
+		if (ret < 0)
+			return ret;
+
+		if ((ret >> 5) == 1)
+			return PB_VOUT_MODE_DIRECT;
+	default:
+		return -ENODATA;
+	}
+
+	return ret;
+}
+
 static int mpm82504_read_word_data(struct i2c_client *client, int page,
 				   int phase, int reg)
 {
@@ -133,6 +152,7 @@ static int mpq8785_probe(struct i2c_client *client)
 	case mpq8785:
 	case mpq8786:
 		info->identify = mpq8785_identify;
+		info->read_byte_data = mpq8785_read_byte_data;
 		break;
 	default:
 		return -ENODEV;

-- 
2.34.1

Re: [PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786

Posted by kernel test robot 3 days, 10 hours ago

Hi Carl,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 4c87cdd0328495759f6e9f9f4e1e53ef8032a76f]

url:    https://github.com/intel-lab-lkp/linux/commits/Carl-Lee-via-B4-Relay/dt-bindings-hwmon-pmbus-mpq8785-add-MPQ8786-support/20260205-180428
base:   4c87cdd0328495759f6e9f9f4e1e53ef8032a76f
patch link:    https://lore.kernel.org/r/20260205-dt-bindings-hwmon-pmbus-mpq8785-add-mpq8786-support-v2-3-3744cd9b2850%40amd.com
patch subject: [PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786
config: s390-randconfig-001-20260207 (https://download.01.org/0day-ci/archive/20260207/202602071904.asgoYuoc-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260207/202602071904.asgoYuoc-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/202602071904.asgoYuoc-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/hwmon/pmbus/mpq8785.c: In function 'mpq8785_read_byte_data':
>> drivers/hwmon/pmbus/mpq8785.c:61:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
      if ((ret >> 5) == 1)
         ^
   drivers/hwmon/pmbus/mpq8785.c:63:2: note: here
     default:
     ^~~~~~~


vim +61 drivers/hwmon/pmbus/mpq8785.c

    50	
    51	static int mpq8785_read_byte_data(struct i2c_client *client, int page, int reg)
    52	{
    53		int ret;
    54	
    55		switch (reg) {
    56		case PMBUS_VOUT_MODE:
    57			ret = pmbus_read_byte_data(client, page, reg);
    58			if (ret < 0)
    59				return ret;
    60	
  > 61			if ((ret >> 5) == 1)
    62				return PB_VOUT_MODE_DIRECT;
    63		default:
    64			return -ENODATA;
    65		}
    66	
    67		return ret;
    68	}
    69	

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

Re: [PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786

Posted by kernel test robot 3 days, 10 hours ago

Hi Carl,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 4c87cdd0328495759f6e9f9f4e1e53ef8032a76f]

url:    https://github.com/intel-lab-lkp/linux/commits/Carl-Lee-via-B4-Relay/dt-bindings-hwmon-pmbus-mpq8785-add-MPQ8786-support/20260205-180428
base:   4c87cdd0328495759f6e9f9f4e1e53ef8032a76f
patch link:    https://lore.kernel.org/r/20260205-dt-bindings-hwmon-pmbus-mpq8785-add-mpq8786-support-v2-3-3744cd9b2850%40amd.com
patch subject: [PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786
config: i386-randconfig-141-20260207 (https://download.01.org/0day-ci/archive/20260207/202602071928.rf2Gjdgd-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
smatch version: v0.5.0-8994-gd50c5a4c
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260207/202602071928.rf2Gjdgd-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/202602071928.rf2Gjdgd-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/pmbus/mpq8785.c:63:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
      63 |         default:
         |         ^
   drivers/hwmon/pmbus/mpq8785.c:63:2: note: insert '__attribute__((fallthrough));' to silence this warning
      63 |         default:
         |         ^
         |         __attribute__((fallthrough)); 
   drivers/hwmon/pmbus/mpq8785.c:63:2: note: insert 'break;' to avoid fall-through
      63 |         default:
         |         ^
         |         break; 
   1 warning generated.


vim +63 drivers/hwmon/pmbus/mpq8785.c

    50	
    51	static int mpq8785_read_byte_data(struct i2c_client *client, int page, int reg)
    52	{
    53		int ret;
    54	
    55		switch (reg) {
    56		case PMBUS_VOUT_MODE:
    57			ret = pmbus_read_byte_data(client, page, reg);
    58			if (ret < 0)
    59				return ret;
    60	
    61			if ((ret >> 5) == 1)
    62				return PB_VOUT_MODE_DIRECT;
  > 63		default:
    64			return -ENODATA;
    65		}
    66	
    67		return ret;
    68	}
    69	

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

Re: [PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786

Posted by Guenter Roeck 5 days, 4 hours ago

On Thu, Feb 05, 2026 at 06:01:39PM +0800, Carl Lee via B4 Relay wrote:
> From: Carl Lee <carl.lee@amd.com>
> 
> According to MPQ8785/MPQ8786 datasheet, VID mode configuration is
> the same as direct mode configuration. Therefore, when VOUT is
> reported in VID mode, it must be forced to use direct format.
> 
> Signed-off-by: Carl Lee <carl.lee@amd.com>
> ---
>  drivers/hwmon/pmbus/mpq8785.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index f35534836cb8..d6624af076c3 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -48,6 +48,25 @@ static int mpq8785_identify(struct i2c_client *client,
>  	return 0;
>  };
>  
> +static int mpq8785_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> +	int ret;
> +
> +	switch (reg) {
> +	case PMBUS_VOUT_MODE:
> +		ret = pmbus_read_byte_data(client, page, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		if ((ret >> 5) == 1)
> +			return PB_VOUT_MODE_DIRECT;
> +	default:
> +		return -ENODATA;
> +	}
> +
> +	return ret;
> +}

In addition to my earlier reply, here is AI code review feedback:

This switch statement appears to fall through to the default case when the
mode is not VID (when (ret >> 5) != 1). If it falls through, it returns
-ENODATA.  The core function _pmbus_read_byte_data() will then see -ENODATA
and call pmbus_read_byte_data() again, resulting in a second I2C transaction
for the same register.

Also, the `return ret;` at the end of the function is unreachable because
the default case returns.

Should the PMBUS_VOUT_MODE case return `ret` instead of falling through?

Guenter

Re: [PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786

Posted by Carl Lee 1 day, 15 hours ago

On Thu, Feb 05, 2026 at 09:46:05AM -0800, Guenter Roeck wrote:
> On Thu, Feb 05, 2026 at 06:01:39PM +0800, Carl Lee via B4 Relay wrote:
> > From: Carl Lee <carl.lee@amd.com>
> > 
> > According to MPQ8785/MPQ8786 datasheet, VID mode configuration is
> > the same as direct mode configuration. Therefore, when VOUT is
> > reported in VID mode, it must be forced to use direct format.
> > 
> > Signed-off-by: Carl Lee <carl.lee@amd.com>
> > ---
> >  drivers/hwmon/pmbus/mpq8785.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> > index f35534836cb8..d6624af076c3 100644
> > --- a/drivers/hwmon/pmbus/mpq8785.c
> > +++ b/drivers/hwmon/pmbus/mpq8785.c
> > @@ -48,6 +48,25 @@ static int mpq8785_identify(struct i2c_client *client,
> >  	return 0;
> >  };
> >  
> > +static int mpq8785_read_byte_data(struct i2c_client *client, int page, int reg)
> > +{
> > +	int ret;
> > +
> > +	switch (reg) {
> > +	case PMBUS_VOUT_MODE:
> > +		ret = pmbus_read_byte_data(client, page, reg);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		if ((ret >> 5) == 1)
> > +			return PB_VOUT_MODE_DIRECT;
> > +	default:
> > +		return -ENODATA;
> > +	}
> > +
> > +	return ret;
> > +}
> 
> In addition to my earlier reply, here is AI code review feedback:
> 
> This switch statement appears to fall through to the default case when the
> mode is not VID (when (ret >> 5) != 1). If it falls through, it returns
> -ENODATA.  The core function _pmbus_read_byte_data() will then see -ENODATA
> and call pmbus_read_byte_data() again, resulting in a second I2C transaction
> for the same register.
> 
> Also, the `return ret;` at the end of the function is unreachable because
> the default case returns.
> 
> Should the PMBUS_VOUT_MODE case return `ret` instead of falling through?
> 
> Guenter

Got it, I’ll correct this.

Re: [PATCH v2 3/3] hwmon: pmbus: mpq8785: force direct mode for VID VOUT on MPQ8785/MPQ8786

Posted by Guenter Roeck 5 days, 5 hours ago

On Thu, Feb 05, 2026 at 06:01:39PM +0800, Carl Lee via B4 Relay wrote:
> From: Carl Lee <carl.lee@amd.com>
> 
> According to MPQ8785/MPQ8786 datasheet, VID mode configuration is
> the same as direct mode configuration. Therefore, when VOUT is
> reported in VID mode, it must be forced to use direct format.
> 

Why "must" ? Yes, the LSB is the same, at least for MPQ8785,
but that doesn't mean that the mode _must_ be overwritten. Maybe
I am missing it, but as far as I can see the datasheet doesn't
say that the VID mode configuration is the same as direct mode
configuration. It says that the _LSB_ is the same for both modes.

I _think_ the problem may be that the output voltages are not really
reported as VID values but as raw voltages, but the datasheet is a bit
vague in that regard. It talks about LSB values but doesn't exactly
say how voltages are reported, and for READ_VIN it is most definitely
wrong ("This bit is in VID mode with 25mv/LSB" doesn't make any sense).

Thanks,
Guenter

> Signed-off-by: Carl Lee <carl.lee@amd.com>
> ---
>  drivers/hwmon/pmbus/mpq8785.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index f35534836cb8..d6624af076c3 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -48,6 +48,25 @@ static int mpq8785_identify(struct i2c_client *client,
>  	return 0;
>  };
>  
> +static int mpq8785_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> +	int ret;
> +
> +	switch (reg) {
> +	case PMBUS_VOUT_MODE:
> +		ret = pmbus_read_byte_data(client, page, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		if ((ret >> 5) == 1)
> +			return PB_VOUT_MODE_DIRECT;
> +	default:
> +		return -ENODATA;
> +	}
> +
> +	return ret;
> +}
> +
>  static int mpm82504_read_word_data(struct i2c_client *client, int page,
>  				   int phase, int reg)
>  {
> @@ -133,6 +152,7 @@ static int mpq8785_probe(struct i2c_client *client)
>  	case mpq8785:
>  	case mpq8786:
>  		info->identify = mpq8785_identify;
> +		info->read_byte_data = mpq8785_read_byte_data;
>  		break;
>  	default:
>  		return -ENODEV;
> 
> -- 
> 2.34.1
> 
>