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

Pradhan, Sanman posted 5 patches 1 week, 3 days ago
There is a newer version of this series
[PATCH 1/5] hwmon: (pmbus) Use -ENODATA for unhandled registers in MPS drivers
Posted by Pradhan, Sanman 1 week, 3 days ago
From: Sanman Pradhan <psanman@juniper.net>

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@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 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;
 	}
 
-- 
2.34.1

Re: [PATCH 1/5] hwmon: (pmbus) Use -ENODATA for unhandled registers in MPS drivers
Posted by Guenter Roeck 1 week, 3 days ago
On 3/23/26 09:34, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> 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@vger.kernel.org
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>

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