linux-kernel - Re: [PATCH 3/5] clk: eyeq5: add controller

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]

Message-Id: <CXUTPV1ZOSID.323RSEP4BL2AT@bootlin.com>
Date: Fri, 22 Dec 2023 12:25:12 +0100
From: Théo Lebrun <theo.lebrun@...tlin.com>
To: "Stephen Boyd" <sboyd@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>,
 "Gregory CLEMENT" <gregory.clement@...tlin.com>, "Krzysztof Kozlowski"
 <krzysztof.kozlowski+dt@...aro.org>, "Michael Turquette"
 <mturquette@...libre.com>, "Rob Herring" <robh+dt@...nel.org>, "Thomas
 Bogendoerfer" <tsbogend@...ha.franken.de>
Cc: "Vladimir Kondratiev" <vladimir.kondratiev@...ileye.com>,
 <linux-mips@...r.kernel.org>, <linux-clk@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>, "Thomas
 Petazzoni" <thomas.petazzoni@...tlin.com>, "Tawfik Bayouk"
 <tawfik.bayouk@...ileye.com>
Subject: Re: [PATCH 3/5] clk: eyeq5: add controller

Hello,

I've seen all your comments, thanks for that. I have a follow up about
one:

On Wed Dec 20, 2023 at 12:09 AM CET, Stephen Boyd wrote:
> Quoting Théo Lebrun (2023-12-18 09:14:18)
> > Add the Mobileye EyeQ5 clock controller driver. See the header comment
> > for more information on how it works.
>
> "See the header" is like saying "Read the code" which is pretty obvious.
> Remove this sentence and tell us why only the PLLs are supported at the
> moment or something like that.
>
> > This driver is specific to this
> > platform; it might grow to add later support of other platforms from
> > Mobileye.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@...tlin.com>
> > ---
> >  MAINTAINERS             |   1 +
> >  drivers/clk/Kconfig     |  11 +++
> >  drivers/clk/Makefile    |   1 +
> >  drivers/clk/clk-eyeq5.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 224 insertions(+)
> > 

[...]

> > diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
> > new file mode 100644
> > index 000000000000..74bcb8cec5c1
> > --- /dev/null
> > +++ b/drivers/clk/clk-eyeq5.c

[...]

> > +       of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data);
> > +}
> > +
> > +CLK_OF_DECLARE_DRIVER(eq5c, "mobileye,eyeq5-clk", eq5c_init);
>
> Please use a platform driver.

I've been trying to do that but I had a stall at boot. I initially
associated it with the UART driver acquiring a clock too early but
instead it is the CPU timer clocksource driver that consumes one of our
clock way earlier than any platform driver initialisation.

The clocksource driver we are talking about is this one for reference:
https://elixir.bootlin.com/linux/v6.6.8/source/drivers/clocksource/mips-gic-timer.c

Its usage of TIMER_OF_DECLARE means it gets probed by timer_probe ->
plat_time_init -> time_init -> start_kernel. This is way before any
initcalls. Our prior use of CLK_OF_DECLARE_DRIVER meant that we got
probed by of_clk_init -> plat_time_init.

I'm guessing we are not the first one in this situation; any advice on
how to deal with it?

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com