ARM: dts: BCM5301X: Specify serial console parameters
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
From: Rafa? Mi?ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/boot/dts/bcm5301x.dtsi +5 -1
@@ -18,8 +18,12 @@ / { interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; }; chipcommonA {
Signature · 1 line
2.11.0
PATCH 2/2 ARM: dts: BCM53573: Specify serial console parameters
From: Rafa? Mi?ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
--- arch/arm/boot/dts/bcm53573.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/boot/dts/bcm53573.dtsi +5 -1
@@ -13,8 +13,12 @@ / { interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; }; cpus {
Signature · 1 line
2.11.0
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
In reply to Rafał MiłeckiQuoted 16 lines · Rafa?? Mi??ecki
From: Rafa?? Mi??ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
Signed-off-by Rafa?? Mi??ecki <[email protected]>--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/arm/boot/dts/bcm5301x.dtsi +0
-0@@ -18,8 +18,12 @@ / {
arch/arm/boot/dts/bcm5301x.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/bcm5301x.dtsi +5
-1interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; };
Hi Rafal
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as bcm5301x.dtsi is very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.
Andrew
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
Quoted 30 lines · Andrew Lunn
Quoted 16 lines · Rafa?? Mi??ecki
From: Rafa?? Mi??ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
Signed-off-by Rafa?? Mi??ecki <[email protected]>--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/arm/boot/dts/bcm5301x.dtsi +0
-0@@ -18,8 +18,12 @@ / {arch/arm/boot/dts/bcm5301x.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/bcm5301x.dtsi +5
-1interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; };Hi Rafal
Quoted 5 lines
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as
bcm5301x.dtsiis very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.
So far I've never seen any board using other uart. Also uart0 is disabled by default and we enable it per board family (BCM4708 / BCM47081 / BCM4709 / BCM47094). I think it's just more practical to have simple DTS that covers 99,9% boards by default and handled that margin of devices separately.
We already got a lot of duplicated code for enabling uart0, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b790d3b2943bf8e7e7bafe184008bd6451fe0bd
Jon Mason from Broadcom who knows this hardware and designs very well confirmed it's a sane/safe assumption.
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
Quoted 42 lines · Rafa? Mi?ecki
Quoted 30 lines · Andrew Lunn
Quoted 16 lines · Rafa?? Mi??ecki
From: Rafa?? Mi??ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
Signed-off-by Rafa?? Mi??ecki <[email protected]>--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/arm/boot/dts/bcm5301x.dtsi +0
-0@@ -18,8 +18,12 @@ / {arch/arm/boot/dts/bcm5301x.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/bcm5301x.dtsi +5
-1interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; };Hi Rafal
Quoted 5 lines
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as
bcm5301x.dtsiis very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.So far I've never seen any board using other uart. Also uart0 is disabled by default and we enable it per board family (BCM4708 / BCM47081 / BCM4709 / BCM47094). I think it's just more practical to have simple DTS that covers 99,9% boards by default and handled that margin of devices separately.
Quoted 5 lines
We already got a lot of duplicated code for enabling uart0, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b790d3b2943bf8e7e7bafe184008bd6451fe0bdJon Mason from Broadcom who knows this hardware and designs very well confirmed it's a sane/safe assumption.
That was supposed to be "any other uart or parameters".
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
In reply to Rafał MiłeckiQuoted 11 lines
Quoted 5 lines
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as
bcm5301x.dtsiis very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.So far I've never seen any board using other uart. Also uart0 is disabled by default and we enable it per board family (BCM4708 / BCM47081 / BCM4709 / BCM47094). I think it's just more practical to have simple DTS that covers 99,9% boards by default and handled that margin of devices separately.
Quoted 5 lines
We already got a lot of duplicated code for enabling uart0, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b790d3b2943bf8e7e7bafe184008bd6451fe0bdJon Mason from Broadcom who knows this hardware and designs very well confirmed it's a sane/safe assumption.
I don't disagree with you, it does make sense. But it is just different to every other SoC .dtsi file Linux has. And we try to avoid things being different.
It is down to Florian to decide, as maintainer.
Andrew
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
In reply to Rafał MiłeckiQuoted 47 lines · Rafa? Mi?ecki
Quoted 34 lines · Andrew Lunn
Quoted 18 lines · Rafa?? Mi??ecki
From: Rafa?? Mi??ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
Signed-off-by Rafa?? Mi??ecki <[email protected]>--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/arm/boot/dts/bcm5301x.dtsi +0
-0@@ -18,8 +18,12 @@ / {arch/arm/boot/dts/bcm5301x.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/bcm5301x.dtsi +5
-1interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; };Hi Rafal
Quoted 5 lines
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as
bcm5301x.dtsiis very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.So far I've never seen any board using other uart. Also uart0 is disabled by default and we enable it per board family (BCM4708 / BCM47081 / BCM4709 / BCM47094). I think it's just more practical to have simple DTS that covers 99,9% boards by default and handled that margin of devices separately.
Quoted 6 lines
We already got a lot of duplicated code for enabling uart0, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b790d3b2943bf8e7e7bafe184008bd6451fe0bdJon Mason from Broadcom who knows this hardware and designs very well confirmed
it's a sane/safe assumption.
It should work for all the boards. The question is whether it should be in the DTSI or DTS file. This code replicates code I have in arch/arm/boot/dts/bcm953012k.dts
So, either it should be removed or we should change all of the other DTS files to have this entry. My understanding is that the latter approach is the acceptable one (which matches Andrew's comment), but I can make the other changes if necessary.
Thanks,
Jon
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
Quoted 56 lines · Jon Mason
Quoted 47 lines · Rafa? Mi?ecki
Quoted 34 lines · Andrew Lunn
Quoted 18 lines · Rafa?? Mi??ecki
From: Rafa?? Mi??ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
Signed-off-by Rafa?? Mi??ecki <[email protected]>--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/arm/boot/dts/bcm5301x.dtsi +0
-0@@ -18,8 +18,12 @@ / {arch/arm/boot/dts/bcm5301x.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/bcm5301x.dtsi +5
-1interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; };Hi Rafal
Quoted 5 lines
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as
bcm5301x.dtsiis very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.So far I've never seen any board using other uart. Also uart0 is disabled by default and we enable it per board family (BCM4708 / BCM47081 / BCM4709 / BCM47094). I think it's just more practical to have simple DTS that covers 99,9% boards by default and handled that margin of devices separately.
Quoted 6 lines
We already got a lot of duplicated code for enabling uart0, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b790d3b2943bf8e7e7bafe184008bd6451fe0bdJon Mason from Broadcom who knows this hardware and designs very well confirmed
it's a sane/safe assumption.
Quoted 7 lines
It should work for all the boards. The question is whether it should be in the DTSI or DTS file. This code replicates code I have in
arch/arm/boot/dts/bcm953012k.dts
So, either it should be removed or we should change all of the other DTS files to have this entry. My understanding is that the latter approach is the acceptable one (which matches Andrew's comment), but I can make the other changes if necessary.
dtsi file is fine to handle 99% of the use cases and not duplicate the entry. This is what we do for newer SoCs (such as Cygnus) where the console will always be a particular UART (as the BootROM already uses a certain UART the boards are designed to use that UART as the console always). For bcm953012 a similar situation exists as a common bootloader is used across boards. So, the uart used as the terminal will always be the same.
I don't see a good reason for duplicating the code in every dts file.
Quoted 4 lines
Thanks,
Jon
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
Quoted 77 lines · Scott Branden
Quoted 66 lines · Jon Mason
Quoted 54 lines · Rafa? Mi?ecki
Quoted 37 lines · Andrew Lunn
Quoted 19 lines · Rafa?? Mi??ecki
From: Rafa?? Mi??ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
Signed-off-by Rafa?? Mi??ecki <[email protected]>--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/arm/boot/dts/bcm5301x.dtsi +0
-0@@ -18,8 +18,12 @@ / {arch/arm/boot/dts/bcm5301x.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/bcm5301x.dtsi +5
-1interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; };Hi Rafal
Quoted 5 lines
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as
bcm5301x.dtsiis very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.So far I've never seen any board using other uart. Also uart0 is disabled by
default and we enable it per board family (BCM4708 / BCM47081 / BCM4709 / BCM47094). I think it's just more practical to have simple DTS that covers
99,9% boards by default and handled that margin of devices separately.Quoted 7 lines
We already got a lot of duplicated code for enabling uart0, see:
Jon Mason from Broadcom who knows this hardware and designs very well confirmed
it's a sane/safe assumption.Quoted 7 lines
It should work for all the boards. The question is whether it should be in the DTSI or DTS file. This code replicates code I have in
arch/arm/boot/dts/bcm953012k.dts
So, either it should be removed or we should change all of the other DTS files to have this entry. My understanding is that the latter approach is the acceptable one (which matches Andrew's comment), but I can make the other changes if necessary.
Quoted 6 lines
dtsi file is fine to handle 99% of the use cases and not duplicate the entry. This is what we do for newer SoCs (such as Cygnus) where the console will always be a particular UART (as the BootROM already uses a certain UART the boards are designed to use that UART as the console always). For bcm953012 a similar situation exists as a common bootloader is used across boards. So, the uart used as the terminal will always be the same.
Unfortunately, there is not a common bootloader. CFE is used for the 4708/9 boards, and u-boot is used for the 5301x boards.
Thanks,
Jon
Quoted 9 lines
I don't see a good reason for duplicating the code in every dts file.
Acked-by Scott Branden <[email protected]>Quoted 5 lines
Thanks,
Jon
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
Quoted 86 lines · Jon Mason
Quoted 77 lines · Scott Branden
Quoted 66 lines · Jon Mason
Quoted 54 lines · Rafa? Mi?ecki
Quoted 37 lines · Andrew Lunn
Quoted 19 lines · Rafa?? Mi??ecki
From: Rafa?? Mi??ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
Signed-off-by Rafa?? Mi??ecki <[email protected]>--- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/arm/boot/dts/bcm5301x.dtsi +0
-0@@ -18,8 +18,12 @@ / {arch/arm/boot/dts/bcm5301x.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/bcm5301x.dtsi +5
-1interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; };Hi Rafal
Quoted 5 lines
The alias is fine. But putting the stdout-path here is unusual. Which serial port is used for console is board specific, where as
bcm5301x.dtsiis very generic, it describes the SoC, not a board. If you look at other .dtsi files, those that specify stdout-path contain properties which are common to a range of similar boards.So far I've never seen any board using other uart. Also uart0 is disabled by
default and we enable it per board family (BCM4708 / BCM47081 / BCM4709 / BCM47094). I think it's just more practical to have simple DTS that covers
99,9% boards by default and handled that margin of devices separately.Quoted 7 lines
We already got a lot of duplicated code for enabling uart0, see:
Jon Mason from Broadcom who knows this hardware and designs very well confirmed
it's a sane/safe assumption.Quoted 7 lines
It should work for all the boards. The question is whether it should be in the DTSI or DTS file. This code replicates code I have in
arch/arm/boot/dts/bcm953012k.dts
So, either it should be removed or we should change all of the other DTS files to have this entry. My understanding is that the latter approach is the acceptable one (which matches Andrew's comment), but I can make the other changes if necessary.Quoted 6 lines
dtsi file is fine to handle 99% of the use cases and not duplicate the entry. This is what we do for newer SoCs (such as Cygnus) where the console will always be a particular UART (as the BootROM already uses a certain UART the boards are designed to use that UART as the console always). For bcm953012 a similar situation exists as a common bootloader is used across boards. So, the uart used as the terminal will always be the same.
Quoted 2 lines
Unfortunately, there is not a common bootloader. CFE is used for the 4708/9 boards, and u-boot is used for the 5301x boards.
OK, little different situation not having a boot rom with console output enforcing all designs to use the same UART.
Do both of the bootloaders happen to use the same UART and all the boards are designed to use the same UART?
Quoted 13 lines
Thanks,
JonQuoted 9 lines
I don't see a good reason for duplicating the code in every dts file.
Acked-by Scott Branden <[email protected]>Quoted 5 lines
Thanks,
Jon
PATCH 1/2 ARM: dts: BCM5301X: Specify serial console parameters
Quoted 6 lines
Quoted 2 lines
Unfortunately, there is not a common bootloader. CFE is used for the 4708/9 boards, and u-boot is used for the 5301x boards.
OK, little different situation not having a boot rom with console output enforcing all designs to use the same UART.
Do both of the bootloaders happen to use the same UART and all the boards are designed to use the same UART?
You need to be a little bit careful here. Some vendor could use a different UART in there design, use some other bootloader, etc. Somebody in the community might want to throw out uboot and use barebox etc....
Device tree does allow .dtsi properties to be over written, so it is not an issue in practice.
Andrew
PATCH v2 1/2 ARM: dts: BCM5301X: Specify serial console params in dtsi files
In reply to Rafał MiłeckiFrom: Rafa? Mi?ecki <[email protected]>
So far every Northstar device we have seen was using the same serial console params (115200n8). It probably make the most sense to put it in some proper dtsi files instead of repeating over and over for every single device. As different boards may use different bootloaders it seems the safest idea is to use board specific dtsi files.
Just in case some vendor decides to use different UART (parameters) this can be always easily overwritten.
---
V2: Use board dtsi files, update existing bcm9*dts files, update commit message.
--- arch/arm/boot/dts/bcm4708.dtsi | 8 ++++++++ arch/arm/boot/dts/bcm47081.dtsi | 8 ++++++++ arch/arm/boot/dts/bcm5301x.dtsi | 4 ---- arch/arm/boot/dts/bcm94708.dts | 8 -------- arch/arm/boot/dts/bcm94709.dts | 8 -------- arch/arm/boot/dts/bcm953012er.dts | 8 -------- arch/arm/boot/dts/bcm953012hr.dts | 8 -------- arch/arm/boot/dts/bcm953012k.dts | 4 ---- 8 files changed, 16 insertions(+), 40 deletions(-)
arch/arm/boot/dts/bcm4708.dtsi +8 -0
@@ -12,6 +12,14 @@ / { compatible = "brcm,bcm4708"; + aliases { + serial0 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + cpus { #address-cells = <1>; #size-cells = <0>;
arch/arm/boot/dts/bcm47081.dtsi +8 -0
@@ -22,6 +22,14 @@ / { compatible = "brcm,bcm47081"; + aliases { + serial0 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + cpus { #address-cells = <1>; #size-cells = <0>;
arch/arm/boot/dts/bcm5301x.dtsi +0 -4
@@ -18,10 +18,6 @@ / { interrupt-parent = <&gic>; - chosen { - stdout-path = &uart0; - }; - chipcommonA { compatible = "simple-bus"; ranges = <0x00000000 0x18000000 0x00001000>;
arch/arm/boot/dts/bcm94708.dts +0 -8
@@ -38,14 +38,6 @@ model = "NorthStar SVK (BCM94708)"; compatible = "brcm,bcm94708", "brcm,bcm4708"; - aliases { - serial0 = &uart0; - }; - - chosen { - stdout-path = "serial0:115200n8"; - }; - memory { reg = <0x00000000 0x08000000>; };
arch/arm/boot/dts/bcm94709.dts +0 -8
@@ -38,14 +38,6 @@ model = "NorthStar SVK (BCM94709)"; compatible = "brcm,bcm94709", "brcm,bcm4709", "brcm,bcm4708"; - aliases { - serial0 = &uart0; - }; - - chosen { - stdout-path = "serial0:115200n8"; - }; - memory { reg = <0x00000000 0x08000000>; };
arch/arm/boot/dts/bcm953012er.dts +0 -8
@@ -39,14 +39,6 @@ model = "NorthStar Enterprise Router (BCM953012ER)"; compatible = "brcm,bcm953012er", "brcm,brcm53012", "brcm,bcm4708"; - aliases { - serial0 = &uart0; - }; - - chosen { - stdout-path = "serial0:115200n8"; - }; - memory { reg = <0x00000000 0x8000000>; };
arch/arm/boot/dts/bcm953012hr.dts +0 -8
@@ -39,14 +39,6 @@ model = "NorthStar HR (BCM953012HR)"; compatible = "brcm,bcm953012hr", "brcm,brcm53012", "brcm,bcm4708"; - aliases { - serial0 = &uart0; - }; - - chosen { - stdout-path = "serial0:115200n8"; - }; - memory at 80000000 { reg = <0x80000000 0x10000000>; };
arch/arm/boot/dts/bcm953012k.dts +0 -4
@@ -43,10 +43,6 @@ serial1 = &uart1; }; - chosen { - stdout-path = "serial0:115200n8"; - }; - memory { reg = <0x00000000 0x10000000>; };
Signature · 1 line
2.11.0
PATCH v2 2/2 ARM: dts: BCM53573: Specify serial console parameters
From: Rafa? Mi?ecki <[email protected]>
This adds baud rate, parity & number of data bits. It's required to get serial working correctly.
--- arch/arm/boot/dts/bcm53573.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/boot/dts/bcm53573.dtsi +5 -1
@@ -13,8 +13,12 @@ / { interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; }; cpus {
Signature · 1 line
2.11.0
Florian Fainelli
AppliedPATCH v2 1/2 ARM: dts: BCM5301X: Specify serial console params in dtsi files
In reply to Rafał MiłeckiQuoted 8 lines · Rafa? Mi?ecki
From: Rafa? Mi?ecki <[email protected]>
So far every Northstar device we have seen was using the same serial console params (115200n8). It probably make the most sense to put it in some proper dtsi files instead of repeating over and over for every single device. As different boards may use different bootloaders it seems the safest idea is to use board specific dtsi files.
Quoted 4 lines
Just in case some vendor decides to use different UART (parameters) this can be always easily overwritten.
Signed-off-by Rafa? Mi?ecki <[email protected]>
Applied, thanks!
Signature · 1 line
Florian