linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] hwmon: ltc4282: add support for the LTC4282 chip
@ 2024-02-07  9:51 Dan Carpenter
  2024-02-07 10:19 ` Nuno Sá
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2024-02-07  9:51 UTC (permalink / raw)
  To: nuno.sa; +Cc: linux-hwmon

Hello Nuno Sa,

The patch 848a5019ded5: "hwmon: ltc4282: add support for the LTC4282
chip" from Jan 29, 2024 (linux-next), leads to the following Smatch
static checker warning:

	drivers/hwmon/ltc4282.c:732 ltc4282_write_voltage_byte_cached()
	warn: no lower bound on 'val' rl='s64min-4294967294'

drivers/hwmon/ltc4282.c
    723 static int ltc4282_write_voltage_byte_cached(const struct ltc4282_state *st,
    724                                              u32 reg, u32 fs, long val,
    725                                              u32 *cache_raw)
    726 {
    727         u32 in;
    728 
    729         if (val >= fs)
    730                 in = U8_MAX;
    731         else
--> 732                 in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);

It looks like val comes from the user via hwmon_attr_store().  Do we
need to check if it's negative?

    733 
    734         if (cache_raw) {
    735                 *cache_raw = in;
    736                 return 0;
    737         }
    738 
    739         return regmap_write(st->map, reg, in);
    740 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [bug report] hwmon: ltc4282: add support for the LTC4282 chip
  2024-02-07  9:51 [bug report] hwmon: ltc4282: add support for the LTC4282 chip Dan Carpenter
@ 2024-02-07 10:19 ` Nuno Sá
  2024-02-07 16:49   ` Guenter Roeck
  0 siblings, 1 reply; 6+ messages in thread
From: Nuno Sá @ 2024-02-07 10:19 UTC (permalink / raw)
  To: Dan Carpenter, nuno.sa; +Cc: linux-hwmon

Hi Dan,

On Wed, 2024-02-07 at 12:51 +0300, Dan Carpenter wrote:
> Hello Nuno Sa,
> 
> The patch 848a5019ded5: "hwmon: ltc4282: add support for the LTC4282
> chip" from Jan 29, 2024 (linux-next), leads to the following Smatch
> static checker warning:
> 
> 	drivers/hwmon/ltc4282.c:732 ltc4282_write_voltage_byte_cached()
> 	warn: no lower bound on 'val' rl='s64min-4294967294'
> 
> drivers/hwmon/ltc4282.c
>     723 static int ltc4282_write_voltage_byte_cached(const struct
> ltc4282_state *st,
>     724                                              u32 reg, u32 fs, long
> val,
>     725                                              u32 *cache_raw)
>     726 {
>     727         u32 in;
>     728 
>     729         if (val >= fs)
>     730                 in = U8_MAX;
>     731         else
> --> 732                 in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);

Yeah, I guess we should likely clamp it between 0 and fs

- Nuno Sá


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [bug report] hwmon: ltc4282: add support for the LTC4282 chip
  2024-02-07 10:19 ` Nuno Sá
@ 2024-02-07 16:49   ` Guenter Roeck
  2024-02-08  8:12     ` Nuno Sá
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2024-02-07 16:49 UTC (permalink / raw)
  To: Nuno Sá, Dan Carpenter, nuno.sa; +Cc: linux-hwmon

On 2/7/24 02:19, Nuno Sá wrote:
> Hi Dan,
> 
> On Wed, 2024-02-07 at 12:51 +0300, Dan Carpenter wrote:
>> Hello Nuno Sa,
>>
>> The patch 848a5019ded5: "hwmon: ltc4282: add support for the LTC4282
>> chip" from Jan 29, 2024 (linux-next), leads to the following Smatch
>> static checker warning:
>>
>> 	drivers/hwmon/ltc4282.c:732 ltc4282_write_voltage_byte_cached()
>> 	warn: no lower bound on 'val' rl='s64min-4294967294'
>>
>> drivers/hwmon/ltc4282.c
>>      723 static int ltc4282_write_voltage_byte_cached(const struct
>> ltc4282_state *st,
>>      724                                              u32 reg, u32 fs, long
>> val,
>>      725                                              u32 *cache_raw)
>>      726 {
>>      727         u32 in;
>>      728
>>      729         if (val >= fs)
>>      730                 in = U8_MAX;
>>      731         else
>> --> 732                 in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
> 
> Yeah, I guess we should likely clamp it between 0 and fs
> 

	val = clamp_val(val, 0, fs);
	in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);

maybe.

Should I fix that inline or do you want to send a follow-up patch ?

Guenter





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [bug report] hwmon: ltc4282: add support for the LTC4282 chip
  2024-02-07 16:49   ` Guenter Roeck
@ 2024-02-08  8:12     ` Nuno Sá
  2024-02-08 14:19       ` Guenter Roeck
  0 siblings, 1 reply; 6+ messages in thread
From: Nuno Sá @ 2024-02-08  8:12 UTC (permalink / raw)
  To: Guenter Roeck, Dan Carpenter, nuno.sa; +Cc: linux-hwmon

On Wed, 2024-02-07 at 08:49 -0800, Guenter Roeck wrote:
> On 2/7/24 02:19, Nuno Sá wrote:
> > Hi Dan,
> > 
> > On Wed, 2024-02-07 at 12:51 +0300, Dan Carpenter wrote:
> > > Hello Nuno Sa,
> > > 
> > > The patch 848a5019ded5: "hwmon: ltc4282: add support for the LTC4282
> > > chip" from Jan 29, 2024 (linux-next), leads to the following Smatch
> > > static checker warning:
> > > 
> > > 	drivers/hwmon/ltc4282.c:732 ltc4282_write_voltage_byte_cached()
> > > 	warn: no lower bound on 'val' rl='s64min-4294967294'
> > > 
> > > drivers/hwmon/ltc4282.c
> > >      723 static int ltc4282_write_voltage_byte_cached(const struct
> > > ltc4282_state *st,
> > >      724                                              u32 reg, u32 fs,
> > > long
> > > val,
> > >      725                                              u32 *cache_raw)
> > >      726 {
> > >      727         u32 in;
> > >      728
> > >      729         if (val >= fs)
> > >      730                 in = U8_MAX;
> > >      731         else
> > > --> 732                 in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
> > 
> > Yeah, I guess we should likely clamp it between 0 and fs
> > 
> 
> 	val = clamp_val(val, 0, fs);
> 	in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
> 
> maybe.
> 
> Should I fix that inline or do you want to send a follow-up patch ?
> 
> Guenter
> 
> 

Whatever makes your life easier... I can send a patch if that is your
preference.

- Nuno Sá

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [bug report] hwmon: ltc4282: add support for the LTC4282 chip
  2024-02-08  8:12     ` Nuno Sá
@ 2024-02-08 14:19       ` Guenter Roeck
  2024-02-08 15:22         ` Nuno Sá
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2024-02-08 14:19 UTC (permalink / raw)
  To: Nuno Sá, Dan Carpenter, nuno.sa; +Cc: linux-hwmon

On 2/8/24 00:12, Nuno Sá wrote:
> On Wed, 2024-02-07 at 08:49 -0800, Guenter Roeck wrote:
>> On 2/7/24 02:19, Nuno Sá wrote:
>>> Hi Dan,
>>>
>>> On Wed, 2024-02-07 at 12:51 +0300, Dan Carpenter wrote:
>>>> Hello Nuno Sa,
>>>>
>>>> The patch 848a5019ded5: "hwmon: ltc4282: add support for the LTC4282
>>>> chip" from Jan 29, 2024 (linux-next), leads to the following Smatch
>>>> static checker warning:
>>>>
>>>> 	drivers/hwmon/ltc4282.c:732 ltc4282_write_voltage_byte_cached()
>>>> 	warn: no lower bound on 'val' rl='s64min-4294967294'
>>>>
>>>> drivers/hwmon/ltc4282.c
>>>>       723 static int ltc4282_write_voltage_byte_cached(const struct
>>>> ltc4282_state *st,
>>>>       724                                              u32 reg, u32 fs,
>>>> long
>>>> val,
>>>>       725                                              u32 *cache_raw)
>>>>       726 {
>>>>       727         u32 in;
>>>>       728
>>>>       729         if (val >= fs)
>>>>       730                 in = U8_MAX;
>>>>       731         else
>>>> --> 732                 in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
>>>
>>> Yeah, I guess we should likely clamp it between 0 and fs
>>>
>>
>> 	val = clamp_val(val, 0, fs);
>> 	in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
>>
>> maybe.
>>
>> Should I fix that inline or do you want to send a follow-up patch ?
>>
>> Guenter
>>
>>
> 
> Whatever makes your life easier... I can send a patch if that is your
> preference.
> 

I updated the original patch.

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [bug report] hwmon: ltc4282: add support for the LTC4282 chip
  2024-02-08 14:19       ` Guenter Roeck
@ 2024-02-08 15:22         ` Nuno Sá
  0 siblings, 0 replies; 6+ messages in thread
From: Nuno Sá @ 2024-02-08 15:22 UTC (permalink / raw)
  To: Guenter Roeck, Dan Carpenter, nuno.sa; +Cc: linux-hwmon

On Thu, 2024-02-08 at 06:19 -0800, Guenter Roeck wrote:
> On 2/8/24 00:12, Nuno Sá wrote:
> > On Wed, 2024-02-07 at 08:49 -0800, Guenter Roeck wrote:
> > > On 2/7/24 02:19, Nuno Sá wrote:
> > > > Hi Dan,
> > > > 
> > > > On Wed, 2024-02-07 at 12:51 +0300, Dan Carpenter wrote:
> > > > > Hello Nuno Sa,
> > > > > 
> > > > > The patch 848a5019ded5: "hwmon: ltc4282: add support for the LTC4282
> > > > > chip" from Jan 29, 2024 (linux-next), leads to the following Smatch
> > > > > static checker warning:
> > > > > 
> > > > > 	drivers/hwmon/ltc4282.c:732
> > > > > ltc4282_write_voltage_byte_cached()
> > > > > 	warn: no lower bound on 'val' rl='s64min-4294967294'
> > > > > 
> > > > > drivers/hwmon/ltc4282.c
> > > > >       723 static int ltc4282_write_voltage_byte_cached(const struct
> > > > > ltc4282_state *st,
> > > > >       724                                              u32 reg, u32
> > > > > fs,
> > > > > long
> > > > > val,
> > > > >       725                                              u32 *cache_raw)
> > > > >       726 {
> > > > >       727         u32 in;
> > > > >       728
> > > > >       729         if (val >= fs)
> > > > >       730                 in = U8_MAX;
> > > > >       731         else
> > > > > --> 732                 in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
> > > > 
> > > > Yeah, I guess we should likely clamp it between 0 and fs
> > > > 
> > > 
> > > 	val = clamp_val(val, 0, fs);
> > > 	in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
> > > 
> > > maybe.
> > > 
> > > Should I fix that inline or do you want to send a follow-up patch ?
> > > 
> > > Guenter
> > > 
> > > 
> > 
> > Whatever makes your life easier... I can send a patch if that is your
> > preference.
> > 
> 
> I updated the original patch.
> 
> Thanks,
> Guenter
> 

Thanks!
- Nuno Sá

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-02-08 15:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-07  9:51 [bug report] hwmon: ltc4282: add support for the LTC4282 chip Dan Carpenter
2024-02-07 10:19 ` Nuno Sá
2024-02-07 16:49   ` Guenter Roeck
2024-02-08  8:12     ` Nuno Sá
2024-02-08 14:19       ` Guenter Roeck
2024-02-08 15:22         ` Nuno Sá

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).