mbox series

[0/2] usb: dwc3: gadget: Check for multiple start/stop

Message ID cover.1609865348.git.Thinh.Nguyen@synopsys.com
Headers show
Series usb: dwc3: gadget: Check for multiple start/stop | expand

Message

Thinh Nguyen Jan. 5, 2021, 4:56 p.m. UTC
Add some checks to avoid going through the start/stop sequence if the gadget
had already started/stopped. This series base-commit is Greg's usb-linus
branch.



Thinh Nguyen (2):
  usb: dwc3: gadget: Check if the gadget had started
  usb: dwc3: gadget: Check if the gadget had stopped

 drivers/usb/dwc3/gadget.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)


base-commit: 96ebc9c871d8a28fb22aa758dd9188a4732df482

Comments

Felipe Balbi Jan. 6, 2021, 7:54 a.m. UTC | #1
Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> If the gadget had already stopped, don't try to stop again. Otherwise

> we'd get a warning for trying to free an already freed irq. This can

> happen if a user tries to trigger a soft-disconnect from soft_connect

> sysfs multiple times. The fix is to check if there's a bounded gadget

> driver to determined if the gadget had stopped.

>

> Cc: stable@vger.kernel.org

> Fixes: 8698e2acf3a5 ("usb: dwc3: gadget: introduce and use enable/disable irq methods")

> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

> ---

>  drivers/usb/dwc3/gadget.c | 4 ++++

>  1 file changed, 4 insertions(+)

>

> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c

> index 51d81a32ce78..9ec70282e610 100644

> --- a/drivers/usb/dwc3/gadget.c

> +++ b/drivers/usb/dwc3/gadget.c

> @@ -2338,6 +2338,10 @@ static int dwc3_gadget_stop(struct usb_gadget *g)

>  	struct dwc3		*dwc = gadget_to_dwc(g);

>  	unsigned long		flags;

>  

> +	/* The controller is already stopped if there's no gadget driver */

> +	if (!dwc->gadget_driver)

> +		return 0;


same here. Better done at the framework

-- 
balbi
Peter Chen Jan. 8, 2021, 2:36 a.m. UTC | #2
On 21-01-05 08:56:28, Thinh Nguyen wrote:
> Add some checks to avoid going through the start/stop sequence if the gadget

> had already started/stopped. This series base-commit is Greg's usb-linus

> branch.

> 


Hi Thinh,

What's the sequence your could reproduce it?

Peter
> 

> 

> Thinh Nguyen (2):

>   usb: dwc3: gadget: Check if the gadget had started

>   usb: dwc3: gadget: Check if the gadget had stopped

> 

>  drivers/usb/dwc3/gadget.c | 28 ++++++++++++----------------

>  1 file changed, 12 insertions(+), 16 deletions(-)

> 

> 

> base-commit: 96ebc9c871d8a28fb22aa758dd9188a4732df482

> -- 

> 2.28.0

> 


-- 

Thanks,
Peter Chen
Thinh Nguyen Jan. 8, 2021, 2:40 a.m. UTC | #3
Hi Peter,

Peter Chen wrote:
> On 21-01-05 08:56:28, Thinh Nguyen wrote:

>> Add some checks to avoid going through the start/stop sequence if the gadget

>> had already started/stopped. This series base-commit is Greg's usb-linus

>> branch.

>>

> Hi Thinh,

>

> What's the sequence your could reproduce it?

>

> Peter


You can test as follow:

# echo connect > /sys/class/udc/<UDC>/soft_connect
# echo connect > /sys/class/udc/<UDC>/soft_connect

and

# echo disconnect > /sys/class/udc/<UDC>/soft_connect
# echo disconnect > /sys/class/udc/<UDC>/soft_connect

Thinh

>>

>> Thinh Nguyen (2):

>>   usb: dwc3: gadget: Check if the gadget had started

>>   usb: dwc3: gadget: Check if the gadget had stopped

>>

>>  drivers/usb/dwc3/gadget.c | 28 ++++++++++++----------------

>>  1 file changed, 12 insertions(+), 16 deletions(-)

>>

>>

>> base-commit: 96ebc9c871d8a28fb22aa758dd9188a4732df482

>> -- 

>> 2.28.0

>>
Peter Chen Jan. 8, 2021, 9:40 a.m. UTC | #4
On 21-01-08 02:40:55, Thinh Nguyen wrote:
> Hi Peter,

> 

> Peter Chen wrote:

> > On 21-01-05 08:56:28, Thinh Nguyen wrote:

> >> Add some checks to avoid going through the start/stop sequence if the gadget

> >> had already started/stopped. This series base-commit is Greg's usb-linus

> >> branch.

> >>

> > Hi Thinh,

> >

> > What's the sequence your could reproduce it?

> >

> > Peter

> 

> You can test as follow:

> 

> # echo connect > /sys/class/udc/<UDC>/soft_connect

> # echo connect > /sys/class/udc/<UDC>/soft_connect

> 

> and

> 

> # echo disconnect > /sys/class/udc/<UDC>/soft_connect

> # echo disconnect > /sys/class/udc/<UDC>/soft_connect

> 

> Thinh

> 


Thanks, now I reproduce the issue. Another improvement you
might consider adding is checking return value for usb_gadget_udc_start
at soft_connect_store.

-- 

Thanks,
Peter Chen