isp1362-hcd.c: usb message always saved in case of underrun

PATCH In review Claudio Scordino 2 participants 7 messages Jun 27, 2012 – Jul 30, 2012 USB

lore

Share & tools

lore

Claudio Scordino

author

2012-06-27 16:11 UTC #0 lore raw

PATCH 1/1 isp1362-hcd.c: usb message always saved in case of underrun

Hi Olav,

please find below a patch for the isp1362-hcd.c driver to always
save the message in case of underrun. More information is provided inside the patch comment. Let us know if you need any further information.

Best regards,

Claudio

Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli <[email protected]>

The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.

The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.

Signed-off-by Bruno Morelli <[email protected]>

Signed-off-by Claudio Scordino <[email protected]>

Tested-by Bruno Morelli <[email protected]>

---
 drivers/usb/host/isp1362-hcd.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

drivers/usb/host/isp1362-hcd.c +6 -5

index 2ed112d..61bf1b2 100644
@@ -543,13 +543,14 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
 			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
 			    short_ok ? "" : "not_",
 			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			BUG_ON(urb->actual_length >
+			    urb->transfer_buffer_length);
 			if (usb_pipecontrol(urb->pipe)) {
 				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				BUG_ON(urb->actual_length > urb->transfer_buffer_length);
 
 				if (urb->status == -EINPROGRESS)
 					urb->status = cc_to_error[PTD_DATAUNDERRUN];

Signature · 1 line

1.7.1

Greg KH

maintainer

2012-07-06 17:42 UTC #1 lore raw

Quoted 34 lines · Claudio Scordino

Hi Olav,
please find below a patch for the isp1362-hcd.c driver to always
save the message in case of underrun. More information is provided inside the patch comment. Let us know if you need any further information.
Best regards,
Claudio
Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli <[email protected]>
The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.
The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.
Signed-off-by Bruno Morelli <[email protected]>
Signed-off-by Claudio Scordino <[email protected]>
Tested-by Bruno Morelli <[email protected]>
---
 drivers/usb/host/isp1362-hcd.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)
drivers/usb/host/isp1362-hcd.c +0 -0
index 2ed112d..61bf1b2 100644

drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 551

Quoted 9 lines

drivers/usb/host/isp1362-hcd.c +6 -0

@@ -543,13 +543,14 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
 			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
 			    short_ok ? "" : "not_",
 			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			BUG_ON(urb->actual_length >
+			    urb->transfer_buffer_length);

Please NEVER crash the machine in a driver like this, it's bad and can cause problems. Yes, I know you are just moving it from the lines below:

drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 552

Quoted 7 lines

drivers/usb/host/isp1362-hcd.c +0 -5

 			if (usb_pipecontrol(urb->pipe)) {
 				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				BUG_ON(urb->actual_length > urb->transfer_buffer_length);

But really, it should not be in the driver at all. Please remove it, at the most, do a WARN_ON() so that someone can see the problem and at least report it.

Actually, what is this checking? How can someone recover from it? Who is this check for? The developer of this driver? Another driver? Hardware developer? End user? Who would be able to fix the problem if this happens?

As it is, I can't take it, sorry.

greg k-h

Claudio Scordino

author Tested-by Bruno Morelli

2012-07-18 08:53 UTC #2 lore raw

Il 06/07/2012 19:41, Greg KH ha scritto:

Quoted 61 lines · Claudio Scordino

Quoted 34 lines
Hi Olav,
please find below a patch for the isp1362-hcd.c driver to always
save the message in case of underrun. More information is provided inside the patch comment. Let us know if you need any further information.
Best regards,
Claudio
Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli<[email protected]>
The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.
The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.
Signed-off-by Bruno Morelli<[email protected]>
Signed-off-by Claudio Scordino<[email protected]>
Tested-by Bruno Morelli<[email protected]>
---
  drivers/usb/host/isp1362-hcd.c |   11 ++++++-----
  1 files changed, 6 insertions(+), 5 deletions(-)
drivers/usb/host/isp1362-hcd.c +0 -0
index 2ed112d..61bf1b2 100644
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 551
Quoted 9 lines
drivers/usb/host/isp1362-hcd.c +6 -0
@@ -543,13 +543,14 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
  			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
  			    short_ok ? "" : "not_",
  			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			BUG_ON(urb->actual_length>
+			    urb->transfer_buffer_length);
Please NEVER crash the machine in a driver like this, it's bad and can cause problems. Yes, I know you are just moving it from the lines below:
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 552
Quoted 7 lines
drivers/usb/host/isp1362-hcd.c +0 -5
  			if (usb_pipecontrol(urb->pipe)) {
  				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				BUG_ON(urb->actual_length>  urb->transfer_buffer_length);
But really, it should not be in the driver at all. Please remove it, at the most, do a WARN_ON() so that someone can see the problem and at least report it.

Quoted 6 lines

Actually, what is this checking? How can someone recover from it? Who is this check for? The developer of this driver? Another driver? Hardware developer? End user? Who would be able to fix the problem if this happens?
As it is, I can't take it, sorry.

Hi Greg.

I understand. As you have already said, this driver is full of BUG_ON statements.

So, we can shift just the assignment as in the patch below, to have a correct behavior, leaving the BUG_ON where it already is. Then, we may propose a different patch to change BUG_ONs to WARN_ONs.

Best regards,

Claudio

Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli <[email protected]>

The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.

The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.

Signed-off-by Bruno Morelli <[email protected]>

Signed-off-by Claudio Scordino <[email protected]>

Tested-by Bruno Morelli <[email protected]>

---
 drivers/usb/host/isp1362-hcd.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

drivers/usb/host/isp1362-hcd.c +4 -4

index 2ed112d..2563263 100644
@@ -543,12 +543,12 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
 			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
 			    short_ok ? "" : "not_",
 			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
 			if (usb_pipecontrol(urb->pipe)) {
 				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
 				BUG_ON(urb->actual_length > urb->transfer_buffer_length);
 
 				if (urb->status == -EINPROGRESS)

Signature · 1 line

1.7.1

Greg KH

maintainer

2012-07-19 22:58 UTC #3 lore raw

Quoted 70 lines · Claudio Scordino

Il 06/07/2012 19:41, Greg KH ha scritto:
Quoted 61 lines · Claudio Scordino
Quoted 34 lines
Hi Olav,
please find below a patch for the isp1362-hcd.c driver to always
save the message in case of underrun. More information is provided inside the patch comment. Let us know if you need any further information.
Best regards,
Claudio
Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli<[email protected]>
The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.
The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.
Signed-off-by Bruno Morelli<[email protected]>
Signed-off-by Claudio Scordino<[email protected]>
Tested-by Bruno Morelli<[email protected]>
---
 drivers/usb/host/isp1362-hcd.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)
drivers/usb/host/isp1362-hcd.c +0 -0
index 2ed112d..61bf1b2 100644
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 551
Quoted 9 lines
drivers/usb/host/isp1362-hcd.c +6 -0
@@ -543,13 +543,14 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
 			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
 			    short_ok ? "" : "not_",
 			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			BUG_ON(urb->actual_length>
+			    urb->transfer_buffer_length);
Please NEVER crash the machine in a driver like this, it's bad and can cause problems. Yes, I know you are just moving it from the lines below:
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
Quoted 7 lines
drivers/usb/host/isp1362-hcd.c +0 -5
 			if (usb_pipecontrol(urb->pipe)) {
 				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				BUG_ON(urb->actual_length>  urb->transfer_buffer_length);
But really, it should not be in the driver at all. Please remove it, at the most, do a WARN_ON() so that someone can see the problem and at least report it.
Quoted 6 lines
Actually, what is this checking? How can someone recover from it? Who is this check for? The developer of this driver? Another driver? Hardware developer? End user? Who would be able to fix the problem if this happens?
As it is, I can't take it, sorry.

Quoted 8 lines

Hi Greg.
I understand. As you have already said, this driver is full of BUG_ON statements.
So, we can shift just the assignment as in the patch below, to have a correct behavior, leaving the BUG_ON where it already is. Then, we may propose a different patch to change BUG_ONs to WARN_ONs.

Your updated patch is much better, thanks.

But it doesn't apply to my tree right now. Can you please refresh it against the usb-next tree and resend it?

thanks,

greg k-h

Claudio Scordino

author

2012-07-20 10:26 UTC #4 lore raw

Il 20/07/2012 00:58, Greg KH ha scritto:

Quoted 76 lines · Claudio Scordino

Quoted 75 lines
Il 06/07/2012 19:41, Greg KH ha scritto:
Quoted 61 lines · Claudio Scordino
Quoted 34 lines
Hi Olav,
please find below a patch for the isp1362-hcd.c driver to always
save the message in case of underrun. More information is provided inside the patch comment. Let us know if you need any further information.
Best regards,
Claudio
Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli<[email protected]>
The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.
The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.
Signed-off-by Bruno Morelli<[email protected]>
Signed-off-by Claudio Scordino<[email protected]>
Tested-by Bruno Morelli<[email protected]>
---
  drivers/usb/host/isp1362-hcd.c |   11 ++++++-----
  1 files changed, 6 insertions(+), 5 deletions(-)
drivers/usb/host/isp1362-hcd.c +0 -0
index 2ed112d..61bf1b2 100644
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 551
Quoted 9 lines
drivers/usb/host/isp1362-hcd.c +6 -0
@@ -543,13 +543,14 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
  			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
  			    short_ok ? "" : "not_",
  			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			BUG_ON(urb->actual_length>
+			    urb->transfer_buffer_length);
Please NEVER crash the machine in a driver like this, it's bad and can cause problems. Yes, I know you are just moving it from the lines below:
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
Quoted 7 lines
drivers/usb/host/isp1362-hcd.c +0 -5
  			if (usb_pipecontrol(urb->pipe)) {
  				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				BUG_ON(urb->actual_length>   urb->transfer_buffer_length);
But really, it should not be in the driver at all. Please remove it, at the most, do a WARN_ON() so that someone can see the problem and at least report it.
Quoted 6 lines
Actually, what is this checking? How can someone recover from it? Who is this check for? The developer of this driver? Another driver? Hardware developer? End user? Who would be able to fix the problem if this happens?
As it is, I can't take it, sorry.
Hi Greg.
I understand. As you have already said, this driver is full of BUG_ON statements.

Quoted 8 lines

Quoted 3 lines
So, we can shift just the assignment as in the patch below, to have a correct behavior, leaving the BUG_ON where it already is. Then, we may propose a different patch to change BUG_ONs to WARN_ONs.
Your updated patch is much better, thanks.
But it doesn't apply to my tree right now. Can you please refresh it against the usb-next tree and resend it?

Actually, I did.

So, this means that I'm using the wrong tree...

I'm using the "usb-next" branch available on your tree at

git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git

Is this the wrong one ?

Many thanks,

Claudio

Greg KH

maintainer

2012-07-20 15:15 UTC #5 lore raw

Quoted 88 lines · Claudio Scordino

Il 20/07/2012 00:58, Greg KH ha scritto:
Quoted 76 lines · Claudio Scordino
Quoted 75 lines
Il 06/07/2012 19:41, Greg KH ha scritto:
Quoted 61 lines · Claudio Scordino
Quoted 34 lines
Hi Olav,
please find below a patch for the isp1362-hcd.c driver to always
save the message in case of underrun. More information is provided inside the patch comment. Let us know if you need any further information.
Best regards,
Claudio
Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli<[email protected]>
The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.
The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.
Signed-off-by Bruno Morelli<[email protected]>
Signed-off-by Claudio Scordino<[email protected]>
Tested-by Bruno Morelli<[email protected]>
---
 drivers/usb/host/isp1362-hcd.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)
drivers/usb/host/isp1362-hcd.c +0 -0
index 2ed112d..61bf1b2 100644
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 551
Quoted 9 lines
drivers/usb/host/isp1362-hcd.c +6 -0
@@ -543,13 +543,14 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
 			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
 			    short_ok ? "" : "not_",
 			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			BUG_ON(urb->actual_length>
+			    urb->transfer_buffer_length);
Please NEVER crash the machine in a driver like this, it's bad and can cause problems. Yes, I know you are just moving it from the lines below:
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
Quoted 7 lines
drivers/usb/host/isp1362-hcd.c +0 -5
 			if (usb_pipecontrol(urb->pipe)) {
 				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				BUG_ON(urb->actual_length>   urb->transfer_buffer_length);
But really, it should not be in the driver at all. Please remove it, at the most, do a WARN_ON() so that someone can see the problem and at least report it.
Quoted 6 lines
Actually, what is this checking? How can someone recover from it? Who is this check for? The developer of this driver? Another driver? Hardware developer? End user? Who would be able to fix the problem if this happens?
As it is, I can't take it, sorry.
Hi Greg.
I understand. As you have already said, this driver is full of BUG_ON statements.
Quoted 8 lines
Quoted 3 lines
So, we can shift just the assignment as in the patch below, to have a correct behavior, leaving the BUG_ON where it already is. Then, we may propose a different patch to change BUG_ONs to WARN_ONs.
Your updated patch is much better, thanks.
But it doesn't apply to my tree right now. Can you please refresh it against the usb-next tree and resend it?
Actually, I did.

Quoted 7 lines

So, this means that I'm using the wrong tree...
I'm using the "usb-next" branch available on your tree at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
Is this the wrong one ?

That is the correct one. It didn't work for me, so try refreshing your patch and resending it.

thanks,

greg k-h

Claudio Scordino

author Tested-by Bruno Morelli

2012-07-30 13:27 UTC #6 lore raw

Il 20/07/2012 17:15, Greg KH ha scritto:

Quoted 91 lines · Claudio Scordino

Quoted 90 lines
Il 20/07/2012 00:58, Greg KH ha scritto:
Quoted 76 lines · Claudio Scordino
Quoted 75 lines
Il 06/07/2012 19:41, Greg KH ha scritto:
Quoted 61 lines · Claudio Scordino
Quoted 34 lines
Hi Olav,
please find below a patch for the isp1362-hcd.c driver to always
save the message in case of underrun. More information is provided inside the patch comment. Let us know if you need any further information.
Best regards,
Claudio
Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli<[email protected]>
The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.
The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.
Signed-off-by Bruno Morelli<[email protected]>
Signed-off-by Claudio Scordino<[email protected]>
Tested-by Bruno Morelli<[email protected]>
---
  drivers/usb/host/isp1362-hcd.c |   11 ++++++-----
  1 files changed, 6 insertions(+), 5 deletions(-)
drivers/usb/host/isp1362-hcd.c +0 -0
index 2ed112d..61bf1b2 100644
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 551
Quoted 9 lines
drivers/usb/host/isp1362-hcd.c +6 -0
@@ -543,13 +543,14 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
  			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
  			    short_ok ? "" : "not_",
  			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			BUG_ON(urb->actual_length>
+			    urb->transfer_buffer_length);
Please NEVER crash the machine in a driver like this, it's bad and can cause problems. Yes, I know you are just moving it from the lines below:
drivers/usb/host/isp1362-hcd.c static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep) · line 552
Quoted 7 lines
drivers/usb/host/isp1362-hcd.c +0 -5
  			if (usb_pipecontrol(urb->pipe)) {
  				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				BUG_ON(urb->actual_length>    urb->transfer_buffer_length);
But really, it should not be in the driver at all. Please remove it, at the most, do a WARN_ON() so that someone can see the problem and at least report it.
Quoted 6 lines
Actually, what is this checking? How can someone recover from it? Who is this check for? The developer of this driver? Another driver? Hardware developer? End user? Who would be able to fix the problem if this happens?
As it is, I can't take it, sorry.
Hi Greg.
I understand. As you have already said, this driver is full of BUG_ON statements.
Quoted 8 lines
Quoted 3 lines
So, we can shift just the assignment as in the patch below, to have a correct behavior, leaving the BUG_ON where it already is. Then, we may propose a different patch to change BUG_ONs to WARN_ONs.
Your updated patch is much better, thanks.
But it doesn't apply to my tree right now. Can you please refresh it against the usb-next tree and resend it?
Actually, I did.
So, this means that I'm using the wrong tree...

Quoted 8 lines

Quoted 5 lines
I'm using the "usb-next" branch available on your tree at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
Is this the wrong one ?
That is the correct one. It didn't work for me, so try refreshing your patch and resending it.

Hi Greg.

I've just refreshed the patch by cloning the repo from scratch and creating the patch against the "usb-next" branch.

You can find the patch below, but the content is still the same.

Can you please check it again ? Is it possible that you have local changes not yet committed ?

Many thanks,

Claudio

Subject: isp1362-hcd.c: usb message always saved in case of underrun
From: Bruno Morelli <[email protected]>

The usb message must be saved also in case the USB endpoint is not a control endpoint (i.e., "endpoint 0"), otherwise in some circumstances we don't have a payload in case of error.

The patch has been created by tracing with usbmon the different error messages generated by this driver with respect to the ehci-hcd driver.

Signed-off-by Bruno Morelli <[email protected]>

Signed-off-by Claudio Scordino <[email protected]>

Tested-by Bruno Morelli <[email protected]>

---
 drivers/usb/host/isp1362-hcd.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

drivers/usb/host/isp1362-hcd.c +4 -4

index 2ed112d..2563263 100644
@@ -543,12 +543,12 @@ static void postproc_ep(struct isp1362_hcd *isp1362_hcd, struct isp1362_ep *ep)
 			    usb_pipein(urb->pipe) ? "IN" : "OUT", ep->nextpid,
 			    short_ok ? "" : "not_",
 			    PTD_GET_COUNT(ptd), ep->maxpacket, len);
+			/* save the data underrun error code for later and
+			 * proceed with the status stage
+			 */
+			urb->actual_length += PTD_GET_COUNT(ptd);
 			if (usb_pipecontrol(urb->pipe)) {
 				ep->nextpid = USB_PID_ACK;
-				/* save the data underrun error code for later and
-				 * proceed with the status stage
-				 */
-				urb->actual_length += PTD_GET_COUNT(ptd);
 				BUG_ON(urb->actual_length > urb->transfer_buffer_length);
 
 				if (urb->status == -EINPROGRESS)

Signature · 1 line

1.7.1