On Fri, Apr 26, 2024 at 7:19 PM Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote: > > On Fri, Apr 26, 2024 at 11:31 AM Lorenzo Bianconi <lorenzo@xxxxxxxxxx> 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
