RE: [PATCH RESEND] usb: dwc2: Fix a bug in reading the endpoint directions from reg.

2015-02-03 Thread Kaukab, Yousaf
Hi Roshan,

 -Original Message-
 From: Roshan Pius [mailto:rp...@chromium.org]
 Sent: Monday, February 2, 2015 11:56 PM
 To: linux-usb@vger.kernel.org
 Cc: benc...@chromium.org; Kaukab, Yousaf; Roshan Pius
 Subject: [PATCH RESEND] usb: dwc2: Fix a bug in reading the endpoint
 directions from reg.
 
 According to  the DWC2 datasheet, the HWCFG1 register stores the configured
 endpoint directions for endpoints 0-15 in bit positions 0-31.
 ==
 Endpoint Direction (EpDir)
 This 32-bit field uses two bits per endpoint to determine the endpoint 
 direction.
 Endpoint
 Bits [31:30]: Endpoint 15 direction
 Bits [29:28]: Endpoint 14 direction
 
 Bits [3:2]: Endpoint 1 direction
 Bits[1:0]: Endpoint 0 direction (always BIDIR) ==
 
 The DWC2 driver is currently interpreting the contents of the register as
 directions for endpoints 1-15 which leads to an error in determining the
 configured endpoint directions in the core because the first 2 bits determine
 the direction of endpoint 0 and not 1.
 
 This is based on testing/next branch in Felipe's git.
 
 Signed-off-by: Roshan Pius rp...@chromium.org
 ---
  drivers/usb/dwc2/gadget.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index
 50ae096..706165c 100644
 --- a/drivers/usb/dwc2/gadget.c
 +++ b/drivers/usb/dwc2/gadget.c
 @@ -3167,7 +3167,7 @@ static int s3c_hsotg_hw_cfg(struct dwc2_hsotg
 *hsotg)
   hsotg-eps_out[0] = hsotg-eps_in[0];
 
   cfg = readl(hsotg-regs + GHWCFG1);
 - for (i = 1; i  hsotg-num_of_eps; i++, cfg = 2) {
 + for (i = 1, cfg = 2; i  hsotg-num_of_eps; i++, cfg = 2) {

Nice catch! I wouldn't have found it on my hardware as GHWCFG1 is always 0.

   ep_type = cfg  3;
   /* Direction in or both */
   if (!(ep_type  2)) {
 --
 2.2.0.rc0.207.ga3a616c

BR,
Yousaf

--
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: [PATCH RESEND] usb: dwc2: Fix a bug in reading the endpoint directions from reg.

2015-02-03 Thread John Youn
On 02/02/2015 02:55 PM, Roshan Pius wrote:
 According to  the DWC2 datasheet, the HWCFG1 register stores
 the configured endpoint directions for endpoints 0-15 in bit positions
 0-31.
 ==
 Endpoint Direction (EpDir)
 This 32-bit field uses two bits per endpoint to determine the endpoint
 direction.
 Endpoint
 Bits [31:30]: Endpoint 15 direction
 Bits [29:28]: Endpoint 14 direction
 
 Bits [3:2]: Endpoint 1 direction
 Bits[1:0]: Endpoint 0 direction (always BIDIR)
 ==
 
 The DWC2 driver is currently interpreting the contents of the register
 as directions for endpoints 1-15 which leads to an error in determining
 the configured endpoint directions in the core because the first 2 bits
 determine the direction of endpoint 0 and not 1.
 
 This is based on testing/next branch in Felipe's git.
 
 Signed-off-by: Roshan Pius rp...@chromium.org
 ---
  drivers/usb/dwc2/gadget.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
 index 50ae096..706165c 100644
 --- a/drivers/usb/dwc2/gadget.c
 +++ b/drivers/usb/dwc2/gadget.c
 @@ -3167,7 +3167,7 @@ static int s3c_hsotg_hw_cfg(struct dwc2_hsotg *hsotg)
   hsotg-eps_out[0] = hsotg-eps_in[0];
  
   cfg = readl(hsotg-regs + GHWCFG1);
 - for (i = 1; i  hsotg-num_of_eps; i++, cfg = 2) {
 + for (i = 1, cfg = 2; i  hsotg-num_of_eps; i++, cfg = 2) {
   ep_type = cfg  3;
   /* Direction in or both */
   if (!(ep_type  2)) {
 


Good catch on this. 

Acked-by: John Youn johny...@synopsys.com


Hi Felipe,

Any chance you'll accept this into your 3.20 queue?

It affects any DWC2 core instance that has unidirectional endpoints defined.

Regards,
John







--
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