[PATCH v2] tty: msm_serial: remove static clk rate setting in probe

[PATCH v2] tty: msm_serial: remove static clk rate setting in probe

[Srinivas Kandagatla]

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 at least not required with the mainline driver, 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

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---

Changes since v1:
	- removed empty line suggested by Greg KH
	- added reviewed-by 
	- added extra comment suggested by Stephen Boyd.

 drivers/tty/serial/msm_serial.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index dcde955..3bf41ab 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -1579,8 +1579,6 @@ static int msm_serial_probe(struct platform_device *pdev)
 		msm_port->pclk = devm_clk_get(&pdev->dev, "iface");
 		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

Re: [PATCH v2] tty: msm_serial: remove static clk rate setting in probe

[Andy Gross]

On Mon, Mar 07, 2016 at 02:46:40AM +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 at least not required with the mainline driver, 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
> 
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Acked-by: Andy Gross <andy.gross@linaro.org>

Greg,

Can you pick this one up?

Thanks

Andy

Re: [PATCH v2] tty: msm_serial: remove static clk rate setting in probe

[gpramod]

On 2016-03-07 08:16, 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 at least not required with the mainline driver, 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
> 
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---

Tested on DB410C. Fixed below curruption with earlycon:

[    0.815843] msm_iommu_ctx 1e3b000.qcom,iommu-ctx: context periph_SDC2 
using bank 27
����[    1.411080] console [ttyMSM0] enabled
[    1.411080] console [ttyMSM0] enabled

Tested-by: Pramod Gurav <gpramod@codeaurora.org>

> 
> Changes since v1:
> 	- removed empty line suggested by Greg KH
> 	- added reviewed-by
> 	- added extra comment suggested by Stephen Boyd.
> 
>  drivers/tty/serial/msm_serial.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/msm_serial.c 
> b/drivers/tty/serial/msm_serial.c
> index dcde955..3bf41ab 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -1579,8 +1579,6 @@ static int msm_serial_probe(struct 
> platform_device *pdev)
>  		msm_port->pclk = devm_clk_get(&pdev->dev, "iface");
>  		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);