Messages in this thread | | | From | Anson Huang <> | Subject | RE: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array | Date | Tue, 16 Oct 2018 04:37:31 +0000 |
| |
Hi, Stephen
Anson Huang Best Regards!
> -----Original Message----- > From: Stephen Boyd <sboyd@kernel.org> > Sent: Tuesday, October 16, 2018 12:46 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 <anson.huang@nxp.com>; Fabio Estevam > <fabio.estevam@nxp.com>; Jerome Forissier <jerome.forissier@linaro.org>; > Peng Fan <peng.fan@nxp.com>; Rob Herring <robh@kernel.org> > Cc: dl-linux-imx <linux-imx@nxp.com> > Subject: RE: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array > > Quoting Anson Huang (2018-10-15 02:33:35) > > > > > > > > > > > > > > > > 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. > > > > > > > And I think if op-tee has such requirement, can we have another > > > > patch to cover it? > > > > > > Yes. > > > > > > > > > > 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? > > > > > > > > > > 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? > > > > 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.
| |