在 2023/12/8 21:52, Thomas Gleixner 写道:
On Thu, Dec 07 2023 at 09:40, xiongxin@xxxxxxxxxx wrote:
When an interrupt controller uses a function such as handle_level_irq()
as an interrupt handler and the controller implements the irq_disable()
callback, the following scenario will appear in the i2c-hid driver in
the sleep scenario:
in the sleep flow, while the user is still triggering the i2c-hid
interrupt, we get the following function call:
handle_level_irq()
-> mask_ack_irq()
-> mask_irq()
i2c_hid_core_suspend()
-> disable_irq()
-> __irq_disable()
-> irq_state_set_disabled()
-> irq_state_set_masked()
irq_thread_fn()
-> irq_finalize_oneshot()
-> if (!desc->threads_oneshot && !irqd_irq_disabled() &&
irqd_irq_masked())
unmask_threaded_irq()
-> unmask_irq()
That is, when __irq_disable() is called between mask_irq() and
irq_finalize_oneshot(), the code in irq_finalize_oneshot() will cause
the !irqd_irq_disabled() fails to enter the unmask_irq() branch, which
causes mask_irq/unmask_irq to be called unpaired and the i2c-hid
interrupt to be masked.
Since mask_irq/unmask_irq and irq_disabled() belong to two different
hardware registers or policies, the !irqd_irq_disabled() assertion may
not be used to determine whether unmask_irq() needs to be called.
No. That's fundamentally wrong.
Disabled interrupts are disabled and can only be reenabled by the
corresponding enable call. The existing code is entirely correct.
What you are trying to do is unmasking a disabled interrupt, which
results in inconsistent state.
Which interrupt chip is involved here?
i2c hid driver use gpio interrupt controller like
drivers/gpio/gpio-dwapb.c, The gpio interrupt controller code implements
handle_level_irq() and irq_disabled().
Thanks,
tglx
According to my code tracking and analysis:
Normally, when using the i2c hid device, the gpio interrupt controller's
mask_irq() and unmask_irq() are called in pairs.For example, the process
is as follows:
mask_irq();
if (!irqd_irq_disabled() && irqd_irq_masked())
unmask_irq();
irq_state_set_disabled();
irq_state_set_masked();
In this process, unmask_irq() will be called, and the following
mask_irq()/unmask_irq() will return directly.
But when doing a sleep process, such as suspend to RAM,
i2c_hid_core_suspend() of the i2c hid driver is called, which implements
the disable_irq() function, which finally calls __irq_disable(). Because
the desc parameter is set to the __irq_disabled() function without a
lock (desk->lock), the __irq_disabled() function can be called during
the execution of the handle_level_irq() function, which causes the
following:
mask_irq();
irq_state_set_disabled();
irq_state_set_masked();
if (!irqd_irq_disabled() && irqd_irq_masked())
unmask_irq();
In this scenario, unmask_irq() will not be called, and then gpio
corresponding interrupt pin will be masked. Finally, in the suspend()
process driven by gpio interrupt controller, the interrupt mask register
will be saved, and then masked will continue to be read when resuming ()
process. After the kernel resumed, the i2c hid gpio interrupt was masked
and the i2c hid device was unavailable.
