Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932762AbbHYWf6 (ORCPT ); Tue, 25 Aug 2015 18:35:58 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:51739 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932420AbbHYWfz (ORCPT ); Tue, 25 Aug 2015 18:35:55 -0400 Date: Tue, 25 Aug 2015 17:35:50 -0500 From: Felipe Balbi To: Roman Bacik CC: John Youn , Scott Branden , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , bcm-kernel-feedback-list Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode Message-ID: <20150825223550.GF10704@saruman.tx.rr.com> Reply-To: References: <1439912718-21045-1-git-send-email-sbranden@broadcom.com> <1439912718-21045-2-git-send-email-sbranden@broadcom.com> <2B3535C5ECE8B5419E3ECBE30077290901DC33F275@us01wembx1.internal.synopsys.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="924gEkU1VlJlwnwX" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5746 Lines: 168 --924gEkU1VlJlwnwX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 25, 2015 at 10:00:17PM +0000, 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 m= ode > >=20 > > 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 &=3D ~DXEPINT_XFERCOMPL; > > > > > > if (ints & DXEPINT_XFERCOMPL) { > > > + if (hs_ep->isochronous && !hs_ep->parity_set) > > > + hs_ep->parity_set =3D 1; it shouldn't be a problem to set the flag which was already set, so this could be simplified to: hs_ep->has_correct_parity =3D !!hs_ep0>isochronous; > > > if (hs_ep->isochronous && hs_ep->interval =3D=3D 1) { > > > if (ctrl & DXEPCTL_EOFRNUM) > > > ctrl |=3D 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, why the two extra bits ? What are they doing ? > > > 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 =3D 1; idx < MAX_EPS_CHANNELS; idx++) { u32 epctl_reg; u32 ctrl; > > > + hs_ep =3D hsotg->eps_in[idx]; you can decrease some indentation here: if (!hs_ep->isochronous) continue; if (hs_ep->has_correct_parity) continue; epctl_reg =3D DIEPCTL(idx); ctrl =3D readl(hsotg->regs + epctl_reg); if (ctrl & DXEPCTL_EOFRNUM) ctrl |=3D DXEPCTL_SETEVENFR; else ctrl |=3D DXEPCTL_SETODDFR; writel(ctrl, hsotg->regs + epctl_reg); ditto to the other loop below > > 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? > >=20 > > John > >=20 >=20 > 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. then how about calling it has_correct_parity instead ? --=20 balbi --924gEkU1VlJlwnwX Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV3O3GAAoJEIaOsuA1yqREIUAP/jsFhlLDs9/ue8uN38JJhAgE c+7l/PEpIsjYsjC68yHVVSbGtQs4MHrjh/+4X9zLYY8+WOcXScFwpf5u3eQn592l 9MePgjZxu9i0ErqJSNXIG5MRdoXDQm7lvNnUFSNx8opiRso9TnJQZ2Ve5NCai8Cy R6yOvPdRfghIk+IcRjwpy/x4ju0d8g+Rr89r9uTAo1gpmEAA4n4yA8l+SywB5jDZ rb+ZYfTItfZolJibU/rVg/CXC5uB/4hTZnl7ULA7g8K2ByxpwPupqyWD1ggLHfdq OdAvN7nr9OPxItsXwoTqDAMd2/eGZU/FPC8xaEdfq1hdVmbCwzb88ByhbDNvNuzV G1L4JduL0tjPiWx+7Ouibicgjdnp4San7+WMo/mNwV2rC7wpVyXILCH651mOcB57 FiW2XeCQl/fur7YW5gJXNDfILzQ0LxGanTIYSq50KDtVjFrw1mFrUeTkvrZ0xO2E a/8/cyiqV5Qu4eGj1AV1qDRcCxpdb07XTrNXiPrmSBNnHGCxcOeRbZv8jLb32/+C 129loliwRcETF8SEAh2nNM0FATkT30vKxc0j1Ys6Obn3GMaHS3u8MORXozcqKX9D 9X6VW7TfxpC1fIHCSDDyujeMiBQLtjKWY1Mqfp0kzMpTzIM6Z7vOP/ApHY+WYyvA ykAKMGpD+ZbOnWM0VF9Y =+0UW -----END PGP SIGNATURE----- --924gEkU1VlJlwnwX-- -- 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/