Hi, Stephen

Anson Huang
Best Regards!

-----Original Message-----
From: Stephen Boyd <sboyd@kernel.org>
Sent: Saturday, October 13, 2018 3:48 AM
To: kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org;
linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org;
mturquette@baylibre.com; s.hauer@pengutronix.de; shawnguo@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-08 01:34:59)

quoted

quoted

Quoting Anson Huang (2018-09-03 00:20:53)

quoted

quoted

quoted

On 08/31/2018 03:29 AM, Stephen Boyd wrote:

quoted

Quoting Peng Fan (2018-08-12 18:15:41)

quoted

Hi Anson,

quoted

quoted

quoted

-----Original Message-----
From: Anson Huang
Sent: 2018年8月8日 12:39
To: shawnguo@kernel.org; s.hauer@pengutronix.de;
kernel@pengutronix.de; Fabio Estevam
[off-list ref]; mturquette@baylibre.com;
sboyd@kernel.org; linux-arm-kernel@lists.infradead.org;
linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: dl-linux-imx <redacted>
Subject: [PATCH 2/2] clk: imx: imx7d: remove
clks_init_on array

Clock framework will enable those clocks registered with
CLK_IS_CRITICAL flag, so no need to have clks_init_on
array during clock

initialization now.

Will it be more flexible to parse dts saying "critical-clocks =

<xxx>"

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

or "init-on-arrary=<xxx>"
and enable those clocks?

Parsing the clocks arrays from dtb is another way of
enabling critical clocks, but for current i.MX6/7
platforms, we implement it in same way as most of other
SoCs, currently I did NOT see any necessity of putting
them in dtb, just adding flag during clock registering is
more simple, if there is any special requirement for
different clocks set to be enabled, then we can add
support to enable

the method of parsing critical-clocks from dtb. Just my two cents.

quoted

quoted

quoted

Thinking about OP-TEE want to use one device, but it's
clocks are registered by Linux, because there is no module
in Linux side use it, it will shutdown the clock, which
cause OP-TEE could not access the

device.

quoted

quoted

quoted

Then people have to modify clk code to add CLK_IS_CRITICAL
flag to make sure the clocks are not shutdown by Linux.

However adding a new property in clk node and let driver
code parse the dts, there is no need to modify clk driver
code when OP-TEE needs

another device clock.

quoted

quoted

quoted

If OP-TEE needs linux to keep things on then why can't the
OP-TEE driver in Linux probe, acquire clocks, and keep the
clks enabled

forever?

quoted

quoted

quoted

Sounds reasonable, but how could this be done without
introducing platform-specific stuff in the OP-TEE driver?

Why is that a goal?

I do NOT think we should consider such case in this patch series,
whatever OP-TEE needs for its own feature, it should do necessary
operations

either in its driver or somewhere else by adding new patch.

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?
It should NOT block this patch set, do you think we can add this patch set first?

Anson.