Re: [PATCH] hwmon: emc2103: use min_t() for explicit type in fan target clamp

[Animesh Rai]

Sorry for the noise, I misread the types, the operand was already u16
and the cast was unnecessary. I once again apologize for wasting your
time and disturbing you.


On Tue, Jul 7, 2026 at 1:27 AM David Laight
<david.laight.linux@xxxxxxxxx> wrote:
>
> On Mon,  6 Jul 2026 21:55:19 +0530
> Animesh Rai <animeshrai853@xxxxxxxxx> wrote:
>
> > Using min() with an explicit cast on one operand is fragile. Replace
> > with min_t(u16, ...) to make the intended comparison type explicit and
> > avoid implicit type conversion.
>
> min_t() is worse than having a cast on the argument to min().
> It just casts both arguments to the specified type.
> If you'd tried you's have found you could have just deleted the cast.
> But why is new_target u16, it could just be 'unsigned int'.
> That saves a load of masking instructions.
> Were old_div 9 and new_div 1 the rescale could overflow 16 bits,
> overflowing 32 is much less likely.
> (The surrounding code may make the overflow impossible...)
>
> If the code even right?
> It ignores values 0x1fe0 to 0x1fff (assuming the high bits can't
> be set) so they must be 'special' in some way, but doesn't stop the
> same 'special' values being generated when rescaled.
>
>         David
>
> >
> > Signed-off-by: Animesh Rai <animeshrai853@xxxxxxxxx>
> > ---
> >  drivers/hwmon/emc2103.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/emc2103.c b/drivers/hwmon/emc2103.c
> > index 27dc149a3ed9..fbb1b4025eb7 100644
> > --- a/drivers/hwmon/emc2103.c
> > +++ b/drivers/hwmon/emc2103.c
> > @@ -348,7 +348,7 @@ static ssize_t fan1_div_store(struct device *dev, struct device_attribute *da,
> >       /* update fan target if high byte is not disabled */
> >       if ((data->fan_target & 0x1fe0) != 0x1fe0) {
> >               u16 new_target = (data->fan_target * old_div) / new_div;
> > -             data->fan_target = min(new_target, (u16)0x1fff);
> > +             data->fan_target = min_t(u16, new_target, 0x1fff);
> >               write_fan_target_to_i2c(client, data->fan_target);
> >       }
> >
>