Re: [PATCH v1 1/3] pinctrl: intel: Replace ifdeffery by pm_sleep_ptr() macro

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Wed, Sep 4, 2024 at 10:47 AM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Wed, Sep 4, 2024 at 8:05 AM Mika Westerberg
> <mika.westerberg@xxxxxxxxxxxxxxx> 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