Re: [PATCH v2 3/7] rtc: rzn1: Add new RTC driver

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Miquel,

On Thu, Apr 28, 2022 at 11:09 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
> geert@linux-m68k.org wrote on Thu, 28 Apr 2022 10:58:03 +0200:
> > On Thu, Apr 21, 2022 at 11:00 AM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:
> > > From: Michel Pollet <michel.pollet@bp.renesas.com>
> > >
> > > Add a basic RTC driver for the RZ/N1.
> > >
> > > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> > > Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

> > > --- /dev/null
> > > +++ b/drivers/rtc/rtc-rzn1.c
> >
> > > +static int rzn1_rtc_probe(struct platform_device *pdev)
> > > +{
> > > +       struct rzn1_rtc *rtc;
> > > +       int ret;
> > > +
> > > +       rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > > +       if (!rtc)
> > > +               return -ENOMEM;
> > > +
> > > +       platform_set_drvdata(pdev, rtc);
> > > +
> > > +       rtc->clk = devm_clk_get(&pdev->dev, "hclk");
> > > +       if (IS_ERR(rtc->clk))
> > > +               return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk), "Missing hclk\n");
> >
> > As you don't care about the clock rate, only about enabling/disabling
> > the clock, I recommend using Runtime PM instead of explicit clock
> > handling.
>
> That's right.
>
> > That does depend on:
> > [PATCH v3 4/8] soc: renesas: rzn1: Select PM and PM_GENERIC_DOMAINS configs[1]
> > [PATCH v3 5/8] ARM: dts: r9a06g032: Add missing '#power-domain-cells'[2]
>
> There should not be any dependency with the RTC tree so that should not
> be too complex to handle.

Agreed.

> > and on documenting the power-domains property to the RTC DT bindings,
> > and on adding a proper power-domains property to the RTC node in DTS.
>
> Right.
>
> Do we need to define these properties in the UART, DMA and NAND
> controller nodes as well? I seem to remember that you mentioned it but
> I don't recall for which one and I was too focused (lazy?) on other
> features so I forgot about it.

It would be good to have them everywhere.  The initial RZ/N1D DTS
lacked them, as it only had the uart driver, which is not a Renesas
IP core.

The dw-dmac and 8250_dw drivers already use Runtime PM.
The renesas-nand-controller driver can be updated later, after the
power-domains properties have been added to the DTS.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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