2024-02-23 07:08:02

by Peng Fan (OSS)

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

From: Peng Fan <[email protected]>

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 <[email protected]>
---

V1:
Based on https://lore.kernel.org/all/[email protected]/
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



2024-02-26 13:18:14

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type

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 <[email protected]>
>
> 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 <[email protected]>
> ---
>
> V1:
> Based on https://lore.kernel.org/all/20240223-pinctrl-scmi-v4-0-
> [email protected]/
> 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


2024-02-26 15:25:01

by Cristian Marussi

[permalink] [raw]
Subject: Re: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type

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

2024-02-27 00:52:30

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type

> Subject: Re: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type
>
> 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?

No need the last 2 downstream patches, the custom OEM params could
make it work, I just need to convert the imx95-pinfunc.h header file
to new format.

We might be able to only keep imx,pin-conf in future after some firmware
work.

Thanks,
Peng.

>
> Thanks,
> Cristian

2024-02-29 13:52:15

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type

On Fri, Feb 23, 2024 at 8:07 AM Peng Fan (OSS) <[email protected]> wrote:

> From: Peng Fan <[email protected]>
>
> 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 <[email protected]>

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.

If the spec allows for this, and NXP Freescale is using it, I will just
have to accept it.

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?

Yours,
Linus Walleij

2024-03-14 02:01:32

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH] pinctrl: scmi: support i.MX OEM pin configuration type

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) <[email protected]>
> wrote:
>
> > From: Peng Fan <[email protected]>
> >
> > 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 <[email protected]>
>
> 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