[PATCH v2 1/1] usb: chipidea: udc: workaround for endpoint conflict issue

[PATCH v2 1/1] usb: chipidea: udc: workaround for endpoint conflict issue

[Peter Chen]

An endpoint conflict occurs when the USB is working in device mode
during an isochronous communication. When the endpointA IN direction
is an isochronous IN endpoint, and the host sends an IN token to
endpointA on another device, then the OUT transaction may be missed
regardless the OUT endpoint number. Generally, this occurs when the
device is connected to the host through a hub and other devices are
connected to the same hub.

The affected OUT endpoint can be either control, bulk, isochronous, or
an interrupt endpoint. After the OUT endpoint is primed, if an IN token
to the same endpoint number on another device is received, then the OUT
endpoint may be unprimed (cannot be detected by software), which causes
this endpoint to no longer respond to the host OUT token, and thus, no
corresponding interrupt occurs.

There is no good workaround for this issue, the only thing the software
could do is numbering isochronous IN from the highest endpoint since we
have observed most of device number endpoint from the lowest.

Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: Jun Li <jun.li@nxp.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
Changes for v2:
- Improve the code sytle

 drivers/usb/chipidea/udc.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 829e947cabf5..21c1344bfc42 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1622,6 +1622,28 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
 static int ci_udc_start(struct usb_gadget *gadget,
 			 struct usb_gadget_driver *driver);
 static int ci_udc_stop(struct usb_gadget *gadget);
+
+
+/* Match ISOC IN from the highest endpoint */
+static struct usb_ep *ci_udc_match_ep(struct usb_gadget *gadget,
+			      struct usb_endpoint_descriptor *desc,
+			      struct usb_ss_ep_comp_descriptor *comp_desc)
+{
+	struct ci_hdrc *ci = container_of(gadget, struct ci_hdrc, gadget);
+	struct usb_ep *ep;
+	u8 type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
+
+	if ((type == USB_ENDPOINT_XFER_ISOC) &&
+			(desc->bEndpointAddress & USB_DIR_IN)) {
+		list_for_each_entry_reverse(ep, &ci->gadget.ep_list, ep_list) {
+			if (ep->caps.dir_in && !ep->claimed)
+				return ep;
+		}
+	}
+
+	return NULL;
+}
+
 /**
  * Device operations part of the API to the USB controller hardware,
  * which don't involve endpoints (or i/o)
@@ -1635,6 +1657,7 @@ static const struct usb_gadget_ops usb_gadget_ops = {
 	.vbus_draw	= ci_udc_vbus_draw,
 	.udc_start	= ci_udc_start,
 	.udc_stop	= ci_udc_stop,
+	.match_ep 	= ci_udc_match_ep,
 };
 
 static int init_eps(struct ci_hdrc *ci)
-- 
2.14.1

Re: [PATCH v2 1/1] usb: chipidea: udc: workaround for endpoint conflict issue

[Greg KH]

On Thu, May 30, 2019 at 04:50:39PM +0800, Peter Chen wrote:
> An endpoint conflict occurs when the USB is working in device mode
> during an isochronous communication. When the endpointA IN direction
> is an isochronous IN endpoint, and the host sends an IN token to
> endpointA on another device, then the OUT transaction may be missed
> regardless the OUT endpoint number. Generally, this occurs when the
> device is connected to the host through a hub and other devices are
> connected to the same hub.
> 
> The affected OUT endpoint can be either control, bulk, isochronous, or
> an interrupt endpoint. After the OUT endpoint is primed, if an IN token
> to the same endpoint number on another device is received, then the OUT
> endpoint may be unprimed (cannot be detected by software), which causes
> this endpoint to no longer respond to the host OUT token, and thus, no
> corresponding interrupt occurs.
> 
> There is no good workaround for this issue, the only thing the software
> could do is numbering isochronous IN from the highest endpoint since we
> have observed most of device number endpoint from the lowest.
> 
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: Jun Li <jun.li@nxp.com>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
> Changes for v2:
> - Improve the code sytle
> 
>  drivers/usb/chipidea/udc.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 829e947cabf5..21c1344bfc42 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1622,6 +1622,28 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
>  static int ci_udc_start(struct usb_gadget *gadget,
>  			 struct usb_gadget_driver *driver);
>  static int ci_udc_stop(struct usb_gadget *gadget);
> +
> +
> +/* Match ISOC IN from the highest endpoint */
> +static struct usb_ep *ci_udc_match_ep(struct usb_gadget *gadget,
> +			      struct usb_endpoint_descriptor *desc,
> +			      struct usb_ss_ep_comp_descriptor *comp_desc)
> +{
> +	struct ci_hdrc *ci = container_of(gadget, struct ci_hdrc, gadget);
> +	struct usb_ep *ep;
> +	u8 type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> +
> +	if ((type == USB_ENDPOINT_XFER_ISOC) &&

usb_endpoint_xfer_isoc()?

> +			(desc->bEndpointAddress & USB_DIR_IN)) {

usb_endpoint_dir_in()?

thanks,

greg k-h

Re: [PATCH v2 1/1] usb: chipidea: udc: workaround for endpoint conflict issue

[Fabio Estevam]

Hi Peter,

On Thu, May 30, 2019 at 5:50 AM Peter Chen <peter.chen@nxp.com> wrote:
>
> An endpoint conflict occurs when the USB is working in device mode
> during an isochronous communication. When the endpointA IN direction
> is an isochronous IN endpoint, and the host sends an IN token to
> endpointA on another device, then the OUT transaction may be missed
> regardless the OUT endpoint number. Generally, this occurs when the
> device is connected to the host through a hub and other devices are
> connected to the same hub.
>
> The affected OUT endpoint can be either control, bulk, isochronous, or
> an interrupt endpoint. After the OUT endpoint is primed, if an IN token
> to the same endpoint number on another device is received, then the OUT
> endpoint may be unprimed (cannot be detected by software), which causes
> this endpoint to no longer respond to the host OUT token, and thus, no
> corresponding interrupt occurs.
>
> There is no good workaround for this issue, the only thing the software
> could do is numbering isochronous IN from the highest endpoint since we
> have observed most of device number endpoint from the lowest.
>
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: Jun Li <jun.li@nxp.com>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>

Should this patch have Cc stable so that it gets applied for older kernels?

> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 829e947cabf5..21c1344bfc42 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1622,6 +1622,28 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
>  static int ci_udc_start(struct usb_gadget *gadget,
>                          struct usb_gadget_driver *driver);
>  static int ci_udc_stop(struct usb_gadget *gadget);
> +
> +

One blank line is enough.

Re: [PATCH v2 1/1] usb: chipidea: udc: workaround for endpoint conflict issue

[Peter Chen]

On Thu, May 30, 2019 at 10:14 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Peter,
>
> On Thu, May 30, 2019 at 5:50 AM Peter Chen <peter.chen@nxp.com> wrote:
> >
> > An endpoint conflict occurs when the USB is working in device mode
> > during an isochronous communication. When the endpointA IN direction
> > is an isochronous IN endpoint, and the host sends an IN token to
> > endpointA on another device, then the OUT transaction may be missed
> > regardless the OUT endpoint number. Generally, this occurs when the
> > device is connected to the host through a hub and other devices are
> > connected to the same hub.
> >
> > The affected OUT endpoint can be either control, bulk, isochronous, or
> > an interrupt endpoint. After the OUT endpoint is primed, if an IN token
> > to the same endpoint number on another device is received, then the OUT
> > endpoint may be unprimed (cannot be detected by software), which causes
> > this endpoint to no longer respond to the host OUT token, and thus, no
> > corresponding interrupt occurs.
> >
> > There is no good workaround for this issue, the only thing the software
> > could do is numbering isochronous IN from the highest endpoint since we
> > have observed most of device number endpoint from the lowest.
> >
> > Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> > Cc: Jun Li <jun.li@nxp.com>
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
>
> Should this patch have Cc stable so that it gets applied for older kernels?
>
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 829e947cabf5..21c1344bfc42 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1622,6 +1622,28 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
> >  static int ci_udc_start(struct usb_gadget *gadget,
> >                          struct usb_gadget_driver *driver);
> >  static int ci_udc_stop(struct usb_gadget *gadget);
> > +
> > +
>
> One blank line is enough.

Thanks, both.

Will be addressed at next version.

Peter