Hi Kevin, Nicolas, On Tue, Jan 19, 2021 at 7:45 PM Kevin Hilman <khil...@baylibre.com> wrote: > [ + Geert.. renesas SoCs are the primary user of PM clk ]
Thanks! > Nicolas Pitre <npi...@baylibre.com> writes: > > The clock API splits its interface into sleepable ant atomic contexts: > > > > - clk_prepare/clk_unprepare for stuff that might sleep > > > > - clk_enable_clk_disable for anything that may be done in atomic context > > > > The code handling runtime PM for clocks only calls clk_disable() on > > suspend requests, and clk_enable on resume requests. This means that > > runtime PM with clock providers that only have the prepare/unprepare > > methods implemented is basically useless. > > > > Many clock implementations can't accommodate atomic contexts. This is > > often the case when communication with the clock happens through another > > subsystem like I2C or SCMI. > > > > Let's make the clock PM code useful with such clocks by safely invoking > > clk_prepare/clk_unprepare upon resume/suspend requests. Of course, when > > such clocks are registered with the PM layer then pm_runtime_irq_safe() > > can't be used, and neither pm_runtime_suspend() nor pm_runtime_resume() > > may be invoked in atomic context. > > > > For clocks that do implement the enable and disable methods then > > everything just works as before. > > > > Signed-off-by: Nicolas Pitre <npi...@baylibre.com> Thanks for your patch! > > --- a/drivers/base/power/clock_ops.c > > +++ b/drivers/base/power/clock_ops.c > > +/** > > + * pm_clk_op_lock - ensure exclusive access for performing clock > > operations. > > + * @psd: pm_subsys_data instance corresponding to the PM clock entry list > > + * and clk_op_might_sleep count being used. > > + * @flags: stored irq flags. > > + * @fn: string for the caller function's name. > > + * > > + * This is used by pm_clk_suspend() and pm_clk_resume() to guard > > + * against concurrent modifications to the clock entry list and the > > + * clock_op_might_sleep count. If clock_op_might_sleep is != 0 then > > + * only the mutex can be locked and those functions can only be used in > > + * non atomic context. If clock_op_might_sleep == 0 then these functions > > + * may be used in any context and only the spinlock can be locked. > > + * Returns -EINVAL if called in atomic context when clock ops might sleep. > > + */ > > +static int pm_clk_op_lock(struct pm_subsys_data *psd, unsigned long *flags, > > + const char *fn) > > +{ > > + bool atomic_context = in_atomic() || irqs_disabled(); Is this safe? Cfr. https://lore.kernel.org/dri-devel/20200914204209.256266...@linutronix.de/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds