Re: [PATCH v4 3/3] spi: airoha: add SPI-NAND Flash controller driver

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Fri, Apr 26, 2024 at 7:19 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > On Fri, Apr 26, 2024 at 11:31 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:

...

> > > +       default:
> > > +               break;
> > > +       }
> > > +
> > > +       return false;
> >
> > Why not return false directly from the default case?
>
> it is because we still need the 'return false' at the end of routine for the
> other cases due to SPI_MEM_DATA_IN and SPI_MEM_DATA_OUT.

Hmm... Can it be refactored? I think it would still be possible to
slightly reduce the amount of LoCs.

...

> > > +               op->data.nbytes = min_t(size_t, op->data.nbytes, 160 - len);
> >
> > You probably wanted clamp(). It's discouraged to use min_t() for unsigned types.
>
> do you mean doing something like:
>
> op->data.nbytes = clamp(op->data.nbytes, op->data.nbytes, 160 - len);
>
> maybe an 'if' condition is more readable, what do you think?

Yes, since we have only one branch of change.

...

> > > +       base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> >
> > How is 'res' being used exactly?
>
> right, we can pass NULL here to devm_platform_get_and_ioremap_resource()

No, just use another call that doesn't require this parameter at all.

...

> > > +       base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> >
> > Ditto.

-- 
With Best Regards,
Andy Shevchenko