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

Conor Dooley
2025-10-01T16:00:45Z

[-- Attachment #1: Type: text/plain, Size: 2361 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

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

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