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

2014-02-04 Thread Dinh Nguyen
On Tue, 2014-02-04 at 21:43 +, Paul Zimmerman wrote:
> > 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 
> > Cc: Paul Zimmerman 
> > Cc: Matt Porter 
> > Cc: Matthijs Kooijman 
> > ---
> > 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?
> 

Yes, the attached patch also works for me.

Thanks,
Dinh


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-02-04 Thread 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 
> Cc: Paul Zimmerman 
> Cc: Matt Porter 
> Cc: Matthijs Kooijman 
> ---
> 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


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

2014-02-04 Thread dinguyen
From: Dinh Nguyen 

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 
Cc: Paul Zimmerman 
Cc: Matt Porter 
Cc: Matthijs Kooijman 
---
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);
+   }
+}
+
+/**
  * 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;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html