Re: linux-next20080314 build fails with !CONFIG_PM

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Friday, 14 of March 2008, Andrew Morton wrote:
> On Fri, 14 Mar 2008 13:27:10 +0530
> Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> wrote:
> 
> > The next-20080314 tree build fails 
> > 
> > drivers/serial/serial_core.c: In function `uart_add_one_port':
> > drivers/serial/serial_core.c:2359: error: invalid lvalue in assignment
> > make[2]: *** [drivers/serial/serial_core.o] Error 1
> > 
> > The config # CONFIG_PM was not set.The code which is causing the 
> > build failure is 
> > 
> > 	device_can_wakeup(tty_dev) = 1;
> > 
> > when the CONFIG_PM is set the macro is preprocessed as
> > 
> > #define device_can_wakeup(dev) \
> >         ((dev)->power.can_wakeup)
> > 
> > and when not set, it becomes 0 = 1 
> > 
> > #define device_can_wakeup(dev)                  0

Kamalesh, thanks for reporting the problem.

> Caused by Alan's "PM: make wakeup flags available whenever CONFIG_PM is
> set" which I assume Len merged.

It's in the Greg's tree actually.

> Sorry, but that code should be dragged out and shot.  Doing this:
> 
> > 	device_can_wakeup(tty_dev) = 1;
> 
> is just gross and stupid.  It looks daft, it isn't C and it *requires* that
> device_can_wakeup() be implemented as a macro, which totally busts any
> concept of abstraction.
> 
> 
> The code previously had quite reasonable accessors:
> 
>  #define device_set_wakeup_enable(dev,val) \
>         ((dev)->power.should_wakeup = !!(val))
>  #define device_may_wakeup(dev) \
>         (device_can_wakeup(dev) && (dev)->power.should_wakeup)
> 
> can we please go back to that scheme?  Please also convert all these
> magical macros into inlined C functions.  One reason is that this:
> 
> +#define device_init_wakeup(dev,val) \
> +       do { \
> +               device_can_wakeup(dev) = !!(val); \
> +               device_set_wakeup_enable(dev,val); \
> +       } while(0)
> 
> is buggy.  It evaluates its arguments multiple times and will cause
> failures when passed expressions which have side-effects.
> 
> Alan, please also pass all future patches through checkpatch - there is no
> need to be adding trivial coding-style errors in this day and age.
> 
> Thanks.

Well, Greg please drop the "PM: make wakeup flags available whenever CONFIG_PM
is set" and Alan please resubmit the patch with the problems pointed out by
Andrew fixed.

In fact I'd even prefer it to be two patches, one that moves should_wakeup and
the related macros out of the CONFIG_PM_SLEEP #ifdef, because that's what fixes
the problem described in the changelog, and one that makes the remaining
changes with a separate changelog.

Thanks,
Rafael