[PATCH v8 03/19] firmware: scmi_agent: add SCMI pin control protocol support

[Marek Vasut]

On 3/21/25 8:15 AM, Alice Guo (OSS) wrote:
[...]

> @@ -352,6 +358,22 @@ static int scmi_fill_base_info(struct udevice *agent, struct udevice *dev)
>   	return 0;
>   }
>   
> +static struct driver *scmi_proto_driver_get(unsigned int proto_id)
> +{
> +	struct scmi_proto_driver *start, *entry;
> +	int n_ents;
> +
> +	start = ll_entry_start(struct scmi_proto_driver, scmi_proto_driver);
> +	n_ents = ll_entry_count(struct scmi_proto_driver, scmi_proto_driver);
> +
> +	for (entry = start; entry != start + n_ents; entry++) {
> +		if (entry->match->proto_id == proto_id)
> +			return entry->driver;
> +	}
> +
> +	return NULL;
> +}

Ideally, the addition of core code like scmi_proto_driver_get() should 
be a separate preparatory patch.

Please also make sure to convert the four SCMI protocol drivers to 
U_BOOT_SCMI_PROTO_DRIVER() , otherwise the code become inconsistent. 
That should be easy, since there are literally four SCMI protocol drivers.

>   /*
>    * SCMI agent devices binds devices of various uclasses depending on
>    * the FDT description. scmi_bind_protocol() is a generic bind sequence
> @@ -436,6 +458,9 @@ static int scmi_bind_protocols(struct udevice *dev)
>   				drv = DM_DRIVER_GET(scmi_voltage_domain);
>   			}
>   			break;
> +		case SCMI_PROTOCOL_ID_PINCTRL:
> +			drv = scmi_proto_driver_get(SCMI_PROTOCOL_ID_PINCTRL);

Please have a look at patch I just sent and give it a try:

[PATCH] power: regulator: scmi: Move regulator subnode hack to 
scmi_regulator

With that and the conversion above in place, you should be able to 
entirely eliminate this switch-case code and make use of the full 
potential of your newly added scmi_proto_driver_get(), this way:

diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c 
b/drivers/firmware/scmi/scmi_agent-uclass.c
index 09a25d0067c..98e0c4d7c93 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -431,34 +431,10 @@ static int scmi_bind_protocols(struct udevice *dev)

  		drv = NULL;
  		name = ofnode_get_name(node);
-		switch (protocol_id) {
-		case SCMI_PROTOCOL_ID_POWER_DOMAIN:
-			if (CONFIG_IS_ENABLED(SCMI_POWER_DOMAIN) &&
-			    scmi_protocol_is_supported(dev, protocol_id))
-				drv = DM_DRIVER_GET(scmi_power_domain);
-			break;
-		case SCMI_PROTOCOL_ID_CLOCK:
-			if (CONFIG_IS_ENABLED(CLK_SCMI) &&
-			    scmi_protocol_is_supported(dev, protocol_id))
-				drv = DM_DRIVER_GET(scmi_clock);
-			break;
-		case SCMI_PROTOCOL_ID_RESET_DOMAIN:
-			if (IS_ENABLED(CONFIG_RESET_SCMI) &&
-			    scmi_protocol_is_supported(dev, protocol_id))
-				drv = DM_DRIVER_GET(scmi_reset_domain);
-			break;
-		case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
-			if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) &&
-			    scmi_protocol_is_supported(dev, protocol_id))
-				drv = DM_DRIVER_GET(scmi_voltage_domain);
-			break;
-		case SCMI_PROTOCOL_ID_PINCTRL:
-			drv = scmi_proto_driver_get(SCMI_PROTOCOL_ID_PINCTRL);
-			break;
-		default:
-			break;
-		}
+		if (!scmi_protocol_is_supported(dev, protocol_id))
+			continue;

+		drv = scmi_proto_driver_get(protocol_id);
  		if (!drv) {
  			dev_dbg(dev, "Ignore unsupported SCMI protocol %#x\n",
  				protocol_id);

> +			break;
>   		default:
>   			break;
>   		}
> diff --git a/drivers/pinctrl/nxp/pinctrl-imx-scmi.c b/drivers/pinctrl/nxp/pinctrl-imx-scmi.c
> index 0ed971369a3..dbd6ff437e5 100644
> --- a/drivers/pinctrl/nxp/pinctrl-imx-scmi.c
> +++ b/drivers/pinctrl/nxp/pinctrl-imx-scmi.c
> @@ -9,6 +9,7 @@
>   #include <dm/device_compat.h>
>   #include <dm/pinctrl.h>
>   #include <scmi_agent.h>
> +#include <scmi_agent-uclass.h>
>   #include <scmi_protocols.h>
>   
>   #include "pinctrl-imx.h"
> @@ -150,3 +151,9 @@ U_BOOT_DRIVER(scmi_pinctrl_imx) = {
>   	.ops = &imx_scmi_pinctrl_ops,
>   	.flags = DM_FLAG_PRE_RELOC,
>   };
> +
> +static struct scmi_proto_match match = {
> +	.proto_id = SCMI_PROTOCOL_ID_PINCTRL,
> +};


This should be a zero terminated array I think ?

static struct scmi_proto_match match[] = {
   { SCMI_PROTOCOL_ID_PINCTRL },
   { /* Sentinel */ }
};

[...]