linux-kernel - Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support

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] [day] [month] [year] [list]

Date: Thu, 8 Feb 2024 13:17:11 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Peng Fan <peng.fan@....com>
Cc: Cristian Marussi <cristian.marussi@....com>, Sudeep Holla <sudeep.holla@....com>, 
	"Peng Fan (OSS)" <peng.fan@....nxp.com>, 
	"souvik.chakravarty@....com" <Souvik.Chakravarty@....com>, Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	Oleksii Moisieiev <oleksii_moisieiev@...m.com>, Shawn Guo <shawnguo@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Fabio Estevam <festevam@...il.com>, 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>, 
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, 
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>, AKASHI Takahiro <takahiro.akashi@...aro.org>, 
	Rob Herring <robh@...nel.org>, Nitin Garg <nitin.garg_3@....com>
Subject: Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol
 protocol basic support

On Mon, Feb 5, 2024 at 10:11 AM Peng Fan <peng.fan@....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 says.

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 groups and
> > functions as strings is better for abstraction. And the point of firmware is to
> > abstract things so they work the same on all systems.
>
> With current:
>         pinctrl_uart1: uart1grp {
>                 fsl,pins = <
>                         IMX95_PAD_UART1_RXD__AONMIX_TOP_LPUART1_RX      0x31e
>                         IMX95_PAD_UART1_TXD__AONMIX_TOP_LPUART1_TX      0x31e
>                 >;
>         };
>
> It is very easy for people to know the meaning from reading reference manual.

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 = "uart1txd", "uart1rxd";
>         functions = "uart1";
>         bias-xx
>         drive-strength =
> };

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 settings,
> 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