linux-kernel - Re: [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU 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: <4a187add-462b-dfe4-868a-fdab85258b8d@sholland.org>
Date:   Fri, 3 Sep 2021 10:21:13 -0500
From:   Samuel Holland <samuel@...lland.org>
To:     Maxime Ripard <maxime@...no.tech>
Cc:     Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
        linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver

On 9/3/21 9:50 AM, Maxime Ripard wrote:
> Hi,
> 
> On Wed, Sep 01, 2021 at 12:39:44AM -0500, Samuel Holland wrote:
>> This patch series adds a CCU driver for the RTC in the H616 and R329.
>> The extra patches at the end of this series show how it would be
>> explanded to additional hardware variants.
>>
>> The driver is intended to support the existing binding used for the H6,
>> but also an updated binding which includes all RTC input clocks. I do
>> not know how to best represent that binding -- that is a major reason
>> why this series is an RFC.
>>
>> A future patch series could add functionality to the driver to manage
>> IOSC calibration at boot and during suspend/resume.
>>
>> It may be possible to support all of these hardware variants in the
>> existing RTC clock driver and avoid some duplicate code, but I'm
>> concerned about the complexity there, without any of the CCU
>> abstraction.
>>
>> This series is currently based on top of the other series I just sent
>> (clk: sunxi-ng: Lifetime fixes and module support), but I can rebase it
>> elsewhere.
> 
> I'm generally ok with this, it makes sense to move it to sunxi-ng,
> especially with that other series of yours.
> 
> My main concern about this is the split driver approach. We used to have
> that before in the RTC too, but it was mostly due to the early clock
> requirements. With your previous work, that requirement is not there
> anymore and we can just register it as a device just like the other
> clock providers.

That's a good point. Originally, I had this RTC CCU providing osc24M, so
it did need to be an early provider. But with the current version, we
could have the RTC platform driver call devm_sunxi_ccu_probe. That does
seem cleaner.

(Since it wasn't immediately obvious to me why this works: the only
early provider remaining is the sun5i CCU, and it doesn't use the sun6i
RTC driver.)

> And since we can register all those clocks at device probe time, we
> don't really need to split the driver in two (and especially in two
> different places). The only obstacle to this after your previous series
> is that we don't have of_sunxi_ccu_probe / devm_sunxi_ccu_probe
> functions public, but that can easily be fixed by moving their
> definition to include/linux/clk/sunxi-ng.h

Where are you thinking the clock definitions would go? We don't export
any of those structures (ccu_mux, ccu_common) or macros
(SUNXI_CCU_GATE_DATA) in a public header either.

Would you want to export those? That seems like a lot of churn. Or would
we put the CCU descriptions in drivers/clk/sunxi-ng and export a
function that the RTC driver can call? (Or some other idea?)

Regards,
Samuel