ina238_write_temp() was attempting to clamp the user input but was
throwing away the result. Ensure that we clamp the value to the
appropriate range before it is converted into a register value.

Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - New. Split off bugfix from main patch

 drivers/hwmon/ina238.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 5a394eeff676..4d3dc018ead9 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
 		return -EOPNOTSUPP;
 
 	/* Signed */
-	regval = clamp_val(val, -40000, 125000);
+	val = clamp_val(val, -40000, 125000);
 	regval = div_s64(val * 10000, data->config->temp_lsb) << data->config->temp_shift;
 	regval = clamp_val(regval, S16_MIN, S16_MAX) & (0xffff << data->config->temp_shift);
 
-- 
2.51.0

On Fri, Aug 29, 2025 at 03:05:10PM +1200, Chris Packham wrote:
> ina238_write_temp() was attempting to clamp the user input but was
> throwing away the result. Ensure that we clamp the value to the
> appropriate range before it is converted into a register value.
> 
> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Applied.

Guenter

On 8/28/25 20:05, Chris Packham wrote:
> ina238_write_temp() was attempting to clamp the user input but was
> throwing away the result. Ensure that we clamp the value to the
> appropriate range before it is converted into a register value.
> 
> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>      Changes in v3:
>      - New. Split off bugfix from main patch
> 
>   drivers/hwmon/ina238.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
> index 5a394eeff676..4d3dc018ead9 100644
> --- a/drivers/hwmon/ina238.c
> +++ b/drivers/hwmon/ina238.c
> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
>   		return -EOPNOTSUPP;
>   
>   	/* Signed */
> -	regval = clamp_val(val, -40000, 125000);
> +	val = clamp_val(val, -40000, 125000);

That needs another correction: As it turns out, the default register value
is 0x7ff0, or 255875. That means we need to accept that range. The same is
probably true for negative temperatures, but I'll need to see the real chip
to be sure.

Yes, the chips only support a limited temperature range, but that is the
limit register, not the supported range. Other chips have a similar problem.
It is ok to limit the input range if the chip has a reasonable default set,
but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
support writing that value.

Guenter

On 29/08/25 21:55, Guenter Roeck wrote:
> On 8/28/25 20:05, Chris Packham wrote:
>> ina238_write_temp() was attempting to clamp the user input but was
>> throwing away the result. Ensure that we clamp the value to the
>> appropriate range before it is converted into a register value.
>>
>> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula 
>> to adapt to different chips")
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>>      Changes in v3:
>>      - New. Split off bugfix from main patch
>>
>>   drivers/hwmon/ina238.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
>> index 5a394eeff676..4d3dc018ead9 100644
>> --- a/drivers/hwmon/ina238.c
>> +++ b/drivers/hwmon/ina238.c
>> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, 
>> u32 attr, long val)
>>           return -EOPNOTSUPP;
>>         /* Signed */
>> -    regval = clamp_val(val, -40000, 125000);
>> +    val = clamp_val(val, -40000, 125000);
>
> That needs another correction: As it turns out, the default register 
> value
> is 0x7ff0, or 255875. That means we need to accept that range. The 
> same is
> probably true for negative temperatures, but I'll need to see the real 
> chip
> to be sure.
>
> Yes, the chips only support a limited temperature range, but that is the
> limit register, not the supported range. Other chips have a similar 
> problem.
> It is ok to limit the input range if the chip has a reasonable default 
> set,
> but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
> support writing that value.
I'm guessing that might change my introduction of temp_max in the next 
patch. I'll re-order my series to put the bugfix first with the changes 
mentioned.
>
> Guenter
>

On 8/31/25 14:12, Chris Packham wrote:
> 
> On 29/08/25 21:55, Guenter Roeck wrote:
>> On 8/28/25 20:05, Chris Packham wrote:
>>> ina238_write_temp() was attempting to clamp the user input but was
>>> throwing away the result. Ensure that we clamp the value to the
>>> appropriate range before it is converted into a register value.
>>>
>>> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula
>>> to adapt to different chips")
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>
>>> Notes:
>>>       Changes in v3:
>>>       - New. Split off bugfix from main patch
>>>
>>>    drivers/hwmon/ina238.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
>>> index 5a394eeff676..4d3dc018ead9 100644
>>> --- a/drivers/hwmon/ina238.c
>>> +++ b/drivers/hwmon/ina238.c
>>> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev,
>>> u32 attr, long val)
>>>            return -EOPNOTSUPP;
>>>          /* Signed */
>>> -    regval = clamp_val(val, -40000, 125000);
>>> +    val = clamp_val(val, -40000, 125000);
>>
>> That needs another correction: As it turns out, the default register
>> value
>> is 0x7ff0, or 255875. That means we need to accept that range. The
>> same is
>> probably true for negative temperatures, but I'll need to see the real
>> chip
>> to be sure.
>>
>> Yes, the chips only support a limited temperature range, but that is the
>> limit register, not the supported range. Other chips have a similar
>> problem.
>> It is ok to limit the input range if the chip has a reasonable default
>> set,
>> but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
>> support writing that value.
> I'm guessing that might change my introduction of temp_max in the next
> patch. I'll re-order my series to put the bugfix first with the changes
> mentioned.

Please wait a bit before resending; I have a series almost ready to send out
with various other changes.

Guenter