On Thu, Jan 08, 2015 at 03:22:08PM +0200, Mikko Perttunen wrote:
> From: Paul Walmsley <[email protected]>
>
> 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 <[email protected]> for identifying this and
> saving hours of debugging time.
>
Should this be done as a reset driver?
> Signed-off-by: Paul Walmsley <[email protected]>
> [ttynkkynen: ported to tegra124 from tegra114]
> Signed-off-by: Tuomas Tynkkynen <[email protected]>
> Signed-off-by: Mikko Perttunen <[email protected]>
> ---
> drivers/clk/tegra/clk-tegra124.c | 47 ++++++++++++++++++++++++++++++++++++++++
> drivers/clk/tegra/clk.h | 3 +++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
> index f5f9bac..623b77f 100644
> --- a/drivers/clk/tegra/clk-tegra124.c
> +++ b/drivers/clk/tegra/clk-tegra124.c
> @@ -31,6 +31,9 @@
> #define CLK_SOURCE_CSITE 0x1d4
> #define CLK_SOURCE_EMC 0x19c
>
> +#define RST_DFLL_DVCO 0x2f4
> +#define DVFS_DFLL_RESET_SHIFT 0
> +
> #define PLLC_BASE 0x80
> #define PLLC_OUT 0x84
> #define PLLC_MISC2 0x88
> @@ -1399,6 +1402,50 @@ static void __init tegra124_clock_apply_init_table(void)
> tegra_init_from_table(init_table, clks, TEGRA124_CLK_CLK_MAX);
> }
>
> +/**
> + * tegra124_car_barrier - wait for pending writes to the CAR to complete
> + *
> + * Wait for any outstanding writes to the CAR MMIO space from this CPU
> + * to complete before continuing execution. No return value.
> + */
> +static void tegra124_car_barrier(void)
> +{
> + readl_relaxed(clk_base + RST_DFLL_DVCO);
> +}
> +
> +/**
> + * tegra124_clock_assert_dfll_dvco_reset - assert the DFLL's DVCO reset
> + *
> + * Assert the reset line of the DFLL's DVCO. No return value.
> + */
> +void tegra124_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);
> + tegra124_car_barrier();
> +}
> +EXPORT_SYMBOL(tegra124_clock_assert_dfll_dvco_reset);
> +
> +/**
> + * tegra124_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 tegra124_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);
> + tegra124_car_barrier();
> +}
> +EXPORT_SYMBOL(tegra124_clock_deassert_dfll_dvco_reset);
> +
> static void __init tegra124_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 4e458aa..def0ea4 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -629,6 +629,9 @@ void tegra114_clock_tune_cpu_trimmers_init(void);
> void tegra114_clock_assert_dfll_dvco_reset(void);
> void tegra114_clock_deassert_dfll_dvco_reset(void);
>
> +void tegra124_clock_assert_dfll_dvco_reset(void);
> +void tegra124_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;
>
> --
> 2.2.1
>
On 02/12/2015 04:19 PM, Peter De Schrijver wrote:
> On Thu, Jan 08, 2015 at 03:22:08PM +0200, Mikko Perttunen wrote:
>> From: Paul Walmsley <[email protected]>
>>
>> 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 <[email protected]> for identifying this and
>> saving hours of debugging time.
>>
>
> Should this be done as a reset driver?
Probably through the already existing CAR reset driver. This reset
doesn't fit well with the existing numbering scheme there, though.
Perhaps a magic high-valued constant that represents it.
>
>> Signed-off-by: Paul Walmsley <[email protected]>
>> [ttynkkynen: ported to tegra124 from tegra114]
>> Signed-off-by: Tuomas Tynkkynen <[email protected]>
>> Signed-off-by: Mikko Perttunen <[email protected]>
>> ---
>> drivers/clk/tegra/clk-tegra124.c | 47 ++++++++++++++++++++++++++++++++++++++++
>> drivers/clk/tegra/clk.h | 3 +++
>> 2 files changed, 50 insertions(+)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
>> index f5f9bac..623b77f 100644
>> --- a/drivers/clk/tegra/clk-tegra124.c
>> +++ b/drivers/clk/tegra/clk-tegra124.c
>> @@ -31,6 +31,9 @@
>> #define CLK_SOURCE_CSITE 0x1d4
>> #define CLK_SOURCE_EMC 0x19c
>>
>> +#define RST_DFLL_DVCO 0x2f4
>> +#define DVFS_DFLL_RESET_SHIFT 0
>> +
>> #define PLLC_BASE 0x80
>> #define PLLC_OUT 0x84
>> #define PLLC_MISC2 0x88
>> @@ -1399,6 +1402,50 @@ static void __init tegra124_clock_apply_init_table(void)
>> tegra_init_from_table(init_table, clks, TEGRA124_CLK_CLK_MAX);
>> }
>>
>> +/**
>> + * tegra124_car_barrier - wait for pending writes to the CAR to complete
>> + *
>> + * Wait for any outstanding writes to the CAR MMIO space from this CPU
>> + * to complete before continuing execution. No return value.
>> + */
>> +static void tegra124_car_barrier(void)
>> +{
>> + readl_relaxed(clk_base + RST_DFLL_DVCO);
>> +}
>> +
>> +/**
>> + * tegra124_clock_assert_dfll_dvco_reset - assert the DFLL's DVCO reset
>> + *
>> + * Assert the reset line of the DFLL's DVCO. No return value.
>> + */
>> +void tegra124_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);
>> + tegra124_car_barrier();
>> +}
>> +EXPORT_SYMBOL(tegra124_clock_assert_dfll_dvco_reset);
>> +
>> +/**
>> + * tegra124_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 tegra124_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);
>> + tegra124_car_barrier();
>> +}
>> +EXPORT_SYMBOL(tegra124_clock_deassert_dfll_dvco_reset);
>> +
>> static void __init tegra124_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 4e458aa..def0ea4 100644
>> --- a/drivers/clk/tegra/clk.h
>> +++ b/drivers/clk/tegra/clk.h
>> @@ -629,6 +629,9 @@ void tegra114_clock_tune_cpu_trimmers_init(void);
>> void tegra114_clock_assert_dfll_dvco_reset(void);
>> void tegra114_clock_deassert_dfll_dvco_reset(void);
>>
>> +void tegra124_clock_assert_dfll_dvco_reset(void);
>> +void tegra124_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;
>>
>> --
>> 2.2.1
>>
On Fri, Feb 13, 2015 at 12:39:03PM +0200, Mikko Perttunen wrote:
> On 02/12/2015 04:19 PM, Peter De Schrijver wrote:
> >On Thu, Jan 08, 2015 at 03:22:08PM +0200, Mikko Perttunen wrote:
> >>From: Paul Walmsley <[email protected]>
> >>
> >>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 <[email protected]> for identifying this and
> >>saving hours of debugging time.
> >>
> >
> >Should this be done as a reset driver?
>
> Probably through the already existing CAR reset driver. This reset
> doesn't fit well with the existing numbering scheme there, though.
> Perhaps a magic high-valued constant that represents it.
>
Indeed. Just like only the lower part of the clock IDs map have a realtion
with the hardware registers. The rest is just arbitrary numbers.
Cheers,
Peter.