Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1530918 > unrolled thread

[PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path

Started byAndreas Färber <afaerber@suse.de>
First post2016-11-27 20:40 +0100
Last post2016-11-27 23:40 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path Andreas Färber <afaerber@suse.de> - 2016-11-27 20:40 +0100
    Re: [PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path Andrew Lunn <andrew@lunn.ch> - 2016-11-27 22:20 +0100
    Re: [PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path Uwe Kleine-König          <u.kleine-koenig@pengutronix.de> - 2016-11-27 22:30 +0100
      Re: [PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path Andreas Färber <afaerber@suse.de> - 2016-11-27 23:20 +0100
        Re: [PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path Uwe Kleine-König          <u.kleine-koenig@pengutronix.de> - 2016-11-27 23:40 +0100

#1530918 — [PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path

FromAndreas Färber <afaerber@suse.de>
Date2016-11-27 20:40 +0100
Subject[PATCH] ARM: dts: mvebu: Fix armada-385-turris-omnia stdout-path
Message-ID<sIdsS-b8-11@gated-at.bofh.it>
Specify the baudrate.

Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index f53cb8b73610..2eff012287d4 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -52,7 +52,7 @@
 	compatible = "cznic,turris-omnia", "marvell,armada385", "marvell,armada380";
 
 	chosen {
-		stdout-path = &uart0;
+		stdout-path = "serial0:115200n8";
 	};
 
 	memory {
-- 
2.6.6

[toc] | [next] | [standalone]


#1530933

FromAndrew Lunn <andrew@lunn.ch>
Date2016-11-27 22:20 +0100
Message-ID<sIf1E-1fZ-19@gated-at.bofh.it>
In reply to#1530918
On Sun, Nov 27, 2016 at 08:37:24PM +0100, Andreas Färber wrote:
> Specify the baudrate.

Hi Andreas

Please put each patch/patchset in a new thread.

> Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
> Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[toc] | [prev] | [next] | [standalone]


#1530936

FromUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date2016-11-27 22:30 +0100
Message-ID<sIfbk-1jb-11@gated-at.bofh.it>
In reply to#1530918
On Sun, Nov 27, 2016 at 08:37:24PM +0100, Andreas Färber wrote:
> Specify the baudrate.
> 
> Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
> Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

You said with plain &uart0 the kernel uses a wrong baud rate? That's
strange. For me it works and I think it's the intended behaviour to
dermine the baud rate setup by the bootloader and use this.

I'd prefer it this way over hard coding the baud rate.

>  arch/arm/boot/dts/armada-385-turris-omnia.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> index f53cb8b73610..2eff012287d4 100644
> --- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> @@ -52,7 +52,7 @@
>  	compatible = "cznic,turris-omnia", "marvell,armada385", "marvell,armada380";
>  
>  	chosen {
> -		stdout-path = &uart0;
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	memory {

This has the downside to depend on the alias. Not sure this is
considered modern. An alternative would be:

	stdout-path = "/soc/internal-regs/serial@12000:115200n8";

(maybe there even exists syntactic sugar to express this using &uart0?)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[toc] | [prev] | [next] | [standalone]


#1530946

FromAndreas Färber <afaerber@suse.de>
Date2016-11-27 23:20 +0100
Message-ID<sIfXH-1TO-7@gated-at.bofh.it>
In reply to#1530936
Am 27.11.2016 um 22:25 schrieb Uwe Kleine-König:
> On Sun, Nov 27, 2016 at 08:37:24PM +0100, Andreas Färber wrote:
>> Specify the baudrate.
>>
>> Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
>> Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> You said with plain &uart0 the kernel uses a wrong baud rate? That's
> strange. For me it works and I think it's the intended behaviour to
> dermine the baud rate setup by the bootloader and use this.

IIRC the 8250 driver defaults to 9600n8 if unspecified.

Kernel tested: 4.9.0-rc2-next-20161028-00010-g4fb44d9-dirty

Maybe you used some console= argument overriding it?

> I'd prefer it this way over hard coding the baud rate.
> 
>>  arch/arm/boot/dts/armada-385-turris-omnia.dts | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
>> index f53cb8b73610..2eff012287d4 100644
>> --- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
>> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
>> @@ -52,7 +52,7 @@
>>  	compatible = "cznic,turris-omnia", "marvell,armada385", "marvell,armada380";
>>  
>>  	chosen {
>> -		stdout-path = &uart0;
>> +		stdout-path = "serial0:115200n8";
>>  	};
>>  
>>  	memory {
> 
> This has the downside to depend on the alias. Not sure this is
> considered modern. An alternative would be:
> 
> 	stdout-path = "/soc/internal-regs/serial@12000:115200n8";

Please don't unroll the path I'm trying to abstract elsewhere.

Like I said, the "serialX:115200n8" syntax is what all other Armada 38x
boards use:

git grep stdout-path -- arch/arm/boot/dts/ | grep armada-38
arch/arm/boot/dts/armada-385-db-ap.dts:		stdout-path = "serial1:115200n8";
arch/arm/boot/dts/armada-385-linksys.dtsi:		stdout-path =
"serial0:115200n8";
arch/arm/boot/dts/armada-388-clearfog.dts:		stdout-path =
"serial0:115200n8";
arch/arm/boot/dts/armada-388-db.dts:		stdout-path = "serial0:115200n8";
arch/arm/boot/dts/armada-388-gp.dts:		stdout-path = "serial0:115200n8";
arch/arm/boot/dts/armada-388-rd.dts:		stdout-path = "serial0:115200n8";

The alias is needed to reliably determine the tty device number and is
set "globally" in armada-38x.dtsi, so why is it a problem to rely on?

Regards,
Andreas

> 
> (maybe there even exists syntactic sugar to express this using &uart0?)

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]


#1530951

FromUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date2016-11-27 23:40 +0100
Message-ID<sIgh4-207-17@gated-at.bofh.it>
In reply to#1530946
Hello,

On Sun, Nov 27, 2016 at 11:14:37PM +0100, Andreas Färber wrote:
> Am 27.11.2016 um 22:25 schrieb Uwe Kleine-König:
> > On Sun, Nov 27, 2016 at 08:37:24PM +0100, Andreas Färber wrote:
> >> Specify the baudrate.
> >>
> >> Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
> >> Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> > 
> > You said with plain &uart0 the kernel uses a wrong baud rate? That's
> > strange. For me it works and I think it's the intended behaviour to
> > dermine the baud rate setup by the bootloader and use this.
> 
> IIRC the 8250 driver defaults to 9600n8 if unspecified.
> 
> Kernel tested: 4.9.0-rc2-next-20161028-00010-g4fb44d9-dirty
> 
> Maybe you used some console= argument overriding it?

Yes, you're right.

> > I'd prefer it this way over hard coding the baud rate.
> > 
> >>  arch/arm/boot/dts/armada-385-turris-omnia.dts | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> >> index f53cb8b73610..2eff012287d4 100644
> >> --- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
> >> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> >> @@ -52,7 +52,7 @@
> >>  	compatible = "cznic,turris-omnia", "marvell,armada385", "marvell,armada380";
> >>  
> >>  	chosen {
> >> -		stdout-path = &uart0;
> >> +		stdout-path = "serial0:115200n8";
> >>  	};
> >>  
> >>  	memory {
> > 
> > This has the downside to depend on the alias. Not sure this is
> > considered modern. An alternative would be:
> > 
> > 	stdout-path = "/soc/internal-regs/serial@12000:115200n8";
> 
> Please don't unroll the path I'm trying to abstract elsewhere.

Yeah, specifying the path isn't nice.

> Like I said, the "serialX:115200n8" syntax is what all other Armada 38x
> boards use:

This isn't a reason to not think about better alternatives. An if
something like:

	stdout-path = &uart0 + ":115200n8";

would be possible, I'd definitely prefer it over "serial0:115200n8".

> git grep stdout-path -- arch/arm/boot/dts/ | grep armada-38
> arch/arm/boot/dts/armada-385-db-ap.dts:		stdout-path = "serial1:115200n8";
> arch/arm/boot/dts/armada-385-linksys.dtsi:		stdout-path =
> "serial0:115200n8";
> arch/arm/boot/dts/armada-388-clearfog.dts:		stdout-path =
> "serial0:115200n8";
> arch/arm/boot/dts/armada-388-db.dts:		stdout-path = "serial0:115200n8";
> arch/arm/boot/dts/armada-388-gp.dts:		stdout-path = "serial0:115200n8";
> arch/arm/boot/dts/armada-388-rd.dts:		stdout-path = "serial0:115200n8";
> 
> The alias is needed to reliably determine the tty device number and is
> set "globally" in armada-38x.dtsi, so why is it a problem to rely on?

AFAIK aliases are seen as (still necessary) evil by the dt people. So if
you can stop making use of them, that would be nice.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web