RE: [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map

[Peng Fan <peng.fan@xxxxxxx>]

> Subject: Re: [PATCH 7/7] pinctrl: scmi: implement
> pinctrl_scmi_imx_dt_node_to_map
> 
> On Fri, Dec 15, 2023 at 07:56:35PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses
> OEM
> > Pin Configuration type, so need i.MX specific dt_node_to_map.
> >
> 
> This does not even compile for me, as of now, when configuring the Pinctrl
> SCMI driver as a module with your IMX custom additions. (I think the
> Makefile with the additional pinctrl-imx is wrong in how describes the objects
> composing the pinctrl-scmi module with IMX addons...)
> 
> ERROR: modpost: "pinctrl_scmi_imx_dt_node_to_map"
> [drivers/pinctrl/pinctrl-scmi.ko] undefined!
> make[3]: *** [dev/src/linux/scripts/Makefile.modpost:145: Module.symvers]
> Error 1
> make[2]: *** [dev/src/linux/Makefile:1863: modpost] Error 2
> make[1]: *** [dev/src/linux/Makefile:234: __sub-make] Error 2
> make[1]: Leaving directory dev/out_linux
> make: *** [Makefile:234: __sub-make] Error 2

Oh, sorry for this. I could post a new version if you require. But before
that we may better align on the approach on how to support i.MX.

> 
> More in general, I think that this NXP OEM specific additions, which are in
> general welcome (and indeed as you know part of the spec was modified to
> allow for OEM specific needs), do NOT belong to this generic SCMI Pinctrl
> driver, because the driver from Oleksii/EPAM was born as a generic SCMI
> driver and it fits perfectly with the Generic Pinctrl Linux subsystem and
> related generic bindings parsing: now with this you are trying to stick a
> custom OEM slight varied behaviour (and related binding) on top of a generic
> thing.
> 
> And this choice leads to a number of additional changes in the SCMI core to
> support an even more complex handling of SCMI devices, which is already too
> complex IMO..
> 
> IOW...I dont think that the whole idea of the per-protocol optional
> compatible to be able to select slightly different behaviours/parsing would
> have a great chance to fly sincerely...
> 
> I know there is an issue with having a completely distinct SCMI IMX pinctrl
> driver that uses the same protocol node @19 (without the need for the
> compatible trick) due to the way in which the Pinctrl subsystem searches for
> devices (by of_node)...I'll think about an alternative way to allow this but I am
> not sure (as you saw) that would be so easily doable...

For all protocols supports VENDOR extension attributes, we need a way
to handle I think. 

As Linus wrote in
https://lore.kernel.org/all/CACRpkdaRY+rU+md-r5gVyFH5ATt3Pqp9=M4=+WArYkfVLAFdpw@xxxxxxxxxxxxxx/:

We may need:
protocol@19 {
    compatible = "vendor,soc-scmi-pinctrl";
(...)

> 
> Also, I am wondering if this is really a problem in reality since I would NOT
> expect you to load/ship both the OEM/NXP custom specific SCMI pinctrl
> driver AND the generic one on the same platform (after having made them
> distinct I mean...) am I wrong ?

You are right, but that means the upstream ARM64 defconfig will not 
able to support both i.MX9 and others.

Thanks,
Peng.

> (so you could even made them exclude each other at compile time...far from
> being the best option I agree...)
> 
> Thanks,
> Cristian