Message ID | 20191203104648.29291-1-ejh@nvidia.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | usb: gadget: fix wrong endpoint desc | expand |
Hi, EJ Hsu <ejh@nvidia.com> writes: > Gadget driver should always use config_ep_by_speed() to initialize > usb_ep struct according to usb device's operating speed. Otherwise, > usb_ep struct may be wrong if usb devcie's operating speed is changed. your commit log doesn't match the patch body. What gives? > Signed-off-by: EJ Hsu <ejh@nvidia.com> > --- > drivers/usb/gadget/function/f_ecm.c | 4 ++++ > drivers/usb/gadget/function/f_rndis.c | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c > index 6ce044008cf6..494fe4e38a65 100644 > --- a/drivers/usb/gadget/function/f_ecm.c > +++ b/drivers/usb/gadget/function/f_ecm.c > @@ -623,6 +623,10 @@ static void ecm_disable(struct usb_function *f) > > if (ecm->port.in_ep->enabled) > gether_disconnect(&ecm->port); > + else { coding style. If one branch has {}, both branches should have {} ;-)
Hi, Felipe Balb writes: >EJ Hsu <ejh@nvidia.com> writes: >> Gadget driver should always use config_ep_by_speed() to initialize >> usb_ep struct according to usb device's operating speed. Otherwise, >> usb_ep struct may be wrong if usb devcie's operating speed is changed. > >your commit log doesn't match the patch body. What gives? The key point in this patch is that we want to make sure the desc pointer in usb_ep struct will be set to NULL when gadget is disconnected. This will force it to call config_ep_by_speed() to correctly initialize usb_ep struct based on the new operating speed when gadget is re-connected later. >> Signed-off-by: EJ Hsu <ejh@nvidia.com> >> --- >> drivers/usb/gadget/function/f_ecm.c | 4 ++++ >> drivers/usb/gadget/function/f_rndis.c | 1 + >> 2 files changed, 5 insertions(+) >> >> diff --git a/drivers/usb/gadget/function/f_ecm.c >> b/drivers/usb/gadget/function/f_ecm.c >> index 6ce044008cf6..494fe4e38a65 100644 >> --- a/drivers/usb/gadget/function/f_ecm.c >> +++ b/drivers/usb/gadget/function/f_ecm.c >> @@ -623,6 +623,10 @@ static void ecm_disable(struct usb_function *f) >> >> if (ecm->port.in_ep->enabled) >> gether_disconnect(&ecm->port); >> + else { > >coding style. If one branch has {}, both branches should have {} ;-) Thanks. Will correct it. EJ --nvpublic
Hi, EJ Hsu <ejh@nvidia.com> writes: > Felipe Balb writes: >>EJ Hsu <ejh@nvidia.com> writes: >>> Gadget driver should always use config_ep_by_speed() to initialize >>> usb_ep struct according to usb device's operating speed. Otherwise, >>> usb_ep struct may be wrong if usb devcie's operating speed is changed. >> >>your commit log doesn't match the patch body. What gives? > > The key point in this patch is that we want to make sure the desc pointer in > usb_ep struct will be set to NULL when gadget is disconnected. This will force > it to call config_ep_by_speed() to correctly initialize usb_ep struct based on > the new operating speed when gadget is re-connected later. this would be a commit log that matches the implementation ;-)
diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c index 6ce044008cf6..494fe4e38a65 100644 --- a/drivers/usb/gadget/function/f_ecm.c +++ b/drivers/usb/gadget/function/f_ecm.c @@ -623,6 +623,10 @@ static void ecm_disable(struct usb_function *f) if (ecm->port.in_ep->enabled) gether_disconnect(&ecm->port); + else { + ecm->port.in_ep->desc = NULL; + ecm->port.out_ep->desc = NULL; + } usb_ep_disable(ecm->notify); ecm->notify->desc = NULL; diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c index d48df36622b7..0d8e4a364ca6 100644 --- a/drivers/usb/gadget/function/f_rndis.c +++ b/drivers/usb/gadget/function/f_rndis.c @@ -618,6 +618,7 @@ static void rndis_disable(struct usb_function *f) gether_disconnect(&rndis->port); usb_ep_disable(rndis->notify); + rndis->notify->desc = NULL; } /*-------------------------------------------------------------------------*/
Gadget driver should always use config_ep_by_speed() to initialize usb_ep struct according to usb device's operating speed. Otherwise, usb_ep struct may be wrong if usb devcie's operating speed is changed. Signed-off-by: EJ Hsu <ejh@nvidia.com> --- drivers/usb/gadget/function/f_ecm.c | 4 ++++ drivers/usb/gadget/function/f_rndis.c | 1 + 2 files changed, 5 insertions(+)