linux-kernel - Re: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2B3535C5ECE8B5419E3ECBE30077290901DC37C268@US01WEMBX2.internal.synopsys.com>
Date:	Fri, 25 Sep 2015 03:15:59 +0000
From:	John Youn <John.Youn@...opsys.com>
To:	Roman Bacik <rbacik@...adcom.com>,
	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

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


--
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/