RE: [PATCHv2] usb: dwc2: Handle the Host Port Interrupt when it occurs in device mode

[Paul Zimmerman]

> From: dingu...@altera.com [mailto:dingu...@altera.com]
> Sent: Tuesday, February 04, 2014 8:11 AM
> 
> According to the spec for the DWC2 controller, when the PRTINT interrupt 
> fires,
> the application must clear the appropriate status bit in the Host Port Control
> and Status register to clear this bit.
> 
> When disconnecting an A-cable when the dwc2 host driver, the PRTINT fires, but
> only the GINTSTS_PRTINT status is cleared, no action is done with the HPRT0
> register. The HPRT0_ENACHG bit in the HPRT0 must also be poked to correctly
> clear the GINTSTS_PRTINT interrupt.
> 
> I am seeing this behavoir on v2.93 of the DWC2 IP. When I disconnect an OTG
> A-cable adapter, the PRTINT interrupt fires when the DWC2 is in device mode
> and is never cleared.
> 
> This patch adds the function to read the HPRT0 register when the PRTINT fires
> and the dwc2 IP has already transitioned to device mode. This function is only
> clearing the HPRT0_ENACHG bit for now, but can be modified to handle more.
> 
> Signed-off-by: Dinh Nguyen <dingu...@altera.com>
> Cc: Paul Zimmerman <pa...@synopsys.com>
> Cc: Matt Porter <mpor...@linaro.org>
> Cc: Matthijs Kooijman <matth...@stdin.nl>
> ---
> v2: only need to call dwc2_handle_usb_port_intr() once
> ---
>  drivers/usb/dwc2/core_intr.c |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index bad298a..e037ad5 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -72,6 +72,23 @@ static const char *dwc2_op_state_str(struct dwc2_hsotg 
> *hsotg)
>  }
> 
>  /**
> + * dwc2_handle_usb_port_intr - handles OTG PRTINT interrupts.
> + * When the PRTINT interrupt fires, there are certain status bits in the Host
> + * Port that needs to get cleared.
> + *
> + * @hsotg: Programming view of DWC_otg controller
> + */
> +static void dwc2_handle_usb_port_intr(struct dwc2_hsotg *hsotg)
> +{
> +     u32 hprt0 = readl(hsotg->regs + HPRT0);
> +
> +     if (hprt0 & HPRT0_ENACHG) {
> +             hprt0 |= HPRT0_ENACHG;
> +             writel(hprt0, hsotg->regs + HPRT0);
> +     }
> +}

Hi Dinh,

On second thought, I'm not sure it is safe to blindly write to HPRT0
like this. There is the write-1-to-clear HPRT0_ENA bit in there that
could cause the port to get disabled by this write.

> +/**
>   * dwc2_handle_mode_mismatch_intr() - Logs a mode mismatch warning message
>   *
>   * @hsotg: Programming view of DWC_otg controller
> @@ -572,6 +589,7 @@ irq_retry:
>               if (dwc2_is_device_mode(hsotg)) {
>                       dev_dbg(hsotg->dev,
>                               " --Port interrupt received in Device 
> mode--\n");
> +                     dwc2_handle_usb_port_intr(hsotg);
>                       gintsts = GINTSTS_PRTINT;
>                       writel(gintsts, hsotg->regs + GINTSTS);
>                       retval = 1;

Also, for consistency the write to GINTSTS should be done inside
dwc2_handle_usb_port_intr() like is done for all the other handlers.

So, something like the attached alternate patch. Does it also work for
you?

-- 
Paul

dwc2-hprt0.patch
Description: dwc2-hprt0.patch