Messages in this thread | | | Subject | Re: [PATCH v1] clk: Add devm_clk_{prepare,enable,prepare_enable} | From | Marc Gonzalez <> | Date | Mon, 25 Nov 2019 15:11:22 +0100 |
| |
On 25/11/2019 14:37, Russell King - ARM Linux admin wrote:
> On Mon, Nov 25, 2019 at 02:10:21PM +0100, Marc Gonzalez wrote: > >> On 25/11/2019 13:55, Russell King - ARM Linux admin wrote: >> >>> It's also worth reading https://lore.kernel.org/patchwork/patch/755667/ >>> and considering whether you really are using the clk_prepare() and >>> clk_enable() APIs correctly. Wanting these devm functions suggests >>> you aren't... >> >> In that older thread, you wrote: >> >>> If you take the view that trying to keep clocks disabled is a good way >>> to save power, then you'd have the clk_prepare() or maybe >>> clk_prepare_enable() in your run-time PM resume handler, or maybe even >>> deeper in the driver... the original design goal of the clk API was to >>> allow power saving and clock control. >>> >>> With that in mind, getting and enabling the clock together in the >>> probe function didn't make sense. >>> >>> I feel that aspect has been somewhat lost, and people now regard much >>> of the clk API as a bit of a probe-time nuisance. >> >> In the few drivers I've written, I call clk_prepare_enable() at probe. > > Right, so the clocks are enabled as soon as the device is probed, > in other words at boot time. It remains enabled for as long as the > device is bound to its driver, whether or not the device is actually > being used. Every switching edge causes heat to be generated. Every > switching edge causes energy to be wasted. > > That's fine if you have an infinite energy supply. That hasn't been > discovered yet. > > Given the prevalence of technology, don't you think we should be > doing as much as we possibly can to reduce the energy consumption > of the devices we use? It may be peanuts per device, but at scale > it all adds up.
OK, I'm starting to see the bigger picture.
(To provide some rationale for the patch, I think devm is a huge improvement for probe error-handling, and I did not understand why every driver must do manual error-handling when dealing with clocks in probe.)
I did envision kernel modules being loaded on an as-needed basis, somewhat side-stepping the energy-waste issue you point out. But I realize that such a use-case may be uncommon. (Especially due to module auto-loading.)
A few months ago, I was discussing a similar issue with GKH: Consider a device with a "START" register. Basically, if we write 0, the device turns itself off; if we write 1, it runs as configured.
I was trying to start the device only when at least one user had it "open". So I used reference counting, and started the device on 0->1 open transitions, and stopped the device on 1->0 close transitions. GKH told me that was the wrong way to do it, and IIRC suggested to start the device in probe.
I probably misunderstood Greg's suggestion. Where is the right place to start/stop a device (or gate its clocks)?
Regards.
| |