| Message ID | 20130607121901.21868.65416.stgit@dusk.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
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
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
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
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
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
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
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.
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;
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(+)