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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Dec 2018 13:53:49 +0000
From:   Anson Huang <anson.huang@....com>
To:     Lucas Stach <l.stach@...gutronix.de>
CC:     Fabio Estevam <festevam@...il.com>,
        "sboyd@...nel.org" <sboyd@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] clk: imx: add CLK_GET_RATE_NOCACHE flag for i.MX8M
 composite clock

Hi, Lucas

From Anson's iPhone 6


> 在 2018年12月18日，21:41，Lucas Stach <l.stach@...gutronix.de> 写道：
> 
> 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@...gutronix.de> 写道：
>>> 
>>> 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@....com
>>>>> 
>>>> 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