linux-kernel - Re: [PATCH v1 1/3] pinctrl: intel: Replace ifdeffery by pm_sleep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75VdzNBct2N+DgEW9mDUytnoOronn4biDEifcpJvtq9Wzug@mail.gmail.com>
Date: Wed, 4 Sep 2024 10:48:42 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Andy Shevchenko <andy@...nel.org>, 
	Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH v1 1/3] pinctrl: intel: Replace ifdeffery by
 pm_sleep_ptr() macro

On Wed, Sep 4, 2024 at 10:47 AM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
> On Wed, Sep 4, 2024 at 8:05 AM Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> > On Tue, Sep 03, 2024 at 08:04:49PM +0300, Andy Shevchenko wrote:
> > > Explicit ifdeffery is ugly and theoretically might be not synchronised
> > > with the rest of functions that are assigned via pm_sleep_ptr() macro.
> > > Replace ifdeffery by pm_sleep_ptr() macro to improve this.

...

> > Can't we make this a stub when !PM_SLEEP?
> >
> > #ifdef CONFIG_PM_SLEEP
> > static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
> > {
> > ...
> > }
> > #else
> > static inline int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
> > {
> >         return 0;
> > }
> > #endif
>
> There is no benefit. It's actually the opposite, i.e. it expands more ifdeffery.
>
> ...
>
> > > -     ret = intel_pinctrl_pm_init(pctrl);
> > > +     ret = pm_sleep_ptr(intel_pinctrl_pm_init) ? intel_pinctrl_pm_init(pctrl) : 0;
> >
> > Then this still looks like a function call and not like some weird
> > conditional.
>
> I understand that, but the point is to make all PM callbacks use the
> same approach against kernel configuration. Current state of affairs
> is simple inconsistency, but it might, however quite unlikely, lead to
> desynchronization between two pm_sleep_ptr() and ifdeffery approaches.
>
> Approach that I have before this one (and I kinda agree that ternary
> here looks a bit weird) is to typedef the function and do something
> like
>
> pinctrl-intel.h:

> typedef alloc_fn;

Actually typedef is not needed as it may be embedded in the below
inline as it's used only once.

> static inline int ctx_alloc(pctrl, alloc_fn)
> {
>   if (alloc_fn)
>     return alloc_fn(pctrl);
>
>   return 0;
> }
>
> pinctrl-intel.c:
>
>   ret = ctx_alloc(pctrl, pm_sleep_ptr(_pm_init))
>   if (ret)
>     return ret;
>
> Altogether it will be ~20+ LoCs in addition to the current codebase.

-- 
With Best Regards,
Andy Shevchenko