Thread (1 message) 1 message, 1 author, 2020-08-17

Re: [PATCH] clk: clk-hi3670: Add CLK_IGNORE_UNUSED flag

From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Date: 2020-08-17 06:18:08
Also in: bpf, linux-clk, lkml

Em Sat, 15 Aug 2020 19:33:31 -0700
Stephen Boyd [off-list ref] escreveu:
Please send patches To: somebody. Sending them to nobody causes my MUA
pain.
Ok. Should I send it to you or to someone else?
Quoting Mauro Carvalho Chehab (2020-08-14 07:16:20)
quoted
There are several clocks that are required for Kirin 970 to
work. Without them, the system hangs. However, most of
the clocks defined at clk-hi3670 aren't specified on its
device tree, nor at Hikey 970 one.

A few of them are defined at the Linaro's official tree
for Hikey 970, but, even there, distros use

        clk_ignore_unused=true

as a boot option.

So, instead, let's modify the driver to use CLK_IGNORE_UNUSED
flags, removing the need for this boot parameter.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/clk/hisilicon/clk-hi3670.c | 731 +++++++++++++++++------------
 1 file changed, 425 insertions(+), 306 deletions(-)  
This is very many. Are all of these clks actually enabled out of boot
and are getting turned off at late init?
That's a very good question. Unfortunately, I don't know.

There are some documentation at:
	https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/

Including schematics for HiKey 970, but it doesn't seem to show all
the clock lines, plus the clock names at the driver don't seem to match
what's there at the datasheet.
Is there some set of clks that can be marked as CLK_IS_CRITICAL instead?
Maybe, but identifying those would require a huge amount of work. See,
this patch marks 306 clock lines with CLK_IGNORE_UNUSED:

	$ git grep CLK_IGNORE_UNUSED drivers/clk/hisilicon/clk-hi3670.c|wc -l
	306

At vanilla Kernel 5.8, there are 49 known clock lines:

	$ git grep -E 'HI\S+CLK' arch/arm64/boot/dts/hisilicon/*70*|wc -l
	49

As I'm porting several drivers in order to support DRM and hopefully USB,
this count should increase as drivers get merged.

At downstream 4.9 Kernel, there are 99 known clock lines:

	$ git grep -E 'KI\S+CLK' arch/arm64/boot/dts/hisilicon/*70*|wc -l
	99

In other words, there are still 207 lines that we have no clue about
them. What among those are critical or not is a very good question.

The CLK_IGNORE_UNUSED flag shouldn't be used very much at all. Instead,
drivers should be using the CLK_IS_CRITICAL flag. We have a lot of
CLK_IGNORE_UNUSED in the kernel right now, but the hope is that we can
get rid of this flag one day.
I see the point. Yet, I can't see any solution for that, except not letting 
PM to disable unused clocks on this chipset. See, the only way to use the
HiKey 970 board (which is the only one with DT bindings upstream for this
chipset) is to boot the Kernel with clk_ignore_unused=true.

Ok, if someone has enough time and some robot infrastructure that would
automatically be patching the driver, detect broken boots, and powering
down/up the device after each attempt, he could be disabling each one of 
the clock lines that are not specified at the DT, identifying the
critical ones.

Then, he may need to port more drivers, together with their DT bindings,
from the downstream tree.

That is a lot of work for, IMHO, not much gain.

Thanks,
Mauro
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help