On Wed, 2019-02-20 at 17:41 +0100, Aaron Ma wrote: > CAUTION: Email originated externally, do not click links or open > attachments unless you recognize the sender and know the content is > safe. > > > rmi4 got spam data after S3 resume on some ThinkPads. > Then TrackPoint lost when be detected by psmouse. > Clear irqs status before set irqs will make TrackPoint back. > > BugLink: > https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.launchpad.net_bugs_1791427&d=DwIBAg&c=7dfBJ8cXbWjhc0BhImu8wQ&r=veOxv1_7HLXIaWG-OKLqp-qvZc3r7ucT1d-68JSWqpM&m=3nNm4ob6G1wtf502YFuxorJVkSQvdBasje2RrZLxhTM&s=Z0nHSPAKhQLzdoENAZBYuDC6QmZNOliyiE7h1OOVkBA&e= > Cc: <stable@xxxxxxxxxxxxxxx> > Signed-off-by: Aaron Ma <aaron.ma@xxxxxxxxxxxxx> > --- > drivers/input/rmi4/rmi_driver.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/input/rmi4/rmi_driver.c > b/drivers/input/rmi4/rmi_driver.c > index fc3ab93b7aea..20631b272f43 100644 > --- a/drivers/input/rmi4/rmi_driver.c > +++ b/drivers/input/rmi4/rmi_driver.c > @@ -374,6 +374,17 @@ static int rmi_driver_set_irq_bits(struct > rmi_device *rmi_dev, > struct device *dev = &rmi_dev->dev; > > mutex_lock(&data->irq_mutex); > + > + /* Dummy read in order to clear irqs */ > + error = rmi_read_block(rmi_dev, > + data->f01_container->fd.data_base_addr + 1, > + data->irq_status, data->num_of_irq_regs); > + if (error < 0) { > + dev_err(dev, "%s: Failed to read interrupt status!", > + __func__); > + goto error_unlock; > + } > + > bitmap_or(data->new_irq_mask, > data->current_irq_mask, mask, data->irq_count); > > -- > 2.17.1 > Sorry for the long delay in following up on this. I'm not sure this is a safe action, due to a race condition with the actual IRQ handler (rmi_process_interrupt_requests from rmi_driver.c). Remember that reading the IRQ status register clears all the IRQ bits. So you're faced with this possible scenario: - ATTN asserted, indicating new data in IRQ status register - rmi_driver_set_irq_bits called - rmi_driver_set_irq_bits reads IRQ status, clearing bits - rmi_process_interrupt_requests called - rmi_process_interrupt_request reads IRQ status, sees no bits set, nested IRQs are not handled This could lead to loss of data or inconsistent system state information. For example, a button up or down event may be lost, with consequent weird behavior by the user interface. CAVEAT: I haven't had much to do with the RMI4 driver for a long while, and am just starting to poke around with it again. I am not very familiar with the current IRQ handling implementation. Perhaps there is a guarantee in the kernel IRQ mechanism that once ATTN is asserted, then rmi_process_interrupt_requests will be called before anyone else gets a chance to read the IRQ status register. If that's the case, let me know I'm worried about nothing, and ignore this comment. Cheers, Chris
