Hi Miquel, On Tue, May 10, 2022 at 11:03 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > geert@xxxxxxxxxxxxxx wrote on Tue, 10 May 2022 10:52:41 +0200: > > On Mon, May 9, 2022 at 5:49 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > > This NAND controller is part of a well defined power domain handled by > > > the runtime PM core. Let's keep the harmony with the other RZ/N1 drivers > > > and exclusively use the runtime PM API to enable/disable the clocks. > > > > > > We still need to retrieve the external clock rate in order to derive the > > > NAND timings, but that is not a big deal, we can still do that in the > > > probe and just save this value to reuse it later. > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx> > > > @@ -1365,12 +1347,27 @@ static int rnandc_probe(struct platform_device *pdev) > > > ret = devm_request_irq(&pdev->dev, irq, rnandc_irq_handler, 0, > > > "renesas-nand-controller", rnandc); > > > if (ret < 0) > > > - goto disable_eclk; > > > + return ret; > > > } > > > > > > ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)); > > > if (ret) > > > - goto disable_eclk; > > > + return ret; > > > + > > > + devm_pm_runtime_enable(&pdev->dev); > > > + ret = pm_runtime_get_sync(&pdev->dev); > > > + if (ret < 0) > > > + return ret; > > > + > > > + /* The external NAND bus clock rate is needed for computing timings */ > > > + eclk = clk_get(&pdev->dev, "eclk"); > > > + if (IS_ERR(eclk)) { > > > + ret = PTR_ERR(eclk); > > > + goto dis_runtime_pm; > > > + } > > > + > > > + rnandc->ext_clk_rate = clk_get_rate(eclk); > > > > Personally, I would do this before requesting the interrupt. > > But I guess it's fine to do that here, too. > > What do you mean by "do this"? If you are talking about getting the Getting the clock (which could -EPROBE_DEFER, in theory) and its rate. > rate of the clock it's only needed when initializing the timings which > happens later in the probe path and has no relationship with the > interrupts at all. Anyway, I will move all of that earlier so that we > don't have a possible situation where the device is not clocked when > accessing it. OK. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds