Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756421AbbHZOoy (ORCPT ); Wed, 26 Aug 2015 10:44:54 -0400 Received: from mail-gw3-out.broadcom.com ([216.31.210.64]:58295 "EHLO mail-gw3-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752641AbbHZOox convert rfc822-to-8bit (ORCPT ); Wed, 26 Aug 2015 10:44:53 -0400 X-IronPort-AV: E=Sophos;i="5.17,416,1437462000"; d="scan'208";a="73295398" From: Roman Bacik To: 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 1/1] usb: dwc2: gadget: parity fix in isochronous mode Thread-Topic: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode Thread-Index: AQHQ2czv/3F+l3lNJku12UbnWaDFep4eZmyQ Date: Wed, 26 Aug 2015 14:44:51 +0000 Message-ID: References: <1439912718-21045-1-git-send-email-sbranden@broadcom.com> <1439912718-21045-2-git-send-email-sbranden@broadcom.com> <2B3535C5ECE8B5419E3ECBE30077290901DC33F275@us01wembx1.internal.synopsys.com> <2B3535C5ECE8B5419E3ECBE30077290901DC33FD84@us01wembx1.internal.synopsys.com> In-Reply-To: <2B3535C5ECE8B5419E3ECBE30077290901DC33FD84@us01wembx1.internal.synopsys.com> Accept-Language: en-CA, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.16.203.100] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6442 Lines: 177 > -----Original Message----- > From: John Youn [mailto:John.Youn@synopsys.com] > Sent: August-25-15 7:06 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 1/1] usb: dwc2: gadget: parity fix in isochronous mode > > On 8/25/2015 3:00 PM, Roman Bacik wrote: > >> -----Original Message----- > >> From: John Youn [mailto:John.Youn@synopsys.com] > >> Sent: August-25-15 2:52 PM > >> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux- > >> usb@vger.kernel.org > >> Cc: linux-kernel@vger.kernel.org; bcm-kernel-feedback-list; Roman > >> Bacik > >> Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous > >> mode > >> > >> On 8/18/2015 8:45 AM, Scott Branden wrote: > >>> From: Roman Bacik > >>> > >>> 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 > >>> Reviewed-by: Abhinav Ratna > >>> Reviewed-by: Srinath Mannam > >>> Reviewed-by: Scott Branden > >>> Signed-off-by: Scott Branden > >>> --- > >>> drivers/usb/dwc2/core.h | 1 + > >>> drivers/usb/dwc2/gadget.c | 48 > >> ++++++++++++++++++++++++++++++++++++++++++++++- > >>> drivers/usb/dwc2/hw.h | 1 + > >>> 3 files changed, 49 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index > >>> 0ed87620..954d1cd 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 parity_set:1; > >>> > >>> char name[10]; > >>> }; > >>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > >>> index 4d47b7c..28e4393 100644 > >>> --- a/drivers/usb/dwc2/gadget.c > >>> +++ b/drivers/usb/dwc2/gadget.c > >>> @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg > >> *hsotg, unsigned int idx, > >>> ints &= ~DXEPINT_XFERCOMPL; > >>> > >>> if (ints & DXEPINT_XFERCOMPL) { > >>> + if (hs_ep->isochronous && !hs_ep->parity_set) > >>> + hs_ep->parity_set = 1; > >>> if (hs_ep->isochronous && hs_ep->interval == 1) { > >>> if (ctrl & DXEPCTL_EOFRNUM) > >>> ctrl |= DXEPCTL_SETEVENFR; > >>> @@ -2316,7 +2318,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 +2584,48 @@ irq_retry: > >>> s3c_hsotg_dump(hsotg); > >>> } > >>> > >>> + if (gintsts & GINTSTS_INCOMPL_SOIN) { > >>> + u32 idx; > >>> + 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->parity_set) { > >>> + u32 epctl_reg = DIEPCTL(idx); > >>> + u32 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; > >>> + 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->parity_set) { > >>> + u32 epctl_reg = DOEPCTL(idx); > >>> + u32 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); > >>> + } > >>> + > >>> /* > >>> * if we've had fifo events, we should try and go around the > >>> * loop again to see if there's any point in returning yet. > >>> @@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep > >> *ep, > >>> hs_ep->periodic = 0; > >>> hs_ep->halted = 0; > >>> hs_ep->interval = desc->bInterval; > >>> + hs_ep->parity_set = 0; > >> > >> > >> I'm not quite sure what the parity_set flag does in this patch. > >> Shouldn't you be able to just toggle the even/odd frame when you get > >> the interrupt? > >> > >> John > >> > > > > When Transfer Complete interrupt is received, we have the correct parity. > Therefore we set the flag and we stop toggling. The parity_set flag indicates > whether we have the correct parity set. > > Regards, > > > > Roman > > > > I'm not sure that "parity" is the proper term in this context but I can't think of > a more concise way to phrase it. > > What if the host switches again some time after the first xfer complete? > > What function or gadget driver are you using to test this? Did you test both > the ISO IN and OUT case? > > John > > This patch is related to isochronous or periodic transfers. So server uses the same microframe number and hence parity for transfers. I tested both IN and OUT directions using audio gadget on Altera SocKit and Broadcom development boards. Regards, Roman -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/