Re: [PATCH v8 02/19] pinctrl: nxp: add a pin controller driver based on SCMI pin control protocol

[Peng Fan <peng.fan@oss.nxp.com>]

On Sat, Mar 22, 2025 at 01:46:02AM +0100, Marek Vasut wrote:
>On 3/21/25 8:15 AM, Alice Guo (OSS) wrote:
>
>[...]
>
>> +static int imx_scmi_pinctrl_probe(struct udevice *dev)
>> +{
>> +	struct imx_scmi_pinctrl_priv *priv = dev_get_priv(dev);
>> +
>> +	priv->daisy_offset = is_imx93() ? DAISY_OFFSET_IMX93 : DAISY_OFFSET_IMX95;
>> +
>> +	return devm_scmi_of_get_channel(dev);
>> +}
>> +
>> +static int imx_scmi_pinctrl_bind(struct udevice *dev)
>> +{
>> +	if (IS_ENABLED(CONFIG_IMX95))
>> +		return 0;
>
>Why does this driver support iMX93 , but it is explicitly not going to bind
>on iMX93 ?
>
>In case the MX93 support is going to be added, you probably need something
>like:
>
>if (IS_ENABLED(CONFIG_IMX95) && is_imx95())
>  return 0;
>
>Because IS_ENABLED(CONFIG_IMX95) does not automatically imply that this code
>is started on MX95 , that is when is_imx95() comes into play and does runtime
>check for MX95 .
>
>> +	return -ENODEV;
>> +}
>> +
>> +U_BOOT_DRIVER(scmi_pinctrl_imx) = {
>> +	.name = "scmi_pinctrl_imx",
>> +	.id = UCLASS_PINCTRL,
>> +	.bind = imx_scmi_pinctrl_bind,
>> +	.probe = imx_scmi_pinctrl_probe,
>> +	.priv_auto = sizeof(struct imx_scmi_pinctrl_priv),
>> +	.ops = &imx_scmi_pinctrl_ops,
>> +	.flags = DM_FLAG_PRE_RELOC,
>> +};
>> diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
>> index 7abb2a6f36b..279ebbad440 100644
>> --- a/include/scmi_protocols.h
>> +++ b/include/scmi_protocols.h
>> @@ -24,6 +24,7 @@ enum scmi_std_protocol {
>>   	SCMI_PROTOCOL_ID_SENSOR = 0x15,
>>   	SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16,
>>   	SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN = 0x17,
>> +	SCMI_PROTOCOL_ID_PINCTRL = 0x19,
>If this is the IMX specific pinctrl protocol, please make sure to name it
>accordingly , SCMI_PROTOCOL_ID_PINCTRL_IMX or something .

This ID is generic ID, not i.MX specific.

i.MX SCMI pinctrl follows ARM SCMI spec, but i.MX pinctrl bindings
are different compared with ARM SCMI generic pinconf bindings, so we
need a separate driver for i.MX.

Regards,
Peng