Message-ID: <aPYq4cnaAHu5ags5@makrotopia.org>
Date: Mon, 20 Oct 2025 13:28:17 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: Sjoerd Simons <sjoerd@...labora.com>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Ryder Lee <ryder.lee@...iatek.com>,
	Jianjun Wang <jianjun.wang@...iatek.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Manivannan Sadhasivam <mani@...nel.org>,
	Chunfeng Yun <chunfeng.yun@...iatek.com>,
	Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Lee Jones <lee@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Lorenzo Bianconi <lorenzo@...nel.org>, Felix Fietkau <nbd@....name>,
	kernel@...labora.com, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, linux-pci@...r.kernel.org,
	linux-phy@...ts.infradead.org, netdev@...r.kernel.org,
	Bryan Hinton <bryan@...anhinton.com>
Subject: Re: [PATCH 02/15] arm64: dts: mediatek: mt7981b-openwrt-one:
 Configure UART0 pinmux

On Mon, Oct 20, 2025 at 12:23:14PM +0200, AngeloGioacchino Del Regno wrote:
> Il 16/10/25 18:37, Daniel Golle ha scritto:
> > On Thu, Oct 16, 2025 at 04:29:14PM +0200, AngeloGioacchino Del Regno wrote:
> > > Il 16/10/25 14:38, Daniel Golle ha scritto:
> > > > On Thu, Oct 16, 2025 at 12:08:38PM +0200, Sjoerd Simons wrote:
> > > > > Add explicit pinctrl configuration for UART0 on the OpenWrt One board,
> > > > > 
> > > > > Signed-off-by: Sjoerd Simons <sjoerd@...labora.com>
> > > > > ---
> > > > >    arch/arm64/boot/dts/mediatek/mt7981b-openwrt-one.dts | 11 +++++++++++
> > > > >    1 file changed, 11 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt7981b-openwrt-one.dts b/arch/arm64/boot/dts/mediatek/mt7981b-openwrt-one.dts
> > > > > index 968b91f55bb27..f836059d7f475 100644
> > > > > --- a/arch/arm64/boot/dts/mediatek/mt7981b-openwrt-one.dts
> > > > > +++ b/arch/arm64/boot/dts/mediatek/mt7981b-openwrt-one.dts
> > > > > @@ -22,6 +22,17 @@ memory@...00000 {
> > > > >    	};
> > > > >    };
> > > > > +&pio {
> > > > > +	uart0_pins: uart0-pins {
> > > > > +		mux {
> > > > > +			function = "uart";
> > > > > +			groups = "uart0";
> > > > > +		};
> > > > > +	};
> > > > > +};
> > > > > +
> > > > >    &uart0 {
> > > > > +	pinctrl-names = "default";
> > > > > +	pinctrl-0 = <&uart0_pins>;
> > > > >    	status = "okay";
> > > > >    };
> > > > 
> > > > As there is only a single possible pinctrl configuration for uart0,
> > > > both the pinmux definition as well as the pinctrl properties should go
> > > > into mt7981b.dtsi rather than in the board's dts.
> > > 
> > > If there's really one single possible pin configuration for the UART0 pins,
> > > as in, those pins *do not* have a GPIO mode, then yes I agree.
> > > 
> > > If those pins can be as well configured as GPIOs, this goes to board DTS.
> > 
> > I respectfully disagree and will explain below.
> > 
> 
> Thanks a lot for taking the time to write all this - explains everything,
> and even too much :) :)
> 
> Though, there's something funny here! The following snippet of "main" text
> does explain stuff that is interesting, but that I (not other people, so
> thanks again for saying all this) know already, but.....
> 
> > All pinmux pins on the MediaTek platform also allow being configured as
> > GPIOs. However, if you configure those as GPIOs the consequence is that
> > you cannot use UART0 any more at all. So using UART0 at all always
> > implies using exactly those pins, there is no alternative to that.
> > 
> > Hence every board with every possible uses of pins 32 and 33 (there is
> > only RX and TX for UART0, RTS/CTS flow-control is not possible) can be
> > represented without needing to configure the pinctrl for uart0 on the
> > board level. There isn't going to be any variation on the board-level
> > when it comes to uart0. Either it is enabled (status = "okay";), and
> > that will always imply using the 'uart0' group in mode 'uart', or, in
> > case any of the two pins of uart0 is used for something else that means
> > uart0 cannot be enabled. Simple as that.
> > 
> > Hence there is no need to duplicate that pinctrl settings on each and
> > every board, as controlling the 'status' property on the board-level
> > already gives 100% freedom.
> > 
> 
> ...all of this is not justifying your point.

So what is the rule then? I understand the logic of describing the
pins eg. for uart1 only on board-level as there are actual alternatives
regarding the pins to be used, and if also including RTS/CTS pins.
Hence, for uart1, there are several possible pingroups which can be
used. What would be the argument to keep a pinctrl description for
which the SoC doesn't offer any alternatives to be on the board-level?
There is nothing to be decided by the board, literally 0 freedom.

> 
> > (Sidenote: As even the BootROM already uses those two pins as UART for
> > debug output,
> 
> Funny thing is, your side note is what *fully* justifies your disagreement
> and it's also what triggers me to say that you're right, lol :)
> 
> Okay then, I am fine with this commit now and I can renew my
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>

Note that the patch you have just added your Reviewed-by:-tag to does
*not* add the uart0 pinctrl on SoC-level but board-level, so different
from what I argued for above. Did you mean to add Reviewed-by: for that
(which contraticts what you just wrote) or rather to the to-be-submitted
v2 of this series which includes the change to move the uart0 pinctrl
to mt7981b.dtsi?

Powered by blists - more mailing lists