linux-kernel - Re: [PATCH 0/5] provide the XTAL clock via OF on Meson8/8b/8m2

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: <CAFBinCA0NaCJEDfNEg+LRfW3wxfNFGbXmGS+z7D5792TsupVAA@mail.gmail.com>
Date:   Mon, 23 Sep 2019 22:56:28 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Jerome Brunet <jbrunet@...libre.com>
Cc:     Neil Armstrong <narmstrong@...libre.com>, robh+dt@...nel.org,
        mark.rutland@....com, linux-amlogic@...ts.infradead.org,
        devicetree@...r.kernel.org, khilman@...libre.com,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH 0/5] provide the XTAL clock via OF on Meson8/8b/8m2

Hi Jerome,

On Mon, Sep 23, 2019 at 11:29 AM Jerome Brunet <jbrunet@...libre.com> wrote:
>
> On Sat 21 Sep 2019 at 17:12, Martin Blumenstingl <martin.blumenstingl@...glemail.com> wrote:
>
> > So far the HHI clock controller has been providing the XTAL clock on
> > Amlogic Meson8/Meson8b/Meson8m2 SoCs.
> > This is not correct because the XTAL is actually a crystal on the
> > boards and the SoC has a dedicated input for it.
> >
> > This updates the dt-bindings of the HHI clock controller and defines
> > a fixed-clock in meson.dtsi (along with switching everything over to
> > use this clock).
> > The clock driver needs three updates to use this:
> > - patch #2 uses clk_hw_set_parent in the CPU clock notifier. This drops
> >   the explicit reference to CLKID_XTAL while at the same time making
> >   the code much easier (thanks to Neil for providing this new method
> >   as part of the G12A CPU clock bringup!)
> > - patch #3 ensures that the clock driver doesn't rely on it's internal
> >   XTAL clock while not losing support for older .dtbs that don't have
> >   the XTAL clock input yet
> > - with patch #4 the clock controller's own XTAL clock is not registered
> >   anymore when a clock input is provided via OF
> >
> > This series is a functional no-op. It's main goal is to better represent
> > how the actual hardware looks like.
>
> I'm a bit unsure about this series.
>
> On one hand, I totally agree with you ... having the xtal in DT is the
> right way to do it ... when done from the start
yep

> On the other hand, things have been this way for years, they are working
> and going for xtal in DT does not solve any pending issue. Doing this
> means adding complexity in the driver to support both methods. It is
> also quite a significant change in DT :/
my two main motivations were:
- keeping the 32-bit SoCs as similar as possible to the 64-bit ones in
terms of "how are the [clock] drivers implemented"
- with the DDR clock controller the .dts looked weird: &ddr_clkc took
CLKID_XTAL from &clkc as input and &clkc took DDR_CLKID_DDR_PLL as
input from &ddr_clkc

RE complexity in the driver to support both:
I still have a cleanup of the meson8b.c init code on my TODO-list
because we're still supporting .dtbs without parent syscon
my plan is to drop that code-path along with the newly added fallback
for "skip CLKID_XTAL" (assuming this is accepted) together for v5.6 or
v5.7


Martin