Re: [PATCH 1/5] hwmon: (pmbus) Use -ENODATA for unhandled registers in MPS drivers

[Guenter Roeck]

On 3/23/26 09:34, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@xxxxxxxxxxx>
>
> The read_word_data and write_word_data callbacks in mp2869, mp29502, and
> mp2925 return -EINVAL for unhandled register addresses. In the PMBus core,
> -ENODATA has a special meaning: it tells the core to fall through to the
> standard PMBus register read/write path. Any other negative value (such
> as -EINVAL) tells the core the register does not exist, causing valid
> PMBus standard registers to be silently hidden.
>
> Replace -EINVAL with -ENODATA in the default case of all affected
> read_word_data and write_word_data callbacks so that standard PMBus
> registers not handled by the driver are properly served by the core.
>
> Fixes: a3a2923aaf7f ("hwmon: add MP2869,MP29608,MP29612 and MP29816 series driver")
> Fixes: 90bad684e9ac ("hwmon: add MP29502 driver")
> Fixes: a79472e30be4 ("hwmon: Add MP2925 and MP2929 driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sanman Pradhan <psanman@xxxxxxxxxxx>

At least some of those have explicit -ENODATA returns for individual registers
(the mp2925 driver shows it below). Please combine those into the default:
case.

This was originally introduced because some chips react badly if an attempt
is made to read an unsupported register. I don't have any of the chips
available for testing, so I can not verify myself. Is it well known that
returning -ENODATA causes no problems for those chips ? If so, please mention
in the commit message.

Thanks,
Guenter

> ---
>   drivers/hwmon/pmbus/mp2869.c  | 4 ++--
>   drivers/hwmon/pmbus/mp2925.c  | 4 ++--
>   drivers/hwmon/pmbus/mp29502.c | 4 ++--
>   3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/mp2869.c b/drivers/hwmon/pmbus/mp2869.c
> index cc69a1e91dfe..4f8543801298 100644
> --- a/drivers/hwmon/pmbus/mp2869.c
> +++ b/drivers/hwmon/pmbus/mp2869.c
> @@ -391,7 +391,7 @@ static int mp2869_read_word_data(struct i2c_client *client, int page, int phase,
>   		ret = (ret & GENMASK(7, 0)) * MP2869_POUT_OP_GAIN;
>   		break;
>   	default:
> -		ret = -EINVAL;
> +		ret = -ENODATA;
>   		break;
>   	}
>   
> @@ -536,7 +536,7 @@ static int mp2869_write_word_data(struct i2c_client *client, int page, int reg,
>   								     MP2869_POUT_OP_GAIN)));
>   		break;
>   	default:
> -		ret = -EINVAL;
> +		ret = -ENODATA;
>   		break;
>   	}
>   
> diff --git a/drivers/hwmon/pmbus/mp2925.c b/drivers/hwmon/pmbus/mp2925.c
> index ad094842cf2d..a62f6c644bb5 100644
> --- a/drivers/hwmon/pmbus/mp2925.c
> +++ b/drivers/hwmon/pmbus/mp2925.c
> @@ -132,7 +132,7 @@ static int mp2925_read_word_data(struct i2c_client *client, int page, int phase,
>   		ret = -ENODATA;
>   		break;
>   	default:
> -		ret = -EINVAL;
> +		ret = -ENODATA;
>   		break;
>   	}
>   
> @@ -203,7 +203,7 @@ static int mp2925_write_word_data(struct i2c_client *client, int page, int reg,
>   										 ret)));
>   		break;
>   	default:
> -		ret = -EINVAL;
> +		ret = -ENODATA;
>   		break;
>   	}
>   
> diff --git a/drivers/hwmon/pmbus/mp29502.c b/drivers/hwmon/pmbus/mp29502.c
> index 7241373f1557..4556bc8350ae 100644
> --- a/drivers/hwmon/pmbus/mp29502.c
> +++ b/drivers/hwmon/pmbus/mp29502.c
> @@ -456,7 +456,7 @@ static int mp29502_read_word_data(struct i2c_client *client, int page,
>   		ret = (ret & GENMASK(7, 0)) - MP29502_TEMP_LIMIT_OFFSET;
>   		break;
>   	default:
> -		ret = -EINVAL;
> +		ret = -ENODATA;
>   		break;
>   	}
>   
> @@ -555,7 +555,7 @@ static int mp29502_write_word_data(struct i2c_client *client, int page, int reg,
>   						   word + MP29502_TEMP_LIMIT_OFFSET));
>   		break;
>   	default:
> -		ret = -EINVAL;
> +		ret = -ENODATA;
>   		break;
>   	}
>