> From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx] > Sent: Tuesday, April 16, 2013 9:47 AM > > > > --- a/drivers/staging/dwc2/core.c > > > +++ b/drivers/staging/dwc2/core.c > ... > > > + /* Enable common interrupts without disturbing host mode interrupts */ > > > + intmsk = readl(hsotg->regs + GINTMSK); > > > + intmsk |= GINTSTS_DISCONNINT | GINTSTS_MODEMIS | GINTSTS_OTGINT | > > > + GINTSTS_CONIDSTSCHNG | GINTSTS_WKUPINT | GINTSTS_USBSUSP | > > > GINTSTS_SESSREQINT; > > > > This should use the interrupt list that you just added in patch 8/10, right? > > Well, the list is not necessarily the same. > > For the common interrupts, the GINTSTS_RESTOREDONE interrupt is in > GINTMSK_COMMON, but it is never enabled. There is only some > dummy handling for this interrupt, so I'll submit a separate patch to > remove that dummy handling (just like for I2CINT). This should make > GINTMSK_COMMON identical to the above list. > > However, for host-mode interrupts, there are a number of interrupts that > are not enabled in dwc2_enable_host_interrupts, but later on when > needed. They still need to be in GINTMSK_HOST, since they must be > handled by dwc2_handle_hcd_intr and disabled by > dwc2_disable_host_interrupts (both of which use GINTMSK_HOST). > > Given that dwc2_enable_host_interrupts cannot simply use GINTMSK_HOST, > it makes sense to not use GINTMSK_COMMON in > dwc2_enable_common_interrupts either. I don't really see why. > However, I did ponder about using these masks in the enable functions, > since it helps robustness if you cannot accidentally enable an interrupt > that is never disabled. How about the following: > > - in dwc2_enable_common_interrupts we just enable GINTMSK_COMMON > - in dwc2_enable_host_interrupts we enable > GINTMSK_HOST & ~(GINTSTS_SOF|GINTSTS_PTXFEMP|GINTSTS_NPTXFEMP) > (e.g., writing a list of interrupts to enable later, instead of > only specifying the interrupts to enable now). The status of > GINTSTS_RXFLVL would need to be handled separately, as it is now, of > course. > > How does that look? Yeah, I think that's better. And maybe add a comment saying "these three interrupts will be enabled later if needed" or something like that. -- 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
