* [PATCH 1/2] usb: usbtest: Add timetout to simple_io() @ 2013-12-18 10:10 Roger Quadros 2013-12-18 10:10 ` [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail Roger Quadros 2013-12-18 14:46 ` [PATCH 1/2] usb: usbtest: Add timetout to simple_io() Huang Rui 0 siblings, 2 replies; 9+ messages in thread From: Roger Quadros @ 2013-12-18 10:10 UTC (permalink / raw) To: gregkh; +Cc: stern, ray.huang, balbi, linux-usb, linux-kernel, Roger Quadros Without a timetout some tests e.g. test_halt() can remain stuck forever. Signed-off-by: Roger Quadros <rogerq@ti.com> Reviewed-by: Felipe Balbi <balbi@ti.com> --- drivers/usb/misc/usbtest.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index b415282..6294e1b 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -10,6 +10,7 @@ #include <linux/usb.h> +#define SIMPLE_IO_TIMEOUT 10000 /* in milliseconds */ /*-------------------------------------------------------------------------*/ @@ -366,6 +367,7 @@ static int simple_io( int max = urb->transfer_buffer_length; struct completion completion; int retval = 0; + unsigned long expire; urb->context = &completion; while (retval == 0 && iterations-- > 0) { @@ -378,9 +380,15 @@ static int simple_io( if (retval != 0) break; - /* NOTE: no timeouts; can't be broken out of by interrupt */ - wait_for_completion(&completion); - retval = urb->status; + expire = msecs_to_jiffies(SIMPLE_IO_TIMEOUT); + if (!wait_for_completion_timeout(&completion, expire)) { + usb_kill_urb(urb); + retval = (urb->status == -ENOENT ? + -ETIMEDOUT : urb->status); + } else { + retval = urb->status; + } + urb->dev = udev; if (retval == 0 && usb_pipein(urb->pipe)) retval = simple_check_buf(tdev, urb); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail 2013-12-18 10:10 [PATCH 1/2] usb: usbtest: Add timetout to simple_io() Roger Quadros @ 2013-12-18 10:10 ` Roger Quadros 2013-12-19 4:01 ` Huang Rui 2013-12-18 14:46 ` [PATCH 1/2] usb: usbtest: Add timetout to simple_io() Huang Rui 1 sibling, 1 reply; 9+ messages in thread From: Roger Quadros @ 2013-12-18 10:10 UTC (permalink / raw) To: gregkh; +Cc: stern, ray.huang, balbi, linux-usb, linux-kernel, Roger Quadros In test_halt() we set an endpoint halt condition and return on halt verification failure, then the enpoint will remain halted and all further tests related to that enpoint will fail. This is because we don't tackle endpoint halt error condition in any of the tests. To avoid that situation, make sure to clear the halt condition before exiting test_halt(). Signed-off-by: Roger Quadros <rogerq@ti.com> Reviewed-by: Felipe Balbi <balbi@ti.com> --- drivers/usb/misc/usbtest.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index 6294e1b..300b726 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -1545,8 +1545,17 @@ static int test_halt(struct usbtest_dev *tdev, int ep, struct urb *urb) return retval; } retval = verify_halted(tdev, ep, urb); - if (retval < 0) + if (retval < 0) { + int ret; + + /* clear halt anyways, else further tests will fail */ + ret = usb_clear_halt(urb->dev, urb->pipe); + if (ret) + ERROR(tdev, "ep %02x couldn't clear halt, %d\n", + ep, ret); + return retval; + } /* clear halt (tests API + protocol), verify it worked */ retval = usb_clear_halt(urb->dev, urb->pipe); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail 2013-12-18 10:10 ` [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail Roger Quadros @ 2013-12-19 4:01 ` Huang Rui 2013-12-19 5:46 ` Huang Rui 0 siblings, 1 reply; 9+ messages in thread From: Huang Rui @ 2013-12-19 4:01 UTC (permalink / raw) To: Roger Quadros; +Cc: gregkh, stern, balbi, linux-usb, linux-kernel On Wed, Dec 18, 2013 at 03:40:11PM +0530, Roger Quadros wrote: > In test_halt() we set an endpoint halt condition and return on halt verification > failure, then the enpoint will remain halted and all further tests related > to that enpoint will fail. This is because we don't tackle endpoint halt error condition > in any of the tests. To avoid that situation, make sure to clear the > halt condition before exiting test_halt(). > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Reviewed-by: Felipe Balbi <balbi@ti.com> Acked-by: Huang Rui <ray.huang@amd.com> > --- > drivers/usb/misc/usbtest.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c > index 6294e1b..300b726 100644 > --- a/drivers/usb/misc/usbtest.c > +++ b/drivers/usb/misc/usbtest.c > @@ -1545,8 +1545,17 @@ static int test_halt(struct usbtest_dev *tdev, int ep, struct urb *urb) > return retval; > } > retval = verify_halted(tdev, ep, urb); > - if (retval < 0) > + if (retval < 0) { > + int ret; > + > + /* clear halt anyways, else further tests will fail */ > + ret = usb_clear_halt(urb->dev, urb->pipe); > + if (ret) > + ERROR(tdev, "ep %02x couldn't clear halt, %d\n", > + ep, ret); > + > return retval; > + } > > /* clear halt (tests API + protocol), verify it worked */ > retval = usb_clear_halt(urb->dev, urb->pipe); > -- > 1.8.3.2 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail 2013-12-19 4:01 ` Huang Rui @ 2013-12-19 5:46 ` Huang Rui 2013-12-19 6:21 ` Roger Quadros 0 siblings, 1 reply; 9+ messages in thread From: Huang Rui @ 2013-12-19 5:46 UTC (permalink / raw) To: Roger Quadros; +Cc: gregkh, stern, balbi, linux-usb, linux-kernel On Thu, Dec 19, 2013 at 12:01:47PM +0800, Huang Rui wrote: > On Wed, Dec 18, 2013 at 03:40:11PM +0530, Roger Quadros wrote: > > In test_halt() we set an endpoint halt condition and return on halt verification > > failure, then the enpoint will remain halted and all further tests related ^^^^^^^ > > to that enpoint will fail. This is because we don't tackle endpoint halt error condition ^^^^^^^ BTW, please fix these typo. Thanks, Rui > > in any of the tests. To avoid that situation, make sure to clear the > > halt condition before exiting test_halt(). > > > > Signed-off-by: Roger Quadros <rogerq@ti.com> > > Reviewed-by: Felipe Balbi <balbi@ti.com> > > Acked-by: Huang Rui <ray.huang@amd.com> > > > --- > > drivers/usb/misc/usbtest.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c > > index 6294e1b..300b726 100644 > > --- a/drivers/usb/misc/usbtest.c > > +++ b/drivers/usb/misc/usbtest.c > > @@ -1545,8 +1545,17 @@ static int test_halt(struct usbtest_dev *tdev, int ep, struct urb *urb) > > return retval; > > } > > retval = verify_halted(tdev, ep, urb); > > - if (retval < 0) > > + if (retval < 0) { > > + int ret; > > + > > + /* clear halt anyways, else further tests will fail */ > > + ret = usb_clear_halt(urb->dev, urb->pipe); > > + if (ret) > > + ERROR(tdev, "ep %02x couldn't clear halt, %d\n", > > + ep, ret); > > + > > return retval; > > + } > > > > /* clear halt (tests API + protocol), verify it worked */ > > retval = usb_clear_halt(urb->dev, urb->pipe); > > -- > > 1.8.3.2 > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail 2013-12-19 5:46 ` Huang Rui @ 2013-12-19 6:21 ` Roger Quadros 2013-12-19 6:35 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Roger Quadros @ 2013-12-19 6:21 UTC (permalink / raw) To: Huang Rui, gregkh; +Cc: stern, balbi, linux-usb, linux-kernel On 12/19/2013 11:16 AM, Huang Rui wrote: > On Thu, Dec 19, 2013 at 12:01:47PM +0800, Huang Rui wrote: >> On Wed, Dec 18, 2013 at 03:40:11PM +0530, Roger Quadros wrote: >>> In test_halt() we set an endpoint halt condition and return on halt verification >>> failure, then the enpoint will remain halted and all further tests related > ^^^^^^^ >>> to that enpoint will fail. This is because we don't tackle endpoint halt error condition > ^^^^^^^ > BTW, please fix these typo. Hi Rui, These patches have been already applied to Greg's usb tree without your Ack's or the typo fix. Greg, do you want me to resend the patches? cheers, -roger > > Thanks, > Rui > >>> in any of the tests. To avoid that situation, make sure to clear the >>> halt condition before exiting test_halt(). >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> Reviewed-by: Felipe Balbi <balbi@ti.com> >> >> Acked-by: Huang Rui <ray.huang@amd.com> >> >>> --- >>> drivers/usb/misc/usbtest.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c >>> index 6294e1b..300b726 100644 >>> --- a/drivers/usb/misc/usbtest.c >>> +++ b/drivers/usb/misc/usbtest.c >>> @@ -1545,8 +1545,17 @@ static int test_halt(struct usbtest_dev *tdev, int ep, struct urb *urb) >>> return retval; >>> } >>> retval = verify_halted(tdev, ep, urb); >>> - if (retval < 0) >>> + if (retval < 0) { >>> + int ret; >>> + >>> + /* clear halt anyways, else further tests will fail */ >>> + ret = usb_clear_halt(urb->dev, urb->pipe); >>> + if (ret) >>> + ERROR(tdev, "ep %02x couldn't clear halt, %d\n", >>> + ep, ret); >>> + >>> return retval; >>> + } >>> >>> /* clear halt (tests API + protocol), verify it worked */ >>> retval = usb_clear_halt(urb->dev, urb->pipe); >>> -- >>> 1.8.3.2 >>> >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail 2013-12-19 6:21 ` Roger Quadros @ 2013-12-19 6:35 ` Greg KH 0 siblings, 0 replies; 9+ messages in thread From: Greg KH @ 2013-12-19 6:35 UTC (permalink / raw) To: Roger Quadros; +Cc: Huang Rui, stern, balbi, linux-usb, linux-kernel On Thu, Dec 19, 2013 at 11:51:45AM +0530, Roger Quadros wrote: > On 12/19/2013 11:16 AM, Huang Rui wrote: > > On Thu, Dec 19, 2013 at 12:01:47PM +0800, Huang Rui wrote: > >> On Wed, Dec 18, 2013 at 03:40:11PM +0530, Roger Quadros wrote: > >>> In test_halt() we set an endpoint halt condition and return on halt verification > >>> failure, then the enpoint will remain halted and all further tests related > > ^^^^^^^ > >>> to that enpoint will fail. This is because we don't tackle endpoint halt error condition > > ^^^^^^^ > > BTW, please fix these typo. > > Hi Rui, > > These patches have been already applied to Greg's usb tree without your Ack's or the typo fix. > > Greg, > > do you want me to resend the patches? Nope, it's not a big deal, typo's in changelog comments isn't a worry. greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] usb: usbtest: Add timetout to simple_io() 2013-12-18 10:10 [PATCH 1/2] usb: usbtest: Add timetout to simple_io() Roger Quadros 2013-12-18 10:10 ` [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail Roger Quadros @ 2013-12-18 14:46 ` Huang Rui 2013-12-18 16:37 ` Felipe Balbi 1 sibling, 1 reply; 9+ messages in thread From: Huang Rui @ 2013-12-18 14:46 UTC (permalink / raw) To: Roger Quadros; +Cc: gregkh, stern, balbi, linux-usb, linux-kernel Hi Roger, On Wed, Dec 18, 2013 at 03:40:10PM +0530, Roger Quadros wrote: > Without a timetout some tests e.g. test_halt() can remain stuck forever. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Reviewed-by: Felipe Balbi <balbi@ti.com> > --- > drivers/usb/misc/usbtest.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c > index b415282..6294e1b 100644 > --- a/drivers/usb/misc/usbtest.c > +++ b/drivers/usb/misc/usbtest.c > @@ -10,6 +10,7 @@ > > #include <linux/usb.h> > > +#define SIMPLE_IO_TIMEOUT 10000 /* in milliseconds */ > Only one question, how do you confirm the timeout value? Thanks, Rui > /*-------------------------------------------------------------------------*/ > > @@ -366,6 +367,7 @@ static int simple_io( > int max = urb->transfer_buffer_length; > struct completion completion; > int retval = 0; > + unsigned long expire; > > urb->context = &completion; > while (retval == 0 && iterations-- > 0) { > @@ -378,9 +380,15 @@ static int simple_io( > if (retval != 0) > break; > > - /* NOTE: no timeouts; can't be broken out of by interrupt */ > - wait_for_completion(&completion); > - retval = urb->status; > + expire = msecs_to_jiffies(SIMPLE_IO_TIMEOUT); > + if (!wait_for_completion_timeout(&completion, expire)) { > + usb_kill_urb(urb); > + retval = (urb->status == -ENOENT ? > + -ETIMEDOUT : urb->status); > + } else { > + retval = urb->status; > + } > + > urb->dev = udev; > if (retval == 0 && usb_pipein(urb->pipe)) > retval = simple_check_buf(tdev, urb); > -- > 1.8.3.2 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] usb: usbtest: Add timetout to simple_io() 2013-12-18 14:46 ` [PATCH 1/2] usb: usbtest: Add timetout to simple_io() Huang Rui @ 2013-12-18 16:37 ` Felipe Balbi 2013-12-19 4:00 ` Huang Rui 0 siblings, 1 reply; 9+ messages in thread From: Felipe Balbi @ 2013-12-18 16:37 UTC (permalink / raw) To: Huang Rui; +Cc: Roger Quadros, gregkh, stern, balbi, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1114 bytes --] On Wed, Dec 18, 2013 at 10:46:03PM +0800, Huang Rui wrote: > Hi Roger, > > On Wed, Dec 18, 2013 at 03:40:10PM +0530, Roger Quadros wrote: > > Without a timetout some tests e.g. test_halt() can remain stuck forever. > > > > Signed-off-by: Roger Quadros <rogerq@ti.com> > > Reviewed-by: Felipe Balbi <balbi@ti.com> > > --- > > drivers/usb/misc/usbtest.c | 14 +++++++++++--- > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c > > index b415282..6294e1b 100644 > > --- a/drivers/usb/misc/usbtest.c > > +++ b/drivers/usb/misc/usbtest.c > > @@ -10,6 +10,7 @@ > > > > #include <linux/usb.h> > > > > +#define SIMPLE_IO_TIMEOUT 10000 /* in milliseconds */ > > > > Only one question, how do you confirm the timeout value? dude, it's just a timeout. It could be anything, really. 10 seconds is just large enough value that would allow even the slowest scenarios (host + device) to complete, while not being so slow that you would wanna kill yourself after waiting for such a long time ;-) -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] usb: usbtest: Add timetout to simple_io() 2013-12-18 16:37 ` Felipe Balbi @ 2013-12-19 4:00 ` Huang Rui 0 siblings, 0 replies; 9+ messages in thread From: Huang Rui @ 2013-12-19 4:00 UTC (permalink / raw) To: Felipe Balbi; +Cc: Roger Quadros, gregkh, stern, linux-usb, linux-kernel On Wed, Dec 18, 2013 at 10:37:44AM -0600, Felipe Balbi wrote: > On Wed, Dec 18, 2013 at 10:46:03PM +0800, Huang Rui wrote: > > Hi Roger, > > > > On Wed, Dec 18, 2013 at 03:40:10PM +0530, Roger Quadros wrote: > > > Without a timetout some tests e.g. test_halt() can remain stuck forever. > > > > > > Signed-off-by: Roger Quadros <rogerq@ti.com> > > > Reviewed-by: Felipe Balbi <balbi@ti.com> > > > --- > > > drivers/usb/misc/usbtest.c | 14 +++++++++++--- > > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c > > > index b415282..6294e1b 100644 > > > --- a/drivers/usb/misc/usbtest.c > > > +++ b/drivers/usb/misc/usbtest.c > > > @@ -10,6 +10,7 @@ > > > > > > #include <linux/usb.h> > > > > > > +#define SIMPLE_IO_TIMEOUT 10000 /* in milliseconds */ > > > > > > > Only one question, how do you confirm the timeout value? > > dude, it's just a timeout. It could be anything, really. 10 seconds is > just large enough value that would allow even the slowest scenarios > (host + device) to complete, while not being so slow that you would > wanna kill yourself after waiting for such a long time ;-) > OK. I've tested in my side. Acked-by: Huang Rui <ray.huang@amd.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-12-19 6:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-18 10:10 [PATCH 1/2] usb: usbtest: Add timetout to simple_io() Roger Quadros 2013-12-18 10:10 ` [PATCH 2/2] usb: usbtest: Always clear halt else further tests will fail Roger Quadros 2013-12-19 4:01 ` Huang Rui 2013-12-19 5:46 ` Huang Rui 2013-12-19 6:21 ` Roger Quadros 2013-12-19 6:35 ` Greg KH 2013-12-18 14:46 ` [PATCH 1/2] usb: usbtest: Add timetout to simple_io() Huang Rui 2013-12-18 16:37 ` Felipe Balbi 2013-12-19 4:00 ` Huang Rui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox