On Tue, Feb 11, 2025 at 07:43:11PM +0000, Jonathan Cameron wrote:
> On Sun, 9 Feb 2025 16:47:44 +0200
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Sat, Feb 08, 2025 at 04:13:24PM -0500, Aren Moynihan wrote:
> > > Using dev_err_probe instead of dev_err and return makes the errors
> >
> > Use dev_err_probe()
> > dev_err()
> >
> > > easier to understand by including the error name, and saves a little
> > > code.
> >
> > I believe this patch will make more sense before switching to local 'dev'
> > variable. Then the previous one will have an additional justification as
> > the "struct device *dev = ...;" lines in some cases will be added already
> > by this patch.
>
> I'm not sure I follow this one comment.
> The only line that has struct device *dev = added in this patch is
> replacing an existing client->dev lookup that could have been pushed
> to previous patch if this patch ordering was maintained.
>
> For dev_err() to dev_err_probe() the number of references to dev
> is the same after all. The only additional justification this patch
> makes is some longer lines that using a local dev pointer shortens
> again.
When converting to dev_err_probe() in some cases it makes sense to add a
temporary variable at the same time.
if (ret) {
dev_err(&pdev->dev, ...);
return ...;
}
===>
struct device *dev = &pdev->dev;
...
if (ret)
return dev_err_probe(dev, ...);
which reduces automatically the churn in the patch that wants to (re)use that
temporary variable and also adds to the justification as "we already have that
variable, just want to use it".
--
With Best Regards,
Andy Shevchenko
