Message ID | 1508757172-13030-1-git-send-email-jonathanh@nvidia.com (mailing list archive) |
---|---|
State | Awaiting Upstream, archived |
Headers | show |
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-clk" 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-clk" 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-clk" 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-clk" 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-clk" 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
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(-)