Hi Heiner, On 24/08/17 22:53, Heiner Kallweit wrote: > Am 24.08.2017 um 22:44 schrieb Heiner Kallweit: >> Am 24.08.2017 um 22:13 schrieb Enric Balletbo i Serra: >>> Hi Heiner >>> >>> On 24/08/17 21:16, Heiner Kallweit wrote: >>>> Am 24.08.2017 um 20:46 schrieb Heiner Kallweit: >>>>> Hi Enric, >>>>> >>>>> I just saw your submitted patch on patchwork. As I haven't been subscribed >>>>> to linux-rtc list yet, I can't reply to the original mail. >>>>> >>>>> Few remarks: >>>>> I think the same can be achieved easier (apart from the fact that member >>>>> irq was just removed from struct ds1307). >>>>> The curent call to device_set_wakeup_capable has to be replaced with >>>>> device_init_wakeup, in addition we have to call dev_pm_set_wake_irq to >>>>> register the interrupt with the Linux wakeup core. Then the core takes >>>>> care of everything. >>>>> >>> >>> Oh I just saw your patches, thanks to advice me about them. >>> >>>> After further checking not even this may be necessary. >>>> If flag I2C_CLIENT_WAKE is set for the i2c client then i2c_device_probe() >>>> enables the device as wakeup device and registers the interrupt with the >>>> wakeup core. >>>> If the i2c client is defined via DT, then of_i2c_register_device() sets >>>> this flag based on the "wakeup-source" property. >>>> Is your device configured via DT? If yes, did you check whether wakeup >>>> works w/o your patch with just setting property wakeup-source ? >>>> >>> >>> Yes, device is configured via DT and without my patch, just setting property >>> wakeup-source didn't work, the reason is that code: >>> >>> http://elixir.free-electrons.com/linux/v4.13-rc6/source/drivers/rtc/rtc-ds1307.c#L1382 >>> >> This shouldn't be the reason because registering the interrupt with the wakeup core >> happens earlier in i2c_device_probe(). >> Can you check whether dev_pm_set_wake_irq() is actually called with the right >> irq number in i2c_device_probe()? >> You have reason again, I did something wrong on my tests and read the code in a wrong way, to sum up, adding both, the interrupt pin and the wakeup-source propriety, works as expected. So forget this patch. Sorry for the noise and thanks for replying and look at this. > See also description of commit 4990d4fe327b9d9a7a > "PM / Wakeirq: Add automated device wake IRQ handling" from 2015. > It explains why the code you want to add shouldn't be needed any longer. > >> Would be interesting to see the DT config of your rtc. rtc0: rtc@68 { compatible = "dallas,ds1339"; pinctrl-names = "default"; pinctrl-0 = <&rtc0_irq_pins>; interrupt-parent = <&gpio0>; interrupts = <23 IRQ_TYPE_EDGE_FALLING>; /* gpio 23 */ wakeup-source; trickle-resistor-ohms = <2000>; reg = <0x68>; }; >> >>> I didn't test on top of your patches so let me rebase/rethink my patch on top of >>> yours. >>> >>>> >>>>> See also rtc-ds1343, although I think the calls to enable/disable_irq_wake >>>>> are not needed there because the core takes care of this already >>>>> (enable_irq_wake is called from dev_pm_arm_wake_irq). >>>>> >>>>> Rgds, Heiner >>>>> >>>> >>> >>> Thanks, >>> Enric >>> >> >
