diff mbox series

Re: [PATCH v3] USB: hub.c: decrease the number of attempts of enumeration scheme

Message ID 20200921140342.3813-1-yazzep@gmail.com
State New
Headers show
Series Re: [PATCH v3] USB: hub.c: decrease the number of attempts of enumeration scheme | expand

Commit Message

yasushi asano Sept. 21, 2020, 2:03 p.m. UTC 
From: Yasushi Asano <yasano@jp.adit-jv.com>

Dear Alan

Thank you very much for providing the patch.
I really appreciate your kindness.

When I added a pseudo error code and checked it,
the for statement of the "operation" in the new scheme
runs unconditionally three times. Therefore It doesn't
seem to meet the requirements (30seconds).

After applying your patch, I added a patch that can
change the loop number of "operation" as shown below,
and it worked fine in the pseudo error environment.
Is this modification acceptable?

If it is good, I'll add this patch and test it.
Since the PET tool is only in the customer,
I will ask the customer to test it. I will report the
result when I receive the results.

Signed-off-by: Yasushi Asano <yasano@jp.adit-jv.com>
---
 drivers/usb/core/hub.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Alan Stern Sept. 21, 2020, 2:48 p.m. UTC | #1 
On Mon, Sep 21, 2020 at 11:03:42PM +0900, Yasushi Asano wrote:
> From: Yasushi Asano <yasano@jp.adit-jv.com>

> 

> Dear Alan

> 

> Thank you very much for providing the patch.

> I really appreciate your kindness.

> 

> When I added a pseudo error code and checked it,

> the for statement of the "operation" in the new scheme

> runs unconditionally three times. Therefore It doesn't

> seem to meet the requirements (30seconds).

> 

> After applying your patch, I added a patch that can

> change the loop number of "operation" as shown below,

> and it worked fine in the pseudo error environment.

> Is this modification acceptable?


Ah, I missed that change.  Yes, this is the right approach.

> If it is good, I'll add this patch and test it.

> Since the PET tool is only in the customer,

> I will ask the customer to test it. I will report the

> result when I receive the results.

> 

> Signed-off-by: Yasushi Asano <yasano@jp.adit-jv.com>

> ---

>  drivers/usb/core/hub.c | 4 +++-

>  1 file changed, 3 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c

> index 61effd5..2f07f7c 100644

> --- a/drivers/usb/core/hub.c

> +++ b/drivers/usb/core/hub.c

> @@ -2709,12 +2709,14 @@ static unsigned hub_is_wusb(struct usb_hub *hub)

>  #define PORT_RESET_TRIES	2

>  #define SET_ADDRESS_TRIES	1

>  #define GET_DESCRIPTOR_TRIES	1

> +#define GET_DESCRIPTOR_OPERATIONS	1

>  #define PORT_INIT_TRIES		5

>  

>  #else

>  #define PORT_RESET_TRIES	5

>  #define SET_ADDRESS_TRIES	2

>  #define GET_DESCRIPTOR_TRIES	2

> +#define GET_DESCRIPTOR_OPERATIONS	3

>  #define PORT_INIT_TRIES		4

>  #endif	/* CONFIG_USB_FEW_INIT_RETRIES */

>  

> @@ -4699,7 +4701,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,

>  			 * 255 is for WUSB devices, we actually need to use

>  			 * 512 (WUSB1.0[4.8.1]).

>  			 */

> -			for (operations = 0; operations < 3; ++operations) {

> +			for (operations = 0; operations < GET_DESCRIPTOR_OPERATIONS; ++operations) {

>  				buf->bMaxPacketSize0 = 0;

>  				r = usb_control_msg(udev, usb_rcvaddr0pipe(),

>  					USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,


Okay.  I will merge your change into my patch.

Alan Stern
Alan Stern Sept. 21, 2020, 3:06 p.m. UTC | #2 
On Mon, Sep 21, 2020 at 11:59:31PM +0900, yasushi asano wrote:
> Dear Alan

> 

> Thank you very much for the reply.

> please merge my modification to your patch.


Yes.

I will wait to hear the result of your test before I submit the changes.

Alan Stern
yasushi asano Sept. 25, 2020, 1:05 a.m. UTC | #3 
Dear Alan,
I am waiting for the test result from the customer since Tuesday, but
I have not received a reply yet. I will inform you as soon as I
receive the result.

Best regards
Yasushi Asano

2020年9月22日(火) 0:06 Alan Stern <stern@rowland.harvard.edu>:
>

> On Mon, Sep 21, 2020 at 11:59:31PM +0900, yasushi asano wrote:

> > Dear Alan

> >

> > Thank you very much for the reply.

> > please merge my modification to your patch.

>

> Yes.

>

> I will wait to hear the result of your test before I submit the changes.

>

> Alan Stern
yasushi asano Sept. 25, 2020, 5:21 p.m. UTC | #4 
Dear Alan,
I received the test result of applying your patch and
my change from customer.
It was tested using PETtool. it barely passed the test.
It took 29.497 seconds. The patch worked well.
The following is an excerpt from dmesg.
Could you please merge my change and proceed with this PR?
I really appreciate your kindness.

[2] - [1] = 869.860348 - 840.363348 = 29.497

[  829.307248] *** Setting Test Suite ***
[  838.173580] *** Ready OK Test Start***
[  840.363348] usb 1-1.1: new full-speed USB device number 7 using
ehci-platform ... [1]
[  845.650354] usb 1-1.1: device descriptor read/64, error -110
[  845.841346] usb 1-1.1: new full-speed USB device number 8 using ehci-platform
[  851.283362] usb 1-1.1: device descriptor read/64, error -110
[  851.397049] usb 1-1-port1: attempt power cycle
[  853.216340] usb 1-1.1: new full-speed USB device number 9 using ehci-platform
[  858.451345] usb 1-1.1: device descriptor read/64, error -110
[  858.640352] usb 1-1.1: new full-speed USB device number 10 using
ehci-platform
[  864.212349] usb 1-1.1: device not accepting address 10, error -110
[  864.295350] usb 1-1.1: new full-speed USB device number 11 using
ehci-platform
[  869.844342] usb 1-1.1: device not accepting address 11, error -110
[  869.860348] usb 1-1-port1: unable to enumerate USB device ... [2]
[  885.407051] *** End of Test        ***

Best Regards
Yasushi Asano

2020年9月25日(金) 10:05 yasushi asano <yazzep@gmail.com>:
>

> Dear Alan,

> I am waiting for the test result from the customer since Tuesday, but

> I have not received a reply yet. I will inform you as soon as I

> receive the result.

>

> Best regards

> Yasushi Asano

>

> 2020年9月22日(火) 0:06 Alan Stern <stern@rowland.harvard.edu>:

> >

> > On Mon, Sep 21, 2020 at 11:59:31PM +0900, yasushi asano wrote:

> > > Dear Alan

> > >

> > > Thank you very much for the reply.

> > > please merge my modification to your patch.

> >

> > Yes.

> >

> > I will wait to hear the result of your test before I submit the changes.

> >

> > Alan Stern
Alan Stern Sept. 25, 2020, 6:41 p.m. UTC | #5 
On Sat, Sep 26, 2020 at 02:21:50AM +0900, yasushi asano wrote:
> Dear Alan,

> I received the test result of applying your patch and

> my change from customer.

> It was tested using PETtool. it barely passed the test.

> It took 29.497 seconds. The patch worked well.


That's awfully close to the limit.  I think PORT_INIT_TRIES should be 
reduced to 4.

> The following is an excerpt from dmesg.

> Could you please merge my change and proceed with this PR?

> I really appreciate your kindness.


I'll go ahead and submit the patches.

Alan Stern
yasushi asano Sept. 27, 2020, 3:43 p.m. UTC | #6 
Dear Alan,

>That's awfully close to the limit.  I think PORT_INIT_TRIES should be

>reduced to 4.

I understood. I agree with you.

>I'll go ahead and submit the patches.

Thank you very much.

Best regards
Yasushi Asano

2020年9月26日(土) 3:41 Alan Stern <stern@rowland.harvard.edu>:
>

> On Sat, Sep 26, 2020 at 02:21:50AM +0900, yasushi asano wrote:

> > Dear Alan,

> > I received the test result of applying your patch and

> > my change from customer.

> > It was tested using PETtool. it barely passed the test.

> > It took 29.497 seconds. The patch worked well.

>

> That's awfully close to the limit.  I think PORT_INIT_TRIES should be

> reduced to 4.

>

> > The following is an excerpt from dmesg.

> > Could you please merge my change and proceed with this PR?

> > I really appreciate your kindness.

>

> I'll go ahead and submit the patches.

>

> Alan Stern
diff mbox series

Patch

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 61effd5..2f07f7c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2709,12 +2709,14 @@  static unsigned hub_is_wusb(struct usb_hub *hub)
 #define PORT_RESET_TRIES	2
 #define SET_ADDRESS_TRIES	1
 #define GET_DESCRIPTOR_TRIES	1
+#define GET_DESCRIPTOR_OPERATIONS	1
 #define PORT_INIT_TRIES		5
 
 #else
 #define PORT_RESET_TRIES	5
 #define SET_ADDRESS_TRIES	2
 #define GET_DESCRIPTOR_TRIES	2
+#define GET_DESCRIPTOR_OPERATIONS	3
 #define PORT_INIT_TRIES		4
 #endif	/* CONFIG_USB_FEW_INIT_RETRIES */
 
@@ -4699,7 +4701,7 @@  hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 			 * 255 is for WUSB devices, we actually need to use
 			 * 512 (WUSB1.0[4.8.1]).
 			 */
-			for (operations = 0; operations < 3; ++operations) {
+			for (operations = 0; operations < GET_DESCRIPTOR_OPERATIONS; ++operations) {
 				buf->bMaxPacketSize0 = 0;
 				r = usb_control_msg(udev, usb_rcvaddr0pipe(),
 					USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,