Re: linux-next20080314 build fails with !CONFIG_PM

[Andrew Morton <akpm@linux-foundation.org>]

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
> 

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


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.