[PATCH v2 2/2] mtd: rawnand: renesas: Use runtime PM instead of the raw clock API

[Geert Uytterhoeven]

Hi Miquel,

On Tue, May 10, 2022 at 11:03 AM Miquel Raynal
<miquel.raynal at bootlin.com> wrote:
> geert at linux-m68k.org wrote on Tue, 10 May 2022 10:52:41 +0200:
> > On Mon, May 9, 2022 at 5:49 PM Miquel Raynal <miquel.raynal at bootlin.com> 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 at bootlin.com>

> > > @@ -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 at linux-m68k.org

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