On Sat, 31 Jul 2021 14:00:04 +0200
Uwe Kleine-König [off-list ref] wrote:

Hi Russell, hi Stephen,

On Sat, Jul 31, 2021 at 12:41:19AM -0700, Stephen Boyd wrote:

quoted

Quoting Russell King (Oracle) (2021-07-28 13:40:34)  

quoted

quoted

I adapted the Subject in the hope to catch Stephen's and Michael's
attention. My impression is that this thread isn't on their radar yet,
but the topic here seems important enough to get a matching Subject.  

The thread is on my radar but...
  

quoted

Have you thought about sending your pull request to the clk API
maintainer (iow, me) ?  

I wasn't really aware that Russell has the clk API hat (or that this
separate hat actually exists and this isn't purely a CCF topic). I
assume I only did

	$ scripts/get_maintainer.pl -f drivers/clk/clk-devres.c

which is where the current and new code implementing devm_clk_get et al
lives.

@Russell: What is your position here, do you like the approach of
devm_clk_get_enabled? I can send a new pull request in your direction if
you like it and are willing to take it.

quoted

+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?

quoted

Finally, this sort of patch has been discussed for years and I didn't
see any mention of those previous attempts in the patch series. Has
something changed since that time? I think we've got a bunch of hand
rolled devm things in the meantime but not much else.   

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.

quoted

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.

Agreed. It's not unheard of to have a driver start out just enabing
clock at probe and dropping it at remove. When the driver gets more
sophisticated it will then manage the clock more frequently.
Whilst that's often tied to runtime_pm I'm not sure it always is.

Given the mess that would be involved in having a property that we
need to later ignore for particular drivers, I'd keep this management
explicit in the driver. This series makes that trivial to do for these
easy cases.

Jonathan

quoted

That would probably work well for quite a few drivers that don't want
to ever call clk_get() or clk_prepare_enable() and could tie into the
assigned-clock-rates property in a way that let us keep the platform
details out of the drivers. We could also go one step further and make
a clk pm domain when this new property is present and then have the
clk be enabled based on runtime PM of the device (and if runtime PM is
disabled then just enable it at driver probe time).  

clk pm domain sounds good, but introducing devm_clk_get_enabled() is
much easier and converting to it can be done without dt changes and more
or less mechanically. So I consider the cost-usage-value of
devm_clk_get_enabled() much better.

Best regards
Uwe

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