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

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: <aO20G22p7OwJq6C-@pie>
Date: Tue, 14 Oct 2025 02:23:23 +0000
From: Yao Zi <ziyao@...root.org>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Huacai Chen <chenhuacai@...nel.org>,
	WANG Xuerui <kernel@...0n.name>, linux-gpio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	loongarch@...ts.linux.dev, Mingcong Bai <jeffbai@...c.io>,
	Kexy Biscuit <kexybiscuit@...c.io>
Subject: Re: [PATCH 1/3] dt-binding: pinctrl: Document Loongson 2K0300 pin
 controller

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