[PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

aaro.koskinen at nokia.com aaro.koskinen at nokia.com
Thu Jan 6 07:05:51 EST 2011


Hi,

Russell King - ARM Linux [linux at arm.linux.org.uk]:
> > > -static struct platform_suspend_ops omap_pm_ops = {
> > > -   .begin          = omap2_pm_begin,
> > > -   .enter          = omap2_pm_enter,
> > > -   .end            = omap2_pm_end,
> > > -   .valid          = suspend_valid_only_mem,
> > > +static const struct platform_suspend_ops omap_pm_ops[] = {
> > > +   {
> > > +           .begin          = omap2_pm_begin,
> > > +           .enter          = omap2_pm_enter,
> > > +           .end            = omap2_pm_end,
> > > +           .valid          = suspend_valid_only_mem,
> > > +   }
> > >  };
> > > -#else
> > > -static const struct platform_suspend_ops __initdata omap_pm_ops;
> > >  #endif /* CONFIG_SUSPEND */
> > >
> > >  /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
> > > @@ -582,7 +582,7 @@ static int __init omap2_pm_init(void)
> > >                                                 omap24xx_cpu_suspend_sz);
> > >     }
> > >
> > > -   suspend_set_ops(&omap_pm_ops);
> > > +   suspend_set_ops(omap_pm_ops);
>
> Utterly yuck.  Declaring it as a single element array just to avoid an
> ifdef.  That's worse than having an ifdef here.

Why it is worse? Reducing the amount of different preprocessor branches will
result in better compile test / static analysis coverage.

> There's another solution.  Don't mess about with sticking such stuff in
> the header either.
>
> #ifdef WHATEVER
> static const struct platform_suspend_ops omap_pm_ops = {
>         .begin          = omap2_pm_begin,
>         .enter          = omap2_pm_enter,
>         .end            = omap2_pm_end,
>         .valid          = suspend_valid_only_mem,
> };
> #define PM_OPS omap_pm_ops
> #else
> #define PM_OPS NULL
> #endif
> ...
> 
>         suspend_set_ops(PM_OPS);
>
> That keeps it all nice and local, and you can see exactly what's going on
> without having it spread across many different random files.

I don't think it's obviously better or simpler. There's already made a mistake in
your example...

A.



More information about the linux-arm-kernel mailing list