Re: [PATCH v11 6/9] clk: renesas: r9a06g032: Probe possible children

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Geert,

geert@linux-m68k.org wrote on Mon, 25 Apr 2022 18:18:28 +0200:

> Hi Miquel,
> 
> On Thu, Apr 21, 2022 at 10:51 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> > The clock controller device on r9a06g032 takes all the memory range that
> > is described as being a system controller. This range contains many
> > different (unrelated?) registers besides the ones belonging to the clock
> > controller, that can necessitate to be accessed from other peripherals.
> >
> > For instance, the dmamux registers are there. The dmamux "device" will
> > be described as a child node of the clock/system controller node, which
> > means we need the top device driver (the clock controller driver in this
> > case) to populate its children manually.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Acked-by: Stephen Boyd <sboyd@kernel.org>  
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> > @@ -996,7 +997,7 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
> >
> >         sysctrl_priv = clocks;
> >
> > -       return 0;
> > +       return of_platform_populate(np, NULL, NULL, dev);  
> 
> This is a bit dangerous: in the (very unlikely) case that
> of_platform_populate() fails, the clock driver will fail to probe,
> and all managed cleanup will be done (not everything will be cleant
> up, though), while sysctrl_priv will still point to the now-freed
> r9a06g032_priv structure.
> 
> So I think you just want to ignore the failure from
> of_platform_populate(), and return zero anyway.

That is a very good point. I've changed the logic to just print an
error message and return 0 anyway.

Thanks,
Miquèl