| Message ID | 1508757172-13030-1-git-send-email-jonathanh@nvidia.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | clk: tegra: Mark APB clock as critical | expand |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 }, -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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? > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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". -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --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; }
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(-)