| Message ID | 1366011105-2351-1-git-send-email-dev@lynxeye.de |
|---|---|
| State | Accepted, archived |
| Headers | show |
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@nvidia.com> Cheers, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@lynxeye.de> > --- > Trace produced by system with 13MHz clk_m: Reviewed-by: Prashant Gaikwad <pgaikwad@nvidia.com> > 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}, -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@kernel.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@nvidia.com> Tested-by: Stephen Warren <swarren@nvidia.com> (Tested for regressions; I didn't actually test using the AC'97 clock, since I don't have the HW) -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@kernel.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@nvidia.com> > Tested-by: Stephen Warren <swarren@nvidia.com> > > (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@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@kernel.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@nvidia.com> > Tested-by: Stephen Warren <swarren@nvidia.com> > > (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 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@kernel.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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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},
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@lynxeye.de> --- 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(-)