Re: [PATCH 02/15] arm64: dts: mediatek: mt7981b-openwrt-one: Configure UART0 pinmux

[Daniel Golle <daniel@makrotopia.org>]

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@collabora.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@40000000 {
> > > > >    	};
> > > > >    };
> > > > > +&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@collabora.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?