Messages in this thread | | | Subject | Re: [PATCH v1] clk: Add devm_clk_{prepare,enable,prepare_enable} | From | Marc Gonzalez <> | Date | Mon, 25 Nov 2019 14:50:56 +0100 |
| |
Doh! Your reply never made it to my inbox, and I never thought to check the mailing list...
On 15/07/2019 23:46, Bjorn Andersson wrote:
> On Mon 15 Jul 08:34 PDT 2019, Marc Gonzalez wrote: > > [..] >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c >> index c0990703ce54..5e85548357c0 100644 >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -914,6 +914,18 @@ int clk_prepare(struct clk *clk) >> } >> EXPORT_SYMBOL_GPL(clk_prepare); >> >> +static void unprepare(void *clk) > > This deserves a less generic name.
Fair enough. Though it's only because of C's function pointer idiosyncrasies that a function wrapper is even needed.
> clk_enable() is used in code that can't sleep, in what scenario do you > envision it being useful to enable a clock from such region until devres > cleans up the associated device?
The use-case I had in mind was "Device drivers that call 1) clk_prepare_enable from probe() 2) clk_disable_unprepare() in remove()"
(Russell King has pointed out the short-comings of such an approach in a different sub-thread.)
>> +int devm_clk_prepare(struct device *dev, struct clk *clk); >> +int devm_clk_enable(struct device *dev, struct clk *clk); >> +static inline int devm_clk_prepare_enable(struct device *dev, struct clk *clk) > > devm_clk_prepare_enable() sounds very useful, devm_clk_prepare() might > be useful, so keep those and drop devm_clk_enable().
Oooh, I think I understand what you mean...
I saw clk_prepare_enable() defined as clk_prepare() + clk_enable(), and figured I'd define devm_clk_prepare_enable() as devm_clk_prepare() + devm_clk_enable() without realizing that devm_clk_enable() made no sense.
Solution: drop devm_clk_enable() from include/linux/clk.h Consequence devm_clk_prepare_enable() cannot be static inline, but that may not be a big deal...
Regards.
| |