Hi Russell, hi Stephen, On Sat, Jul 31, 2021 at 12:41:19AM -0700, Stephen Boyd wrote: > Quoting Russell King (Oracle) (2021-07-28 13:40:34) > > > 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... > > > > > 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. > +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? > 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. > 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. > 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 -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
