All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
To: "zajec5@gmail.com" <zajec5@gmail.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"jonmason@broadcom.com" <jonmason@broadcom.com>,
	"hauke@hauke-m.de" <hauke@hauke-m.de>,
	Alexey Brodkin <Alexey.Brodkin@synopsys.com>,
	"bcm-kernel-feedback-list@broadcom.com"
	<bcm-kernel-feedback-list@broadcom.com>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	"jslaby@suse.com" <jslaby@suse.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: ns16550 earlycon baud broken on BCM5301X since commit 31cb9a8575ca0 ("earlycon: initialise baud field of earlycon device structure")
Date: Thu, 29 Mar 2018 13:17:01 +0000	[thread overview]
Message-ID: <1522329420.32101.3.camel@synopsys.com> (raw)
In-Reply-To: <CACna6rzki=sjXQS12eF3yPXH7XeFYTCXVOdj--F=P=jkJZX7fg@mail.gmail.com>

Hi Rafał,

Hm, your 'port->uartclk' value (and therefore 'BASE_BAUD' value) looks strange to me...
Looks like you have 'BASE_BAUD' set to 115200.


Here is my example:
uart clock is 33333333Hz (fixed 33.33MHz xtal clock)
So 'BASE_BAUD' is 33333333/16 = 2083333
So 'port->uartclk' is BASE_BAUD*16 = 33333328
'device->baud' is 115200 (which is read from device tree)
So when we calculate divisor with this code
------------------->8---------------
divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
------------------->8---------------
We got 'divisor' = 181 which is correct value for us.


On Thu, 2018-03-29 at 14:34 +0200, Rafał Miłecki wrote:
> Hi,
> 
> I upgraded my BCM5301X device based on BCM4708 SoC from 4.13 to 4.14
> and noticed earlycon output is corrupted (a wrong baud rate is used).
> 
> 
> I bisected this problem down to the:
> 
> commit 31cb9a8575ca04f47ea113434d4782b695638b62
> Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> Date:   Mon Aug 21 19:22:13 2017 +0300
> 
>     earlycon: initialise baud field of earlycon device structure
> 
> 
> My device uses arch/arm/boot/dts/bcm4708.dtsi:
> 
> uart0: serial@0300 {
>         compatible = "ns16550";
>         reg = <0x0300 0x100>;
>         interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>         clocks = <&iprocslow>;
>         status = "okay";
> };
> 
> aliases {
>         serial0 = &uart0;
> };
> 
> chosen {
>         stdout-path = "serial0:115200n8";
> };
> 
> 
> A valid UART divisor for my device is 54. This is what bootloader sets
> and what works with my serial console running 115200 8n1.
> 
> 
> Before the commit 31cb9a8575ca0 early_serial8250_setup() never tried
> setting baud because device->baud was 0. It left baud (divisor) to
> whatever was configured by the bootloader. It has changed with above
> commit though. So now the setup looks like that:
> 1) port->uartclk equals 1843200 as set in the of_setup_earlycon():
> port->uartclk = BASE_BAUD * 16;
> 2) [NEW] device->baud equals 115200 as set in the of_setup_earlycon():
> early_console_dev.baud = simple_strtoul(options, NULL, 0);
> (a value of options is "115200n8")
> 3) [NEW] divisor is calculated to 1 in the init_port():
> divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
> 4) [NEW] divisor is set using UART_DLL and UART_DLM in the init_port()
> 
> Obviously setting divisor 1 instead of 54 results in a wrong baud.
> 
> 
> So right now my serial console output looks like that:
> �6;'+{.��s�.���.".��4��.�����.��J|�.�.8��.�..������g��~.����L>.��,�9z9�{�Z�.."���NC�<�9.�/���.�.�}��~.���"�.�|����;?�.��yy�.[
>    0.043623] console [ttyS0] enabled
> [    0.043623] console [ttyS0] enabled
> [    0.050842] bootconsole [ns16550] disabled
> [    0.050842] bootconsole [ns16550] disabled
> [    0.062939] libphy: Fixed MDIO Bus: probed
> (...)
> 
> For a complete log (coming from dmesg command) see attachment.
> 
> 
> Can you take a look at this problem, please? Is there something wrong
> with my DT? Or is a problem in 8250 or earlycon?
> 
-- 
 Eugeniy Paltsev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Eugeniy.Paltsev@synopsys.com (Eugeniy Paltsev)
To: linux-arm-kernel@lists.infradead.org
Subject: ns16550 earlycon baud broken on BCM5301X since commit 31cb9a8575ca0 ("earlycon: initialise baud field of earlycon device structure")
Date: Thu, 29 Mar 2018 13:17:01 +0000	[thread overview]
Message-ID: <1522329420.32101.3.camel@synopsys.com> (raw)
In-Reply-To: <CACna6rzki=sjXQS12eF3yPXH7XeFYTCXVOdj--F=P=jkJZX7fg@mail.gmail.com>

Hi Rafa?,

Hm, your 'port->uartclk' value (and therefore 'BASE_BAUD' value) looks strange to me...
Looks like you have 'BASE_BAUD' set to 115200.


Here is my example:
uart clock is 33333333Hz (fixed 33.33MHz xtal clock)
So 'BASE_BAUD' is 33333333/16 = 2083333
So 'port->uartclk' is BASE_BAUD*16 = 33333328
'device->baud' is 115200 (which is read from device tree)
So when we calculate divisor with this code
------------------->8---------------
divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
------------------->8---------------
We got 'divisor' = 181 which is correct value for us.


On Thu, 2018-03-29 at 14:34 +0200, Rafa? Mi?ecki wrote:
> Hi,
> 
> I upgraded my BCM5301X device based on BCM4708 SoC from 4.13 to 4.14
> and noticed earlycon output is corrupted (a wrong baud rate is used).
> 
> 
> I bisected this problem down to the:
> 
> commit 31cb9a8575ca04f47ea113434d4782b695638b62
> Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> Date:   Mon Aug 21 19:22:13 2017 +0300
> 
>     earlycon: initialise baud field of earlycon device structure
> 
> 
> My device uses arch/arm/boot/dts/bcm4708.dtsi:
> 
> uart0: serial at 0300 {
>         compatible = "ns16550";
>         reg = <0x0300 0x100>;
>         interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>         clocks = <&iprocslow>;
>         status = "okay";
> };
> 
> aliases {
>         serial0 = &uart0;
> };
> 
> chosen {
>         stdout-path = "serial0:115200n8";
> };
> 
> 
> A valid UART divisor for my device is 54. This is what bootloader sets
> and what works with my serial console running 115200 8n1.
> 
> 
> Before the commit 31cb9a8575ca0 early_serial8250_setup() never tried
> setting baud because device->baud was 0. It left baud (divisor) to
> whatever was configured by the bootloader. It has changed with above
> commit though. So now the setup looks like that:
> 1) port->uartclk equals 1843200 as set in the of_setup_earlycon():
> port->uartclk = BASE_BAUD * 16;
> 2) [NEW] device->baud equals 115200 as set in the of_setup_earlycon():
> early_console_dev.baud = simple_strtoul(options, NULL, 0);
> (a value of options is "115200n8")
> 3) [NEW] divisor is calculated to 1 in the init_port():
> divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
> 4) [NEW] divisor is set using UART_DLL and UART_DLM in the init_port()
> 
> Obviously setting divisor 1 instead of 54 results in a wrong baud.
> 
> 
> So right now my serial console output looks like that:
> ?6;'+{.??s?.???.".??4??.?????.??J|?.?.8??.?..??????g??~.????L>.??,?9z9?{?Z?.."???NC?<?9.?/???.?.?}??~.???"?.?|????;??.??yy?.[
>    0.043623] console [ttyS0] enabled
> [    0.043623] console [ttyS0] enabled
> [    0.050842] bootconsole [ns16550] disabled
> [    0.050842] bootconsole [ns16550] disabled
> [    0.062939] libphy: Fixed MDIO Bus: probed
> (...)
> 
> For a complete log (coming from dmesg command) see attachment.
> 
> 
> Can you take a look at this problem, please? Is there something wrong
> with my DT? Or is a problem in 8250 or earlycon?
> 
-- 
 Eugeniy Paltsev

  reply	other threads:[~2018-03-29 13:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-29 12:34 ns16550 earlycon baud broken on BCM5301X since commit 31cb9a8575ca0 ("earlycon: initialise baud field of earlycon device structure") Rafał Miłecki
2018-03-29 12:34 ` Rafał Miłecki
2018-03-29 13:17 ` Eugeniy Paltsev [this message]
2018-03-29 13:17   ` Eugeniy Paltsev
2018-03-29 13:23   ` Koen Vandeputte
2018-03-29 13:23     ` Koen Vandeputte
2018-03-29 14:37   ` Rafał Miłecki
2018-03-29 14:37     ` Rafał Miłecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1522329420.32101.3.camel@synopsys.com \
    --to=eugeniy.paltsev@synopsys.com \
    --cc=Alexey.Brodkin@synopsys.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hauke@hauke-m.de \
    --cc=jonmason@broadcom.com \
    --cc=jslaby@suse.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=zajec5@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.