Re: [PATCH 1/3] dt-binding: pinctrl: Document Loongson 2K0300 pin controller

[Yao Zi <ziyao@disroot.org>]

On Tue, Oct 14, 2025 at 12:48:31AM +0200, Linus Walleij wrote:
> On Mon, Aug 11, 2025 at 6:39 PM Yao Zi <ziyao@disroot.org> wrote:
> 
> > The pincontroller integarted in Loongson 2K0300 is able to configure
> > function multiplexing for all the pins. It could also configure drive
> > strength on basis of functions, which means all pins set to the same
> > function share drive-strength setting. Drive-strength configuration
> > isn't available for all functions, either.
> >
> > This binding utilizes two levels of subnodes, where the outer represents
> > function and the inner represents groups. Drive-strength is allowed in
> > the outer since it's shared among all groups configured to the function.
> >
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> (...)
> > +patternProperties:
> > +  '^func-':
> > +    type: object
> > +
> > +    $ref: pincfg-node.yaml#
> > +
> > +    properties:
> > +      drive-strength:
> > +        description:
> > +          Maximum sink or source current as defined in pincfg-node.yaml. Note
> > +          that drive strength could only be configured on function basis, i.e.,
> > +          all pins multiplexed to the same function share the same
> > +          configuration.
> > +
> > +          This could only be configured for several functions, including jtag,
> > +          dvo, uart, gmac, sdio, spi, i2s, timer, usb and emmc.
> > +        enum: [2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
> 
> As Rob points out this is really odd, or rather the first time I see
> something like this.
> 
> It is clear from the driver that these are all set by writing bits
> in a 32bit register, with 2 or 3 bits dedicated to each function.
> 
> Its a bit weird, like each function has driver totempoles/stages
> before the mux instead of after (which is the normal). But
> I guess it is engineered like that then!

Yes, it occurs strange to me at the first sight, too.

> 
> It now looks like this:
> 
> +        func-uart {
> +            drive-strength = <2>;
> +
> +            uart0-pins {
> +                pinmux = <((40 << 8) | 0x3)>, <((41 << 8) | 0x3)>;
> +            };
> 
> I think this is better:
> 
> uart0_default: uart0-pins {
>      function = "uart0";
>      drive-strength = <2>;
>      pinmux = <((40 << 8) | 0x3)>, <((41 << 8) | 0x3)>;
>  };
> 
> This is consistent with the bindings if you include both
> $ref: pinmux-node.yaml#
> $ref: pincfg-node.yaml#
> 
> And will configure it all in one go.

To confirm my understanding, would you like to drop the outside node
(which now refers to pincfg-node.yaml) and refer to both
pinmux-node.yaml and pincfg-node.yaml in the single level of subnode?
i.e.

	pinctrl {
		uart0_defaults: uart0-pins {
			function = "uart0";
			drive-strength = <2>;
			pinmux = <...>;
		};
	};

> Sure you need a lookup for the function strings in the driver
> but it's OK.
> 
> It's odd to have "function" without "group" but it seems legal
> and works fine for your usecase. The normal would be to
> skip pinmux, have just pins = <40, 41>; and look up the
> function value for each function for a pin from a table that
> cross-reference in this case "uart0" to 3. But I
> guess you don't wanna do that so pinmux is fine too.

Thanks, glad to hear it's okay.

> <((40 << 8) | 0x3)>, <((41 << 8) | 0x3)> is a bit odd, maybe
> you want to also include an explanatory macro in the bindings.

Sure, I will add a macro like the one included in the DTS change.

> Yours,
> Linus Walleij
> 

Thank you and Rob for the help! Will adapt the suggestions in v2.

Best regards,
Yao Zi