RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode

From: Roman Bacik
Date: Wed Sep 09 2015 - 22:16:54 EST


> -----Original Message-----
> From: John Youn [mailto:John.Youn@xxxxxxxxxxxx]
> Sent: September-09-15 7:11 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> usb@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; bcm-kernel-feedback-list
> Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>
> On 9/9/2015 11:16 AM, Roman Bacik wrote:
> >> -----Original Message-----
> >> From: John Youn [mailto:John.Youn@xxxxxxxxxxxx]
> >> Sent: September-03-15 11:53 PM
> >> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> >> usb@xxxxxxxxxxxxxxx; Roman Bacik
> >> Cc: linux-kernel@xxxxxxxxxxxxxxx; bcm-kernel-feedback-list
> >> Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> >> isochronous mode
> >>
> >> On 8/31/2015 9:17 AM, Scott Branden wrote:
> >>> From: Roman Bacik <rbacik@xxxxxxxxxxxx>
> >>>
> >>> USB OTG driver in isochronous mode has to set the parity of the
> >>> receiving microframe. The parity is set to even by default. This
> >>> causes problems for an audio gadget, if the host starts transmitting
> >>> on odd
> >> microframes.
> >>>
> >>> This fix uses Incomplete Periodic Transfer interrupt to toggle
> >>> between even and odd parity until the Transfer Complete interrupt is
> received.
> >>>
> >>> Signed-off-by: Roman Bacik <rbacik@xxxxxxxxxxxx>
> >>> Reviewed-by: Abhinav Ratna <aratna@xxxxxxxxxxxx>
> >>> Reviewed-by: Srinath Mannam <srinath.mannam@xxxxxxxxxxxx>
> >>> Signed-off-by: Scott Branden <sbranden@xxxxxxxxxxxx>
> >>> ---
> >>> drivers/usb/dwc2/core.h | 1 +
> >>> drivers/usb/dwc2/gadget.c | 51
> >> ++++++++++++++++++++++++++++++++++++++++++++++-
> >>> drivers/usb/dwc2/hw.h | 1 +
> >>> 3 files changed, 52 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index
> >>> 0ed87620..a5634fd 100644
> >>> --- a/drivers/usb/dwc2/core.h
> >>> +++ b/drivers/usb/dwc2/core.h
> >>> @@ -150,6 +150,7 @@ struct s3c_hsotg_ep {
> >>> unsigned int periodic:1;
> >>> unsigned int isochronous:1;
> >>> unsigned int send_zlp:1;
> >>> + unsigned int has_correct_parity:1;
> >>>
> >>> char name[10];
> >>> };
> >>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> >>> index 4d47b7c..fac3e2f 100644
> >>> --- a/drivers/usb/dwc2/gadget.c
> >>> +++ b/drivers/usb/dwc2/gadget.c
> >>> @@ -1954,6 +1954,7 @@ static void s3c_hsotg_epint(struct dwc2_hsotg
> >> *hsotg, unsigned int idx,
> >>> ints &= ~DXEPINT_XFERCOMPL;
> >>>
> >>> if (ints & DXEPINT_XFERCOMPL) {
> >>> + hs_ep->has_correct_parity = 1;
> >>> if (hs_ep->isochronous && hs_ep->interval == 1) {
> >>> if (ctrl & DXEPCTL_EOFRNUM)
> >>> ctrl |= DXEPCTL_SETEVENFR;
> >>> @@ -2316,7 +2317,8 @@ void s3c_hsotg_core_init_disconnected(struct
> >> dwc2_hsotg *hsotg,
> >>> GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST |
> >>> GINTSTS_RESETDET | GINTSTS_ENUMDONE |
> >>> GINTSTS_OTGINT | GINTSTS_USBSUSP |
> >>> - GINTSTS_WKUPINT,
> >>> + GINTSTS_WKUPINT |
> >>> + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT,
> >>> hsotg->regs + GINTMSK);
> >>>
> >>> if (using_dma(hsotg))
> >>> @@ -2581,6 +2583,52 @@ irq_retry:
> >>> s3c_hsotg_dump(hsotg);
> >>> }
> >>>
> >>> + if (gintsts & GINTSTS_INCOMPL_SOIN) {
> >>> + u32 idx, epctl_reg, ctrl;
> >>> + struct s3c_hsotg_ep *hs_ep;
> >>> +
> >>> + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOIN\n",
> >> __func__);
> >>> + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) {
> >>
> >> Valid endpoints are only up to hsotg->num_of_eps so this might crash
> >> on certain configurations.
> >>
> >> Also, have you tried to find the endpoint which caused the incomplete
> >> interrupt by reading the control registers as described in the databook?
> >>
> >
> > We are using procedure based on description from this source:
> >
> > Synopsys, Inc. SolvNet 527
> > DesignWare.com
> > 3.00a
> > April 2012
> > USB 2.0 Hi-Speed On-The-Go (OTG) Databook Isochronous Endpoints in
> > DWC_otg Slave Mode
> >
> > Synopsys databook is not in a public domain to quote the exact paragraph
> here. You can find it in Chapter E, pp 526-527. There is no register in this
> databook, which would provide information about the source endpoint of
> the incomplete interrupt, as you have described.
> >
> > Please, provide an exact reference and possibly enough information that
> we can turn it into a working code.
> >
>
> You can check the source of the interrupt by reading the DSTS and ep control
> registers as described in the databook page 526, last bullet, and 527 first
> bullet.
>
> John
>

Yes, that is exactly what we are doing what is described in the last bullet of 526 and the first two bullets of page 527. So what is the problem?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/