回复: 回复: 回复: [PATCH v4 04/20] firmware: scmi: add pin control protocol support to SCMI agent

[Alice Guo (OSS)]

> -----邮件原件-----
> 发件人: Marek Vasut <marex at denx.de>
> 发送时间: 2025年3月1日 2:58
> 收件人: Peng Fan (OSS) <peng.fan at oss.nxp.com>
> 抄送: Alice Guo (OSS) <alice.guo at oss.nxp.com>; Tom Rini
> <trini at konsulko.com>; Stefano Babic <sbabic at denx.de>; Fabio Estevam
> <festevam at gmail.com>; dl-uboot-imx <uboot-imx at nxp.com>; Lukasz
> Majewski <lukma at denx.de>; Sean Anderson <seanga2 at gmail.com>; Simon
> Glass <sjg at chromium.org>; Alper Nebi Yasak <alpernebiyasak at gmail.com>;
> u-boot at lists.denx.de; tharvey at gateworks.com; Alice Guo
> <alice.guo at nxp.com>
> 主题: Re: 回复: 回复: [PATCH v4 04/20] firmware: scmi: add pin control
> protocol support to SCMI agent
> 
> 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.

Is it acceptable to rename " drivers/pinctrl/nxp/pinctrl-scmi.c" to "drivers/pinctrl/nxp/ pinctrl-imx-scmi.c"? SCMI_PROTOCOL_ID_PINCTRL = 0x19 is not i.MX specific.