[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VdPNCvhVSeKESbd7VNVPa8GVXVnw04jaE4HMUjK_b8+dA@mail.gmail.com>
Date: Tue, 18 Jul 2023 15:57:00 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Paul Cercueil <paul@...pouillou.net>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Linus Walleij <linus.walleij@...aro.org>,
Balsam CHIHI <bchihi@...libre.com>,
Claudiu Beznea <claudiu.beznea@...rochip.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Thierry Reding <thierry.reding@...il.com>,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org,
linux-renesas-soc@...r.kernel.org, linux-tegra@...r.kernel.org,
linux-pm@...r.kernel.org, Andy Shevchenko <andy@...nel.org>,
Sean Wang <sean.wang@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
Andrew Lunn <andrew@...n.ch>,
Gregory Clement <gregory.clement@...tlin.com>,
Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
Ludovic Desroches <ludovic.desroches@...rochip.com>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Jonathan Hunter <jonathanh@...dia.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>
Subject: Re: [PATCH v2 04/10] pinctrl: intel: Switch to use
DEFINE_NOIRQ_DEV_PM_OPS() helper
On Mon, Jul 17, 2023 at 10:56 PM Paul Cercueil <paul@...pouillou.net> wrote:
> Le lundi 17 juillet 2023 à 22:33 +0300, Andy Shevchenko a écrit :
> > On Mon, Jul 17, 2023 at 10:02 PM Paul Cercueil <paul@...pouillou.net>
> > wrote:
> > > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
...
> > > So the correct way to update this driver would be to have a
> > > conditionally-exported dev_pm_ops structure:
> > >
> > > EXPORT_GPL_DEV_PM_OPS(intel_pinctrl_pm_ops) = {
> > > NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq,
> > > intel_pinctrl_resume_noirq),
> > > };
> >
> > This looks ugly. I didn't know that EXPORT*PM_OPS designed that way,
> > but it seems pm.h in such case needs EXPORT for NOIRQ case as well.
>
> It's designed so that when CONFIG_PM is disabled, the dev_pm_ops is
> garbage-collected along with all its callbacks.
>
> I know it looks ugly, but we already have 4 variants (regular,
> namespace, GPL, namespace + GPL), if we start to add macros for
> specific use-cases then it will become bloated really quick.
Maybe macros can be replaced / changed to make it scale?
> And the "bloat" I'm trying to avoid here is the extreme expansion of
> the API which makes it hard for people not familiar to the code to
> understand what should be used and how.
So far, based on the rest of the messages in the thread the
EXPORT*PM_OPS() have the following issues:
1) do not scale (for variants with different scope we need new set of macros);
2) do not cover cases with pm_sleep_ptr();
3) export symbols in case when it's not needed.
Am I right?
> > > Then your two callbacks can be "static" and without #ifdef guards.
> > >
> > > The resulting "intel_pinctrl_pm_ops" can be marked as "extern" in
> > > the
> > > pinctrl-intel.h without any guards, as long as it is only
> > > referenced
> > > with the pm_ptr() macro.
> >
> > I'm not sure I got this. Currently drivers do not have any guards.
> > Moreover, the correct one for noirq is pm_sleep_ptr(), isn't it?
>
> The EXPORT_*_DEV_PM_OPS() macros do export the "dev_pm_ops"
> conditionally depending on CONFIG_PM. We could add variants that export
> it conditionally depending on CONFIG_PM_SLEEP, but we're back at the
> problem of adding bloat.
Exactly.
> You could use pm_sleep_ptr() indeed, with the existing macros, with the
> drawback that in the case where CONFIG_PM && !CONFIG_PM_SLEEP, the
> dev_pm_ops + callbacks are compiled in but never referenced.
And exactly.
I don't think they are ready to use (in the current form). But let's
see what we may do better here...
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists