Discussion:
[PATCH] storage: Fix timeout in usb_stor_euscsi_init() and usb_stor_huawei_e220_init()
Mark Knibbs
2014-10-17 20:07:40 UTC
Permalink
Hi,

After the recent linux-usb mailing list thread "usb_stor_control_msg()
timeout argument"...

The timeout argument to usb_stor_control_msg() is specified in jiffies, not
milliseconds.

Signed-off-by: Mark Knibbs <markk-yYnZraf+/sb10XsdtD+***@public.gmane.org>

---
diff -up linux-3.17/drivers/usb/storage/initializers.c.orig linux-3.17/drivers/usb/storage/initializers.c
--- linux-3.17/drivers/usb/storage/initializers.c.orig 2014-10-05 20:23:04.000000000 +0100
+++ linux-3.17/drivers/usb/storage/initializers.c 2014-10-17 21:01:49.063077637 +0100
@@ -52,7 +52,7 @@ int usb_stor_euscsi_init(struct us_data
us->iobuf[0] = 0x1;
result = usb_stor_control_msg(us, us->send_ctrl_pipe,
0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR,
- 0x01, 0x0, us->iobuf, 0x1, 5000);
+ 0x01, 0x0, us->iobuf, 0x1, 5*HZ);
usb_stor_dbg(us, "-- result is %d\n", result);

return 0;
@@ -100,7 +100,7 @@ int usb_stor_huawei_e220_init(struct us_
result = usb_stor_control_msg(us, us->send_ctrl_pipe,
USB_REQ_SET_FEATURE,
USB_TYPE_STANDARD | USB_RECIP_DEVICE,
- 0x01, 0x0, NULL, 0x0, 1000);
+ 0x01, 0x0, NULL, 0x0, 1*HZ);
usb_stor_dbg(us, "Huawei mode set result is %d\n", result);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov
2014-10-18 16:28:11 UTC
Permalink
Hello.
Post by Mark Knibbs
Hi,
After the recent linux-usb mailing list thread "usb_stor_control_msg()
timeout argument"...
Please place such stuff under the --- tear line.
Post by Mark Knibbs
The timeout argument to usb_stor_control_msg() is specified in jiffies, not
milliseconds.
---
diff -up linux-3.17/drivers/usb/storage/initializers.c.orig linux-3.17/drivers/usb/storage/initializers.c
--- linux-3.17/drivers/usb/storage/initializers.c.orig 2014-10-05 20:23:04.000000000 +0100
+++ linux-3.17/drivers/usb/storage/initializers.c 2014-10-17 21:01:49.063077637 +0100
@@ -52,7 +52,7 @@ int usb_stor_euscsi_init(struct us_data
us->iobuf[0] = 0x1;
result = usb_stor_control_msg(us, us->send_ctrl_pipe,
0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR,
- 0x01, 0x0, us->iobuf, 0x1, 5000);
+ 0x01, 0x0, us->iobuf, 0x1, 5*HZ);
Pleas surround * with spaces, for consistency with the code above (and
thye general kernel coding style).
Post by Mark Knibbs
usb_stor_dbg(us, "-- result is %d\n", result);
return 0;
@@ -100,7 +100,7 @@ int usb_stor_huawei_e220_init(struct us_
result = usb_stor_control_msg(us, us->send_ctrl_pipe,
USB_REQ_SET_FEATURE,
USB_TYPE_STANDARD | USB_RECIP_DEVICE,
- 0x01, 0x0, NULL, 0x0, 1000);
+ 0x01, 0x0, NULL, 0x0, 1*HZ);
Likewise.

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Knibbs
2014-10-20 18:34:09 UTC
Permalink
On Sat, 18 Oct 2014 20:28:11 +0400
Post by Sergei Shtylyov
Post by Mark Knibbs
...
0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR,
- 0x01, 0x0, us->iobuf, 0x1, 5000);
+ 0x01, 0x0, us->iobuf, 0x1, 5*HZ);
Pleas surround * with spaces, for consistency with the code above (and
thye general kernel coding style).
I noticed that spaces aren't used consistently. For example in
drivers/usb/storage/transport.c there are four instances of n*HZ. Also in
various files not in drivers/usb/

I assume it's not worth submitting a patch to only fix coding-style issues
like that?


Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov
2014-10-20 18:37:43 UTC
Permalink
Hello.
Post by Mark Knibbs
Post by Sergei Shtylyov
Post by Mark Knibbs
...
0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR,
- 0x01, 0x0, us->iobuf, 0x1, 5000);
+ 0x01, 0x0, us->iobuf, 0x1, 5*HZ);
Pleas surround * with spaces, for consistency with the code above (and
thye general kernel coding style).
I noticed that spaces aren't used consistently. For example in
drivers/usb/storage/transport.c there are four instances of n*HZ. Also in
various files not in drivers/usb/
I assume it's not worth submitting a patch to only fix coding-style issues
like that?
And you're right...
... unless scripts/checkpatch.pl reports them.
Post by Mark Knibbs
Mark
WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...