RE: [PATCH] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-10-15 6:14 PM
> To: Scott Branden; Roman Bacik; linux-usb@vger.kernel.org
> Cc: bcm-kernel-feedback-list
> Subject: [PATCH] usb: dwc2: gadget: parity fix in isochronous mode
>
> From: Roman Bacik <rba...@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 <rba...@broadcom.com>
> Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> Reviewed-by: Srinath Mannam <srinath.man...@broadcom.com>
> Signed-off-by: Scott Branden <sbran...@broadcom.com>
> Signed-off-by: John Youn <johny...@synopsys.com>
> ---
>
> Hi Roman, Scott,
>
> I have updated this patch for slave mode ISOC OUT and rebased against
> latest code.
>
> It shouldn't affect your platform in dma mode but just in case could you try
> it
> and make sure it still works the same?
>
> Regards,
> John
John,
I have verified that your modification still works the same on our platform.
Thank you very much.
Regards,
Roman
>
>
> drivers/usb/dwc2/core.h | 1 +
> drivers/usb/dwc2/gadget.c | 71
> +--
> drivers/usb/dwc2/hw.h | 1 +
> 3 files changed, 65 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index
> 3056981..ebf2504 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -166,6 +166,7 @@ struct dwc2_hsotg_ep {
> unsigned intperiodic:1;
> unsigned intisochronous:1;
> unsigned intsend_zlp:1;
> + unsigned inthas_correct_parity:1;
>
> charname[10];
> };
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index
> 19202c1c..7e5670c 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -1513,6 +1513,19 @@ static void dwc2_hsotg_ep0_zlp(struct
> dwc2_hsotg *hsotg, bool dir_in)
> dwc2_hsotg_program_zlp(hsotg, hsotg->eps_out[0]); }
>
> +static void dwc2_hsotg_change_ep_iso_parity(struct dwc2_hsotg *hsotg,
> + u32 epctl_reg)
> +{
> + u32 ctrl;
> +
> + ctrl = dwc2_readl(hsotg->regs + epctl_reg);
> + if (ctrl & DXEPCTL_EOFRNUM)
> + ctrl |= DXEPCTL_SETEVENFR;
> + else
> + ctrl |= DXEPCTL_SETODDFR;
> + dwc2_writel(ctrl, hsotg->regs + epctl_reg); }
> +
> /**
> * dwc2_hsotg_handle_outdone - handle receiving OutDone/SetupDone
> from RXFIFO
> * @hsotg: The device instance
> @@ -1583,6 +1596,16 @@ static void dwc2_hsotg_handle_outdone(struct
> dwc2_hsotg *hsotg, int epnum)
> return;
> }
>
> + /*
> + * Slave mode OUT transfers do not go through XferComplete so
> + * adjust the ISOC parity here.
> + */
> + if (!using_dma(hsotg)) {
> + hs_ep->has_correct_parity = 1;
> + if (hs_ep->isochronous && hs_ep->interval == 1)
> + dwc2_hsotg_change_ep_iso_parity(hsotg,
> DOEPCTL(epnum));
> + }
> +
> dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, result); }
>
> @@ -1955,13 +1978,9 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg
> *hsotg, unsigned int idx,
> ints &= ~DXEPINT_XFERCOMPL;
>
> if (ints & DXEPINT_XFERCOMPL) {
> - if (hs_ep->isochronous && hs_ep->interval == 1) {
> - if (ctrl & DXEPCTL_EOFRNUM)
> - ctrl |= DXEPCTL_SETEVENFR;
> - else
> - ctrl |= DXEPCTL_SETODDFR;
> - dwc2_writel(ctrl, hsotg->regs + epctl_reg);
> - }
> + hs_ep->has_correct_parity = 1;
> + if (hs_ep->isochronous && hs_ep->interval == 1)
> + dwc2_hsotg_change_ep_iso_parity(hsotg,
> epctl_reg);
>
> dev_dbg(hsotg->dev,
> "%s: XferCompl: DxEPCTL=0x%08x,
> DXEPTSIZ=%08x\n", @@ -2321,7 +2340,8 @@ void
> dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg,
> GINTSTS_GOUTNAKEFF | GINTSTS_GINNAKEFF |
> GINTSTS_USBRST | GINTSTS_RESETDET |
> GINTSTS_ENUMDONE |
RE: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-24-15 8:16 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> Subject: Re: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
>
> On 9/24/2015 10:28 AM, Roman Bacik wrote:
> >> -Original Message-
> >> From: John Youn [mailto:john.y...@synopsys.com]
> >> Sent: September-23-15 9:21 PM
> >> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> >> u...@vger.kernel.org; Roman Bacik
> >> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> >> Subject: Re: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
> >>
> >> On 9/10/2015 6:14 PM, Scott Branden wrote:
> >>> This patch contains a fix for a real world interop problem found
> >>> when using the Synopsis DWC2 USB controller with isochronous audio
> >>> as detailed in the commit message.
> >>>
> >>> Changes from v2:
> >>> - created s2c_hsotg_chage_ep_iso_parity function to call function
> >>> in
> >>> 3 places in code
> >>> - used hsotg->num_of_eps instead of MAX_EPS_CHANNELS
> >>>
> >>> Changes from v1:
> >>> - Address code review comments as per previous responses:
> >>> - renamed parity_set to has_correct_parity and reorder some logic
> >>>
> >>>
> >>> Roman Bacik (1):
> >>> usb: dwc2: gadget: parity fix in isochronous mode
> >>>
> >>> drivers/usb/dwc2/core.h | 1 +
> >>> drivers/usb/dwc2/gadget.c | 58
> >> ++-
> >>> drivers/usb/dwc2/hw.h | 1 +
> >>> 3 files changed, 54 insertions(+), 6 deletions(-)
> >>>
> >>
> >> This seems to break slave mode on my platform. It seems to be
> >> dropping a lot of packets. I tried bInterval=4,5,6, ISO OUT.
> >> I'll need to take a closer look to determine why. Probably later this week.
> >>
> >> Are you able to run in slave mode on your platform? If so can you try it
> out?
> >>
> >> Regards,
> >> John
> >
> > Our current test procedure is as follows:
> >
> > Build Linux kernel with:
> >
> > Device Drivers
> > - Sound Card Support 'SOUND=y': ALSA 'SND=y'
> > - Generic sound devices: Dummy (/dev/null) soundcard
> 'SND_DUMMY=y'
> > - USB support 'USB_SUPPORT=y':
> > DesignWare USB2 DRD Core support 'USB_DWC2=y'
> > - Gadget only mode 'USB_DWC2_PERIPHERAL=y'
> > DWC2 Platform 'USB_DWC2_PLATFORM=y'
> > - USB Gadget Support 'USB_GADGET=y': Audio Gadget 'USB_AUDIO=y'
> > - PHY Subsystem: Broadcom Kona USB2 PHY Driver
> 'BCM_KONA_USB2_PHY=y'
> >
> > If you have only even hosts, you can change the default micro frame parity
> to odd:
> >
> > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> > index ad45b0b..80bde75 100644 @@ -2709,7 +2709,7 @@ static int
> > s3c_hsotg_ep_enable(struct usb_ep *ep,
> > switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
> > case USB_ENDPOINT_XFER_ISOC:
> > epctrl |= DXEPCTL_EPTYPE_ISO;
> > -epctrl |= DXEPCTL_SETEVENFR;
> > +epctrl |= DXEPCTL_SETODDFR;
> > hs_ep->isochronous = 1;
> > if (dir_in)
> > hs_ep->periodic = 1;
> > @@ -2777,7 +2777,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep
> > *ep,
> >
> > /* for non control endpoints, set PID to D0 */
> > if (index)
> > -epctrl |= DXEPCTL_SETD0PID;
> > +epctrl |= DXEPCTL_SETD1PID;
> >
> > dev_dbg(hsotg->dev, "%s: write DxEPCTL=0x%08x\n",
> > __func__, epctrl);
> >
> > To test OUT direction:
> > Host:
> > aplay -D plughw:2 -r 48000 -f S16_LE tone_48stereo.wav
> > Device:
> > arecord -D plughw:0 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm
> >
> > To test IN direction:
> > Host:
> > arecord -D plughw:2 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm
> > Device:
> > aplay -D plughw:0 -r 48000 -f S16_LE /tmp/rec.pcm
> >
> > If you would like, we can try to test your procedure provided you send us
> enough details.
> > Regards,
> >
> > Roman
> >
> >
>
> I looked at it a bit more and I think there are two issues.
>
> In slave-mo
RE: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-23-15 9:21 PM
> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org; Roman Bacik
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> Subject: Re: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
>
> On 9/10/2015 6:14 PM, Scott Branden wrote:
> > This patch contains a fix for a real world interop problem found when
> > using the Synopsis DWC2 USB controller with isochronous audio as
> > detailed in the commit message.
> >
> > Changes from v2:
> > - created s2c_hsotg_chage_ep_iso_parity function to call function in
> > 3 places in code
> > - used hsotg->num_of_eps instead of MAX_EPS_CHANNELS
> >
> > Changes from v1:
> > - Address code review comments as per previous responses:
> > - renamed parity_set to has_correct_parity and reorder some logic
> >
> >
> > Roman Bacik (1):
> > usb: dwc2: gadget: parity fix in isochronous mode
> >
> > drivers/usb/dwc2/core.h | 1 +
> > drivers/usb/dwc2/gadget.c | 58
> ++-
> > drivers/usb/dwc2/hw.h | 1 +
> > 3 files changed, 54 insertions(+), 6 deletions(-)
> >
>
> This seems to break slave mode on my platform. It seems to be dropping a
> lot of packets. I tried bInterval=4,5,6, ISO OUT.
> I'll need to take a closer look to determine why. Probably later this week.
>
> Are you able to run in slave mode on your platform? If so can you try it out?
>
> Regards,
> John
Our current test procedure is as follows:
Build Linux kernel with:
Device Drivers
- Sound Card Support 'SOUND=y': ALSA 'SND=y'
- Generic sound devices: Dummy (/dev/null) soundcard 'SND_DUMMY=y'
- USB support 'USB_SUPPORT=y':
DesignWare USB2 DRD Core support 'USB_DWC2=y'
- Gadget only mode 'USB_DWC2_PERIPHERAL=y'
DWC2 Platform 'USB_DWC2_PLATFORM=y'
- USB Gadget Support 'USB_GADGET=y': Audio Gadget 'USB_AUDIO=y'
- PHY Subsystem: Broadcom Kona USB2 PHY Driver 'BCM_KONA_USB2_PHY=y'
If you have only even hosts, you can change the default micro frame parity to
odd:
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index ad45b0b..80bde75 100644
@@ -2709,7 +2709,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_ISOC:
epctrl |= DXEPCTL_EPTYPE_ISO;
-epctrl |= DXEPCTL_SETEVENFR;
+epctrl |= DXEPCTL_SETODDFR;
hs_ep->isochronous = 1;
if (dir_in)
hs_ep->periodic = 1;
@@ -2777,7 +2777,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
/* for non control endpoints, set PID to D0 */
if (index)
-epctrl |= DXEPCTL_SETD0PID;
+epctrl |= DXEPCTL_SETD1PID;
dev_dbg(hsotg->dev, "%s: write DxEPCTL=0x%08x\n",
__func__, epctrl);
To test OUT direction:
Host:
aplay -D plughw:2 -r 48000 -f S16_LE tone_48stereo.wav
Device:
arecord -D plughw:0 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm
To test IN direction:
Host:
arecord -D plughw:2 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm
Device:
aplay -D plughw:0 -r 48000 -f S16_LE /tmp/rec.pcm
If you would like, we can try to test your procedure provided you send us
enough details.
Regards,
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: September-10-15 12:08 PM > To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux- > u...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list > Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode > > On 9/10/2015 10:56 AM, Roman Bacik wrote: > >> -----Original Message- > >> From: Roman Bacik > >> Sent: September-09-15 7:59 PM > >> To: 'John Youn'; Scott Branden; 'Greg Kroah-Hartman'; 'linux- > >> u...@vger.kernel.org' > >> Cc: 'linux-ker...@vger.kernel.org'; bcm-kernel-feedback-list > >> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in > >> isochronous mode > >> > >>> -Original Message- > >>> From: Roman Bacik > >>> Sent: September-09-15 7:36 PM > >>> To: 'John Youn'; Scott Branden; Greg Kroah-Hartman; linux- > >>> u...@vger.kernel.org > >>> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list > >>> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in > >>> isochronous mode > >>> > >>>> -Original Message- > >>>> From: John Youn [mailto:john.y...@synopsys.com] > >>>> Sent: September-09-15 7:25 PM > >>>> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; > >>>> linux- u...@vger.kernel.org > >>>> Cc: linux-ker...@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.y...@synopsys.com] > >>>>>> Sent: September-09-15 7:11 PM > >>>>>> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; > >>>>>> linux- u...@vger.kernel.org > >>>>>> Cc: linux-ker...@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.y...@synopsys.com] > >>>>>>>> Sent: September-03-15 11:53 PM > >>>>>>>> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux- > >>>>>>>> u...@vger.kernel.org; Roman Bacik > >>>>>>>> Cc: linux-ker...@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 <rba...@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 <rba...@broadcom.com> > >>>>>>>>> Reviewed-by: Abhinav Ratna <ara...@broadcom.com> > >>>>>>>>> Reviewed-by: Srinath Mannam > >> <srinath.man...@broadcom.com> > >>>>>>>>> Signed-off-by: Scott Branden <sbran...@broadcom.com> > >>>>>>>>> --- > >>>>>>>>> drivers/usb/dwc2/core.h | 1 + > >>>>>>>>> drivers/usb/dwc2/gadget.c | 51 > >>>>>>>> ++- > >>>>>>>&g
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message-
> From: Roman Bacik
> Sent: September-09-15 7:59 PM
> To: 'John Youn'; Scott Branden; 'Greg Kroah-Hartman'; 'linux-
> u...@vger.kernel.org'
> Cc: 'linux-ker...@vger.kernel.org'; bcm-kernel-feedback-list
> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>
> > -Original Message-
> > From: Roman Bacik
> > Sent: September-09-15 7:36 PM
> > To: 'John Youn'; Scott Branden; Greg Kroah-Hartman; linux-
> > u...@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> > Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> > isochronous mode
> >
> > > -Original Message-
> > > From: John Youn [mailto:john.y...@synopsys.com]
> > > Sent: September-09-15 7:25 PM
> > > To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
> > > linux- u...@vger.kernel.org
> > > Cc: linux-ker...@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.y...@synopsys.com]
> > > >> Sent: September-09-15 7:11 PM
> > > >> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
> > > >> linux- u...@vger.kernel.org
> > > >> Cc: linux-ker...@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.y...@synopsys.com]
> > > >>>> Sent: September-03-15 11:53 PM
> > > >>>> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> > > >>>> u...@vger.kernel.org; Roman Bacik
> > > >>>> Cc: linux-ker...@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 <rba...@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 <rba...@broadcom.com>
> > > >>>>> Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> > > >>>>> Reviewed-by: Srinath Mannam
> <srinath.man...@broadcom.com>
> > > >>>>> Signed-off-by: Scott Branden <sbran...@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 intperiodic:1;
> > > >>>>> unsigned intisochronous:1;
> > > >>>>> unsigned intsend_zlp:1;
> > > >>>>> + unsigned inthas_correct_parity:1;
> > > >>>>>
&g
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-09-15 7:11 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org
> Cc: linux-ker...@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.y...@synopsys.com]
> >> Sent: September-03-15 11:53 PM
> >> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> >> u...@vger.kernel.org; Roman Bacik
> >> Cc: linux-ker...@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 <rba...@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 <rba...@broadcom.com>
> >>> Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> >>> Reviewed-by: Srinath Mannam <srinath.man...@broadcom.com>
> >>> Signed-off-by: Scott Branden <sbran...@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 intperiodic:1;
> >>> unsigned intisochronous:1;
> >>> unsigned intsend_zlp:1;
> >>> + unsigned inthas_correct_parity:1;
> >>>
> >>> charname[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
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-09-15 7:25 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org
> Cc: linux-ker...@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.y...@synopsys.com]
> >> Sent: September-09-15 7:11 PM
> >> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> >> u...@vger.kernel.org
> >> Cc: linux-ker...@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.y...@synopsys.com]
> >>>> Sent: September-03-15 11:53 PM
> >>>> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> >>>> u...@vger.kernel.org; Roman Bacik
> >>>> Cc: linux-ker...@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 <rba...@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 <rba...@broadcom.com>
> >>>>> Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> >>>>> Reviewed-by: Srinath Mannam <srinath.man...@broadcom.com>
> >>>>> Signed-off-by: Scott Branden <sbran...@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 intperiodic:1;
> >>>>> unsigned intisochronous:1;
> >>>>> unsigned intsend_zlp:1;
> >>>>> + unsigned inthas_correct_parity:1;
> >>>>>
> >>>>> charname[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 |
> >>>>>
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message-
> From: Roman Bacik
> Sent: September-09-15 7:36 PM
> To: 'John Youn'; Scott Branden; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>
> > -Original Message-
> > From: John Youn [mailto:john.y...@synopsys.com]
> > Sent: September-09-15 7:25 PM
> > To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> > u...@vger.kernel.org
> > Cc: linux-ker...@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.y...@synopsys.com]
> > >> Sent: September-09-15 7:11 PM
> > >> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
> > >> linux- u...@vger.kernel.org
> > >> Cc: linux-ker...@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.y...@synopsys.com]
> > >>>> Sent: September-03-15 11:53 PM
> > >>>> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> > >>>> u...@vger.kernel.org; Roman Bacik
> > >>>> Cc: linux-ker...@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 <rba...@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 <rba...@broadcom.com>
> > >>>>> Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> > >>>>> Reviewed-by: Srinath Mannam <srinath.man...@broadcom.com>
> > >>>>> Signed-off-by: Scott Branden <sbran...@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 intperiodic:1;
> > >>>>> unsigned intisochronous:1;
> > >>>>> unsigned intsend_zlp:1;
> > >>>>> + unsigned inthas_correct_parity:1;
> > >>>>>
> > >>>>> charname[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 (i
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message-
> From: Roman Bacik
> Sent: September-09-15 7:17 PM
> To: 'John Youn'; Scott Branden; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>
> > -Original Message-
> > From: John Youn [mailto:john.y...@synopsys.com]
> > Sent: September-09-15 7:11 PM
> > To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> > u...@vger.kernel.org
> > Cc: linux-ker...@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.y...@synopsys.com]
> > >> Sent: September-03-15 11:53 PM
> > >> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> > >> u...@vger.kernel.org; Roman Bacik
> > >> Cc: linux-ker...@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 <rba...@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 <rba...@broadcom.com>
> > >>> Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> > >>> Reviewed-by: Srinath Mannam <srinath.man...@broadcom.com>
> > >>> Signed-off-by: Scott Branden <sbran...@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 intperiodic:1;
> > >>> unsigned intisochronous:1;
> > >>> unsigned intsend_zlp:1;
> > >>> + unsigned inthas_correct_parity:1;
> > >>>
> > >>> charname[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);
> &g
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-03-15 11:53 PM
> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org; Roman Bacik
> Cc: linux-ker...@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 <rba...@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 <rba...@broadcom.com>
> > Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> > Reviewed-by: Srinath Mannam <srinath.man...@broadcom.com>
> > Signed-off-by: Scott Branden <sbran...@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 intperiodic:1;
> > unsigned intisochronous:1;
> > unsigned intsend_zlp:1;
> > + unsigned inthas_correct_parity:1;
> >
> > charname[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.
> > + 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);
>
> This toggling code could be moved out to a helper function to reduce
> redundancy as it is in three places now.
>
> Regards,
> John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
Is there anything else we should address in this patch?
Thanks,
Roman
> -Original Message-
> From: Scott Branden [mailto:sbran...@broadcom.com]
> Sent: August-31-15 9:17 AM
> To: John Youn; Greg Kroah-Hartman; linux-usb@vger.kernel.org; Roman
> Bacik
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list; Scott Branden
> Subject: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>
> From: Roman Bacik <rba...@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 <rba...@broadcom.com>
> Reviewed-by: Abhinav Ratna <ara...@broadcom.com>
> Reviewed-by: Srinath Mannam <srinath.man...@broadcom.com>
> Signed-off-by: Scott Branden <sbran...@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 intperiodic:1;
> unsigned intisochronous:1;
> unsigned intsend_zlp:1;
> + unsigned inthas_correct_parity:1;
>
> charname[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++) {
> + 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
> +
RE: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
-Original Message-
From: John Youn [mailto:john.y...@synopsys.com]
Sent: August-25-15 7:06 PM
To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
u...@vger.kernel.org
Cc: linux-ker...@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.y...@synopsys.com]
Sent: August-25-15 2:52 PM
To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
u...@vger.kernel.org
Cc: linux-ker...@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 rba...@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 rba...@broadcom.com
Reviewed-by: Abhinav Ratna ara...@broadcom.com
Reviewed-by: Srinath Mannam srinath.man...@broadcom.com
Reviewed-by: Scott Branden sbran...@broadcom.com
Signed-off-by: Scott Branden sbran...@broadcom.com
---
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 intperiodic:1;
unsigned intisochronous:1;
unsigned intsend_zlp:1;
+ unsigned intparity_set:1;
charname[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
RE: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
-Original Message-
From: John Youn [mailto:john.y...@synopsys.com]
Sent: August-25-15 2:52 PM
To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
u...@vger.kernel.org
Cc: linux-ker...@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 rba...@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 rba...@broadcom.com
Reviewed-by: Abhinav Ratna ara...@broadcom.com
Reviewed-by: Srinath Mannam srinath.man...@broadcom.com
Reviewed-by: Scott Branden sbran...@broadcom.com
Signed-off-by: Scott Branden sbran...@broadcom.com
---
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 intperiodic:1;
unsigned intisochronous:1;
unsigned intsend_zlp:1;
+ unsigned intparity_set:1;
charname[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
RE: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
-Original Message-
From: Felipe Balbi [mailto:ba...@ti.com]
Sent: August-25-15 3:36 PM
To: Roman Bacik
Cc: John Youn; Scott Branden; Greg Kroah-Hartman; linux-
u...@vger.kernel.org; linux-ker...@vger.kernel.org; bcm-kernel-feedback-
list
Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
On Tue, Aug 25, 2015 at 10:00:17PM +, Roman Bacik wrote:
-Original Message-
From: John Youn [mailto:john.y...@synopsys.com]
Sent: August-25-15 2:52 PM
To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
u...@vger.kernel.org
Cc: linux-ker...@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 rba...@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 rba...@broadcom.com
Reviewed-by: Abhinav Ratna ara...@broadcom.com
Reviewed-by: Srinath Mannam srinath.man...@broadcom.com
Reviewed-by: Scott Branden sbran...@broadcom.com
Signed-off-by: Scott Branden sbran...@broadcom.com
---
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 intperiodic:1;
unsigned intisochronous:1;
unsigned intsend_zlp:1;
+ unsigned intparity_set:1;
charname[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;
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 = !!hs_ep0isochronous;
It can be simplified to:
hs_ep-has_correct_parity = 1;
I just thought that the original shows better what we are trying to do. I do
not mind to simplify it and remove the condition.
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,
why the two extra bits ? What are they doing ?
This fix uses Incomplete Periodic Transfer interrupt (GINTSTS_INCOMPL) to
toggle between even and odd parity until the Transfer Complete interrupt is
received. We also need to set correct parity on both IN and OUT endpoints.
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++) {
u32 epctl_reg;
u32 ctrl;
+ hs_ep = hsotg-eps_in[idx];
you can decrease some indentation here:
if (!hs_ep-isochronous)
continue;
if (hs_ep-has_correct_parity)
continue;
epctl_reg = DIEPCTL(idx);
ctrl = readl(hsotg-regs + epctl_reg);
if (ctrl DXEPCTL_EOFRNUM)
ctrl |= DXEPCTL_SETEVENFR
