| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <D2C8DF24C22DB24BA948A8B0D097284002F2BBDE@SJEXCHMB05.corp.ad.broadcom.com>
Date: Mon, 28 Sep 2015 15:27:50 +0000
From: Roman Bacik <rbacik@...adcom.com>
To: John Youn <John.Youn@...opsys.com>,
Scott Branden <sbranden@...adcom.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>
Subject: RE: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
> -----Original Message-----
> From: John Youn [mailto:John.Youn@...opsys.com]
> Sent: September-24-15 8:16 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> usb@...r.kernel.org
> Cc: linux-kernel@...r.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.Youn@...opsys.com]
> >> Sent: September-23-15 9:21 PM
> >> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> >> usb@...r.kernel.org; Roman Bacik
> >> Cc: linux-kernel@...r.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-mode, the xfercomplete interrupt is not used for OUT transfers, so
> the has_correct_parity flag is never set.
>
> The other issue is that we occasionally get the incomplete interrupt twice in a
> microframe causing the parity to be set incorrectly for the next frame. Thus
> we will continuously miss until this occurs again, correcting it.
>
> These two problems in combination cause dropped packets throughout
> operation.
>
> If you give me some time I can see if I can fix up this patch to work with slave
> mode.
>
> I've tried all our hosts here and they are all even hosts, so I tried flipping the
> default parity. The issue exists in either case.
>
> Regards,
> John
>
Is there a way to recognize we are in slave-mode that we would not flip parity in that case?
Thanks,
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists