[PATCH resend 1/2] clk: tegra: remove USB from clk init table

[PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Lucas Stach]

The USB clocks are just clock gates, so no need to set a specific clock.
In fact trying to set a specific clock is just a NOP if the requested
clockrate is the same as those of the parent (clk_m) or will trigger a
WARN_ON() if rates don't match up.

As we are not setting a specific rate, nor activating the clocks at
init, there is no point in keeping the the usb entries in the clock init
table.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
Trace produced by system with 13MHz clk_m:

tegra_init_from_table: Failed to set rate 12000000 of usbd
------------[ cut here ]------------
WARNING: at drivers/clk/tegra/clk.c:64
tegra_init_from_table+0xc0/0x158()
Modules linked in:
[<c0013a84>] (unwind_backtrace+0x0/0xf8) from
[<c0021a24>](warn_slowpath_common+0x4c/0x64)
[<c0021a24>] (warn_slowpath_common+0x4c/0x64) from [<c0021a58>]
(warn_slowpath_null+0x1c/0x24)
[<c0021a58>] (warn_slowpath_null+0x1c/0x24) from [<c069343c>]
(tegra_init_from_table+0xc0/0x158)
[<c069343c>] (tegra_init_from_table+0xc0/0x158) from [<c0694878>]
(tegra20_clock_init+0x1398/0x13d4)
[<c0694878>] (tegra20_clock_init+0x1398/0x13d4) from [<c0693298>]
(of_clk_init+0x30/0x58)
[<c0693298>] (of_clk_init+0x30/0x58) from [<c0681e5c>]
(tegra_dt_init_irq+0x8/0x1c)
[<c0681e5c>] (tegra_dt_init_irq+0x8/0x1c) from [<c067d334>]
(init_IRQ+0x14/0x1c)
[<c067d334>] (init_IRQ+0x14/0x1c) from [<c067b6b4>]
(start_kernel+0x1a0/0x2f8)
[<c067b6b4>] (start_kernel+0x1a0/0x2f8) from [<0000807c>] (0x807c)
---[ end trace 1b75b31a2719ed1c ]---
---
 drivers/clk/tegra/clk-tegra20.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index f873dce..a73278f 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1259,9 +1259,6 @@ static __initdata struct tegra_clk_init_table init_table[] = {
 	{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},
 	{pll_a, clk_max, 56448000, 1},
 	{pll_a_out0, clk_max, 11289600, 1},
 	{cdev1, clk_max, 0, 1},
-- 
1.8.1.4

[PATCH resend 2/2] clk: tegra: add ac97 controller clock

[Lucas Stach]

AC97 controller clock is hardwired to pll_a_out0.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/clk/tegra/clk-tegra20.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index a73278f..bbcca91 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -897,6 +897,14 @@ static void __init tegra20_periph_clk_init(void)
 	struct clk *clk;
 	int i;
 
+	/* ac97 */
+	clk = tegra_clk_register_periph_gate("ac97", "pll_a_out0",
+				    TEGRA_PERIPH_ON_APB,
+				    clk_base, 0, 3, &periph_l_regs,
+				    periph_clk_enb_refcnt);
+	clk_register_clkdev(clk, NULL, "tegra20-ac97");
+	clks[ac97] = clk;
+
 	/* apbdma */
 	clk = tegra_clk_register_periph_gate("apbdma", "pclk", 0, clk_base,
 				    0, 34, &periph_h_regs,
-- 
1.8.1.4

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Peter De Schrijver]

On Mon, Apr 15, 2013 at 09:31:44AM +0200, Lucas Stach wrote:
> The USB clocks are just clock gates, so no need to set a specific clock.
> In fact trying to set a specific clock is just a NOP if the requested
> clockrate is the same as those of the parent (clk_m) or will trigger a
> WARN_ON() if rates don't match up.
> 
> As we are not setting a specific rate, nor activating the clocks at
> init, there is no point in keeping the the usb entries in the clock init
> table.
> 

less entries in the clock init table is always good.

Acked-By: Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Cheers,

Peter.

Re: [PATCH resend 2/2] clk: tegra: add ac97 controller clock

[Peter De Schrijver]

On Mon, Apr 15, 2013 at 09:31:45AM +0200, Lucas Stach wrote:
> AC97 controller clock is hardwired to pll_a_out0.
> 

Prashant, did you just forget to add this clock?

> Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> ---
>  drivers/clk/tegra/clk-tegra20.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index a73278f..bbcca91 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -897,6 +897,14 @@ static void __init tegra20_periph_clk_init(void)
>  	struct clk *clk;
>  	int i;
>  
> +	/* ac97 */
> +	clk = tegra_clk_register_periph_gate("ac97", "pll_a_out0",
> +				    TEGRA_PERIPH_ON_APB,
> +				    clk_base, 0, 3, &periph_l_regs,
> +				    periph_clk_enb_refcnt);
> +	clk_register_clkdev(clk, NULL, "tegra20-ac97");
> +	clks[ac97] = clk;
> +
>  	/* apbdma */
>  	clk = tegra_clk_register_periph_gate("apbdma", "pclk", 0, clk_base,
>  				    0, 34, &periph_h_regs,

Otherwise:

Acked-By: Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Stephen Warren]

On 04/15/2013 01:31 AM, Lucas Stach wrote:
> The USB clocks are just clock gates, so no need to set a specific clock.
> In fact trying to set a specific clock is just a NOP if the requested
> clockrate is the same as those of the parent (clk_m) or will trigger a
> WARN_ON() if rates don't match up.
> 
> As we are not setting a specific rate, nor activating the clocks at
> init, there is no point in keeping the the usb entries in the clock init
> table.

Peter, Prashant, I'd like to confirm that the usb* clocks really do have
clk_m as their parent; we're sure they aren't driven by the 12MHz PLL_U
output?

Either way, I guess it's safe to take this patch since the clock would
be fixed rate; I'd just like to make sure the clock driver is accurate.

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Prashant Gaikwad]

On Monday 15 April 2013 01:01 PM, Lucas Stach wrote:
> The USB clocks are just clock gates, so no need to set a specific clock.
> In fact trying to set a specific clock is just a NOP if the requested
> clockrate is the same as those of the parent (clk_m) or will trigger a
> WARN_ON() if rates don't match up.
>
> As we are not setting a specific rate, nor activating the clocks at
> init, there is no point in keeping the the usb entries in the clock init
> table.
>
> Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> ---
> Trace produced by system with 13MHz clk_m:

Reviewed-by: Prashant Gaikwad <pgaikwad-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

> tegra_init_from_table: Failed to set rate 12000000 of usbd
> ------------[ cut here ]------------
> WARNING: at drivers/clk/tegra/clk.c:64
> tegra_init_from_table+0xc0/0x158()
> Modules linked in:
> [<c0013a84>] (unwind_backtrace+0x0/0xf8) from
> [<c0021a24>](warn_slowpath_common+0x4c/0x64)
> [<c0021a24>] (warn_slowpath_common+0x4c/0x64) from [<c0021a58>]
> (warn_slowpath_null+0x1c/0x24)
> [<c0021a58>] (warn_slowpath_null+0x1c/0x24) from [<c069343c>]
> (tegra_init_from_table+0xc0/0x158)
> [<c069343c>] (tegra_init_from_table+0xc0/0x158) from [<c0694878>]
> (tegra20_clock_init+0x1398/0x13d4)
> [<c0694878>] (tegra20_clock_init+0x1398/0x13d4) from [<c0693298>]
> (of_clk_init+0x30/0x58)
> [<c0693298>] (of_clk_init+0x30/0x58) from [<c0681e5c>]
> (tegra_dt_init_irq+0x8/0x1c)
> [<c0681e5c>] (tegra_dt_init_irq+0x8/0x1c) from [<c067d334>]
> (init_IRQ+0x14/0x1c)
> [<c067d334>] (init_IRQ+0x14/0x1c) from [<c067b6b4>]
> (start_kernel+0x1a0/0x2f8)
> [<c067b6b4>] (start_kernel+0x1a0/0x2f8) from [<0000807c>] (0x807c)
> ---[ end trace 1b75b31a2719ed1c ]---
> ---
>   drivers/clk/tegra/clk-tegra20.c | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index f873dce..a73278f 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -1259,9 +1259,6 @@ static __initdata struct tegra_clk_init_table init_table[] = {
>   	{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},
>   	{pll_a, clk_max, 56448000, 1},
>   	{pll_a_out0, clk_max, 11289600, 1},
>   	{cdev1, clk_max, 0, 1},

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Prashant Gaikwad]

On Monday 15 April 2013 11:17 PM, Stephen Warren wrote:
> On 04/15/2013 01:31 AM, Lucas Stach wrote:
>> The USB clocks are just clock gates, so no need to set a specific clock.
>> In fact trying to set a specific clock is just a NOP if the requested
>> clockrate is the same as those of the parent (clk_m) or will trigger a
>> WARN_ON() if rates don't match up.
>>
>> As we are not setting a specific rate, nor activating the clocks at
>> init, there is no point in keeping the the usb entries in the clock init
>> table.
> Peter, Prashant, I'd like to confirm that the usb* clocks really do have
> clk_m as their parent; we're sure they aren't driven by the 12MHz PLL_U
> output?
>
> Either way, I guess it's safe to take this patch since the clock would
> be fixed rate; I'd just like to make sure the clock driver is accurate.

These are controller clocks and are not driven by PLL_U.

Re: [PATCH resend 2/2] clk: tegra: add ac97 controller clock

[Prashant Gaikwad]

On Monday 15 April 2013 01:01 PM, Lucas Stach wrote:
> AC97 controller clock is hardwired to pll_a_out0.
>
> Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> ---

It was not there in previous implementation neither do we implement it 
in our downstream kernel.

Lucas, are you using this clock anywhere?

Reviewed-by: Prashant Gaikwad <pgaikwad-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

>   drivers/clk/tegra/clk-tegra20.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index a73278f..bbcca91 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -897,6 +897,14 @@ static void __init tegra20_periph_clk_init(void)
>   	struct clk *clk;
>   	int i;
>   
> +	/* ac97 */
> +	clk = tegra_clk_register_periph_gate("ac97", "pll_a_out0",
> +				    TEGRA_PERIPH_ON_APB,
> +				    clk_base, 0, 3, &periph_l_regs,
> +				    periph_clk_enb_refcnt);
> +	clk_register_clkdev(clk, NULL, "tegra20-ac97");
> +	clks[ac97] = clk;
> +
>   	/* apbdma */
>   	clk = tegra_clk_register_periph_gate("apbdma", "pclk", 0, clk_base,
>   				    0, 34, &periph_h_regs,

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Stephen Warren]

On 04/17/2013 06:14 AM, Prashant Gaikwad wrote:
> On Monday 15 April 2013 11:17 PM, Stephen Warren wrote:
>> On 04/15/2013 01:31 AM, Lucas Stach wrote:
>>> The USB clocks are just clock gates, so no need to set a specific clock.
>>> In fact trying to set a specific clock is just a NOP if the requested
>>> clockrate is the same as those of the parent (clk_m) or will trigger a
>>> WARN_ON() if rates don't match up.
>>>
>>> As we are not setting a specific rate, nor activating the clocks at
>>> init, there is no point in keeping the the usb entries in the clock init
>>> table.
>> Peter, Prashant, I'd like to confirm that the usb* clocks really do have
>> clk_m as their parent; we're sure they aren't driven by the 12MHz PLL_U
>> output?
>>
>> Either way, I guess it's safe to take this patch since the clock would
>> be fixed rate; I'd just like to make sure the clock driver is accurate.
> 
> These are controller clocks and are not driven by PLL_U.

So just to confirm: does that mean that they truly /are/ direct children
of clk_m, just like the driver says right now?

Re: [PATCH resend 2/2] clk: tegra: add ac97 controller clock

[Stephen Warren]

On 04/17/2013 06:17 AM, Prashant Gaikwad wrote:
> On Monday 15 April 2013 01:01 PM, Lucas Stach wrote:
>> AC97 controller clock is hardwired to pll_a_out0.
>>
>> Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
>> ---
> 
> It was not there in previous implementation neither do we implement it
> in our downstream kernel.
> 
> Lucas, are you using this clock anywhere?

Yes, there's an AC'97 driver upstream now, which Lucas wrote. It's used
on the Colibri T20 board.

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Stephen Warren]

On 04/15/2013 01:31 AM, Lucas Stach wrote:
> The USB clocks are just clock gates, so no need to set a specific clock.
> In fact trying to set a specific clock is just a NOP if the requested
> clockrate is the same as those of the parent (clk_m) or will trigger a
> WARN_ON() if rates don't match up.
> 
> As we are not setting a specific rate, nor activating the clocks at
> init, there is no point in keeping the the usb entries in the clock init
> table.

Lucas, these two patches look good for me. I'm just waiting on Mike's
ack to forward them to arm-soc. If that doesn't happen today, feel free
to forward them to arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org when it does, with a note requesting
they be applied since I'm away on vacation. It might be useful for them
to know that they're best applied wherever Tegra's for-3.10/clk branch
was merged.

Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

(Tested for regressions; I didn't actually test using the AC'97 clock,
since I don't have the HW)

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Mike Turquette]

Quoting Stephen Warren (2013-04-17 10:29:47)
> On 04/15/2013 01:31 AM, Lucas Stach wrote:
> > The USB clocks are just clock gates, so no need to set a specific clock.
> > In fact trying to set a specific clock is just a NOP if the requested
> > clockrate is the same as those of the parent (clk_m) or will trigger a
> > WARN_ON() if rates don't match up.
> > 
> > As we are not setting a specific rate, nor activating the clocks at
> > init, there is no point in keeping the the usb entries in the clock init
> > table.
> 
> Lucas, these two patches look good for me. I'm just waiting on Mike's
> ack to forward them to arm-soc. If that doesn't happen today, feel free
> to forward them to arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org when it does, with a note requesting
> they be applied since I'm away on vacation. It might be useful for them
> to know that they're best applied wherever Tegra's for-3.10/clk branch
> was merged.
> 
> Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> (Tested for regressions; I didn't actually test using the AC'97 clock,
> since I don't have the HW)

Acked-by: Mike Turquette <mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Olof Johansson]

On Wed, Apr 17, 2013 at 11:29:47AM -0600, Stephen Warren wrote:
> On 04/15/2013 01:31 AM, Lucas Stach wrote:
> > The USB clocks are just clock gates, so no need to set a specific clock.
> > In fact trying to set a specific clock is just a NOP if the requested
> > clockrate is the same as those of the parent (clk_m) or will trigger a
> > WARN_ON() if rates don't match up.
> > 
> > As we are not setting a specific rate, nor activating the clocks at
> > init, there is no point in keeping the the usb entries in the clock init
> > table.
> 
> Lucas, these two patches look good for me. I'm just waiting on Mike's
> ack to forward them to arm-soc. If that doesn't happen today, feel free
> to forward them to arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org when it does, with a note requesting
> they be applied since I'm away on vacation. It might be useful for them
> to know that they're best applied wherever Tegra's for-3.10/clk branch
> was merged.
> 
> Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> (Tested for regressions; I didn't actually test using the AC'97 clock,
> since I don't have the HW)

Ok, I take back my earlier agreement to apply patches that I just get
a fresh acked-by cc for. This doesn't seem to scale very well since I
have to go from mutt in a terminal back to gmail, find the patch, make
sure I have the right version of that patch, add the thread to my inbox,
run offlineimap, wait for it to show up in mutt and _then_ "git am" it,
adding any acked-by or other tags.

So, if you want me to apply patches from email, please do so by
(re)sending me the patch directly instead.


-Olof

Re: [PATCH resend 1/2] clk: tegra: remove USB from clk init table

[Stephen Warren]

On 04/17/2013 11:29 AM, Stephen Warren wrote:
> On 04/15/2013 01:31 AM, Lucas Stach wrote:
>> The USB clocks are just clock gates, so no need to set a specific clock.
>> In fact trying to set a specific clock is just a NOP if the requested
>> clockrate is the same as those of the parent (clk_m) or will trigger a
>> WARN_ON() if rates don't match up.
>>
>> As we are not setting a specific rate, nor activating the clocks at
>> init, there is no point in keeping the the usb entries in the clock init
>> table.
> 
> Lucas, these two patches look good for me. I'm just waiting on Mike's
> ack to forward them to arm-soc. If that doesn't happen today, feel free
> to forward them to arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org when it does, with a note requesting
> they be applied since I'm away on vacation. ...

It looks like this didn't happen, so I'll forward the patches today.