* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters @ 2017-03-14 7:58 Rafał Miłecki 2017-03-14 7:58 ` [PATCH 2/2] ARM: dts: BCM53573: " Rafał Miłecki ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Rafał Miłecki @ 2017-03-14 7:58 UTC (permalink / raw) To: linux-arm-kernel From: Rafa? Mi?ecki <rafal@milecki.pl> This adds baud rate, parity & number of data bits. It's required to get serial working correctly. Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl> --- arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi index 8fd1ef9f0c2d..468107166a6f 100644 --- a/arch/arm/boot/dts/bcm5301x.dtsi +++ b/arch/arm/boot/dts/bcm5301x.dtsi @@ -18,8 +18,12 @@ / { interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; }; chipcommonA { -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] ARM: dts: BCM53573: Specify serial console parameters 2017-03-14 7:58 [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters Rafał Miłecki @ 2017-03-14 7:58 ` Rafał Miłecki 2017-03-14 12:26 ` [PATCH 1/2] ARM: dts: BCM5301X: " Andrew Lunn 2017-03-15 17:03 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Rafał Miłecki 2 siblings, 0 replies; 14+ messages in thread From: Rafał Miłecki @ 2017-03-14 7:58 UTC (permalink / raw) To: linux-arm-kernel From: Rafa? Mi?ecki <rafal@milecki.pl> This adds baud rate, parity & number of data bits. It's required to get serial working correctly. Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl> --- arch/arm/boot/dts/bcm53573.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi index 71ce23e46d30..eae623f76401 100644 --- a/arch/arm/boot/dts/bcm53573.dtsi +++ b/arch/arm/boot/dts/bcm53573.dtsi @@ -13,8 +13,12 @@ / { interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; }; cpus { -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 7:58 [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters Rafał Miłecki 2017-03-14 7:58 ` [PATCH 2/2] ARM: dts: BCM53573: " Rafał Miłecki @ 2017-03-14 12:26 ` Andrew Lunn 2017-03-14 12:32 ` Rafał Miłecki 2017-03-15 17:03 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Rafał Miłecki 2 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2017-03-14 12:26 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 14, 2017 at 08:58:29AM +0100, Rafa?? Mi??ecki wrote: > From: Rafa?? Mi??ecki <rafal@milecki.pl> > > This adds baud rate, parity & number of data bits. It's required to get > serial working correctly. > > Signed-off-by: Rafa?? Mi??ecki <rafal@milecki.pl> > --- > arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi > index 8fd1ef9f0c2d..468107166a6f 100644 > --- a/arch/arm/boot/dts/bcm5301x.dtsi > +++ b/arch/arm/boot/dts/bcm5301x.dtsi > @@ -18,8 +18,12 @@ > / { > interrupt-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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 12:26 ` [PATCH 1/2] ARM: dts: BCM5301X: " Andrew Lunn @ 2017-03-14 12:32 ` Rafał Miłecki 2017-03-14 12:37 ` Rafał Miłecki ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Rafał Miłecki @ 2017-03-14 12:32 UTC (permalink / raw) To: linux-arm-kernel On 03/14/2017 01:26 PM, Andrew Lunn wrote: > On Tue, Mar 14, 2017 at 08:58:29AM +0100, Rafa?? Mi??ecki wrote: >> From: Rafa?? Mi??ecki <rafal@milecki.pl> >> >> This adds baud rate, parity & number of data bits. It's required to get >> serial working correctly. >> >> Signed-off-by: Rafa?? Mi??ecki <rafal@milecki.pl> >> --- >> arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi >> index 8fd1ef9f0c2d..468107166a6f 100644 >> --- a/arch/arm/boot/dts/bcm5301x.dtsi >> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >> @@ -18,8 +18,12 @@ >> / { >> interrupt-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. 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 12:32 ` Rafał Miłecki @ 2017-03-14 12:37 ` Rafał Miłecki 2017-03-14 12:51 ` Andrew Lunn 2017-03-14 14:53 ` Jon Mason 2 siblings, 0 replies; 14+ messages in thread From: Rafał Miłecki @ 2017-03-14 12:37 UTC (permalink / raw) To: linux-arm-kernel On 03/14/2017 01:32 PM, Rafa? Mi?ecki wrote: > On 03/14/2017 01:26 PM, Andrew Lunn wrote: >> On Tue, Mar 14, 2017 at 08:58:29AM +0100, Rafa?? Mi??ecki wrote: >>> From: Rafa?? Mi??ecki <rafal@milecki.pl> >>> >>> This adds baud rate, parity & number of data bits. It's required to get >>> serial working correctly. >>> >>> Signed-off-by: Rafa?? Mi??ecki <rafal@milecki.pl> >>> --- >>> arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi >>> index 8fd1ef9f0c2d..468107166a6f 100644 >>> --- a/arch/arm/boot/dts/bcm5301x.dtsi >>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >>> @@ -18,8 +18,12 @@ >>> / { >>> interrupt-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. > > 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. That was supposed to be "any other uart or parameters". ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 12:32 ` Rafał Miłecki 2017-03-14 12:37 ` Rafał Miłecki @ 2017-03-14 12:51 ` Andrew Lunn 2017-03-14 14:53 ` Jon Mason 2 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2017-03-14 12:51 UTC (permalink / raw) To: linux-arm-kernel > >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. > > 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. 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 12:32 ` Rafał Miłecki 2017-03-14 12:37 ` Rafał Miłecki 2017-03-14 12:51 ` Andrew Lunn @ 2017-03-14 14:53 ` Jon Mason 2017-03-14 20:44 ` Scott Branden 2 siblings, 1 reply; 14+ messages in thread From: Jon Mason @ 2017-03-14 14:53 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 14, 2017 at 8:32 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > On 03/14/2017 01:26 PM, Andrew Lunn wrote: >> >> On Tue, Mar 14, 2017 at 08:58:29AM +0100, Rafa?? Mi??ecki wrote: >>> >>> From: Rafa?? Mi??ecki <rafal@milecki.pl> >>> >>> This adds baud rate, parity & number of data bits. It's required to get >>> serial working correctly. >>> >>> Signed-off-by: Rafa?? Mi??ecki <rafal@milecki.pl> >>> --- >>> arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi >>> b/arch/arm/boot/dts/bcm5301x.dtsi >>> index 8fd1ef9f0c2d..468107166a6f 100644 >>> --- a/arch/arm/boot/dts/bcm5301x.dtsi >>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >>> @@ -18,8 +18,12 @@ >>> / { >>> interrupt-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. > > > 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. 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 14:53 ` Jon Mason @ 2017-03-14 20:44 ` Scott Branden 2017-03-14 21:18 ` Jon Mason 0 siblings, 1 reply; 14+ messages in thread From: Scott Branden @ 2017-03-14 20:44 UTC (permalink / raw) To: linux-arm-kernel On 17-03-14 07:53 AM, Jon Mason wrote: > On Tue, Mar 14, 2017 at 8:32 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >> On 03/14/2017 01:26 PM, Andrew Lunn wrote: >>> >>> On Tue, Mar 14, 2017 at 08:58:29AM +0100, Rafa?? Mi??ecki wrote: >>>> >>>> From: Rafa?? Mi??ecki <rafal@milecki.pl> >>>> >>>> This adds baud rate, parity & number of data bits. It's required to get >>>> serial working correctly. >>>> >>>> Signed-off-by: Rafa?? Mi??ecki <rafal@milecki.pl> >>>> --- >>>> arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi >>>> b/arch/arm/boot/dts/bcm5301x.dtsi >>>> index 8fd1ef9f0c2d..468107166a6f 100644 >>>> --- a/arch/arm/boot/dts/bcm5301x.dtsi >>>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >>>> @@ -18,8 +18,12 @@ >>>> / { >>>> interrupt-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. >> >> >> 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. > > > 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. Acked-by: Scott Branden <scott.branden@broadcom.com> > > Thanks, > Jon > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 20:44 ` Scott Branden @ 2017-03-14 21:18 ` Jon Mason 2017-03-14 21:28 ` Scott Branden 0 siblings, 1 reply; 14+ messages in thread From: Jon Mason @ 2017-03-14 21:18 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 14, 2017 at 4:44 PM, Scott Branden <scott.branden@broadcom.com> wrote: > > > On 17-03-14 07:53 AM, Jon Mason wrote: >> >> On Tue, Mar 14, 2017 at 8:32 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>> >>> On 03/14/2017 01:26 PM, Andrew Lunn wrote: >>>> >>>> >>>> On Tue, Mar 14, 2017 at 08:58:29AM +0100, Rafa?? Mi??ecki wrote: >>>>> >>>>> >>>>> From: Rafa?? Mi??ecki <rafal@milecki.pl> >>>>> >>>>> This adds baud rate, parity & number of data bits. It's required to get >>>>> serial working correctly. >>>>> >>>>> Signed-off-by: Rafa?? Mi??ecki <rafal@milecki.pl> >>>>> --- >>>>> arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- >>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi >>>>> b/arch/arm/boot/dts/bcm5301x.dtsi >>>>> index 8fd1ef9f0c2d..468107166a6f 100644 >>>>> --- a/arch/arm/boot/dts/bcm5301x.dtsi >>>>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >>>>> @@ -18,8 +18,12 @@ >>>>> / { >>>>> interrupt-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. >>> >>> >>> >>> 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. >> >> >> >> 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. 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 > I don't see a good reason for duplicating the code in every dts file. > > Acked-by: Scott Branden <scott.branden@broadcom.com> >> >> >> Thanks, >> Jon >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 21:18 ` Jon Mason @ 2017-03-14 21:28 ` Scott Branden 2017-03-14 21:51 ` Andrew Lunn 0 siblings, 1 reply; 14+ messages in thread From: Scott Branden @ 2017-03-14 21:28 UTC (permalink / raw) To: linux-arm-kernel On 17-03-14 02:18 PM, Jon Mason wrote: > On Tue, Mar 14, 2017 at 4:44 PM, Scott Branden > <scott.branden@broadcom.com> wrote: >> >> >> On 17-03-14 07:53 AM, Jon Mason wrote: >>> >>> On Tue, Mar 14, 2017 at 8:32 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>>> >>>> On 03/14/2017 01:26 PM, Andrew Lunn wrote: >>>>> >>>>> >>>>> On Tue, Mar 14, 2017 at 08:58:29AM +0100, Rafa?? Mi??ecki wrote: >>>>>> >>>>>> >>>>>> From: Rafa?? Mi??ecki <rafal@milecki.pl> >>>>>> >>>>>> This adds baud rate, parity & number of data bits. It's required to get >>>>>> serial working correctly. >>>>>> >>>>>> Signed-off-by: Rafa?? Mi??ecki <rafal@milecki.pl> >>>>>> --- >>>>>> arch/arm/boot/dts/bcm5301x.dtsi | 6 +++++- >>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi >>>>>> b/arch/arm/boot/dts/bcm5301x.dtsi >>>>>> index 8fd1ef9f0c2d..468107166a6f 100644 >>>>>> --- a/arch/arm/boot/dts/bcm5301x.dtsi >>>>>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >>>>>> @@ -18,8 +18,12 @@ >>>>>> / { >>>>>> interrupt-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. >>>> >>>> >>>> >>>> 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. >>> >>> >>> >>> 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. > > 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? > > Thanks, > Jon > >> I don't see a good reason for duplicating the code in every dts file. >> >> Acked-by: Scott Branden <scott.branden@broadcom.com> >>> >>> >>> Thanks, >>> Jon >>> >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters 2017-03-14 21:28 ` Scott Branden @ 2017-03-14 21:51 ` Andrew Lunn 0 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2017-03-14 21:51 UTC (permalink / raw) To: linux-arm-kernel > >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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files 2017-03-14 7:58 [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters Rafał Miłecki 2017-03-14 7:58 ` [PATCH 2/2] ARM: dts: BCM53573: " Rafał Miłecki 2017-03-14 12:26 ` [PATCH 1/2] ARM: dts: BCM5301X: " Andrew Lunn @ 2017-03-15 17:03 ` Rafał Miłecki 2017-03-15 17:03 ` [PATCH V2 2/2] ARM: dts: BCM53573: Specify serial console parameters Rafał Miłecki 2017-03-22 16:49 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Florian Fainelli 2 siblings, 2 replies; 14+ messages in thread From: Rafał Miłecki @ 2017-03-15 17:03 UTC (permalink / raw) To: linux-arm-kernel From: Rafa? Mi?ecki <rafal@milecki.pl> 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. Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl> --- 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(-) diff --git a/arch/arm/boot/dts/bcm4708.dtsi b/arch/arm/boot/dts/bcm4708.dtsi index d0eec099f1f8..1a19e97a987d 100644 --- a/arch/arm/boot/dts/bcm4708.dtsi +++ b/arch/arm/boot/dts/bcm4708.dtsi @@ -12,6 +12,14 @@ / { compatible = "brcm,bcm4708"; + aliases { + serial0 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + cpus { #address-cells = <1>; #size-cells = <0>; diff --git a/arch/arm/boot/dts/bcm47081.dtsi b/arch/arm/boot/dts/bcm47081.dtsi index 7bcd536198fc..9829d044aaf4 100644 --- a/arch/arm/boot/dts/bcm47081.dtsi +++ b/arch/arm/boot/dts/bcm47081.dtsi @@ -22,6 +22,14 @@ / { compatible = "brcm,bcm47081"; + aliases { + serial0 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + cpus { #address-cells = <1>; #size-cells = <0>; diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi index 8fd1ef9f0c2d..65dc67e86ae1 100644 --- a/arch/arm/boot/dts/bcm5301x.dtsi +++ b/arch/arm/boot/dts/bcm5301x.dtsi @@ -18,10 +18,6 @@ / { interrupt-parent = <&gic>; - chosen { - stdout-path = &uart0; - }; - chipcommonA { compatible = "simple-bus"; ranges = <0x00000000 0x18000000 0x00001000>; diff --git a/arch/arm/boot/dts/bcm94708.dts b/arch/arm/boot/dts/bcm94708.dts index 42855a7c1bfa..2e08c895f281 100644 --- a/arch/arm/boot/dts/bcm94708.dts +++ b/arch/arm/boot/dts/bcm94708.dts @@ -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>; }; diff --git a/arch/arm/boot/dts/bcm94709.dts b/arch/arm/boot/dts/bcm94709.dts index 95e8be65f2f1..c37616c67edc 100644 --- a/arch/arm/boot/dts/bcm94709.dts +++ b/arch/arm/boot/dts/bcm94709.dts @@ -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>; }; diff --git a/arch/arm/boot/dts/bcm953012er.dts b/arch/arm/boot/dts/bcm953012er.dts index decd86bae901..40e694bfe5ca 100644 --- a/arch/arm/boot/dts/bcm953012er.dts +++ b/arch/arm/boot/dts/bcm953012er.dts @@ -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>; }; diff --git a/arch/arm/boot/dts/bcm953012hr.dts b/arch/arm/boot/dts/bcm953012hr.dts index 85f9b0b872ac..5be4225324a3 100644 --- a/arch/arm/boot/dts/bcm953012hr.dts +++ b/arch/arm/boot/dts/bcm953012hr.dts @@ -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>; }; diff --git a/arch/arm/boot/dts/bcm953012k.dts b/arch/arm/boot/dts/bcm953012k.dts index a00c4e1dfb4c..fbc6219e977a 100644 --- a/arch/arm/boot/dts/bcm953012k.dts +++ b/arch/arm/boot/dts/bcm953012k.dts @@ -43,10 +43,6 @@ serial1 = &uart1; }; - chosen { - stdout-path = "serial0:115200n8"; - }; - memory { reg = <0x00000000 0x10000000>; }; -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 2/2] ARM: dts: BCM53573: Specify serial console parameters 2017-03-15 17:03 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Rafał Miłecki @ 2017-03-15 17:03 ` Rafał Miłecki 2017-03-22 16:49 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Florian Fainelli 1 sibling, 0 replies; 14+ messages in thread From: Rafał Miłecki @ 2017-03-15 17:03 UTC (permalink / raw) To: linux-arm-kernel From: Rafa? Mi?ecki <rafal@milecki.pl> This adds baud rate, parity & number of data bits. It's required to get serial working correctly. Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl> --- arch/arm/boot/dts/bcm53573.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi index 71ce23e46d30..eae623f76401 100644 --- a/arch/arm/boot/dts/bcm53573.dtsi +++ b/arch/arm/boot/dts/bcm53573.dtsi @@ -13,8 +13,12 @@ / { interrupt-parent = <&gic>; + aliases { + serial0 = &uart0; + }; + chosen { - stdout-path = &uart0; + stdout-path = "serial0:115200n8"; }; cpus { -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files 2017-03-15 17:03 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Rafał Miłecki 2017-03-15 17:03 ` [PATCH V2 2/2] ARM: dts: BCM53573: Specify serial console parameters Rafał Miłecki @ 2017-03-22 16:49 ` Florian Fainelli 1 sibling, 0 replies; 14+ messages in thread From: Florian Fainelli @ 2017-03-22 16:49 UTC (permalink / raw) To: linux-arm-kernel On 03/15/2017 10:03 AM, Rafa? Mi?ecki wrote: > From: Rafa? Mi?ecki <rafal@milecki.pl> > > 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. > > Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl> Applied, thanks! -- Florian ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-03-22 16:49 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-14 7:58 [PATCH 1/2] ARM: dts: BCM5301X: Specify serial console parameters Rafał Miłecki 2017-03-14 7:58 ` [PATCH 2/2] ARM: dts: BCM53573: " Rafał Miłecki 2017-03-14 12:26 ` [PATCH 1/2] ARM: dts: BCM5301X: " Andrew Lunn 2017-03-14 12:32 ` Rafał Miłecki 2017-03-14 12:37 ` Rafał Miłecki 2017-03-14 12:51 ` Andrew Lunn 2017-03-14 14:53 ` Jon Mason 2017-03-14 20:44 ` Scott Branden 2017-03-14 21:18 ` Jon Mason 2017-03-14 21:28 ` Scott Branden 2017-03-14 21:51 ` Andrew Lunn 2017-03-15 17:03 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Rafał Miłecki 2017-03-15 17:03 ` [PATCH V2 2/2] ARM: dts: BCM53573: Specify serial console parameters Rafał Miłecki 2017-03-22 16:49 ` [PATCH V2 1/2] ARM: dts: BCM5301X: Specify serial console params in dtsi files Florian Fainelli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).