All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime@cerno.tech>
To: Samuel Holland <samuel@sholland.org>
Cc: Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-clk@vger.kernel.org, linux-sunxi@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver
Date: Fri, 3 Sep 2021 16:50:13 +0200	[thread overview]
Message-ID: <20210903145013.hn6dv7lfyvfys374@gilmour> (raw)
In-Reply-To: <20210901053951.60952-1-samuel@sholland.org>

[-- Attachment #1: Type: text/plain, Size: 1849 bytes --]

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.

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

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech>
To: Samuel Holland <samuel@sholland.org>
Cc: Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-clk@vger.kernel.org, linux-sunxi@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver
Date: Fri, 3 Sep 2021 16:50:13 +0200	[thread overview]
Message-ID: <20210903145013.hn6dv7lfyvfys374@gilmour> (raw)
In-Reply-To: <20210901053951.60952-1-samuel@sholland.org>


[-- Attachment #1.1: Type: text/plain, Size: 1849 bytes --]

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.

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

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

  parent reply	other threads:[~2021-09-03 14:50 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  5:39 [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver Samuel Holland
2021-09-01  5:39 ` Samuel Holland
2021-09-01  5:39 ` [RFC PATCH 1/7] dt-bindings: rtc: sun6i: Add H616 and R329 compatibles Samuel Holland
2021-09-01  5:39   ` Samuel Holland
2021-09-01 12:06   ` Rob Herring
2021-09-01 12:06     ` Rob Herring
2021-09-02 15:27   ` Rob Herring
2021-09-02 15:27     ` Rob Herring
2021-09-03 15:36     ` Samuel Holland
2021-09-03 15:36       ` Samuel Holland
2021-09-07 14:44       ` Rob Herring
2021-09-07 14:44         ` Rob Herring
2021-09-07 14:44         ` Rob Herring
2021-09-08  2:26         ` Samuel Holland
2021-09-08  2:26           ` Samuel Holland
2021-09-01  5:39 ` [RFC PATCH 2/7] clk: sunxi-ng: div: Add macro using CLK_HW_INIT_FW_NAME Samuel Holland
2021-09-01  5:39   ` Samuel Holland
2021-09-01  5:39 ` [RFC PATCH 3/7] clk: sunxi-ng: mux: Add macro using CLK_HW_INIT_PARENTS_DATA Samuel Holland
2021-09-01  5:39   ` Samuel Holland
2021-09-01  5:39 ` [RFC PATCH 4/7] clk: sunxi-ng: mux: Allow muxes to have keys Samuel Holland
2021-09-01  5:39   ` Samuel Holland
2021-09-01  5:39 ` [RFC PATCH 5/7] clk: sunxi-ng: Add support for the sun50i RTC clocks Samuel Holland
2021-09-01  5:39   ` Samuel Holland
2021-09-01  5:39 ` [RFC PATCH 6/7] [DO NOT MERGE] clk: sunxi-ng: Add support for H6 Samuel Holland
2021-09-01  5:39   ` Samuel Holland
2021-09-03 14:51   ` Maxime Ripard
2021-09-03 14:51     ` Maxime Ripard
2021-09-03 15:07     ` Samuel Holland
2021-09-03 15:07       ` Samuel Holland
2021-09-01  5:39 ` [RFC PATCH 7/7] [DO NOT MERGE] clk: sunxi-ng: Add support for T5 Samuel Holland
2021-09-01  5:39   ` Samuel Holland
2021-09-03 14:50 ` Maxime Ripard [this message]
2021-09-03 14:50   ` [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver Maxime Ripard
2021-09-03 15:21   ` Samuel Holland
2021-09-03 15:21     ` Samuel Holland
2021-09-09  8:45     ` Maxime Ripard
2021-09-09  8:45       ` Maxime Ripard
2021-09-28  7:46       ` Samuel Holland
2021-09-28  7:46         ` Samuel Holland
2021-09-28  9:06         ` Maxime Ripard
2021-09-28  9:06           ` Maxime Ripard
2021-09-29  3:54           ` Samuel Holland
2021-09-29  3:54             ` Samuel Holland
2021-10-25 15:54             ` Maxime Ripard
2021-10-25 15:54               ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210903145013.hn6dv7lfyvfys374@gilmour \
    --to=maxime@cerno.tech \
    --cc=devicetree@vger.kernel.org \
    --cc=jernej.skrabec@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=mturquette@baylibre.com \
    --cc=robh+dt@kernel.org \
    --cc=samuel@sholland.org \
    --cc=sboyd@kernel.org \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.