diff mbox series

usb: cdns3: Enable TDL_CHK only for OUT ep

Message ID 1621263912-13175-1-git-send-email-sparmar@cadence.com
State New
Headers show
Series usb: cdns3: Enable TDL_CHK only for OUT ep | expand

Commit Message

Sanket Parmar May 17, 2021, 3:05 p.m. UTC 
ZLP gets stuck if TDL_CHK bit is set and TDL_FROM_TRB is used
as TDL source for IN endpoints. To fix it, TDL_CHK is only
enabled for OUT endpoints.

Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Reported-by: Aswath Govindraju <a-govindraju@ti.com>
Signed-off-by: Sanket Parmar <sparmar@cadence.com>
---
 drivers/usb/cdns3/cdns3-gadget.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

Comments

Peter Chen May 22, 2021, 10:09 a.m. UTC | #1 
On 21-05-17 17:05:12, Sanket Parmar wrote:
> ZLP gets stuck if TDL_CHK bit is set and TDL_FROM_TRB is used

> as TDL source for IN endpoints. To fix it, TDL_CHK is only

> enabled for OUT endpoints.

> 

> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")

> Reported-by: Aswath Govindraju <a-govindraju@ti.com>

> Signed-off-by: Sanket Parmar <sparmar@cadence.com>

> ---

>  drivers/usb/cdns3/cdns3-gadget.c |    8 +++-----

>  1 files changed, 3 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c

> index 105855a..f3c0276 100644

> --- a/drivers/usb/cdns3/cdns3-gadget.c

> +++ b/drivers/usb/cdns3/cdns3-gadget.c

> @@ -2007,7 +2007,7 @@ static void cdns3_configure_dmult(struct cdns3_device *priv_dev,

>  		else

>  			mask = BIT(priv_ep->num);

>  

> -		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC) {

> +		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC  && !priv_ep->dir) {

>  			cdns3_set_register_bit(&regs->tdl_from_trb, mask);

>  			cdns3_set_register_bit(&regs->tdl_beh, mask);

>  			cdns3_set_register_bit(&regs->tdl_beh2, mask);

> @@ -2046,15 +2046,13 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)

>  	case USB_ENDPOINT_XFER_INT:

>  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_INT);

>  

> -		if ((priv_dev->dev_ver == DEV_VER_V2 && !priv_ep->dir) ||

> -		    priv_dev->dev_ver > DEV_VER_V2)

> +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

>  			ep_cfg |= EP_CFG_TDL_CHK;

>  		break;

>  	case USB_ENDPOINT_XFER_BULK:

>  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_BULK);

>  

> -		if ((priv_dev->dev_ver == DEV_VER_V2  && !priv_ep->dir) ||

> -		    priv_dev->dev_ver > DEV_VER_V2)

> +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

>  			ep_cfg |= EP_CFG_TDL_CHK;

>  		break;

>  	default:

> -- 


Sanket & Pawel, please confirm this behaviour could apply for DEV_VER_V3,
that is TDL_CHK can't be used for bulk in for DEV_VER_V3?

-- 

Thanks,
Peter Chen
Sanket Parmar May 24, 2021, 5:39 a.m. UTC | #2 
> On 21-05-17 17:05:12, Sanket Parmar wrote:

> > ZLP gets stuck if TDL_CHK bit is set and TDL_FROM_TRB is used

> > as TDL source for IN endpoints. To fix it, TDL_CHK is only

> > enabled for OUT endpoints.

> >

> > Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")

> > Reported-by: Aswath Govindraju <a-govindraju@ti.com>

> > Signed-off-by: Sanket Parmar <sparmar@cadence.com>

> > ---

> >  drivers/usb/cdns3/cdns3-gadget.c |    8 +++-----

> >  1 files changed, 3 insertions(+), 5 deletions(-)

> >

> > diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-

> gadget.c

> > index 105855a..f3c0276 100644

> > --- a/drivers/usb/cdns3/cdns3-gadget.c

> > +++ b/drivers/usb/cdns3/cdns3-gadget.c

> > @@ -2007,7 +2007,7 @@ static void cdns3_configure_dmult(struct

> cdns3_device *priv_dev,

> >  		else

> >  			mask = BIT(priv_ep->num);

> >

> > -		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC) {

> > +		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC  && !priv_ep-

> >dir) {

> >  			cdns3_set_register_bit(&regs->tdl_from_trb, mask);

> >  			cdns3_set_register_bit(&regs->tdl_beh, mask);

> >  			cdns3_set_register_bit(&regs->tdl_beh2, mask);

> > @@ -2046,15 +2046,13 @@ int cdns3_ep_config(struct cdns3_endpoint

> *priv_ep, bool enable)

> >  	case USB_ENDPOINT_XFER_INT:

> >  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_INT);

> >

> > -		if ((priv_dev->dev_ver == DEV_VER_V2 && !priv_ep->dir) ||

> > -		    priv_dev->dev_ver > DEV_VER_V2)

> > +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

> >  			ep_cfg |= EP_CFG_TDL_CHK;

> >  		break;

> >  	case USB_ENDPOINT_XFER_BULK:

> >  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_BULK);

> >

> > -		if ((priv_dev->dev_ver == DEV_VER_V2  && !priv_ep->dir) ||

> > -		    priv_dev->dev_ver > DEV_VER_V2)

> > +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

> >  			ep_cfg |= EP_CFG_TDL_CHK;

> >  		break;

> >  	default:

> > --

> 

> Sanket & Pawel, please confirm this behaviour could apply for DEV_VER_V3,

> that is TDL_CHK can't be used for bulk in for DEV_VER_V3?


Yes, TDL_CHK shouldn't be used for bulk in for DEV_VER_V3.

> 

> --

> 

> Thanks,

> Peter Chen
Aswath Govindraju May 26, 2021, 3 p.m. UTC | #3 
Hi Peter,

On 17/05/21 8:35 pm, Sanket Parmar wrote:
> ZLP gets stuck if TDL_CHK bit is set and TDL_FROM_TRB is used

> as TDL source for IN endpoints. To fix it, TDL_CHK is only

> enabled for OUT endpoints.

> 

> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")

> Reported-by: Aswath Govindraju <a-govindraju@ti.com>

> Signed-off-by: Sanket Parmar <sparmar@cadence.com>

> ---


May I know if this patch will be picked up as a bug fix for -rc.

Thanks,
Aswath

>  drivers/usb/cdns3/cdns3-gadget.c |    8 +++-----

>  1 files changed, 3 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c

> index 105855a..f3c0276 100644

> --- a/drivers/usb/cdns3/cdns3-gadget.c

> +++ b/drivers/usb/cdns3/cdns3-gadget.c

> @@ -2007,7 +2007,7 @@ static void cdns3_configure_dmult(struct cdns3_device *priv_dev,

>  		else

>  			mask = BIT(priv_ep->num);

>  

> -		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC) {

> +		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC  && !priv_ep->dir) {

>  			cdns3_set_register_bit(&regs->tdl_from_trb, mask);

>  			cdns3_set_register_bit(&regs->tdl_beh, mask);

>  			cdns3_set_register_bit(&regs->tdl_beh2, mask);

> @@ -2046,15 +2046,13 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)

>  	case USB_ENDPOINT_XFER_INT:

>  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_INT);

>  

> -		if ((priv_dev->dev_ver == DEV_VER_V2 && !priv_ep->dir) ||

> -		    priv_dev->dev_ver > DEV_VER_V2)

> +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

>  			ep_cfg |= EP_CFG_TDL_CHK;

>  		break;

>  	case USB_ENDPOINT_XFER_BULK:

>  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_BULK);

>  

> -		if ((priv_dev->dev_ver == DEV_VER_V2  && !priv_ep->dir) ||

> -		    priv_dev->dev_ver > DEV_VER_V2)

> +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

>  			ep_cfg |= EP_CFG_TDL_CHK;

>  		break;

>  	default:

>
Peter Chen May 27, 2021, 1:23 a.m. UTC | #4 
On 21-05-26 20:30:37, Aswath Govindraju wrote:
> Hi Peter,

> 

> On 17/05/21 8:35 pm, Sanket Parmar wrote:

> > ZLP gets stuck if TDL_CHK bit is set and TDL_FROM_TRB is used

> > as TDL source for IN endpoints. To fix it, TDL_CHK is only

> > enabled for OUT endpoints.

> > 

> > Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")

> > Reported-by: Aswath Govindraju <a-govindraju@ti.com>

> > Signed-off-by: Sanket Parmar <sparmar@cadence.com>

> > ---

> 

> May I know if this patch will be picked up as a bug fix for -rc.


Yes, it has already at my fixes tree, to make sure no one report issues,
I will send to Greg after several days later.

Peter
> 

> Thanks,

> Aswath

> 

> >  drivers/usb/cdns3/cdns3-gadget.c |    8 +++-----

> >  1 files changed, 3 insertions(+), 5 deletions(-)

> > 

> > diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c

> > index 105855a..f3c0276 100644

> > --- a/drivers/usb/cdns3/cdns3-gadget.c

> > +++ b/drivers/usb/cdns3/cdns3-gadget.c

> > @@ -2007,7 +2007,7 @@ static void cdns3_configure_dmult(struct cdns3_device *priv_dev,

> >  		else

> >  			mask = BIT(priv_ep->num);

> >  

> > -		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC) {

> > +		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC  && !priv_ep->dir) {

> >  			cdns3_set_register_bit(&regs->tdl_from_trb, mask);

> >  			cdns3_set_register_bit(&regs->tdl_beh, mask);

> >  			cdns3_set_register_bit(&regs->tdl_beh2, mask);

> > @@ -2046,15 +2046,13 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)

> >  	case USB_ENDPOINT_XFER_INT:

> >  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_INT);

> >  

> > -		if ((priv_dev->dev_ver == DEV_VER_V2 && !priv_ep->dir) ||

> > -		    priv_dev->dev_ver > DEV_VER_V2)

> > +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

> >  			ep_cfg |= EP_CFG_TDL_CHK;

> >  		break;

> >  	case USB_ENDPOINT_XFER_BULK:

> >  		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_BULK);

> >  

> > -		if ((priv_dev->dev_ver == DEV_VER_V2  && !priv_ep->dir) ||

> > -		    priv_dev->dev_ver > DEV_VER_V2)

> > +		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)

> >  			ep_cfg |= EP_CFG_TDL_CHK;

> >  		break;

> >  	default:

> > 

> 


-- 

Thanks,
Peter Chen
diff mbox series

Patch

diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index 105855a..f3c0276 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -2007,7 +2007,7 @@  static void cdns3_configure_dmult(struct cdns3_device *priv_dev,
 		else
 			mask = BIT(priv_ep->num);
 
-		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC) {
+		if (priv_ep->type != USB_ENDPOINT_XFER_ISOC  && !priv_ep->dir) {
 			cdns3_set_register_bit(&regs->tdl_from_trb, mask);
 			cdns3_set_register_bit(&regs->tdl_beh, mask);
 			cdns3_set_register_bit(&regs->tdl_beh2, mask);
@@ -2046,15 +2046,13 @@  int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)
 	case USB_ENDPOINT_XFER_INT:
 		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_INT);
 
-		if ((priv_dev->dev_ver == DEV_VER_V2 && !priv_ep->dir) ||
-		    priv_dev->dev_ver > DEV_VER_V2)
+		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)
 			ep_cfg |= EP_CFG_TDL_CHK;
 		break;
 	case USB_ENDPOINT_XFER_BULK:
 		ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_BULK);
 
-		if ((priv_dev->dev_ver == DEV_VER_V2  && !priv_ep->dir) ||
-		    priv_dev->dev_ver > DEV_VER_V2)
+		if (priv_dev->dev_ver >= DEV_VER_V2 && !priv_ep->dir)
 			ep_cfg |= EP_CFG_TDL_CHK;
 		break;
 	default: