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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2797 bytes --]

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
> arch/arm/boot/dts/gemini/gemini.dtsi
> 
> This is another SoC that muxes pins in groups, not in single per-pin
> settings.

This looks like something that the "gpio2" stuff could definitely go to,
since it covers multiple functions trying to access the same pin. Do you
have an "approved" example for a more demultiplexer case, where the
contention is about which of multiple possible pins (or pin analogues)
an IO from a particular block must be routed to?

> 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.

In that case, the mutual exclusion would be that a function can only be
routed to one "pin", but there's no concern about multiple functions
being routed to any given "pin".

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]