2016-03-01 11:23:12

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH] tty: msm_serial: remove static clk rate setting in probe

The issue with setting up a fixed clock rate at probe is that it would
overwrite the console rate set by the bootloader for its console device.
This would result in serial out corruption or missing log when we system
is booted with earlycon. This is not a issue if we boot system without
earlycon.

This setup is ateast not required with the mainline driver.

Originally the issue was noticed on DB410c which is based on APQ8016
chipset.

Without this patch the console log with earlycon would look like:
...
[ 0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=1
[ 0.000000] NR_IRQS:64 nr_irqs:64 0
����+HH��0.699378] console [ttyMSM0] enabled
[ 0.699378] console [ttyMSM0] enabled
[ 0.702003] bootconsole [uart0] disabled
[ 0.702003] bootconsole [uart0] disabled
...

with this patch I can see all the skipped lines on the console

Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/tty/serial/msm_serial.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index dcde955..0a31b3e 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -1580,7 +1580,6 @@ static int msm_serial_probe(struct platform_device *pdev)
if (IS_ERR(msm_port->pclk))
return PTR_ERR(msm_port->pclk);

- clk_set_rate(msm_port->clk, 1843200);
}

port->uartclk = clk_get_rate(msm_port->clk);
--
1.9.1


2016-03-03 01:41:15

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] tty: msm_serial: remove static clk rate setting in probe

On 03/01, Srinivas Kandagatla wrote:
> The issue with setting up a fixed clock rate at probe is that it would
> overwrite the console rate set by the bootloader for its console device.
> This would result in serial out corruption or missing log when we system
> is booted with earlycon. This is not a issue if we boot system without
> earlycon.
>
> This setup is ateast not required with the mainline driver.

at least? May be worth mentioning that this code used to be
required because the clk_enable() call would fail if
clk_set_rate() wasn't called first.

>
> Originally the issue was noticed on DB410c which is based on APQ8016
> chipset.
>
> Without this patch the console log with earlycon would look like:
> ...
> [ 0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=1
> [ 0.000000] NR_IRQS:64 nr_irqs:64 0
> ����+HH��0.699378] console [ttyMSM0] enabled
> [ 0.699378] console [ttyMSM0] enabled
> [ 0.702003] bootconsole [uart0] disabled
> [ 0.702003] bootconsole [uart0] disabled
> ...
>
> with this patch I can see all the skipped lines on the console
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>

otherwise

Reviewed-by: Stephen Boyd <[email protected]>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2016-03-05 22:27:15

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] tty: msm_serial: remove static clk rate setting in probe

On Tue, Mar 01, 2016 at 11:22:53AM +0000, Srinivas Kandagatla wrote:
> The issue with setting up a fixed clock rate at probe is that it would
> overwrite the console rate set by the bootloader for its console device.
> This would result in serial out corruption or missing log when we system
> is booted with earlycon. This is not a issue if we boot system without
> earlycon.
>
> This setup is ateast not required with the mainline driver.
>
> Originally the issue was noticed on DB410c which is based on APQ8016
> chipset.
>
> Without this patch the console log with earlycon would look like:
> ...
> [ 0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=1
> [ 0.000000] NR_IRQS:64 nr_irqs:64 0
> ����+HH��0.699378] console [ttyMSM0] enabled
> [ 0.699378] console [ttyMSM0] enabled
> [ 0.702003] bootconsole [uart0] disabled
> [ 0.702003] bootconsole [uart0] disabled
> ...
>
> with this patch I can see all the skipped lines on the console
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> drivers/tty/serial/msm_serial.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index dcde955..0a31b3e 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -1580,7 +1580,6 @@ static int msm_serial_probe(struct platform_device *pdev)
> if (IS_ERR(msm_port->pclk))
> return PTR_ERR(msm_port->pclk);
>
> - clk_set_rate(msm_port->clk, 1843200);

Delete the unneeded blank line as well please, when you resend this.

thanks,

greg k-h