Re: [PATCH] clk: imx: add CLK_GET_RATE_NOCACHE flag for i.MX8M composite clock

[Anson Huang]

Hi, Lucas

From Anson's iPhone 6


> å 2018å12æ18æï21:41ïLucas Stach <l.stach@xxxxxxxxxxxxxx> åéï
> 
> Hi Anson,
> 
> Am Dienstag, den 18.12.2018, 13:35 +0000 schrieb Anson Huang:
>> Hi, Lucas
>> 
>> From Anson's iPhone 6
>> 
>> 
>>> å 2018å12æ18æï18:40ïLucas Stach <l.stach@xxxxxxxxxxxxxx> åéï
>>> 
>>> Am Dienstag, den 18.12.2018, 08:24 -0200 schrieb Fabio Estevam:
>>>> Hi Anson,
>>>> 
>>>> On Tue, Dec 18, 2018 at 12:56 AM Anson Huang <anson.huang@xxxxxxx
>>>>> 
>>>> wrote:
>>>>> 
>>>>> On i.MX8M, some of the bus clocks' rate could be changed in TF-
>>>>> A,
>>>> 
>>>> Do you mean ATF (ARM Trusted Firmware) instead?
>>> 
>>> TF-A is the name of the day for what was formerly known as ATF...
>>> 
>>> However I don't think that it's correct to just don't cache the
>>> clock
>>> settings. Normally the secure world firmware should not change any
>>> clock settings at runtime, or it would run into all kinds of
>>> conflicts
>>> with the clock driver. So there are probably some well known points
>>> in
>>> time like a suspend or resume event when the firmware might change
>>> clock settings, so we could instead use those to trigger an
>>> explicit
>>> invalidate of the clock caches with much lower overhead.
>>> 
>>> Regards,
>>> Lucas
>> 
>> There is bus-freq feature on imx8m which is to scale ddr clock, this
>> is done in ARM Trusted Firmware, for some setpoints, the DDR PLL
>> clock rate must be changed directly in TF-A, but its child clock like
>> dram core is unaware in Linux kernel, so the clock rate will mismatch
>> with hardware, since ddr related clocks will NOT used by any module
>> in Linux kernel, so it will NOT introduce any conflict.
> 
> I don't think there is anything implementing the bus frequency scaling
> in mainline, right?

Yes, mainline has no bus-freq scaling so far, but internally we use same composite clock driver as mainline, and bus-freq clock rate issue/bug reported during our internal test, that is why I create this patch to easy our next kernel upgrade.

> 
>> Regarding about the over head, yes, the change in common composite
>> clock register has too many over head for other clocks, what if I
>> ONLY have dram core clock to pass the CLK_GET_RATE_NOCACHE flag to
>> register the composite clock?
> 
> IMHO marking clocks under TF-A control explicitly as nocache would be
> much more acceptable than doing it for every composite clock. This
> seems okay for a short term solution.
> 
> Still I think that whatever is causing the bus frequency scale to
> change should have a way to explicitly invalidate the clock cache for
> the affected clocks eventually.

It is because the DDR PLL/clocks can only be changed with strict DDR freq change flow, and it is done in TF-A, Linux kernel can NOT touch it in runtime, so we have to mark the child clock of DDR PLL to be uncached, in V2 patch, I will just add the flag for the DDR PLL child clocks to be a shorten solution, should be only very few ones, hope it is acceptable, thanks.

Anson.


> 
> Regards,
> Lucas