From: Peng Fan <peng.fan@nxp.com>
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.
So bypass setting fwnode for scmi pinctrl devices that non
compatible with socs.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/bus.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 12190d4dabb65484543044b4424fbe3b67245466..87665b09c8ff492953c8300f80ed73eab6cce4fd 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -345,6 +345,11 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev)
device_unregister(&scmi_dev->dev);
}
+static const char * const scmi_pinctrl_imx_lists[] = {
+ "fsl,imx95",
+ NULL
+};
+
static int
__scmi_device_set_node(struct scmi_device *scmi_dev, struct device_node *np,
int protocol, const char *name)
@@ -353,6 +358,15 @@ __scmi_device_set_node(struct scmi_device *scmi_dev, struct device_node *np,
if ((protocol == SCMI_PROTOCOL_PERF) && !strcmp(name, "cpufreq"))
return 0;
+ if (protocol == SCMI_PROTOCOL_PINCTRL) {
+ if (!strcmp(name, "pinctrl") &&
+ of_machine_compatible_match(scmi_pinctrl_imx_lists))
+ return 0;
+ if (!strcmp(name, "pinctrl-imx") &&
+ !of_machine_compatible_match(scmi_pinctrl_imx_lists))
+ return 0;
+ }
+
device_set_node(&scmi_dev->dev, of_fwnode_handle(np));
return 0;
--
2.37.1On Wed, Dec 25, 2024 at 04:20:45PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> 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.
>
> So bypass setting fwnode for scmi pinctrl devices that non
> compatible with socs.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/bus.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
> index 12190d4dabb65484543044b4424fbe3b67245466..87665b09c8ff492953c8300f80ed73eab6cce4fd 100644
> --- a/drivers/firmware/arm_scmi/bus.c
> +++ b/drivers/firmware/arm_scmi/bus.c
> @@ -345,6 +345,11 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev)
> device_unregister(&scmi_dev->dev);
> }
>
> +static const char * const scmi_pinctrl_imx_lists[] = {
> + "fsl,imx95",
> + NULL
> +};
> +
> static int
> __scmi_device_set_node(struct scmi_device *scmi_dev, struct device_node *np,
> int protocol, const char *name)
> @@ -353,6 +358,15 @@ __scmi_device_set_node(struct scmi_device *scmi_dev, struct device_node *np,
> if ((protocol == SCMI_PROTOCOL_PERF) && !strcmp(name, "cpufreq"))
> return 0;
>
> + if (protocol == SCMI_PROTOCOL_PINCTRL) {
> + if (!strcmp(name, "pinctrl") &&
> + of_machine_compatible_match(scmi_pinctrl_imx_lists))
> + return 0;
> + if (!strcmp(name, "pinctrl-imx") &&
> + !of_machine_compatible_match(scmi_pinctrl_imx_lists))
> + return 0;
> + }
...and same here, you could set a flag scmi_dev->avoid_devlink and
just avoid calling device_link_add instead of killing the device_node...
Thanks,
Cristian
On Wed, Dec 25, 2024 at 04:20:45PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > 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. > I wonder if we can prevent creation of "pinctrl-imx" scmi device on non i.MX platforms instead of this hack which IMO is little less hackier (and little more cleaner as we don't create problem and then fix here) than this change. -- Regards, Sudeep
On Fri, Dec 27, 2024 at 03:28:07PM +0000, Sudeep Holla wrote: > On Wed, Dec 25, 2024 at 04:20:45PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > 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. > > > > I wonder if we can prevent creation of "pinctrl-imx" scmi device on non > i.MX platforms instead of this hack which IMO is little less hackier > (and little more cleaner as we don't create problem and then fix here) > than this change. ...or indeed this is another possibility Thanks, Cristian
On Tue, Dec 31, 2024 at 06:16:12PM +0000, Cristian Marussi wrote:
>On Fri, Dec 27, 2024 at 03:28:07PM +0000, Sudeep Holla wrote:
>> On Wed, Dec 25, 2024 at 04:20:45PM +0800, Peng Fan (OSS) wrote:
>> > From: Peng Fan <peng.fan@nxp.com>
>> >
>> > 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.
>> >
>>
>> I wonder if we can prevent creation of "pinctrl-imx" scmi device on non
>> i.MX platforms instead of this hack which IMO is little less hackier
>> (and little more cleaner as we don't create problem and then fix here)
>> than this change.
>
>...or indeed this is another possibility
I am doing a patch as below, how to do you think?
With below patch, we could resolve the devlink issue and also support mutitple
vendor drivers built in, with each vendor driver has a machine_allowlist.
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 1d2aedfcfdb4..c1c45b545480 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -55,6 +55,20 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
unsigned int id = 0;
struct list_head *head, *phead = NULL;
struct scmi_requested_dev *rdev;
+ const char * const *allowlist = id_table->machine_allowlist;
+ const char * const *blocklist = id_table->machine_blocklist;
+
+ if (blocklist && of_machine_compatible_match(blocklist)) {
+ pr_debug("block SCMI device (%s) for protocol %x\n",
+ id_table->name, id_table->protocol_id);
+ return 0;
+ }
+
+ if (allowlist && !of_machine_compatible_match(allowlist)) {
+ pr_debug("block SCMI device (%s) for protocol %x\n",
+ id_table->name, id_table->protocol_id);
+ return 0;
+ }
pr_debug("Requesting SCMI device (%s) for protocol %x\n",
id_table->name, id_table->protocol_id);
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 688466a0e816..e1b822d3522f 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -950,6 +950,9 @@ struct scmi_device {
struct scmi_device_id {
u8 protocol_id;
const char *name;
+ /* Optional */
+ const char * const *machine_blocklist;
+ const char * const *machine_allowlist;
};
struct scmi_driver {
Thanks,
Peng
>
>Thanks,
>Cristian
On Fri, Dec 27, 2024 at 03:28:07PM +0000, Sudeep Holla wrote: >On Wed, Dec 25, 2024 at 04:20:45PM +0800, Peng Fan (OSS) wrote: >> From: Peng Fan <peng.fan@nxp.com> >> >> 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. >> > >I wonder if we can prevent creation of "pinctrl-imx" scmi device on non >i.MX platforms instead of this hack which IMO is little less hackier >(and little more cleaner as we don't create problem and then fix here) >than this change. I thought two ways that introduce new entries in scmi_device_id, 1. compatible string. 2. allowed machine string and blcoked machine string. Thanks, Peng > >-- >Regards, >Sudeep
© 2016 - 2026 Red Hat, Inc.