Hi folks,
I've been staring at this part of the code for a while. To make things
easier here's the part of XHCI I mean:
> irqreturn_t xhci_irq(struct usb_hcd *hcd)
> {
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> union xhci_trb *event_ring_deq;
> irqreturn_t ret = IRQ_NONE;
> dma_addr_t deq;
> u64 temp_64;
> u32 status;
>
> spin_lock(&xhci->lock);
considering this is running in hardirq context, this is lock *really*
necessary? I mean, could this race with khubd or something else?
My understanding is that this lock is completely unnecessary and it
should be safe to remove it. In fact we would have the benefit of
removing __releases()/__acquires() from a few places (some of which are
wrong, btw!!) and removing the actual spin_unlock(); do_something();
spin_lock(); which have been sprinkled in several places.
Please clarify
> /* Check if the xHC generated the interrupt, or the irq is shared */
> status = readl(&xhci->op_regs->status);
> if (status == 0xffffffff) {
[...]
--
balbi
Attachment:
signature.asc
Description: PGP signature
