Received-SPF: pass (google.com: domain of linux-kernel+bounces-58036-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240121-pinctrl-scmi-v3-0-8d94ba79dca8@nxp.com>
 <f88d07ef-83b2-4d14-976a-6dbbd71e036f@oss.nxp.com> <CACRpkdYV=qYQ9qDUWYTLDAV1niay30gYH5S=zjfi31GpeY5o-A@mail.gmail.com>
 <DU0PR04MB9417A9074C5DC49AE689E65288432@DU0PR04MB9417.eurprd04.prod.outlook.com>
 <Zbt-QkWhz5d9P-v6@pluto> <DU0PR04MB9417CA6CF089B264112C32A088402@DU0PR04MB9417.eurprd04.prod.outlook.com>
 <CACRpkdYCrbNB8wu4rO3Yx0qomxR3kTt0P7YH7kc2HPCbrgt=tg@mail.gmail.com> <DU0PR04MB9417335A3A41BE4E373B915188472@DU0PR04MB9417.eurprd04.prod.outlook.com>
In-Reply-To: <DU0PR04MB9417335A3A41BE4E373B915188472@DU0PR04MB9417.eurprd04.prod.outlook.com>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Thu, 8 Feb 2024 13:17:11 +0100
Message-ID: <CACRpkdbguqMvx5w2fEDaNkNTmNtwphns-W1c2i+ntoi1CHqaVg@mail.gmail.com>
Subject: Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol
 protocol basic support
To: Peng Fan <peng.fan@nxp.com>
Cc: Cristian Marussi <cristian.marussi@arm.com>, Sudeep Holla <sudeep.holla@arm.com>, 
	"Peng Fan (OSS)" <peng.fan@oss.nxp.com>, 
	"souvik.chakravarty@arm.com" <Souvik.Chakravarty@arm.com>, Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Oleksii Moisieiev <oleksii_moisieiev@epam.com>, Shawn Guo <shawnguo@kernel.org>, 
	Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team <kernel@pengutronix.de>, 
	Fabio Estevam <festevam@gmail.com>, dl-linux-imx <linux-imx@nxp.com>, 
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, 
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>, AKASHI Takahiro <takahiro.akashi@linaro.org>, 
	Rob Herring <robh@kernel.org>, Nitin Garg <nitin.garg_3@nxp.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Feb 5, 2024 at 10:11=E2=80=AFAM Peng Fan <peng.fan@nxp.com> wrote:
> [Me]

> > This is very much to the core of the problem isn't it?
>
> Yes. Code size should be as small as possible.

This is a valid argument, I don't know how big your SCMI FW is, or
if it has to fit into some on-chip memory or so.

> And using SCMI generic pinconf, there will be multiple
> SCMI calls(not MMIO access), such as setting mux(ops->set_mux)
> needs an SCMI call, pad settings(ops->pin_config_set) needs an SCMI call.
> And maybe ops->get_function_name  needs an extra SCMI call.
>
> With current i.MX design, only one SCMI call is used, which
> use less time.

I think this could be fixed in the spec, let's see what the spec author say=
s.

In any case: pin control is pretty much never on a hot path, a few
microseconds more or less doesn't matter. It's usually just used
and probe, suspend/resume and maybe shutdown. All usecases on
the slowpath, so I think it's a premature optimization.

> > Over the years I have come to regret it a bit, I think insisting on gro=
ups and
> > functions as strings is better for abstraction. And the point of firmwa=
re is to
> > abstract things so they work the same on all systems.
>
> With current:
>         pinctrl_uart1: uart1grp {
>                 fsl,pins =3D <
>                         IMX95_PAD_UART1_RXD__AONMIX_TOP_LPUART1_RX      0=
x31e
>                         IMX95_PAD_UART1_TXD__AONMIX_TOP_LPUART1_TX      0=
x31e
>                 >;
>         };
>
> It is very easy for people to know the meaning from reading reference man=
ual.

Yes If the defines are provided it's quite readable. And I think Freescale
people really get used to it.

But from a maintenance and standardization point of view, it's nice if
everything works the same way and looks the same.

> If using generic pinconf, the dt node will be like
> Uartgrp {
>         pins =3D "uart1txd", "uart1rxd";
>         functions =3D "uart1";
>         bias-xx
>         drive-strength =3D
> };

Well that looks good to me :)

It looks the same as e.g. Qualcomm or nVidia pins. So it's easy for me
to read and understand, and that's my perspective as a maintainer.

> The firmware needs add more code to export functions, pack the conf setti=
ngs,
> each pins needs a function name per current i.MX HW logic.

Indeed, but I think any standard requires a bit of code to implement.
How much is "too much" code is a matter of taste and physical contraints.
(A PC UEFI BIOS isn't exactly small either.)

I understand your point of view, and I think it is pretty much
the same stance that the Freescale maintainers put forward for the DT
bindings for Freescale.

Yours,
Linus Walleij