lkml.org 
[lkml]   [2015]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
Date
> -----Original Message-----
> From: John Youn [mailto:John.Youn@synopsys.com]
> Sent: September-09-15 7:25 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> usb@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; bcm-kernel-feedback-list
> Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>
> On 9/9/2015 7:16 PM, Roman Bacik wrote:
> >> -----Original Message-----
> >> From: John Youn [mailto:John.Youn@synopsys.com]
> >> Sent: September-09-15 7:11 PM
> >> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> >> usb@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org; 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@synopsys.com]
> >>>> Sent: September-03-15 11:53 PM
> >>>> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> >>>> usb@vger.kernel.org; Roman Bacik
> >>>> Cc: linux-kernel@vger.kernel.org; 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@broadcom.com>
> >>>>>
> >>>>> 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@broadcom.com>
> >>>>> Reviewed-by: Abhinav Ratna <aratna@broadcom.com>
> >>>>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
> >>>>> Signed-off-by: Scott Branden <sbranden@broadcom.com>
> >>>>> ---
> >>>>> 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?
> >
>
> Maybe I missed it. Where exactly are you doing it like it says in these two
> paragraphs?
>
> Quote:
>
> GINTSTS.incompISOIN is a global interrupt. Therefore, when more than one
> isochronous endpoints is active, then the application must determine which
> isochronous IN endpoints have incomplete data.
>
> To accomplish this, read DSTS and DIEPCTLn for all isochronous endpoints. If
> DSTS.SOFFN[0] =DIEPCTLn.EO_FrNum (Bit 16),then that endpoint has an
> incomplete transfer
>
> John
>
>

That is the content of the two for loops. I will paste the code here again for your convenience:

> + 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++) {
> + hs_ep = hsotg->eps_in[idx];
> +
> + if (!hs_ep->isochronous || hs_ep-
> >has_correct_parity)
> + continue;
> +
> + epctl_reg = DIEPCTL(idx);
> + ctrl = readl(hsotg->regs + epctl_reg);
> +
> + if (ctrl & DXEPCTL_EOFRNUM)
> + ctrl |= DXEPCTL_SETEVENFR;
> + else
> + ctrl |= DXEPCTL_SETODDFR;
> + writel(ctrl, hsotg->regs + epctl_reg);
> + }
> + writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS);
> + }
> +
> + if (gintsts & GINTSTS_INCOMPL_SOOUT) {
> + u32 idx, epctl_reg, ctrl;
> + struct s3c_hsotg_ep *hs_ep;
> +
> + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOOUT\n",
> __func__);
> + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) {
> + hs_ep = hsotg->eps_out[idx];
> +
> + if (!hs_ep->isochronous || hs_ep-
> >has_correct_parity)
> + continue;
> +
> + epctl_reg = DOEPCTL(idx);
> + ctrl = readl(hsotg->regs + epctl_reg);
> +
> + if (ctrl & DXEPCTL_EOFRNUM)
> + ctrl |= DXEPCTL_SETEVENFR;
> + else
> + ctrl |= DXEPCTL_SETODDFR;
> + writel(ctrl, hsotg->regs + epctl_reg);
> + }
> + writel(GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTSTS);
> + }
> +





\
 
 \ /
  Last update: 2015-09-10 05:01    [W:0.082 / U:0.928 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site