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

[Marek Vasut]

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 ?

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