Re: [PATCH v5 3/5] mfd: airoha: Add support for Airoha EN7581 MFD

[Lee Jones <lee@xxxxxxxxxx>]

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.

-- 
Lee Jones [李琼斯]