| Message ID | 20240307190408.23443-1-justin.swartz@risingedge.co.za (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 | expand |
On Thu, Mar 7, 2024 at 8:05 PM Justin Swartz <justin.swartz@risingedge.co.za> wrote: > > 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@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@c00 { > reg-shift = <2>; > reg-io-width = <4>; > no-loopback-test; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&uart1_pins>; > }; > > spi0: spi@b00 { > -- > Please add Acked-by/Reviewed-by tags when posting new versions. Thanks, Sergio Paracuellos
On Fri, Mar 8, 2024 at 8:27 AM Sergio Paracuellos <sergio.paracuellos@gmail.com> wrote: > > On Thu, Mar 7, 2024 at 8:05 PM Justin Swartz > <justin.swartz@risingedge.co.za> wrote: > > > > 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@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@c00 { > > reg-shift = <2>; > > reg-io-width = <4>; > > no-loopback-test; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&uart1_pins>; > > }; > > > > spi0: spi@b00 { > > -- > > > > Please add Acked-by/Reviewed-by tags when posting new versions. > > Thanks, > Sergio Paracuellos Anyway: Acked-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> Thanks, Sergio Paracuellos
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@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@c00 { > reg-shift = <2>; > reg-io-width = <4>; > no-loopback-test; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&uart1_pins>; > }; > > spi0: spi@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. Regards, Angelo
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@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@c00 { >> reg-shift = <2>; >> reg-io-width = <4>; >> no-loopback-test; >> + >> + pinctrl-names = "default"; >> + pinctrl-0 = <&uart1_pins>; >> }; >> spi0: spi@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? Regards Justin
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@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@c00 { >>> reg-shift = <2>; >>> reg-io-width = <4>; >>> no-loopback-test; >>> + >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&uart1_pins>; >>> }; >>> spi0: spi@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@c00 as we can expect every board to use it. Same goes for ethernet@1e100000. Some boards use the pins on the rgmii2 pin group as GPIO, so the pinctrl-0 property on ethernet@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@arinc9.com> Arınç
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@c00 { reg-shift = <2>; reg-io-width = <4>; no-loopback-test; + + pinctrl-names = "default"; + pinctrl-0 = <&uart1_pins>; }; spi0: spi@b00 {
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@risingedge.co.za> --- arch/mips/boot/dts/ralink/mt7621.dtsi | 3 +++ 1 file changed, 3 insertions(+) --