linux-next20080314 build fails with !CONFIG_PM - Kernel

This is a discussion on linux-next20080314 build fails with !CONFIG_PM - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: linux-next20080314 build fails with !CONFIG_PM

  1. linux-next20080314 build fails with !CONFIG_PM

    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

    --
    Thanks & Regards,
    Kamalesh Babulal,
    Linux Technology Center,
    IBM, ISTL.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: linux-next20080314 build fails with !CONFIG_PM

    On Fri, 14 Mar 2008 13:27:10 +0530
    Kamalesh Babulal 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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: linux-next20080314 build fails with !CONFIG_PM

    On Friday, 14 of March 2008, Andrew Morton wrote:
    > On Fri, 14 Mar 2008 13:27:10 +0530
    > Kamalesh Babulal 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. [PATCH 0/3] PM wakeup flags revisited

    On Fri, 14 Mar 2008, Rafael J. Wysocki wrote:

    > On Friday, 14 of March 2008, Andrew Morton wrote:


    > > 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.


    It seems to have been a one-time occurrence. This patch series fixes
    it.

    > > 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.


    This series converts the macros to inline functions.

    > > 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.


    Rest assurred that checkpath is now an integral part of my normal
    workflow. All the patches in this series pass with flying colors.

    > 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.


    Done.

    The 1/3 patch fixes the unfortunate code in the serial-core driver.

    The 2/3 patch moves the macros out from under CONFIG_PM_SLEEP.

    The 3/3 patch converts the macros to inline functions and creates a new
    pm_wakeup.h file for them. They can't remain in pm.h, because they
    depend on the definition of struct device -- and the compiler hasn't
    seen that yet when pm.h is read.

    It's not clear why the can_wakeup accessors are written to work even
    when CONFIG_PM isn't set. Nevertheless, the patches retain that
    behavior.

    Alan Stern

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. [PATCH 1/3] Fix misuse of wakeup flag accessors in serial core

    This patch (as1059) fixes a mistake in the way the serial core
    initializes a device's wakeup settings. It should use the accessor
    routine instead of relying on a macro producing an lvalue.

    Signed-off-by: Alan Stern

    ---

    Index: usb-2.6/drivers/serial/serial_core.c
    ================================================== =================
    --- usb-2.6.orig/drivers/serial/serial_core.c
    +++ usb-2.6/drivers/serial/serial_core.c
    @@ -2356,7 +2356,7 @@ int uart_add_one_port(struct uart_driver
    */
    tty_dev = tty_register_device(drv->tty_driver, port->line, port->dev);
    if (likely(!IS_ERR(tty_dev))) {
    - device_can_wakeup(tty_dev) = 1;
    + device_init_wakeup(tty_dev, 1);
    device_set_wakeup_enable(tty_dev, 0);
    } else
    printk(KERN_ERR "Cannot register tty device on line %d\n",

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. [PATCH 2/3] PM: make wakeup flags available whenever CONFIG_PM is set (ver 2)

    The various wakeup flags and their accessor macros in struct
    dev_pm_info should be available whenever CONFIG_PM is enabled, not
    just when CONFIG_PM_SLEEP is on. Otherwise remote wakeup won't always
    be configurable for runtime power management. This patch (as1056b)
    fixes the oversight.

    Signed-off-by: Alan Stern

    ---

    Index: usb-2.6/include/linux/pm.h
    ================================================== =================
    --- usb-2.6.orig/include/linux/pm.h
    +++ usb-2.6/include/linux/pm.h
    @@ -183,8 +183,8 @@ typedef struct pm_message {
    struct dev_pm_info {
    pm_message_t power_state;
    unsigned can_wakeup:1;
    -#ifdef CONFIG_PM_SLEEP
    unsigned should_wakeup:1;
    +#ifdef CONFIG_PM_SLEEP
    struct list_head entry;
    #endif
    };
    @@ -197,11 +197,6 @@ extern void device_resume(void);
    extern int device_suspend(pm_message_t state);
    extern int device_prepare_suspend(pm_message_t state);

    -#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)
    -
    extern void __suspend_report_result(const char *function, void *fn, int ret);

    #define suspend_report_result(fn, ret) \
    @@ -209,6 +204,24 @@ extern void __suspend_report_result(cons
    __suspend_report_result(__FUNCTION__, fn, ret); \
    } while (0)

    +#else /* !CONFIG_PM_SLEEP */
    +
    +static inline int device_suspend(pm_message_t state)
    +{
    + return 0;
    +}
    +
    +#define suspend_report_result(fn, ret) do { } while (0)
    +
    +#endif /* !CONFIG_PM_SLEEP */
    +
    +#ifdef CONFIG_PM
    +
    +#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)
    +
    /*
    * Platform hook to activate device wakeup capability, if that's not already
    * handled by enable_irq_wake() etc.
    @@ -223,24 +236,17 @@ static inline int call_platform_enable_w
    return 0;
    }

    -#else /* !CONFIG_PM_SLEEP */
    -
    -static inline int device_suspend(pm_message_t state)
    -{
    - return 0;
    -}
    +#else /* !CONFIG_PM */

    #define device_set_wakeup_enable(dev,val) do{}while(0)
    #define device_may_wakeup(dev) (0)

    -#define suspend_report_result(fn, ret) do { } while (0)
    -
    static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
    {
    return 0;
    }

    -#endif /* !CONFIG_PM_SLEEP */
    +#endif /* !CONFIG_PM */

    /* changes to device_may_wakeup take effect on the next pm state change.
    * by default, devices should wakeup if they can.
    Index: usb-2.6/drivers/base/power/main.c
    ================================================== =================
    --- usb-2.6.orig/drivers/base/power/main.c
    +++ usb-2.6/drivers/base/power/main.c
    @@ -56,8 +56,6 @@ static DEFINE_MUTEX(dpm_list_mtx);

    static DECLARE_RWSEM(pm_sleep_rwsem);

    -int (*platform_enable_wakeup)(struct device *dev, int is_on);
    -
    /**
    * device_pm_add - add a device to the list of active devices
    * @dev: Device to be added to the list
    Index: usb-2.6/drivers/base/power/sysfs.c
    ================================================== =================
    --- usb-2.6.orig/drivers/base/power/sysfs.c
    +++ usb-2.6/drivers/base/power/sysfs.c
    @@ -6,6 +6,8 @@
    #include
    #include "power.h"

    +int (*platform_enable_wakeup)(struct device *dev, int is_on);
    +

    /*
    * wakeup - Report/change current wakeup option for device

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. [PATCH 3/3] PM: convert wakeup flag accessors to inline functions

    This patch (as1058) improves the wakeup macros in include/linux/pm.h.
    All but the trivial ones are converted to inline routines, which
    requires moving them to a separate header file since they depend on
    the definition of struct device.

    Signed-off-by: Alan Stern

    ---

    Index: usb-2.6/include/linux/pm.h
    ================================================== =================
    --- usb-2.6.orig/include/linux/pm.h
    +++ usb-2.6/include/linux/pm.h
    @@ -211,54 +211,10 @@ static inline int device_suspend(pm_mess
    return 0;
    }

    -#define suspend_report_result(fn, ret) do { } while (0)
    +#define suspend_report_result(fn, ret) do {} while (0)

    #endif /* !CONFIG_PM_SLEEP */

    -#ifdef CONFIG_PM
    -
    -#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)
    -
    -/*
    - * Platform hook to activate device wakeup capability, if that's not already
    - * handled by enable_irq_wake() etc.
    - * Returns zero on success, else negative errno
    - */
    -extern int (*platform_enable_wakeup)(struct device *dev, int is_on);
    -
    -static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
    -{
    - if (platform_enable_wakeup)
    - return (*platform_enable_wakeup)(dev, is_on);
    - return 0;
    -}
    -
    -#else /* !CONFIG_PM */
    -
    -#define device_set_wakeup_enable(dev,val) do{}while(0)
    -#define device_may_wakeup(dev) (0)
    -
    -static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
    -{
    - return 0;
    -}
    -
    -#endif /* !CONFIG_PM */
    -
    -/* changes to device_may_wakeup take effect on the next pm state change.
    - * by default, devices should wakeup if they can.
    - */
    -#define device_can_wakeup(dev) \
    - ((dev)->power.can_wakeup)
    -#define device_init_wakeup(dev,val) \
    - do { \
    - device_can_wakeup(dev) = !!(val); \
    - device_set_wakeup_enable(dev,val); \
    - } while(0)
    -
    /*
    * Global Power Management flags
    * Used to keep APM and ACPI from both being active
    Index: usb-2.6/include/linux/device.h
    ================================================== =================
    --- usb-2.6.orig/include/linux/device.h
    +++ usb-2.6/include/linux/device.h
    @@ -475,6 +475,9 @@ struct device {
    void (*release)(struct device *dev);
    };

    +/* Get the wakeup routines, which depend on struct device */
    +#include
    +
    #ifdef CONFIG_NUMA
    static inline int dev_to_node(struct device *dev)
    {
    Index: usb-2.6/include/linux/pm_wakeup.h
    ================================================== =================
    --- /dev/null
    +++ usb-2.6/include/linux/pm_wakeup.h
    @@ -0,0 +1,90 @@
    +/*
    + * pm_wakeup.h - Power management wakeup interface
    + *
    + * Copyright (C) 2008 Alan Stern
    + *
    + * This program is free software; you can redistribute it and/or modify
    + * it under the terms of the GNU General Public License as published by
    + * the Free Software Foundation; either version 2 of the License, or
    + * (at your option) any later version.
    + *
    + * This program is distributed in the hope that it will be useful,
    + * but WITHOUT ANY WARRANTY; without even the implied warranty of
    + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
    + * GNU General Public License for more details.
    + *
    + * You should have received a copy of the GNU General Public License
    + * along with this program; if not, write to the Free Software
    + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
    + */
    +
    +#ifndef _LINUX_PM_WAKEUP_H
    +#define _LINUX_PM_WAKEUP_H
    +
    +#ifndef _DEVICE_H_
    +# error "please don't include this file directly"
    +#endif
    +
    +#ifdef CONFIG_PM
    +
    +/* changes to device_may_wakeup take effect on the next pm state change.
    + * by default, devices should wakeup if they can.
    + */
    +static inline void device_init_wakeup(struct device *dev, int val)
    +{
    + dev->power.can_wakeup = dev->power.should_wakeup = !!val;
    +}
    +
    +static inline int device_can_wakeup(struct device *dev)
    +{
    + return dev->power.can_wakeup;
    +}
    +
    +static inline void device_set_wakeup_enable(struct device *dev, int val)
    +{
    + dev->power.should_wakeup = !!val;
    +}
    +
    +static inline int device_may_wakeup(struct device *dev)
    +{
    + return dev->power.can_wakeup & dev->power.should_wakeup;
    +}
    +
    +/*
    + * Platform hook to activate device wakeup capability, if that's not already
    + * handled by enable_irq_wake() etc.
    + * Returns zero on success, else negative errno
    + */
    +extern int (*platform_enable_wakeup)(struct device *dev, int is_on);
    +
    +static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
    +{
    + if (platform_enable_wakeup)
    + return (*platform_enable_wakeup)(dev, is_on);
    + return 0;
    +}
    +
    +#else /* !CONFIG_PM */
    +
    +/* For some reason the next two routines work even without CONFIG_PM */
    +static inline void device_init_wakeup(struct device *dev, int val)
    +{
    + dev->power.can_wakeup = !!val;
    +}
    +
    +static inline int device_can_wakeup(struct device *dev)
    +{
    + return dev->power.can_wakeup;
    +}
    +
    +#define device_set_wakeup_enable(dev, val) do {} while (0)
    +#define device_may_wakeup(dev) 0
    +
    +static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
    +{
    + return 0;
    +}
    +
    +#endif /* !CONFIG_PM */
    +
    +#endif /* _LINUX_PM_WAKEUP_H */

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread