On Thu, 2024-06-06 at 13:17 +0300, Andy Shevchenko wrote:
> On Thu, Jun 06, 2024 at 09:22:38AM +0200, Nuno Sa wrote:
> > Use dev_err_probe() (and variants) in the probe() path. While at it, made
> > some simple improvements:
> > * Explicitly included the err.h and errno.h headers;
> > * Removed some unnecessary line breaks;
> > * Removed a redundant 'else';
> > * Added some missing \n to prink.
>
> ...
>
> > - if (ret) {
> > + if (ret)
> > /*
> > * This would be catched later but we can just
> > return
> > * the error right away.
> > */
> > - dev_err(&st->spi->dev, "Property reg must be
> > given\n");
> > - return ERR_PTR(ret);
> > - }
> > + return dev_err_ptr_probe(&st->spi->dev, ret,
> > + "Property reg must be
> > given\n");
>
> Even if it becomes a one line of code, it's still a multiline branch, due to
> comment. I think {} is better to be there. What does checkpatch say about
> this?
Checkpatch is fine about it...
>
>
> ...
>
> > + return dev_err_ptr_probe(&st->spi->dev, -EINVAL,
>
> You can make all these lines shorter by using
>
> struct device *dev = &st->spi->dev; // or analogue
>
> at the top of the function.
>
Well, I had that in v2 (making the whole driver coherent with the local struct
device helper but you kind of "complained" for a precursor patch (on a
devm_kzalloc() call). So basically I deferred that change for a follow up patch.
- Nuno Sá
>
