Re: [PATCH/RFC 1/2] PM / wakeup: Add callback for wake-up change notification
Hi Rafael,
On Mon, Apr 23, 2018 at 11:37 AM, Rafael J. Wysocki wrote:
> On Mon, Apr 23, 2018 at 11:32 AM, Geert Uytterhoeven
> wrote:
>> On Mon, Apr 23, 2018 at 11:18 AM, Rafael J. Wysocki
>> 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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH/RFC 1/2] PM / wakeup: Add callback for wake-up change notification
On Mon, Apr 23, 2018 at 11:32 AM, Geert Uytterhoeven wrote: > Hi Rafael, > > On Mon, Apr 23, 2018 at 11:18 AM, Rafael J. Wysocki > 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?
Re: [PATCH/RFC 1/2] PM / wakeup: Add callback for wake-up change notification
Hi Rafael,
On Mon, Apr 23, 2018 at 11:18 AM, Rafael J. Wysocki 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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH/RFC 1/2] PM / wakeup: Add callback for wake-up change notification
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.
Re: [PATCH/RFC 1/2] PM / wakeup: Add callback for wake-up change notification
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 [...] MBR, Sergei
[PATCH/RFC 1/2] PM / wakeup: Add callback for wake-up change notification
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
---
Is there a better way to handle this?
---
drivers/base/power/wakeup.c | 4
include/linux/pm.h | 1 +
2 files changed, 5 insertions(+)
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..3dec274bc6f8 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -599,6 +599,7 @@ struct dev_pm_info {
struct list_headentry;
struct completion completion;
struct wakeup_source*wakeup;
+ void (*wakeup_change_notify)(struct device *dev, bool enable);
boolwakeup_path:1;
boolsyscore:1;
boolno_pm_callbacks:1; /* Owned by the PM core
*/
--
2.7.4
