| Message ID | 1521026785-26456-2-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On 03/14/2018 02:26 PM, Geert Uytterhoeven wrote: > Add a callback to inform a device that his wake-up setting has been s/his/its/. > changed. This allows a device to synchronize device configuration with > an external user action. > > E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power > switch, the system suspend procedure is: > 1. Configure PMIC for DDR backup mode, which changes the role of the > accessory power switch from a power to a wake-up switch, > 2. Switch accessory power switch off, to prepare for system suspend, > 3. Suspend system. > > Hence step 1 cannot be done in the PMIC's suspend callback, but it can > be done in the new callback, in response to the user writing "enabled" > to the PMIC's wakeup virtual file in sysfs. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> [...] MBR, Sergei
First, sorry for the huge delay. On Wednesday, March 14, 2018 12:26:24 PM CEST Geert Uytterhoeven wrote: > Add a callback to inform a device that his wake-up setting has been > changed. This allows a device to synchronize device configuration with > an external user action. > > E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power > switch, the system suspend procedure is: > 1. Configure PMIC for DDR backup mode, which changes the role of the > accessory power switch from a power to a wake-up switch, > 2. Switch accessory power switch off, to prepare for system suspend, > 3. Suspend system. > > Hence step 1 cannot be done in the PMIC's suspend callback, I don't quite understand this, so can you please explain? What can't it be done from ->prepare() or even from a suspend notifier? > but it can be done in the new callback, in response to the user writing "enabled" > to the PMIC's wakeup virtual file in sysfs.
Hi Rafael, On Mon, Apr 23, 2018 at 11:18 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Wednesday, March 14, 2018 12:26:24 PM CEST Geert Uytterhoeven wrote: >> Add a callback to inform a device that his wake-up setting has been >> changed. This allows a device to synchronize device configuration with >> an external user action. >> >> E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power >> switch, the system suspend procedure is: >> 1. Configure PMIC for DDR backup mode, which changes the role of the >> accessory power switch from a power to a wake-up switch, >> 2. Switch accessory power switch off, to prepare for system suspend, >> 3. Suspend system. >> >> Hence step 1 cannot be done in the PMIC's suspend callback, > > I don't quite understand this, so can you please explain? > > What can't it be done from ->prepare() or even from a suspend notifier? > >> but it can be done in the new callback, in response to the user writing "enabled" >> to the PMIC's wakeup virtual file in sysfs. Step 2 (flip a switch on the board) is a manual step, to be done by the user. So it cannot be done from any suspend callback or notifier. Gr{oetje,eeting}s, Geert
On Mon, Apr 23, 2018 at 11:32 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Rafael, > > On Mon, Apr 23, 2018 at 11:18 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> On Wednesday, March 14, 2018 12:26:24 PM CEST Geert Uytterhoeven wrote: >>> Add a callback to inform a device that his wake-up setting has been >>> changed. This allows a device to synchronize device configuration with >>> an external user action. >>> >>> E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power >>> switch, the system suspend procedure is: >>> 1. Configure PMIC for DDR backup mode, which changes the role of the >>> accessory power switch from a power to a wake-up switch, >>> 2. Switch accessory power switch off, to prepare for system suspend, >>> 3. Suspend system. >>> >>> Hence step 1 cannot be done in the PMIC's suspend callback, >> >> I don't quite understand this, so can you please explain? >> >> What can't it be done from ->prepare() or even from a suspend notifier? >> >>> but it can be done in the new callback, in response to the user writing "enabled" >>> to the PMIC's wakeup virtual file in sysfs. > > Step 2 (flip a switch on the board) is a manual step, to be done by the user. > So it cannot be done from any suspend callback or notifier. I still find it somewhat difficult to follow you here. :-) How is this expected to work? Is the user expected to flip the switch on the board and then write to "wakeup" or is that the other way around?
Hi Rafael, On Mon, Apr 23, 2018 at 11:37 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Mon, Apr 23, 2018 at 11:32 AM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: >> On Mon, Apr 23, 2018 at 11:18 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >>> On Wednesday, March 14, 2018 12:26:24 PM CEST Geert Uytterhoeven wrote: >>>> Add a callback to inform a device that his wake-up setting has been >>>> changed. This allows a device to synchronize device configuration with >>>> an external user action. >>>> >>>> E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power >>>> switch, the system suspend procedure is: >>>> 1. Configure PMIC for DDR backup mode, which changes the role of the >>>> accessory power switch from a power to a wake-up switch, >>>> 2. Switch accessory power switch off, to prepare for system suspend, >>>> 3. Suspend system. >>>> >>>> Hence step 1 cannot be done in the PMIC's suspend callback, >>> >>> I don't quite understand this, so can you please explain? >>> >>> What can't it be done from ->prepare() or even from a suspend notifier? >>> >>>> but it can be done in the new callback, in response to the user writing "enabled" >>>> to the PMIC's wakeup virtual file in sysfs. >> >> Step 2 (flip a switch on the board) is a manual step, to be done by the user. >> So it cannot be done from any suspend callback or notifier. > > I still find it somewhat difficult to follow you here. :-) > > How is this expected to work? Is the user expected to flip the switch > on the board and then write to "wakeup" or is that the other way > around? The other way around. The user must not flip the switch before the PMIC is configured for DDR backup mode, else the system will be powered off immediately. 1. User writes to wakeup, which invokes the new callback, and configures the PMIC for DDR backup mode, 2. User flips the switch, 3. User writes to /sys/power/state to suspend. After system suspend: 4. User flips the switch to resume. Yes, it is convoluted. I hope I managed to explain it clearly this time. Currently step 1 is done using i2set from userspace, which is very unfriendly (cfr. https://elinux.org/R-Car/Boards/Salvator-XS#PSCI_System_Suspend). Note that on systems with a momentary power switch (e.g. R-Car Starter Kit Pro or Premier aka [HM]3ULCB), step 2 is not needed. Hence things are simplified a lot, as PMIC configuration can be done from the PMIC's suspend callback on these platforms. Gr{oetje,eeting}s, Geert
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index ea01621ed769ab26..40bda13d4dcd36f3 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -256,6 +256,8 @@ static int device_wakeup_attach(struct device *dev, struct wakeup_source *ws) if (dev->power.wakeirq) device_wakeup_attach_irq(dev, dev->power.wakeirq); spin_unlock_irq(&dev->power.lock); + if (dev->power.wakeup_change_notify) + dev->power.wakeup_change_notify(dev, true); return 0; } @@ -373,6 +375,8 @@ static struct wakeup_source *device_wakeup_detach(struct device *dev) { struct wakeup_source *ws; + if (dev->power.wakeup_change_notify) + dev->power.wakeup_change_notify(dev, false); spin_lock_irq(&dev->power.lock); ws = dev->power.wakeup; dev->power.wakeup = NULL; diff --git a/include/linux/pm.h b/include/linux/pm.h index e723b78d835706f2..3dec274bffffc6f8 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -599,6 +599,7 @@ struct dev_pm_info { struct list_head entry; struct completion completion; struct wakeup_source *wakeup; + void (*wakeup_change_notify)(struct device *dev, bool enable); bool wakeup_path:1; bool syscore:1; bool no_pm_callbacks:1; /* Owned by the PM core */
Add a callback to inform a device that his wake-up setting has been changed. This allows a device to synchronize device configuration with an external user action. E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power switch, the system suspend procedure is: 1. Configure PMIC for DDR backup mode, which changes the role of the accessory power switch from a power to a wake-up switch, 2. Switch accessory power switch off, to prepare for system suspend, 3. Suspend system. Hence step 1 cannot be done in the PMIC's suspend callback, but it can be done in the new callback, in response to the user writing "enabled" to the PMIC's wakeup virtual file in sysfs. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Is there a better way to handle this? --- drivers/base/power/wakeup.c | 4 ++++ include/linux/pm.h | 1 + 2 files changed, 5 insertions(+)