Peter Chen <peter.chen@xxxxxxx> writes: >> >> >> + irqreturn_t ret = IRQ_NONE; >> >> >> + unsigned long flags; >> >> >> + u32 reg; >> >> >> + >> >> >> + priv_dev = cdns->gadget_dev; >> >> >> + spin_lock_irqsave(&priv_dev->lock, flags); >> >> > >> >> >you're already running in hardirq context. Why do you need this lock >> >> >at all? I would be better to use the hardirq handler to mask your >> >> >interrupts, so they don't fire again, then used the top-half >> >> >(softirq) handler to actually handle the interrupts. >> >> >> > >> > This controller may be ran at SMP environment, register and flag >> > access needs to be protected among CPUs running. >> >> in hardirq context? When interrupts are already disabled? > > Interrupt handler (hardirq context) at CPU0, and process at CPU1, eg > role switch, unload module, etc. the process at CPU1 would need to disable interrupts (spin_lock_irq() or spin_lock_irqsave()), not the hardirq on CPU0 as that already runs with interrrupts disabled. https://www.kernel.org/doc/html/latest/kernel-hacking/locking.html#table-of-minimum-requirements -- balbi
Attachment:
signature.asc
Description: PGP signature
