2015-06-24 06:52:25

by Sunny Kumar

[permalink] [raw]
Subject: [PATCH 1/1] usb: usleep_range is preferred over udelay where wakeup is flexible

According to Documentation/timers/timers-howto.txt"
udelay() is only called once from a place where sleeping is allowed.
We can replace it with a call to usleep_range()
with a reasonable upper limit.

Signed-off-by: Sunny Kumar <[email protected]>
---
drivers/usb/storage/transport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 540add2..7cd45ac 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -1111,7 +1111,7 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us)
* command phase and the data phase. Some devices need a little
* more than that, probably because of clock rate inaccuracies. */
if (unlikely(us->fflags & US_FL_GO_SLOW))
- udelay(125);
+ usleep_range(100, 125);

if (transfer_length) {
unsigned int pipe = srb->sc_data_direction == DMA_FROM_DEVICE ?
--
2.1.4


2015-06-24 13:44:41

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/1] usb: usleep_range is preferred over udelay where wakeup is flexible

On Wed, 24 Jun 2015, Sunny Kumar wrote:

> According to Documentation/timers/timers-howto.txt"
> udelay() is only called once from a place where sleeping is allowed.
> We can replace it with a call to usleep_range()
> with a reasonable upper limit.
>
> Signed-off-by: Sunny Kumar <[email protected]>
> ---
> drivers/usb/storage/transport.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 540add2..7cd45ac 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -1111,7 +1111,7 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us)
> * command phase and the data phase. Some devices need a little
> * more than that, probably because of clock rate inaccuracies. */
> if (unlikely(us->fflags & US_FL_GO_SLOW))
> - udelay(125);
> + usleep_range(100, 125);

You said you were going to use a reasonable upper limit. Instead, you
left the upper limit the same and decreased the lower limit, which
could cause errors.

Alan Stern

2015-06-24 14:26:35

by Sunny Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/1] usb: usleep_range is preferred over udelay where wakeup is flexible

Ok so what about putting lower limit to 125 and increase upper limit 200


Sunny

On Wed, Jun 24, 2015 at 12:24:29PM +0530, Sunny Kumar wrote:
> According to Documentation/timers/timers-howto.txt"
> udelay() is only called once from a place where sleeping is allowed.
> We can replace it with a call to usleep_range()
> with a reasonable upper limit.
>
> Signed-off-by: Sunny Kumar <[email protected]>
> ---
> drivers/usb/storage/transport.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 540add2..7cd45ac 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -1111,7 +1111,7 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us)
> * command phase and the data phase. Some devices need a little
> * more than that, probably because of clock rate inaccuracies. */
> if (unlikely(us->fflags & US_FL_GO_SLOW))
> - udelay(125);
> + usleep_range(100, 125);
>
> if (transfer_length) {
> unsigned int pipe = srb->sc_data_direction == DMA_FROM_DEVICE ?
> --
> 2.1.4
>


Attachments:
(No filename) (1.12 kB)
0001-usb-usleep_range-is-preferred-over-udelay-where-wake.patch (1.17 kB)
Download all attachments

2015-06-24 14:35:48

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/1] usb: usleep_range is preferred over udelay where wakeup is flexible

Please don't top-post.

On Wed, 24 Jun 2015, Sunny Kumar wrote:

> Ok so what about putting lower limit to 125 and increase upper limit 200

Or even just 150. That would be fine.

Alan Stern

2015-06-24 15:45:17

by Sunny Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/1] usb: usleep_range is preferred over udelay where wakeup is flexible

Ok then,
This might be the final lower and upper limit value.

Sunny Kumar

On Wed, Jun 24, 2015 at 12:24:29PM +0530, Sunny Kumar wrote:
> According to Documentation/timers/timers-howto.txt"
> udelay() is only called once from a place where sleeping is allowed.
> We can replace it with a call to usleep_range()
> with a reasonable upper limit.
>
> Signed-off-by: Sunny Kumar <[email protected]>
> ---
> drivers/usb/storage/transport.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 540add2..7cd45ac 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -1111,7 +1111,7 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us)
> * command phase and the data phase. Some devices need a little
> * more than that, probably because of clock rate inaccuracies. */
> if (unlikely(us->fflags & US_FL_GO_SLOW))
> - udelay(125);
> + usleep_range(100, 125);
>
> if (transfer_length) {
> unsigned int pipe = srb->sc_data_direction == DMA_FROM_DEVICE ?
> --
> 2.1.4
>


Attachments:
(No filename) (1.12 kB)
0001-usb-usleep_range-is-preferred-over-udelay-where-wake.patch (1.17 kB)
Download all attachments

2015-06-25 15:31:43

by Sunny Kumar

[permalink] [raw]
Subject: [PATCH 1/1] usb: usleep_range is preferred over udelay where wakeup is flexible

According to Documentation/timers/timers-howto.txt"
udelay() is only called once from a place where sleeping is allowed.
We can replace it with a call to usleep_range()
with a reasonable upper limit.

Signed-off-by: Sunny Kumar <[email protected]>
---
drivers/usb/storage/transport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 540add2..5e67f63 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -1111,7 +1111,7 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us)
* command phase and the data phase. Some devices need a little
* more than that, probably because of clock rate inaccuracies. */
if (unlikely(us->fflags & US_FL_GO_SLOW))
- udelay(125);
+ usleep_range(125, 150);

if (transfer_length) {
unsigned int pipe = srb->sc_data_direction == DMA_FROM_DEVICE ?
--
1.9.1