Quoting Tri Vo (2019-08-19 10:04:56) > On Mon, Aug 19, 2019 at 8:06 AM Stephen Boyd <[email protected]> wrote: > > > > The device_set_wakeup_enable() function can be called on a device that > > hasn't been registered with device_add() yet. This allows the device to > > be in a state where wakeup is enabled for it but the device isn't > > published to userspace in sysfs yet. > > > > After commit 986845e747af ("PM / wakeup: Show wakeup sources stats in > > sysfs"), calling device_set_wakeup_enable() will fail for a device that > > hasn't been registered with the driver core via device_add(). This is > > because we try to create sysfs entries for the device and associate a > > wakeup class kobject with it before the device has been registered. > > Let's follow a similar approach that device_set_wakeup_capable() takes > > here and register the wakeup class either from > > device_set_wakeup_enable() when the device is already registered, or > > from dpm_sysfs_add() when the device is being registered with the driver > > core via device_add(). > > > > Fixes: 986845e747af ("PM / wakeup: Show wakeup sources stats in sysfs") > > Reported-by: Qian Cai <[email protected]> > > Cc: Qian Cai <[email protected]> > > Cc: Tri Vo <[email protected]> > > Signed-off-by: Stephen Boyd <[email protected]> > > --- > > > > Changes from v1: > > * Export wakeup_source_sysfs_add/remove stubs > > * New function to check if we should add the device from > > dpm_sysfs_add() > > > > drivers/base/power/power.h | 9 +++++++++ > > drivers/base/power/sysfs.c | 10 +++++++++- > > drivers/base/power/wakeup.c | 10 ++++++---- > > include/linux/pm_wakeup.h | 10 ++++++++++ > > 4 files changed, 34 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h > > index 57b1d1d88c8e..22a1533ec56b 100644 > > --- a/drivers/base/power/power.h > > +++ b/drivers/base/power/power.h > > @@ -156,5 +156,14 @@ static inline void device_pm_init(struct device *dev) > > extern int wakeup_source_sysfs_add(struct device *parent, > > struct wakeup_source *ws); > > extern void wakeup_source_sysfs_remove(struct wakeup_source *ws); > > +#else /* !CONFIG_PM_SLEEP */ > > + > > +static inline int wakeup_source_sysfs_add(struct device *parent, > > + struct wakeup_source *ws) > > +{ > > + return 0; > > +} > > + > > +static inline void wakeup_source_sysfs_remove(struct wakeup_source *ws) {} > > > > #endif /* CONFIG_PM_SLEEP */ > > diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c > > index 1b9c281cbe41..1468d03ae9fb 100644 > > --- a/drivers/base/power/sysfs.c > > +++ b/drivers/base/power/sysfs.c > > @@ -5,6 +5,7 @@ > > #include <linux/export.h> > > #include <linux/pm_qos.h> > > #include <linux/pm_runtime.h> > > +#include <linux/pm_wakeup.h> > > #include <linux/atomic.h> > > #include <linux/jiffies.h> > > #include "power.h" > > @@ -661,14 +662,21 @@ int dpm_sysfs_add(struct device *dev) > > if (rc) > > goto err_runtime; > > } > > + if (!device_has_wakeup_dev(dev)) { > > This evaluates to true if dev->power.wakeup is NULL, which will result > in a null pointer dereference later in wakeup_source_sysfs_add(). > > I think the condition you want to check for is the one you pointed out > in previous patch. > > if (dev->power.wakeup && !dev->power.wakeup->dev)
Aha thanks. I need to wrapper it because CONFIG_PM_SLEEP is the one that defines the power path. > > > + rc = wakeup_source_sysfs_add(dev, dev->power.wakeup); But also, dev->power.wakeup doesn't exist with !CONFIG_PM_SLEEP.
