| 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 |
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
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
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 --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 */