On 2/24/25 3:45 AM, Peng Fan wrote:

[...]

@@ -436,6 +442,11 @@ static int scmi_bind_protocols(struct udevice
*dev)
                                drv = DM_DRIVER_GET(scmi_voltage_domain);
                        }
                        break;
+               case SCMI_PROTOCOL_ID_PINCTRL:
+                       if (IS_ENABLED(CONFIG_PINCTRL_IMX_SCMI) &&

Is this pinctrl protocol really imx specific ?

If not, this needs to use some other config option to gate access to it.

Currently, it is used for some product families of the i.MX9 series products.
Is the protocol iMX specific or is it generic protocol ?

SCMI_PROTOCOL_ID_PINCTRL is not unique to iMX, but 
drivers/pinctrl/nxp/pinctrl-scmi.c (drv = DM_DRIVER_GET(scmi_pinctrl_imx)) is 
only for iMX.
This patch is changing common code, it shouldn't be littered with
vendor-specific ifdeffery or if(IS_ENABLED(...))ery . Can this be made fully
generic, similar to e.g. regulator protocol ?

In Linux Kernel, there are two drivers, pinctrl-scmi.c and pinctrl-imx-scmi.c.
Both follows ARM SCMI 3.2, but pinctrl-imx-scmi has some special settings
to align with i.mx iomuxc array based settings, mux,input,pad and etc.

In gerneral, imx part could be merged with pinctrl-scmi.c but that will
make code not clean.
In that case, why does U-Boot not have pinctrl-imx-scmi.c to contain the iMX customization too ? If this protocol is IMX specific, it shouldn't be ifdeffed in common code.

Reply via email to