> From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx] > Sent: Wednesday, April 17, 2013 10:06 AM > > On Tue, Apr 16, 2013 at 07:39:22PM +0000, Paul Zimmerman wrote: > > > > Yes. Actually, it would probably be best to check the core's mode at > > the start of the host interrupt handler, and if it's in device mode, > > just clear ALL asserted host-only interrupts and return without doing > > anything further. > Yeah, that thought crossed my mind at some point as well. However, the > handler for DISCONNINT doesn't just clear the interrupt, it also does: > > /* Change to L3 (OFF) state */ > hsotg->lx_state = DWC2_L3; > > Is this one no longer relevant when the hardware switched to device > mode, or should the above be moved into the host interrupt handler as > well? Ah yes, good point. I guess code like this which changes the driver state should still be executed. What should _not_ be executed is any code that touches host-mode registers, because the host-mode registers disappear once the core has switched itself to device mode. > > According to the databook, the host-only interrupts are DisconnInt, > > PTxFEmp, HChInt, and PrtInt. > In the current code / my patches I also have SOF, NPTxFEmp and RxFLvl > marked (and handled) as a host interrupt, but I suspect that these three > are used for device mode as well? However, since we don't have > device handlers for them already, shouldn't they be cleared by > dwc2_hcd_intr() as well if it detects device mode? Yes, these are relevant in both modes. I wonder if it would make sense to have a stub device-mode interrupt handler for now, which would be responsible for clearing these. Once device mode is implemented, it would be replaced with a real device- mode interrupt handler. > I suspect not clearing them might currently be a race condition, but > it's just not so likely that one of the fifo's is empty at the same > moment a cable is unplugged. The SOF interrupt is more likely, but that > is cleared directly in the GINTSTS_CONIDSTSCHNG handler (at first glance > there still seems to be a race condition here, but assuming that > GINTSTS_CONIDSTSCHNG is triggered before switching to device mode, SOF > will eventually be masked out). > > Perhaps it makes sense to actually disable host (and later device) > interrupts from GINTSTS_CONIDSTSCHNG? The interrupts for host mode are > actually enabled in response to this interrupt already > (dwc2_handle_conn_id_status_change_intr -> dwc2_conn_id_status_change -> > dwc2_hcd_start -> dwc2_hcd_start_func -> dwc2_host_start -> > _dwc2_hcd_start -> dwc2_hcd_reinit -> dwc2_core_host_init -> > dwc2_enable_host_interrupts). > > If we go this route, my concerns for interrupts that are used in both > device and host mode are also lifted: These "shared" interrupts can then > disabled in disable_(host|device)_interrupts, since we can be sure that > one is called before the matching enable_(device|host)_interrupts. > Having all interrupts disabled while the core and kernel is switching > seems like a good idea to me as well. It turns out that dwc2_handle_conn_id_status_change_intr() already disables all interrupts. It calls dwc2_core_init(), which calls dwc2_core_reset(), which does a soft reset of the core. According to the databook, that resets most of the core's state, and clears most of the registers, including GINTMSK. So should not be necessary to explicitly disable interrupts. -- Paul -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
