linux-kernel - Re: [PATCH 0/3] Apple M1 clock gate driver

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: <1ff54382-7137-49d6-841d-318e400e956e@www.fastmail.com>
Date:   Thu, 03 Jun 2021 14:55:45 +0200
From:   "Sven Peter" <sven@...npeter.dev>
To:     "Tony Lindgren" <tony@...mide.com>
Cc:     "Rob Herring" <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-clk <linux-clk@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Hector Martin" <marcan@...can.st>,
        "Michael Turquette" <mturquette@...libre.com>,
        "Stephen Boyd" <sboyd@...nel.org>,
        "Mark Kettenis" <mark.kettenis@...all.nl>,
        "Arnd Bergmann" <arnd@...nel.org>
Subject: Re: [PATCH 0/3] Apple M1 clock gate driver

Hi Tony,

On Wed, Jun 2, 2021, at 11:26, Tony Lindgren wrote:
> * Sven Peter <sven@...npeter.dev> [210530 11:09]:
> > 
> > Now the big question is *how* to describe this additional data in the
> > dts. Essentially I need to specify that e.g. to enable clock 0x270
> > I first need to enable the (internal) clocks 0x1c0 and then 0x220.
> > Are you aware of any generic way to describe this? I'm not even sure
> > how a sane non-generic way would look like when I just have a single
> > clock controller node.
> 
> To me it seems you might be able to recycle the assigned-clocks and
> assigned-clock-parents etc properties in the clock controller node.
> 
> Sure the assigned-clocks property will point to clocks in the
> clock controller itself, and will have tens of entries, but should
> work :)

Thanks for the suggestion, I really like that idea!

If I understand the assigned-clocks-parent property correctly it's meant to select
one of the possible parents of e.g. a mux clock at startup. Internally it just
uses clk_set_parent which, unless I'm mistaken, would require to assign each
clock as a possible parent of every other clock.

Now the good news is that Apple seems to have sorted the clocks topologically
on the bus, i.e. there never is a clock at address X that requires a parent
with an address bigger than X. 
The bad news is that I'd probably still have to setup clocks at 0x0, 0x4,
0x8, ..., X-0x4 as possible parents of the clock at X.

Another possibility this made me think of is to instead just use the clocks
property the way it's usually used and simply refer to the controller itself, e.g.

#define APPLE_CLK_UART0  0x270
#define APPLE_CLK_UART_P 0x220
#define APPLE_CLK_SIO    0x1c0

pmgr0: clock-controller@...700000 {
        compatible = "apple,t8103-gate-clock";
        #clock-cells = <1>;
        reg = <0x2 0x3b700000 0x0 0x4000>;
        clock-indices = <APPLE_CLK_SIO>, <APPLE_CLK_UART_P>, <APPLE_CLK_UART0>;
        clock-output-names = "clock-sio", "clock-uart-", "clock-uart0";
        clocks = <&some_dummy_root_clock>, <&pmgr0 APPLE_CLK_SIO>,
                 <&pmgr0 APPLE_CLK_UART_P>;
};

to represent the UART clocks

    UART0  (0x23b700270), parent: UART_P
    UART_P (0x23b700220), parent: SIO
    SIO    (0x23b7001c0), parent: n/a

and then have the consumer as

serial0: serial@...200000 {
    // ...
    clocks = <&pmgr0 APPLE_CLK_UART0>, <&clk24>;
    clock-names = "uart", "clk_uart_baud0";
    // ...
};

I'd have to see if it's possible to implement this sanely though if this binding
was acceptable. The self-reference to a node that hasn't been fully initialized
yet may turn out to be ugly.

> 
> And sounds like you can generate that list with some script from the
> Apple dtb.

Yup, absolutely. I don't want to write this all out by hand if I can avoid
that :-)

> 
> Regards,
> 
> Tony
> 

Thanks,

Sven