[PATCH] umc-bus.c: fix usage of device_trylock
Hi all. I've probably found a wrong usage of device_trylock inside the driver umc-bus.c: device_trylock has the same semantics of mutex_trylock, so it returns 1 if the lock has been acquired successfully. Please, find below a patch. Best regards, Claudio Subject: umc-bus.c: fix usage of device_trylock From: Claudio Scordino device_trylock has the same semantics of mutex_trylock, so it returns 1 if the lock has been acquired successfully. Signed-off-by: Claudio Scordino --- drivers/uwb/umc-bus.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/uwb/umc-bus.c b/drivers/uwb/umc-bus.c index 82a84d5..5c5b3fc 100644 --- a/drivers/uwb/umc-bus.c +++ b/drivers/uwb/umc-bus.c @@ -63,7 +63,7 @@ int umc_controller_reset(struct umc_dev *umc) struct device *parent = umc->dev.parent; int ret = 0; - if (device_trylock(parent)) + if (!device_trylock(parent)) return -EAGAIN; ret = device_for_each_child(parent, parent, umc_bus_pre_reset_helper); if (ret >= 0) -- 1.7.1 -- 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
[PATCH RESEND] umc-bus.c: fix usage of device_trylock
Hi all. I've not received any feedback about this patch, so I'm resending just to check if someone is going to take care of it. The patch fixes the usage of device_trylock inside the driver umc-bus.c: device_trylock has the same semantics of mutex_trylock, so it returns 1 if the lock has been acquired successfully. Best regards, Claudio Subject: umc-bus.c: fix usage of device_trylock From: Claudio Scordino Fix usage of device_trylock. It has the same semantics of mutex_trylock, so it returns 1 if the lock has been acquired successfully. Signed-off-by: Claudio Scordino Signed-off-by: Bruno Morelli --- drivers/uwb/umc-bus.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/uwb/umc-bus.c b/drivers/uwb/umc-bus.c index 82a84d5..5c5b3fc 100644 --- a/drivers/uwb/umc-bus.c +++ b/drivers/uwb/umc-bus.c @@ -63,7 +63,7 @@ int umc_controller_reset(struct umc_dev *umc) struct device *parent = umc->dev.parent; int ret = 0; - if (device_trylock(parent)) + if (!device_trylock(parent)) return -EAGAIN; ret = device_for_each_child(parent, parent, umc_bus_pre_reset_helper); if (ret >= 0) -- 1.7.1 -- 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] isp1362-hcd.c: usb message always saved in case of underrun
Il 06/07/2012 19:41, Greg KH ha scritto:
On Wed, Jun 27, 2012 at 06:01:39PM +0200, Claudio Scordino wrote:
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
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
Signed-off-by: Claudio Scordino
Tested-by: Bruno Morelli
---
drivers/usb/host/isp1362-hcd.c | 11 ++-
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 2ed112d..61bf1b2 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -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:
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.
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
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
Signed-off-by: Claudio Scordino
Tested-by: Bruno Morelli
---
drivers/usb/host/isp1362-hcd.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 2ed112d..2563263 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -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
-*/
-
Re: [PATCH] isp1362-hcd.c: usb message always saved in case of underrun
Il 20/07/2012 00:58, Greg KH ha scritto:
On Wed, Jul 18, 2012 at 10:53:09AM +0200, Claudio Scordino wrote:
Il 06/07/2012 19:41, Greg KH ha scritto:
On Wed, Jun 27, 2012 at 06:01:39PM +0200, Claudio Scordino wrote:
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
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
Signed-off-by: Claudio Scordino
Tested-by: Bruno Morelli
---
drivers/usb/host/isp1362-hcd.c | 11 ++-
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 2ed112d..61bf1b2 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -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:
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.
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?
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
--
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] isp1362-hcd.c: usb message always saved in case of underrun
Il 20/07/2012 17:15, Greg KH ha scritto:
On Fri, Jul 20, 2012 at 12:26:10PM +0200, Claudio Scordino wrote:
Il 20/07/2012 00:58, Greg KH ha scritto:
On Wed, Jul 18, 2012 at 10:53:09AM +0200, Claudio Scordino wrote:
Il 06/07/2012 19:41, Greg KH ha scritto:
On Wed, Jun 27, 2012 at 06:01:39PM +0200, Claudio Scordino wrote:
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
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
Signed-off-by: Claudio Scordino
Tested-by: Bruno Morelli
---
drivers/usb/host/isp1362-hcd.c | 11 ++-
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 2ed112d..61bf1b2 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -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:
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.
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?
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 ?
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
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
Signed-off-by: Claudio Scordino
Tested-by: Bruno Morelli
---
drivers/usb/host/isp1362-hcd.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp136
