diff mbox series

[V6,09/21] clk: tegra: clk-super: Fix to enable PLLP branches to CPU

Message ID 1563738060-30213-10-git-send-email-skomatineni@nvidia.com
State New
Headers show
Series SC7 entry and exit support for Tegra210 | expand

Commit Message

Sowjanya Komatineni July 21, 2019, 7:40 p.m. UTC
This patch has a fix to enable PLLP branches to CPU before changing
the CPU clusters clock source to PLLP for Gen5 Super clock.

During system suspend entry and exit, CPU source will be switched
to PLLP and this needs PLLP branches to be enabled to CPU prior to
the switch.

On system resume, warmboot code enables PLLP branches to CPU and
powers up the CPU with PLLP clock source.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/clk/tegra/clk-super.c            | 11 +++++++++++
 drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
 drivers/clk/tegra/clk.h                  |  4 ++++
 3 files changed, 17 insertions(+), 2 deletions(-)

Comments

Dmitry Osipenko July 21, 2019, 9:16 p.m. UTC | #1
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch has a fix to enable PLLP branches to CPU before changing
> the CPU clusters clock source to PLLP for Gen5 Super clock.
> 
> During system suspend entry and exit, CPU source will be switched
> to PLLP and this needs PLLP branches to be enabled to CPU prior to
> the switch.
> 
> On system resume, warmboot code enables PLLP branches to CPU and
> powers up the CPU with PLLP clock source.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>  drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>  drivers/clk/tegra/clk.h                  |  4 ++++
>  3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
> index 39ef31b46df5..d73c587e4853 100644
> --- a/drivers/clk/tegra/clk-super.c
> +++ b/drivers/clk/tegra/clk-super.c
> @@ -28,6 +28,9 @@
>  #define super_state_to_src_shift(m, s) ((m->width * s))
>  #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>  
> +#define CCLK_SRC_PLLP_OUT0 4
> +#define CCLK_SRC_PLLP_OUT4 5
> +
>  static u8 clk_super_get_parent(struct clk_hw *hw)
>  {
>  	struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw *hw, u8 index)
>  		if (index == mux->div2_index)
>  			index = mux->pllx_index;
>  	}
> +
> +	/*
> +	 * Enable PLLP branches to CPU before selecting PLLP source
> +	 */
> +	if ((mux->flags & TEGRA_CPU_CLK) &&
> +	    ((index == CCLK_SRC_PLLP_OUT0) || (index == CCLK_SRC_PLLP_OUT4)))
> +		tegra_clk_set_pllp_out_cpu(true);

Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
switching from PLLP?

>  	val &= ~((super_state_to_src_mask(mux)) << shift);
>  	val |= (index & (super_state_to_src_mask(mux))) << shift;
>  
> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
> index cdfe7c9697e1..cd208d0eca2a 100644
> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
> @@ -180,7 +180,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>  					gen_info->num_cclk_g_parents,
>  					CLK_SET_RATE_PARENT,
>  					clk_base + CCLKG_BURST_POLICY,
> -					0, 4, 8, 0, NULL);
> +					TEGRA_CPU_CLK, 4, 8, 0, NULL);
>  		} else {
>  			clk = tegra_clk_register_super_mux("cclk_g",
>  					gen_info->cclk_g_parents,
> @@ -201,7 +201,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>  					gen_info->num_cclk_lp_parents,
>  					CLK_SET_RATE_PARENT,
>  					clk_base + CCLKLP_BURST_POLICY,
> -					0, 4, 8, 0, NULL);
> +					TEGRA_CPU_CLK, 4, 8, 0, NULL);
>  		} else {
>  			clk = tegra_clk_register_super_mux("cclk_lp",
>  					gen_info->cclk_lp_parents,
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index ac6de3a0b91f..c357b49e49b0 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -694,6 +694,9 @@ struct clk *tegra_clk_register_periph_data(void __iomem *clk_base,
>   * Flags:
>   * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag indicates
>   *     that this is LP cluster clock.
> + * TEGRA_CPU_CLK - This flag indicates this is CPU cluster clock. To use PLLP
> + * for CPU clock source, need to enable PLLP branches to CPU by setting the
> + * additional bit PLLP_OUT_CPU for gen5 super clock.
>   */
>  struct tegra_clk_super_mux {
>  	struct clk_hw	hw;
> @@ -710,6 +713,7 @@ struct tegra_clk_super_mux {
>  #define to_clk_super_mux(_hw) container_of(_hw, struct tegra_clk_super_mux, hw)
>  
>  #define TEGRA_DIVIDER_2 BIT(0)
> +#define TEGRA_CPU_CLK	BIT(1)

I'd name this TEGRA210_CPU_CLK for clarity.

>  extern const struct clk_ops tegra_clk_super_ops;
>  struct clk *tegra_clk_register_super_mux(const char *name,
> 

Will be better to move the tegra_clk_set_pllp_out_cpu() definition into
this patch, otherwise this looks inconsistent for reviewer.
Sowjanya Komatineni July 21, 2019, 10:39 p.m. UTC | #2
On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>> This patch has a fix to enable PLLP branches to CPU before changing
>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>
>> During system suspend entry and exit, CPU source will be switched
>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>> the switch.
>>
>> On system resume, warmboot code enables PLLP branches to CPU and
>> powers up the CPU with PLLP clock source.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>>   drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>   drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>   drivers/clk/tegra/clk.h                  |  4 ++++
>>   3 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
>> index 39ef31b46df5..d73c587e4853 100644
>> --- a/drivers/clk/tegra/clk-super.c
>> +++ b/drivers/clk/tegra/clk-super.c
>> @@ -28,6 +28,9 @@
>>   #define super_state_to_src_shift(m, s) ((m->width * s))
>>   #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>   
>> +#define CCLK_SRC_PLLP_OUT0 4
>> +#define CCLK_SRC_PLLP_OUT4 5
>> +
>>   static u8 clk_super_get_parent(struct clk_hw *hw)
>>   {
>>   	struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw *hw, u8 index)
>>   		if (index == mux->div2_index)
>>   			index = mux->pllx_index;
>>   	}
>> +
>> +	/*
>> +	 * Enable PLLP branches to CPU before selecting PLLP source
>> +	 */
>> +	if ((mux->flags & TEGRA_CPU_CLK) &&
>> +	    ((index == CCLK_SRC_PLLP_OUT0) || (index == CCLK_SRC_PLLP_OUT4)))
>> +		tegra_clk_set_pllp_out_cpu(true);
> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
> switching from PLLP?
PLLP may be used for other CPU clusters.
>>   	val &= ~((super_state_to_src_mask(mux)) << shift);
>>   	val |= (index & (super_state_to_src_mask(mux))) << shift;
>>   
>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> index cdfe7c9697e1..cd208d0eca2a 100644
>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> @@ -180,7 +180,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>>   					gen_info->num_cclk_g_parents,
>>   					CLK_SET_RATE_PARENT,
>>   					clk_base + CCLKG_BURST_POLICY,
>> -					0, 4, 8, 0, NULL);
>> +					TEGRA_CPU_CLK, 4, 8, 0, NULL);
>>   		} else {
>>   			clk = tegra_clk_register_super_mux("cclk_g",
>>   					gen_info->cclk_g_parents,
>> @@ -201,7 +201,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>>   					gen_info->num_cclk_lp_parents,
>>   					CLK_SET_RATE_PARENT,
>>   					clk_base + CCLKLP_BURST_POLICY,
>> -					0, 4, 8, 0, NULL);
>> +					TEGRA_CPU_CLK, 4, 8, 0, NULL);
>>   		} else {
>>   			clk = tegra_clk_register_super_mux("cclk_lp",
>>   					gen_info->cclk_lp_parents,
>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>> index ac6de3a0b91f..c357b49e49b0 100644
>> --- a/drivers/clk/tegra/clk.h
>> +++ b/drivers/clk/tegra/clk.h
>> @@ -694,6 +694,9 @@ struct clk *tegra_clk_register_periph_data(void __iomem *clk_base,
>>    * Flags:
>>    * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag indicates
>>    *     that this is LP cluster clock.
>> + * TEGRA_CPU_CLK - This flag indicates this is CPU cluster clock. To use PLLP
>> + * for CPU clock source, need to enable PLLP branches to CPU by setting the
>> + * additional bit PLLP_OUT_CPU for gen5 super clock.
>>    */
>>   struct tegra_clk_super_mux {
>>   	struct clk_hw	hw;
>> @@ -710,6 +713,7 @@ struct tegra_clk_super_mux {
>>   #define to_clk_super_mux(_hw) container_of(_hw, struct tegra_clk_super_mux, hw)
>>   
>>   #define TEGRA_DIVIDER_2 BIT(0)
>> +#define TEGRA_CPU_CLK	BIT(1)
> I'd name this TEGRA210_CPU_CLK for clarity.
>
>>   extern const struct clk_ops tegra_clk_super_ops;
>>   struct clk *tegra_clk_register_super_mux(const char *name,
>>
> Will be better to move the tegra_clk_set_pllp_out_cpu() definition into
> this patch, otherwise this looks inconsistent for reviewer.
ok, Will move to this patch
Sowjanya Komatineni July 22, 2019, 3:17 a.m. UTC | #3
On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:
>
> On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>> This patch has a fix to enable PLLP branches to CPU before changing
>>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>>
>>> During system suspend entry and exit, CPU source will be switched
>>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>>> the switch.
>>>
>>> On system resume, warmboot code enables PLLP branches to CPU and
>>> powers up the CPU with PLLP clock source.
>>>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>>   drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>>   drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>>   drivers/clk/tegra/clk.h                  |  4 ++++
>>>   3 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/clk/tegra/clk-super.c 
>>> b/drivers/clk/tegra/clk-super.c
>>> index 39ef31b46df5..d73c587e4853 100644
>>> --- a/drivers/clk/tegra/clk-super.c
>>> +++ b/drivers/clk/tegra/clk-super.c
>>> @@ -28,6 +28,9 @@
>>>   #define super_state_to_src_shift(m, s) ((m->width * s))
>>>   #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>>   +#define CCLK_SRC_PLLP_OUT0 4
>>> +#define CCLK_SRC_PLLP_OUT4 5
>>> +
>>>   static u8 clk_super_get_parent(struct clk_hw *hw)
>>>   {
>>>       struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw 
>>> *hw, u8 index)
>>>           if (index == mux->div2_index)
>>>               index = mux->pllx_index;
>>>       }
>>> +
>>> +    /*
>>> +     * Enable PLLP branches to CPU before selecting PLLP source
>>> +     */
>>> +    if ((mux->flags & TEGRA_CPU_CLK) &&
>>> +        ((index == CCLK_SRC_PLLP_OUT0) || (index == 
>>> CCLK_SRC_PLLP_OUT4)))
>>> +        tegra_clk_set_pllp_out_cpu(true);
>> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
>> switching from PLLP?
> PLLP may be used for other CPU clusters.

Though to avoid flag and check needed to make sure other CPU is not 
using before disabling PLLP branch to CPU.

But leaving it enabled shouldn't impact much as clock source mux is 
after this in design anyway.

But can add as well if its clear that way.

>>>       val &= ~((super_state_to_src_mask(mux)) << shift);
>>>       val |= (index & (super_state_to_src_mask(mux))) << shift;
>>>   diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c 
>>> b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> index cdfe7c9697e1..cd208d0eca2a 100644
>>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> @@ -180,7 +180,7 @@ static void __init tegra_super_clk_init(void 
>>> __iomem *clk_base,
>>>                       gen_info->num_cclk_g_parents,
>>>                       CLK_SET_RATE_PARENT,
>>>                       clk_base + CCLKG_BURST_POLICY,
>>> -                    0, 4, 8, 0, NULL);
>>> +                    TEGRA_CPU_CLK, 4, 8, 0, NULL);
>>>           } else {
>>>               clk = tegra_clk_register_super_mux("cclk_g",
>>>                       gen_info->cclk_g_parents,
>>> @@ -201,7 +201,7 @@ static void __init tegra_super_clk_init(void 
>>> __iomem *clk_base,
>>>                       gen_info->num_cclk_lp_parents,
>>>                       CLK_SET_RATE_PARENT,
>>>                       clk_base + CCLKLP_BURST_POLICY,
>>> -                    0, 4, 8, 0, NULL);
>>> +                    TEGRA_CPU_CLK, 4, 8, 0, NULL);
>>>           } else {
>>>               clk = tegra_clk_register_super_mux("cclk_lp",
>>>                       gen_info->cclk_lp_parents,
>>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>>> index ac6de3a0b91f..c357b49e49b0 100644
>>> --- a/drivers/clk/tegra/clk.h
>>> +++ b/drivers/clk/tegra/clk.h
>>> @@ -694,6 +694,9 @@ struct clk *tegra_clk_register_periph_data(void 
>>> __iomem *clk_base,
>>>    * Flags:
>>>    * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag 
>>> indicates
>>>    *     that this is LP cluster clock.
>>> + * TEGRA_CPU_CLK - This flag indicates this is CPU cluster clock. 
>>> To use PLLP
>>> + * for CPU clock source, need to enable PLLP branches to CPU by 
>>> setting the
>>> + * additional bit PLLP_OUT_CPU for gen5 super clock.
>>>    */
>>>   struct tegra_clk_super_mux {
>>>       struct clk_hw    hw;
>>> @@ -710,6 +713,7 @@ struct tegra_clk_super_mux {
>>>   #define to_clk_super_mux(_hw) container_of(_hw, struct 
>>> tegra_clk_super_mux, hw)
>>>     #define TEGRA_DIVIDER_2 BIT(0)
>>> +#define TEGRA_CPU_CLK    BIT(1)
>> I'd name this TEGRA210_CPU_CLK for clarity.
>>
>>>   extern const struct clk_ops tegra_clk_super_ops;
>>>   struct clk *tegra_clk_register_super_mux(const char *name,
>>>
>> Will be better to move the tegra_clk_set_pllp_out_cpu() definition into
>> this patch, otherwise this looks inconsistent for reviewer.
> ok, Will move to this patch
Dmitry Osipenko July 22, 2019, 6:32 a.m. UTC | #4
22.07.2019 6:17, Sowjanya Komatineni пишет:
> 
> On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:
>>
>> On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>> This patch has a fix to enable PLLP branches to CPU before changing
>>>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>>>
>>>> During system suspend entry and exit, CPU source will be switched
>>>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>>>> the switch.
>>>>
>>>> On system resume, warmboot code enables PLLP branches to CPU and
>>>> powers up the CPU with PLLP clock source.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>>   drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>>>   drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>>>   drivers/clk/tegra/clk.h                  |  4 ++++
>>>>   3 files changed, 17 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/tegra/clk-super.c
>>>> b/drivers/clk/tegra/clk-super.c
>>>> index 39ef31b46df5..d73c587e4853 100644
>>>> --- a/drivers/clk/tegra/clk-super.c
>>>> +++ b/drivers/clk/tegra/clk-super.c
>>>> @@ -28,6 +28,9 @@
>>>>   #define super_state_to_src_shift(m, s) ((m->width * s))
>>>>   #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>>>   +#define CCLK_SRC_PLLP_OUT0 4
>>>> +#define CCLK_SRC_PLLP_OUT4 5
>>>> +
>>>>   static u8 clk_super_get_parent(struct clk_hw *hw)
>>>>   {
>>>>       struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>>>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw
>>>> *hw, u8 index)
>>>>           if (index == mux->div2_index)
>>>>               index = mux->pllx_index;
>>>>       }
>>>> +
>>>> +    /*
>>>> +     * Enable PLLP branches to CPU before selecting PLLP source
>>>> +     */
>>>> +    if ((mux->flags & TEGRA_CPU_CLK) &&
>>>> +        ((index == CCLK_SRC_PLLP_OUT0) || (index ==
>>>> CCLK_SRC_PLLP_OUT4)))
>>>> +        tegra_clk_set_pllp_out_cpu(true);
>>> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
>>> switching from PLLP?
>> PLLP may be used for other CPU clusters.
> 
> Though to avoid flag and check needed to make sure other CPU is not
> using before disabling PLLP branch to CPU.
> 
> But leaving it enabled shouldn't impact much as clock source mux is
> after this in design anyway.
> 
> But can add as well if its clear that way.

The TRM doc says "The CPU subsystem supports a switch-cluster mode
meaning that only one of the clusters can be active at any given time".

Given that cluster-switching isn't supported in upstream, I don't think
that you need to care about the other cluster at all, at least for now.

The cluster-switching implementation in upstream is very complicated
because it requires a special "hotplugging" CPU governor, which
apparently no other platform needs.

[snip]
Sowjanya Komatineni July 22, 2019, 7:12 a.m. UTC | #5
On 7/21/19 11:32 PM, Dmitry Osipenko wrote:
> 22.07.2019 6:17, Sowjanya Komatineni пишет:
>> On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:
>>> On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>> This patch has a fix to enable PLLP branches to CPU before changing
>>>>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>>>>
>>>>> During system suspend entry and exit, CPU source will be switched
>>>>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>>>>> the switch.
>>>>>
>>>>> On system resume, warmboot code enables PLLP branches to CPU and
>>>>> powers up the CPU with PLLP clock source.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>>    drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>>>>    drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>>>>    drivers/clk/tegra/clk.h                  |  4 ++++
>>>>>    3 files changed, 17 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-super.c
>>>>> b/drivers/clk/tegra/clk-super.c
>>>>> index 39ef31b46df5..d73c587e4853 100644
>>>>> --- a/drivers/clk/tegra/clk-super.c
>>>>> +++ b/drivers/clk/tegra/clk-super.c
>>>>> @@ -28,6 +28,9 @@
>>>>>    #define super_state_to_src_shift(m, s) ((m->width * s))
>>>>>    #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>>>>    +#define CCLK_SRC_PLLP_OUT0 4
>>>>> +#define CCLK_SRC_PLLP_OUT4 5
>>>>> +
>>>>>    static u8 clk_super_get_parent(struct clk_hw *hw)
>>>>>    {
>>>>>        struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>>>>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw
>>>>> *hw, u8 index)
>>>>>            if (index == mux->div2_index)
>>>>>                index = mux->pllx_index;
>>>>>        }
>>>>> +
>>>>> +    /*
>>>>> +     * Enable PLLP branches to CPU before selecting PLLP source
>>>>> +     */
>>>>> +    if ((mux->flags & TEGRA_CPU_CLK) &&
>>>>> +        ((index == CCLK_SRC_PLLP_OUT0) || (index ==
>>>>> CCLK_SRC_PLLP_OUT4)))
>>>>> +        tegra_clk_set_pllp_out_cpu(true);
>>>> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
>>>> switching from PLLP?
>>> PLLP may be used for other CPU clusters.
>> Though to avoid flag and check needed to make sure other CPU is not
>> using before disabling PLLP branch to CPU.
>>
>> But leaving it enabled shouldn't impact much as clock source mux is
>> after this in design anyway.
>>
>> But can add as well if its clear that way.
> The TRM doc says "The CPU subsystem supports a switch-cluster mode
> meaning that only one of the clusters can be active at any given time".
>
> Given that cluster-switching isn't supported in upstream, I don't think
> that you need to care about the other cluster at all, at least for now.
>
> The cluster-switching implementation in upstream is very complicated
> because it requires a special "hotplugging" CPU governor, which
> apparently no other platform needs.
>
> [snip]

This patch enables PLLP branches to CPU for both CPUG & CPULP if they 
use PLLP source.

So, to disable PLLP out CPU when not in use, we still need check for 
other cluster because during resume both LP CPU and G CPU gets restored. 
CPUG runs from PLLP on resume and when it does super clk restore for LP 
CPU which may not be using PLLP, but as both uses same super mux 
clk_ops, without check (for PLLP branch to CPU in use) disabling PLLP 
branch to CPU during LP CPU restore looses clock to CPU G as well which 
is running from PLLP.

Will add check and disable PLLP if not in use in next version... this 
need extern flag as well to mark PLLP usage with either of CPU's.
Dmitry Osipenko July 22, 2019, 7:17 a.m. UTC | #6
22.07.2019 10:12, Sowjanya Komatineni пишет:
> 
> On 7/21/19 11:32 PM, Dmitry Osipenko wrote:
>> 22.07.2019 6:17, Sowjanya Komatineni пишет:
>>> On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:
>>>> On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>>> This patch has a fix to enable PLLP branches to CPU before changing
>>>>>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>>>>>
>>>>>> During system suspend entry and exit, CPU source will be switched
>>>>>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>>>>>> the switch.
>>>>>>
>>>>>> On system resume, warmboot code enables PLLP branches to CPU and
>>>>>> powers up the CPU with PLLP clock source.
>>>>>>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>>    drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>>>>>    drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>>>>>    drivers/clk/tegra/clk.h                  |  4 ++++
>>>>>>    3 files changed, 17 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/clk/tegra/clk-super.c
>>>>>> b/drivers/clk/tegra/clk-super.c
>>>>>> index 39ef31b46df5..d73c587e4853 100644
>>>>>> --- a/drivers/clk/tegra/clk-super.c
>>>>>> +++ b/drivers/clk/tegra/clk-super.c
>>>>>> @@ -28,6 +28,9 @@
>>>>>>    #define super_state_to_src_shift(m, s) ((m->width * s))
>>>>>>    #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>>>>>    +#define CCLK_SRC_PLLP_OUT0 4
>>>>>> +#define CCLK_SRC_PLLP_OUT4 5
>>>>>> +
>>>>>>    static u8 clk_super_get_parent(struct clk_hw *hw)
>>>>>>    {
>>>>>>        struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>>>>>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw
>>>>>> *hw, u8 index)
>>>>>>            if (index == mux->div2_index)
>>>>>>                index = mux->pllx_index;
>>>>>>        }
>>>>>> +
>>>>>> +    /*
>>>>>> +     * Enable PLLP branches to CPU before selecting PLLP source
>>>>>> +     */
>>>>>> +    if ((mux->flags & TEGRA_CPU_CLK) &&
>>>>>> +        ((index == CCLK_SRC_PLLP_OUT0) || (index ==
>>>>>> CCLK_SRC_PLLP_OUT4)))
>>>>>> +        tegra_clk_set_pllp_out_cpu(true);
>>>>> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
>>>>> switching from PLLP?
>>>> PLLP may be used for other CPU clusters.
>>> Though to avoid flag and check needed to make sure other CPU is not
>>> using before disabling PLLP branch to CPU.
>>>
>>> But leaving it enabled shouldn't impact much as clock source mux is
>>> after this in design anyway.
>>>
>>> But can add as well if its clear that way.
>> The TRM doc says "The CPU subsystem supports a switch-cluster mode
>> meaning that only one of the clusters can be active at any given time".
>>
>> Given that cluster-switching isn't supported in upstream, I don't think
>> that you need to care about the other cluster at all, at least for now.
>>
>> The cluster-switching implementation in upstream is very complicated
>> because it requires a special "hotplugging" CPU governor, which
>> apparently no other platform needs.
>>
>> [snip]
> 
> This patch enables PLLP branches to CPU for both CPUG & CPULP if they
> use PLLP source.
> 
> So, to disable PLLP out CPU when not in use, we still need check for
> other cluster because during resume both LP CPU and G CPU gets restored.
> CPUG runs from PLLP on resume and when it does super clk restore for LP
> CPU which may not be using PLLP, but as both uses same super mux
> clk_ops, without check (for PLLP branch to CPU in use) disabling PLLP
> branch to CPU during LP CPU restore looses clock to CPU G as well which
> is running from PLLP.
> 
> Will add check and disable PLLP if not in use in next version... this
> need extern flag as well to mark PLLP usage with either of CPU's.

I still don't understand why do you need to care about LP cluster at
all, given that it's always in a power-gated state.
Sowjanya Komatineni July 22, 2019, 7:24 a.m. UTC | #7
On 7/22/19 12:17 AM, Dmitry Osipenko wrote:
> 22.07.2019 10:12, Sowjanya Komatineni пишет:
>> On 7/21/19 11:32 PM, Dmitry Osipenko wrote:
>>> 22.07.2019 6:17, Sowjanya Komatineni пишет:
>>>> On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:
>>>>> On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
>>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>>>> This patch has a fix to enable PLLP branches to CPU before changing
>>>>>>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>>>>>>
>>>>>>> During system suspend entry and exit, CPU source will be switched
>>>>>>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>>>>>>> the switch.
>>>>>>>
>>>>>>> On system resume, warmboot code enables PLLP branches to CPU and
>>>>>>> powers up the CPU with PLLP clock source.
>>>>>>>
>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>> ---
>>>>>>>     drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>>>>>>     drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>>>>>>     drivers/clk/tegra/clk.h                  |  4 ++++
>>>>>>>     3 files changed, 17 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/clk/tegra/clk-super.c
>>>>>>> b/drivers/clk/tegra/clk-super.c
>>>>>>> index 39ef31b46df5..d73c587e4853 100644
>>>>>>> --- a/drivers/clk/tegra/clk-super.c
>>>>>>> +++ b/drivers/clk/tegra/clk-super.c
>>>>>>> @@ -28,6 +28,9 @@
>>>>>>>     #define super_state_to_src_shift(m, s) ((m->width * s))
>>>>>>>     #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>>>>>>     +#define CCLK_SRC_PLLP_OUT0 4
>>>>>>> +#define CCLK_SRC_PLLP_OUT4 5
>>>>>>> +
>>>>>>>     static u8 clk_super_get_parent(struct clk_hw *hw)
>>>>>>>     {
>>>>>>>         struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>>>>>>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw
>>>>>>> *hw, u8 index)
>>>>>>>             if (index == mux->div2_index)
>>>>>>>                 index = mux->pllx_index;
>>>>>>>         }
>>>>>>> +
>>>>>>> +    /*
>>>>>>> +     * Enable PLLP branches to CPU before selecting PLLP source
>>>>>>> +     */
>>>>>>> +    if ((mux->flags & TEGRA_CPU_CLK) &&
>>>>>>> +        ((index == CCLK_SRC_PLLP_OUT0) || (index ==
>>>>>>> CCLK_SRC_PLLP_OUT4)))
>>>>>>> +        tegra_clk_set_pllp_out_cpu(true);
>>>>>> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
>>>>>> switching from PLLP?
>>>>> PLLP may be used for other CPU clusters.
>>>> Though to avoid flag and check needed to make sure other CPU is not
>>>> using before disabling PLLP branch to CPU.
>>>>
>>>> But leaving it enabled shouldn't impact much as clock source mux is
>>>> after this in design anyway.
>>>>
>>>> But can add as well if its clear that way.
>>> The TRM doc says "The CPU subsystem supports a switch-cluster mode
>>> meaning that only one of the clusters can be active at any given time".
>>>
>>> Given that cluster-switching isn't supported in upstream, I don't think
>>> that you need to care about the other cluster at all, at least for now.
>>>
>>> The cluster-switching implementation in upstream is very complicated
>>> because it requires a special "hotplugging" CPU governor, which
>>> apparently no other platform needs.
>>>
>>> [snip]
>> This patch enables PLLP branches to CPU for both CPUG & CPULP if they
>> use PLLP source.
>>
>> So, to disable PLLP out CPU when not in use, we still need check for
>> other cluster because during resume both LP CPU and G CPU gets restored.
>> CPUG runs from PLLP on resume and when it does super clk restore for LP
>> CPU which may not be using PLLP, but as both uses same super mux
>> clk_ops, without check (for PLLP branch to CPU in use) disabling PLLP
>> branch to CPU during LP CPU restore looses clock to CPU G as well which
>> is running from PLLP.
>>
>> Will add check and disable PLLP if not in use in next version... this
>> need extern flag as well to mark PLLP usage with either of CPU's.
> I still don't understand why do you need to care about LP cluster at
> all, given that it's always in a power-gated state.

cclk_lp is registered thru super clk mux which uses same clk_ops as cclk_g.

during restore, cclk_lp also gets restored. So both cclk_lp & cclk_g 
goes thru same clk_ops

In this patch, I marked super flags with TEGRA_CPU_CLK for both cclk_lp 
& cclk_g.

So when cclk_lp restore happens, it goes thru same set_parent clk_ops 
and as its source is not PLLP, it tries to disable PLLP_OUT_CPU if its 
disabled without adding check for PLLP being in use by other cluster.

So either I should not mark cclk_lp as TEGRA_CPU_CLK and mark cclk_g 
only as TEGRA_CPU_CLK so PLLP out to CPU can be disabled without check 
if its not the source.

OR

With TEGRA_CPU_CLK used for both cclk_lp & cclk_g, need to add check if 
PLLP is in use so during cclk_lp restore it doesnt disable PLLP out to CPU.


To simplify without check, will just mark cclk_g super clock flag only 
as TEGRA_CPU_CLK so PLLP_OUT_CPU enable or disable happens only for CPUG
Dmitry Osipenko July 22, 2019, 7:30 a.m. UTC | #8
22.07.2019 10:24, Sowjanya Komatineni пишет:
> 
> On 7/22/19 12:17 AM, Dmitry Osipenko wrote:
>> 22.07.2019 10:12, Sowjanya Komatineni пишет:
>>> On 7/21/19 11:32 PM, Dmitry Osipenko wrote:
>>>> 22.07.2019 6:17, Sowjanya Komatineni пишет:
>>>>> On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:
>>>>>> On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
>>>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>>>>> This patch has a fix to enable PLLP branches to CPU before changing
>>>>>>>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>>>>>>>
>>>>>>>> During system suspend entry and exit, CPU source will be switched
>>>>>>>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>>>>>>>> the switch.
>>>>>>>>
>>>>>>>> On system resume, warmboot code enables PLLP branches to CPU and
>>>>>>>> powers up the CPU with PLLP clock source.
>>>>>>>>
>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>> ---
>>>>>>>>     drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>>>>>>>     drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>>>>>>>     drivers/clk/tegra/clk.h                  |  4 ++++
>>>>>>>>     3 files changed, 17 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/clk/tegra/clk-super.c
>>>>>>>> b/drivers/clk/tegra/clk-super.c
>>>>>>>> index 39ef31b46df5..d73c587e4853 100644
>>>>>>>> --- a/drivers/clk/tegra/clk-super.c
>>>>>>>> +++ b/drivers/clk/tegra/clk-super.c
>>>>>>>> @@ -28,6 +28,9 @@
>>>>>>>>     #define super_state_to_src_shift(m, s) ((m->width * s))
>>>>>>>>     #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>>>>>>>     +#define CCLK_SRC_PLLP_OUT0 4
>>>>>>>> +#define CCLK_SRC_PLLP_OUT4 5
>>>>>>>> +
>>>>>>>>     static u8 clk_super_get_parent(struct clk_hw *hw)
>>>>>>>>     {
>>>>>>>>         struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>>>>>>>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw
>>>>>>>> *hw, u8 index)
>>>>>>>>             if (index == mux->div2_index)
>>>>>>>>                 index = mux->pllx_index;
>>>>>>>>         }
>>>>>>>> +
>>>>>>>> +    /*
>>>>>>>> +     * Enable PLLP branches to CPU before selecting PLLP source
>>>>>>>> +     */
>>>>>>>> +    if ((mux->flags & TEGRA_CPU_CLK) &&
>>>>>>>> +        ((index == CCLK_SRC_PLLP_OUT0) || (index ==
>>>>>>>> CCLK_SRC_PLLP_OUT4)))
>>>>>>>> +        tegra_clk_set_pllp_out_cpu(true);
>>>>>>> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
>>>>>>> switching from PLLP?
>>>>>> PLLP may be used for other CPU clusters.
>>>>> Though to avoid flag and check needed to make sure other CPU is not
>>>>> using before disabling PLLP branch to CPU.
>>>>>
>>>>> But leaving it enabled shouldn't impact much as clock source mux is
>>>>> after this in design anyway.
>>>>>
>>>>> But can add as well if its clear that way.
>>>> The TRM doc says "The CPU subsystem supports a switch-cluster mode
>>>> meaning that only one of the clusters can be active at any given time".
>>>>
>>>> Given that cluster-switching isn't supported in upstream, I don't think
>>>> that you need to care about the other cluster at all, at least for now.
>>>>
>>>> The cluster-switching implementation in upstream is very complicated
>>>> because it requires a special "hotplugging" CPU governor, which
>>>> apparently no other platform needs.
>>>>
>>>> [snip]
>>> This patch enables PLLP branches to CPU for both CPUG & CPULP if they
>>> use PLLP source.
>>>
>>> So, to disable PLLP out CPU when not in use, we still need check for
>>> other cluster because during resume both LP CPU and G CPU gets restored.
>>> CPUG runs from PLLP on resume and when it does super clk restore for LP
>>> CPU which may not be using PLLP, but as both uses same super mux
>>> clk_ops, without check (for PLLP branch to CPU in use) disabling PLLP
>>> branch to CPU during LP CPU restore looses clock to CPU G as well which
>>> is running from PLLP.
>>>
>>> Will add check and disable PLLP if not in use in next version... this
>>> need extern flag as well to mark PLLP usage with either of CPU's.
>> I still don't understand why do you need to care about LP cluster at
>> all, given that it's always in a power-gated state.
> 
> cclk_lp is registered thru super clk mux which uses same clk_ops as cclk_g.
> 
> during restore, cclk_lp also gets restored. So both cclk_lp & cclk_g
> goes thru same clk_ops
> 
> In this patch, I marked super flags with TEGRA_CPU_CLK for both cclk_lp
> & cclk_g.
> 
> So when cclk_lp restore happens, it goes thru same set_parent clk_ops
> and as its source is not PLLP, it tries to disable PLLP_OUT_CPU if its
> disabled without adding check for PLLP being in use by other cluster.

Ah, okay.

> So either I should not mark cclk_lp as TEGRA_CPU_CLK and mark cclk_g
> only as TEGRA_CPU_CLK so PLLP out to CPU can be disabled without check
> if its not the source.
> 
> OR
> 
> With TEGRA_CPU_CLK used for both cclk_lp & cclk_g, need to add check if
> PLLP is in use so during cclk_lp restore it doesnt disable PLLP out to CPU.
> 
> 
> To simplify without check, will just mark cclk_g super clock flag only
> as TEGRA_CPU_CLK so PLLP_OUT_CPU enable or disable happens only for CPUG

Sounds good. Then please add a brief comment to the CPULP, telling why
it misses the flag, for the record.
Sowjanya Komatineni July 22, 2019, 7:36 a.m. UTC | #9
On 7/22/19 12:30 AM, Dmitry Osipenko wrote:
> 22.07.2019 10:24, Sowjanya Komatineni пишет:
>> On 7/22/19 12:17 AM, Dmitry Osipenko wrote:
>>> 22.07.2019 10:12, Sowjanya Komatineni пишет:
>>>> On 7/21/19 11:32 PM, Dmitry Osipenko wrote:
>>>>> 22.07.2019 6:17, Sowjanya Komatineni пишет:
>>>>>> On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:
>>>>>>> On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
>>>>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>>>>>> This patch has a fix to enable PLLP branches to CPU before changing
>>>>>>>>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>>>>>>>>
>>>>>>>>> During system suspend entry and exit, CPU source will be switched
>>>>>>>>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>>>>>>>>> the switch.
>>>>>>>>>
>>>>>>>>> On system resume, warmboot code enables PLLP branches to CPU and
>>>>>>>>> powers up the CPU with PLLP clock source.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>> ---
>>>>>>>>>      drivers/clk/tegra/clk-super.c            | 11 +++++++++++
>>>>>>>>>      drivers/clk/tegra/clk-tegra-super-gen4.c |  4 ++--
>>>>>>>>>      drivers/clk/tegra/clk.h                  |  4 ++++
>>>>>>>>>      3 files changed, 17 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/clk/tegra/clk-super.c
>>>>>>>>> b/drivers/clk/tegra/clk-super.c
>>>>>>>>> index 39ef31b46df5..d73c587e4853 100644
>>>>>>>>> --- a/drivers/clk/tegra/clk-super.c
>>>>>>>>> +++ b/drivers/clk/tegra/clk-super.c
>>>>>>>>> @@ -28,6 +28,9 @@
>>>>>>>>>      #define super_state_to_src_shift(m, s) ((m->width * s))
>>>>>>>>>      #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>>>>>>>>      +#define CCLK_SRC_PLLP_OUT0 4
>>>>>>>>> +#define CCLK_SRC_PLLP_OUT4 5
>>>>>>>>> +
>>>>>>>>>      static u8 clk_super_get_parent(struct clk_hw *hw)
>>>>>>>>>      {
>>>>>>>>>          struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>>>>>>>>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw
>>>>>>>>> *hw, u8 index)
>>>>>>>>>              if (index == mux->div2_index)
>>>>>>>>>                  index = mux->pllx_index;
>>>>>>>>>          }
>>>>>>>>> +
>>>>>>>>> +    /*
>>>>>>>>> +     * Enable PLLP branches to CPU before selecting PLLP source
>>>>>>>>> +     */
>>>>>>>>> +    if ((mux->flags & TEGRA_CPU_CLK) &&
>>>>>>>>> +        ((index == CCLK_SRC_PLLP_OUT0) || (index ==
>>>>>>>>> CCLK_SRC_PLLP_OUT4)))
>>>>>>>>> +        tegra_clk_set_pllp_out_cpu(true);
>>>>>>>> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
>>>>>>>> switching from PLLP?
>>>>>>> PLLP may be used for other CPU clusters.
>>>>>> Though to avoid flag and check needed to make sure other CPU is not
>>>>>> using before disabling PLLP branch to CPU.
>>>>>>
>>>>>> But leaving it enabled shouldn't impact much as clock source mux is
>>>>>> after this in design anyway.
>>>>>>
>>>>>> But can add as well if its clear that way.
>>>>> The TRM doc says "The CPU subsystem supports a switch-cluster mode
>>>>> meaning that only one of the clusters can be active at any given time".
>>>>>
>>>>> Given that cluster-switching isn't supported in upstream, I don't think
>>>>> that you need to care about the other cluster at all, at least for now.
>>>>>
>>>>> The cluster-switching implementation in upstream is very complicated
>>>>> because it requires a special "hotplugging" CPU governor, which
>>>>> apparently no other platform needs.
>>>>>
>>>>> [snip]
>>>> This patch enables PLLP branches to CPU for both CPUG & CPULP if they
>>>> use PLLP source.
>>>>
>>>> So, to disable PLLP out CPU when not in use, we still need check for
>>>> other cluster because during resume both LP CPU and G CPU gets restored.
>>>> CPUG runs from PLLP on resume and when it does super clk restore for LP
>>>> CPU which may not be using PLLP, but as both uses same super mux
>>>> clk_ops, without check (for PLLP branch to CPU in use) disabling PLLP
>>>> branch to CPU during LP CPU restore looses clock to CPU G as well which
>>>> is running from PLLP.
>>>>
>>>> Will add check and disable PLLP if not in use in next version... this
>>>> need extern flag as well to mark PLLP usage with either of CPU's.
>>> I still don't understand why do you need to care about LP cluster at
>>> all, given that it's always in a power-gated state.
>> cclk_lp is registered thru super clk mux which uses same clk_ops as cclk_g.
>>
>> during restore, cclk_lp also gets restored. So both cclk_lp & cclk_g
>> goes thru same clk_ops
>>
>> In this patch, I marked super flags with TEGRA_CPU_CLK for both cclk_lp
>> & cclk_g.
>>
>> So when cclk_lp restore happens, it goes thru same set_parent clk_ops
>> and as its source is not PLLP, it tries to disable PLLP_OUT_CPU if its
>> disabled without adding check for PLLP being in use by other cluster.
> Ah, okay.
>
>> So either I should not mark cclk_lp as TEGRA_CPU_CLK and mark cclk_g
>> only as TEGRA_CPU_CLK so PLLP out to CPU can be disabled without check
>> if its not the source.
>>
>> OR
>>
>> With TEGRA_CPU_CLK used for both cclk_lp & cclk_g, need to add check if
>> PLLP is in use so during cclk_lp restore it doesnt disable PLLP out to CPU.
>>
>>
>> To simplify without check, will just mark cclk_g super clock flag only
>> as TEGRA_CPU_CLK so PLLP_OUT_CPU enable or disable happens only for CPUG
> Sounds good. Then please add a brief comment to the CPULP, telling why
> it misses the flag, for the record.

Sure, will add comment.
diff mbox series

Patch

diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
index 39ef31b46df5..d73c587e4853 100644
--- a/drivers/clk/tegra/clk-super.c
+++ b/drivers/clk/tegra/clk-super.c
@@ -28,6 +28,9 @@ 
 #define super_state_to_src_shift(m, s) ((m->width * s))
 #define super_state_to_src_mask(m) (((1 << m->width) - 1))
 
+#define CCLK_SRC_PLLP_OUT0 4
+#define CCLK_SRC_PLLP_OUT4 5
+
 static u8 clk_super_get_parent(struct clk_hw *hw)
 {
 	struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
@@ -97,6 +100,14 @@  static int clk_super_set_parent(struct clk_hw *hw, u8 index)
 		if (index == mux->div2_index)
 			index = mux->pllx_index;
 	}
+
+	/*
+	 * Enable PLLP branches to CPU before selecting PLLP source
+	 */
+	if ((mux->flags & TEGRA_CPU_CLK) &&
+	    ((index == CCLK_SRC_PLLP_OUT0) || (index == CCLK_SRC_PLLP_OUT4)))
+		tegra_clk_set_pllp_out_cpu(true);
+
 	val &= ~((super_state_to_src_mask(mux)) << shift);
 	val |= (index & (super_state_to_src_mask(mux))) << shift;
 
diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
index cdfe7c9697e1..cd208d0eca2a 100644
--- a/drivers/clk/tegra/clk-tegra-super-gen4.c
+++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
@@ -180,7 +180,7 @@  static void __init tegra_super_clk_init(void __iomem *clk_base,
 					gen_info->num_cclk_g_parents,
 					CLK_SET_RATE_PARENT,
 					clk_base + CCLKG_BURST_POLICY,
-					0, 4, 8, 0, NULL);
+					TEGRA_CPU_CLK, 4, 8, 0, NULL);
 		} else {
 			clk = tegra_clk_register_super_mux("cclk_g",
 					gen_info->cclk_g_parents,
@@ -201,7 +201,7 @@  static void __init tegra_super_clk_init(void __iomem *clk_base,
 					gen_info->num_cclk_lp_parents,
 					CLK_SET_RATE_PARENT,
 					clk_base + CCLKLP_BURST_POLICY,
-					0, 4, 8, 0, NULL);
+					TEGRA_CPU_CLK, 4, 8, 0, NULL);
 		} else {
 			clk = tegra_clk_register_super_mux("cclk_lp",
 					gen_info->cclk_lp_parents,
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index ac6de3a0b91f..c357b49e49b0 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -694,6 +694,9 @@  struct clk *tegra_clk_register_periph_data(void __iomem *clk_base,
  * Flags:
  * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag indicates
  *     that this is LP cluster clock.
+ * TEGRA_CPU_CLK - This flag indicates this is CPU cluster clock. To use PLLP
+ * for CPU clock source, need to enable PLLP branches to CPU by setting the
+ * additional bit PLLP_OUT_CPU for gen5 super clock.
  */
 struct tegra_clk_super_mux {
 	struct clk_hw	hw;
@@ -710,6 +713,7 @@  struct tegra_clk_super_mux {
 #define to_clk_super_mux(_hw) container_of(_hw, struct tegra_clk_super_mux, hw)
 
 #define TEGRA_DIVIDER_2 BIT(0)
+#define TEGRA_CPU_CLK	BIT(1)
 
 extern const struct clk_ops tegra_clk_super_ops;
 struct clk *tegra_clk_register_super_mux(const char *name,