On 9/3/21 9:50 AM, Maxime Ripard wrote:

Hi,

On Wed, Sep 01, 2021 at 12:39:44AM -0500, Samuel Holland wrote:

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel