RE: [PATCH v11 2/6] clocksource/drivers: Add Renesas RZ/G2L MTU3a core driver

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Lee Jones <lee@kernel.org>, Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	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>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	William Breathitt Gray <william.gray@linaro.org>
Subject: RE: [PATCH v11 2/6] clocksource/drivers: Add Renesas RZ/G2L MTU3a core driver
Date: Thu, 2 Feb 2023 16:22:56 +0000	[thread overview]
Message-ID: <TYCPR01MB5933577B72FE9E63362FD76F86D69@TYCPR01MB5933.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <Y9Krcd6tPI2jdYfd@google.com>

Hi Lee Jones and Daniel Lezcano,

> Subject: Re: [PATCH v11 2/6] clocksource/drivers: Add Renesas RZ/G2L MTU3a
> core driver
> 
> On Thu, 26 Jan 2023, Biju Das wrote:
> 
> > Hi Lee Jones,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > From: Lee Jones <lee@kernel.org>
> > > Sent: Thursday, January 26, 2023 2:52 PM
> > > To: Biju Das <biju.das.jz@bp.renesas.com>
> > > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>; Philipp Zabel
> > > <p.zabel@pengutronix.de>; Thomas Gleixner <tglx@linutronix.de>;
> > > Geert Uytterhoeven <geert+renesas@glider.be>; Chris Paterson
> > > <Chris.Paterson2@renesas.com>; Prabhakar Mahadev Lad
> > > <prabhakar.mahadev- lad.rj@bp.renesas.com>;
> > > linux-renesas-soc@vger.kernel.org; Rob Herring <robh+dt@kernel.org>;
> > > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > Subject: Re: [PATCH v11 2/6] clocksource/drivers: Add Renesas RZ/G2L
> > > MTU3a core driver
> > >
> > > On Thu, 26 Jan 2023, Biju Das wrote:
> > >
> > > > Hi Daniel,
> > > >
> > > > + Rob and Krzysztof Kozlowski
> > > >
> > > > > -----Original Message-----
> > > > > From: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > > > Sent: Thursday, January 26, 2023 10:53 AM
> > > > > To: Biju Das <biju.das.jz@bp.renesas.com>; Philipp Zabel
> > > > > <p.zabel@pengutronix.de>
> > > > > Cc: Thomas Gleixner <tglx@linutronix.de>; Geert Uytterhoeven
> > > > > <geert+renesas@glider.be>; Chris Paterson
> > > > > <Chris.Paterson2@renesas.com>; Prabhakar Mahadev Lad
> > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>; linux-
> > > > > renesas-soc@vger.kernel.org; Lee Jones <lee@kernel.org>
> > > > > Subject: Re: [PATCH v11 2/6] clocksource/drivers: Add Renesas
> > > > > RZ/G2L MTU3a core driver
> > > > >
> > > > > On 13/01/2023 17:17, Biju Das wrote:
> > > > >
> > > > > [ ... ]
> > > > >
> > > > > > +config RZ_MTU3
> > > > > > +	bool "Renesas RZ/G2L MTU3a core driver"
> > > > > > +	select MFD_CORE
> > > > > > +	depends on (ARCH_RZG2L && OF) || COMPILE_TEST
> > > > > > +	help
> > > > > > +	  Select this option to enable Renesas RZ/G2L MTU3a core
> > > driver for
> > > > > > +	  the Multi-Function Timer Pulse Unit 3 (MTU3a) hardware
> > > available
> > > > > > +	  on SoCs from Renesas. The core driver shares the clk and
> > > channel
> > > > > > +	  register access for the other child devices like Counter,
> > > PWM,
> > > > > > +	  Clock Source, and Clock event.
> > > > >
> > > > > Do you really want to have this option manually selectable?
> > > > > Usually we try to avoid that and keep a silent option which is
> > > > > selected by the platform config.
> > > >
> > > > For critical drivers like CPG, Pinctrl we enable it by default by
> > > > silent
> > > option in platform config.
> > > > For the others we add it to defconfig, once the device tree
> > > > support is
> > > available.
> > > >
> > > >
> > > > >
> > > > > [ ... ]
> > > > >
> > > > > > +
> > > > > > +	ret = mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
> > > > > > +			      ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
> > > > > > +	if (ret < 0)
> > > > > > +		goto err_assert;
> > > > > > +
> > > > > > +	return devm_add_action_or_reset(&pdev->dev,
> > > rz_mtu3_reset_assert,
> > > > > > +					&pdev->dev);
> > > > > > +
> > > > > > +err_assert:
> > > > > > +	reset_control_assert(ddata->rstc);
> > > > > > +	return ret;
> > > > > > +}
> > > > >
> > > > > I'm not sure this driver falls under the clocksource umbrella
> > > > > but under mfd [cc'ed Lee Jones]
> > > > >
> > > >
> > > >
> > > > Please find [1],
> > > >
> > > > After a long discussion with dt maintainers, counter maintainer,
> > > > MFD maintainer and PWM maintainer, it is concluded to Add the core
> > > > driver to
> > > timer subsystem.
> > >
> > > Which is fine.  However, you cannot then use the MFD API.
> >
> > Is it ok to keep the bindings in timer subsystem and move the core
> > driver to MFD as it is using MFD api's?
> 
> Sounds reasonable.
> 
> I guess we'll have to see what that looks like.

As discussed, I have moved the core driver to MFD and keeping the bindings in
timer. I will be sending next version based on this.

Cheers,
Biju