RE: [PATCH v5 2/5] clocksource/drivers: Add RZ/G2L MTU3 core driver

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>, Lee Jones <lee@kernel.org>
Subject: RE: [PATCH v5 2/5] clocksource/drivers: Add RZ/G2L MTU3 core driver
Date: Mon, 7 Nov 2022 14:27:09 +0000	[thread overview]
Message-ID: <OS0PR01MB5922AE3F1A115CB00E6B1599863C9@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdVghus=mN0PajCsY5CAO+sT-NQPL9NJ5e=3wtqMOcahSQ@mail.gmail.com>

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v5 2/5] clocksource/drivers: Add RZ/G2L MTU3 core driver
> 
> Hi Biju,
> 
> On Thu, Oct 27, 2022 at 4:49 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Add RZ/G2L MTU3 core driver for the Multi-Function Timer Pulse Unit 3
> > (MTU3a) hardware available on SoCs from Renesas. The core driver allow
> > to share the clk and channel between the other drivers like PWM, Clock
> > Source, Clock event and Counter.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v4->v5:
> 
> Thanks for the update!
> 
> >  * Moved core driver from MFD to timer
> >  * Child devices instatiated using mfd_add_devices()
> 
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -524,6 +524,16 @@ config SH_TIMER_MTU2
> >           Timer Pulse Unit 2 (MTU2) hardware available on SoCs from
> Renesas.
> >           This hardware comes with 16-bit timer registers.
> >
> > +config RZ_MTU3
> > +       bool "Renesas RZ/G2L MTU3 core driver"
> > +       depends on (ARCH_RZG2L && OF) || COMPILE_TEST
> 
> As you are calling mfd_add_devices(), this driver needs (a very small part
> of) the functionality provided by MFD.
> 
> If CONFIG_MFD_CORE is not set:
> 
>     aarch64-linux-gnu-ld: drivers/clocksource/rz-mtu3.o: in function
> `rz_mtu3_reset_assert':
>     rz-mtu3.c:(.text+0x38): undefined reference to `mfd_remove_devices'
>     aarch64-linux-gnu-ld: drivers/clocksource/rz-mtu3.o: in function
> `rz_mtu3_probe':
>     rz-mtu3.c:(.text+0x180): undefined reference to `mfd_add_devices'
> 
> So either you should make this depend on MFD_CORE, or move to another
> mechanism for registering the sub devices (e.g. calling
> platform_device_{alloc,add}(), cfr. rpcif_probe()).

OK. I will add depends on MFD_CORE.
Here we can make use of mfd_cell rz_mtu3_devs to define subdevices
and a single call for instantiating sub devices.

Cheers,
Biju

> 
> > +       help
> > +         Select this option to enable Renesas RZ/G2L MTU3 core driver for
> > +         the Multi-Function Timer Pulse Unit 3 (MTU3a) hardware available
> on
> > +         SoCs from Renesas. The core driver allow to share the clk and
> channel
> > +         between the others drivers like PWM, Clock Source, Clock event
> and
> > +         Counter.
> > +
> 
> 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