On Wed, 09 Oct 2024, Lee Jones wrote:
> On Wed, 09 Oct 2024, Lorenzo Bianconi wrote:
>
> > On Oct 02, Lee Jones wrote:
> > > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote:
> > >
> > > > From: Christian Marangi <ansuelsmth@xxxxxxxxx>
> > > >
> > > > Support for Airoha EN7581 Multi Function Device that
> > > > expose PINCTRL functionality and PWM functionality.
> > >
> > > The device is a jumble of pinctrl registers, some of which can oscillate.
> > >
> > > This is *still* not an MFD.
> > >
> > > If you wish to spread this functionality over 2 drivers, use syscon to
> > > obtain the registers and simple-mfd to automatically probe the drivers.
> >
> > Hi Lee,
> >
> > IIUC you are suggesting two possible approaches here:
> >
> > 1- have a single driver implementing both pinctrl and pwm functionalities.
> > This approach will not let us reuse the code for future devices that
> > have just one of them in common, like pwm (but we can live with that).
>
> If you can have one without the other, then they are separate devices.
>
> > 2- use a device node like the one below (something similar to [0])
> >
> > system-controller@1fbf0200 {
> > compatible = "syscon", "simple-mfd";
> > reg = <0x0 0x1fbf0200 0x0 0xc0>;
> >
> > interrupt-parent = <&gic>;
> > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> >
> > gpio-controller;
> > #gpio-cells = <2>;
> >
> > interrupt-controller;
> > #interrupt-cells = <2>;
> >
> > pio: pinctrl {
> > compatible = "airoha,en7581-pinctrl";
> >
> > [ some pinctrl properties here ]
> > };
> >
> > #pwm-cells = <3>;
> >
> > pwm {
> > compatible = "airoha,en7581-pwm";
> > };
> > };
> >
> > Please correct me if I am wrong, but using syscon/simple-mfd as compatible
> > string for the 'parent' device, will require to introduce the compatible strings
> > even for the child devices in order to probe them, correct?
> > If so, as pointed out by Christian, this is something nacked by Rob/Krzysztof/Conor
> > (this is the main reason why we introduced a full mfd driver here).
> >
> > @Rob, Krzysztof, Conor: am I right?
>
> I don't see why separate functionality shouldn't have separate
> compatible strings, even if the registers are together. Register layout
> and functionality separation are not related.
We've been happy to support both pinctrl and pwm devices before:
git grep "\-pinctrl\|\-pwm" -- drivers/mfd
git grep "\-pinctrl\|\-pwm" -- arch/*/boot/dts
git grep "\-pinctrl" -- arch/*/boot/dts | wc -l
602
git grep "\-pwm" -- arch/*/boot/dts | wc -l
856
What makes this particular device different to all of the others?
--
Lee Jones [李琼斯]
