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

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

Hi Geert,

geert@linux-m68k.org wrote on Thu, 28 Apr 2022 10:58:03 +0200:

> Hi Miquel,
> 
> 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>  
> 
> Thanks for your patch!
> 
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -1548,6 +1548,13 @@ config RTC_DRV_RS5C313
> >         help
> >           If you say yes here you get support for the Ricoh RS5C313 RTC chips.
> >
> > +config RTC_DRV_RZN1
> > +       tristate "Renesas RZN1 RTC"  
> 
> RZ/N1
> 
> > +       depends on ARCH_RZN1 || COMPILE_TEST
> > +       depends on OF && HAS_IOMEM
> > +       help
> > +         If you say yes here you get support for the Renesas RZ/N1 RTC.
> > +
> >  config RTC_DRV_GENERIC
> >         tristate "Generic RTC support"
> >         # Please consider writing a new RTC driver instead of using the generic  
> 
> > --- /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.

> 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.

> [1] https://lore.kernel.org/linux-renesas-soc/20220422120850.769480-5-herve.codina@bootlin.com
> [2] https://lore.kernel.org/linux-renesas-soc/20220422120850.769480-6-herve.codina@bootlin.com

Thanks,
Miquèl