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@xxxxxxxxxxxxxxx>
> > 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@xxxxxxxxxx>
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
