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

On Thu, Feb 06, 2025 at 07:05:08PM +0800, Peng Fan wrote:

Hi Dan,

On Thu, Feb 06, 2025 at 11:02:04AM +0300, Dan Carpenter wrote:

quoted

On Mon, Jan 20, 2025 at 03:13:30PM +0800, Peng Fan (OSS) wrote:

quoted

From: Peng Fan <peng.fan@nxp.com>

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.

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.

Introduce machine_allowlist and machine_blocklist to allow or block
the creation of scmi devices to address above issues.

machine_blocklist is non-vendor protocols, but vendor has its own
implementation. Saying need to block pinctrl-scmi.c on i.MX95.
machine_allowlist is for vendor protocols. Saying vendor A drivers only
allow vendor A machine, vendor B machines only allow vendor B machine.

I think patches 2-4 should be combined into one patch.  This commit

They are in different subsystems, so I separate them.

I mean if the i.MX driver prevents the generic driver from working then
we need a Fixes tag.  It really makes it simpler to understand and backport
if they're sent as one patch.  Normally we would collect Acks from the
maintainers who're involved and but still do it as one patch.

quoted

message is a bit confusing.  I don't really understand how the
"fwnode device points to the scmi device".  I understand vaguely
what that means but in terms of code, I couldn't point to it.

Sorry for not being clear.

The devlink framework will take i.MX as pinctrl provider, because the
fwnode is occupied by i.MX pinctrl scmi device which is created earlier
than generic pinctrl scmi device.

Ah.  Got it.  Thanks.

quoted

quoted

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.

You're describing the code before.  Is it a problem that only one driver
is probed successfully?  I thought that would be fine.  What's the
problem?

VendorA 0x80
VendorB 0x80

If both drivers runs into probe, VenderB 0x80 driver may crash VendorA firmware
if the firmware not designed well.

Not big issue. I just think we should block the probe.

This is a theoretical issue for now.  I would just leave it out of the
commit message because it's kind of confusing and it might not even happen
in real life.

regards,
dan carpenter