3.7 kernel hangs when doing scp

Commit Message

Fabio Estevam Dec. 21, 2012, 9:24 a.m. UTC

Hi Peter,

On Fri, Dec 21, 2012 at 12:22 AM, Peter Chen <[e-mail]> wrote:

> Current chipidea driver only considers disable stream mode at device
> mode, in fact, it may be related to below chipidea bug, and needs
> to consider all usb modes.
>
> STAR 9000378958
> Title: Non-Double Word Aligned Buffer Address Sometimes Causes Host to Hang on OUT Retry
> www.synopsys.com/dw/star.php?c=dwc_usb2_hs_otg_controller&fixedIn=2.20a
>
> To fix this, we need to add CI13XXX_DISABLE_STREAMING after role->start/init.

Yes, setting CI13XXX_DISABLE_STREAMING inside ci_role_start does work.

If you think the patch below is fine I can properly submit it.

---
 drivers/usb/chipidea/ci.h |   39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

 	enum ci_role role = ci->role;
@@ -307,6 +293,27 @@ static inline u32 hw_test_and_write(struct
ci13xxx *ci, enum ci13xxx_regs reg,
 	return (val & mask) >> ffs_nr(mask);
 }

+
+static inline int ci_role_start(struct ci13xxx *ci, enum ci_role role)
+{
+	int ret;
+
+	if (role >= CI_ROLE_END)
+		return -EINVAL;
+
+	if (!ci->roles[role])
+		return -ENXIO;
+
+	ret = ci->roles[role]->start(ci);
+	if (!ret)
+		ci->role = role;
+
+	if (ci->platdata->flags & CI13XXX_DISABLE_STREAMING)
+		hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
+
+	return ret;
+}
+
 int hw_device_reset(struct ci13xxx *ci, u32 mode);

 int hw_port_test_set(struct ci13xxx *ci, u8 mode);
--

Comments

Wolfram Sang Dec. 21, 2012, 11:49 a.m. UTC | #1

On Fri, Dec 21, 2012 at 07:24:14AM -0200, Fabio Estevam wrote:
> Hi Peter,
> 
> On Fri, Dec 21, 2012 at 12:22 AM, Peter Chen <[e-mail]> wrote:
> 
> > Current chipidea driver only considers disable stream mode at device
> > mode, in fact, it may be related to below chipidea bug, and needs
> > to consider all usb modes.
> >
> > STAR 9000378958
> > Title: Non-Double Word Aligned Buffer Address Sometimes Causes Host to Hang on OUT Retry
> > www.synopsys.com/dw/star.php?c=dwc_usb2_hs_otg_controller&fixedIn=2.20a
> >
> > To fix this, we need to add CI13XXX_DISABLE_STREAMING after role->start/init.
> 
> Yes, setting CI13XXX_DISABLE_STREAMING inside ci_role_start does work.
> 
> If you think the patch below is fine I can properly submit it.

Why did you need to move this function?

Fabio Estevam Dec. 21, 2012, 11:53 a.m. UTC | #2

On Fri, Dec 21, 2012 at 9:49 AM, Wolfram Sang <[e-mail]> wrote:

>> Yes, setting CI13XXX_DISABLE_STREAMING inside ci_role_start does work.
>>
>> If you think the patch below is fine I can properly submit it.
>
> Why did you need to move this function?

Only to make the compiler happy. hw_write() is now used by
ci_role_start(), so I re-ordered so that  hw_write comes first in the
code and compiler does not complain.

Regards,

Fabio Estevam

Wolfram Sang Dec. 21, 2012, 11:57 a.m. UTC | #3

> Only to make the compiler happy. hw_write() is now used by
> ci_role_start(), so I re-ordered so that  hw_write comes first in the
> code and compiler does not complain.

Yup, spotted this on second glance. If you submit the patch, might be
helpful to mention this.

Patch

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index e25d126..7fe652a 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -18,6 +18,8 @@ 
 #include <linux/usb.h>
 #include <linux/usb/gadget.h>

+#define USBMODE_CI_SDIS       BIT(4)
+
 /******************************************************************************
  * DEFINE
  *****************************************************************************/
@@ -173,22 +175,6 @@  static inline struct ci_role_driver
*ci_role(struct ci13xxx *ci)
 	return ci->roles[ci->role];
 }

-static inline int ci_role_start(struct ci13xxx *ci, enum ci_role role)
-{
-	int ret;
-
-	if (role >= CI_ROLE_END)
-		return -EINVAL;
-
-	if (!ci->roles[role])
-		return -ENXIO;
-
-	ret = ci->roles[role]->start(ci);
-	if (!ret)
-		ci->role = role;
-	return ret;
-}
-
 static inline void ci_role_stop(struct ci13xxx *ci)
 {