回复: [PATCH v6 03/20] pinctrl: nxp: add a pin controller driver based on SCMI pin control protocol

[Alice Guo (OSS)]

> -----邮件原件-----
> 发件人: Marek Vasut <marex at denx.de>
> 发送时间: 2025年3月1日 3:17
> 收件人: 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>;
> Alice Guo <alice.guo at nxp.com>
> 抄送: u-boot at lists.denx.de; Ranjani Vaidyanathan
> <ranjani.vaidyanathan at nxp.com>; Peng Fan <peng.fan at nxp.com>; Ye Li
> <ye.li at nxp.com>
> 主题: Re: [PATCH v6 03/20] pinctrl: nxp: add a pin controller driver based on
> SCMI pin control protocol
> 
> On 2/28/25 11:53 AM, Alice Guo (OSS) wrote:
> > From: Alice Guo <alice.guo at nxp.com>
> >
> > This patch provides a pinctrl driver based on SCMI pin control protocol.
> > Currently, only the PINCTRL_CONFIG_SET command is implemented.
> 
> This can also be a separate patch and go in separately , right ?
> 
> If so, send this as separate patch.
> 
> [...]
> 
> > diff --git a/drivers/pinctrl/nxp/pinctrl-scmi.c
> > b/drivers/pinctrl/nxp/pinctrl-scmi.c
> > new file mode 100644
> > index 0000000000..4a791b7e95
> > --- /dev/null
> > +++ b/drivers/pinctrl/nxp/pinctrl-scmi.c
> > @@ -0,0 +1,143 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2025 NXP
> > + */
> > +
> > +#include <asm/io.h>
> > +#include <asm/mach-imx/sys_proto.h>
> > +#include <dm.h>
> > +#include <dm/device_compat.h>
> > +#include <dm/pinctrl.h>
> > +#include <scmi_agent.h>
> > +#include <scmi_protocols.h>
> > +
> > +#include "pinctrl-imx.h"
> > +
> > +#define DAISY_OFFSET_IMX93	0x360
> > +#define DAISY_OFFSET_IMX95	0x408
> > +
> > +/* SCMI pin control types */
> > +#define PINCTRL_TYPE_MUX        192
> > +#define PINCTRL_TYPE_CONFIG     193
> > +#define PINCTRL_TYPE_DAISY_ID   194
> > +#define PINCTRL_TYPE_DAISY_CFG  195
> > +#define PINCTRL_NUM_CFGS_SHIFT  2
> > +
> > +struct imx_scmi_pinctrl_priv {
> > +	u16		daisy_offset;
> > +};
> > +
> > +static int imx_pinconf_scmi_set(struct udevice *dev, u32 mux_ofs, u32 mux,
> u32 config_val,
> > +				u32 input_ofs, u32 input_val)
> > +{
> > +	struct imx_scmi_pinctrl_priv *priv = dev_get_priv(dev);
> > +	int ret, num_cfgs = 0;
> > +	struct scmi_msg msg;
> > +
> > +	/* Call SCMI API to set the pin mux and configuration. */
> > +	struct scmi_pinctrl_config_set_out out;
> > +	struct scmi_pinctrl_config_set_in in = {
> > +		.identifier = mux_ofs / 4,
> > +		.function_id = 0xFFFFFFFF,
> > +		.attributes = 0,
> > +	};
> > +
> > +	if (mux_ofs != 0) {
> 
> if (mux) { ... } is enough, the !=0 is unnecessary.
> 
> > +		in.configs[num_cfgs].type = PINCTRL_TYPE_MUX;
> > +		in.configs[num_cfgs].val = mux;
> > +		num_cfgs++;
> > +	}
> > +
> > +	if (config_val != 0) {
> 
> DTTO
> 
> > +		in.configs[num_cfgs].type = PINCTRL_TYPE_CONFIG;
> > +		in.configs[num_cfgs].val = config_val;
> > +		num_cfgs++;
> > +	}
> > +
> > +	if (input_ofs != 0) {
> 
> DTTO
> 
> > +		in.configs[num_cfgs].type = PINCTRL_TYPE_DAISY_ID;
> > +		in.configs[num_cfgs].val = (input_ofs -  priv->daisy_offset) / 4;
> > +		num_cfgs++;
> > +		in.configs[num_cfgs].type = PINCTRL_TYPE_DAISY_CFG;
> > +		in.configs[num_cfgs].val = input_val;
> > +		num_cfgs++;
> > +	}
> > +
> > +	/* Update the number of configs sent in this call. */
> > +	in.attributes = num_cfgs << PINCTRL_NUM_CFGS_SHIFT;
> > +
> > +	msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_PINCTRL,
> > +			  SCMI_MSG_PINCTRL_CONFIG_SET, in, out);
> > +
> > +	ret = devm_scmi_process_msg(dev, &msg);
> > +	if (ret || out.status != 0) {
> > +		dev_err(dev, "Failed to set PAD = %d, daisy = %d, scmi_err = %d, ret
> = %d\n",
> > +			mux_ofs / 4, input_ofs / 4, out.status, ret);
> > +	}
> > +
> > +	return ret;
> 
> [...]
> 
> > diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h index
> > 7abb2a6f36..279ebbad44 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,
> This is the custom iMX specific pinctrl protocol, isn't it ?

Hi Marek,

Protocol id 0x19 is not i.MX specific, and it is defined in Arm® System Control and Management Interface Platform Design Document Non-Confidential Version 3.2.

For pin control protocol, drivers/pinctrl/nxp/pinctrl-scmi.c are used on i.MX95.

Best Regards,
Alice Guo

> Can you rename it to some SCMI_PROTOCOL_ID_NXP_IMX9_PINCTRL to make
> it clear this is custom protocol ?