[PATCH 1/2] clk: tegra: Export peripheral reset functions

[PATCH 1/2] clk: tegra: Export peripheral reset functions

[Thierry Reding]

The tegra_periph_reset_assert() and tegra_periph_reset_deassert()
functions can be used by drivers to reset peripherals. In order to allow
such drivers to be built as modules, export the functions.

Note that this restores the status quo as the functions were exported
before the move to the drivers/clk tree.

Signed-off-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
---
 drivers/clk/tegra/clk-periph.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/tegra/clk-periph.c b/drivers/clk/tegra/clk-periph.c
index 788486e..9dbd301 100644
--- a/drivers/clk/tegra/clk-periph.c
+++ b/drivers/clk/tegra/clk-periph.c
@@ -16,6 +16,7 @@
 
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 
@@ -128,6 +129,7 @@ void tegra_periph_reset_deassert(struct clk *c)
 
 	tegra_periph_reset(gate, 0);
 }
+EXPORT_SYMBOL(tegra_periph_reset_deassert);
 
 void tegra_periph_reset_assert(struct clk *c)
 {
@@ -147,6 +149,7 @@ void tegra_periph_reset_assert(struct clk *c)
 
 	tegra_periph_reset(gate, 1);
 }
+EXPORT_SYMBOL(tegra_periph_reset_assert);
 
 const struct clk_ops tegra_clk_periph_ops = {
 	.get_parent = clk_periph_get_parent,
-- 
1.8.1.5

[PATCH 2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

[Thierry Reding]

By default these clocks are children of pll_m, but in downstream kernels
they are reparented to pll_c. While at it, decrease their frequencies to
300 MHz because the defaults aren't in the specified range.

gr2d can reportedly run at much higher frequencies, but 300 MHz works
and is a more conservative default.

Signed-off-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
---
 drivers/clk/tegra/clk-tegra20.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index bf19400..b020beb 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1247,6 +1247,8 @@ static __initdata struct tegra_clk_init_table init_table[] = {
 	{host1x, pll_c, 150000000, 0},
 	{disp1, pll_p, 600000000, 0},
 	{disp2, pll_p, 600000000, 0},
+	{gr2d, pll_c, 300000000, 0},
+	{gr3d, pll_c, 300000000, 0},
 	{clk_max, clk_max, 0, 0}, /* This MUST be the last entry */
 };
 
-- 
1.8.1.5

Re: [PATCH 2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

[Stephen Warren]

On 03/28/2013 02:31 PM, Thierry Reding wrote:
> By default these clocks are children of pll_m, but in downstream kernels
> they are reparented to pll_c. While at it, decrease their frequencies to
> 300 MHz because the defaults aren't in the specified range.
> 
> gr2d can reportedly run at much higher frequencies, but 300 MHz works
> and is a more conservative default.

Questions on this patch:

Do we need to do the same thing for Tegra30 and/or Tegra114?

Is 300MHz the right value?

I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.

Re: [PATCH 2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

[Mark Zhang]

On 03/30/2013 05:46 AM, Stephen Warren wrote:
> On 03/28/2013 02:31 PM, Thierry Reding wrote:
>> By default these clocks are children of pll_m, but in downstream kernels
>> they are reparented to pll_c. While at it, decrease their frequencies to
>> 300 MHz because the defaults aren't in the specified range.
>>
>> gr2d can reportedly run at much higher frequencies, but 300 MHz works
>> and is a more conservative default.
> 
> Questions on this patch:
> 
> Do we need to do the same thing for Tegra30 and/or Tegra114?
> 

I think yes. For Tegra114, it's pll_c2.

> Is 300MHz the right value?
> 

Right. I use 300MHz on Tegra114 and it seems OK. I recall sometimes
276MHz can be used as well.

Mark
> I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

[Terje Bergström]

On 29.03.2013 23:46, Stephen Warren wrote:
> On 03/28/2013 02:31 PM, Thierry Reding wrote:
>> By default these clocks are children of pll_m, but in downstream kernels
>> they are reparented to pll_c. While at it, decrease their frequencies to
>> 300 MHz because the defaults aren't in the specified range.
>>
>> gr2d can reportedly run at much higher frequencies, but 300 MHz works
>> and is a more conservative default.
> 
> Questions on this patch:
> 
> Do we need to do the same thing for Tegra30 and/or Tegra114?
> 
> Is 300MHz the right value?
> 
> I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.

We need a patch for all SoC's. 2D can fail subtly with the too high
clocks, even though most of the time it seems to be doing just fine.

In Tegra20, 300MHz is the max rate we drive 2D in. Later Tegras have a
higher max clock, and in Tegra114 we drive it from a different PLL. But
for all of them 300MHz and PLLC should be a working configuration. I
haven't checked how we use PLLC in upstream, so I'm not 100% sure.

In downstream, the nvhost driver is responsible for setting a sane value
to the host1x clients. In upstream host1x, we missed that because we
didn't include the clock management code.

Terje

Re: [PATCH 2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]

On Tue, Apr 02, 2013 at 08:28:53AM +0300, Terje Bergström wrote:
> On 29.03.2013 23:46, Stephen Warren wrote:
> > On 03/28/2013 02:31 PM, Thierry Reding wrote:
> >> By default these clocks are children of pll_m, but in downstream kernels
> >> they are reparented to pll_c. While at it, decrease their frequencies to
> >> 300 MHz because the defaults aren't in the specified range.
> >>
> >> gr2d can reportedly run at much higher frequencies, but 300 MHz works
> >> and is a more conservative default.
> > 
> > Questions on this patch:
> > 
> > Do we need to do the same thing for Tegra30 and/or Tegra114?
> > 
> > Is 300MHz the right value?
> > 
> > I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.
> 
> We need a patch for all SoC's.

I think we also need that for all host1x children as well. But I think
we can tackle those at the same time as driver support is added. Right
now only 2D and 3D have some form of user code.

> 2D can fail subtly with the too high clocks, even though most of the
> time it seems to be doing just fine.
> 
> In Tegra20, 300MHz is the max rate we drive 2D in. Later Tegras have a
> higher max clock, and in Tegra114 we drive it from a different PLL. But
> for all of them 300MHz and PLLC should be a working configuration. I
> haven't checked how we use PLLC in upstream, so I'm not 100% sure.

At least on Tegra30 (CardHu) I can see that the clocks are also children
of PLLC. I can add a similar hunk for Tegra30 to the patch. I have no
Tegra114 hardware available and none of the downstream kernel branches I
have seem to include Tegra114 support either so I can't check what's in
use there, but if you say it should be fine I can include that in the
patch as well.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

[Terje Bergström]

On 02.04.2013 08:47, Thierry Reding wrote:
> On Tue, Apr 02, 2013 at 08:28:53AM +0300, Terje Bergström wrote:
>> In Tegra20, 300MHz is the max rate we drive 2D in. Later Tegras have a
>> higher max clock, and in Tegra114 we drive it from a different PLL. But
>> for all of them 300MHz and PLLC should be a working configuration. I
>> haven't checked how we use PLLC in upstream, so I'm not 100% sure.
> 
> At least on Tegra30 (CardHu) I can see that the clocks are also children
> of PLLC. I can add a similar hunk for Tegra30 to the patch. I have no

Yes, Tegra30 is similar to Tegra20 in this respect. Only max clock is
different, so similar hunk for Tegra30 would be a good idea.

> Tegra114 hardware available and none of the downstream kernel branches I
> have seem to include Tegra114 support either so I can't check what's in
> use there, but if you say it should be fine I can include that in the
> patch as well.

I think it's better we add this when we have a patch that enables host1x
driver for Tegra114 and we have tested. Until then, I suggest we just
leave the Tegra114 part out.

Terje

Re: [PATCH 2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

[Peter De Schrijver]

On Thu, Mar 28, 2013 at 09:31:28PM +0100, Thierry Reding wrote:
> By default these clocks are children of pll_m, but in downstream kernels
> they are reparented to pll_c. While at it, decrease their frequencies to
> 300 MHz because the defaults aren't in the specified range.
> 
> gr2d can reportedly run at much higher frequencies, but 300 MHz works
> and is a more conservative default.
> 
> Signed-off-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> ---
>  drivers/clk/tegra/clk-tegra20.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index bf19400..b020beb 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -1247,6 +1247,8 @@ static __initdata struct tegra_clk_init_table init_table[] = {
>  	{host1x, pll_c, 150000000, 0},
>  	{disp1, pll_p, 600000000, 0},
>  	{disp2, pll_p, 600000000, 0},
> +	{gr2d, pll_c, 300000000, 0},
> +	{gr3d, pll_c, 300000000, 0},
>  	{clk_max, clk_max, 0, 0}, /* This MUST be the last entry */
>  };
>  

In the end we should move to a more flexible scheme where the pll_c frequency
is determined by the needs of the various users, but for now this should do I
think.

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

Cheers,

Peter.

Re: [PATCH 1/2] clk: tegra: Export peripheral reset functions

[Stephen Warren]

On 03/28/2013 02:31 PM, Thierry Reding wrote:
> The tegra_periph_reset_assert() and tegra_periph_reset_deassert()
> functions can be used by drivers to reset peripherals. In order to allow
> such drivers to be built as modules, export the functions.
> 
> Note that this restores the status quo as the functions were exported
> before the move to the drivers/clk tree.

Hopefully soon we can replace these with the new module reset API that's
being cooked up. But this should do for now.

Peter, Prashant, can you please review this series too?

Re: [PATCH 1/2] clk: tegra: Export peripheral reset functions

[Stephen Warren]

On 03/28/2013 02:31 PM, Thierry Reding wrote:
> The tegra_periph_reset_assert() and tegra_periph_reset_deassert()
> functions can be used by drivers to reset peripherals. In order to allow
> such drivers to be built as modules, export the functions.
> 
> Note that this restores the status quo as the functions were exported
> before the move to the drivers/clk tree.

Mike,

Since I'm taking the Tegra CCF changes through the Tegra tree for this
kernel cycle, do you want me to wait for your ack on these small fixes,
or just go ahead and apply them?

Similarly, I assume you want to review/ack the Tegra114 clock driver
that Peter posted? If not, do let me know, since I'm currently waiting
for that. (Although I know Peter will post a V9 with minor fixes, but
99% of any review should still be applicable)

Thanks.

Re: [PATCH 1/2] clk: tegra: Export peripheral reset functions

[Stephen Warren]

On 03/28/2013 02:31 PM, Thierry Reding wrote:
> The tegra_periph_reset_assert() and tegra_periph_reset_deassert()
> functions can be used by drivers to reset peripherals. In order to allow
> such drivers to be built as modules, export the functions.
> 
> Note that this restores the status quo as the functions were exported
> before the move to the drivers/clk tree.

I have applied this one patch to Tegra's for-3.10/clk branch, with
Mike's ack.

Re: [PATCH 1/2] clk: tegra: Export peripheral reset functions

[Peter De Schrijver]

On Thu, Mar 28, 2013 at 09:31:27PM +0100, Thierry Reding wrote:
> The tegra_periph_reset_assert() and tegra_periph_reset_deassert()
> functions can be used by drivers to reset peripherals. In order to allow
> such drivers to be built as modules, export the functions.
> 
> Note that this restores the status quo as the functions were exported
> before the move to the drivers/clk tree.
> 

I guess we should move to the 'reset framework' at some point in the future?

Otherwise:

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


Cheers,

Peter.

Re: [PATCH 1/2] clk: tegra: Export peripheral reset functions

[Mike Turquette]

Quoting Peter De Schrijver (2013-04-02 02:34:26)
> On Thu, Mar 28, 2013 at 09:31:27PM +0100, Thierry Reding wrote:
> > The tegra_periph_reset_assert() and tegra_periph_reset_deassert()
> > functions can be used by drivers to reset peripherals. In order to allow
> > such drivers to be built as modules, export the functions.
> > 
> > Note that this restores the status quo as the functions were exported
> > before the move to the drivers/clk tree.
> > 
> 
> I guess we should move to the 'reset framework' at some point in the future?
> 

Yes please do.  Peripheral reset stuff should not lurk in the clock
code.  I kind of brought this point up back in June of 2012:
http://article.gmane.org/gmane.linux.ports.tegra/5346/

Back then the discussion was around removing this export from your clock
driver and putting common reset code in clk.h (which is a bad idea).
Now that the reset framework exists this stuff really should be migrated
out of the clock framework entirely.

But as long as this patch is considered an incremental change then it's
OK by me.

Regards,
Mike

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