Hi, On Fri, Dec 13, 2013 at 08:48:30AM +0100, Andreas Larsson wrote: > >On Wed, Dec 04, 2013 at 09:13:58AM +0100, Andreas Larsson wrote: > >>+static void gr_finish_request(struct gr_ep *ep, struct gr_request *req, > >>+ int status) > >>+{ > >>+ struct gr_udc *dev; > >>+ > >>+ list_del_init(&req->queue); > >>+ > >>+ if (likely(req->req.status == -EINPROGRESS)) > >>+ req->req.status = status; > >>+ else > >>+ status = req->req.status; > >>+ > >>+ dev = ep->dev; > >>+ usb_gadget_unmap_request(&dev->gadget, &req->req, ep->is_in); > >>+ gr_free_dma_desc_chain(dev, req); > >>+ > >>+ if (ep->is_in) /* For OUT, actual gets updated bit by bit */ > >>+ req->req.actual = req->req.length; > >>+ > >>+ if (!status) { > >>+ if (ep->is_in) > >>+ gr_dbgprint_request("SENT", ep, req); > >>+ else > >>+ gr_dbgprint_request("RECV", ep, req); > >>+ } > >>+ > >>+ /* Prevent changes to ep->queue during callback */ > >>+ ep->callback = 1; > >>+ if (req == dev->ep0reqo && !status) { > >>+ if (req->setup) > >>+ gr_ep0_setup(dev, req); > >>+ else > >>+ dev_err(dev->dev, > >>+ "Unexpected non setup packet on ep0in\n"); > >>+ } else if (req->req.complete) { > >>+ unsigned long flags; > >>+ > >>+ /* > >>+ * Complete should be called with interrupts disabled according > >>+ * to the contract of struct usb_request > >>+ */ > >>+ local_irq_save(flags); > > > >sorry but this driver isn't ready for inclusion. local_irq_save() is a > >pretty good hint that there's something wrong in the driver. Consider > >the fact that local_irq_save() will disable preemption even when > >CONFIG_PREEMPT_FULL is enabled and you have a bit a problem. > > This connection between local_irq_save and CONFIG_PREEMPT_RT_FULL was > unknown to me. Sure, I can disable interrupts right at spin lock > time.
that's better. > >Also, the way you're using thread IRQs is quite wrong. I can't let that > >pass and get merged upstream, sorry. > > What is quite wrong? What is it that I need to fix? Ideally the hardirq handler should be usually to actually check if $this_device generated the IRQ, that should involve reading a IRQSTATUS register of some sort. Sure, check that IRQs are actually enabled, but you also need to read STATUS register before waking the thread up. -- balbi
signature.asc
Description: Digital signature