* [PATCH v2 2/3] mips: dts: ralink: mt7621: reorder serial0 properties
2024-03-07 19:04 ` [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 Justin Swartz
@ 2024-03-07 19:04 ` Justin Swartz
2024-03-08 7:29 ` Sergio Paracuellos
2024-03-08 8:42 ` AngeloGioacchino Del Regno
2024-03-07 19:04 ` [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes Justin Swartz
` (2 subsequent siblings)
3 siblings, 2 replies; 21+ messages in thread
From: Justin Swartz @ 2024-03-07 19:04 UTC (permalink / raw)
To: Arınç ÜNAL, Sergio Paracuellos, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Bogendoerfer,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Justin Swartz, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Reorder serial0 properties according to the guidelines laid
out in Documentation/devicetree/bindings/dts-coding-style.rst
Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
---
arch/mips/boot/dts/ralink/mt7621.dtsi | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/mips/boot/dts/ralink/mt7621.dtsi b/arch/mips/boot/dts/ralink/mt7621.dtsi
index dca415fdd..3ad4e2343 100644
--- a/arch/mips/boot/dts/ralink/mt7621.dtsi
+++ b/arch/mips/boot/dts/ralink/mt7621.dtsi
@@ -114,16 +114,12 @@ memc: memory-controller@5000 {
serial0: serial@c00 {
compatible = "ns16550a";
reg = <0xc00 0x100>;
-
+ reg-io-width = <4>;
+ reg-shift = <2>;
clocks = <&sysc MT7621_CLK_UART1>;
-
interrupt-parent = <&gic>;
interrupts = <GIC_SHARED 26 IRQ_TYPE_LEVEL_HIGH>;
-
- reg-shift = <2>;
- reg-io-width = <4>;
no-loopback-test;
-
pinctrl-names = "default";
pinctrl-0 = <&uart1_pins>;
};
--
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/3] mips: dts: ralink: mt7621: reorder serial0 properties
2024-03-07 19:04 ` [PATCH v2 2/3] mips: dts: ralink: mt7621: reorder serial0 properties Justin Swartz
@ 2024-03-08 7:29 ` Sergio Paracuellos
2024-03-08 8:42 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 21+ messages in thread
From: Sergio Paracuellos @ 2024-03-08 7:29 UTC (permalink / raw)
To: Justin Swartz
Cc: Arınç ÜNAL, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Bogendoerfer, Matthias Brugger,
AngeloGioacchino Del Regno, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
On Thu, Mar 7, 2024 at 8:05 PM Justin Swartz
<justin.swartz@risingedge.co.za> wrote:
>
> Reorder serial0 properties according to the guidelines laid
> out in Documentation/devicetree/bindings/dts-coding-style.rst
>
> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
> ---
> arch/mips/boot/dts/ralink/mt7621.dtsi | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
Acked-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
Thanks,
Sergio Paracuellos
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/3] mips: dts: ralink: mt7621: reorder serial0 properties
2024-03-07 19:04 ` [PATCH v2 2/3] mips: dts: ralink: mt7621: reorder serial0 properties Justin Swartz
2024-03-08 7:29 ` Sergio Paracuellos
@ 2024-03-08 8:42 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 21+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-03-08 8:42 UTC (permalink / raw)
To: Justin Swartz, Arınç ÜNAL, Sergio Paracuellos,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Thomas Bogendoerfer, Matthias Brugger
Cc: linux-mips, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
Il 07/03/24 20:04, Justin Swartz ha scritto:
> Reorder serial0 properties according to the guidelines laid
> out in Documentation/devicetree/bindings/dts-coding-style.rst
>
> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes
2024-03-07 19:04 ` [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 Justin Swartz
2024-03-07 19:04 ` [PATCH v2 2/3] mips: dts: ralink: mt7621: reorder serial0 properties Justin Swartz
@ 2024-03-07 19:04 ` Justin Swartz
2024-03-08 7:28 ` Sergio Paracuellos
` (2 more replies)
2024-03-08 7:27 ` [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 Sergio Paracuellos
2024-03-08 8:41 ` AngeloGioacchino Del Regno
3 siblings, 3 replies; 21+ messages in thread
From: Justin Swartz @ 2024-03-07 19:04 UTC (permalink / raw)
To: Arınç ÜNAL, Sergio Paracuellos, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Bogendoerfer,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Justin Swartz, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Add serial1 and serial2 nodes to define the existence of
the MT7621's second and third UARTs.
Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
---
arch/mips/boot/dts/ralink/mt7621.dtsi | 28 +++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/mips/boot/dts/ralink/mt7621.dtsi b/arch/mips/boot/dts/ralink/mt7621.dtsi
index 3ad4e2343..5a89f0b8c 100644
--- a/arch/mips/boot/dts/ralink/mt7621.dtsi
+++ b/arch/mips/boot/dts/ralink/mt7621.dtsi
@@ -124,6 +124,34 @@ serial0: serial@c00 {
pinctrl-0 = <&uart1_pins>;
};
+ serial1: serial@d00 {
+ compatible = "ns16550a";
+ reg = <0xd00 0x100>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ clocks = <&sysc MT7621_CLK_UART2>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SHARED 27 IRQ_TYPE_LEVEL_HIGH>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart2_pins>;
+ status = "disabled";
+ };
+
+ serial2: serial@e00 {
+ compatible = "ns16550a";
+ reg = <0xe00 0x100>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ clocks = <&sysc MT7621_CLK_UART3>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SHARED 28 IRQ_TYPE_LEVEL_HIGH>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart3_pins>;
+ status = "disabled";
+ };
+
spi0: spi@b00 {
status = "disabled";
--
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes
2024-03-07 19:04 ` [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes Justin Swartz
@ 2024-03-08 7:28 ` Sergio Paracuellos
2024-03-08 8:44 ` AngeloGioacchino Del Regno
2024-03-08 13:50 ` Arınç ÜNAL
2 siblings, 0 replies; 21+ messages in thread
From: Sergio Paracuellos @ 2024-03-08 7:28 UTC (permalink / raw)
To: Justin Swartz
Cc: Arınç ÜNAL, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Bogendoerfer, Matthias Brugger,
AngeloGioacchino Del Regno, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
On Thu, Mar 7, 2024 at 8:05 PM Justin Swartz
<justin.swartz@risingedge.co.za> wrote:
>
> Add serial1 and serial2 nodes to define the existence of
> the MT7621's second and third UARTs.
>
> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
> ---
> arch/mips/boot/dts/ralink/mt7621.dtsi | 28 +++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
Acked-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
Thanks,
Sergio Paracuellos
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes
2024-03-07 19:04 ` [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes Justin Swartz
2024-03-08 7:28 ` Sergio Paracuellos
@ 2024-03-08 8:44 ` AngeloGioacchino Del Regno
2024-03-08 10:56 ` Arınç ÜNAL
2024-03-08 13:50 ` Arınç ÜNAL
2 siblings, 1 reply; 21+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-03-08 8:44 UTC (permalink / raw)
To: Justin Swartz, Arınç ÜNAL, Sergio Paracuellos,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Thomas Bogendoerfer, Matthias Brugger
Cc: linux-mips, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
Il 07/03/24 20:04, Justin Swartz ha scritto:
> Add serial1 and serial2 nodes to define the existence of
> the MT7621's second and third UARTs.
>
> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
> ---
> arch/mips/boot/dts/ralink/mt7621.dtsi | 28 +++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/mips/boot/dts/ralink/mt7621.dtsi b/arch/mips/boot/dts/ralink/mt7621.dtsi
> index 3ad4e2343..5a89f0b8c 100644
> --- a/arch/mips/boot/dts/ralink/mt7621.dtsi
> +++ b/arch/mips/boot/dts/ralink/mt7621.dtsi
> @@ -124,6 +124,34 @@ serial0: serial@c00 {
> pinctrl-0 = <&uart1_pins>;
> };
>
> + serial1: serial@d00 {
> + compatible = "ns16550a";
> + reg = <0xd00 0x100>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + clocks = <&sysc MT7621_CLK_UART2>;
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SHARED 27 IRQ_TYPE_LEVEL_HIGH>;
> + no-loopback-test;
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_pins>;
As already commented on patch [1/3], pin muxing is board specific. Please remove.
Also, is there any reason why you can't simply use the `interrupts-extended`
property instead of interrupt-parent and interrupts?
Regards,
Angelo
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes
2024-03-08 8:44 ` AngeloGioacchino Del Regno
@ 2024-03-08 10:56 ` Arınç ÜNAL
0 siblings, 0 replies; 21+ messages in thread
From: Arınç ÜNAL @ 2024-03-08 10:56 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, Justin Swartz, Sergio Paracuellos,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Thomas Bogendoerfer, Matthias Brugger
Cc: linux-mips, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
On 8.03.2024 11:44, AngeloGioacchino Del Regno wrote:
> Il 07/03/24 20:04, Justin Swartz ha scritto:
>> Add serial1 and serial2 nodes to define the existence of
>> the MT7621's second and third UARTs.
>>
>> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
>> ---
>> arch/mips/boot/dts/ralink/mt7621.dtsi | 28 +++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/mips/boot/dts/ralink/mt7621.dtsi b/arch/mips/boot/dts/ralink/mt7621.dtsi
>> index 3ad4e2343..5a89f0b8c 100644
>> --- a/arch/mips/boot/dts/ralink/mt7621.dtsi
>> +++ b/arch/mips/boot/dts/ralink/mt7621.dtsi
>> @@ -124,6 +124,34 @@ serial0: serial@c00 {
>> pinctrl-0 = <&uart1_pins>;
>> };
>> + serial1: serial@d00 {
>> + compatible = "ns16550a";
>> + reg = <0xd00 0x100>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + clocks = <&sysc MT7621_CLK_UART2>;
>> + interrupt-parent = <&gic>;
>> + interrupts = <GIC_SHARED 27 IRQ_TYPE_LEVEL_HIGH>;
>> + no-loopback-test;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart2_pins>;
>
> As already commented on patch [1/3], pin muxing is board specific. Please remove.
>
> Also, is there any reason why you can't simply use the `interrupts-extended`
> property instead of interrupt-parent and interrupts?
I'm looking at the documentation [1], it seems to be useful when multiple
interrupt parents need to be defined on a node. I'd continue using
interrupt-parent and interrupts in this case.
[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
Arınç
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes
2024-03-07 19:04 ` [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes Justin Swartz
2024-03-08 7:28 ` Sergio Paracuellos
2024-03-08 8:44 ` AngeloGioacchino Del Regno
@ 2024-03-08 13:50 ` Arınç ÜNAL
2024-03-08 13:56 ` Justin Swartz
2 siblings, 1 reply; 21+ messages in thread
From: Arınç ÜNAL @ 2024-03-08 13:50 UTC (permalink / raw)
To: Justin Swartz, Sergio Paracuellos, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Bogendoerfer,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: linux-mips, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
On 7.03.2024 22:04, Justin Swartz wrote:
> Add serial1 and serial2 nodes to define the existence of
> the MT7621's second and third UARTs.
>
> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
> ---
> arch/mips/boot/dts/ralink/mt7621.dtsi | 28 +++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/mips/boot/dts/ralink/mt7621.dtsi b/arch/mips/boot/dts/ralink/mt7621.dtsi
> index 3ad4e2343..5a89f0b8c 100644
> --- a/arch/mips/boot/dts/ralink/mt7621.dtsi
> +++ b/arch/mips/boot/dts/ralink/mt7621.dtsi
> @@ -124,6 +124,34 @@ serial0: serial@c00 {
> pinctrl-0 = <&uart1_pins>;
> };
>
> + serial1: serial@d00 {
> + compatible = "ns16550a";
> + reg = <0xd00 0x100>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + clocks = <&sysc MT7621_CLK_UART2>;
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SHARED 27 IRQ_TYPE_LEVEL_HIGH>;
> + no-loopback-test;
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_pins>;
> + status = "disabled";
> + };
I would group this:
serial1: serial@d00 {
compatible = "ns16550a";
reg = <0xd00 0x100>;
reg-io-width = <4>;
reg-shift = <2>;
clocks = <&sysc MT7621_CLK_UART2>;
interrupt-parent = <&gic>;
interrupts = <GIC_SHARED 27 IRQ_TYPE_LEVEL_HIGH>;
no-loopback-test;
pinctrl-names = "default";
pinctrl-0 = <&uart2_pins>;
status = "disabled";
};
Same goes for patch 2.
Arınç
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes
2024-03-08 13:50 ` Arınç ÜNAL
@ 2024-03-08 13:56 ` Justin Swartz
0 siblings, 0 replies; 21+ messages in thread
From: Justin Swartz @ 2024-03-08 13:56 UTC (permalink / raw)
To: Arınç ÜNAL
Cc: Sergio Paracuellos, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Bogendoerfer, Matthias Brugger,
AngeloGioacchino Del Regno, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
On 2024-03-08 15:50, Arınç ÜNAL wrote:
> On 7.03.2024 22:04, Justin Swartz wrote:
>> Add serial1 and serial2 nodes to define the existence of
>> the MT7621's second and third UARTs.
>>
>> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
>> ---
>> arch/mips/boot/dts/ralink/mt7621.dtsi | 28
>> +++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/mips/boot/dts/ralink/mt7621.dtsi
>> b/arch/mips/boot/dts/ralink/mt7621.dtsi
>> index 3ad4e2343..5a89f0b8c 100644
>> --- a/arch/mips/boot/dts/ralink/mt7621.dtsi
>> +++ b/arch/mips/boot/dts/ralink/mt7621.dtsi
>> @@ -124,6 +124,34 @@ serial0: serial@c00 {
>> pinctrl-0 = <&uart1_pins>;
>> };
>> + serial1: serial@d00 {
>> + compatible = "ns16550a";
>> + reg = <0xd00 0x100>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + clocks = <&sysc MT7621_CLK_UART2>;
>> + interrupt-parent = <&gic>;
>> + interrupts = <GIC_SHARED 27 IRQ_TYPE_LEVEL_HIGH>;
>> + no-loopback-test;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart2_pins>;
>> + status = "disabled";
>> + };
>
> I would group this:
>
> serial1: serial@d00 {
> compatible = "ns16550a";
> reg = <0xd00 0x100>;
>
> reg-io-width = <4>;
> reg-shift = <2>;
>
> clocks = <&sysc MT7621_CLK_UART2>;
>
> interrupt-parent = <&gic>;
> interrupts = <GIC_SHARED 27 IRQ_TYPE_LEVEL_HIGH>;
>
> no-loopback-test;
>
> pinctrl-names = "default";
> pinctrl-0 = <&uart2_pins>;
>
> status = "disabled";
> };
>
> Same goes for patch 2.
Thanks for the example.
Regards
Justin
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0
2024-03-07 19:04 ` [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 Justin Swartz
2024-03-07 19:04 ` [PATCH v2 2/3] mips: dts: ralink: mt7621: reorder serial0 properties Justin Swartz
2024-03-07 19:04 ` [PATCH v2 3/3] mips: dts: ralink: mt7621: add serial1 and serial2 nodes Justin Swartz
@ 2024-03-08 7:27 ` Sergio Paracuellos
2024-03-08 7:30 ` Sergio Paracuellos
2024-03-08 8:41 ` AngeloGioacchino Del Regno
3 siblings, 1 reply; 21+ messages in thread
From: Sergio Paracuellos @ 2024-03-08 7:27 UTC (permalink / raw)
To: Justin Swartz
Cc: Arınç ÜNAL, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Bogendoerfer, Matthias Brugger,
AngeloGioacchino Del Regno, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0
2024-03-08 7:27 ` [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 Sergio Paracuellos
@ 2024-03-08 7:30 ` Sergio Paracuellos
0 siblings, 0 replies; 21+ messages in thread
From: Sergio Paracuellos @ 2024-03-08 7:30 UTC (permalink / raw)
To: Justin Swartz
Cc: Arınç ÜNAL, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Bogendoerfer, Matthias Brugger,
AngeloGioacchino Del Regno, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0
2024-03-07 19:04 ` [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 Justin Swartz
` (2 preceding siblings ...)
2024-03-08 7:27 ` [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0 Sergio Paracuellos
@ 2024-03-08 8:41 ` AngeloGioacchino Del Regno
2024-03-08 12:40 ` Justin Swartz
3 siblings, 1 reply; 21+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-03-08 8:41 UTC (permalink / raw)
To: Justin Swartz, Arınç ÜNAL, Sergio Paracuellos,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Thomas Bogendoerfer, Matthias Brugger
Cc: linux-mips, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0
2024-03-08 8:41 ` AngeloGioacchino Del Regno
@ 2024-03-08 12:40 ` Justin Swartz
2024-03-08 13:24 ` Arınç ÜNAL
0 siblings, 1 reply; 21+ messages in thread
From: Justin Swartz @ 2024-03-08 12:40 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: Arınç ÜNAL, Sergio Paracuellos, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Bogendoerfer,
Matthias Brugger, linux-mips, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/3] mips: dts: ralink: mt7621: associate uart1_pins with serial0
2024-03-08 12:40 ` Justin Swartz
@ 2024-03-08 13:24 ` Arınç ÜNAL
0 siblings, 0 replies; 21+ messages in thread
From: Arınç ÜNAL @ 2024-03-08 13:24 UTC (permalink / raw)
To: Justin Swartz, AngeloGioacchino Del Regno
Cc: Sergio Paracuellos, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Bogendoerfer, Matthias Brugger, linux-mips,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
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ç
^ permalink raw reply [flat|nested] 21+ messages in thread