Hi, Stephen

Anson Huang
Best Regards!

-----Original Message-----
From: Stephen Boyd <sboyd@kernel.org>
Sent: Tuesday, October 16, 2018 12:46 AM
To: kernel at pengutronix.de; linux-arm-kernel at lists.infradead.org;
linux-clk at vger.kernel.org; linux-kernel at vger.kernel.org;
mturquette at baylibre.com; s.hauer at pengutronix.de; shawnguo at kernel.org;
Anson Huang [off-list ref]; Fabio Estevam
[off-list ref]; Jerome Forissier [off-list ref];
Peng Fan [off-list ref]; Rob Herring [off-list ref]
Cc: dl-linux-imx <redacted>
Subject: RE: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array

Quoting Anson Huang (2018-10-15 02:33:35)

quoted

quoted

quoted

quoted

quoted

Why can't we add clks to the op-tee node in DT's /firmware container?
Then any clks in there can be turned on forever and left enabled
by the linux driver?

I did NOT run op-tee with Linux-next kernel before, can you advise more?

Neither have I, so I can't advise more.

quoted

And I think if op-tee has such requirement, can we have another
patch to cover it?

Yes.

quoted

I believe all other i.MX platforms also have same requirements if
considering op-tee support, so I think it should be another topic,
what do you

think?

quoted

I'm going to drop these patches from my review queue. Please resend
them and please include the op-tee patches too.

I do NOT know how to include the op-tee patch to meet special
requirement, should the op-tee related patch be added later when someone

actually add the op-tee support for i.MX7?

quoted

It should NOT block this patch set, do you think we can add this patch set

first?

quoted

Please resend the two patches. In the commit text for the second patch,
describe the plan to remove CLK_IS_CRITICAL from these clks by adding an
OP-TEE device/driver to the kernel to keep these clks enabled. My
understanding is that there isn't an OP-TEE driver right now, but these clks are
used by the firmware and can't be turned off in Linux. If in the future we want
to be able to turn them on and off, we'll need to add them to an OP-TEE device
node and have that driver manage the clks.

How that will work when a system doesn't enable the OP-TEE driver I'm not
sure. We may need to develop some system whereby clks like this are handed
from the clk controller to the consumer driver when it's enabled for further
power managment, but if they're never handed off, they're kept on forever like
is done here. Anyway, please resend with a note about why these are marked
CLK_IS_CRITICAL.

 
I think there is some misunderstanding here, this patch sets those critical clocks
with CLK_IS_CRITICAL flag to let clock driver keep them always ON, as they are
critical clocks and system (Linux kernel, NOT include op-tee) can NOT run normally
if anyone of them is OFF.

The question about op-tee is, there might be some special clocks needed by op-tee,
for example, currently clk A, B and C are kept always ON for Linux kernel to run normally,
but when op-tee is executing, it might need clk D to be ON, so I think the clk D is needed
ONLY for op-tee, op-tee should have a driver or firmware to runtime ON/OFF clk D as needed.
Since I do NOT know what clocks op-tee needs, so I will leave them to be added by op-tee
owner, whoever enables op-tee, he should consider where to manage its special clocks,
either in clk driver or op-tee driver/firmware.

So do we still need to mention the op-tee related info in commit text? Actually, this patch
is just to replace those always-ON clocks in clks_init_on array with CLK_IS_CRITICAL flag.
Then no need to explicitly enable clocks in clks_init_on array and just rely on common clk
framework which will enable all clocks with CLK_IS_CRITICAL flag set, it will align with all
other i.MX SoCs clk driver.

Anson.