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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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.

> (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>

Cheers!
Angelo

> it is very unlikely that anyone would actually use them
> for anything else in production. Apart from being used as GPIOs you can
> also use pins 32 and 33 as an I2C target for external debug access to the
> registers of either the sgmii0_phy, sgmii1_phy or u3_phy. However, that
> doesn't matter in terms of the debate above, as the crucial point there
> is that using uart0 always implies using group 'uart0' in 'uart' mode.)