[PATCH] clk: tegra: Mark APB clock as critical

[PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
fixed the parent clock for APBDMA, but the consequence of this that
after probing the APBDMA device, the APB Clock (or PCLK) is now
disabled. Disabling the APB clock causes accesses to any other device
on the APB to hang and prevent Tegra from booting.

Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
to prevent the clock being disabled if unused on boot. However, even
if it is used, it still needs to be always kept enabled and so update
the flag for the APB clock to be "CLK_IS_CRITICAL".

Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")

Suggested-by: Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
index 4f6fd307cb70..10047107c1dc 100644
--- a/drivers/clk/tegra/clk-tegra-super-gen4.c
+++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
@@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
 				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
 				   &sysrate_lock);
 	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
-				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
+				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
 				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
 	*dt_clk = clk;
 }
-- 
2.7.4

[PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
fixed the parent clock for APBDMA, but the consequence of this that
after probing the APBDMA device, the APB Clock (or PCLK) is now
disabled. Disabling the APB clock causes accesses to any other device
on the APB to hang and prevent Tegra from booting.

Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
to prevent the clock being disabled if unused on boot. However, even
if it is used, it still needs to be always kept enabled and so update
the flag for the APB clock to be "CLK_IS_CRITICAL".

Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")

Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
index 4f6fd307cb70..10047107c1dc 100644
--- a/drivers/clk/tegra/clk-tegra-super-gen4.c
+++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
@@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
 				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
 				   &sysrate_lock);
 	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
-				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
+				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
 				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
 	*dt_clk = clk;
 }
-- 
2.7.4

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Peter De Schrijver]

On Mon, Oct 23, 2017 at 12:12:52PM +0100, Jon Hunter wrote:
> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
> fixed the parent clock for APBDMA, but the consequence of this that
> after probing the APBDMA device, the APB Clock (or PCLK) is now
> disabled. Disabling the APB clock causes accesses to any other device
> on the APB to hang and prevent Tegra from booting.
> 
> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
> to prevent the clock being disabled if unused on boot. However, even
> if it is used, it still needs to be always kept enabled and so update
> the flag for the APB clock to be "CLK_IS_CRITICAL".
> 
> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
> 
> Suggested-by: Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

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

> ---
>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
> index 4f6fd307cb70..10047107c1dc 100644
> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>  				   &sysrate_lock);
>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>  	*dt_clk = clk;
>  }
> -- 
> 2.7.4
> 

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Peter De Schrijver]

On Mon, Oct 23, 2017 at 12:12:52PM +0100, Jon Hunter wrote:
> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
> fixed the parent clock for APBDMA, but the consequence of this that
> after probing the APBDMA device, the APB Clock (or PCLK) is now
> disabled. Disabling the APB clock causes accesses to any other device
> on the APB to hang and prevent Tegra from booting.
> 
> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
> to prevent the clock being disabled if unused on boot. However, even
> if it is used, it still needs to be always kept enabled and so update
> the flag for the APB clock to be "CLK_IS_CRITICAL".
> 
> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
> 
> Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

Acked-By: Peter De Schrijver <pdeschrijver@nvidia.com>

> ---
>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
> index 4f6fd307cb70..10047107c1dc 100644
> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>  				   &sysrate_lock);
>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>  	*dt_clk = clk;
>  }
> -- 
> 2.7.4
> 

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 23.10.2017 14:12, Jon Hunter wrote:
> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
> fixed the parent clock for APBDMA, but the consequence of this that
> after probing the APBDMA device, the APB Clock (or PCLK) is now
> disabled. Disabling the APB clock causes accesses to any other device
> on the APB to hang and prevent Tegra from booting.
> 
> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
> to prevent the clock being disabled if unused on boot. However, even
> if it is used, it still needs to be always kept enabled and so update
> the flag for the APB clock to be "CLK_IS_CRITICAL".
> 
> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
> 
> Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
> index 4f6fd307cb70..10047107c1dc 100644
> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>  				   &sysrate_lock);
>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>  	*dt_clk = clk;
>  }
> 

Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
now I don't know what's the cause of the issue, may take a more detailed look
soon. If you have any suggestions, please tell.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 29.11.2017 02:30, Dmitry Osipenko wrote:
> On 23.10.2017 14:12, Jon Hunter wrote:
>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>> fixed the parent clock for APBDMA, but the consequence of this that
>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>> disabled. Disabling the APB clock causes accesses to any other device
>> on the APB to hang and prevent Tegra from booting.
>>
>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>> to prevent the clock being disabled if unused on boot. However, even
>> if it is used, it still needs to be always kept enabled and so update
>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>
>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>
>> Suggested-by: Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> index 4f6fd307cb70..10047107c1dc 100644
>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>  				   &sysrate_lock);
>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>  	*dt_clk = clk;
>>  }
>>
> 
> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
> now I don't know what's the cause of the issue, may take a more detailed look
> soon. If you have any suggestions, please tell.
> 

It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
clock rate is setup and in result it is enabled with some invalid rate config.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 29.11.2017 02:30, Dmitry Osipenko wrote:
> On 23.10.2017 14:12, Jon Hunter wrote:
>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>> fixed the parent clock for APBDMA, but the consequence of this that
>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>> disabled. Disabling the APB clock causes accesses to any other device
>> on the APB to hang and prevent Tegra from booting.
>>
>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>> to prevent the clock being disabled if unused on boot. However, even
>> if it is used, it still needs to be always kept enabled and so update
>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>
>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>
>> Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> index 4f6fd307cb70..10047107c1dc 100644
>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>  				   &sysrate_lock);
>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>  	*dt_clk = clk;
>>  }
>>
> 
> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
> now I don't know what's the cause of the issue, may take a more detailed look
> soon. If you have any suggestions, please tell.
> 

It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
clock rate is setup and in result it is enabled with some invalid rate config.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 29/11/17 00:09, Dmitry Osipenko wrote:
> On 29.11.2017 02:30, Dmitry Osipenko wrote:
>> On 23.10.2017 14:12, Jon Hunter wrote:
>>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>> fixed the parent clock for APBDMA, but the consequence of this that
>>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>>> disabled. Disabling the APB clock causes accesses to any other device
>>> on the APB to hang and prevent Tegra from booting.
>>>
>>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>>> to prevent the clock being disabled if unused on boot. However, even
>>> if it is used, it still needs to be always kept enabled and so update
>>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>>
>>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>
>>> Suggested-by: Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> index 4f6fd307cb70..10047107c1dc 100644
>>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>>  				   &sysrate_lock);
>>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>>  	*dt_clk = clk;
>>>  }
>>>
>>
>> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
>> now I don't know what's the cause of the issue, may take a more detailed look
>> soon. If you have any suggestions, please tell.
>>
> 
> It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
> clock rate is setup and in result it is enabled with some invalid rate config.

What Tegra20 platform? I have not seen any issues with booting Tegra20
trimslice with v4.15-rc1 or next-20171129. I am surprised this clock
would not have been enabled by the bootloader and hence rate set correctly.

Cheers
Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 29/11/17 00:09, Dmitry Osipenko wrote:
> On 29.11.2017 02:30, Dmitry Osipenko wrote:
>> On 23.10.2017 14:12, Jon Hunter wrote:
>>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>> fixed the parent clock for APBDMA, but the consequence of this that
>>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>>> disabled. Disabling the APB clock causes accesses to any other device
>>> on the APB to hang and prevent Tegra from booting.
>>>
>>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>>> to prevent the clock being disabled if unused on boot. However, even
>>> if it is used, it still needs to be always kept enabled and so update
>>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>>
>>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>
>>> Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> index 4f6fd307cb70..10047107c1dc 100644
>>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>>  				   &sysrate_lock);
>>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>>  	*dt_clk = clk;
>>>  }
>>>
>>
>> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
>> now I don't know what's the cause of the issue, may take a more detailed look
>> soon. If you have any suggestions, please tell.
>>
> 
> It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
> clock rate is setup and in result it is enabled with some invalid rate config.

What Tegra20 platform? I have not seen any issues with booting Tegra20
trimslice with v4.15-rc1 or next-20171129. I am surprised this clock
would not have been enabled by the bootloader and hence rate set correctly.

Cheers
Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 29.11.2017 13:12, Jon Hunter wrote:
> 
> On 29/11/17 00:09, Dmitry Osipenko wrote:
>> On 29.11.2017 02:30, Dmitry Osipenko wrote:
>>> On 23.10.2017 14:12, Jon Hunter wrote:
>>>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>> fixed the parent clock for APBDMA, but the consequence of this that
>>>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>>>> disabled. Disabling the APB clock causes accesses to any other device
>>>> on the APB to hang and prevent Tegra from booting.
>>>>
>>>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>>>> to prevent the clock being disabled if unused on boot. However, even
>>>> if it is used, it still needs to be always kept enabled and so update
>>>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>>>
>>>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>>
>>>> Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>>> ---
>>>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>> index 4f6fd307cb70..10047107c1dc 100644
>>>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>>>  				   &sysrate_lock);
>>>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>>>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>>>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>>>  	*dt_clk = clk;
>>>>  }
>>>>
>>>
>>> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
>>> now I don't know what's the cause of the issue, may take a more detailed look
>>> soon. If you have any suggestions, please tell.
>>>
>>
>> It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
>> clock rate is setup and in result it is enabled with some invalid rate config.
> 
> What Tegra20 platform? I have not seen any issues with booting Tegra20
> trimslice with v4.15-rc1 or next-20171129. I am surprised this clock
> would not have been enabled by the bootloader and hence rate set correctly.
> 

It is not an upstream'ed device and it uses proprietary bootloader. But turned
out it is unrelated.. the actual problem is that with the offending patch
applied, for some reason PLL_M is now getting disabled on SCLK reparent, PLL_M
is a critical clock that feeds EMC and marking it as CLK_IS_CRITICAL fixes issue
for me.

Jon, could you please try to revert "Mark APB clock as critical" patch, mark the
PLL_M as critical and re-test?

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 29/11/17 15:08, Dmitry Osipenko wrote:
> On 29.11.2017 13:12, Jon Hunter wrote:
>>
>> On 29/11/17 00:09, Dmitry Osipenko wrote:
>>> On 29.11.2017 02:30, Dmitry Osipenko wrote:
>>>> On 23.10.2017 14:12, Jon Hunter wrote:
>>>>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>>> fixed the parent clock for APBDMA, but the consequence of this that
>>>>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>>>>> disabled. Disabling the APB clock causes accesses to any other device
>>>>> on the APB to hang and prevent Tegra from booting.
>>>>>
>>>>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>>>>> to prevent the clock being disabled if unused on boot. However, even
>>>>> if it is used, it still needs to be always kept enabled and so update
>>>>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>>>>
>>>>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>>>
>>>>> Suggested-by: Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>> ---
>>>>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>> index 4f6fd307cb70..10047107c1dc 100644
>>>>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>>>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>>>>  				   &sysrate_lock);
>>>>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>>>>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>>>>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>>>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>>>>  	*dt_clk = clk;
>>>>>  }
>>>>>
>>>>
>>>> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
>>>> now I don't know what's the cause of the issue, may take a more detailed look
>>>> soon. If you have any suggestions, please tell.
>>>>
>>>
>>> It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
>>> clock rate is setup and in result it is enabled with some invalid rate config.
>>
>> What Tegra20 platform? I have not seen any issues with booting Tegra20
>> trimslice with v4.15-rc1 or next-20171129. I am surprised this clock
>> would not have been enabled by the bootloader and hence rate set correctly.
>>
> 
> It is not an upstream'ed device and it uses proprietary bootloader. But turned
> out it is unrelated.. the actual problem is that with the offending patch
> applied, for some reason PLL_M is now getting disabled on SCLK reparent, PLL_M
> is a critical clock that feeds EMC and marking it as CLK_IS_CRITICAL fixes issue
> for me.
> 
> Jon, could you please try to revert "Mark APB clock as critical" patch, mark the
> PLL_M as critical and re-test?

Sorry but you have lost me here. I am not sure I understand the relation
between the problem you are describing and this change.

Please note that the problem I was fixing with this patch was seen on
Tegra114/124 and not Tegra20. I don't recall seeing any issues with
Tegra20 and I just checked Tegra20 is still booting for me even when
reverting this. So it sounds like a different problem AFAICT.

Cheers
Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 29/11/17 15:08, Dmitry Osipenko wrote:
> On 29.11.2017 13:12, Jon Hunter wrote:
>>
>> On 29/11/17 00:09, Dmitry Osipenko wrote:
>>> On 29.11.2017 02:30, Dmitry Osipenko wrote:
>>>> On 23.10.2017 14:12, Jon Hunter wrote:
>>>>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>>> fixed the parent clock for APBDMA, but the consequence of this that
>>>>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>>>>> disabled. Disabling the APB clock causes accesses to any other device
>>>>> on the APB to hang and prevent Tegra from booting.
>>>>>
>>>>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>>>>> to prevent the clock being disabled if unused on boot. However, even
>>>>> if it is used, it still needs to be always kept enabled and so update
>>>>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>>>>
>>>>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>>>
>>>>> Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>>>> ---
>>>>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>> index 4f6fd307cb70..10047107c1dc 100644
>>>>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>>>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>>>>  				   &sysrate_lock);
>>>>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>>>>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>>>>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>>>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>>>>  	*dt_clk = clk;
>>>>>  }
>>>>>
>>>>
>>>> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
>>>> now I don't know what's the cause of the issue, may take a more detailed look
>>>> soon. If you have any suggestions, please tell.
>>>>
>>>
>>> It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
>>> clock rate is setup and in result it is enabled with some invalid rate config.
>>
>> What Tegra20 platform? I have not seen any issues with booting Tegra20
>> trimslice with v4.15-rc1 or next-20171129. I am surprised this clock
>> would not have been enabled by the bootloader and hence rate set correctly.
>>
> 
> It is not an upstream'ed device and it uses proprietary bootloader. But turned
> out it is unrelated.. the actual problem is that with the offending patch
> applied, for some reason PLL_M is now getting disabled on SCLK reparent, PLL_M
> is a critical clock that feeds EMC and marking it as CLK_IS_CRITICAL fixes issue
> for me.
> 
> Jon, could you please try to revert "Mark APB clock as critical" patch, mark the
> PLL_M as critical and re-test?

Sorry but you have lost me here. I am not sure I understand the relation
between the problem you are describing and this change.

Please note that the problem I was fixing with this patch was seen on
Tegra114/124 and not Tegra20. I don't recall seeing any issues with
Tegra20 and I just checked Tegra20 is still booting for me even when
reverting this. So it sounds like a different problem AFAICT.

Cheers
Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 30.11.2017 01:55, Jon Hunter wrote:
> 
> On 29/11/17 15:08, Dmitry Osipenko wrote:
>> On 29.11.2017 13:12, Jon Hunter wrote:
>>>
>>> On 29/11/17 00:09, Dmitry Osipenko wrote:
>>>> On 29.11.2017 02:30, Dmitry Osipenko wrote:
>>>>> On 23.10.2017 14:12, Jon Hunter wrote:
>>>>>> Commit a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>>>> fixed the parent clock for APBDMA, but the consequence of this that
>>>>>> after probing the APBDMA device, the APB Clock (or PCLK) is now
>>>>>> disabled. Disabling the APB clock causes accesses to any other device
>>>>>> on the APB to hang and prevent Tegra from booting.
>>>>>>
>>>>>> Currently, the APB clock is registered with the flag "CLK_IGNORE_UNUSED"
>>>>>> to prevent the clock being disabled if unused on boot. However, even
>>>>>> if it is used, it still needs to be always kept enabled and so update
>>>>>> the flag for the APB clock to be "CLK_IS_CRITICAL".
>>>>>>
>>>>>> Fixes: a140614373ae ("clk: tegra: Correct parent of the APBDMA clock")
>>>>>>
>>>>>> Suggested-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>>>>> ---
>>>>>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>>> index 4f6fd307cb70..10047107c1dc 100644
>>>>>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>>>>>> @@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>>>>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>>>>>  				   &sysrate_lock);
>>>>>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>>>>>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>>>>>> +				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
>>>>>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>>>>>  	*dt_clk = clk;
>>>>>>  }
>>>>>>
>>>>>
>>>>> Unfortunately this patch somehow breaks Tegra20, getting a hang during boot. For
>>>>> now I don't know what's the cause of the issue, may take a more detailed look
>>>>> soon. If you have any suggestions, please tell.
>>>>>
>>>>
>>>> It looks like that with CLK_IS_CRITICAL flag, pclk is getting enabled before
>>>> clock rate is setup and in result it is enabled with some invalid rate config.
>>>
>>> What Tegra20 platform? I have not seen any issues with booting Tegra20
>>> trimslice with v4.15-rc1 or next-20171129. I am surprised this clock
>>> would not have been enabled by the bootloader and hence rate set correctly.
>>>
>>
>> It is not an upstream'ed device and it uses proprietary bootloader. But turned
>> out it is unrelated.. the actual problem is that with the offending patch
>> applied, for some reason PLL_M is now getting disabled on SCLK reparent, PLL_M
>> is a critical clock that feeds EMC and marking it as CLK_IS_CRITICAL fixes issue
>> for me.
>>
>> Jon, could you please try to revert "Mark APB clock as critical" patch, mark the
>> PLL_M as critical and re-test?
> 
> Sorry but you have lost me here. I am not sure I understand the relation
> between the problem you are describing and this change.
> 

The patch "Mark APB clock as critical" causes a hang on my Tegra20. Marking
PLL_M as critical on top of the offending patch fixes issue, as well as
reverting the offending patch. I know that it looks like there is something
fishy here, yet not sure what is going on and why it is not failing on your
Trmislice.

> Please note that the problem I was fixing with this patch was seen on
> Tegra114/124 and not Tegra20. I don't recall seeing any issues with
> Tegra20 and I just checked Tegra20 is still booting for me even when
> reverting this. So it sounds like a different problem AFAICT.
> 

I've asked you to re-test Tegra114/124 or whatever was failing for you with the
PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
caused trouble on Tegra114/124.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 29/11/17 23:13, Dmitry Osipenko wrote:
> On 30.11.2017 01:55, Jon Hunter wrote:

...

> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
> caused trouble on Tegra114/124.

Please share the exact change you would like me to test and I will.

Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 29/11/17 23:13, Dmitry Osipenko wrote:
> On 30.11.2017 01:55, Jon Hunter wrote:

...

> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
> caused trouble on Tegra114/124.

Please share the exact change you would like me to test and I will.

Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 30.11.2017 14:31, Jon Hunter wrote:
> 
> On 29/11/17 23:13, Dmitry Osipenko wrote:
>> On 30.11.2017 01:55, Jon Hunter wrote:
> 
> ...
> 
>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>> caused trouble on Tegra114/124.
> 
> Please share the exact change you would like me to test and I will.

Please try this:

diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
index 16e0aee14773..58874c1bbf5e 100644
--- a/drivers/clk/tegra/clk-divider.c
+++ b/drivers/clk/tegra/clk-divider.c
@@ -194,6 +194,7 @@ static const struct clk_div_table mc_div_table[] = {
 struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
 				  void __iomem *reg, spinlock_t *lock)
 {
-	return clk_register_divider_table(NULL, name, parent_name, 0, reg,
-					  16, 1, 0, mc_div_table, lock);
+	return clk_register_divider_table(NULL, name, parent_name,
+					  CLK_IS_CRITICAL, reg, 16, 1, 0,
+					  mc_div_table, lock);
 }
diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c
index 11a5066e5c27..5234acd30e89 100644
--- a/drivers/clk/tegra/clk-emc.c
+++ b/drivers/clk/tegra/clk-emc.c
@@ -515,7 +515,7 @@ struct clk *tegra_clk_register_emc(void __iomem *base,
struct device_node *np,

 	init.name = "emc";
 	init.ops = &tegra_clk_emc_ops;
-	init.flags = 0;
+	init.flags = CLK_IS_CRITICAL;
 	init.parent_names = emc_parent_clk_names;
 	init.num_parents = ARRAY_SIZE(emc_parent_clk_names);

diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c
b/drivers/clk/tegra/clk-tegra-super-gen4.c
index 10047107c1dc..4f6fd307cb70 100644
--- a/drivers/clk/tegra/clk-tegra-super-gen4.c
+++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
@@ -166,7 +166,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
 				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
 				   &sysrate_lock);
 	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
-				CLK_IS_CRITICAL, clk_base + SYSTEM_CLK_RATE,
+				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
 				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
 	*dt_clk = clk;
 }
diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index 63087d17c3e2..10f92178b6f2 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -1161,6 +1161,7 @@ static const struct of_device_id pmc_match[] __initconst = {
  * breaks
  */
 static struct tegra_clk_init_table init_table[] __initdata = {
+	{ TEGRA114_CLK_EMC, TEGRA114_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA114_CLK_UARTA, TEGRA114_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA114_CLK_UARTB, TEGRA114_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA114_CLK_UARTC, TEGRA114_CLK_PLL_P, 408000000, 0 },
diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index e81ea5b11577..7936b86f9a2a 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -1089,7 +1089,7 @@ static void __init tegra124_pll_init(void __iomem *clk_base,

 	/* PLLM */
 	clk = tegra_clk_register_pllm("pll_m", "pll_ref", clk_base, pmc,
-			     CLK_IGNORE_UNUSED | CLK_SET_RATE_GATE,
+			     CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
 			     &pll_m_params, NULL);
 	clk_register_clkdev(clk, "pll_m", NULL);
 	clks[TEGRA124_CLK_PLL_M] = clk;
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index e33d7548a4e9..18c2a0ea3f0f 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -819,8 +819,9 @@ static void __init tegra20_periph_clk_init(void)
 			       CLK_SET_RATE_NO_REPARENT,
 			       clk_base + CLK_SOURCE_EMC,
 			       30, 2, 0, &emc_lock);
-	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0,
-				    57, periph_clk_enb_refcnt);
+	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base,
+					     CLK_IS_CRITICAL, 57,
+					     periph_clk_enb_refcnt);
 	clks[TEGRA20_CLK_EMC] = clk;

 	clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
@@ -1030,7 +1031,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA20_CLK_HCLK, TEGRA20_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA20_CLK_PCLK, TEGRA20_CLK_CLK_MAX, 60000000, 1 },
 	{ TEGRA20_CLK_CSITE, TEGRA20_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA20_CLK_EMC, TEGRA20_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA20_CLK_CCLK, TEGRA20_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA20_CLK_UARTA, TEGRA20_CLK_PLL_P, 0, 0 },
 	{ TEGRA20_CLK_UARTB, TEGRA20_CLK_PLL_P, 0, 0 },
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 9e6260869eb9..3f54c48701c0 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -2786,7 +2786,8 @@ static void __init tegra210_pll_init(void __iomem *clk_base,

 	/* PLLM */
 	clk = tegra_clk_register_pllm("pll_m", "osc", clk_base, pmc,
-			     CLK_SET_RATE_GATE, &pll_m_params, NULL);
+				      CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
+				      &pll_m_params, NULL);
 	clk_register_clkdev(clk, "pll_m", NULL);
 	clks[TEGRA210_CLK_PLL_M] = clk;

@@ -3009,6 +3010,7 @@ static const struct of_device_id pmc_match[] __initconst = {
 };

 static struct tegra_clk_init_table init_table[] __initdata = {
+	{ TEGRA210_CLK_EMC, TEGRA210_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA210_CLK_UARTA, TEGRA210_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA210_CLK_UARTB, TEGRA210_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA210_CLK_UARTC, TEGRA210_CLK_PLL_P, 408000000, 0 },
@@ -3040,7 +3042,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA210_CLK_XUSB_DEV_SRC, TEGRA210_CLK_PLL_P_OUT_XUSB, 102000000, 0 },
 	{ TEGRA210_CLK_SATA, TEGRA210_CLK_PLL_P, 104000000, 0 },
 	{ TEGRA210_CLK_SATA_OOB, TEGRA210_CLK_PLL_P, 204000000, 0 },
-	{ TEGRA210_CLK_EMC, TEGRA210_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA210_CLK_MSELECT, TEGRA210_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA210_CLK_CSITE, TEGRA210_CLK_CLK_MAX, 0, 1 },
 	/* TODO find a way to enable this on-demand */
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index bee84c554932..f4c483c02d91 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1060,8 +1060,9 @@ static void __init tegra30_periph_clk_init(void)
 			       CLK_SET_RATE_NO_REPARENT,
 			       clk_base + CLK_SOURCE_EMC,
 			       30, 2, 0, &emc_lock);
-	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0,
-				    57, periph_clk_enb_refcnt);
+	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base,
+					     CLK_IS_CRITICAL, 57,
+					     periph_clk_enb_refcnt);
 	clks[TEGRA30_CLK_EMC] = clk;

 	clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
@@ -1255,7 +1256,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA30_CLK_PLL_M, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_PCLK, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_CSITE, TEGRA30_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA30_CLK_EMC, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_MSELECT, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_SBC1, TEGRA30_CLK_PLL_P, 100000000, 0 },
 	{ TEGRA30_CLK_SBC2, TEGRA30_CLK_PLL_P, 100000000, 0 },

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 30/11/17 13:24, Dmitry Osipenko wrote:
> On 30.11.2017 14:31, Jon Hunter wrote:
>>
>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>
>> ...
>>
>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>> caused trouble on Tegra114/124.
>>
>> Please share the exact change you would like me to test and I will.
> 
> Please try this:

I tried the patch, but this does not work for Tegra124 it still hangs.
Tracing the clk calls the last thing I see is ...

[    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
[    2.694403] clk_prepare: hclk_div
[    2.695929] clk_prepare_complete: hclk_div
[    2.700027] clk_prepare: hclk
[    2.702947] clk_prepare_complete: hclk
[    2.706673] clk_prepare: pclk_div
[    2.709986] clk_prepare_complete: pclk_div
[    2.714039] clk_prepare: pclk
[    2.716985] clk_prepare_complete: pclk
[    2.720739] clk_prepare: apbdma
[    2.723833] clk_prepare_complete: apbdma
[    2.727736] clk_enable: hclk_div
[    2.730940] clk_enable_complete: hclk_div
[    2.734926] clk_enable: hclk
[    2.737788] clk_enable_complete: hclk
[    2.741426] clk_enable: pclk_div
[    2.744633] clk_enable_complete: pclk_div
[    2.748619] clk_enable: pclk
[    2.751481] clk_enable_complete: pclk
[    2.755120] clk_enable: apbdma
[    2.758153] clk_enable_complete: apbdma
[    2.762390] clk_disable: apbdma
[    2.765088] clk_disable_complete: apbdma
[    2.768986] clk

So I believe this change is correct and that Peter's analysis on IRC
seems correct, that this change has exposed another issue with the clock
driver.

Peter, can you summarise which other clocks should be made critical?

Cheers
Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 30/11/17 13:24, Dmitry Osipenko wrote:
> On 30.11.2017 14:31, Jon Hunter wrote:
>>
>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>
>> ...
>>
>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>> caused trouble on Tegra114/124.
>>
>> Please share the exact change you would like me to test and I will.
> 
> Please try this:

I tried the patch, but this does not work for Tegra124 it still hangs.
Tracing the clk calls the last thing I see is ...

[    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
[    2.694403] clk_prepare: hclk_div
[    2.695929] clk_prepare_complete: hclk_div
[    2.700027] clk_prepare: hclk
[    2.702947] clk_prepare_complete: hclk
[    2.706673] clk_prepare: pclk_div
[    2.709986] clk_prepare_complete: pclk_div
[    2.714039] clk_prepare: pclk
[    2.716985] clk_prepare_complete: pclk
[    2.720739] clk_prepare: apbdma
[    2.723833] clk_prepare_complete: apbdma
[    2.727736] clk_enable: hclk_div
[    2.730940] clk_enable_complete: hclk_div
[    2.734926] clk_enable: hclk
[    2.737788] clk_enable_complete: hclk
[    2.741426] clk_enable: pclk_div
[    2.744633] clk_enable_complete: pclk_div
[    2.748619] clk_enable: pclk
[    2.751481] clk_enable_complete: pclk
[    2.755120] clk_enable: apbdma
[    2.758153] clk_enable_complete: apbdma
[    2.762390] clk_disable: apbdma
[    2.765088] clk_disable_complete: apbdma
[    2.768986] clk

So I believe this change is correct and that Peter's analysis on IRC
seems correct, that this change has exposed another issue with the clock
driver.

Peter, can you summarise which other clocks should be made critical?

Cheers
Jon

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 30.11.2017 19:39, Jon Hunter wrote:
> 
> On 30/11/17 13:24, Dmitry Osipenko wrote:
>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>
>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>
>>> ...
>>>
>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>> caused trouble on Tegra114/124.
>>>
>>> Please share the exact change you would like me to test and I will.
>>
>> Please try this:
> 
> I tried the patch, but this does not work for Tegra124 it still hangs.
> Tracing the clk calls the last thing I see is ...
> 
> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
> [    2.694403] clk_prepare: hclk_div
> [    2.695929] clk_prepare_complete: hclk_div
> [    2.700027] clk_prepare: hclk
> [    2.702947] clk_prepare_complete: hclk
> [    2.706673] clk_prepare: pclk_div
> [    2.709986] clk_prepare_complete: pclk_div
> [    2.714039] clk_prepare: pclk
> [    2.716985] clk_prepare_complete: pclk
> [    2.720739] clk_prepare: apbdma
> [    2.723833] clk_prepare_complete: apbdma
> [    2.727736] clk_enable: hclk_div
> [    2.730940] clk_enable_complete: hclk_div
> [    2.734926] clk_enable: hclk
> [    2.737788] clk_enable_complete: hclk
> [    2.741426] clk_enable: pclk_div
> [    2.744633] clk_enable_complete: pclk_div
> [    2.748619] clk_enable: pclk
> [    2.751481] clk_enable_complete: pclk
> [    2.755120] clk_enable: apbdma
> [    2.758153] clk_enable_complete: apbdma
> [    2.762390] clk_disable: apbdma
> [    2.765088] clk_disable_complete: apbdma
> [    2.768986] clk
> 
> So I believe this change is correct and that Peter's analysis on IRC
> seems correct, that this change has exposed another issue with the clock
> driver.
> 

Could you please show the /sys/kernel/debug/clk/clk_summary?

> Peter, can you summarise which other clocks should be made critical?
> 

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 30.11.2017 19:39, Jon Hunter wrote:
> 
> On 30/11/17 13:24, Dmitry Osipenko wrote:
>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>
>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>
>>> ...
>>>
>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>> caused trouble on Tegra114/124.
>>>
>>> Please share the exact change you would like me to test and I will.
>>
>> Please try this:
> 
> I tried the patch, but this does not work for Tegra124 it still hangs.
> Tracing the clk calls the last thing I see is ...
> 
> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
> [    2.694403] clk_prepare: hclk_div
> [    2.695929] clk_prepare_complete: hclk_div
> [    2.700027] clk_prepare: hclk
> [    2.702947] clk_prepare_complete: hclk
> [    2.706673] clk_prepare: pclk_div
> [    2.709986] clk_prepare_complete: pclk_div
> [    2.714039] clk_prepare: pclk
> [    2.716985] clk_prepare_complete: pclk
> [    2.720739] clk_prepare: apbdma
> [    2.723833] clk_prepare_complete: apbdma
> [    2.727736] clk_enable: hclk_div
> [    2.730940] clk_enable_complete: hclk_div
> [    2.734926] clk_enable: hclk
> [    2.737788] clk_enable_complete: hclk
> [    2.741426] clk_enable: pclk_div
> [    2.744633] clk_enable_complete: pclk_div
> [    2.748619] clk_enable: pclk
> [    2.751481] clk_enable_complete: pclk
> [    2.755120] clk_enable: apbdma
> [    2.758153] clk_enable_complete: apbdma
> [    2.762390] clk_disable: apbdma
> [    2.765088] clk_disable_complete: apbdma
> [    2.768986] clk
> 
> So I believe this change is correct and that Peter's analysis on IRC
> seems correct, that this change has exposed another issue with the clock
> driver.
> 

Could you please show the /sys/kernel/debug/clk/clk_summary?

> Peter, can you summarise which other clocks should be made critical?
> 

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 30/11/17 16:45, Dmitry Osipenko wrote:
> On 30.11.2017 19:39, Jon Hunter wrote:
>>
>> On 30/11/17 13:24, Dmitry Osipenko wrote:
>>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>>
>>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>>
>>>> ...
>>>>
>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>>> caused trouble on Tegra114/124.
>>>>
>>>> Please share the exact change you would like me to test and I will.
>>>
>>> Please try this:
>>
>> I tried the patch, but this does not work for Tegra124 it still hangs.
>> Tracing the clk calls the last thing I see is ...
>>
>> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
>> [    2.694403] clk_prepare: hclk_div
>> [    2.695929] clk_prepare_complete: hclk_div
>> [    2.700027] clk_prepare: hclk
>> [    2.702947] clk_prepare_complete: hclk
>> [    2.706673] clk_prepare: pclk_div
>> [    2.709986] clk_prepare_complete: pclk_div
>> [    2.714039] clk_prepare: pclk
>> [    2.716985] clk_prepare_complete: pclk
>> [    2.720739] clk_prepare: apbdma
>> [    2.723833] clk_prepare_complete: apbdma
>> [    2.727736] clk_enable: hclk_div
>> [    2.730940] clk_enable_complete: hclk_div
>> [    2.734926] clk_enable: hclk
>> [    2.737788] clk_enable_complete: hclk
>> [    2.741426] clk_enable: pclk_div
>> [    2.744633] clk_enable_complete: pclk_div
>> [    2.748619] clk_enable: pclk
>> [    2.751481] clk_enable_complete: pclk
>> [    2.755120] clk_enable: apbdma
>> [    2.758153] clk_enable_complete: apbdma
>> [    2.762390] clk_disable: apbdma
>> [    2.765088] clk_disable_complete: apbdma
>> [    2.768986] clk
>>
>> So I believe this change is correct and that Peter's analysis on IRC
>> seems correct, that this change has exposed another issue with the clock
>> driver.
>>
> 
> Could you please show the /sys/kernel/debug/clk/clk_summary?

With what patches present?

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 30/11/17 16:45, Dmitry Osipenko wrote:
> On 30.11.2017 19:39, Jon Hunter wrote:
>>
>> On 30/11/17 13:24, Dmitry Osipenko wrote:
>>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>>
>>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>>
>>>> ...
>>>>
>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>>> caused trouble on Tegra114/124.
>>>>
>>>> Please share the exact change you would like me to test and I will.
>>>
>>> Please try this:
>>
>> I tried the patch, but this does not work for Tegra124 it still hangs.
>> Tracing the clk calls the last thing I see is ...
>>
>> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
>> [    2.694403] clk_prepare: hclk_div
>> [    2.695929] clk_prepare_complete: hclk_div
>> [    2.700027] clk_prepare: hclk
>> [    2.702947] clk_prepare_complete: hclk
>> [    2.706673] clk_prepare: pclk_div
>> [    2.709986] clk_prepare_complete: pclk_div
>> [    2.714039] clk_prepare: pclk
>> [    2.716985] clk_prepare_complete: pclk
>> [    2.720739] clk_prepare: apbdma
>> [    2.723833] clk_prepare_complete: apbdma
>> [    2.727736] clk_enable: hclk_div
>> [    2.730940] clk_enable_complete: hclk_div
>> [    2.734926] clk_enable: hclk
>> [    2.737788] clk_enable_complete: hclk
>> [    2.741426] clk_enable: pclk_div
>> [    2.744633] clk_enable_complete: pclk_div
>> [    2.748619] clk_enable: pclk
>> [    2.751481] clk_enable_complete: pclk
>> [    2.755120] clk_enable: apbdma
>> [    2.758153] clk_enable_complete: apbdma
>> [    2.762390] clk_disable: apbdma
>> [    2.765088] clk_disable_complete: apbdma
>> [    2.768986] clk
>>
>> So I believe this change is correct and that Peter's analysis on IRC
>> seems correct, that this change has exposed another issue with the clock
>> driver.
>>
> 
> Could you please show the /sys/kernel/debug/clk/clk_summary?

With what patches present?

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 30.11.2017 19:51, Jon Hunter wrote:
> 
> On 30/11/17 16:45, Dmitry Osipenko wrote:
>> On 30.11.2017 19:39, Jon Hunter wrote:
>>>
>>> On 30/11/17 13:24, Dmitry Osipenko wrote:
>>>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>>>
>>>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>>>
>>>>> ...
>>>>>
>>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>>>> caused trouble on Tegra114/124.
>>>>>
>>>>> Please share the exact change you would like me to test and I will.
>>>>
>>>> Please try this:
>>>
>>> I tried the patch, but this does not work for Tegra124 it still hangs.
>>> Tracing the clk calls the last thing I see is ...
>>>
>>> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
>>> [    2.694403] clk_prepare: hclk_div
>>> [    2.695929] clk_prepare_complete: hclk_div
>>> [    2.700027] clk_prepare: hclk
>>> [    2.702947] clk_prepare_complete: hclk
>>> [    2.706673] clk_prepare: pclk_div
>>> [    2.709986] clk_prepare_complete: pclk_div
>>> [    2.714039] clk_prepare: pclk
>>> [    2.716985] clk_prepare_complete: pclk
>>> [    2.720739] clk_prepare: apbdma
>>> [    2.723833] clk_prepare_complete: apbdma
>>> [    2.727736] clk_enable: hclk_div
>>> [    2.730940] clk_enable_complete: hclk_div
>>> [    2.734926] clk_enable: hclk
>>> [    2.737788] clk_enable_complete: hclk
>>> [    2.741426] clk_enable: pclk_div
>>> [    2.744633] clk_enable_complete: pclk_div
>>> [    2.748619] clk_enable: pclk
>>> [    2.751481] clk_enable_complete: pclk
>>> [    2.755120] clk_enable: apbdma
>>> [    2.758153] clk_enable_complete: apbdma
>>> [    2.762390] clk_disable: apbdma
>>> [    2.765088] clk_disable_complete: apbdma
>>> [    2.768986] clk
>>>
>>> So I believe this change is correct and that Peter's analysis on IRC
>>> seems correct, that this change has exposed another issue with the clock
>>> driver.
>>>
>>
>> Could you please show the /sys/kernel/debug/clk/clk_summary?
> 
> With what patches present?
> 

linux-next without any additional patches

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 30.11.2017 19:51, Jon Hunter wrote:
> 
> On 30/11/17 16:45, Dmitry Osipenko wrote:
>> On 30.11.2017 19:39, Jon Hunter wrote:
>>>
>>> On 30/11/17 13:24, Dmitry Osipenko wrote:
>>>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>>>
>>>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>>>
>>>>> ...
>>>>>
>>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>>>> caused trouble on Tegra114/124.
>>>>>
>>>>> Please share the exact change you would like me to test and I will.
>>>>
>>>> Please try this:
>>>
>>> I tried the patch, but this does not work for Tegra124 it still hangs.
>>> Tracing the clk calls the last thing I see is ...
>>>
>>> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
>>> [    2.694403] clk_prepare: hclk_div
>>> [    2.695929] clk_prepare_complete: hclk_div
>>> [    2.700027] clk_prepare: hclk
>>> [    2.702947] clk_prepare_complete: hclk
>>> [    2.706673] clk_prepare: pclk_div
>>> [    2.709986] clk_prepare_complete: pclk_div
>>> [    2.714039] clk_prepare: pclk
>>> [    2.716985] clk_prepare_complete: pclk
>>> [    2.720739] clk_prepare: apbdma
>>> [    2.723833] clk_prepare_complete: apbdma
>>> [    2.727736] clk_enable: hclk_div
>>> [    2.730940] clk_enable_complete: hclk_div
>>> [    2.734926] clk_enable: hclk
>>> [    2.737788] clk_enable_complete: hclk
>>> [    2.741426] clk_enable: pclk_div
>>> [    2.744633] clk_enable_complete: pclk_div
>>> [    2.748619] clk_enable: pclk
>>> [    2.751481] clk_enable_complete: pclk
>>> [    2.755120] clk_enable: apbdma
>>> [    2.758153] clk_enable_complete: apbdma
>>> [    2.762390] clk_disable: apbdma
>>> [    2.765088] clk_disable_complete: apbdma
>>> [    2.768986] clk
>>>
>>> So I believe this change is correct and that Peter's analysis on IRC
>>> seems correct, that this change has exposed another issue with the clock
>>> driver.
>>>
>>
>> Could you please show the /sys/kernel/debug/clk/clk_summary?
> 
> With what patches present?
> 

linux-next without any additional patches

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 30/11/17 16:53, Dmitry Osipenko wrote:

...

>>> Could you please show the /sys/kernel/debug/clk/clk_summary?
>>
>> With what patches present?
>>
> linux-next without any additional patches

Here you go. This is from Tegra124 Jetson TK1 ...

   clock                         enable_cnt  prepare_cnt        rate   accuracy   phase
----------------------------------------------------------------------------------------
 dfllCPU_out                              2            2  1326000000          0 0  
    cclk_g                                1            1  1326000000          0 0  
 clock                                    0            0       32768          0 0  
 vimclk_sync                              0            0    24000000          0 0  
 i2s4_sync                                0            0    24000000          0 0  
 i2s3_sync                                0            0    24000000          0 0  
 i2s2_sync                                0            0    24000000          0 0  
 i2s1_sync                                0            0    24000000          0 0  
 i2s0_sync                                0            0    24000000          0 0  
 spdif_in_sync                            0            0    24000000          0 0  
    spdif_mux                             0            0    24000000          0 0  
       spdif                              0            0    24000000          0 0  
          spdif_doubler                   0            0    48000000          0 0  
             spdif_div                    0            0    48000000          0 0  
                spdif_2x                  0            0    48000000          0 0  
    audio4_mux                            0            0    24000000          0 0  
       audio4                             0            0    24000000          0 0  
          audio4_doubler                  0            0    48000000          0 0  
             audio4_div                   0            0    48000000          0 0  
                audio4_2x                 0            0    48000000          0 0  
    audio3_mux                            0            0    24000000          0 0  
       audio3                             0            0    24000000          0 0  
          audio3_doubler                  0            0    48000000          0 0  
             audio3_div                   0            0    48000000          0 0  
                audio3_2x                 0            0    48000000          0 0  
    audio2_mux                            0            0    24000000          0 0  
       audio2                             0            0    24000000          0 0  
          audio2_doubler                  0            0    48000000          0 0  
             audio2_div                   0            0    48000000          0 0  
                audio2_2x                 0            0    48000000          0 0  
    audio1_mux                            0            0    24000000          0 0  
       audio1                             0            0    24000000          0 0  
          audio1_doubler                  0            0    48000000          0 0  
             audio1_div                   0            0    48000000          0 0  
                audio1_2x                 0            0    48000000          0 0  
    audio0_mux                            0            0    24000000          0 0  
       audio0                             0            0    24000000          0 0  
          audio0_doubler                  0            0    48000000          0 0  
             audio0_div                   0            0    48000000          0 0  
                audio0_2x                 0            0    48000000          0 0  
 clk_32k                                  1            1       32768          0 0  
    blink_override                        0            0       32768          0 0  
       blink                              0            0       32768          0 0  
    kbc                                   0            0       32768          0 0  
    rtc                                   2            2       32768          0 0  
 osc                                      2            2    12000000          0 0  
    pll_ref                               5            5    12000000          0 0  
       pll_x                              0            0   696000000          0 0  
          pll_x_out0                      0            0   348000000          0 0  
       gpu                                0            0    12000000          0 0  
       pll_p                             16           16   408000000          0 0  
          hda2codec_2x                    1            1    48000000          0 0  
          hda                             1            1   102000000          0 0  
          sata_oob                        1            1   204000000          0 0  
          sata                            2            2   102000000          0 0  
          sbc4                            1            1    19902440          0 0  
          dfll_ref                        2            2    51000000          0 0  
          dfll_soc                        2            2    51000000          0 0  
          dsiblp                          0            0    68000000          0 0  
          dsialp                          0            0    68000000          0 0  
          host1x                          2            2   136000000          0 0  
          vde                             0            0   408000000          0 0  
          uartc                           0            0   408000000          0 0  
          uartb                           0            0   408000000          0 0  
          uarta                           0            0   408000000          0 0  
          uartd                           1            1   408000000          0 0  
          csite                           1            1   204000000          0 0  
          soc_therm                       0            0    51000000          0 0  
          sdmmc4                          1            1    51000000          0 0  
          sdmmc3                          1            1    48000000          0 0  
          spdif_in                        0            0   408000000          0 0  
          mselect                         2            2   102000000          0 0  
             afi                          1            1   102000000          0 0  
          se                              0            0   102000000          0 0  
          i2c5                            1            2    81600000          0 0  
          pll_p_out5_div                  0            0   408000000          0 0  
             pll_p_out5                   0            0   408000000          0 0  
          pll_p_out4_div                  0            0   204000000          0 0  
             pll_p_out4                   0            0   204000000          0 0  
          pll_p_out3_div                  0            0   102000000          0 0  
             pll_p_out3                   0            0   102000000          0 0  
                csi                       0            0   102000000          0 0  
                clk72mhz                  0            0   102000000          0 0  
                   mipi-cal               0            0   102000000          0 0  
                hdmi_audio                0            0   102000000          0 0  
          pll_p_out2_div                  1            1   102000000          0 0  
             pll_p_out2                   1            1   102000000          0 0  
                sclk                      2            2   102000000          0 0  
                   hclk_div               1            1    51000000          0 0  
                      hclk                1            1    51000000          0 0  
                         pclk_div           1            1    51000000          0 0  
                            pclk           2            2    51000000          0 0  
                               apbdma           1            1    51000000          0 0  
                               cec           0            0    51000000          0 0  
          pll_p_out1_div                  1            1     9600000          0 0  
             pll_p_out1                   1            1     9600000          0 0  
                pll_a                     2            2   282240000          0 0  
                   pll_a_out0_div           1            1    11289600          0 0  
                      pll_a_out0           2            2    11289600          0 0  
                         i2s4             0            0    11289600          0 0  
                         i2s3             0            0    11289600          0 0  
                         i2s2             0            0    11289600          0 0  
                         i2s1             0            0    11289600          0 0  
                         i2s0             0            0    11289600          0 0  
                         extern1           2            2    11289600          0 0  
                            clk_out_1_mux           2            2    11289600          0 0  
                               clk_out_1           1            1    11289600          0 0  
          dpaux                           0            0    24000000          0 0  
       pll_d2                             1            1   296000000          0 0  
          pll_d2_out0                     2            2   296000000          0 0  
             disp1                        1            1   296000000          0 0  
             hdmi                         1            1   296000000          0 0  
       pll_dp                             0            0    18750000          0 0  
       pll_c4                             0            0    18750000          0 0  
       pll_e                              4            4   100000000          0 0  
          cml1                            1            1   100000000          0 0  
          cml0                            1            1   100000000          0 0  
       pll_re_vco                         0            0   672000000          0 0  
          pll_re_out                      0            0   672000000          0 0  
             xusb_host_src                0            0   112000000          0 0  
                xusb_host                 0            0   112000000          0 0  
             xusb_falcon_src              0            0   224000000          0 0  
       pll_d                              0            0   925000000          0 0  
          pll_d_out0                      0            0   462500000          0 0  
             pll_d_dsi_out                0            0   462500000          0 0  
                dsib                      0            0   462500000          0 0  
                dsia                      0            0   462500000          0 0  
       pll_u                              3            3   480000000          0 0  
          pll_u_12M                       0            0    12000000          0 0  
          pll_u_48M                       0            0    48000000          0 0  
             xusb_fs_src                  0            0    48000000          0 0  
          pll_u_60M                       0            0    60000000          0 0  
             xusb_hs_src                  0            0    60000000          0 0  
          pll_u_480M                      0            0   480000000          0 0  
             xusb_ss_src                  0            0   120000000          0 0  
                xusb_ss                   0            0   120000000          0 0  
                xusb_ss_div2              0            0    60000000          0 0  
       pll_m                              1            1   924000000          0 0  
          isp                             0            0   924000000          0 0  
             ispb                         0            0   924000000          0 0  
          vi_sensor                       0            0   924000000          0 0  
          vi_sensor2                      0            0   924000000          0 0  
          vic03                           0            0   924000000          0 0  
          vi                              0            0   924000000          0 0  
          pll_m_ud                        1            1   924000000          0 0  
             emc                          1            1   924000000          0 0  
                mc                        0            0   462000000          0 0  
          pll_m_out1_div                  0            0   924000000          0 0  
             pll_m_out1                   0            0   924000000          0 0  
       pll_c3                             0            0   300000000          0 0  
          msenc                           0            0   300000000          0 0  
          tsec                            0            0   300000000          0 0  
       pll_c2                             0            0   300000000          0 0  
       pll_c                              0            0   768000000          0 0  
          pll_c_ud                        0            0   768000000          0 0  
          pll_c_out1_div                  0            0    96000000          0 0  
             pll_c_out1                   0            0    96000000          0 0  
    clk_m                                 6           10    12000000          0 0  
       tsensor                            0            0      400000          0 0  
       cclk_lp                            0            0    12000000          0 0  
       clk_out_3_mux                      0            0    12000000          0 0  
          clk_out_3                       0            0    12000000          0 0  
       clk_out_2_mux                      0            0    12000000          0 0  
          clk_out_2                       0            0    12000000          0 0  
       pcie                               1            1    12000000          0 0  
       vim2_clk                           0            0    12000000          0 0  
       sata_cold                          0            0    12000000          0 0  
       dtv                                0            0    12000000          0 0  
       dp2                                0            0    12000000          0 0  
       dds                                0            0    12000000          0 0  
       csus                               0            0    12000000          0 0  
       usb3                               1            1    12000000          0 0  
       usb2                               1            1    12000000          0 0  
       usbd                               1            1    12000000          0 0  
       bsev                               0            0    12000000          0 0  
       bsea                               0            0    12000000          0 0  
       hda2hdmi                           1            1    12000000          0 0  
       apbif                              0            0    12000000          0 0  
       kfuse                              0            0    12000000          0 0  
       fuse_burn                          0            0    12000000          0 0  
       fuse                               0            0    12000000          0 0  
       vcp                                0            0    12000000          0 0  
       timer                              2            2    12000000          0 0  
       xusb_dev_src                       0            0    12000000          0 0  
          xusb_dev                        0            0    12000000          0 0  
       sor0                               0            0    12000000          0 0  
       disp2                              0            0    12000000          0 0  
       sor0_lvds                          0            0    12000000          0 0  
       entropy                            0            0    12000000          0 0  
       extern3                            0            0    12000000          0 0  
       extern2                            0            0    12000000          0 0  
       sdmmc2                             0            0    12000000          0 0  
       sdmmc1                             0            0    12000000          0 0  
       amx1                               0            0    12000000          0 0  
       adx1                               0            0    12000000          0 0  
       sbc6                               0            0    12000000          0 0  
       sbc5                               0            0    12000000          0 0  
       sbc3                               0            0    12000000          0 0  
       sbc2                               0            0    12000000          0 0  
       sbc1                               0            0    12000000          0 0  
       i2cslow                            0            0    12000000          0 0  
       actmon                             0            0    12000000          0 0  
       cile                               0            0    12000000          0 0  
       cilcd                              0            0    12000000          0 0  
       cilab                              0            0    12000000          0 0  
       mipi                               0            0    12000000          0 0  
       nor                                0            0    12000000          0 0  
       owr                                0            0    12000000          0 0  
       trace                              0            0    12000000          0 0  
       la                                 0            0    12000000          0 0  
       vfir                               0            0    12000000          0 0  
       amx                                0            0    12000000          0 0  
       adx                                0            0    12000000          0 0  
       pwm                                0            0    12000000          0 0  
       spdif_out                          0            0    12000000          0 0  
       i2c6                               0            0    12000000          0 0  
       i2c4                               0            1    12000000          0 0  
       i2c3                               0            1    12000000          0 0  
       i2c2                               0            1    12000000          0 0  
       i2c1                               0            1    12000000          0 0  
       dam2                               0            0    12000000          0 0  
       dam1                               0            0    12000000          0 0  
       dam0                               0            0    12000000          0 0  
       d_audio                            0            0    12000000          0 0  
       clk_m_div4                         0            0     3000000          0 0  
       clk_m_div2                         0            0     6000000          0 0  
 

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Jon Hunter]

On 30/11/17 16:53, Dmitry Osipenko wrote:

...

>>> Could you please show the /sys/kernel/debug/clk/clk_summary?
>>
>> With what patches present?
>>
> linux-next without any additional patches

Here you go. This is from Tegra124 Jetson TK1 ...

   clock                         enable_cnt  prepare_cnt        rate   accuracy   phase
----------------------------------------------------------------------------------------
 dfllCPU_out                              2            2  1326000000          0 0  
    cclk_g                                1            1  1326000000          0 0  
 clock                                    0            0       32768          0 0  
 vimclk_sync                              0            0    24000000          0 0  
 i2s4_sync                                0            0    24000000          0 0  
 i2s3_sync                                0            0    24000000          0 0  
 i2s2_sync                                0            0    24000000          0 0  
 i2s1_sync                                0            0    24000000          0 0  
 i2s0_sync                                0            0    24000000          0 0  
 spdif_in_sync                            0            0    24000000          0 0  
    spdif_mux                             0            0    24000000          0 0  
       spdif                              0            0    24000000          0 0  
          spdif_doubler                   0            0    48000000          0 0  
             spdif_div                    0            0    48000000          0 0  
                spdif_2x                  0            0    48000000          0 0  
    audio4_mux                            0            0    24000000          0 0  
       audio4                             0            0    24000000          0 0  
          audio4_doubler                  0            0    48000000          0 0  
             audio4_div                   0            0    48000000          0 0  
                audio4_2x                 0            0    48000000          0 0  
    audio3_mux                            0            0    24000000          0 0  
       audio3                             0            0    24000000          0 0  
          audio3_doubler                  0            0    48000000          0 0  
             audio3_div                   0            0    48000000          0 0  
                audio3_2x                 0            0    48000000          0 0  
    audio2_mux                            0            0    24000000          0 0  
       audio2                             0            0    24000000          0 0  
          audio2_doubler                  0            0    48000000          0 0  
             audio2_div                   0            0    48000000          0 0  
                audio2_2x                 0            0    48000000          0 0  
    audio1_mux                            0            0    24000000          0 0  
       audio1                             0            0    24000000          0 0  
          audio1_doubler                  0            0    48000000          0 0  
             audio1_div                   0            0    48000000          0 0  
                audio1_2x                 0            0    48000000          0 0  
    audio0_mux                            0            0    24000000          0 0  
       audio0                             0            0    24000000          0 0  
          audio0_doubler                  0            0    48000000          0 0  
             audio0_div                   0            0    48000000          0 0  
                audio0_2x                 0            0    48000000          0 0  
 clk_32k                                  1            1       32768          0 0  
    blink_override                        0            0       32768          0 0  
       blink                              0            0       32768          0 0  
    kbc                                   0            0       32768          0 0  
    rtc                                   2            2       32768          0 0  
 osc                                      2            2    12000000          0 0  
    pll_ref                               5            5    12000000          0 0  
       pll_x                              0            0   696000000          0 0  
          pll_x_out0                      0            0   348000000          0 0  
       gpu                                0            0    12000000          0 0  
       pll_p                             16           16   408000000          0 0  
          hda2codec_2x                    1            1    48000000          0 0  
          hda                             1            1   102000000          0 0  
          sata_oob                        1            1   204000000          0 0  
          sata                            2            2   102000000          0 0  
          sbc4                            1            1    19902440          0 0  
          dfll_ref                        2            2    51000000          0 0  
          dfll_soc                        2            2    51000000          0 0  
          dsiblp                          0            0    68000000          0 0  
          dsialp                          0            0    68000000          0 0  
          host1x                          2            2   136000000          0 0  
          vde                             0            0   408000000          0 0  
          uartc                           0            0   408000000          0 0  
          uartb                           0            0   408000000          0 0  
          uarta                           0            0   408000000          0 0  
          uartd                           1            1   408000000          0 0  
          csite                           1            1   204000000          0 0  
          soc_therm                       0            0    51000000          0 0  
          sdmmc4                          1            1    51000000          0 0  
          sdmmc3                          1            1    48000000          0 0  
          spdif_in                        0            0   408000000          0 0  
          mselect                         2            2   102000000          0 0  
             afi                          1            1   102000000          0 0  
          se                              0            0   102000000          0 0  
          i2c5                            1            2    81600000          0 0  
          pll_p_out5_div                  0            0   408000000          0 0  
             pll_p_out5                   0            0   408000000          0 0  
          pll_p_out4_div                  0            0   204000000          0 0  
             pll_p_out4                   0            0   204000000          0 0  
          pll_p_out3_div                  0            0   102000000          0 0  
             pll_p_out3                   0            0   102000000          0 0  
                csi                       0            0   102000000          0 0  
                clk72mhz                  0            0   102000000          0 0  
                   mipi-cal               0            0   102000000          0 0  
                hdmi_audio                0            0   102000000          0 0  
          pll_p_out2_div                  1            1   102000000          0 0  
             pll_p_out2                   1            1   102000000          0 0  
                sclk                      2            2   102000000          0 0  
                   hclk_div               1            1    51000000          0 0  
                      hclk                1            1    51000000          0 0  
                         pclk_div           1            1    51000000          0 0  
                            pclk           2            2    51000000          0 0  
                               apbdma           1            1    51000000          0 0  
                               cec           0            0    51000000          0 0  
          pll_p_out1_div                  1            1     9600000          0 0  
             pll_p_out1                   1            1     9600000          0 0  
                pll_a                     2            2   282240000          0 0  
                   pll_a_out0_div           1            1    11289600          0 0  
                      pll_a_out0           2            2    11289600          0 0  
                         i2s4             0            0    11289600          0 0  
                         i2s3             0            0    11289600          0 0  
                         i2s2             0            0    11289600          0 0  
                         i2s1             0            0    11289600          0 0  
                         i2s0             0            0    11289600          0 0  
                         extern1           2            2    11289600          0 0  
                            clk_out_1_mux           2            2    11289600          0 0  
                               clk_out_1           1            1    11289600          0 0  
          dpaux                           0            0    24000000          0 0  
       pll_d2                             1            1   296000000          0 0  
          pll_d2_out0                     2            2   296000000          0 0  
             disp1                        1            1   296000000          0 0  
             hdmi                         1            1   296000000          0 0  
       pll_dp                             0            0    18750000          0 0  
       pll_c4                             0            0    18750000          0 0  
       pll_e                              4            4   100000000          0 0  
          cml1                            1            1   100000000          0 0  
          cml0                            1            1   100000000          0 0  
       pll_re_vco                         0            0   672000000          0 0  
          pll_re_out                      0            0   672000000          0 0  
             xusb_host_src                0            0   112000000          0 0  
                xusb_host                 0            0   112000000          0 0  
             xusb_falcon_src              0            0   224000000          0 0  
       pll_d                              0            0   925000000          0 0  
          pll_d_out0                      0            0   462500000          0 0  
             pll_d_dsi_out                0            0   462500000          0 0  
                dsib                      0            0   462500000          0 0  
                dsia                      0            0   462500000          0 0  
       pll_u                              3            3   480000000          0 0  
          pll_u_12M                       0            0    12000000          0 0  
          pll_u_48M                       0            0    48000000          0 0  
             xusb_fs_src                  0            0    48000000          0 0  
          pll_u_60M                       0            0    60000000          0 0  
             xusb_hs_src                  0            0    60000000          0 0  
          pll_u_480M                      0            0   480000000          0 0  
             xusb_ss_src                  0            0   120000000          0 0  
                xusb_ss                   0            0   120000000          0 0  
                xusb_ss_div2              0            0    60000000          0 0  
       pll_m                              1            1   924000000          0 0  
          isp                             0            0   924000000          0 0  
             ispb                         0            0   924000000          0 0  
          vi_sensor                       0            0   924000000          0 0  
          vi_sensor2                      0            0   924000000          0 0  
          vic03                           0            0   924000000          0 0  
          vi                              0            0   924000000          0 0  
          pll_m_ud                        1            1   924000000          0 0  
             emc                          1            1   924000000          0 0  
                mc                        0            0   462000000          0 0  
          pll_m_out1_div                  0            0   924000000          0 0  
             pll_m_out1                   0            0   924000000          0 0  
       pll_c3                             0            0   300000000          0 0  
          msenc                           0            0   300000000          0 0  
          tsec                            0            0   300000000          0 0  
       pll_c2                             0            0   300000000          0 0  
       pll_c                              0            0   768000000          0 0  
          pll_c_ud                        0            0   768000000          0 0  
          pll_c_out1_div                  0            0    96000000          0 0  
             pll_c_out1                   0            0    96000000          0 0  
    clk_m                                 6           10    12000000          0 0  
       tsensor                            0            0      400000          0 0  
       cclk_lp                            0            0    12000000          0 0  
       clk_out_3_mux                      0            0    12000000          0 0  
          clk_out_3                       0            0    12000000          0 0  
       clk_out_2_mux                      0            0    12000000          0 0  
          clk_out_2                       0            0    12000000          0 0  
       pcie                               1            1    12000000          0 0  
       vim2_clk                           0            0    12000000          0 0  
       sata_cold                          0            0    12000000          0 0  
       dtv                                0            0    12000000          0 0  
       dp2                                0            0    12000000          0 0  
       dds                                0            0    12000000          0 0  
       csus                               0            0    12000000          0 0  
       usb3                               1            1    12000000          0 0  
       usb2                               1            1    12000000          0 0  
       usbd                               1            1    12000000          0 0  
       bsev                               0            0    12000000          0 0  
       bsea                               0            0    12000000          0 0  
       hda2hdmi                           1            1    12000000          0 0  
       apbif                              0            0    12000000          0 0  
       kfuse                              0            0    12000000          0 0  
       fuse_burn                          0            0    12000000          0 0  
       fuse                               0            0    12000000          0 0  
       vcp                                0            0    12000000          0 0  
       timer                              2            2    12000000          0 0  
       xusb_dev_src                       0            0    12000000          0 0  
          xusb_dev                        0            0    12000000          0 0  
       sor0                               0            0    12000000          0 0  
       disp2                              0            0    12000000          0 0  
       sor0_lvds                          0            0    12000000          0 0  
       entropy                            0            0    12000000          0 0  
       extern3                            0            0    12000000          0 0  
       extern2                            0            0    12000000          0 0  
       sdmmc2                             0            0    12000000          0 0  
       sdmmc1                             0            0    12000000          0 0  
       amx1                               0            0    12000000          0 0  
       adx1                               0            0    12000000          0 0  
       sbc6                               0            0    12000000          0 0  
       sbc5                               0            0    12000000          0 0  
       sbc3                               0            0    12000000          0 0  
       sbc2                               0            0    12000000          0 0  
       sbc1                               0            0    12000000          0 0  
       i2cslow                            0            0    12000000          0 0  
       actmon                             0            0    12000000          0 0  
       cile                               0            0    12000000          0 0  
       cilcd                              0            0    12000000          0 0  
       cilab                              0            0    12000000          0 0  
       mipi                               0            0    12000000          0 0  
       nor                                0            0    12000000          0 0  
       owr                                0            0    12000000          0 0  
       trace                              0            0    12000000          0 0  
       la                                 0            0    12000000          0 0  
       vfir                               0            0    12000000          0 0  
       amx                                0            0    12000000          0 0  
       adx                                0            0    12000000          0 0  
       pwm                                0            0    12000000          0 0  
       spdif_out                          0            0    12000000          0 0  
       i2c6                               0            0    12000000          0 0  
       i2c4                               0            1    12000000          0 0  
       i2c3                               0            1    12000000          0 0  
       i2c2                               0            1    12000000          0 0  
       i2c1                               0            1    12000000          0 0  
       dam2                               0            0    12000000          0 0  
       dam1                               0            0    12000000          0 0  
       dam0                               0            0    12000000          0 0  
       d_audio                            0            0    12000000          0 0  
       clk_m_div4                         0            0     3000000          0 0  
       clk_m_div2                         0            0     6000000          0 0  
 

-- 
nvpublic

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 30.11.2017 20:22, Jon Hunter wrote:
> 
> On 30/11/17 16:53, Dmitry Osipenko wrote:
> 
> ...
> 
>>>> Could you please show the /sys/kernel/debug/clk/clk_summary?
>>>
>>> With what patches present?
>>>
>> linux-next without any additional patches
> 
> Here you go. This is from Tegra124 Jetson TK1 ...
> 

[snip]

Thanks, looks fine. Yeah, PCLK definitely must be critical, along with EMC.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Peter De Schrijver]

On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote:
> 
> On 30/11/17 13:24, Dmitry Osipenko wrote:
> > On 30.11.2017 14:31, Jon Hunter wrote:
> >>
> >> On 29/11/17 23:13, Dmitry Osipenko wrote:
> >>> On 30.11.2017 01:55, Jon Hunter wrote:
> >>
> >> ...
> >>
> >>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
> >>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
> >>> caused trouble on Tegra114/124.
> >>
> >> Please share the exact change you would like me to test and I will.
> > 
> > Please try this:
> 
> I tried the patch, but this does not work for Tegra124 it still hangs.
> Tracing the clk calls the last thing I see is ...
> 
> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
> [    2.694403] clk_prepare: hclk_div
> [    2.695929] clk_prepare_complete: hclk_div
> [    2.700027] clk_prepare: hclk
> [    2.702947] clk_prepare_complete: hclk
> [    2.706673] clk_prepare: pclk_div
> [    2.709986] clk_prepare_complete: pclk_div
> [    2.714039] clk_prepare: pclk
> [    2.716985] clk_prepare_complete: pclk
> [    2.720739] clk_prepare: apbdma
> [    2.723833] clk_prepare_complete: apbdma
> [    2.727736] clk_enable: hclk_div
> [    2.730940] clk_enable_complete: hclk_div
> [    2.734926] clk_enable: hclk
> [    2.737788] clk_enable_complete: hclk
> [    2.741426] clk_enable: pclk_div
> [    2.744633] clk_enable_complete: pclk_div
> [    2.748619] clk_enable: pclk
> [    2.751481] clk_enable_complete: pclk
> [    2.755120] clk_enable: apbdma
> [    2.758153] clk_enable_complete: apbdma
> [    2.762390] clk_disable: apbdma
> [    2.765088] clk_disable_complete: apbdma
> [    2.768986] clk
> 
> So I believe this change is correct and that Peter's analysis on IRC
> seems correct, that this change has exposed another issue with the clock
> driver.
> 
> Peter, can you summarise which other clocks should be made critical?

The following clocks should be critical:

pclk
hclk
sclk
emc
mc
pll_p

I think that's it.

Peter.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Peter De Schrijver]

On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote:
> 
> On 30/11/17 13:24, Dmitry Osipenko wrote:
> > On 30.11.2017 14:31, Jon Hunter wrote:
> >>
> >> On 29/11/17 23:13, Dmitry Osipenko wrote:
> >>> On 30.11.2017 01:55, Jon Hunter wrote:
> >>
> >> ...
> >>
> >>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
> >>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
> >>> caused trouble on Tegra114/124.
> >>
> >> Please share the exact change you would like me to test and I will.
> > 
> > Please try this:
> 
> I tried the patch, but this does not work for Tegra124 it still hangs.
> Tracing the clk calls the last thing I see is ...
> 
> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
> [    2.694403] clk_prepare: hclk_div
> [    2.695929] clk_prepare_complete: hclk_div
> [    2.700027] clk_prepare: hclk
> [    2.702947] clk_prepare_complete: hclk
> [    2.706673] clk_prepare: pclk_div
> [    2.709986] clk_prepare_complete: pclk_div
> [    2.714039] clk_prepare: pclk
> [    2.716985] clk_prepare_complete: pclk
> [    2.720739] clk_prepare: apbdma
> [    2.723833] clk_prepare_complete: apbdma
> [    2.727736] clk_enable: hclk_div
> [    2.730940] clk_enable_complete: hclk_div
> [    2.734926] clk_enable: hclk
> [    2.737788] clk_enable_complete: hclk
> [    2.741426] clk_enable: pclk_div
> [    2.744633] clk_enable_complete: pclk_div
> [    2.748619] clk_enable: pclk
> [    2.751481] clk_enable_complete: pclk
> [    2.755120] clk_enable: apbdma
> [    2.758153] clk_enable_complete: apbdma
> [    2.762390] clk_disable: apbdma
> [    2.765088] clk_disable_complete: apbdma
> [    2.768986] clk
> 
> So I believe this change is correct and that Peter's analysis on IRC
> seems correct, that this change has exposed another issue with the clock
> driver.
> 
> Peter, can you summarise which other clocks should be made critical?

The following clocks should be critical:

pclk
hclk
sclk
emc
mc
pll_p

I think that's it.

Peter.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 01.12.2017 11:48, Peter De Schrijver wrote:
> On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote:
>>
>> On 30/11/17 13:24, Dmitry Osipenko wrote:
>>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>>
>>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>>
>>>> ...
>>>>
>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>>> caused trouble on Tegra114/124.
>>>>
>>>> Please share the exact change you would like me to test and I will.
>>>
>>> Please try this:
>>
>> I tried the patch, but this does not work for Tegra124 it still hangs.
>> Tracing the clk calls the last thing I see is ...
>>
>> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
>> [    2.694403] clk_prepare: hclk_div
>> [    2.695929] clk_prepare_complete: hclk_div
>> [    2.700027] clk_prepare: hclk
>> [    2.702947] clk_prepare_complete: hclk
>> [    2.706673] clk_prepare: pclk_div
>> [    2.709986] clk_prepare_complete: pclk_div
>> [    2.714039] clk_prepare: pclk
>> [    2.716985] clk_prepare_complete: pclk
>> [    2.720739] clk_prepare: apbdma
>> [    2.723833] clk_prepare_complete: apbdma
>> [    2.727736] clk_enable: hclk_div
>> [    2.730940] clk_enable_complete: hclk_div
>> [    2.734926] clk_enable: hclk
>> [    2.737788] clk_enable_complete: hclk
>> [    2.741426] clk_enable: pclk_div
>> [    2.744633] clk_enable_complete: pclk_div
>> [    2.748619] clk_enable: pclk
>> [    2.751481] clk_enable_complete: pclk
>> [    2.755120] clk_enable: apbdma
>> [    2.758153] clk_enable_complete: apbdma
>> [    2.762390] clk_disable: apbdma
>> [    2.765088] clk_disable_complete: apbdma
>> [    2.768986] clk
>>
>> So I believe this change is correct and that Peter's analysis on IRC
>> seems correct, that this change has exposed another issue with the clock
>> driver.
>>
>> Peter, can you summarise which other clocks should be made critical?
> 
> The following clocks should be critical:
> 
> pclk
> hclk
> sclk
> emc
> mc
> pll_p
> 
> I think that's it.

Could you please elaborate why pll_p should be critical? Is it applicable to all
Tegra generations?

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Peter De Schrijver]

On Sat, Dec 02, 2017 at 03:47:32PM +0300, Dmitry Osipenko wrote:
> On 01.12.2017 11:48, Peter De Schrijver wrote:
> > On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote:
> >>
> >> On 30/11/17 13:24, Dmitry Osipenko wrote:
> >>> On 30.11.2017 14:31, Jon Hunter wrote:
> >>>>
> >>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
> >>>>> On 30.11.2017 01:55, Jon Hunter wrote:
> >>>>
> >>>> ...
> >>>>
> >>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
> >>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
> >>>>> caused trouble on Tegra114/124.
> >>>>
> >>>> Please share the exact change you would like me to test and I will.
> >>>
> >>> Please try this:
> >>
> >> I tried the patch, but this does not work for Tegra124 it still hangs.
> >> Tracing the clk calls the last thing I see is ...
> >>
> >> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
> >> [    2.694403] clk_prepare: hclk_div
> >> [    2.695929] clk_prepare_complete: hclk_div
> >> [    2.700027] clk_prepare: hclk
> >> [    2.702947] clk_prepare_complete: hclk
> >> [    2.706673] clk_prepare: pclk_div
> >> [    2.709986] clk_prepare_complete: pclk_div
> >> [    2.714039] clk_prepare: pclk
> >> [    2.716985] clk_prepare_complete: pclk
> >> [    2.720739] clk_prepare: apbdma
> >> [    2.723833] clk_prepare_complete: apbdma
> >> [    2.727736] clk_enable: hclk_div
> >> [    2.730940] clk_enable_complete: hclk_div
> >> [    2.734926] clk_enable: hclk
> >> [    2.737788] clk_enable_complete: hclk
> >> [    2.741426] clk_enable: pclk_div
> >> [    2.744633] clk_enable_complete: pclk_div
> >> [    2.748619] clk_enable: pclk
> >> [    2.751481] clk_enable_complete: pclk
> >> [    2.755120] clk_enable: apbdma
> >> [    2.758153] clk_enable_complete: apbdma
> >> [    2.762390] clk_disable: apbdma
> >> [    2.765088] clk_disable_complete: apbdma
> >> [    2.768986] clk
> >>
> >> So I believe this change is correct and that Peter's analysis on IRC
> >> seems correct, that this change has exposed another issue with the clock
> >> driver.
> >>
> >> Peter, can you summarise which other clocks should be made critical?
> > 
> > The following clocks should be critical:
> > 
> > pclk
> > hclk
> > sclk
> > emc
> > mc
> > pll_p
> > 
> > I think that's it.
> 
> Could you please elaborate why pll_p should be critical? Is it applicable to all
> Tegra generations?

There are quite many clocks relying on pll_p as their source. In theory we
might get away with making sure all thsoe clocks are critical if needed, but
it's usually easier to just make sure we never turn off pll_p. We should
certainly never try to change the rate of pll_p. Obviously also osc and clk32k
should never be turned off during runtime, but there is no hw which allows you
to do this.

Peter.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Peter De Schrijver]

On Sat, Dec 02, 2017 at 03:47:32PM +0300, Dmitry Osipenko wrote:
> On 01.12.2017 11:48, Peter De Schrijver wrote:
> > On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote:
> >>
> >> On 30/11/17 13:24, Dmitry Osipenko wrote:
> >>> On 30.11.2017 14:31, Jon Hunter wrote:
> >>>>
> >>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
> >>>>> On 30.11.2017 01:55, Jon Hunter wrote:
> >>>>
> >>>> ...
> >>>>
> >>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
> >>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
> >>>>> caused trouble on Tegra114/124.
> >>>>
> >>>> Please share the exact change you would like me to test and I will.
> >>>
> >>> Please try this:
> >>
> >> I tried the patch, but this does not work for Tegra124 it still hangs.
> >> Tracing the clk calls the last thing I see is ...
> >>
> >> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
> >> [    2.694403] clk_prepare: hclk_div
> >> [    2.695929] clk_prepare_complete: hclk_div
> >> [    2.700027] clk_prepare: hclk
> >> [    2.702947] clk_prepare_complete: hclk
> >> [    2.706673] clk_prepare: pclk_div
> >> [    2.709986] clk_prepare_complete: pclk_div
> >> [    2.714039] clk_prepare: pclk
> >> [    2.716985] clk_prepare_complete: pclk
> >> [    2.720739] clk_prepare: apbdma
> >> [    2.723833] clk_prepare_complete: apbdma
> >> [    2.727736] clk_enable: hclk_div
> >> [    2.730940] clk_enable_complete: hclk_div
> >> [    2.734926] clk_enable: hclk
> >> [    2.737788] clk_enable_complete: hclk
> >> [    2.741426] clk_enable: pclk_div
> >> [    2.744633] clk_enable_complete: pclk_div
> >> [    2.748619] clk_enable: pclk
> >> [    2.751481] clk_enable_complete: pclk
> >> [    2.755120] clk_enable: apbdma
> >> [    2.758153] clk_enable_complete: apbdma
> >> [    2.762390] clk_disable: apbdma
> >> [    2.765088] clk_disable_complete: apbdma
> >> [    2.768986] clk
> >>
> >> So I believe this change is correct and that Peter's analysis on IRC
> >> seems correct, that this change has exposed another issue with the clock
> >> driver.
> >>
> >> Peter, can you summarise which other clocks should be made critical?
> > 
> > The following clocks should be critical:
> > 
> > pclk
> > hclk
> > sclk
> > emc
> > mc
> > pll_p
> > 
> > I think that's it.
> 
> Could you please elaborate why pll_p should be critical? Is it applicable to all
> Tegra generations?

There are quite many clocks relying on pll_p as their source. In theory we
might get away with making sure all thsoe clocks are critical if needed, but
it's usually easier to just make sure we never turn off pll_p. We should
certainly never try to change the rate of pll_p. Obviously also osc and clk32k
should never be turned off during runtime, but there is no hw which allows you
to do this.

Peter.

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 05.12.2017 12:06, Peter De Schrijver wrote:
> On Sat, Dec 02, 2017 at 03:47:32PM +0300, Dmitry Osipenko wrote:
>> On 01.12.2017 11:48, Peter De Schrijver wrote:
>>> On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote:
>>>>
>>>> On 30/11/17 13:24, Dmitry Osipenko wrote:
>>>>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>>>>
>>>>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>>>>> caused trouble on Tegra114/124.
>>>>>>
>>>>>> Please share the exact change you would like me to test and I will.
>>>>>
>>>>> Please try this:
>>>>
>>>> I tried the patch, but this does not work for Tegra124 it still hangs.
>>>> Tracing the clk calls the last thing I see is ...
>>>>
>>>> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
>>>> [    2.694403] clk_prepare: hclk_div
>>>> [    2.695929] clk_prepare_complete: hclk_div
>>>> [    2.700027] clk_prepare: hclk
>>>> [    2.702947] clk_prepare_complete: hclk
>>>> [    2.706673] clk_prepare: pclk_div
>>>> [    2.709986] clk_prepare_complete: pclk_div
>>>> [    2.714039] clk_prepare: pclk
>>>> [    2.716985] clk_prepare_complete: pclk
>>>> [    2.720739] clk_prepare: apbdma
>>>> [    2.723833] clk_prepare_complete: apbdma
>>>> [    2.727736] clk_enable: hclk_div
>>>> [    2.730940] clk_enable_complete: hclk_div
>>>> [    2.734926] clk_enable: hclk
>>>> [    2.737788] clk_enable_complete: hclk
>>>> [    2.741426] clk_enable: pclk_div
>>>> [    2.744633] clk_enable_complete: pclk_div
>>>> [    2.748619] clk_enable: pclk
>>>> [    2.751481] clk_enable_complete: pclk
>>>> [    2.755120] clk_enable: apbdma
>>>> [    2.758153] clk_enable_complete: apbdma
>>>> [    2.762390] clk_disable: apbdma
>>>> [    2.765088] clk_disable_complete: apbdma
>>>> [    2.768986] clk
>>>>
>>>> So I believe this change is correct and that Peter's analysis on IRC
>>>> seems correct, that this change has exposed another issue with the clock
>>>> driver.
>>>>
>>>> Peter, can you summarise which other clocks should be made critical?
>>>
>>> The following clocks should be critical:
>>>
>>> pclk
>>> hclk
>>> sclk
>>> emc
>>> mc
>>> pll_p
>>>
>>> I think that's it.
>>
>> Could you please elaborate why pll_p should be critical? Is it applicable to all
>> Tegra generations?
> 
> There are quite many clocks relying on pll_p as their source. In theory we
> might get away with making sure all thsoe clocks are critical if needed, but
> it's usually easier to just make sure we never turn off pll_p. We should
> certainly never try to change the rate of pll_p. Obviously also osc and clk32k
> should never be turned off during runtime, but there is no hw which allows you
> to do this.
Okay, I see it is explicitly documented in TRM that "modules take in an
additional fix PLL clock source for a portion of their logic" and indeed most of
periperals use pll_p and its dividers for the "portion of logic".

Re: [PATCH] clk: tegra: Mark APB clock as critical

[Dmitry Osipenko]

On 05.12.2017 12:06, Peter De Schrijver wrote:
> On Sat, Dec 02, 2017 at 03:47:32PM +0300, Dmitry Osipenko wrote:
>> On 01.12.2017 11:48, Peter De Schrijver wrote:
>>> On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote:
>>>>
>>>> On 30/11/17 13:24, Dmitry Osipenko wrote:
>>>>> On 30.11.2017 14:31, Jon Hunter wrote:
>>>>>>
>>>>>> On 29/11/17 23:13, Dmitry Osipenko wrote:
>>>>>>> On 30.11.2017 01:55, Jon Hunter wrote:
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the
>>>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually
>>>>>>> caused trouble on Tegra114/124.
>>>>>>
>>>>>> Please share the exact change you would like me to test and I will.
>>>>>
>>>>> Please try this:
>>>>
>>>> I tried the patch, but this does not work for Tegra124 it still hangs.
>>>> Tracing the clk calls the last thing I see is ...
>>>>
>>>> [    2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator
>>>> [    2.694403] clk_prepare: hclk_div
>>>> [    2.695929] clk_prepare_complete: hclk_div
>>>> [    2.700027] clk_prepare: hclk
>>>> [    2.702947] clk_prepare_complete: hclk
>>>> [    2.706673] clk_prepare: pclk_div
>>>> [    2.709986] clk_prepare_complete: pclk_div
>>>> [    2.714039] clk_prepare: pclk
>>>> [    2.716985] clk_prepare_complete: pclk
>>>> [    2.720739] clk_prepare: apbdma
>>>> [    2.723833] clk_prepare_complete: apbdma
>>>> [    2.727736] clk_enable: hclk_div
>>>> [    2.730940] clk_enable_complete: hclk_div
>>>> [    2.734926] clk_enable: hclk
>>>> [    2.737788] clk_enable_complete: hclk
>>>> [    2.741426] clk_enable: pclk_div
>>>> [    2.744633] clk_enable_complete: pclk_div
>>>> [    2.748619] clk_enable: pclk
>>>> [    2.751481] clk_enable_complete: pclk
>>>> [    2.755120] clk_enable: apbdma
>>>> [    2.758153] clk_enable_complete: apbdma
>>>> [    2.762390] clk_disable: apbdma
>>>> [    2.765088] clk_disable_complete: apbdma
>>>> [    2.768986] clk
>>>>
>>>> So I believe this change is correct and that Peter's analysis on IRC
>>>> seems correct, that this change has exposed another issue with the clock
>>>> driver.
>>>>
>>>> Peter, can you summarise which other clocks should be made critical?
>>>
>>> The following clocks should be critical:
>>>
>>> pclk
>>> hclk
>>> sclk
>>> emc
>>> mc
>>> pll_p
>>>
>>> I think that's it.
>>
>> Could you please elaborate why pll_p should be critical? Is it applicable to all
>> Tegra generations?
> 
> There are quite many clocks relying on pll_p as their source. In theory we
> might get away with making sure all thsoe clocks are critical if needed, but
> it's usually easier to just make sure we never turn off pll_p. We should
> certainly never try to change the rate of pll_p. Obviously also osc and clk32k
> should never be turned off during runtime, but there is no hw which allows you
> to do this.
Okay, I see it is explicitly documented in TRM that "modules take in an
additional fix PLL clock source for a portion of their logic" and indeed most of
periperals use pll_p and its dividers for the "portion of logic".