+Cc: Tony
On Thu, Aug 15, 2024 at 5:48 AM Dmitry Osipenko <digetx@xxxxxxxxx> wrote:
> 13.08.2024 18:52, Andy Shevchenko пишет:
> ...
> >>> but somewhere in the replies
> >>> here I would like to hear about roadmap to get rid of the
> >>> pm_runtime_irq_safe() in all Tegra related code.
> >>
> >> What is the problem with pm_runtime_irq_safe()?
> >
> > It's a hack. It has no reasons to stay in the kernel. It also prevents
> > PM from working properly (in some cases, not Tegra).
>
> Why is it a hack? Why it can't be made to work properly for all cases?
Because it messes up with the proper power transitions of the parent
devices. Refer to the initial commit c7b61de5b7b1 ("PM / Runtime: Add
synchronous runtime interface for interrupt handlers (v3)") that
pretty much explains the constraints of it. Also note, it was added
quite a while after the main PM machinery had been introduced.
What you have to use is device links to make sure the parent (PM
speaking) may not go away.
FWIW, if I am not mistaken the whole reconsideration of
pm_runtime_irq_safe() had been started with this [1] thread.
If you want to dive more into the history of this API, run `git log -S
pm_runtime_irq_safe`. It gives you also interesting facts of how it
was started being used and in many cases reverted or reworked for a
reason.
> >> There were multiple
> >> problems with RPM for this driver in the past, it wasn't trivial to make
> >> it work for all Tegra HW generations. Don't expect anyone would want to
> >> invest time into doing it all over again.
> >
> > You may always refer to the OMAP case, which used to have 12 (IIRC,
> > but definitely several) calls to this API and now 0. Taking the OMAP
> > case into consideration I believe it's quite possible to get rid of
> > this hack and retire the API completely. Yes, this may take months or
> > even years. But I would like to have this roadmap be documented.
>
> There should be alternative to the removed API. Otherwise drivers will
> have to have own hacks to work around the RPM limitation, re-invent own
> PM, or not do RPM at all.
>
> Looking at the i2c-omap.c, I see it's doing pm_runtime_get_sync() in the
> atomic transfer, which should cause a lockup without IRQ-safe RPM,
> AFAICT. The OMAP example doesn't look great so far.
Bugs may still appear, but it's not a point. I can easily find a
better example with a hint why it's bad to call that API [2][3][4] and
so on.
[1]: https://lore.kernel.org/all/20180515183409.78046-1-andriy.shevchenko@xxxxxxxxxxxxxxx/T/#u
[2]: https://lore.kernel.org/all/20191114101718.20619-1-peter.ujfalusi@xxxxxx/
[3]: https://lore.kernel.org/all/20180920193532.7714-1-tony@xxxxxxxxxxx/
[4]: https://lore.kernel.org/all/1463014396-4095-1-git-send-email-tony@xxxxxxxxxxx/
--
With Best Regards,
Andy Shevchenko
