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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Lee Jones,

Thanks for the feedback.

> -----Original Message-----
> From: Lee Jones <lee@xxxxxxxxxx>
> Sent: Thursday, January 26, 2023 2:52 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>; Philipp Zabel
> <p.zabel@xxxxxxxxxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Geert
> Uytterhoeven <geert+renesas@xxxxxxxxx>; Chris Paterson
> <Chris.Paterson2@xxxxxxxxxxx>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@xxxxxxxxxxxxxx>; linux-renesas-soc@xxxxxxxxxxxxxxx; Rob Herring
> <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> > > Sent: Thursday, January 26, 2023 10:53 AM
> > > To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>; Philipp Zabel
> > > <p.zabel@xxxxxxxxxxxxxx>
> > > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Geert Uytterhoeven
> > > <geert+renesas@xxxxxxxxx>; Chris Paterson
> > > <Chris.Paterson2@xxxxxxxxxxx>; Prabhakar Mahadev Lad
> > > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>; linux-
> > > renesas-soc@xxxxxxxxxxxxxxx; Lee Jones <lee@xxxxxxxxxx>
> > > 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?

Please let me know

Thanks and regards,
Biju


> 
> > [1]
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> > hwork.kernel.org%2Fproject%2Flinux-renesas-soc%2Fpatch%2F2022101014522
> > 2.1047748-2-biju.das.jz%40bp.renesas.com%2F&data=05%7C01%7Cbiju.das.jz
> > %40bp.renesas.com%7C4c5896d8c0064d5e261808dafface390%7C53d82571da1947e
> > 49cb4625a166a4a2a%7C0%7C0%7C638103415256934306%7CUnknown%7CTWFpbGZsb3d
> > 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> > 3000%7C%7C%7C&sdata=2TkN2%2FEWCvC9yAeiR4fW0ix84AfiD41BIuHMuKMDGnw%3D&r
> > eserved=0
> 
>   "The TL;DR is: if you're not using the MFD Core (and including
>   mfd/core.h), it's not an MFD.  You *could* split this up into its
>   component parts, place them into their own subsystems and use an MFD
>   core driver to register them all, but as Thierry says, this is not a
>   hard requirement either."






[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux