Re: [PATCH v2 2/4] firmware: arm_scmi: Add machine_allowlist and machine_blocklist

[Cristian Marussi]

On Mon, Jan 20, 2025 at 03:13:30PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@xxxxxxx>
> 

Hi,

> There are two cases:
> pinctrl-scmi.c and pinctrl-imx-scmi.c, both use SCMI_PROTOCOL_PINCTRL.
> If both drivers are built in, and the scmi device with name "pinctrl-imx"
> is created earlier, and the fwnode device points to the scmi device,
> non-i.MX platforms will never have the pinctrl supplier ready.
> 

The pinctrl-imx case is an unfortunate exception because you have a
custom Vendor SCMI driver using a STANDARD protocol: this was never
meant to be an allowed normal practice: the idea of SCMI Vendor extensions
is to allow Vendors to write their own Vendor protocols (>0x80) and their
own SCMI drivers on top of their custom vendor protocols.

This list-based generalization seems to me dangerous because allows the
spreading of such bad practice of loading custom Vendor drivers on top of
standard protocols using the same protocol to do the same thing in a
slightly different manner, with all the rfelated issues of fragmentation

...also I feel it could lead to an umaintainable mess of tables of
compatibles....what happens if I write a 3rd pinctrl-cristian driver on
top of it...both the new allowlist and the general pinctrl blocklist
will need to be updated.

The issue as we know is the interaction with devlink given that all of
these same-protocol devices are created with the same device_node, since
there is only one of them per-protocol in the DT....

...not sure what Sudeep thinks..just my opinion...

> Vendor A use 0x80 for feature X, Vendor B use 0x80 for feature Y.
> With both drivers built in, two scmi devices will be created, and both
> drivers will be probed. On A's patform, feature Y probe may fail, vice
> verus.
>

That's definitely an issue much worse than fw_devlink above....we indeed take
care to pick the right vendor-protocol at runtime BUT no check is peformed
against the SCMI drivers so you could end up picking up a completely unrelated
protocol operations...damn...

I think this is more esily solvable though....by adding a Vendor tag in
the device_table (like the protocols do) you could skip device creation
based on a mismatching Vendor, instead of using compatibles that are
doomed to grow indefinitely as a list....

So at this point you would have an optional Vendor and an optional flags
(as mentioned in the other thread) in the device_table...

I wonder if we can use the same logic for the above pinctrl case,
without using compatibles ?
I have not really thougth this through properly....

In general, most of these issues are somehow Vendor-dependent, so I was
also wondering if it was not the case to frame all of this in some sort
of general vendor-quirks framework that could be used also when there
are evident and NOT fixable issues on deployed FW SCMI server, so that
we will have to flex a bit the kernel tolerance to cope with existing
deployed HW that cannot be fixed fw-wise....

...BUT I thought about this even less thoroughly :P...so it could be just a
bad idea of mine...

Thanks,
Cristian