Hello Alexandre,

Sorry for the rush - I should have to think more before sending this
patch ...

On Thu, 18 Nov 2021 10:33:34 +0100
Alexandre Belloni [off-list ref] wrote:

Hello,

On 18/11/2021 11:40:08+0300, Nikita Shubin wrote:

quoted

in case if threaded irq registered successfully - add da9063
as a wakeup source if "wakeup-source" node present in device tree,
set as wakeup capable otherwise.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/rtc/rtc-da9063.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
index d4b72a9fa2ba..1aceb5ba6992 100644
--- a/drivers/rtc/rtc-da9063.c
+++ b/drivers/rtc/rtc-da9063.c

@@ -490,7 +490,15 @@ static int da9063_rtc_probe(struct

platform_device *pdev) da9063_alarm_event,
 					IRQF_TRIGGER_LOW |
IRQF_ONESHOT, "ALARM", rtc);
-	if (ret)
+	if (!ret) {
+		if (device_property_present(&pdev->dev,
"wakeup-source")) {
+			device_init_wakeup(&pdev->dev, true);  

If wakeup-source is present, then this should be done regardless of
the registration of the interrupt handler. Note that wakeup-source and
interrupt are supposed to be mutually exclusive.

We still able to wakeup either ALARM IRQ is present or not.

Actually the only thing is needed in this particular case is the ability
to set "wakealarm" via sysfs - so we can wakeup from
POWER-DOWN/DELIVERY/RTC modes, namely shutdown, regardless of CONFIG_PM.

Setting dev->power.can_wakeup to true is enough for that.

On the other hand device_init_wakeup also sets can_wakeup.

May be it's enough to use device_init_wakeup in case if ALARM IRQ is
present or "wakeup-source" is set ?

I see some construction in drivers/rtc like :

rtc/rtc-pcf2127.c:673:  if (alarm_irq > 0 ||
device_property_read_bool(dev, "wakeup-source")) {
rtc/rtc-ab-eoz9.c:552:  if (client->irq > 0 ||
device_property_read_bool(dev, "wakeup-source")) {

quoted

+			dev_info(&pdev->dev, "registered as wakeup
source.\n");  

This is too verbose please avoid adding new strings

quoted

+		} else {
+			device_set_wakeup_capable(&pdev->dev,
true);  

I think this is misusing the wakeup-source property for configuration
that should be left to userspace.

quoted

+			dev_info(&pdev->dev, "marked as wakeup
capable.\n");  

Ditto

quoted

+		}
+	} else  

unbalanced brackets

quoted

 		dev_err(&pdev->dev, "Failed to request ALARM IRQ
%d: %d\n", irq_alarm, ret);
 
-- 
2.31.1