linux-kernel - RE: [RFC] scmi: pinctrl: support i.MX9

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DU0PR04MB9417FF49B077756B1F9C91AC881DA@DU0PR04MB9417.eurprd04.prod.outlook.com>
Date:   Thu, 24 Aug 2023 12:52:58 +0000
From:   Peng Fan <peng.fan@....com>
To:     Sudeep Holla <sudeep.holla@....com>,
        Linus Walleij <linus.walleij@...aro.org>
CC:     "Peng Fan (OSS)" <peng.fan@....nxp.com>,
        "oleksii_moisieiev@...m.com" <oleksii_moisieiev@...m.com>,
        Aisheng Dong <aisheng.dong@....com>,
        "festevam@...il.com" <festevam@...il.com>,
        Jacky Bai <ping.bai@....com>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        dl-linux-imx <linux-imx@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: RE: [RFC] scmi: pinctrl: support i.MX9

Hi Sudeep,

> Subject: Re: [RFC] scmi: pinctrl: support i.MX9
> 
> Hi Linus,
> 
> Thanks a lot for the quick response.
> 
> On Thu, Aug 24, 2023 at 10:43:20AM +0200, Linus Walleij wrote:
> > On Thu, Aug 24, 2023 at 9:01 AM Peng Fan (OSS) <peng.fan@....nxp.com>
> wrote:
> >
> > > This patch is just to introduce i.MX support to see whether people
> > > have comments for the design.
> >
> > Very interesting!
> >
> > > The binding format:
> > > <mux_reg conf_reg input_reg mux_mode input_val>
> > > dts:
> > >         pinctrl_uart1: uart1grp {
> > >                 fsl,pins = <
> > >                         MX93_PAD_UART1_RXD__LPUART1_RX                  0x31e
> > >                         MX93_PAD_UART1_TXD__LPUART1_TX                  0x31e
> > >                 >;
> > >         };
> > >
> > > i.MX pinctrl not use generic pinconf, this has been agreeed by
> > > maintainers before.
> >
> > Yes, it has historical reasons.
> >
> 
> Good to know.
> 
> > > So after moving to SCMI, we will still keep the same binding format,
> > > and i.MX SCMI firmware also use same format when configure
> > > registers. So we need to use i.MX specific dt_node_to_map function.
> >
> > I thought the idea with SCMI was to abstract and hide the
> > characteristics of the underlying hardware. I.e. the firmware is to
> > present groups and functions and generic config options and then the
> driver will use these.
> >
> 
> Correct.
> 
> > This patch, it seems, creates a hybrid between the old freescale
> > driver and the SCMI firmware communication link where the SCMI is just
> > a transport mechanism to something inside SCMI that poke the same
> > registers that userspace could poke, if it could only access these
> > registers.
> >
> > I.e using SCMI on this platform isn't creating any abstraction of the
> > pin control hardware, it is merely making things more complex and also
> > slower bymaking the registers only accessible from this SCMI link.
> >
> 
> Agreed.
> 
> I don't have much knowledge on generic pinmux conf and suggested Peng to
> post the RFC to start the discussion instead of getting blocked by me during
> some internal/private discussions as the main intention for him was
> upstreaming the changes. I am against the idea of mixing platform specific
> changes the way it is done here but since I didn't have much knowledge on
> pinmux conf to suggest/provide any useful feedback I suggested to trigger
> this discussion.

To use generic pinconf, we still need to extend to use OEM type, our scmi firmware
will not support saying bias/pull-up and etc config type, so just as below, we add:
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 365c4b0ca465..a71721cd321d 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -186,6 +186,16 @@ static const struct pinconf_generic_params dt_params[] = {
        { "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 },
        { "slew-rate", PIN_CONFIG_SLEW_RATE, 0 },
        { "skew-delay", PIN_CONFIG_SKEW_DELAY, 0 },
+       { "nxp-mux", PIN_CONFIG_NXP_MUX, 0 },
+       { "nxp-conf", PIN_CONFIG_NXP_CONF, 0 },
+       { "nxp-daisy-id", PIN_CONFIG_NXP_DAISY_ID, 0 },
+       { "nxp-daisy-val", PIN_CONFIG_NXP_DAISY_VAL, 0 },
};

And in dts:
+       pinctrl_uart1: uart1grp {
+               txd {
+                       pins = "uart1txd";
+                       nxp-mux = <0x0>;
+                       nxp-conf = <0x31e>;
+               };
+
+               rxd {
+                       pins = "uart1rxd";
+                       nxp-mux = <0x0>;
+                       nxp-conf = <0x31e>;
+               };
+       };

But the upper will make device tree diverge w/o scmi, because
we need support both for i.MX95.
I still prefer to use freescale current binding format which
would make dts reusable w/o scmi.

Thanks,
Peng.

> 
> > But I could have misunderstood it, so please correct me!
> 
> +1
> 
> --
> Regards,
> Sudeep