[PATCH 0/3] usb: cdns3: bug-fixes for usb-linus

[PATCH 0/3] usb: cdns3: bug-fixes for usb-linus

[Peter Chen]

Peter Chen (3):
  usb: cdns3: ep0: fix the test mode set incorrectly
  usb: cdns3: trace: using correct dir value
  usb: cdns3: ep0: add spinlock for cdns3_check_new_setup

 drivers/usb/cdns3/ep0.c   | 10 ++++++----
 drivers/usb/cdns3/trace.h |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH 1/3] usb: cdns3: ep0: fix the test mode set incorrectly

[Peter Chen]

The 'tmode' is ctrl->wIndex, changing it as the real test
mode value for register assignment.

Cc: <stable@vger.kernel.org>
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/ep0.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
index 2465a84e8fee..74a1ff5000ba 100644
--- a/drivers/usb/cdns3/ep0.c
+++ b/drivers/usb/cdns3/ep0.c
@@ -327,7 +327,8 @@ static int cdns3_ep0_feature_handle_device(struct cdns3_device *priv_dev,
 		if (!set || (tmode & 0xff) != 0)
 			return -EINVAL;
 
-		switch (tmode >> 8) {
+		tmode >>= 8;
+		switch (tmode) {
 		case TEST_J:
 		case TEST_K:
 		case TEST_SE0_NAK:
-- 
2.17.1

[PATCH 2/3] usb: cdns3: trace: using correct dir value

[Peter Chen]

It should use the correct direction value from register, not depends
on previous software setting. It fixed the EP number wrong issue at
trace when the TRBERR interrupt occurs for EP0IN.

When the EP0IN IOC has finished, software prepares the setup packet
request, the expected direction is OUT, but at that time, the TRBERR
for EP0IN may occur since it is DMULT mode, the DMA does not stop
until TRBERR has met.

Cc: <stable@vger.kernel.org>
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/trace.h b/drivers/usb/cdns3/trace.h
index de2c34d5bfc5..0a2a3269bfac 100644
--- a/drivers/usb/cdns3/trace.h
+++ b/drivers/usb/cdns3/trace.h
@@ -156,7 +156,7 @@ DECLARE_EVENT_CLASS(cdns3_log_ep0_irq,
 		__dynamic_array(char, str, CDNS3_MSG_MAX)
 	),
 	TP_fast_assign(
-		__entry->ep_dir = priv_dev->ep0_data_dir;
+		__entry->ep_dir = priv_dev->selected_ep;
 		__entry->ep_sts = ep_sts;
 	),
 	TP_printk("%s", cdns3_decode_ep0_irq(__get_str(str),
-- 
2.17.1

[PATCH 3/3] usb: cdns3: ep0: add spinlock for cdns3_check_new_setup

[Peter Chen]

The other thread may access other endpoints when the cdns3_check_new_setup
is handling, add spinlock to protect it.

Cc: <stable@vger.kernel.org>
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/ep0.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
index 74a1ff5000ba..5aa69980e7ff 100644
--- a/drivers/usb/cdns3/ep0.c
+++ b/drivers/usb/cdns3/ep0.c
@@ -705,15 +705,17 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep,
 	int ret = 0;
 	u8 zlp = 0;
 
+	spin_lock_irqsave(&priv_dev->lock, flags);
 	trace_cdns3_ep0_queue(priv_dev, request);
 
 	/* cancel the request if controller receive new SETUP packet. */
-	if (cdns3_check_new_setup(priv_dev))
+	if (cdns3_check_new_setup(priv_dev)) {
+		spin_unlock_irqrestore(&priv_dev->lock, flags);
 		return -ECONNRESET;
+	}
 
 	/* send STATUS stage. Should be called only for SET_CONFIGURATION */
 	if (priv_dev->ep0_stage == CDNS3_STATUS_STAGE) {
-		spin_lock_irqsave(&priv_dev->lock, flags);
 		cdns3_select_ep(priv_dev, 0x00);
 
 		erdy_sent = !priv_dev->hw_configured_flag;
@@ -738,7 +740,6 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep,
 		return 0;
 	}
 
-	spin_lock_irqsave(&priv_dev->lock, flags);
 	if (!list_empty(&priv_ep->pending_req_list)) {
 		dev_err(priv_dev->dev,
 			"can't handle multiple requests for ep0\n");
-- 
2.17.1

RE: [PATCH 1/3] usb: cdns3: ep0: fix the test mode set incorrectly

[Pawel Laszczak]

Hi,

Reviewed-by: Pawel Laszczak <pawell@cadence.com>
>
>
>The 'tmode' is ctrl->wIndex, changing it as the real test
>mode value for register assignment.
>
>Cc: <stable@vger.kernel.org>
>Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
>Reviewed-by: Jun Li <jun.li@nxp.com>
>Signed-off-by: Peter Chen <peter.chen@nxp.com>
>---
> drivers/usb/cdns3/ep0.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
>index 2465a84e8fee..74a1ff5000ba 100644
>--- a/drivers/usb/cdns3/ep0.c
>+++ b/drivers/usb/cdns3/ep0.c
>@@ -327,7 +327,8 @@ static int cdns3_ep0_feature_handle_device(struct cdns3_device *priv_dev,
> 		if (!set || (tmode & 0xff) != 0)
> 			return -EINVAL;
>
>-		switch (tmode >> 8) {
>+		tmode >>= 8;
>+		switch (tmode) {

For me it's looks the same, but it's ok. 

> 		case TEST_J:
> 		case TEST_K:
> 		case TEST_SE0_NAK:
>--
>2.17.1

RE: [PATCH 2/3] usb: cdns3: trace: using correct dir value

[Pawel Laszczak]

>
>It should use the correct direction value from register, not depends
>on previous software setting. It fixed the EP number wrong issue at
>trace when the TRBERR interrupt occurs for EP0IN.
>
>When the EP0IN IOC has finished, software prepares the setup packet
>request, the expected direction is OUT, but at that time, the TRBERR
>for EP0IN may occur since it is DMULT mode, the DMA does not stop
>until TRBERR has met.
>
>Cc: <stable@vger.kernel.org>
>Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
>Signed-off-by: Peter Chen <peter.chen@nxp.com>

Reviewed-by: Pawel Laszczak <pawell@cadence.com>

>---
> drivers/usb/cdns3/trace.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/usb/cdns3/trace.h b/drivers/usb/cdns3/trace.h
>index de2c34d5bfc5..0a2a3269bfac 100644
>--- a/drivers/usb/cdns3/trace.h
>+++ b/drivers/usb/cdns3/trace.h
>@@ -156,7 +156,7 @@ DECLARE_EVENT_CLASS(cdns3_log_ep0_irq,
> 		__dynamic_array(char, str, CDNS3_MSG_MAX)
> 	),
> 	TP_fast_assign(
>-		__entry->ep_dir = priv_dev->ep0_data_dir;
>+		__entry->ep_dir = priv_dev->selected_ep;
> 		__entry->ep_sts = ep_sts;
> 	),
> 	TP_printk("%s", cdns3_decode_ep0_irq(__get_str(str),
>--
>2.17.1

RE: [PATCH 3/3] usb: cdns3: ep0: add spinlock for cdns3_check_new_setup

[Pawel Laszczak]

>
>The other thread may access other endpoints when the cdns3_check_new_setup
>is handling, add spinlock to protect it.
>
>Cc: <stable@vger.kernel.org>
>Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
>Signed-off-by: Peter Chen <peter.chen@nxp.com>

Reviewed-by: Pawel Laszczak <pawell@cadence.com>

>---
> drivers/usb/cdns3/ep0.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
>index 74a1ff5000ba..5aa69980e7ff 100644
>--- a/drivers/usb/cdns3/ep0.c
>+++ b/drivers/usb/cdns3/ep0.c
>@@ -705,15 +705,17 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep,
> 	int ret = 0;
> 	u8 zlp = 0;
>
>+	spin_lock_irqsave(&priv_dev->lock, flags);
> 	trace_cdns3_ep0_queue(priv_dev, request);
>
> 	/* cancel the request if controller receive new SETUP packet. */
>-	if (cdns3_check_new_setup(priv_dev))
>+	if (cdns3_check_new_setup(priv_dev)) {
>+		spin_unlock_irqrestore(&priv_dev->lock, flags);
> 		return -ECONNRESET;
>+	}
>
> 	/* send STATUS stage. Should be called only for SET_CONFIGURATION */
> 	if (priv_dev->ep0_stage == CDNS3_STATUS_STAGE) {
>-		spin_lock_irqsave(&priv_dev->lock, flags);
> 		cdns3_select_ep(priv_dev, 0x00);
>
> 		erdy_sent = !priv_dev->hw_configured_flag;
>@@ -738,7 +740,6 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep,
> 		return 0;
> 	}
>
>-	spin_lock_irqsave(&priv_dev->lock, flags);
> 	if (!list_empty(&priv_ep->pending_req_list)) {
> 		dev_err(priv_dev->dev,
> 			"can't handle multiple requests for ep0\n");
>--
>2.17.1

RE: [PATCH 1/3] usb: cdns3: ep0: fix the test mode set incorrectly

[Peter Chen]

 
> >diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c index
> >2465a84e8fee..74a1ff5000ba 100644
> >--- a/drivers/usb/cdns3/ep0.c
> >+++ b/drivers/usb/cdns3/ep0.c
> >@@ -327,7 +327,8 @@ static int cdns3_ep0_feature_handle_device(struct
> cdns3_device *priv_dev,
> > 		if (!set || (tmode & 0xff) != 0)
> > 			return -EINVAL;
> >
> >-		switch (tmode >> 8) {
> >+		tmode >>= 8;
> >+		switch (tmode) {
> 
> For me it's looks the same, but it's ok.
> 

Pawel, please check the coming code, it uses tmode to set the register which
is incorrect before shift. (line 336)

328                 tmode >>= 8;
329                 switch (tmode) {
330                 case TEST_J:
331                 case TEST_K:
332                 case TEST_SE0_NAK:
333                 case TEST_PACKET:
334                         cdns3_set_register_bit(&priv_dev->regs->usb_cmd,
335                                                USB_CMD_STMODE |
336                                                USB_STS_TMODE_SEL(tmode - 1));
337                         break;

Peter

RE: [PATCH 1/3] usb: cdns3: ep0: fix the test mode set incorrectly

[Pawel Laszczak]

>
>> >diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c index
>> >2465a84e8fee..74a1ff5000ba 100644
>> >--- a/drivers/usb/cdns3/ep0.c
>> >+++ b/drivers/usb/cdns3/ep0.c
>> >@@ -327,7 +327,8 @@ static int cdns3_ep0_feature_handle_device(struct
>> cdns3_device *priv_dev,
>> > 		if (!set || (tmode & 0xff) != 0)
>> > 			return -EINVAL;
>> >
>> >-		switch (tmode >> 8) {
>> >+		tmode >>= 8;
>> >+		switch (tmode) {
>>
>> For me it's looks the same, but it's ok.
>>
>
>Pawel, please check the coming code, it uses tmode to set the register which
>is incorrect before shift. (line 336)

Rigth, thanks for clarification.

>
>328                 tmode >>= 8;
>329                 switch (tmode) {
>330                 case TEST_J:
>331                 case TEST_K:
>332                 case TEST_SE0_NAK:
>333                 case TEST_PACKET:
>334                         cdns3_set_register_bit(&priv_dev->regs->usb_cmd,
>335                                                USB_CMD_STMODE |
>336                                                USB_STS_TMODE_SEL(tmode - 1));
>337                         break;
>
>Peter