Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support

[Cristian Marussi <cristian.marussi@xxxxxxx>]

On Sun, Feb 04, 2024 at 09:29:25AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol
> > protocol basic support
> > 
> > On Thu, Feb 01, 2024 at 07:14:17AM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2
> > > > pincontrol protocol basic support
> > > >
> > 
> > Hi Peng,
> > 
> > > > On Mon, Jan 29, 2024 at 1:37 PM Peng Fan <peng.fan@xxxxxxxxxxx>
> > wrote:
> > > >
> > > > > And for i.MX95 OEM extenstion, do you have any suggestions?
> > > > > I have two points:
> > > > > 1. use vendor compatible. This would also benefit when supporting
> > > > > vendor protocol.
> > > > > 2. Introduce a property saying supporting-generic-pinconf
> > > > >
> > > > > How do you think?
> > > >
> > > > While I don't know how OEM extensions to SCMI were designed, the pin
> > > > control subsystem has the philosophy that extensions are for minor
> > > > fringe stuff, such as a pin config option that no other silicon is
> > > > using and thus have no use for anyone else. Well that is actually
> > > > all the custom extensions we have.
> > > > (This notion is even carried over to SCMI pinctrl.)
> > > >
> > > > The i.MX95 OEM extension is really odd to me, it looks like a
> > > > reimplementation of the core aspects of SCMI pin control, and looks
> > > > much more like the old i.MX drivers than like the SCMI driver.
> > >
> > > i.MX SCMI pin protocol conf settings follows non-SCMI pin conf settings.
> > >
> > 
> > It is not just a matter of using custom SCMI OEM types, it is the whole
> > layout/definitions of the i.MX pin/groups/funcs DT bindings that deviates
> > from the generic DT bindings layout as handled and expected by the Linux
> > Pinctrl subsystem (AFAIU), while the SCMI Pinctrl driver as it stands in this
> > series, was conceived, designed and implemented originally by Oleksii to just
> > use the generic existing Pinctrl DT bindings; as a consequence, in your i.MX
> > extensions, you had to add a dedicated i.MX DT parser to interpret the
> > protocol@19 DT snippet in a completely different way, to try to stick your
> > custom solution on top of the generic one.
> 
> The two links shows the drop of i.MX generic pinconf
> https://patchwork.ozlabs.org/project/linux-gpio/patch/1541149669-10857-7-git-send-email-aisheng.dong@xxxxxxx/
> https://lore.kernel.org/all/20230302072132.1051590-1-linux@xxxxxxxxxxxxxxxxxx/
> 
> For non-scmi platforms, the generic pinconf was supported
> for i.MX7ULP for a while, and then dropped in the end per i.MX maintainers
> and agreed by Linus.
> 
> For i.MX95 SCMI platforms, the firmware design is simple and use similar
> programming model to simplify firmware design.
> 
> Using generic pinconf means the firmware needs exporting groups/functions/pins
> and etc, the firmware design will be complicated and code size enlarged.
> 

Understood.

> I have no better ideas without introducing a compatible for dt map hook.
> 
> Build exclusive is not acceptable for distro support.
> 

Indeed, and I understand that, but in any case it wont be required for
both the generic pinconf and the IMX way to coexist and live together on
the same system/platform at runtime, right ?

So, I was thinking as an alternative, while still building all
(generic+IMX) in defconfig, wouldn't be possible to detect at runtime in the
pinctrl-scmi probe() which is the type of binding used in the DT (i.e.
generic VS imx), before registering with the core Pinctrl subsystem, and so
act accordingly parsing and providing different callbacks based on that ?

I mean, without any compatible addition, just looking up the protocol@19
node content @probe and switch to iMX callbacks if the fsl,pins binding
is found ?

Not sure is this is totally viable, nor clean or that it is not less horrific
from the Pinctrl_subsystem/DT point of view for Linus ... just an idea to think
about or discard.

> So the last options is i.MX95 switch back to VENDOR protocol ID saying
> 0x82. But this means to exports functions of pinctrl-scmi.c and reused
> by pinctrl-scmi-imx.c.  If you agree, I will ask firmware developer
> switch back to a new SCMI ID, and I will use new ID for i.mx pinctrl
> driver.
> 

I dont think there is really any chance to upstream a vendor protocol that
is a bare copy of a standard protocol just to workaround this...and if
this is going to be the end-result you can just keep your current 2
small 'IMX custom-compatible' patches downstream.

> But in the end I would think when more SCMI vendor protocols
> comes in, saying vendor A and vendor B both use ID 0x81,
> both use 0x81 as RTC functions, same issue will come back.
>

This wont be a problem once the RFC[1] I posted last week is in...unless
someone objects, you will have to identify your vendor protocol module
at build time ALSO with the vendor/sub_vendor ID string exposed by your fw...
...so each vendor will effectively have the full vendor protocol space
number at their disposal.

Indeed this week, I hope to review your series about custom protocols
and the qualcomm ones and ask both for feedback on [1] and then to
rework those series on some non-RFC similar to 1 that I will post
afterwards.

Thanks,
Cristian

[1]: https://lore.kernel.org/linux-arm-kernel/20240122122437.546621-1-cristian.marussi@xxxxxxx/