Re: [PATCH v5 6/8] iio: light: stk3310: use dev_err_probe where possible

[Aren]

On Sun, Feb 09, 2025 at 04:47:44PM +0200, Andy Shevchenko 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.

That will only be added in one spot, and I skipped updating the dev_err
calls in the previous patch that this patch rewrites, so churn shouldn't
be an issue. That also makes it trivial to reorder them, so I guess it
can't hurt.

> >  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> > -	if (!indio_dev) {
> > -		dev_err(&client->dev, "iio allocation failed!\n");
> > -		return -ENOMEM;
> > -	}
> > +	if (!indio_dev)
> > +		return dev_err_probe(dev, -ENOMEM, "iio allocation failed!\n");
> 
> We don't issue the messages for -ENOMEM.
> 
> If it's in the current code, add a new patch to drop this message and return an
> error code directly.
> 
> ...
> 
> > +	if (ret < 0)
> 
> Perhaps, while at it, drop these ' < 0' parts where they are not hinting about
> anything.

Sure, I can add patches for these, although continuing to rebase this
series is getting a bit cumbersome (perhaps just because I haven't found
a good workflow for it). Would I be better off reordering this so the
refactoring patches come first and can be partially merged?

Regards
 - Aren