Hi Geert, geert@xxxxxxxxxxxxxx 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@xxxxxxxxxxx> 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@xxxxxxxxxxx> > > Acked-by: Stephen Boyd <sboyd@xxxxxxxxxx> > > 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
