Re: [PATCH v3 3/3] iio: temperature: ltc2983: Make use of device properties

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Mar 03, 2022 at 01:31:56PM +0000, Sa, Nuno wrote:
> Hi Andy,
> 
> Good that we waited to test this patch. The fundamental logic change
> for fetching and writing the custom tables are fine. That said, there
> some issues that I had to fix to test the patch. See below...

Thanks and indeed it's a good news that we caught a bug beforehand.

> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: Thursday, February 10, 2022 2:55 PM

...

> > -	phandle = of_parse_phandle(child, "adi,cold-junction-handle",
> > 0);
> > -	if (phandle) {
> > -		ret = of_property_read_u32(phandle, "reg",
> > -					   &thermo-
> > >cold_junction_chan);
> > +	ref = fwnode_find_reference(child, "adi,cold-junction-
> > handle", 0);
> > +	if (ref) {
> 
> This is nok. It needs to be 'if (IS_ERR(ref))'. We then should return
> ERR_CAST() in case of errors inside the if block. As this reference
> is also optional, we need to nullify ref in case we don't find the
> it. Otherwise fwnode_handle_put() breaks.

Got it (I hope).
Lemme go through it again and issue a v4.

> We also need to use ptr error logic in the other places where
> fwnode_find_reference() is used. Although, in the other cases
> the ref is mandatory, so there's no need to care with breaking
> fwnode_handle_put().
> 
> After these changes (I think the changes are straight enough;
> but I can re-test if you or Jonathan ask for it):
> 
> Tested-by: Nuno Sá <nuno.sa@analog.com>

I think of v4 where I may add this, but still would be nice to re-review and
check if I got correctly your testing report.

-- 
With Best Regards,
Andy Shevchenko