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

[Marek Vasut <marex@denx.de>]

On 3/26/25 7:42 AM, Peng Fan wrote:
>> Subject: Re: [PATCH v8 02/19] pinctrl: nxp: add a pin controller driver
>> based on SCMI pin control protocol
>>
>> On 3/26/25 2:52 AM, Peng Fan wrote:
>>>> Subject: Re: [PATCH v8 02/19] pinctrl: nxp: add a pin controller
>>>> driver based on SCMI pin control protocol
>>>>
>>>> On 3/25/25 9:06 AM, Peng Fan wrote:
>>>>
>>>> [...]
>>>>
>>>>>>> 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.
>>>> But then, why is it using the same protocol ID ?
>>>
>>> i.MX System Manager FW pinctrl follows ARM SCMI spec, the ID
>> value is
>>> 0x19.
>>>
>>>
>>> If you look at linux kernel driver
>>>
>>> drivers/pinctrl/pinctrl-scmi.c
>>> drivers/pinctrl/freescale/pinctrl-scmi-imx.c
>>
>> Is the second driver upstream at all ? I don't see it in linux-next ?
> 
> Typo: drivers/pinctrl/freescale/pinctrl-imx-scmi.c

Ah, thank you for clarifying.

>>> Both use same ID 0x19.
>>>
>>> It is fine to use below, if it is fine to you.
>>> SCMI_PROTOCOL_ID_PINCTRL_IMX = 0x19.
>> How does the kernel discern which driver it should use to
>> communicate with the SCMI provider if both drivers are compiled into
>> the kernel ?
> 
> There is a check in drivers probe. Only one will succeed.

I have to admit, this is just ... uhhh :(

But I think now, we should also have a driver-side check and two 
separate drivers, similar to this patch:

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

right ?