| Message ID | 20230405093133.1858140-2-badhri@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/2] usb: gadget: udc: core: Invoke usb_gadget_connect only when started | expand |
On Wed, Apr 05, 2023 at 09:31:33AM +0000, Badhri Jagan Sridharan wrote: > usb_gadget_connect calls gadget->ops->pullup without > checking whether gadget->connected was previously set. > Make this symmetric to usb_gadget_disconnect by returning > early if gadget->connected is already set. > > Cc: stable@vger.kernel.org > > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com> > Fixes: 5a1da544e572 ("usb: gadget: core: do not try to disconnect gadget if it is not connected") Same changelog comment as before. > --- > drivers/usb/gadget/udc/core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index 890f92cb6344..7eeaf7dbb350 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -708,6 +708,9 @@ int usb_gadget_connect(struct usb_gadget *gadget) > goto out; > } > > + if (gadget->connected) > + goto out; > + What prevents this connected value from changing right after you check this? thanks, greg k-h
On Wed, Apr 5, 2023 at 10:16 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Apr 05, 2023 at 09:31:33AM +0000, Badhri Jagan Sridharan wrote: > > usb_gadget_connect calls gadget->ops->pullup without > > checking whether gadget->connected was previously set. > > Make this symmetric to usb_gadget_disconnect by returning > > early if gadget->connected is already set. > > > > Cc: stable@vger.kernel.org > > > > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com> > > Fixes: 5a1da544e572 ("usb: gadget: core: do not try to disconnect gadget if it is not connected") > > Same changelog comment as before. Thanks for the feedback Greg ! Have fixed it in v2. > > > --- > > drivers/usb/gadget/udc/core.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > > index 890f92cb6344..7eeaf7dbb350 100644 > > --- a/drivers/usb/gadget/udc/core.c > > +++ b/drivers/usb/gadget/udc/core.c > > @@ -708,6 +708,9 @@ int usb_gadget_connect(struct usb_gadget *gadget) > > goto out; > > } > > > > + if (gadget->connected) > > + goto out; > > + > > What prevents this connected value from changing right after you check > this? Nothing in V1 :) However, in v2, the newly introduced mutex guards gadget->connected as well. > > thanks, > > greg k-h
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 890f92cb6344..7eeaf7dbb350 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -708,6 +708,9 @@ int usb_gadget_connect(struct usb_gadget *gadget) goto out; } + if (gadget->connected) + goto out; + if (gadget->deactivated) { /* * If gadget is deactivated we only save new state.
usb_gadget_connect calls gadget->ops->pullup without checking whether gadget->connected was previously set. Make this symmetric to usb_gadget_disconnect by returning early if gadget->connected is already set. Cc: stable@vger.kernel.org Signed-off-by: Badhri Jagan Sridharan <badhri@google.com> Fixes: 5a1da544e572 ("usb: gadget: core: do not try to disconnect gadget if it is not connected") --- drivers/usb/gadget/udc/core.c | 3 +++ 1 file changed, 3 insertions(+)