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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Russell King - ARM Linux [linux@xxxxxxxxxxxxxxxx]:
> > > -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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux