linux-kernel - Re: [RFC 0/5] microchip mpfs/pic64gx pinctrl questions

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: <20251001-arrive-tattle-2dafbd375da0@spud>
Date: Wed, 1 Oct 2025 17:00:41 +0100
From: Conor Dooley <conor@...nel.org>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Conor Dooley <conor.dooley@...rochip.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [RFC 0/5] microchip mpfs/pic64gx pinctrl questions

On Wed, Oct 01, 2025 at 01:29:01PM +0200, Linus Walleij wrote:
> Hi Conor,
> 
> thanks for your patches!
> 
> looking at the drivers it appears to be trying extensively to make use
> of the pinmux = <>; property to mux entire groups of pins.
> 
> pinmux = <nn>; is supposed to mux *one* pin per group, not entire
> groups of pins from one property. See
> Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml:
> 
>   The pinmux property accepts an array of pinmux groups, each of them describing
>   a single pin multiplexing configuration.
> 
>   pincontroller {
>     state_0_node_a {
>       pinmux = <PINMUX_GROUP>, <PINMUX_GROUP>, ...;
>     };
>   };
> 
> So e.g. when you do this:
> 
>        spi0_mssio: spi0-mssio-pins {
>          pinmux = <MPFS_PINFUNC(0, 0)>;
>        };
> 
> We all know SPI uses more than one pin so this is clearly abusing
> the pinmux property.
> 
> It is unfortunate that so many drivers now use this "mux one pin
> individually" concept that we cannot see the diversity of pin
> controllers.
> 
> I cannot recommend using the pinmux property for this SoC.
> 
> What you need to do is to define the actual pins and groups
> that you have.
> 
> Look for example at
> Documentation/devicetree/bindings/pinctrl/cortina,gemini-pinctrl.txt
> drivers/pinctrl/pinctrl-gemini.c

Does this driver have a mistake in L2074, using ARRAY_SIZE(idegrps) for
dram?

> arch/arm/boot/dts/gemini/gemini.dtsi
> 
> This is another SoC that muxes pins in groups, not in single per-pin
> settings.
> 
> Notice that the driver in this case enumerates and registers all 323
> pins on the package! This is done because some of the groups
> are mutually exclusive and this way the pin control framework
> will do its job to detect collisions between pin groups and disallow
> this, and that is what pin control is supposed to be doing.
> 
> I.e. do not orient your design around which registers and settings
> you have, and do not model your driver around that, instead
> model the driver around which actual pins exist on the physical
> component, how these are sorted into groups, how the groups
> are related to function (such as the group of SPI pins being
> related to the spi function) and define these pins, groups
> and functions in your driver.
> 
> Yours,
> Linus Walleij

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)