linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] clk: tegra: initialise parent of uart clocks
@ 2013-02-12 15:17 Laxman Dewangan
  2013-02-12 17:46 ` Stephen Warren
  0 siblings, 1 reply; 6+ messages in thread
From: Laxman Dewangan @ 2013-02-12 15:17 UTC (permalink / raw)
  To: swarren, mturquette
  Cc: pgaikwad, pdeschrijver, linux-kernel, linux-tegra, Laxman Dewangan

Initialise the parent of UARTs to PLLP and disabling clock by
default.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
As suggested by Stephen, Make other uarts clock state to disable as
driver already enable these clocks.

 drivers/clk/tegra/clk-tegra20.c |    7 +++++--
 drivers/clk/tegra/clk-tegra30.c |    6 +++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 4612b2e..8b5241e 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1254,8 +1254,11 @@ static __initdata struct tegra_clk_init_table init_table[] = {
 	{csite, clk_max, 0, 1},
 	{emc, clk_max, 0, 1},
 	{cclk, clk_max, 0, 1},
-	{uarta, pll_p, 0, 1},
-	{uartd, pll_p, 0, 1},
+	{uarta, pll_p, 0, 0},
+	{uartb, pll_p, 0, 0},
+	{uartc, pll_p, 0, 0},
+	{uartd, pll_p, 0, 0},
+	{uarte, pll_p, 0, 0},
 	{usbd, clk_max, 12000000, 0},
 	{usb2, clk_max, 12000000, 0},
 	{usb3, clk_max, 12000000, 0},
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index bf050bc..56925e1 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1877,7 +1877,11 @@ static struct tegra_cpu_car_ops tegra30_cpu_car_ops = {
 };
 
 static __initdata struct tegra_clk_init_table init_table[] = {
-	{uarta, pll_p, 408000000, 1},
+	{uarta, pll_p, 408000000, 0},
+	{uartb, pll_p, 408000000, 0},
+	{uartc, pll_p, 408000000, 0},
+	{uartd, pll_p, 408000000, 0},
+	{uarte, pll_p, 408000000, 0},
 	{pll_a, clk_max, 564480000, 1},
 	{pll_a_out0, clk_max, 11289600, 1},
 	{extern1, pll_a_out0, 0, 1},
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] clk: tegra: initialise parent of uart clocks
  2013-02-12 15:17 [PATCH V2] clk: tegra: initialise parent of uart clocks Laxman Dewangan
@ 2013-02-12 17:46 ` Stephen Warren
  2013-02-12 19:34   ` Mike Turquette
  2013-02-13  7:02   ` Laxman Dewangan
  0 siblings, 2 replies; 6+ messages in thread
From: Stephen Warren @ 2013-02-12 17:46 UTC (permalink / raw)
  To: Laxman Dewangan, mturquette
  Cc: swarren, pgaikwad, pdeschrijver, linux-kernel, linux-tegra

On 02/12/2013 08:17 AM, Laxman Dewangan wrote:
> Initialise the parent of UARTs to PLLP and disabling clock by
> default.

This patch wasn't tested, was it? Without the patch I just sent titled
"ARM: tegra: remove clock-frequency properties from serial nodes", the
UART clocks get turned off and the console breaks.

Anyway, I've applied this patch to Tegra's for-3.9/soc-ccf-fixes branch
along with that other patch, although I'll hold off on pushing them out
while I wait for an ack from Mike on this patch, and give the other one
time to get reviews.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] clk: tegra: initialise parent of uart clocks
  2013-02-12 17:46 ` Stephen Warren
@ 2013-02-12 19:34   ` Mike Turquette
  2013-02-13  7:02   ` Laxman Dewangan
  1 sibling, 0 replies; 6+ messages in thread
From: Mike Turquette @ 2013-02-12 19:34 UTC (permalink / raw)
  To: Stephen Warren, Laxman Dewangan
  Cc: swarren, pgaikwad, pdeschrijver, linux-kernel, linux-tegra

Quoting Stephen Warren (2013-02-12 09:46:25)
> On 02/12/2013 08:17 AM, Laxman Dewangan wrote:
> > Initialise the parent of UARTs to PLLP and disabling clock by
> > default.
> 
> This patch wasn't tested, was it? Without the patch I just sent titled
> "ARM: tegra: remove clock-frequency properties from serial nodes", the
> UART clocks get turned off and the console breaks.
> 
> Anyway, I've applied this patch to Tegra's for-3.9/soc-ccf-fixes branch
> along with that other patch, although I'll hold off on pushing them out
> while I wait for an ack from Mike on this patch, and give the other one
> time to get reviews.

Acked-by: Mike Turquette <mturquette@linaro.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] clk: tegra: initialise parent of uart clocks
  2013-02-12 17:46 ` Stephen Warren
  2013-02-12 19:34   ` Mike Turquette
@ 2013-02-13  7:02   ` Laxman Dewangan
  2013-02-13 16:34     ` Stephen Warren
  1 sibling, 1 reply; 6+ messages in thread
From: Laxman Dewangan @ 2013-02-13  7:02 UTC (permalink / raw)
  To: Stephen Warren
  Cc: mturquette, Stephen Warren, Prashant Gaikwad, Peter De Schrijver,
	linux-kernel, linux-tegra

On Tuesday 12 February 2013 11:16 PM, Stephen Warren wrote:
> On 02/12/2013 08:17 AM, Laxman Dewangan wrote:
>> Initialise the parent of UARTs to PLLP and disabling clock by
>> default.
> This patch wasn't tested, was it? Without the patch I just sent titled
> "ARM: tegra: remove clock-frequency properties from serial nodes", the
> UART clocks get turned off and the console breaks.

I tested this in next-20130212 where "clock-frequency" is there in 
tegra30.dtsi file.
If you remove this from dts file at all then it will not work as there 
is no execution path to call the clk_prepare_enable().

of_serial.c file:
         if (of_property_read_u32(np, "clock-frequency", &clk)) {

                 /* Get clk rate through clk driver if present */
                 info->clk = clk_get(&ofdev->dev, NULL);
                 if (IS_ERR(info->clk)) {
                         dev_warn(&ofdev->dev,
                                 "clk or clock-frequency not defined\n");
                         return PTR_ERR(info->clk);
                 }

                 clk_prepare_enable(info->clk);
                 clk = clk_get_rate(info->clk);
         }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] clk: tegra: initialise parent of uart clocks
  2013-02-13  7:02   ` Laxman Dewangan
@ 2013-02-13 16:34     ` Stephen Warren
  2013-02-14 10:51       ` Laxman Dewangan
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2013-02-13 16:34 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: mturquette, Stephen Warren, Prashant Gaikwad, Peter De Schrijver,
	linux-kernel, linux-tegra

On 02/13/2013 12:02 AM, Laxman Dewangan wrote:
> On Tuesday 12 February 2013 11:16 PM, Stephen Warren wrote:
>> On 02/12/2013 08:17 AM, Laxman Dewangan wrote:
>>> Initialise the parent of UARTs to PLLP and disabling clock by
>>> default.
>>
>> This patch wasn't tested, was it? Without the patch I just sent titled
>> "ARM: tegra: remove clock-frequency properties from serial nodes", the
>> UART clocks get turned off and the console breaks.
> 
> I tested this in next-20130212 where "clock-frequency" is there in
> tegra30.dtsi file.

I tested on Cardhu/Tegra30 (I'd previously only tested on
Harmony/Tegra20), and I see the exact same problem; the clock core turns
off the UART clock and the system hangs.

Are you sure you don't have any other local patches that somehow keep
the UART clock on (e.g. disabling clk_disable_unused, calling clk_get()
on the UART clock somewhere else, have the Tegra HS UART driver enabled
on the console port for testing which perhaps does an unconditional
clk_get). Are you sure you're testing with U-Boot rather than our binary
bootloader; who knows what kind of voodoo that does to clocks.

Note: I tested: a merge of arm-soc/for-next and Tegra's for-next branch,
with my patch to remove the clock-frequency properties reverted, plus a
fix for the boot crash ("clockevents: fix generic broadcast for
FEAT_C3STOP").

> If you remove this from dts file at all then it will not work as there
> is no execution path to call the clk_prepare_enable().
> 
> of_serial.c file:
>         if (of_property_read_u32(np, "clock-frequency", &clk)) {

That returns 0 if the property is present (so doesn't get/enable the
clock), and an error code if the property is missing (so does get/enable
the clock).

> 
>                 /* Get clk rate through clk driver if present */
>                 info->clk = clk_get(&ofdev->dev, NULL);
>                 if (IS_ERR(info->clk)) {
>                         dev_warn(&ofdev->dev,
>                                 "clk or clock-frequency not defined\n");
>                         return PTR_ERR(info->clk);
>                 }
> 
>                 clk_prepare_enable(info->clk);
>                 clk = clk_get_rate(info->clk);
>         }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] clk: tegra: initialise parent of uart clocks
  2013-02-13 16:34     ` Stephen Warren
@ 2013-02-14 10:51       ` Laxman Dewangan
  0 siblings, 0 replies; 6+ messages in thread
From: Laxman Dewangan @ 2013-02-14 10:51 UTC (permalink / raw)
  To: Stephen Warren
  Cc: mturquette, Stephen Warren, Prashant Gaikwad, Peter De Schrijver,
	linux-kernel, linux-tegra

On Wednesday 13 February 2013 10:04 PM, Stephen Warren wrote:
> On 02/13/2013 12:02 AM, Laxman Dewangan wrote:
>> On Tuesday 12 February 2013 11:16 PM, Stephen Warren wrote:
>>> On 02/12/2013 08:17 AM, Laxman Dewangan wrote:
>>>> Initialise the parent of UARTs to PLLP and disabling clock by
>>>> default.
>>> This patch wasn't tested, was it? Without the patch I just sent titled
>>> "ARM: tegra: remove clock-frequency properties from serial nodes", the
>>> UART clocks get turned off and the console breaks.
>> I tested this in next-20130212 where "clock-frequency" is there in
>> tegra30.dtsi file.
> I tested on Cardhu/Tegra30 (I'd previously only tested on
> Harmony/Tegra20), and I see the exact same problem; the clock core turns
> off the UART clock and the system hangs.
>
> Are you sure you don't have any other local patches that somehow keep
> the UART clock on (e.g. disabling clk_disable_unused, calling clk_get()
> on the UART clock somewhere else, have the Tegra HS UART driver enabled
> on the console port for testing which perhaps does an unconditional
> clk_get). Are you sure you're testing with U-Boot rather than our binary
> bootloader; who knows what kind of voodoo that does to clocks.
>
> Note: I tested: a merge of arm-soc/for-next and Tegra's for-next branch,
> with my patch to remove the clock-frequency properties reverted, plus a
> fix for the boot crash ("clockevents: fix generic broadcast for
> FEAT_C3STOP").
>
>> If you remove this from dts file at all then it will not work as there
>> is no execution path to call the clk_prepare_enable().
>>
>> of_serial.c file:
>>          if (of_property_read_u32(np, "clock-frequency", &clk)) {
> That returns 0 if the property is present (so doesn't get/enable the
> clock), and an error code if the property is missing (so does get/enable
> the clock).

Ahha..Yes, got it. I tested this in next-20130212 with the maintainer's 
patch for broadcast, reverting one change "OF: convert devtree lock from 
rw_lock to raw spinlock" and
uart clock change. The system was booting but not reaching to login. I 
was assumign that it may be the issue with other reason as I saw the 
uart log.

After taking your patch for removing clock-freq and this patch, the 
system booted to login prompt means it is working fine.


So I was wrong when telling clock-frequency is required. Actually it 
should not be to work properly.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-14 10:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-12 15:17 [PATCH V2] clk: tegra: initialise parent of uart clocks Laxman Dewangan
2013-02-12 17:46 ` Stephen Warren
2013-02-12 19:34   ` Mike Turquette
2013-02-13  7:02   ` Laxman Dewangan
2013-02-13 16:34     ` Stephen Warren
2013-02-14 10:51       ` Laxman Dewangan

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).