Re: [PATCH v2 1/2] hwmon: (pmbus/isl68137) Fix unchecked return value and use sysfs_emit()

[Guenter Roeck]

On Wed, Mar 18, 2026 at 07:40:19PM +0000, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@xxxxxxxxxxx>
> 
> isl68137_avs_enable_show_page() uses the return value of
> pmbus_read_byte_data() without checking for errors. If the I2C transaction
> fails, a negative error code is passed through bitwise operations,
> producing incorrect output.
> 
> Add an error check to propagate the return value if it is negative.
> Additionally, modernize the callback by replacing sprintf()
> with sysfs_emit().
> 
> Fixes: 038a9c3d1e424 ("hwmon: (pmbus/isl68137) Add driver for Intersil ISL68137 PWM Controller")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sanman Pradhan <psanman@xxxxxxxxxxx>

Applied.

Thanks,
Guenter

> ---
> v2:
>   - Kept explicit bitmask comparison instead of using !! operator
> ---
>  drivers/hwmon/pmbus/isl68137.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c
> index 97b61836f53a4..e7dac26b5be61 100644
> --- a/drivers/hwmon/pmbus/isl68137.c
> +++ b/drivers/hwmon/pmbus/isl68137.c
> @@ -98,8 +98,11 @@ static ssize_t isl68137_avs_enable_show_page(struct i2c_client *client,
>  {
>  	int val = pmbus_read_byte_data(client, page, PMBUS_OPERATION);
>  
> -	return sprintf(buf, "%d\n",
> -		       (val & ISL68137_VOUT_AVS) == ISL68137_VOUT_AVS ? 1 : 0);
> +	if (val < 0)
> +		return val;
> +
> +	return sysfs_emit(buf, "%d\n",
> +			   (val & ISL68137_VOUT_AVS) == ISL68137_VOUT_AVS);
>  }
>  
>  static ssize_t isl68137_avs_enable_store_page(struct i2c_client *client,