On Tue, Aug 03, 2021 at 01:11:54AM -0700, Stephen Boyd wrote:

Quoting Russell King (Oracle) (2021-08-02 09:38:24)

quoted

On Mon, Aug 02, 2021 at 05:27:55PM +0200, Uwe Kleine-Konig wrote:

quoted

Hello Russell,

On Mon, Aug 02, 2021 at 10:48:10AM +0100, Russell King (Oracle) wrote:

quoted

quoted

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?

Yeah, sounds compelling, but I wonder if that's of practical importance.
How many driver authors do you expect to lure into making a better
driver just because devm_clk_get_prepared() doesn't exist? In contrast:
How many drivers become simpler with devm_clk_get_prepared() and so
it becomes easier to maintain them and easier to spot bugs?
In the absence of devm_clk_get_prepared(), is it better that several
frameworks (or drivers) open code it?

It probably depends on where you stand on power management and power
efficiency issues. Personally, I would like to see more effort put
into drivers to make them more power efficient, and I believe in the
coming years, power efficiency is going to become a big issue.

I agree we should put more effort into power efficiency in the kernel.
I've occasionally heard from driver writers that they never will turn
the clk off even in low power modes though. They feel like it's a
nuisance to have to do anything with the clk framework in their driver.
When I say "why not use runtime PM?" I get told that they're not turning
the clk off because it needs to be on all the time, so using runtime PM
makes the driver more complicated, not less, and adds no value. I think
some touchscreens are this way, and watchdogs too. Looking at the
drivers being converted in this series I suspect RTC is one of those
sorts of devices as well. But SPI and I2C most likely could benefit from
using runtime PM and so those ones don't feel appropriate to convert.

Maybe this series would be more compelling if those various drivers that
are hand rolling the devm action were converted to the consolidated
official devm function. The truth is it's already happening in various
subsystems so consolidating that logic into one place would be a win
code size wise and very hard to ignore.

Doing

 $ git grep devm_add_action | grep clk

seems to catch quite a few of them.

Another upside is that grepping for these drivers with a potential for
further improvement become easier to grep for as
devm_clk_get_{prepared,enabled} is a much better hint :-)

The changes to these drivers probably won't go through a clk tree, so
adding these patches before adding devm_clk_get_enabled() would only
help for the warm and cozy feeling that it is right to do so, correct?

As my focus is limited to (mostly) drivers/pwm and I already have quite
some other patch quests on my list:

So can I lure you in merging the new functions and I will create a
kernel janitor task to convert more existing drivers?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |