Re: About clk maintainership [Was: Re: [PULL] Add variants of devm_clk_get for prepared and enabled clocks enabled clocks]

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Stephen Boyd <sboyd@kernel.org>,
	alexandre.belloni@bootlin.com,
	Michael Turquette <mturquette@baylibre.com>,
	thierry.reding@gmail.com, lee.jones@linaro.org,
	linux-clk@vger.kernel.org, linux-rtc@vger.kernel.org,
	Ludovic.Desroches@microchip.com, o.rempel@pengutronix.de,
	andy.shevchenko@gmail.com, aardelean@deviqon.com,
	linux-pwm@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	broonie@kernel.org, Jonathan.Cameron@huawei.com,
	linux-arm-kernel@lists.infradead.org, a.zummo@towertech.it,
	linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org,
	wsa@kernel.org, kernel@pengutronix.de, akpm@linux-foundation.org,
	torvalds@linux-foundation.org, Claudiu.Beznea@microchip.com
Subject: Re: About clk maintainership [Was: Re: [PULL] Add variants of devm_clk_get for prepared and enabled clocks enabled clocks]
Date: Mon, 2 Aug 2021 10:48:10 +0100	[thread overview]
Message-ID: <20210802094810.GJ22278@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210731120004.i3affxw7upl5y4c5@pengutronix.de>

On Sat, Jul 31, 2021 at 02:00:04PM +0200, Uwe Kleine-König wrote:
> Hi Russell, hi Stephen,
> 
> On Sat, Jul 31, 2021 at 12:41:19AM -0700, Stephen Boyd wrote:
> > +1 This patch doesn't fall under CCF maintainer.
> 
> Given that CCF is the only implementer of devm_clk_get at least an Ack
> from your side would still be good I guess?

I think devm_clk_get() should not be part of CCF but should be
part of the interface level - it's silly to have devm_clk_get()
being CCF but not clk_get(). The same should go for the other
devm wrappers around the plain clk_* interfaces.

> I found a patch set adding devm variants of clk_enable (e.g.
> https://lore.kernel.org/patchwork/patch/755667/) but this approach is
> different as it also contains clk_get which IMHO makes more sense 
> The discussion considered wrapping get+enable at one point, but I didn't
> find a followup.

There have been several different approaches to wrapping things up,
but here's a question: should we make it easier to do the lazy thing
(get+enable) or should we make it easier to be power efficient?
Shouldn't we be encouraging people to write power efficient drivers?

> > I still wonder if it would be better if we had some sort of DT binding
> > that said "turn this clk on when the driver probes this device and keep
> > it on until the driver is unbound".
> 
> This doesn't sound like a hardware property and so I don't think this
> belongs into DT and I would be surprised if the dt maintainers would be
> willing to accept an idea with this semantic.

I really don't like that idea - enabling or disabling a clock is
an implementation decision, one which can change over time. Even
if a clock is required to be on for e.g. accessing device registers,
we may decide that, if we want maximum power savings, to disable
that clock when the device is not being used, but an initial driver
implementation may not do that. If we encourage people to throw a
property in DT, then the driver's runtime behaviour becomes a
combination of the DT being used and the driver implementation.
Let's keep that to a minimum.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!