Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote on Mon [2019-Jul-01 00:32:33 -0700]: > On Mon, Jun 24, 2019 at 07:24:57AM -0500, Benoit Parrot wrote: > > Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote on Sat [2019-Jun-22 22:59:40 -0700]: > > > On Sat, Jun 22, 2019 at 01:37:10PM +0300, Andy Shevchenko wrote: > > > > On Fri, Jun 21, 2019 at 9:53 PM Benoit Parrot <bparrot@xxxxxx> wrote: > > > > > > > > > > As a wakeup source when the system is in suspend there is little point > > > > > trying to access a register across the i2c bus as it is probably still > > > > > inactive. We need to prevent the irq handler from being called during > > > > > suspend. > > > > > > > > > > > > > Hmm... But how OS will know what the event to handle afterwards? > > > > I mean shouldn't we guarantee somehow the delivery of the event to the > > > > input, in this case, subsystem followed by corresponding user space? > > > > > > If we are using level interrupts then it will work OK, however it is > > > really easy to lose edge here, as replaying disabled edge triggered > > > interrupts is not really reliable. > > > > > > Benoit, what kind of interrupt do you use in your system? > > > > Dmitry, > > > > On our systems we currently used edge trigger. One example is available in > > mainline: arch/arm/boot/dts/am437x-sk-evm.dts > > 632: interrupts = <31 IRQ_TYPE_EDGE_FALLING>; > > Does your device still work if you switch to level-triggered interrupt? That would depend on the device. But for instance on am437x, in order for GPIO IRQ to be detected as a wake up event they need to be edge triggered. > > Regarding your patch I am uncomfortable with disabling interrupts if > interrupt is edge-triggered, as replaying edge interrupts after enabling > is not very reliable. So we should either only disable interrupt if it > is level-triggered, or make sure we read and process data from the > device after re-enabling interrupt to rearm it. We'll need to make sure > suspend does not race with interrupt handler than and also make sure we > handle case when device does not actually has data to report. I am still not sure who would consume these events. Upon waking up from suspend it would take a while for user-space to be ready to consume these events, and by that time there may have been quite a few of them. We are currently missing those events anyways, no? I mean the i2c read operation during suspend is failing anyways, which means that particular event is already missed. Regards, Benoit > > Thanks. > > -- > Dmitry
