diff mbox series

pinctrl: scmi: support i.MX OEM pin configuration type

Message ID 20240223071557.2681316-1-peng.fan@oss.nxp.com
State New
Headers show
Series pinctrl: scmi: support i.MX OEM pin configuration type | expand

Commit Message

Peng Fan (OSS) Feb. 23, 2024, 7:15 a.m. UTC
From: Peng Fan <peng.fan@nxp.com>

i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses
OEM Pin Configuration type, so extend the driver to support custom
params.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V1:
 Based on https://lore.kernel.org/all/20240223-pinctrl-scmi-v4-0-10eb5a379274@nxp.com/
 This is an reimplementation for supporting i.MX95 OEM settings.
 With this patch, the dts will be like:

+#define IMX95_PAD_SD1_CLK__USDHC1_CLK(val)	\
+	sd1clk {				\
+		pins = "sd1clk";		\
+		imx,func-id = <0>;		\
+		imx,pin-conf = <val>;		\
+	}
  ....
+
+	pinctrl_usdhc1: usdhc1grp {
+		IMX95_PAD_SD1_CLK__USDHC1_CLK(0x158e);
+		IMX95_PAD_SD1_CMD__USDHC1_CMD(0x138e);
 ....
+	};

 drivers/pinctrl/pinctrl-scmi.c | 10 ++++++++++
 drivers/pinctrl/pinctrl-scmi.h | 15 +++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-scmi.h

Comments

Peng Fan Feb. 26, 2024, 1:16 p.m. UTC | #1
Hi Linus, Sudeep, Cristian,

> Subject: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type

Sorry to ping early, but this impacts the design and i.MX95 SoC upstream(
although I removed pinctrl to let uboot init pinmux as of now), so I would
like see whether are you ok with the approach or not. This is the best as
of now I could think out to not adding more size to firmware and make the
dts format similar as previous i.MX.

Thanks,
Peng.

> 
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses OEM
> Pin Configuration type, so extend the driver to support custom params.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> V1:
>  Based on https://lore.kernel.org/all/20240223-pinctrl-scmi-v4-0-
> 10eb5a379274@nxp.com/
>  This is an reimplementation for supporting i.MX95 OEM settings.
>  With this patch, the dts will be like:
> 
> +#define IMX95_PAD_SD1_CLK__USDHC1_CLK(val)	\
> +	sd1clk {				\
> +		pins = "sd1clk";		\
> +		imx,func-id = <0>;		\
> +		imx,pin-conf = <val>;		\
> +	}
>   ....
> +
> +	pinctrl_usdhc1: usdhc1grp {
> +		IMX95_PAD_SD1_CLK__USDHC1_CLK(0x158e);
> +		IMX95_PAD_SD1_CMD__USDHC1_CMD(0x138e);
>  ....
> +	};
> 
>  drivers/pinctrl/pinctrl-scmi.c | 10 ++++++++++  drivers/pinctrl/pinctrl-scmi.h
> | 15 +++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-scmi.h
> 
> diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index
> f2fef3fb85ae..e58f1aaf9963 100644
> --- a/drivers/pinctrl/pinctrl-scmi.c
> +++ b/drivers/pinctrl/pinctrl-scmi.c
> @@ -19,6 +19,7 @@
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/pinctrl/pinmux.h>
> 
> +#include "pinctrl-scmi.h"
>  #include "pinctrl-utils.h"
>  #include "core.h"
>  #include "pinconf.h"
> @@ -472,6 +473,13 @@ static const struct pinconf_ops
> pinctrl_scmi_pinconf_ops = {
>  	.pin_config_config_dbg_show = pinconf_generic_dump_config,  };
> 
> +static const struct pinconf_generic_params pinctrl_scmi_oem_dt_params[] =
> {
> +	{"imx,func-id", IMX_SCMI_PIN_MUX, -1},
> +	{"imx,daisy-id", IMX_SCMI_PIN_DAISY_ID, -1},
> +	{"imx,daisy-conf", IMX_SCMI_PIN_DAISY_CFG, -1},
> +	{"imx,pin-conf", IMX_SCMI_PIN_CONF, -1}, };
> +
>  static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
>  				 unsigned int *nr_pins,
>  				 const struct pinctrl_pin_desc **pins) @@ -
> 548,6 +556,8 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
>  	pmx->pctl_desc.pctlops = &pinctrl_scmi_pinctrl_ops;
>  	pmx->pctl_desc.pmxops = &pinctrl_scmi_pinmux_ops;
>  	pmx->pctl_desc.confops = &pinctrl_scmi_pinconf_ops;
> +	pmx->pctl_desc.custom_params = pinctrl_scmi_oem_dt_params;
> +	pmx->pctl_desc.num_custom_params =
> +ARRAY_SIZE(pinctrl_scmi_oem_dt_params);
> 
>  	ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc.npins,
>  				    &pmx->pctl_desc.pins);
> diff --git a/drivers/pinctrl/pinctrl-scmi.h b/drivers/pinctrl/pinctrl-scmi.h new
> file mode 100644 index 000000000000..fcc61bc19c98
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-scmi.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#ifndef __DRIVERS_PINCTRL_SCMI_H
> +#define __DRIVERS_PINCTRL_SCMI_H
> +
> +/* OEM VENDOR Pin Configuration Type */
> +#define IMX_SCMI_PIN_MUX	192
> +#define IMX_SCMI_PIN_CONF	193
> +#define IMX_SCMI_PIN_DAISY_ID	194
> +#define IMX_SCMI_PIN_DAISY_CFG	195
> +
> +#endif /* __DRIVERS_PINCTRL_SCMI_H */
> --
> 2.37.1
Cristian Marussi Feb. 26, 2024, 3:17 p.m. UTC | #2
On Mon, Feb 26, 2024 at 01:16:51PM +0000, Peng Fan wrote:
> Hi Linus, Sudeep, Cristian,
> 
> > Subject: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type
> 
> Sorry to ping early, but this impacts the design and i.MX95 SoC upstream(
> although I removed pinctrl to let uboot init pinmux as of now), so I would
> like see whether are you ok with the approach or not. This is the best as
> of now I could think out to not adding more size to firmware and make the
> dts format similar as previous i.MX.
> 

I'll let Linus and Sudeep argument better, but, for my understanding,
does this solve all the issue with supporting custom iMX DT pinctrl
bindings on top of the current SCMI pinctrl generic driver without the
need of your last 2 downstream patches, or I am missing somethimg?

Thanks,
Cristian
Peng Fan March 14, 2024, 2:01 a.m. UTC | #3
Hi Linus,

Sorry for late reply.

> Subject: Re: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type
> 
> On Fri, Feb 23, 2024 at 8:07 AM Peng Fan (OSS) <peng.fan@oss.nxp.com>
> wrote:
> 
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses
> OEM
> > Pin Configuration type, so extend the driver to support custom params.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> 
> I can't really say much about this as pinctrl maintainer other than that it
> makes me a bit unhappy that i.MX95 is not using the "default"
> SCMI pinctrl bindings.

Sorry about that. I check with our SCMI firmware owner, we could
not afford the memory size, it would require a massive amount of
data in arrays to match pins with functions and then figure out the
mux value for that pin
> 
> If the spec allows for this, and NXP Freescale is using it, I will just have to
> accept it.

In the spec, Table 24 Pin Configuration Type and Enumerations, 192 -255
is for OEM specific units

> 
> It feels like that's the old NXP Freescale pin controller living on just hidden
> behind SCMI, so potentially it should also share code with the old i.MX pin
> controller driver. But I think you wrote part of that driver so you would be the
> best to ask about that in any case I think?

The scmi imx extension code not able to share code with the non-scmi code,
The scmi protocol requires config type and config value, so we need new
way to pack the data.

Thanks,
Peng.
 
> 
> Yours,
> Linus Walleij
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index f2fef3fb85ae..e58f1aaf9963 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -19,6 +19,7 @@ 
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 
+#include "pinctrl-scmi.h"
 #include "pinctrl-utils.h"
 #include "core.h"
 #include "pinconf.h"
@@ -472,6 +473,13 @@  static const struct pinconf_ops pinctrl_scmi_pinconf_ops = {
 	.pin_config_config_dbg_show = pinconf_generic_dump_config,
 };
 
+static const struct pinconf_generic_params pinctrl_scmi_oem_dt_params[] = {
+	{"imx,func-id", IMX_SCMI_PIN_MUX, -1},
+	{"imx,daisy-id", IMX_SCMI_PIN_DAISY_ID, -1},
+	{"imx,daisy-conf", IMX_SCMI_PIN_DAISY_CFG, -1},
+	{"imx,pin-conf", IMX_SCMI_PIN_CONF, -1},
+};
+
 static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 				 unsigned int *nr_pins,
 				 const struct pinctrl_pin_desc **pins)
@@ -548,6 +556,8 @@  static int scmi_pinctrl_probe(struct scmi_device *sdev)
 	pmx->pctl_desc.pctlops = &pinctrl_scmi_pinctrl_ops;
 	pmx->pctl_desc.pmxops = &pinctrl_scmi_pinmux_ops;
 	pmx->pctl_desc.confops = &pinctrl_scmi_pinconf_ops;
+	pmx->pctl_desc.custom_params = pinctrl_scmi_oem_dt_params;
+	pmx->pctl_desc.num_custom_params = ARRAY_SIZE(pinctrl_scmi_oem_dt_params);
 
 	ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc.npins,
 				    &pmx->pctl_desc.pins);
diff --git a/drivers/pinctrl/pinctrl-scmi.h b/drivers/pinctrl/pinctrl-scmi.h
new file mode 100644
index 000000000000..fcc61bc19c98
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-scmi.h
@@ -0,0 +1,15 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef __DRIVERS_PINCTRL_SCMI_H
+#define __DRIVERS_PINCTRL_SCMI_H
+
+/* OEM VENDOR Pin Configuration Type */
+#define IMX_SCMI_PIN_MUX	192
+#define IMX_SCMI_PIN_CONF	193
+#define IMX_SCMI_PIN_DAISY_ID	194
+#define IMX_SCMI_PIN_DAISY_CFG	195
+
+#endif /* __DRIVERS_PINCTRL_SCMI_H */