diff mbox

[3/3] clk: tegra: T114: add DFLL DVCO reset control

Message ID 20130607121901.21868.65416.stgit@dusk.lan
State Not Applicable, archived
Headers show

Commit Message

Paul Walmsley June 7, 2013, 12:19 p.m. UTC 
Add DFLL DVCO reset line control functions to the CAR IP block driver.

The DVCO present in the DFLL IP block has a separate reset line,
exposed via the CAR IP block.  This reset line is asserted upon SoC
reset.  Unless something (such as the DFLL driver) deasserts this
line, the DVCO will not oscillate, although reads and writes to the
DFLL IP block will complete.

Thanks to Aleksandr Frid <afrid@nvidia.com> for identifying this and
saving hours of debugging time.

Signed-off-by: Paul Walmsley <pwalmsley@nvidia.com>
Cc: Aleksandr Frid <afrid@nvidia.com>
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/tegra/clk-tegra114.c |   37 +++++++++++++++++++++++++++++++++++++
 drivers/clk/tegra/clk.h          |    2 ++
 2 files changed, 39 insertions(+)



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

Comments

Stephen Warren June 7, 2013, 4:57 p.m. UTC | #1 
On 06/07/2013 06:19 AM, Paul Walmsley wrote:
> Add DFLL DVCO reset line control functions to the CAR IP block driver.
> 
> The DVCO present in the DFLL IP block has a separate reset line,
> exposed via the CAR IP block.  This reset line is asserted upon SoC
> reset.  Unless something (such as the DFLL driver) deasserts this
> line, the DVCO will not oscillate, although reads and writes to the
> DFLL IP block will complete.
> 
> Thanks to Aleksandr Frid <afrid@nvidia.com> for identifying this and
> saving hours of debugging time.

> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h

>  void tegra114_clock_tune_cpu_trimmers_high(void);
>  void tegra114_clock_tune_cpu_trimmers_low(void);
>  void tegra114_clock_tune_cpu_trimmers_init(void);
> +void tegra114_clock_assert_dfll_dvco_reset(void);
> +void tegra114_clock_deassert_dfll_dvco_reset(void);

Where/what is the code that will call these new APIs? If it's going to
be something in drivers/clk, that seems fine. If not, then this seems to
be inventing a bunch of new custom APIs exported by the clock driver.
I'm not sure if that's a good idea. (Although I guess that
include/linux/clk/tegra.h has a bunch of other custom APIs to support
CPU hotplug and related functionality, so perhaps it's not a big deael).

The reset assert/de-assert functions at least might be worth exposing
using the new generic module reset API. I believe Prashant Gaikwad is
working on converting the Tegra clock driver to be a module reset
provider, hence removing the existing custom
tegra_periph_reset_{de,}assert() APIs.
--
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
Paul Walmsley June 7, 2013, 5:06 p.m. UTC | #2 
Hi Stephen,

On Fri, 7 Jun 2013, Stephen Warren wrote:

> On 06/07/2013 06:19 AM, Paul Walmsley wrote:
> > Add DFLL DVCO reset line control functions to the CAR IP block driver.
> > 
> > The DVCO present in the DFLL IP block has a separate reset line,
> > exposed via the CAR IP block.  This reset line is asserted upon SoC
> > reset.  Unless something (such as the DFLL driver) deasserts this
> > line, the DVCO will not oscillate, although reads and writes to the
> > DFLL IP block will complete.
> > 
> > Thanks to Aleksandr Frid <afrid@nvidia.com> for identifying this and
> > saving hours of debugging time.
> 
> > diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> 
> >  void tegra114_clock_tune_cpu_trimmers_high(void);
> >  void tegra114_clock_tune_cpu_trimmers_low(void);
> >  void tegra114_clock_tune_cpu_trimmers_init(void);
> > +void tegra114_clock_assert_dfll_dvco_reset(void);
> > +void tegra114_clock_deassert_dfll_dvco_reset(void);
> 
> Where/what is the code that will call these new APIs? If it's going to
> be something in drivers/clk, that seems fine.

That's correct - they'll be used by the DFLL clocksource code, which will 
live in drivers/clk/tegra.  You've seen the patches already ;-)

> The reset assert/de-assert functions at least might be worth exposing
> using the new generic module reset API. I believe Prashant Gaikwad is
> working on converting the Tegra clock driver to be a module reset
> provider, hence removing the existing custom
> tegra_periph_reset_{de,}assert() APIs.

OK, will take a look to see if this can be done without getting in the way 
of Prashant's work.  I'd naïvely assume that it might be best to convert 
these as part of his series - that way we won't duplicate effort.

Prashant, what stage are you at in the conversion?  If you're close to 
completion, maybe we can just add this functionality in with your patches?


- Paul
Prashant Gaikwad June 11, 2013, 7:31 a.m. UTC | #3 
On Friday 07 June 2013 10:36 PM, Paul Walmsley wrote:
> Hi Stephen,
>
> On Fri, 7 Jun 2013, Stephen Warren wrote:
>
>> On 06/07/2013 06:19 AM, Paul Walmsley wrote:
>>> Add DFLL DVCO reset line control functions to the CAR IP block driver.
>>>
>>> The DVCO present in the DFLL IP block has a separate reset line,
>>> exposed via the CAR IP block.  This reset line is asserted upon SoC
>>> reset.  Unless something (such as the DFLL driver) deasserts this
>>> line, the DVCO will not oscillate, although reads and writes to the
>>> DFLL IP block will complete.
>>>
>>> Thanks to Aleksandr Frid <afrid@nvidia.com> for identifying this and
>>> saving hours of debugging time.
>>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>>>   void tegra114_clock_tune_cpu_trimmers_high(void);
>>>   void tegra114_clock_tune_cpu_trimmers_low(void);
>>>   void tegra114_clock_tune_cpu_trimmers_init(void);
>>> +void tegra114_clock_assert_dfll_dvco_reset(void);
>>> +void tegra114_clock_deassert_dfll_dvco_reset(void);
>> Where/what is the code that will call these new APIs? If it's going to
>> be something in drivers/clk, that seems fine.
> That's correct - they'll be used by the DFLL clocksource code, which will
> live in drivers/clk/tegra.  You've seen the patches already ;-)

Why not implement these APIs in DFLL clock driver itself and pass RST 
address register to driver?

>> The reset assert/de-assert functions at least might be worth exposing
>> using the new generic module reset API. I believe Prashant Gaikwad is
>> working on converting the Tegra clock driver to be a module reset
>> provider, hence removing the existing custom
>> tegra_periph_reset_{de,}assert() APIs.
> OK, will take a look to see if this can be done without getting in the way
> of Prashant's work.  I'd naïvely assume that it might be best to convert
> these as part of his series - that way we won't duplicate effort.
>
> Prashant, what stage are you at in the conversion?  If you're close to
> completion, maybe we can just add this functionality in with your patches?
>

You can continue with this patch. I do not see any need to add this 
reset control to generic reset module.

> - Paul

--
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
Paul Walmsley June 11, 2013, 9:47 a.m. UTC | #4 
On Tue, 11 Jun 2013, Prashant Gaikwad wrote:

> Why not implement these APIs in DFLL clock driver itself and pass RST address
> register to driver?

The DFLL DVCO reset registers are CAR registers, not DFLL registers.  
Functions that operate on registers in one IP block shouldn't be located 
in another IP block's driver.


- Paul
--
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
Mike Turquette June 16, 2013, 4:22 a.m. UTC | #5 
Quoting Paul Walmsley (2013-06-11 02:47:13)
> On Tue, 11 Jun 2013, Prashant Gaikwad wrote:
> 
> > Why not implement these APIs in DFLL clock driver itself and pass RST address
> > register to driver?
> 
> The DFLL DVCO reset registers are CAR registers, not DFLL registers.  
> Functions that operate on registers in one IP block shouldn't be located 
> in another IP block's driver.

Paul & Co.,

These patches appear fine to me but I did not see any Acks, nor could I
tell if a v2 was necessary based on the comments. Will there be another
version? If not an Acked-by or Reviewed-by would be cool.

Regards,
Mike

> 
> 
> - Paul
--
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
Paul Walmsley June 17, 2013, 8:22 p.m. UTC | #6 
Hi,

On Sat, 15 Jun 2013, Mike Turquette wrote:

> These patches appear fine to me but I did not see any Acks, nor could I
> tell if a v2 was necessary based on the comments. Will there be another
> version?

I'm not planning to do another version at this time.

> If not an Acked-by or Reviewed-by would be cool.

Hmm, not sure who the right people would be.  Looks like the 
drivers/clk/tegra directory is unmaintained, so not sure who could ack it?

Peter or Stephen, care to review these patches and send a Reviewed-by?


- Paul
--
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
Mike Turquette June 18, 2013, 6:28 p.m. UTC | #7 
Quoting Paul Walmsley (2013-06-17 13:22:48)
> Hi,
> 
> On Sat, 15 Jun 2013, Mike Turquette wrote:
> 
> > These patches appear fine to me but I did not see any Acks, nor could I
> > tell if a v2 was necessary based on the comments. Will there be another
> > version?
> 
> I'm not planning to do another version at this time.

Thanks for clarifying.  I couldn't tell based on the discussion.

Anyways I've taken these into clk-next now.  There is not maintainer for
that directory as you pointed out, so no need to wait for Acks.

Regards,
Mike

> 
> > If not an Acked-by or Reviewed-by would be cool.
> 
> Hmm, not sure who the right people would be.  Looks like the 
> drivers/clk/tegra directory is unmaintained, so not sure who could ack it?
> 
> Peter or Stephen, care to review these patches and send a Reviewed-by?
> 
> 
> - Paul
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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
Paul Walmsley June 18, 2013, 6:33 p.m. UTC | #8 
On 06/18/2013 11:28 AM, Mike Turquette wrote:
> Thanks for clarifying. I couldn't tell based on the discussion.

No worries :-)

> Anyways I've taken these into clk-next now. There is not maintainer 
> for that directory as you pointed out, so no need to wait for Acks.

Great, thanks.  Or if you'd prefer to wait for a Reviewed-by:, Peter 
mentioned today that he might take a look at them.  Either way is fine 
with me.


- Paul
--
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
Peter De Schrijver June 19, 2013, 8:45 a.m. UTC | #9 
On Fri, Jun 07, 2013 at 02:19:09PM +0200, Paul Walmsley wrote:
> Add DFLL DVCO reset line control functions to the CAR IP block driver.
> 
> The DVCO present in the DFLL IP block has a separate reset line,
> exposed via the CAR IP block.  This reset line is asserted upon SoC
> reset.  Unless something (such as the DFLL driver) deasserts this
> line, the DVCO will not oscillate, although reads and writes to the
> DFLL IP block will complete.
> 
> Thanks to Aleksandr Frid <afrid@nvidia.com> for identifying this and
> saving hours of debugging time.
> 
> Signed-off-by: Paul Walmsley <pwalmsley@nvidia.com>
> Cc: Aleksandr Frid <afrid@nvidia.com>
> Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Reviewed-by: <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
diff mbox

Patch

diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index b35c78d..4ab15e3 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -29,6 +29,7 @@ 
 #define RST_DEVICES_L			0x004
 #define RST_DEVICES_H			0x008
 #define RST_DEVICES_U			0x00C
+#define RST_DFLL_DVCO			0x2F4
 #define RST_DEVICES_V			0x358
 #define RST_DEVICES_W			0x35C
 #define RST_DEVICES_X			0x28C
@@ -47,6 +48,9 @@ 
 #define CPU_FINETRIM_R			0x4e4	/* rise->rise prop dly inc A */
 #define RST_DEVICES_NUM			5
 
+/* RST_DFLL_DVCO bitfields */
+#define DVFS_DFLL_RESET_SHIFT		0
+
 /* CPU_FINETRIM_SELECT and CPU_FINETRIM_DR bitfields */
 #define CPU_FINETRIM_1_FCPU_1		BIT(0)	/* fcpu0 */
 #define CPU_FINETRIM_1_FCPU_2		BIT(1)	/* fcpu1 */
@@ -2183,6 +2187,39 @@  void tegra114_clock_tune_cpu_trimmers_init(void)
 }
 EXPORT_SYMBOL(tegra114_clock_tune_cpu_trimmers_init);
 
+/**
+ * tegra114_clock_assert_dfll_dvco_reset - assert the DFLL's DVCO reset
+ *
+ * Assert the reset line of the DFLL's DVCO.  No return value.
+ */
+void tegra114_clock_assert_dfll_dvco_reset(void)
+{
+	u32 v;
+
+	v = readl_relaxed(clk_base + RST_DFLL_DVCO);
+	v |= (1 << DVFS_DFLL_RESET_SHIFT);
+	writel_relaxed(v, clk_base + RST_DFLL_DVCO);
+	tegra114_car_barrier();
+}
+EXPORT_SYMBOL(tegra114_clock_assert_dfll_dvco_reset);
+
+/**
+ * tegra114_clock_deassert_dfll_dvco_reset - deassert the DFLL's DVCO reset
+ *
+ * Deassert the reset line of the DFLL's DVCO, allowing the DVCO to
+ * operate.  No return value.
+ */
+void tegra114_clock_deassert_dfll_dvco_reset(void)
+{
+	u32 v;
+
+	v = readl_relaxed(clk_base + RST_DFLL_DVCO);
+	v &= ~(1 << DVFS_DFLL_RESET_SHIFT);
+	writel_relaxed(v, clk_base + RST_DFLL_DVCO);
+	tegra114_car_barrier();
+}
+EXPORT_SYMBOL(tegra114_clock_deassert_dfll_dvco_reset);
+
 static void __init tegra114_clock_init(struct device_node *np)
 {
 	struct device_node *node;
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index c0b72fc..c2d84a1 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -574,6 +574,8 @@  void tegra_init_dup_clks(struct tegra_clk_duplicate *dup_list,
 void tegra114_clock_tune_cpu_trimmers_high(void);
 void tegra114_clock_tune_cpu_trimmers_low(void);
 void tegra114_clock_tune_cpu_trimmers_init(void);
+void tegra114_clock_assert_dfll_dvco_reset(void);
+void tegra114_clock_deassert_dfll_dvco_reset(void);
 
 typedef void (*tegra_clk_apply_init_table_func)(void);
 extern tegra_clk_apply_init_table_func tegra_clk_apply_init_table;