[PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0

[Arınç ÜNAL]

On 8.03.2024 15:40, Justin Swartz wrote:
> Hi Angelo
> 
> On 2024-03-08 10:41, AngeloGioacchino Del Regno wrote:
>> Il 07/03/24 20:04, Justin Swartz ha scritto:
>>> Add missing pinctrl-name and pinctrl-0 properties to declare
>>> that the uart1_pins group is associated with serial0.
>>>
>>> Signed-off-by: Justin Swartz <justin.swartz at risingedge.co.za>
>>> ---
>>>   arch/mips/boot/dts/ralink/mt7621.dtsi | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/arch/mips/boot/dts/ralink/mt7621.dtsi b/arch/mips/boot/dts/ralink/mt7621.dtsi
>>> index 35a10258f..dca415fdd 100644
>>> --- a/arch/mips/boot/dts/ralink/mt7621.dtsi
>>> +++ b/arch/mips/boot/dts/ralink/mt7621.dtsi
>>> @@ -123,6 +123,9 @@ serial0: serial at c00 {
>>>               reg-shift = <2>;
>>>               reg-io-width = <4>;
>>>               no-loopback-test;
>>> +
>>> +            pinctrl-names = "default";
>>> +            pinctrl-0 = <&uart1_pins>;
>>>           };
>>>             spi0: spi at b00 {
>>
>> The pins are muxed and can be either UART, or some other function that
>> is supported by the mux: this means that the pinctrl-xxx properties shall
>> *not* go into the SoC dtsi file, but in board dts files instead.
>>
>> Said differently: the usage of the UART pins is board-specific, not SoC-wide.
> 
> Thanks for the explanation. I agree that the pinctrl properties
> would make more sense in a serial node extension in a board's dts,
> but my reason for including them in the SoC's dtsi is due to the
> precedent set with these existing nodes:
> 
>    i2c
>    spi0
>    mmc
>    ethernet
>    pcie
> 
> There is also a default function declared for each of the pin
> groups defined under the pinctrl node. These functions co-incide
> with what is intended for each of those device nodes to function
> correctly, rather than in the alternative GPIO-mode.
> 
> So I thought that sticking with that existing pattern would get
> the least resistance from the community.
> 
> I can imagine how moving the pinctrl node to the board dts, and
> then moving all of the pinctrl properties associated with device
> nodes to their board dts references could be a better separation
> logically.
> 
> What do you recommend?

As a maintainer, this is the logic I follow on the MT7621 device tree
source files regarding the description of pin groups:

- Claim the relevant pin group with the default function (pinctrl-names &
   pinctrl-0) on the node that describes a component of the SoC.

- Keep the node disabled and leave it to the board DTS file to enable it.

I don't disable serial at c00 as we can expect every board to use it. Same
goes for ethernet at 1e100000. Some boards use the pins on the rgmii2 pin
group as GPIO, so the pinctrl-0 property on ethernet at 1e100000 is
overwritten on the board DTS file without rgmii2_pins listed.

So I'm fine with this patch as is.

Reviewed-by: Arınç ÜNAL <arinc.unal at arinc9.com>

Arınç