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

[Peng Fan]

On Thu, Feb 06, 2025 at 12:06:03PM +0000, Cristian Marussi wrote:
>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...

This is indeed better.

>
>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....

Since i.MX pinctrl driver probe earlier than generic pinctrl scmi driver(
compilation order or whatelse may change the order in future),
so adding a vendor flag to i.MX pinctrl could work for now.

But if order changes..

Anyway I will give a look, then back here.

Thanks,
Peng.

>
>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