Re: [PATCH] iio: dac: adi-axi-dac: drop io_mode check

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 10 Feb 2025 10:05:47 +0000
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sat, 2025-02-08 at 15:45 +0000, Jonathan Cameron wrote:
> > On Thu, 06 Feb 2025 09:36:14 +0100
> > Angelo Dureghello <adureghello@baylibre.com> wrote:
> >   
> > > From: Angelo Dureghello <adureghello@baylibre.com>
> > > 
> > > Drop mode check, producing the following robot test warning:
> > > 
> > > smatch warnings:
> > > drivers/iio/dac/adi-axi-dac.c:731 axi_dac_bus_set_io_mode()
> > >   warn: always true condition '(mode >= 0) => (0-u32max >= 0)'
> > > 
> > > The range check results not useful since these are the only
> > > plausible modes for enum ad3552r_io_mode.
> > > 
> > > Fixes: 493122c53af1 ("iio: dac: adi-axi-dac: add bus mode setup")
> > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>  
> > Ah. I missed this.  Anyhow made the same change directly so all is well
> > than ends well!
> >   
> 
> Hi Angelo, Jonathan,
> 
> I wanted to reply to this one when I saw it but I haven't done right away and
> then totally forgot. Sorry about that!
> 
> I don't really agree with the "fix" in this patch. AFAIU, smatch is complaining
> since the enum is apparently defaulting to an unsigned type which means doing
> the >= 0 check is useless. But we should keep the upper bound...

Why? It's an enum so unless we are messing around with deliberate casts the
compiler should always be able to spot this. The check may be needed on a future
date if we add more types to that enum.

So I agree the check wasn't terrible and perhaps acted as hardening but it
isn't strictly speaking doing anything today.

Jonathan


> 
> - Nuno Sá
>