Hi,
On 08/26/2014 06:19 PM, dinguyen@xxxxxxxxxxxxxxxxxxxxx wrote:
> From: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
>
> Make dwc2_handle_common_intr call the gadget interrupt function when operating
> in peripheral mode. Remove the spinlock functions in s3c_hsotg_irq as
> dwc2_handle_common_intr() already has the spinlocks.
>
> Remove duplicate interrupt conditions that was in gadget, as those are handled
> by dwc2 common interrupt handler.
>
> Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
> Acked-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> v2: Keep interrupt handler for host and peripheral modes separate
> ---
> drivers/usb/dwc2/core.h | 3 +++
> drivers/usb/dwc2/core_intr.c | 3 +++
> drivers/usb/dwc2/gadget.c | 50 ++++----------------------------------------
> 3 files changed, 10 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 3a49a00..bbb0f52 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -965,12 +965,15 @@ extern u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg);
> extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
> extern void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2);
> extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
> +irqreturn_t s3c_hsotg_irq(int irq, void *pw);
> #else
> static inline void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2) {}
> static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
> { return 0; }
> static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> { return 0; }
> +static inline irqreturn_t s3c_hsotg_irq(int irq, void *pw)
> +{ return IRQ_HANDLED; }
> #endif
>
> #if defined(CONFIG_USB_DWC2_HOST) || defined(CONFIG_USB_DWC2_DUAL_ROLE)
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index c93918b..24d4c0d 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -472,6 +472,9 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
>
> spin_lock(&hsotg->lock);
>
> + if (dwc2_is_device_mode(hsotg))
> + retval = s3c_hsotg_irq(irq, dev);
> +
> gintsts = dwc2_read_common_intr(hsotg);
> if (gintsts & ~GINTSTS_PRTINT)
> retval = IRQ_HANDLED;
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index efa68a0..a1c93bf 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2257,14 +2257,13 @@ void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
> * @irq: The IRQ number triggered
> * @pw: The pw value when registered the handler.
> */
> -static irqreturn_t s3c_hsotg_irq(int irq, void *pw)
> +irqreturn_t s3c_hsotg_irq(int irq, void *pw)
> {
> struct dwc2_hsotg *hsotg = pw;
> int retry_count = 8;
> u32 gintsts;
> u32 gintmsk;
>
> - spin_lock(&hsotg->lock);
> irq_retry:
> gintsts = readl(hsotg->regs + GINTSTS);
> gintmsk = readl(hsotg->regs + GINTMSK);
> @@ -2274,33 +2273,12 @@ irq_retry:
>
> gintsts &= gintmsk;
>
> - if (gintsts & GINTSTS_OTGINT) {
> - u32 otgint = readl(hsotg->regs + GOTGINT);
> -
> - dev_info(hsotg->dev, "OTGInt: %08x\n", otgint);
> -
> - writel(otgint, hsotg->regs + GOTGINT);
> - }
> -
> - if (gintsts & GINTSTS_SESSREQINT) {
> - dev_dbg(hsotg->dev, "%s: SessReqInt\n", __func__);
> - writel(GINTSTS_SESSREQINT, hsotg->regs + GINTSTS);
> - }
> -
> if (gintsts & GINTSTS_ENUMDONE) {
> writel(GINTSTS_ENUMDONE, hsotg->regs + GINTSTS);
>
> s3c_hsotg_irq_enumdone(hsotg);
> }
>
> - if (gintsts & GINTSTS_CONIDSTSCHNG) {
> - dev_dbg(hsotg->dev, "ConIDStsChg (DSTS=0x%08x, GOTCTL=%08x)\n",
> - readl(hsotg->regs + DSTS),
> - readl(hsotg->regs + GOTGCTL));
> -
> - writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
> - }
> -
> if (gintsts & (GINTSTS_OEPINT | GINTSTS_IEPINT)) {
> u32 daint = readl(hsotg->regs + DAINT);
> u32 daintmsk = readl(hsotg->regs + DAINTMSK);
> @@ -2381,25 +2359,6 @@ irq_retry:
> s3c_hsotg_handle_rx(hsotg);
> }
>
> - if (gintsts & GINTSTS_MODEMIS) {
> - dev_warn(hsotg->dev, "warning, mode mismatch triggered\n");
> - writel(GINTSTS_MODEMIS, hsotg->regs + GINTSTS);
> - }
> -
> - if (gintsts & GINTSTS_USBSUSP) {
> - dev_info(hsotg->dev, "GINTSTS_USBSusp\n");
> - writel(GINTSTS_USBSUSP, hsotg->regs + GINTSTS);
> -
> - call_gadget(hsotg, suspend);
> - }
> -
> - if (gintsts & GINTSTS_WKUPINT) {
> - dev_info(hsotg->dev, "GINTSTS_WkUpIn\n");
> - writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
> -
> - call_gadget(hsotg, resume);
> - }
> -
> if (gintsts & GINTSTS_ERLYSUSP) {
> dev_dbg(hsotg->dev, "GINTSTS_ErlySusp\n");
> writel(GINTSTS_ERLYSUSP, hsotg->regs + GINTSTS);
> @@ -2435,10 +2394,9 @@ irq_retry:
> if (gintsts & IRQ_RETRY_MASK && --retry_count > 0)
> goto irq_retry;
>
> - spin_unlock(&hsotg->lock);
> -
> return IRQ_HANDLED;
> }
> +EXPORT_SYMBOL(s3c_hsotg_irq);
>
> /**
> * s3c_hsotg_ep_enable - enable the given endpoint
> @@ -3490,8 +3448,8 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> s3c_hsotg_hw_cfg(hsotg);
> s3c_hsotg_init(hsotg);
>
> - ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
> - dev_name(dev), hsotg);
> + ret = devm_request_irq(dev, irq, dwc2_handle_common_intr, IRQF_SHARED,
> + dev_name(dev), hsotg);
You register the same irq handler twice for one interrupt - once here
and once in core.c, so I guess that in dual-role mode
dwc2_handle_common_intr() function will be called twice.
> if (ret < 0) {
> s3c_hsotg_phy_disable(hsotg);
> clk_disable_unprepare(hsotg->clk);
>
Best regards
Robert Baldyga
--
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
