Re: [PATCH v2] iio: dac: adi-axi-dac: Make use of dev_err_probe()

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Fri, 19 Dec 2025 17:46:52 +0000
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Fri, 2025-12-19 at 16:21 +0000, Jonathan Cameron wrote:
> > On Fri, 19 Dec 2025 15:54:29 +0000
> > Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> >   
> > > From: Nuno Sá <nuno.sa@analog.com>
> > > 
> > > Be consistent and use dev_err_probe() as in all other places in the
> > > .probe() path.
> > > 
> > > While at it, remove the line break in the version condition. Yes, it
> > > goes over the 80 column limit but I do think the line break hurts
> > > readability in this case. And use a struct device *dev helper for
> > > neater code.
> > > 
> > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>  
> > 
> > This has turned into a bit of an X and Y and Z patch.  In theory
> > should be split up but I guess it's not too bad.  
> 
> Yeah, I kind of felt a bit like that when looking at the patch diff.
> > 
> > However I'm not sure why you fixed one indent and left a bunch of similar
> > cases looking worse?
> > 
> > Jonathan
> >   
> > > ---
> > > Ended up dropping the dev_info() -> dev_dbg() patch.
> > > ---
> > > Changes in v2:
> > > - Patch 1
> > >   * Added helper struct device variable as suggested by Andy;
> > >   * Removed the braces as suggested by David.
> > > - Link to v1:
> > > https://lore.kernel.org/r/20251203-iio-axi-dac-minor-changes-v1-0-b54650cbeb33@analog.com
> > > ---
> > >  drivers/iio/dac/adi-axi-dac.c | 63 +++++++++++++++++++++----------------------
> > >  1 file changed, 30 insertions(+), 33 deletions(-)
> > > 
> > > diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> > > index 0d525272a8a8..ceab9f6fa3b4 100644
> > > --- a/drivers/iio/dac/adi-axi-dac.c
> > > +++ b/drivers/iio/dac/adi-axi-dac.c  
> > 
> >   
> > >  	/* Let's get the core read only configuration */
> > >  	ret = regmap_read(st->regmap, AXI_DAC_CONFIG_REG, &st->reg_config);
> > > @@ -975,34 +972,34 @@ static int axi_dac_probe(struct platform_device *pdev)
> > >  
> > >  	mutex_init(&st->lock);
> > >  
> > > -	ret = devm_iio_backend_register(&pdev->dev, st->info->backend_info, st);
> > > +	ret = devm_iio_backend_register(dev, st->info->backend_info, st);
> > >  	if (ret)
> > > -		return dev_err_probe(&pdev->dev, ret,
> > > +		return dev_err_probe(dev, ret,
> > >  				     "failed to register iio backend\n");
> > >  
> > > -	device_for_each_child_node_scoped(&pdev->dev, child) {
> > > +	device_for_each_child_node_scoped(dev, child) {
> > >  		int val;
> > >  
> > >  		if (!st->info->has_child_nodes)
> > > -			return dev_err_probe(&pdev->dev, -EINVAL,
> > > +			return dev_err_probe(dev, -EINVAL,
> > >  					     "invalid fdt axi-dac compatible.");
> > >  
> > >  		/* Processing only reg 0 node */
> > >  		ret = fwnode_property_read_u32(child, "reg", &val);
> > >  		if (ret)
> > > -			return dev_err_probe(&pdev->dev, ret,
> > > +			return dev_err_probe(dev, ret,
> > >  						"invalid reg property.");
> > >  		if (val != 0)
> > > -			return dev_err_probe(&pdev->dev, -EINVAL,
> > > +			return dev_err_probe(dev, -EINVAL,
> > >  						"invalid node address.");
> > >  
> > >  		ret = axi_dac_create_platform_device(st, child);
> > >  		if (ret)
> > > -			return dev_err_probe(&pdev->dev, -EINVAL,
> > > -						"cannot create device.");
> > > +			return dev_err_probe(dev, -EINVAL,
> > > +					     "cannot create device.");  
> > I'm not against this fixing up the indent but why not the ones above htat look
> > just as bad?  
> 
> Hmm I think this one were my automated fingers... Maybe the best thing to do is
> to split up the patches and take care of these indents in the patch adding
> the local struct device *dev variable. Does that works for you?

Yes. Sounds good to me.

J
> 
> - Nuno Sá
>  
> 
> 
> >