LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: linux-next20080314 build fails with !CONFIG_PM
@ 2008-03-14 22:37 Rafael J. Wysocki
  2008-03-15 21:53 ` [PATCH 2/3] PM: make wakeup flags available whenever CONFIG_PM is set (ver 2) Alan Stern
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-03-14 22:37 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kamalesh Babulal, linux-next, sfr, linux-kernel, apw, Alan Stern,
	Len Brown, Greg KH

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2008-03-20 23:24 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <200803190315.m2J3FXnK029022@imap1.linux-foundation.org>
2008-03-19 14:55 ` + pm-convert-wakeup-flag-accessors-to-inline-functions-fix.patch added to -mm tree Alan Stern
2008-03-19 18:48   ` Andrew Morton
2008-03-19 18:57     ` Greg KH
2008-03-19 21:33       ` [PATCH 0/3] PM wakeup flags revisited (was: Re: + pm-convert-wakeup-flag-accessors-to-inline-functions-fix.patch added to -mm tree) Rafael J. Wysocki
2008-03-19 21:35         ` [PATCH 1/3] Fix misuse of wakeup flag accessors in serial core Rafael J. Wysocki
2008-03-20 23:23           ` patch pm-fix-misuse-of-wakeup-flag-accessors-in-serial-core.patch added to gregkh-2.6 tree gregkh
2008-03-19 21:37         ` [PATCH 2/3] PM: Make wakeup flags available whenever CONFIG_PM is set (ver 2) Rafael J. Wysocki
2008-03-19 22:22           ` [linux-pm] " David Brownell
2008-03-20 23:23           ` patch pm-make-wakeup-flags-available-whenever-config_pm-is-set.patch added to gregkh-2.6 tree gregkh
2008-03-19 21:39         ` [PATCH 3/3] PM: Convert wakeup flag accessors to inline functions Rafael J. Wysocki
2008-03-20 23:23           ` patch pm-convert-wakeup-flag-accessors-to-inline-functions.patch added to gregkh-2.6 tree gregkh
2008-03-19 21:46       ` + pm-convert-wakeup-flag-accessors-to-inline-functions-fix.patch added to -mm tree Alan Stern
2008-03-19 21:55         ` Rafael J. Wysocki
2008-03-19 22:04           ` Alan Stern
2008-03-14 22:37 linux-next20080314 build fails with !CONFIG_PM Rafael J. Wysocki
2008-03-15 21:53 ` [PATCH 2/3] PM: make wakeup flags available whenever CONFIG_PM is set (ver 2) Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).