Re: USB Gadget driver for USB screen?

[Clemens Ladisch]

Sebastian Nielsen wrote:
> Isn't the "Standard VGA card" appearing in device manager a standarized
> device class?

It was an ISA device.  The PCI and PCI Express buses go to great lengths
to be ISA compatible for VGA cards.

USB is a different bus, and cannot be made compatible.


Regards,
Clemens
--
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: USB Gadget driver for USB screen?

[Clemens Ladisch]

Sebastian Nielsen wrote:
> Does anyone know of a USB Gadget kernel driver for a USB screen?

There is no standardized device class for USB-to-VGA devices.

> So I can have a linux USB gadget emulate as a USB screen to the host PC, and
> aquire it's picture as a picture stream?

You could try reverse engineering the protocol used by the sisusbvga
or udl drivers, and implementing that.


Regards,
Clemens
--
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: usb: usbtmc: Questions of the IVI Foundation

[Clemens Ladisch]

guido.kie...@rohde-schwarz.com wrote:
> linux-usb-ow...@vger.kernel.org schrieb am 15.05.2017 15:20:22:
>> Von: Greg KH 
>> On Mon, May 15, 2017 at 02:47:48PM +0200, Guido.Kiener@x wrote:
>>> 2. As we have looked at the Linux driver, we’ve noticed that performance
>>> of the usbtmc_read() function doesn’t keep up with our fastest
>>> instruments. Do you have any suggestions on how to improve the read
>>> performance?
>>
>> Where exactly are things not going fast enough?  Have you found any
>> specific bottlenecks?  How fast are you needing to go?  What type of
>> interface do you expect userspace to have to handle high rates of data?
>
> I'm not sure for 100%, but I assume that reading the IN pipe could be
> setup asynchronously (e.g. with usb_submit_urb(..)) just before sending
> send_request_dev_dep_msg_in(..).
> USBTMC_SIZE_IOBUFFER = 2kB is a bottleneck when dealing with data size >
> 1MB (Need of Scatter/Gather).

The major problem with usbtmc_read() is that it sends a new TMC read
request for every 2033-byte buffer it wants to read.

As a first step, try enabling the "rigol" quirk, which simply uses
a single read request for the entire buffer.  (I don't know why the
driver author thought this shouldn't be done for every device.)


Regards,
Clemens
--
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] usb: core: Warn if an URB's transfer_buffer is on stack

[Clemens Ladisch]

Florian Fainelli wrote:
> We see a large number of fixes to several drivers to remove the usage of
> on-stack buffers feeding into USB transfer functions. Make it easier to spot
> the offenders by adding a warning in usb_hcd_map_urb_for_dma() checking that
> urb->transfer_buffer is not a stack object.

This description is incomplete.

> + } else if (object_is_on_stack(urb->transfer_buffer)) {
> + WARN_ONCE(1, "transfer buffer is on stack\n");
> + ret = -EAGAIN;
>   } else {
>   urb->transfer_dma = dma_map_single(

Not only is there a warning, but the check also forces all those URBs
to abort with an error.

Well, that makes it even easier to spot the offenders ...


Regards,
Clemens
--
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: [alsa-devel] Jack sensing in snd_usb_audio ?

[Clemens Ladisch]

Bastien Nocera wrote:
> On Wed, 2016-10-12 at 14:43 +0200, Clemens Ladisch wrote:
>> Bastien Nocera wrote:
>>> "
>>> A change of state in the audio function is most often caused by a
>>> certain event that takes place. An event can either be user-
>>> initiated
>>> or device-initiated. User-initiated jack insertion or removal is a
>>> typical example of a user-initiated event.
>>> "
>>
>> There are not many USB Audio 2.0 devices, and I'm not aware of any
>> that actually implements this.
>
> I guess I would see whether there are events if I captured the USB
> traffic even without special handling/turning on a feature in the
> drivers, right?

Most devices do not even have the status endpoint (see "lsusb -v").
To check what events arrive, you can add logging to the
snd_usb_mixer_interrupt() function.


Regards,
Clemens
--
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: [alsa-devel] Jack sensing in snd_usb_audio ?

[Clemens Ladisch]

Bastien Nocera wrote:
> Looks like whether or not jack sensing works depends on the device
> itself, but there is a mechanism to propagate the change in setup in
> the USB Audio 2.0 spec

Some recent Windows 10 beta added partial support for USB Audio 2.0.
Earlier Windowses implement only USB Audio 1.0, which does not mention
jacks.

> "
> A change of state in the audio function is most often caused by a
> certain event that takes place. An event can either be user-initiated
> or device-initiated. User-initiated jack insertion or removal is a
> typical example of a user-initiated event.
> "

There are not many USB Audio 2.0 devices, and I'm not aware of any
that actually implements this.


Regards,
Clemens
--
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 0/3] USB Audio Gadget refactoring

[Clemens Ladisch]

Peter Chen wrote:
> On Tue, Aug 16, 2016 at 11:32:55AM +0200, Clemens Ladisch wrote:
>> Windows does not have UAC2 support.
>
> Thanks, before windows7 or all windows versions have no UAC2 support?

So far, no version has it.


Regards,
Clemens
--
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 0/3] USB Audio Gadget refactoring

[Clemens Ladisch]

Peter Chen wrote:
> I find UAC2 (UAC1 is ok)  support is not well with the latest mainline
> kernel w/o your patch set. The windows7 can't install the driver
> successfully

Windows does not have UAC2 support.

> and the playback shows underrun (using local codec)
> using Linux host.

> # arecord -f dat -t wav -D hw:1,0 | aplay -D hw:0,0 &

The clocks of the two devices are not synchronized.

In the ALSA API, a PCM device is assumed to have its own clock, so it is
not possible to synchronize the USB gadget to the actual sound device
without some separate mechanism (like the old uac1 gadget probably has).


Regards,
Clemens
--
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: [RFC PATCH 0/5] USB Audio Gadget refactoring

[Clemens Ladisch]

Ruslan Bilovol wrote:
> On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clem...@ladisch.de> wrote:
>>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>>> <ruslan.bilo...@gmail.com> wrote:
>>>>> it may break current usecase for some people
>>
>> And what are the benefits that justify breaking the kernel API?
>
> Main limitation with current f_uac1 design is - it can be used only on systems
> with real ALSA card present and can have only exact number of
> channels / sampling rate as sink card has. [...]
> A real cases when it's required to have UAC1 gadget represented as virtual
> sound card on gadget side: [...]

Thanks.

> Of course disadvantage of new approach for UAC1 gadget is you need to
> use some userspace application for routing audio from virtual to real
> sound card, like in case of UAC2 gadget. But thanks to existing
> applications like alsaloop it's not difficult nowadays.

I don't know what the maintainer will say, but you would increase the
chances of this change being accepted when you show a concrete example
of how to change the userspace configuration from the old to the new
driver.  (And add it to the documentation.)


Regards,
Clemens
--
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: [RFC PATCH 0/5] USB Audio Gadget refactoring

[Clemens Ladisch]

>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>  wrote:
>>> it may break current usecase for some people

And what are the benefits that justify breaking the kernel API?


Regards,
Clemens
--
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] hwrng: alea - Add support for Araneus Alea I USB RNG

[Clemens Ladisch]

Bob Ham wrote:
> Adds support for the Araneus Alea I USB hardware Random Number
> Generator.  This RNG creates entropy at a high rate, about 100kb/s.
>
> Signed-off-by: Bob Ham 
> ---
>
> +++ b/drivers/char/hw_random/alea.c

Why didn't you just add the device ID to chaoskey.c?
(Because that one is hidden in drivers/usb/misc? ;-)


Regards,
Clemens
--
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] usb: core: Do not use sizeof on pointer type

[Clemens Ladisch]

Vaishali Thakkar wrote:
> When sizeof is applied to a pointer typed expression, it gives
> the size of the pointer.

And why would that be wrong in this case?

> +++ b/drivers/usb/core/hcd.c
> @@ -1386,7 +1386,7 @@ static int hcd_alloc_coherent(struct usb_bus *bus,
>   return -EFAULT;
>   }
>
> - vaddr = hcd_buffer_alloc(bus, size + sizeof(vaddr),
> + vaddr = hcd_buffer_alloc(bus, size + sizeof(*vaddr),
>mem_flags, dma_handle);
>   if (!vaddr)
>   return -ENOMEM;
>

Please note the following comment:

/*
 * Store the virtual address of the buffer at the end
 * of the allocated dma buffer. [...]


Regards,
Clemens
--
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: Tascam US-122L - Corrupt USB descriptor

[Clemens Ladisch]

Simon Wood wrote:
> On Thu, April 14, 2016 1:35 am, Clemens Ladisch wrote:
>> Simon Wood wrote:
>>> The card shows up under '/proc/asound/cards', but only as Midi.
>>
>> Apparently, there is no PCM device. This driver creates a special
>> hardware-dependent device which is intended to be used with the Jack driver
>> "usb_stream".
>>
>> It might be possible to use the ALSA "usb_stream" plugin:
>> <http://www.alsa-project.org/main/index.php/Matrix:Module-usb-us122l>
>
> I have this '.asoundrc' file (made before previous postings), but that
> does not seem to make a usable card.

What error message do you get when you try to use aplay with that device?

> I'm expecting that a PCM device should /just/ appear under 'aplay -l'

The driver does not implement such a PCM device.


Regards,
Clemens
--
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: Tascam US-122L - Corrupt USB descriptor

[Clemens Ladisch]

Simon Wood wrote:
> The card shows up under '/proc/asound/cards', but only as Midi.

Apparently, there is no PCM device. This driver creates a special
hardware-dependent device which is intended to be used with the
Jack driver "usb_stream".

It might be possible to use the ALSA "usb_stream" plugin:



Regards,
Clemens
--
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: Tascam US-122L - Corrupt USB descriptor

[Clemens Ladisch]

Simon Wood wrote:
> I have been struggling for the past few days to get a Tascam US-122L (USB
> sound-card/midi interface) working, despite reading numerous forum
> postings I have only been able to get the midi portion working.

Does it show up with "aplay -l"?

> I note that the USB descriptor seems to be corrupt. It declares 2
> interfaces, but then describes 3 separate with the same interface number
> for the last 2.

The descriptors describe two alternate settings for the same interface.
This is a feature, which is required for audio devices.

>   iManufacturer   1 (error)   <-- Here be 
> Errors!
>   iProduct2 (error)
>   iSerial 3 (error)

It's possible that lsusb does not have the permissions needed to send
the messages to ask for the strings.  Try running it as root.


Regards,
Clemens
--
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 06/14] USB: ch341: reinitialize chip on reconfiguration

[Clemens Ladisch]

Grigori Goronzy wrote:
> Changing the LCR register after initialization does not seem to be
> reliable on all chips (particularly not on CH341A).  Restructure
> initialization and configuration to always reinit the chip on
> configuration changes instead and pass the LCR register value directly
> to the initialization command.

> +++ b/drivers/usb/serial/ch341.c
> @@ -155,9 +157,7 @@ static int ch341_set_baudrate(struct usb_device *dev,
>   a = (factor & 0xff00) | divisor;
>   b = factor & 0xff;
>
> - r = ch341_control_out(dev, 0x9a, 0x1312, a);
> - if (!r)
> - r = ch341_control_out(dev, 0x9a, 0x0f2c, b);
> + r = ch341_control_out(dev, CH341_SERIAL_INIT, 0x9c | (ctrl << 8) , a | 
> 0x80);

The variable b is no longer used, so it is no longer necessary to
compute the lower eight bits of the factor.


Regards,
Clemens
--
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] usb: gadget: f_midi: Fixed a bug when buflen was smaller than wMaxPacketSize

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On 11/03/16 23:07, Michal Nazarewicz wrote:
>> I’m also wondering whether it would be beneficial to get rid of buflen
>> all together and use wMaxPacketSie for in endpoints as well?  Is that
>> feasible?
>
> Yes, we could just remove the buflen parameter.
>
> The only scenario where I can see buflen been "useful" is if the user
> wants to have a smaller buffer size for the OUT endpoint. Should we
> support this case or not?

Splitting data into multiple packets would not make sense from
a performance perspective.  The only possible reason would be to work
around a (theoretical) bug in some host software that does not handle
larger buffers, but there aren't that many host implementations, and I
am not aware of any with such a bug.


Regards,
Clemens
--
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] usb: gadget: f_midi: Fixed a bug when buflen was smaller than wMaxPacketSize

[Clemens Ladisch]

Steve Calfee wrote:
> On Wed, Mar 9, 2016 at 11:39 AM, Felipe F. Tonello  
> wrote:
>> [...]
>> This patch fixes this problem by setting the minimum usb_request's buffer 
>> size
>> for the OUT endpoint as its wMaxPacketSize.
>>
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -366,7 +366,9 @@ static int f_midi_set_alt(struct usb_function *f, 
>> unsigned intf, unsigned alt)
>> struct usb_request *req =
>> -   midi_alloc_ep_req(midi->out_ep, midi->buflen);
>> +   midi_alloc_ep_req(midi->out_ep,
>> +   max_t(unsigned, midi->buflen,
>> +   bulk_out_desc.wMaxPacketSize));
>
> Won't you get a buffer overrun if midi->buflen is less than wMaxPacketSize?

No.  f_midi_handle_out_data() uses only the request buffer.


Regards,
Clemens
--
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 1/5] usb: gadget: f_midi: refactor state machine

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On March 4, 2016 8:07:40 AM GMT+00:00, Clemens Ladisch <clem...@ladisch.de> 
> wrote:
>> Felipe Ferreri Tonello wrote:
>>> On 03/03/16 11:38, Clemens Ladisch wrote:
>>>> But in what way was the old state machine not "proper"?
>>>
>>> Because it didn't reflect all the correct and possible MIDI states
>>
>> The whole point of the one-byte real-time messages is that they do not
>> affect the parsing of the surrounding MIDI stream.  So not making them
>> part of the state machine is the proper way of handling them.  (Also
>> see the flowchart in appendix A of the spec.)
>
> I really don't get your point. So why do we have a state machine at all?

To parse all the other messages.


Regards,
Clemens
--
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 1/5] usb: gadget: f_midi: refactor state machine

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On 03/03/16 11:38, Clemens Ladisch wrote:
>> But in what way was the old state machine not "proper"?
>
> Because it didn't reflect all the correct and possible MIDI states

The whole point of the one-byte real-time messages is that they do not
affect the parsing of the surrounding MIDI stream.  So not making them
part of the state machine is the proper way of handling them.  (Also
see the flowchart in appendix A of the spec.)

> This patch doesn't change any functionality. But the important thing
> here is that it improves the driver maintainability [...]

Then I won't get in the way of this driver's maintainer.


Regards,
Clemens
--
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 1/5] usb: gadget: f_midi: refactor state machine

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On 02/03/16 21:09, Clemens Ladisch wrote:
>> Felipe F. Tonello wrote:
>>> This refactor results in a cleaner state machine code
>>
>> It increases the number of states, and now juggles two state variables.
>> I cannot agree to it being cleaner.
>
> Yes, it increases the number of states. That was done in order to
> actually implement a proper finite state machine with one state at a
> time and a transition state.

I know, "clean" is subjective.  But in what way was the old state
machine not "proper"?

And how is handling two states (port->state and next_state) cleaner?
As far as I can tell, the requirement for a separate variable comes not
from any inherent complexity of the state machine itself, but only
because the transmit_packet function was inlined.


Regards,
Clemens
--
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 1/5] usb: gadget: f_midi: refactor state machine

[Clemens Ladisch]

Felipe F. Tonello wrote:
> This refactor results in a cleaner state machine code

It increases the number of states, and now juggles two state variables.
I cannot agree to it being cleaner.

> and as a result fixed a bug when packaging a USB-MIDI packet right after
> a non-conformant MIDI byte stream.

I have been unable to determine where exactly the new code behaves
differently.  Can you show an example?


Regards,
Clemens
--
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: USB: gadget: add USB Audio Gadget driver

[Clemens Ladisch]

Alan Stern wrote:
> On Sun, 31 Jan 2016, Clemens Ladisch wrote:
>> Alan Stern wrote:
>>> On Fri, 29 Jan 2016, Dan Carpenter wrote:
>>>> The patch c6994e6f067c: "USB: gadget: add USB Audio Gadget driver"
>>>> from Jun 3, 2009, leads to the following static checker warning:
>>>>
>>>>drivers/usb/gadget/function/f_uac1.c:371 f_audio_complete()
>>>>error: __memcpy() '' too small (4 vs u32max)
>>>>
>>>> drivers/usb/gadget/function/f_uac1.c
>>>>370  else if (audio->set_con) {
>>>>371  memcpy(, req->buf, req->length);
>>>> ^^^
>>>> Back in 2014 Kees ran a USB fuzzer on the kernel.  My take on that was
>>>> that req->length could not be trusted and had to be capped so I changed
>>>> Smatch to complain about these.  But after a while I decided I was not
>>>> sure enough of the rules so I just ignore those bugs these days...  What
>>>> are the rules?
>>>
>>> req->length is set by the driver and not changed anywhere else in the
>>> kernel.  (Or rather, changing it would be a bug.)  Of course, it's
>>> possible that the driver could be fooled into setting req->length to
>>> something larger than 4 -- I haven't looked through the code.
>>
>> That value comes from the control setup packet's wLength field.  Neither
>> audio_set_intf_req() nor f_audio_setup() check it.
>
> Maybe those routines don't check wLength, but it has to get set
> somewhere in the driver.

In a USB gadget driver, these packets are received from the host PC.
Which, for the purposes of this discussion, must be considered evil.


Regards,
Clemens
--
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: USB: gadget: add USB Audio Gadget driver

[Clemens Ladisch]

Alan Stern wrote:
> On Fri, 29 Jan 2016, Dan Carpenter wrote:
>> The patch c6994e6f067c: "USB: gadget: add USB Audio Gadget driver"
>> from Jun 3, 2009, leads to the following static checker warning:
>>
>>  drivers/usb/gadget/function/f_uac1.c:371 f_audio_complete()
>>  error: __memcpy() '' too small (4 vs u32max)
>>
>> drivers/usb/gadget/function/f_uac1.c
>>370  else if (audio->set_con) {
>>371  memcpy(, req->buf, req->length);
>> ^^^
>> Back in 2014 Kees ran a USB fuzzer on the kernel.  My take on that was
>> that req->length could not be trusted and had to be capped so I changed
>> Smatch to complain about these.  But after a while I decided I was not
>> sure enough of the rules so I just ignore those bugs these days...  What
>> are the rules?
>
> req->length is set by the driver and not changed anywhere else in the
> kernel.  (Or rather, changing it would be a bug.)  Of course, it's
> possible that the driver could be fooled into setting req->length to
> something larger than 4 -- I haven't looked through the code.

That value comes from the control setup packet's wLength field.  Neither
audio_set_intf_req() nor f_audio_setup() check it.


Regards,
Clemens
--
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 v7 0/5] usb: usbtmc: Add support for missing functions in USBTMC-USB488 spec

[Clemens Ladisch]

Dave Penkler wrote:
> Implement support for the USB488 defined READ_STATUS_BYTE ioctl (1/5)
> and SRQ notifications with fasync (2/5) and poll/select (3/5) in order
> to be able to synchronize with variable duration instrument
> operations.
>
> Add convenience ioctl to return all device capabilities (4/5)
>
> Add ioctls for other USB488 requests: REN_CONTROL, GOTO_LOCAL and
> LOCAL_LOCKOUT. (5/5)
> [...]
>  Testing:
> All functions tested ok on an USBTMC-USB488 compliant oscilloscope

How?  Did you extend the usbtmc_ioctl tool?


Regards,
Clemens
--
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 1/2] usb: gadget: f_midi: refactor state machine

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
>> Running status is feature.
>
>What do you mean by that?

That this behavior is intended, and required.

> I don't qualify writing a *wrong* MIDI-USB
>packet because of a previous MIDI message as a feature.

The MIDI Specification qualifies Running Status as a feature.


Regards,
Clemens

--
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 1/2] usb: gadget: f_midi: refactor state machine

[Clemens Ladisch]

Felipe F. Tonello wrote:
> This refactor includes the following:
>  * Cleaner state machine code;

It does not correctly handle system real time messages inserted between
the status and data bytes of other messages.

>  * Reset state if MIDI message parsed is non-conformant;

Why?  In a byte stream like "C1 C3 44", where the data byte of the first
message was lost, the reset would also drop the second message.

>  * Fixed bug when a conformant MIDI message was followed by a non-conformant
>causing the MIDI-USB message to use old temporary data (port->data[0..1]),
>thus packing a wrong MIDI-USB request.

Running status is feature.


Regards,
Clemens
--
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 v5 7/7] usb: gadget: f_midi: pre-allocate IN requests

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> One thing to consider is that the ALSA rawmidi device buffer is
> sequential and our USB request buffer is not. This means that our 32
> (qlen) * 256 (buflen) = 8KB of data is non-linear. Some requests might
> have 3 or 4 bytes (average size of a normal MIDI message) of data and
> some others might contain the full 256 bytes (for SysEx messages).

f_midi_transmit() always fills up the USB packet as much as possible, so
the number of MIDI messages per request will increase automatically when
the ALSA buffer fills up faster that it is emptied by f_midi.

> == Conclusion ==
>
> Based on our conversation and your suggestions, I think that to just
> ignore if an overrun occurs to the USB requests is fine. Upon completion
> the request will be reused.
> Important to note that if the overrun occurs, it will cause user-space
> to block until a) the completion function is called successfully or b)
> snd_rawmidi_write() times out. Which I think this is expected by ALSA users.
>
> Does that make sense?

Yes.


Regards,
Clemens
--
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 v5 7/7] usb: gadget: f_midi: pre-allocate IN requests

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On 13/11/15 08:55, Clemens Ladisch wrote:
>> Felipe F. Tonello wrote:
>>> +static void f_midi_transmit(struct f_midi *midi)
>>> +{
>>> +...
>>> +   len = kfifo_peek(>in_req_fifo, );
>>> +   ...
>>> +   if (req->length > 0) {
>>> +   WARNING(midi, "%s: All USB requests have been used. 
>>> Current queue size "
>>> +   "is %u, consider increasing it.\n", __func__, 
>>> midi->in_req_num);
>>> +   goto drop_out;
>>> +   }
>>
>> There are two cases where the in_req FIFO might overflow:
>> 1) the gadget is trying to send too much data at once; or
>> 2) the host does not bother to read any of the data.
>>
>> In case 1), the appropriate action would be to do nothing, so that the
>> remaining data is sent after some currently queued packets have been
>> transmitted.  In case 2), the appropriate action would be to drop the
>> data (even better, the _oldest_ data), and spamming the log with error
>> messages would not help.
>
> True. In this case the log will be spammed.
>
> How would you suggest to drop the oldest data? That doesn't really seem
> to be feasible.

There is usb_ep_dequeue().  Its documentation warns about some hardware,
but it would be possible to at least try it.

>> I'm not quite sure if trying to detect which of these cases we have is
>> possible, or worthwhile.  Anyway, with a packet size of 64, the queue
>> size would be 32*64 = 2KB, which should be enough for everyone.  So I
>> propose to ignore case 1), and to drop the error message.

After some thought, I'm not so sure anymore -- the ability to buffer
more than 2 KB of data is part of the snd_rawmidi_write() API, so this
could introduce a regression.  And I can imagine cases where one would
actually want to transmit large amounts data.

I think the safest approach would be to behave similar to the old driver,
i.e., when the queue overflows, do nothing (not even dropping data), and
rely on the transmit completion handler to continue.  (This implies that
ALSA's buffer can fill up, and that snd_rawmidi_write() can block.)


It you want to dequeue outdated data, I think this should be done with
a timeout, i.e., when the host did not read anything for some tens of
milliseconds or so.  This would be independent of the fill level of the
queue, and could be done either for individual packets, or just on the
entire endpoint queue.


Regards,
Clemens
--
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 v5 6/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On 10/11/15 18:43, Sergei Shtylyov wrote:
>> On 11/10/2015 08:52 PM, Felipe F. Tonello wrote:
>>> @@ -75,6 +75,7 @@ struct f_midi {
>>>   struct usb_ep*in_ep, *out_ep;
>>>   struct snd_card*card;
>>>   struct snd_rawmidi*rmidi;
>>> +u8ms_id;
>>
>>   Why 'ms_id' is not aligned with the above field names?
>
> It is actually aligned.

It's not in the original mail, which contains tab characters.

> Here is from my local git diff:
>
> @@ -75,6 +75,7 @@ struct f_midi {
> struct usb_ep   *in_ep, *out_ep;
> struct snd_card *card;
> struct snd_rawmidi  *rmidi;
> +   u8  ms_id;

Apparently, you're using four spaces per tab.  Linux uses eight.


Regards,
Clemens
--
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 v5 6/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

[Clemens Ladisch]

Felipe F. Tonello wrote:
> This avoids duplication of USB requests for OUT endpoint and
> re-enabling endpoints.

>  ...
>   /* For Control Device interface we do nothing */
> - if (intf == 0)
> + if (intf != midi->ms_id)
>   return 0;

The comment now is misleading.


Regards,
Clemens
--
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 v5 7/7] usb: gadget: f_midi: pre-allocate IN requests

[Clemens Ladisch]

Felipe F. Tonello wrote:
> This patch introduces pre-allocation of IN endpoint USB requests. This
> improves on latency (requires no usb request allocation on transmit) and avoid
> several potential probles on allocating too many usb requests (which involves
> DMA pool allocation problems).
>
> This implementation also handles better multiple MIDI Gadget ports, always
> processing the last processed MIDI substream if the last USB request wasn't
> enought to handle the whole stream.

> ...
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -88,6 +89,9 @@ struct f_midi {
>   int index;
>   char *id;
>   unsigned int buflen, qlen;
> + DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
> + unsigned int in_req_num;
> + unsigned int in_last_port;

As far as I can see, in_req_num must always have the same value as qlen.

> @@ -366,6 +388,20 @@ static void f_midi_disable(struct usb_function *f)
> + while (!kfifo_is_empty(>in_req_fifo)) {
> + ...
> + len = kfifo_get(>in_req_fifo, );
> + if (len == 1)
> + free_ep_req(midi->in_ep, req);
> + else
> + ERROR(midi, "%s couldn't free usb request: something 
> went wrong with kfifo\n",
> +   midi->in_ep->name);
> + }

kfifo_get() already checks for the FIFO being empty, so you can just
drop kfifi_is_empty().

> @@ -487,57 +523,111 @@ static void f_midi_transmit_byte(struct usb_request 
> *req,
> ...
> +static void f_midi_transmit(struct f_midi *midi)
> +{
> +...
> + len = kfifo_peek(>in_req_fifo, );
> + ...
> + if (req->length > 0) {
> + WARNING(midi, "%s: All USB requests have been used. 
> Current queue size "
> + "is %u, consider increasing it.\n", __func__, 
> midi->in_req_num);
> + goto drop_out;
> + }

There are two cases where the in_req FIFO might overflow:
1) the gadget is trying to send too much data at once; or
2) the host does not bother to read any of the data.

In case 1), the appropriate action would be to do nothing, so that the
remaining data is sent after some currently queued packets have been
transmitted.  In case 2), the appropriate action would be to drop the
data (even better, the _oldest_ data), and spamming the log with error
messages would not help.

This code shows the error message for case 1), but does the action for
case 2).

I'm not quite sure if trying to detect which of these cases we have is
possible, or worthwhile.  Anyway, with a packet size of 64, the queue
size would be 32*64 = 2KB, which should be enough for everyone.  So I
propose to ignore case 1), and to drop the error message.

> @@ -1130,6 +1222,12 @@ static struct usb_function *f_midi_alloc(struct 
> usb_function_instance *fi)
> + if (kfifo_alloc(>in_req_fifo, midi->qlen, GFP_KERNEL))
> + goto setup_fail;

The setup_fail code expects an error code in the status variable.


Regards,
Clemens
--
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 v3 1/4] usb: gadget: f_midi: free usb request when done

[Clemens Ladisch]

Felipe Tonello wrote:
> On Fri, Oct 9, 2015 at 10:23 AM, Clemens Ladisch <clem...@ladisch.de> wrote:
>> Felipe Tonello wrote:
>>>   } else if (ep == midi->in_ep) {
>>> - /* Our transmit completed. See if there's more to go.
>>> -  * f_midi_transmit eats req, don't queue it again. */
>>> - f_midi_transmit(midi, req);
>>> + /* Our transmit completed. Don't queue it again. */
>>> + free_ep_req(ep, req);
>>>   return;
>>>   }
>>>   break;
>>
>> The ALSA framework will give you a notification _once_.  If the
>> resulting data is larger than midi->buflen, then you have to continue
>> sending packets.  This is exactly what the call to f_midi_transmit()
>> does.
>
> That's true. But what it will also happen is that f_midi_transmit()
> will potentially eat up data from an unrelated ALSA trigger.

The triggers are for some MIDI port of the _same_ USB endpoint, so
they're not unrelated.  f_midi_transmit() collects data from all ports
anyway; separating the data from different ports into different USB
packets would just needlessly introduce additional latency.

> In the end it is all fine, because the function will realise the
> request->len == 0 so it will free the request. But logically speaking
> it is incorrect and error prone.

It is _not_ incorrect if you realize that f_midi_transmit() applies to
the endpoint, not to any particular port.

>> (To decrease latency, it might be a good idea to queue multiple requests,
>> but you wouldn't want to continue piling up requests if the host isn't
>> listening.  sound/usb/midi.c just allocates a fixed number of requests,
>> and always reuses them.)
>
> I believe that is the best way to implement. Create multiple requests
> until the ALSA substreams buffer are empty and free the request on
> completion.

I believe a better way to implement this is to allocate a fixed number
of requests, and to always reuse them.

> The problem of having requests when host isn't listening will happen
> anyway because there is no way to know that until completion.

But if you have no upper limit on the number of queues requests, you
will eventually run out of (DMA) memory.


Regards,
Clemens
--
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 v3 1/4] usb: gadget: f_midi: free usb request when done

[Clemens Ladisch]

Felipe Tonello wrote:
> On Mon, Oct 12, 2015 at 11:16 AM, Clemens Ladisch <clem...@ladisch.de> wrote:
>> Felipe Tonello wrote:
>>> I believe that is the best way to implement. Create multiple requests
>>> until the ALSA substreams buffer are empty and free the request on
>>> completion.
>>
>> I believe a better way to implement this is to allocate a fixed number
>> of requests, and to always reuse them.
>
> How many?

Enough to get proper pipelining.  At least two, maybe not more.
(Depends on how fast those tiny CPUs can queue the next request.)

>>> The problem of having requests when host isn't listening will happen
>>> anyway because there is no way to know that until completion.
>>
>> But if you have no upper limit on the number of queues requests, you
>> will eventually run out of (DMA) memory.
>
> And that's what happening at the moment. One of my patches are to fix
> a memory leak when that happens.
>
> But it would be ideal to have a FIFO of requests and perhaps ignore
> new requests if the FIFO is full.
>
> So, allocate (pre-allocate?) new requests until the FIFO is full. Upon
> completion, remove the request from FIFO, but still reuse it on
> f_midi_transmit() and queue it on the FIFO again if there is still
> data from ALSA, otherwise just free the request.

Yes, that's exactly what I'm proposing.


Regards,
Clemens
--
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 v3 1/4] usb: gadget: f_midi: free usb request when done

[Clemens Ladisch]

Felipe Balbi wrote:
> Clemens Ladisch <clem...@ladisch.de> writes:
>> Felipe Tonello wrote:
>>> req->actual == req->length means that there is no data left to enqueue,
>>
>> This condition is not checked in the patch.
>>
>>> so free the request.
>>>
>>> Signed-off-by: Felipe F. Tonello <e...@felipetonello.com>
>>> ---
>>>  drivers/usb/gadget/function/f_midi.c | 5 ++---
>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>>> b/drivers/usb/gadget/function/f_midi.c
>>> index edb84ca..93212ca 100644
>>> --- a/drivers/usb/gadget/function/f_midi.c
>>> +++ b/drivers/usb/gadget/function/f_midi.c
>>> @@ -256,9 +256,8 @@ f_midi_complete(struct usb_ep *ep, struct usb_request 
>>> *req)
>>> /* We received stuff. req is queued again, below */
>>> f_midi_handle_out_data(ep, req);
>>> } else if (ep == midi->in_ep) {
>>> -   /* Our transmit completed. See if there's more to go.
>>> -* f_midi_transmit eats req, don't queue it again. */
>>> -   f_midi_transmit(midi, req);
>>> +   /* Our transmit completed. Don't queue it again. */
>>> +   free_ep_req(ep, req);
>>> return;
>>> }
>>> break;
>>
>> The ALSA framework will give you a notification _once_.  If the
>> resulting data is larger than midi->buflen, then you have to continue
>> sending packets.  This is exactly what the call to f_midi_transmit()
>> does.
>
> so, should I drop this series from my TODO queue ?

Yes, this patch needs to be dropped.


Regards,
Clemens
--
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 v3 1/4] usb: gadget: f_midi: free usb request when done

[Clemens Ladisch]

Felipe Tonello wrote:
> req->actual == req->length means that there is no data left to enqueue,

This condition is not checked in the patch.

> so free the request.
>
> Signed-off-by: Felipe F. Tonello 
> ---
>  drivers/usb/gadget/function/f_midi.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index edb84ca..93212ca 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -256,9 +256,8 @@ f_midi_complete(struct usb_ep *ep, struct usb_request 
> *req)
>   /* We received stuff. req is queued again, below */
>   f_midi_handle_out_data(ep, req);
>   } else if (ep == midi->in_ep) {
> - /* Our transmit completed. See if there's more to go.
> -  * f_midi_transmit eats req, don't queue it again. */
> - f_midi_transmit(midi, req);
> + /* Our transmit completed. Don't queue it again. */
> + free_ep_req(ep, req);
>   return;
>   }
>   break;

The ALSA framework will give you a notification _once_.  If the
resulting data is larger than midi->buflen, then you have to continue
sending packets.  This is exactly what the call to f_midi_transmit()
does.

(To decrease latency, it might be a good idea to queue multiple requests,
but you wouldn't want to continue piling up requests if the host isn't
listening.  sound/usb/midi.c just allocates a fixed number of requests,
and always reuses them.)


Regards,
Clemens
--
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 2/3] usb: gadget: f_midi: free usb request when done

[Clemens Ladisch]

Peter Chen wrote:
> I can't make my aplaymidi to receive data

> # aplaymidi
> open /dev/snd/seq failed: No such file or directory

modprobe snd-seq

There are mechanisms to load it automatically, but your embedded system
might not bother about any of them.  Or CONFIG_SND_SEQUENCER isn't
enabled at all.

In any case, raw MIDI (with the amidi tool) should work.


Regards,
Clemens
--
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: USB bug

[Clemens Ladisch]

Andrew Gillis wrote:
> Sep 17 10:07:38 sonicorbiter kernel: xhci_hcd :06:00.0: ERROR: unexpected 
> command completion code 0x11.

This means that the USB controller does not like what the controller
driver has told it to do.  This could be a bug in some driver, but if
it happens only with that NEC controller, the bug is likely to be there.

> Sep 17 10:07:38 sonicorbiter kernel: usb 1-2: Warning! Unlikely big volume 
> range (=32767), cval->res is proba

This is a firmware bug, but probably unrelated.

> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub

This is not the sound device.

> Sep 17 10:07:39 sonicorbiter kernel: usb 1-2: USB disconnect, device number 3

Either the USB controller or the device got really confused.

Try to get the "lsusb -v" output on some other controller/PC.


Regards,
Clemens
--
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: tools: usbip: detach: avoid calling strlen() at each iteration

[Clemens Ladisch]

Aaro Koskinen wrote:
> On Tue, Sep 15, 2015 at 09:27:20PM +0100, Eric Curtin wrote:
>> -for (unsigned int i = 0; i < strlen(port); i++)
>> +unsigned int port_len = strlen(port);
>> +
>> +for (unsigned int i = 0; i < port_len; i++)
>
> port is read only in this function, so maybe just use "const" and the
> compiler should know to do the same without adding a new variable?

If the compiler knows the implementation of strlen() (because it's
a built-in function), then it sees that nobody modifies port[] in the
loop.  If the compiler does not know the implementation of strlen()
(because -fno-builtins is used), then it is possible that some other
function has a valid non-const pointer and modifies the data through it.

(Anyway, the detach_port() function is not time critical, so I don't
think that optimizing it is worthwhile if it reduces readability.  But
seeing the strlen() call at that place grates on me; I'm not against
moving it out of the loop.)

The loop goes through the string one character at a time, so it might
be possible to drop strlen() altogether and just stop the loop when the
end of the string is reached.

But the actual purpose of the loop is to check whether there is a valid
number.  This could be done more easily by replacing the loop and the
following atoi() call with strtol().


Regards,
Clemens
--
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: MIDI gadget allocating too much from coherent pool

[Clemens Ladisch]

Felipe Tonello wrote:
> DSP (read sensors and read/send MIDI) <- UART -> SOC (imx6) <- USB
> MIDI GADGET -> HOST
>
> When the throughput from the DSP is high, thus causing the throughput
> on the USB to be high as well, I get a Kernel Panic saying to increase
> the coherent_pool. I've used some crazy sizes like 1M, 4M and even 8M.
> Obviously when I increase, it takes longer to crash but it still
> crashes.

This sounds like a memory leak in your USB host controller's driver
(whatever it is).

The USB MIDI driver uses URB_NO_TRANSFER_DMA_MAP with bulk transfers,
and continually resubmits the same URBs.  Both might be somewhat
unusual.


Regards,
Clemens
--
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: MIDI gadget allocating too much from coherent pool

[Clemens Ladisch]

I wrote:
> Felipe Tonello wrote:
>> DSP (read sensors and read/send MIDI) <- UART -> SOC (imx6) <- USB
>> MIDI GADGET -> HOST
>>
>> When the throughput from the DSP is high, thus causing the throughput
>> on the USB to be high as well, I get a Kernel Panic saying to increase
>> the coherent_pool. I've used some crazy sizes like 1M, 4M and even 8M.
>> Obviously when I increase, it takes longer to crash but it still
>> crashes.
>
> This sounds like a memory leak in your USB host controller's driver
> (whatever it is).
>
> The USB MIDI driver uses URB_NO_TRANSFER_DMA_MAP with bulk transfers,

Sorry, wrong driver. There's no URB_NO_TRANSFER_DMA_MAP in the gadget
API.

The f_midi driver does not use pre-mapped buffers, and the URB buffers
would use streaming DMA mappings anyway.  The coherent pool is used by
host controller drivers for their internal structures, such as DMA
descriptors.

The "fsl_usb2_udc" driver (if that is what you're using) uses a coherent
DMA pool for "TD management".  I see no obvious problem with how it
calls dma_pool_alloc()/dma_pool_free().  Are there any messages in the
system log?  You might want to modify the kernel to check how often
it calls these functions.


Regards,
Clemens
--
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: USB bug

[Clemens Ladisch]

Andrew Gillis wrote:
> I get an error when trying to play music through USB DACs under Linux.
> This only happens with a few high end DACs and only when using a NEC
> uPD72020x chipset USB 3.0 port.
>
> The people at RedHat think it may be the NEC uPD72020x chipset is
> reporting it's capabilities incorrectly.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1240798

USB streams are not related to audio (isochronous) transfers.

All those paste.fedoraproject.org links are broken.


Regards,
Clemens
--
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: external USB-Speaker and Not enough bandwidth for altsetting 1

[Clemens Ladisch]

Alan Stern wrote:
 On Sun, 2 Aug 2015, Jakob Schürz wrote:
 P:  Vendor=138a ProdID=003f Rev= 1.04

 Do you happen to know what this device is?

usb.ids says Validity Sensors VFS495 Fingerprint Reader.

 Does it make any difference if you remove the Bluetooth radio and the
 HP webcam?  They compete for bandwidth on the same bus as the AC209N
 speakers.

 In theory there should be plenty of bandwidth for all these devices to
 run at the same time.

All these devices have (small) interrupt endpoints for some kind of
status reporting.  It might be possible that the sheer number of
periodically scheduled transactions overflows some limit in the firmware
or driver.


Regards,
Clemens
--
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: urb-interval differently interpreted via xhci for 2.0

[Clemens Ladisch]

(quoting fixed)

Bernd Porr wrote:
 Alan Stern wrote:
 On Wed, 24 Jun 2015, Bernd Porr wrote:
 I use urb-interval to control the sampling rate of these devices.
 That works fine with the ehci driver. When I use the xhci driver it
 seems to be interpreting the urb-interval in a different way and/or
 ignoring it?

 As you have seen from the source code, xhci-hcd ignores the
 urb-interval value provided by the driver and instead uses the value
 from the endpoint descriptor.

And the xHCI specification requires this (section 6.2.3.6):
| For [isochronous] enpoint types, system software shall translate the
| bInterval field in the USB Endpoint Descriptor to the appropriate
| value for this field.

 Currently, I think the only way to do what you want is to set
 ep-desc.bInterval to the desired value and then call usb_set_interface().

 I guess the cleanest solution is to write additional code in both the
 firmware and the driver for xhci and then once the older drivers change
 roll that over.

 The best approach is (I think but correct me) to ditch ISO for now just
 for xhci and then much later also for the other drivers.

Please note that you can implement Alan's suggestion in a mostly
backwards-compatible way by having the device offering multiple
alternate settings; an old driver (using EHCI) would still be able to
use a different interval with the 'wrong' alternate setting.


Regards,
Clemens
--
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: default/target product id /vendor id for usb devices

[Clemens Ladisch]

temp sha wrote:
 I have a new usb dongle Huawei E3531 trying to support in my h/w
 having old kernel version 2.6.16.
 I use usb_modeswitch to change the mode from usb storage to modem
 manually. As I do not have udev rule
 support in my h/w, I need an alternative way of mode switching during
 plugin time automatically.

Does it work if you run usb_modeswitch manually?  If you don't have
udev, what _do_ you have?  hotplug?


Regards,
Clemens
--
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: default/target product id /vendor id for usb devices

[Clemens Ladisch]

temp sha wrote:
 Can any one brief me about the concept of default/target product id /vendor 
 id ?

These are not terms that have a well-known meaning.  Where did you hear them?

And what is the actual problem you're trying to solve?


Regards,
Clemens
--
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] snd-usb-audio: Add support for Akai MPC Element MIDI controller

[Clemens Ladisch]

Paul Bonser wrote:
 On 01/06/2015 07:39 AM, Clemens Ladisch wrote:
 Paul Bonser wrote:
 This device incorrectly reports its bInterfaceClass as 255, when it
 should really be 1 (USB_CLASS_AUDIO).

 +++ b/sound/usb/quirks-table.h
 +{
 +   /* Akai MPC Element */
 +   USB_DEVICE(0x09e8, 0x0021),
 +   .bInterfaceClass = USB_CLASS_AUDIO,
 +},

 This is not a correct usb_device_id entry.

 I based it on an existing entry in the same file:

 /* KeithMcMillen Stringport */
 {
   USB_DEVICE(0x1f38, 0x0001),
   .bInterfaceClass = USB_CLASS_AUDIO,
 },

 It built with no complaints and functioned as expected (showed up as a
 MIDI device and sent/recived commands via ALSA MIDI whereas before it
 didn't).

The bInterfaceClass value has no effect; these entries happen to work
only by chance.

All entries are supposed to define some quirk(s), like this:

{
/* Akai MPC Element */
USB_DEVICE(0x09e8, 0x0021),
.driver_info = (unsigned long)  (const struct snd_usb_audio_quirk) {
.ifnum = 1,
.type = QUIRK_MIDI_STANDARD_INTERFACE
}
},

 I should have sent he email to you (since you're the USB MIDI
 maintainer), and CCd the linux-ker...@vger.kernel.org and
 linux-usb@vger.kernel.org lists?

The only matching entry for sound/usb/quirks-table.h in the MAINTAINERS
file tells you to send to perex, tiwai, and alsa-devel.

 Once we've worked out the proper format for this entry, should I re-send
 it following that pattern?

Yes.


Regards,
Clemens
--
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] snd-usb-audio: Add support for Akai MPC Element MIDI controller

[Clemens Ladisch]

Paul Bonser wrote:
 This device incorrectly reports its bInterfaceClass as 255, when it
 should really be 1 (USB_CLASS_AUDIO).

 +++ b/sound/usb/quirks-table.h
 +{
 + /* Akai MPC Element */
 + USB_DEVICE(0x09e8, 0x0021),
 + .bInterfaceClass = USB_CLASS_AUDIO,
 +},

This is not a correct usb_device_id entry.

Show the output of lsusb -v for this device.

And read Documentation/SubmittingPatches.


Regards,
Clemens
--
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: unfixable usb porthole

[Clemens Ladisch]

Gene Heskett wrote:
 On Thursday 16 October 2014 18:28:16 Greg KH did opine
 And Gene did reply:
 On Thu, Oct 16, 2014 at 06:12:48PM -0400, Gene Heskett wrote:
 Is there a move afoot to write a checker utility that determines if
 the usb device its pointed at is vulnerable, and can therefore be
 reliably blacklisted?

 What do you mean by a vulnerable USB device?

 There is an exploitable error in the usb hardware/firmware, one that
 nearly 100% of the devices have.

That error is the fact that USB devices have a CPU which can execute
arbitrary code.  The BadUSB guys have shown that several widely-used
USB sticks allow the PC to change their firmware, but building USB
devices with malicious firmware has _always_ been possible; the only
difference is that the hardware costs have gone down from $40 for
a Rubber Ducky to $10 for an off-the-shelf memory stick.

 No one ever gave security a seconds thought when writing the usb std.  As
 described it is both hardware and firmware that will need to be addressed
 for an effective fix.

So you want to blacklist every device (USB or any other bus) that can be
connect to a PC?  And outlaw general-purpose computers?


Regards,
Clemens
--
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: [gadget] Testing midi function

[Clemens Ladisch]

Andrzej Pietrasiewicz wrote:
 I would like to try the midi function of usb gadget.

 Can anybody please tell me:

 1) What userspace programs do I need at the gadget side?

I've never used it, but as far as I can see, the MIDI function ends up
as a normal sound card with MIDI port(s).  Your device firmware would
access them through the normal ALSA rawmidi or sequencer APIs.

As for testing:  To monitor received data, use amidi or aseqdump.  To
send data, use amidi or aplaymidi.  To make a loopback device, use
aconnect to connect the input to the output.

This requires the alsa-lib and alsa-utils packages.

 2) What do I need at the host side?

USB MIDI is plugplay in every OS.  For Linux, see above.


Regards,
Clemens
--
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: usb sound cannot get frequency from enpoind

[Clemens Ladisch]

vichy wrote:
 ALSA sound/usb/clock.c:237 2:3:1: cannot get freq at ep 0x82
 ALSA sound/usb/mixer.c:929 5:2: cannot get min/max values for control 2 (id 
 5)
 ALSA sound/usb/mixer.c:929 5:2: cannot get min/max values for control 2 (id 
 5)

 (from the description, the ep 0x82 did have different sample rate
 configuration, but why uac cannot get it?)

This is likely to be a bug in the device's firmware.


Regards,
Clemens
--
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 v6 5/5] usb: gadget: f_uac2: send reasonably sized packets

[Clemens Ladisch]

Daniel Mack wrote:
 [...]
 Please note, however, that you can't do the divison 'uac2-p_residue /
 uac2-p_interval' in a pre-calucated value, as that won't cover cases
 with a per-packet residual value that isn't a multiple of the frame
 size.

Partial frames are not allowed (neither in ALSA nor in USB audio).


Regards,
Clemens
--
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 0/4] usb: gadget: f_uac2: cleanups and capture timing

[Clemens Ladisch]

Daniel Mack wrote:
 a) Linux snd-usb-audio currently pre-calculates the estimated packet
 sizes to expect from a USB device, and will only receive packets that
 have the expected size or are smaller.

snd-usb-audio allows packets to be 25 % larger.

 This can be worked around by setting the UAC_EP_CS_ATTR_FILL_MAX in
 the UAC2 endpoint descriptor

The spec says about this flag (4.10.1.2):
| when receiving data from an IN endpoint, the Host software must be
| prepared to receive wMaxPacketSize bytes and discard any superfluous
| zero bytes in the packet.
| Note:  This bit can only be used for Type II formatted data. The data
|stream must contain enough information (in a header) to
|separate the actual data from the padded zero bytes.

 send 0-byte packets from agdev_iso_complete() if the time passed since
 the last completed buffer does not allow for another one to be sent.

Audio Formats, 2.1:
| To indicate a temporary stop in the isochronous data stream ..., an
| in-band Transfer Delimiter needs to be defined.  This specification
| considers two situations to be a Transfer Delimiter. The first is
| a zero-length data packet and the second is the absence of an
| isochronous transfer

2.3.1.1:
| The goal must be to keep the instantaneous number of audio slots per
| virtual frame, ni as close as possible to the average number of audio
| slots per virtual frame, nav. [...] If the sampling rate is a constant,
| the allowable variation on ni is limited to one audio slot, that is,
| ∆ni = 1. This implies that all virtual frame packets must either
| contain INT(nav) audio slots (small VFP) or INT(nav) + 1 (large VFP)
| audio slots.

 This results in very stable timing behavior in my tests.

But it increases jitter, and might not work with any other driver.


f_uac(2) *must* implement some mechanism to control the clock, i.e., the
packet sizes.  (And this is not part of the standard ALSA API.)


Regards,
Clemens
--
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] usb: use kcalloc instead of kzalloc

[Clemens Ladisch]

Arjun Sreedharan wrote:
 kcalloc has protection from integer overflows
 which could arise from the multiplication of
 number of elements and size.

You are implying that such an overflow could arise in this code.
This is false.

 @@ -380,8 +380,7 @@ static int usb_parse_interface(struct device *ddev, int 
 cfgno,

   if (num_ep  0) {
   /* Can't allocate 0 bytes */
 - len = sizeof(struct usb_host_endpoint) * num_ep;
 - alt-endpoint = kzalloc(len, GFP_KERNEL);
 + alt-endpoint = kcalloc(num_ep, sizeof(struct 
 usb_host_endpoint), GFP_KERNEL);

bNumEndpoints is an unsigned 8-bit integer.

And even if you did not notice this, you should have noticed the if()
directly before this one.

 @@ -683,13 +682,11 @@ int usb_get_configuration(struct usb_device *dev)
   return -EINVAL;
   }

 - length = ncfg * sizeof(struct usb_host_config);
 - dev-config = kzalloc(length, GFP_KERNEL);
 + dev-config = kcalloc(ncfg, sizeof(struct usb_host_config), GFP_KERNEL);
   if (!dev-config)
   goto err2;

 - length = ncfg * sizeof(char *);
 - dev-rawdescriptors = kzalloc(length, GFP_KERNEL);
 + dev-rawdescriptors = kcalloc(ncfg, sizeof(char *), GFP_KERNEL);

Same oversights here.


Regards,
Clemens
--
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: Unknown USB IDs of Intenso Memory Box

[Clemens Ladisch]

Richard Hartmann wrote:
 I got two new Intenso 3.5 Memory Box with 3TB in the metallic edition
 with USB 3.0 interface.

 Please see the attached output of `lsusb -vvv -d 2109:0701`

Why?  Is there a problem?

 smartmontools doesn't seem to know that controller either

It does not need to know the controller.

Most USB bridge chips just do not support SMART commands.


Regards,
Clemens
--
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: Unknown USB IDs of Intenso Memory Box

[Clemens Ladisch]

Richard Hartmann wrote:
 On Wed, May 14, 2014 at 10:14 AM, Clemens Ladisch clem...@ladisch.de wrote:
 Why?  Is there a problem?

 No; it's working as expected. The implied statement was the ids
 should receive clear-text names.

The device has clear-text names:

  idVendor   0x2109
  idProduct  0x0701
  iManufacturer   1 Intenso
  iProduct2 External USB 3.0

 I don't know the exact chip name though, only the model; should I
 submit a patch with a generic name?

Patch for what?

And the generic name would be VIA VL701:
https://usb-ids.gowdy.us/read/UD/2109/0701


Regards,
Clemens
--
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: Fwd: Isochronos audio

[Clemens Ladisch]

Alan Stern wrote:
 The IN transfer was 1 frame long and scheduled for frame 1123, so its
 completion indicates that the current frame number is = 1123.  The OUT
 transfer was 6 frames long and scheduled for frame , so it should
 have completed in frame 1117.  But the timestamps show that the two
 URBs completed at the same time (only 13 us between them).

Looks like interrupt moderation.

What minimum queue length should a driver use to work with all HCs?


Regards,
Clemens
--
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: OTG102/EzCapture2 no audio

[Clemens Ladisch]

David Woodfall wrote:
 mplayer reports no sound:

 mplayer tv:// -tv 
 driver=v4l2:device=/dev/video1:normid=8:alsa:adevice=hw.1:forceaudio:volume=80
 ...
 Selected driver: v4l2
 Opening video decoder: [raw] RAW Uncompressed Video
 Audio: no sound

Does mplayer something.wav work?

 Neither aplay -l nor -L list the device

Because the device is not used for playback.
Try arecord -l.


Regards,
Clemens
--
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: OTG102/EzCapture2 no audio

[Clemens Ladisch]

David Woodfall wrote:
 Success. I used the second command mentioned on the linuxtv wiki:

 mplayer tv:// -tv
 driver=v4l2:device=/dev/video1:normid=8:alsa:adevice=hw.1:forceaudio:volume=80:immediatemode=0:width=720

 Still no sound through arecord though.

It's probably necessary to configure the tuner correctly.


Regards,
Clemens
--
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: [alsa-devel] Fw: Isochronous transfer error on USB3

[Clemens Ladisch]

Mauro Carvalho Chehab wrote:
 I'm getting an weird behavior with em28xx, especially when the device
 is connected into an audio port.

   
 http://git.linuxtv.org/mchehab/experimental.git/blob/refs/heads/em28xx-v4l2-v6:/drivers/media/usb/em28xx/em28xx-audio.c

 What happens is that, when I require xawtv3 to use any latency lower
 than 65 ms, the audio doesn't work, as it gets lots of underruns per
 second.

The driver uses five URBs with 64 frames each, so of course it
will not be able to properly handle periods smaller than that.

 FYI, em28xx works at a 48000 KHz sampling rate, and its PM capture Hw
 is described as:

 static struct snd_pcm_hardware snd_em28xx_hw_capture = {
   .info = SNDRV_PCM_INFO_BLOCK_TRANSFER |
   SNDRV_PCM_INFO_MMAP   |
   SNDRV_PCM_INFO_INTERLEAVED|
   SNDRV_PCM_INFO_BATCH  |
   SNDRV_PCM_INFO_MMAP_VALID,

   .formats = SNDRV_PCM_FMTBIT_S16_LE,

   .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_KNOT,

This should be just SNDRV_PCM_RATE_48000.

   .period_bytes_min = 64, /* 12544/2, */

This is wrong (if the driver doesn't install other constraints on the
period length, like the USB audio class driver does).


Regards,
Clemens
--
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: [alsa-devel] Fw: Isochronous transfer error on USB3

[Clemens Ladisch]

Mauro Carvalho Chehab wrote:
 Clemens Ladisch clem...@ladisch.de escreveu:
 Mauro Carvalho Chehab wrote:
 .period_bytes_min = 64, /* 12544/2, */

 This is wrong (if the driver doesn't install other constraints on the
 period length, like the USB audio class driver does).

 Ok, how should it be estimated?

This value specifies how fast the driver can report period interrupts,
i.e., how often it can call snd_pcm_period_elapsed().  In other words,
if the application configures the device for this minimum period size,
but if it is possible for this amount of bytes to be transferred
_without_ triggering an interrupt (by reaching the end of the URB), then
this value was too low.

The em28xx driver uses a fixed URB size, so actual interrupts happen
every 64 frames, so this value should be at least the number of bytes
that can be transferred in 64 ms (assuming a full-speed device).

Because you do not know the exact number of samples that will be sent
per frame, it is possible that the USB interrupt happens up to 63 ms
after the point where the period interrupt should actually have
happened.  This jitter could be reduced by using shorter URBs.

In any case, this driver does not need the integer constraint on the
period count.


Regards,
Clemens
--
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: [alsa-devel] no reset_resume for driver snd-usb-audio for logitech headset H600

[Clemens Ladisch]

Takashi Iwai wrote:
 On Tue, 8 Oct 2013 baum...@hotmail.com wrote:
 https://bugzilla.kernel.org/show_bug.cgi?id=62691

 Summary: no reset_resume for driver snd-usb-audio for logitech headset H600

 How about simply applying the same resume for reset_resume like below?

 ---

 diff --git a/sound/usb/card.c b/sound/usb/card.c
 index 9d9de8d..c591a83 100644
 --- a/sound/usb/card.c
 +++ b/sound/usb/card.c
 @@ -747,6 +747,7 @@ static struct usb_driver usb_audio_driver = {
   .disconnect =   usb_audio_disconnect,
   .suspend =  usb_audio_suspend,
   .resume =   usb_audio_resume,
 + .reset_resume = usb_audio_resume,
   .id_table = usb_audio_ids,
   .supports_autosuspend = 1,
  };

When the device was reset, the state of all mixer controls has been
lost.  Disconnecting the card (the default behaviour without
reset_resume) is the correct thing to do in this situation.

Userspace should pick up the USB device again when it reappears.
(But it doesn't.  Is this a PulseAudio or a KDE problem?)

And why does the kernel whine about the lack of a callback that is
documented as being optional?


Regards,
Clemens
--
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: [alsa-devel] question about webcam audio capture

[Clemens Ladisch]

vichy wrote:
 I try to do webcam audio capture on my arm platform.
 when I execute audio capture I got below message
 retire_capture_urb: 108 callbacks suppressed

This means that the retire_capture_urb() function printed lots of
messages, and that 108 of them were thrown away because they were
identical.  You have to look at the previous line to get the actual
message.

 2. from usb mon log, it seems HW keep Active status high even until
 ehci_io_watchdog happen, because the ep status is  -EXDEV.

How are you trying to capture data?


Regards,
Clemens
--
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: [alsa-devel] question about webcam audio capture

[Clemens Ladisch]

vichy wrote:
 2013/9/9 Clemens Ladisch clem...@ladisch.de:
 vichy wrote:
 I try to do webcam audio capture on my arm platform.
 when I execute audio capture I got below message
 retire_capture_urb: 108 callbacks suppressed

 This means that the retire_capture_urb() function printed lots of
 messages, and that 108 of them were thrown away because they were
 identical.

 Does that mean:
 1. 108 of messages are the same and through away

No, that at least 109 are the same.

 You have to look at the previous line to get the actual
 message.

 Would you please tell me what the previous line to get the actual
 message  you mean?

The previous line in the message log.

 tinycap record.wav -D 0 -d 0 -c 2 -r 32000

Try increasng -n or -p.

Does recoding on a PC with the same parameters work?


Regards,
Clemens
--
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: [alsa-devel] Buffer size for ALSA USB PCM audio

[Clemens Ladisch]

Alan Stern wrote:
 All the difficulty arises from the fact that we don't know beforehand
 how many URBs will constitute an ALSA period since for playback
 endpoints, the URB sizes can vary.  [...]
 the number of URBs per period is fixed, and the number of packets in
 each URB is adjusted during playback so that all the URBs end up
 having roughly the same number of frames.
 [...]
 The parameter calculation now ends up being the same for both recording
 and playback endpoints.  This may seem odd, but it follows from the
 reasoning above.

There is no reasoning about capture endpoints.

The driver cannot control how many samples actually end up in a capture
packet, so it is possible that URBs end up being one USB frame longer
than a period, in which case the ALSA period interrupts will accumulate
delays of up to one period, or that URBs end up being one USB frame
shorter than a period, in which case the ALSA period interrupt will be
delayed to the end of the _next_ URB.

The current algorithm uses very short capture URBs to ensure that _some_
URB is completed as soon as possible after a period ends.

Your patch makes things worse for running jackd at lower latencies
because playback and capture period interrupts are expected to be more
or less synchronous (jackd waits for both interrupts before acting on
one period).  With only two periods per buffer and the capture period
interrupt randomly delayed by up to one period, the time available for
processing one period is reduced to zero.

I'd suggest to keep the old calculation for capture URBs.  It would
make sense to use longer capture URBs only if the period size is
relatively large.

Note: for capturing, the number of URBs has no effect on latency and
just reduces the risk of overruns, so it is desirable to make the queue
as long as possible (for a given URB length).

 Not having heard any responses to the patch posted last Wednesday,

Sorry, my #patches / free_time quotient is rather large.


Regards,
Clemens
--
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: [alsa-devel] Buffer size for ALSA USB PCM audio

[Clemens Ladisch]

Pavel Hofman wrote:
 On 27.8.2013 09:19, Clemens Ladisch wrote:
 The driver cannot control how many samples actually end up in a capture
 packet,...

 Does this reasoning apply to asynchronous playback too?

No.

 I understand the driver has some control, but has to satisfy the endpoint
 feedback requests.

When the driver wants to submit a playback URB, it knows how many
samples it is copying into the packets.  (There is a delay between the
device reporting its desired rate and the driver actually using it.)


Regards,
Clemens
--
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: [alsa-devel] Buffer size for ALSA USB PCM audio

[Clemens Ladisch]

Alan Stern wrote:
 On Tue, 27 Aug 2013, Clemens Ladisch wrote:
 The current algorithm uses very short capture URBs to ensure that _some_
 URB is completed as soon as possible after a period ends.
 [...]
 I'd suggest to keep the old calculation for capture URBs.  It would
 make sense to use longer capture URBs only if the period size is
 relatively large.

 Okay.  What about playback endpoints with implicit sync?  The current
 driver treats them the same as capture endpoints.  Is that really the
 right thing to do?

The only reason to not have an interrupt after each packet is to avoid
the interrupt overhead (for both CPU and power); shorter URBs would
result in a more precise DMA position reported to user space.  If there
is already an interrupt for some capture URB (or for the feedback packet
in case of explicit feedback), we might as well handle the playback
packets so far.


Regards,
Clemens
--
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: [alsa-devel] Buffer size for ALSA USB PCM audio

[Clemens Ladisch]

Alan Stern wrote:
 On Tue, 13 Aug 2013, Clemens Ladisch wrote:
 The difference is that this version tries always to keep a period's
 worth of bytes in the USB hardware queue.

 Having truncated URBs is possible only when URBs are shorter than one
 period,

 No.  URBs are truncated when a full-sized URB would extend at least one
 packet beyond the end of a frame.

This thought was in the context of avoiding too-short queues.  When
there are multiple URBs per period, the queue is long enough anyway.

 so I think that a queue length of at least two periods, together
 with a minimum period size, should ensure the isochrounous scheduling
 threshold.

 This depends on how long a period is.

In that patch, a period is guaranteed to be no smaller than the IST.

 And while we're at it: the default number of packets per URB was chosen
 before the driver had the ability to estimate the delay from the current
 frame number; it should now be quite safe to increase it.

 I don't understand how this delay estimation is supposed to work, or
 what it is meant to accomplish.  Can you explain?

The delay is the difference between the time when a sample is written
by the application and when that sample is actually played, and is
important for things like A/V synchronization.  It depends on
1) the amount of samples already in the ring buffer;
2) any processing done by the driver; and
3) any processing done by the hardware.

Most drivers don't do any processing, and most of the hardware has very
low delays, so it is common for drivers to pretend 2) and 3) are zero.

snd-usb-audio computes 2) from the current number of packets in the
queue, with the progress estimated based on the current frame.  Without
this computation, it was desirable to have short URBs because the delay
would jump by a large amount whenever a URB was completed.


Regards,
Clemens
--
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: [alsa-devel] Buffer size for ALSA USB PCM audio

[Clemens Ladisch]

Alan Stern wrote:
 On Mon, 12 Aug 2013, Alan Stern wrote:
 Here's a revised version of the patch (still untested).  The difference
 is that this version tries always to keep a period's worth of bytes in
 the USB hardware queue.  This will provide better protection against
 underruns when the period is larger than the queue's minimum
 requirement.

 After more thought, I decided that patch does too much.  It's not
 necessary to keep track of the number of packets.  Instead, the driver
 should always try to keep as much data in the USB hardware queue as it
 is allowed to.

This is what the current code does (i.e., re-submitting all URBs in
their completion handler).

 In other words, there should be enough URBs so that an entire ALSA
 buffer can be queued at any time, subject only to the limit on the
 maximum number of URBs and packets.

The URB queue adds latency, so it should never be made too big, even
for big ALSA buffers.  Once we have reached a queue length that is
practically guaranteed to be safe from underruns, more URBs do not make
sense.  (The current limit of 24 ms is somewhat arbitrary.)

 It doesn't make sense to allocate just enough URBs to cover a single
 period.

Indeed.  I've used two periods in my recent patch, but I don't really
know how I would justify this choice in the commit message.  ;-)

What doesn't make sense either is the current algorithm that computes
a specific value for the total number of packets (total_packs) and then
takes great care to allocate the URBs so that this number is reached,
even if this means that the number of packets per URBs varies.

And while we're at it: the default number of packets per URB was chosen
before the driver had the ability to estimate the delay from the current
frame number; it should now be quite safe to increase it.


Regards,
Clemens
--
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: [alsa-devel] Buffer size for ALSA USB PCM audio

[Clemens Ladisch]

Alan Stern wrote:
 On Mon, 12 Aug 2013, Takashi Iwai wrote:
 So... Clemens, Daniel, Eldad, could you guys review the latest version
 of Alan's patch?  I'd love to sort this out for 3.12.

 Here's a revised version of the patch (still untested).

 - maxsize = ((ep-freqmax + 0x) * (frame_bits  3))
 -  (16 - ep-datainterval);
 + maxsize = ((ep-freqmax + 0x)  (16 - ep-datainterval))
 + * (frame_bits  3);

What do you assume prevents firmware writers from splitting frames
across packages?  The specification?  Sanity?  :)  (see txfr_quirk)

 The difference is that this version tries always to keep a period's
 worth of bytes in the USB hardware queue.

Having truncated URBs is possible only when URBs are shorter than one
period, so I think that a queue length of at least two periods, together
with a minimum period size, should ensure the isochrounous scheduling
threshold.

This isn't tested either.


Regards,
Clemens


 sound/usb/endpoint.c |3 ++-
 sound/usb/pcm.c  |   16 ++--
 2 files changed, 16 insertions(+), 3 deletions(-)

--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -638,7 +638,8 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
if (sync_ep)
minsize -= minsize  3;
minsize = max(minsize, 1u);
-   total_packs = (period_bytes + minsize - 1) / minsize;
+   /* try to make the queue length the same as two periods */
+   total_packs = (2 * period_bytes + minsize - 1) / minsize;
/* we need at least two URBs for queueing */
if (total_packs  2) {
total_packs = 2;
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1131,10 +1131,22 @@ static int setup_hw_info(struct snd_pcm_runtime 
*runtime, struct snd_usb_substre
return err;

param_period_time_if_needed = SNDRV_PCM_HW_PARAM_PERIOD_TIME;
-   if (subs-speed == USB_SPEED_FULL)
+   switch (subs-speed) {
+   case USB_SPEED_FULL:
/* full speed devices have fixed data packet interval */
ptmin = 1000;
-   if (ptmin == 1000)
+   break;
+   case USB_SPEED_HIGH:
+   /*
+* Assume we have an EHCI controller with an Isochronous
+* Scheduling Threshold of one frame (8 microframes), and
+* add 2 microframes for boundary cases.  Increase by 4%
+* to have a margin for clock inaccuracies.
+*/
+   ptmin = max(ptmin, (8 + 2) * 130u);
+   break;
+   }
+   if (ptmin = 1000)
/* if period time doesn't go below 1 ms, no rules needed */
param_period_time_if_needed = -1;
snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_TIME,
--
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: [alsa-devel] [PATCH] 6fire: fix URB transfer buffer for midi output

[Clemens Ladisch]

Takashi Iwai wrote:
 Alan Stern wrote:
 A buffer _can_ be in the middle of a kmalloc'ed space, but the CPU must
 not access any of the fields around it that might occupy the same cache
 line while the buffer is being used for DMA.

 Hrm, but does the kmalloc buffer always guarantee such cache line
 exclusiveness...?  I thought a simple one like SLOB doesn't care.

Documentation/DMA-API-HOWTO.txt says:
|  Architectures must ensure that kmalloc'ed buffer is
|  DMA-safe. Drivers and subsystems depend on it. If an architecture
|  isn't fully DMA-coherent (i.e. hardware doesn't ensure that data in
|  the CPU cache is identical to data in main memory),
|  ARCH_DMA_MINALIGN must be set so that the memory allocator
|  makes sure that kmalloc'ed buffer doesn't share a cache line with
|  the others.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

Alan Stern wrote:
 On Wed, 31 Jul 2013, James Stone wrote:
 On Wed, Jul 31, 2013 at 4:48 PM, Alan Stern st...@rowland.harvard.edu 
 wrote:
 James, see what happens with this patch.  It will print a warning
 message in the system log every time it detects an underrun, but it
 won't cause an URB submission failure any more.

 OK - I will try it, however, it seems a bit like papering over the
 cracks without really understanding what's causing the error..

 It seems likely that the error is caused by an SMI taking too much
 time.  At least, we seem to have ruled out everything else.  Besides,
 this change has to be made eventually in any case -- underruns can
 occur at any time, in principle, and they shouldn't cause the audio
 driver to fail.

Well, the failure is a bug in snd-usb-audio: when usb_submit_urb fails,
it should report the underrun so that the stream can be stopped and
restarted cleanly.  This would be done by the snd_pcm_stop() call in
endpoint.c which is currently commented out because of locking problems.


Regards,
Clemens
--
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] usb: gadget: audio file: wake up sleep thread when device unbind

[Clemens Ladisch]

Qiao Zhou wrote:
 when usb audio device removes, it doesn't notify the ALSA read /
 write thread. due to no data transmitting any more, those threads
 wait for a long timeout(10s), then detects IO error. it causes
 long time blocking in upper layer read/write.

 to fix this issue, wake up potential sleep thread if necessary
 when audio unbind.

 + if (substream  substream-runtime  substream-runtime-twake)
 + wake_up(substream-runtime-tsleep);

The PCM stream should be properly stopped, which automatically takes
care of notifying userspace.  See this code for an example:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/line6/pcm.c?id=86f0b5b86d142b9323432fef078a6cf0fb5dda74#n392

 substream  pcm may still be used after audio unbind, and we
 should only set null to it in audio_pcm_close.

What happens when the application immediately tries to restart the
stream?  Many USB audio drivers set a disconnected flag to prevent
this, but I don't know if the gadget driver has races that would require
such a check.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

Alan Stern wrote:
 On Thu, 1 Aug 2013, Clemens Ladisch wrote:
 It seems likely that the error is caused by an SMI taking too much
 time.  At least, we seem to have ruled out everything else.  Besides,
 this change has to be made eventually in any case -- underruns can
 occur at any time, in principle, and they shouldn't cause the audio
 driver to fail.

 Well, the failure is a bug in snd-usb-audio: when usb_submit_urb fails,
 it should report the underrun so that the stream can be stopped and
 restarted cleanly.  This would be done by the snd_pcm_stop() call in
 endpoint.c which is currently commented out because of locking problems.

 Should we have some sort of threshold for how long an underrun can be
 before it causes a submission failure?

 Presumably an underrun of a few ms should not cause the stream to be
 stopped and restarted.  An underrun of 100 ms or more probably should.
 Where do we put the cutoff?

This is policy which should not be decided by the HCD; the driver can
decide whether to stop the stream when it sees the error in the
completion callback.

If there was an underrun, packets will fail+complete faster until the
queue has caught up to the actual schedule.  When the length of the
underrun is longer than ALSA's ring buffer, that fast draining of data
will result in an underrun of the ring buffer.  The policy for these
underruns (stop or ignore) can be set by ALSA applications.

So a longer cutoff allows drivers and applications to ignore longer
underruns, if they choose to do so, or to treat them as errors anyway.

So there is no reason for HCDs to make the cutoff smaller than required
for technical reasons.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

Alan Stern wrote:
 On Thu, 25 Jul 2013, James Stone wrote:
 The only slight difference I can see is that maybe the 3.10 uses
 slightly higher CPU load than 3.5 at the ridiculously low latency of
 64 frames/period duplex.

 With the new patch, what you actually get is 44.1 frames/period (on
 average).

In ALSA, the number of frames per period is a constant integer, and Jack
requires it to be a power of two.  (Where frame is an audio frame, and
period is the interval between interrupts reported to user space.)

 However, something's not working right.  The number of packets in each
 playback URB changes each time the URB is reused!  That's not supposed
 to happen.  The number of packets should remain fixed while the number
 of samples in each packet changes, based on the feedback info.

 I don't get it.  The usbmon trace shows three URBs, and the number of
 packets goes like this:

   8 8 8   8 4 8   4 8 3   8 4 8   4 8 3   8 4 8   3 8 4   8 4 8

With a sample rate of 44100 Hz and a packet rate of 8000 Hz, there
should be about 5.5 samples per packet.  With a period size of 64 audio
frames, this results in about 11.6 packets per period.

The driver does not completely fill URBs to ensure that interrupts
happen at period boundaries.

 Another problem, not necessarily a bad one: The feedback data from the
 sound device indicates that its internal clock is actually running at
 45168 Hz, even though it claims to be running at 44100.

The feedback data is not measured in real Hz (wall clock time) but
relatively to the 8 kHz bus clock.  Furthermore, it does not show the
device's internal clock but the rate at which the device wants to
receive frames; this can be higher at the beginning of a stream if the
device has an empty FIFO and wants to fill it up.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

James Stone wrote:
 Ok -from the bisect, the problem of not being able to get to sub
 64-frames per period starts with:

 http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=fc600432cd23e35c85de2ff4468d816d6938a112

This is a merge, which includes this commit: 
http://git.kernel.org/linus/e9ba389c5ffc.

 Could this be the offending deletion?

 http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/diff/sound/usb/endpoint.c?id=fc600432cd23e35c85de2ff4468d816d6938a112

(There is a corresponding change in pcm.c.)

The commit that you found by bisection got reverted later:
http://git.kernel.org/linus/015618b902ae

Please check that the code of Fix URB cancellation at stream start is
in your current kernel.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

James Stone wrote:
 On Thu, Jul 25, 2013 at 2:12 PM, Clemens Ladisch wrote:
 The commit that you found by bisection got reverted later:
 http://git.kernel.org/linus/015618b902ae

 Please check that the code of Fix URB cancellation at stream start is
 in your current kernel.

 well, in 3.10, it is not exactly the same as the above revertion. For example:

 /* just to be sure */
 deactivate_urbs(ep, false);
 if (can_sleep)

This is exactly the same as in commit 015618b902ae.


Regards,
Clemens
--
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: Sound Blaster USB X-Fi configuration problem

[Clemens Ladisch]

Alan Stern wrote:
 On Sat, 20 Jul 2013, Mariusz Grecki wrote:
 +u16 buf = 1;
 +
 +snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
 +USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
 +0, 0, buf, 2);

 There's a serious problem here.  It's present in most or all of the
 quirk routines in this file, not just yours.

 Namely, buffers used for USB transfers must not be allocated on the
 stack; they must be allocated using kmalloc or a related function.

This is what snd_usb_ctl_msg() does.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Ming Lei wrote:
 On Fri, Jul 19, 2013 at 5:20 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Thu, 18 Jul 2013, Clemens Ladisch wrote:
 In any case, there must be _some_ mechanism to explicitly restart
 a stream.  I do not really care if this is some URB flag or some
 function call.

 I prefer a function call over the flag.  The function call can easily
 be issued just once, but the completion routine would have to clear the
 flag every time the URB gets used.

 IMO, one good candidate is to do it in usb_set_interface()

This will result in a control request to be sent to the device, which
might confuse its firmware.

 also it is reasonable, see blow:

 From 5.6.3 Isochronous Transfer Packet Size Constraints of USB spec 2.0:

   All device default interface settings must not include any isochronous
   endpoints with non-zero data payload sizes.

That's what the spec says.  In reality, there are devices that have non-
zero data payload sizes in the default interface setting, and have no
other alternate setting.

 that said all drivers which are using isoc endpoints have to call
 usb_set_interface(altsetting) explicitly before starting isoc schedule.

But a set_interface request does not necessarily affect a single stream;
there are devices that have multiple isochronous and bulk endpoints in
a single interface, and where restarting one stream must not affect the
others.

 Maybe we can use usb_reset_endpoint() for this purpose after all.  It
 is a perfect fit, because we want to tell the HCD to reset the
 isochronous endpoint back to the start of stream state.

 I suggest not to introduce extra starting stream function to 
 usb_reset_endpoint(),
 and if we have to do this, I suggest to add a new API for the purpose cleanly.

I agree.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Ming Lei wrote:
 On Fri, Jul 19, 2013 at 1:44 AM, Clemens Ladisch clem...@ladisch.de wrote:
 In any case, there must be _some_ mechanism to explicitly restart
 a stream.  I do not really care if this is some URB flag or some
 function call.

 Thought about the problem further,  looks there is one simple
 approach for detecting underrun in case of empty queue:

 if (list_empty (stream-td_list)) {
 if (running from hcd interrupt or tasklet context)
  underrun
 else
  new stream
 }

Why shouldn't a driver start a stream in an interrupt/tasklet,
or delay URB resubmission to a workqueue?


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Ming Lei wrote:
 On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
 Ming Lei wrote:
 if (list_empty (stream-td_list)) {
 if (running from hcd interrupt or tasklet context)
  underrun
 else
  new stream
 }

 Why shouldn't a driver start a stream in an interrupt/tasklet,

 It should, but actually one driver won't do this because generally
 usb_set_interface() is required before starting stream.

Not always, and the usb_set_interface() call can be done in a different
context.

 If stream is started in another non-isoc URB complete(), this approach
 can cover this situation easily.

 But do you have examples in which one isoc stream is started in another
 isoc URB's complete()?

There are quite a few audio devices using implicit rate feedback, where
playback URBs are submitted from the capture completions handler.

 or delay URB resubmission to a workqueue?

 Yes, it is possible, and the isoc URBs can be resubmitted in tasklet too,
 but it isn't a big deal:

But your proposed code would not be able to differentiate a workqueue
resubmission from a stream start.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Alan Stern wrote:
 On Thu, 18 Jul 2013, Ming Lei wrote:
 On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu 
 wrote:
 On Thu, 4 Jul 2013, Alan Stern wrote:
 On Thu, 4 Jul 2013, Ming Lei wrote:
 I had some more ideas about this.  Instead of requiring drivers to set
 URB_ISO_ASAP on just the first URB of an isochronous stream, we could
 ask drivers to call usb_reset_endpoint() between streams.  This would
 tell the HCD that the next URB marks the start of a new stream, with no
 need for a special flag.

 This idea still requires changes from old drivers.

 Also it might be not appropriate to call usb_reset_endpoint() in this case
 because other host controller's implementation may do other unnecessary
 thing for this situation.

 Perhaps.  I doubt that HCDs need to do anything when they reset an
 isochronous endpoint, but you're right.  It's safer to avoid the issue.

 Another possibility, which would be even simpler, is for HCDs to assume
 that if the endpoint's queue has been empty for more than 100 ms then a
 new stream is starting.  Then drivers wouldn't have to do anything
 special at all.  (Unless they are stopping and restarting streams very
 rapidly,

... which happens when a stream is restarted after an error, or when two
sound files are played back-to-back.  The audio driver will always
explicitly restart the stream (because checking whether the timeout has
elapsed would be too much of a bother), so the timeout is not useful
in practice.

 In this case, we may use changing altsetting to decide start of streaming.

 Yes indeed.  But that could still require changes to old drivers.

 To be even more safe, unlinking an URB should mark the end of a stream.
 That's what snd-usb-audio does when it closes a stream; it kills all
 the outstanding URBs.

But it might be possible that the queue is empty at that point.


In any case, there must be _some_ mechanism to explicitly restart
a stream.  I do not really care if this is some URB flag or some
function call.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

Alan Stern wrote:
 The audio-out stream uses a pipeline of only 2 URBs.  The URBs start
 out alternating between 8 and 7 packets apiece.  This yields a total
 latency around 1.9 ms (equivalent to 2 periods of about 41 frames at
 44.1 KHz), which is smaller than I would expect (2 periods of 64 frames
 works out to 2.9 ms).

 Anyway, this sort of works.  Every so often (roughly at intervals of 15
 ms) there is an underrun.  Evidently the computer's EHCI hardware
 sometimes requires URBs to be submitted more than 7 microframes in
 advance.

It would be trivial to make the driver respect these constraints, if
only there were some mechanism to know about them.

 After a second of this, the URBs suddenly changed.  They began to
 alternate between 7 and 5 packets; one of them had only 4.  I have no
 idea why this happened.

On this device, the host's playback rate is controlled by the device's
frequency feedback.

It's possible that all those underruns have confused the device's sample
counter.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

Alan Stern wrote:
 On Sat, 13 Jul 2013, Clemens Ladisch wrote:
 James Stone wrote:
 On Mon, Jul 8, 2013 at 2:12 PM, James Stone jamesmst...@gmail.com wrote:
 configuring for 44100Hz, period = 64 frames (1.5 ms), buffer = 2 periods
 JackAudioDriver::ProcessAsync: read error, stopping...

 Some further info - on 3.5.0-28, I can start jackd in playback only
 with 8 frames/period, and capture only at 16 frames/period.

 Any thoughts on further investigating this bug with the 3.8.0 kernel
 with the Focusrite Scarlett 2i4?

 Jack assumes that the interrupts for the playback and capture streams
 happen at more or less the same time.  It might be possible that on the
 newer kernels, there is a difference between the arrival times of the
 first completion callback of each stream.

 The interrupts shouldn't differ by more than the duration of one URB,
 which would be 1 ms.  There is an initial delay when a stream is first
 started, which generally lasts 5-10 ms.  But I think that hasn't
 changed since the 3.5 kernel.  Would it make any difference?

The initial delay does not really matter as long as it's the same for
both streams.  (A difference of 1 ms would be significant if the period
size is that short.)

 Bear in mind that the input and output streams are started at totally
 different times.

Jack takes great care to start them together.

 And anyway, James's latest problem occurs even with playback only.

On my machine, 3 x 8 frames works, although Jack often complains that
two periods are already completed when it expected only one.  Anything
less (2 x 8 frames) does not work; and 8 frames is where even my PCI
card begines to make problems.

In any case, poll timeout would not indicate completion delays but
that no data was transferred _at all_.  In theory, this should not be
possible.  I'm stumped at the moment.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

James Stone wrote:
 On Mon, Jul 8, 2013 at 2:12 PM, James Stone jamesmst...@gmail.com wrote:
 configuring for 44100Hz, period = 64 frames (1.5 ms), buffer = 2 periods
 JackAudioDriver::ProcessAsync: read error, stopping...

 Some further info - on 3.5.0-28, I can start jackd in playback only
 with 8 frames/period, and capture only at 16 frames/period.

 Any thoughts on further investigating this bug with the 3.8.0 kernel
 with the Focusrite Scarlett 2i4?

Jack assumes that the interrupts for the playback and capture streams
happen at more or less the same time.  It might be possible that on the
newer kernels, there is a difference between the arrival times of the
first completion callback of each stream.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

James Stone wrote:
 On Mon, Jul 8, 2013 at 2:12 PM, James Stone jamesmst...@gmail.com wrote:
 ALSA: poll time out, polled for 2176094 usecs
 JackAudioDriver::ProcessAsync: read error, stopping...

 This is a definite reduction in performance compared to earlier kernels.

In theory, poll time out indicates that the stream does not appear to
be running.  But I doubt this is a driver problem.

 Some further info - on 3.5.0-28, I can start jackd in playback only
 with 8 frames/period, and capture only at 16 frames/period.

 Any thoughts on further investigating this bug with the 3.8.0 kernel
 with the Focusrite Scarlett 2i4?

I'll see if I can reproduce this with my own async-feedback device.


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

Alan Stern wrote:
 The first half is audio-out only.  About 2 seconds after the start, the
 audio-in stream starts up.  After 2 URBs (2 ms) of data, everything
 is shut down for no apparent reason -- there were no I/O errors.
 [...]
 I can't tell whether all these starts and stops are caused by JACK or
 by the ALSA layer, let alone figure out the reason for them.

Two URBs is the Jack buffer size, isn't it?  Does Jack complain?

James, pleasy try running aplay and arecord at the same time; something
like this:

aplay   -D hw:x -t raw -c 4 -f S32_LE -r 44100 --period-size=128 
--buffer-size=256 /dev/zero 
arecord -D hw:x-c 2 -f S32_LE -r 44100 --period-size=128 
--buffer-size=256 test.wav


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Alan Stern wrote:
 On Thu, 4 Jul 2013, Ming Lei wrote:
 Changing the altsetting is another way to tell the HCD that a stream is
 starting fresh.

 Yes, it might work, but many details need to be payed attention to, such
 as, changing altsetting may not be done during suspend/resume.

 Hmmm.  I don't know how snd-usb-audio handles a suspend/resume in the
 middle of playback or recording.

In this case, the ALSA framework will stop the PCM stream, and on resume,
report this like an underrun; the application will then reinitialize the
stream.  (Handling suspend/resume transparently would require that the
driver can restart playing from an arbitrary position in the ring buffer;
snd-usb-audio does not bother to implement this.)


Regards,
Clemens
--
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: Audio I/O parameters

[Clemens Ladisch]

I wrote:
 I'll fix this in the driver.

James, please test this patch.


Regards,
Clemens

--88--
ALSA: usb-audio: do not trust too-big wMaxPacketSize values

The driver used to assume that the streaming endpoint's wMaxPacketSize
value would be an indication of how much data the endpoint expects or
sends, and compute the number of packets per URB using this value.

However, the Focusrite Scarlett 2i4 declares a value of 1024 bytes,
while only about 88 or 44 bytes are be actually used.  This discrepancy
would result in URBs with far too few packets, which would not work
correctly on the EHCI driver.

To get correct URBs, use wMaxPacketSize only as an upper limit on the
packet size.

Reported-by: James Stone jamesmst...@gmail.com
Not-yet-tested-by: James Stone jamesmst...@gmail.com
Cc: sta...@vger.kernel.org # 2.6.35+
Signed-off-by: Clemens Ladisch clem...@ladisch.de
---
 sound/usb/endpoint.c |   13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 7a444b5..659950e 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -591,17 +591,16 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
ep-stride = frame_bits  3;
ep-silence_value = pcm_format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;

-   /* calculate max. frequency */
-   if (ep-maxpacksize) {
+   /* assume max. frequency is 25% higher than nominal */
+   ep-freqmax = ep-freqn + (ep-freqn  2);
+   maxsize = ((ep-freqmax + 0x) * (frame_bits  3))
+(16 - ep-datainterval);
+   /* but wMaxPacketSize might reduce this */
+   if (ep-maxpacksize  ep-maxpacksize  maxsize) {
/* whatever fits into a max. size packet */
maxsize = ep-maxpacksize;
ep-freqmax = (maxsize / (frame_bits  3))
 (16 - ep-datainterval);
-   } else {
-   /* no max. packet size: just take 25% higher than nominal */
-   ep-freqmax = ep-freqn + (ep-freqn  2);
-   maxsize = ((ep-freqmax + 0x) * (frame_bits  3))
-(16 - ep-datainterval);
}

if (ep-fill_max)
--
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: Audio I/O parameters

[Clemens Ladisch]

Alan Stern wrote:
256 samples/period / (44100 samples/second) * 8000 microframes/second
   = 46.44 microframes/period.

 Therefore I would expect to see snd-usb-audio submitting isochronous
 URBs with 46 or 47 packets, with a pipeline depth of 2 URBs.

 However, that's not what actually happens.  The audio-out stream uses 8
 URBs each containing about 7 packets on average, for a latency of 7 ms.
 The audio-in stream uses 8 URBs each containing 1 packet, for a latency
 of 0.125 ms and a pipeline duration of 1 ms -- which is too small for
 the current version of ehci-hcd to accept.

IIRC 8 x 1 frames worked once upon a time.

Anyway, the driver attempts to keep a minimum size of 1 ms for each URB,
but also uses wMaxPacketSize to compute this and assumes that this is
not too far off from the actual packet size.  This appears to be the
first device to use a ridiculously large value of 1024 bytes.

I'll fix this in the driver.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Pratyush Anand wrote:
 On 6/30/2013 8:32 PM, Alan Stern wrote:
 It could schedule the URB for the first time slot known to be
 available, even if that means skipping some time slots which
 the hardware might (or might not) be able to use.

 IMO, this approach is better.

To quote the USB spec:
| Isochronous Data
| A stream of data whose timing is implied by its delivery rate.

Isochronous transfers are usually used for real-time applications where
it is better to drop a single packet than to delay *all* following
packets.

 It could try to schedule the URB for the next time slot after
 the last one used by the preceding URB, even if that time slot
 has already expired.

 There is no meaning of submitting an URB for expired time slot.

Of course there is.  The HCD cannot exactly know whether the current
slot will expire before the URB is enqueued, so it is not always
possible to label the slot as expired or not during submission.

The meaning of an isochronous URB submission is try to transmit this
data in that frame; whether the URB actually was transferred will be
reported to the completion callback.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Ming Lei wrote:
 On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu 
 wrote:
 Thus, for example, even if the pipeline contains only a single URB,
 with the current code it will not become empty.  But with the new code
 it will.  If the load on the system is too high, the pipeline could
 empty out even if it normally contains two or more URBs.

 [...]
 But I don't know how USB audio driver uses URBs, and could it
 submit only one isoc URB to playback/record audio data?

The audio drivers uses at least two URBs.  (The actual number and length
of URBs depends on how the application configures its buffer.)


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Ming Lei wrote:
 On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu 
 wrote:
 Naturally, under normal circumstances this won't matter, because
 underruns shouldn't occur.  But I know from experience that people try
 to push the latency as far down as they can, which increases the
 likelihood of underruns.

 I understand the latency is effected by packet count in one URB,
 and it shouldn't be related with URB count,

This is true only when capturing.  For playback, the latency is the
length of the entire pipeline.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Ming Lei wrote:
 On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
 Ming Lei wrote:
 I understand the latency is effected by packet count in one URB,
 and it shouldn't be related with URB count,

 This is true only when capturing.  For playback, the latency is the
 length of the entire pipeline.

 For playback, every URB submitted is added into hw table
 immediately, then the data will be played to speaker. I don't
 understand why the latency is the entire pipeline.

A submitted packet will be transmitted only after all the other packets
in the pipeline have been transmitted.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Pratyush Anand wrote:
 On 7/1/2013 4:48 PM, Clemens Ladisch wrote:
 There is no meaning of submitting an URB for expired time slot.

 Of course there is.  The HCD cannot exactly know whether the current
 slot will expire before the URB is enqueued, so it is not always
 possible to label the slot as expired or not during submission.

 The meaning of an isochronous URB submission is try to transmit this
 data in that frame; whether the URB actually was transferred will be
 reported to the completion callback.

 Correct, I understand it. Let me explain with an example what I wanted
 to convey:

 Lets assume that URB1-8 has been submitted to HCD driver from upper
 layer. HCD driver has also further submitted them to the controller
 hardware for execution in n to (n+7)th uf respectively. Now URB3 was
 not able to be transmitted in time and resulted in missed isoc. This
 failure will cause mainly because of two reasons:
 1. SW was slow enough in submission of URB to HW. It actually
submitted when timing was already expired. In this case, it is most
likely that following URBs (4-8) will also result in missed isoc.
So, if we add further URB9 for transmission in (n + 8) uf, most
likely it may also result in missed isoc.

But there is _some_ uf that has not yet expired.  If the HCD delays
enqueuing URBs, it just increases the risk that that uf will expire too.

 2. SW submitted well in time, but HW was slow enough and could not
fetch data in time. In such situation, HW will flush current
descriptor and most likely will be able to transmit next URB
correctly.

 So, What I was proposing, to go for dynamic, something like this...
 When an HCD gets an URB request for any isoc pipe, and if there is any
 pending missed isoc flag then wait.. do not submit it to controller.
 May be just keep them in received list(submitted_list). If during any
 of the pending completion callback, SW observes success then submit
 all from submitted_list to be scheduled in very next uf

This is not possible with a very short pipeline where there is no other
packet that could complete.

And even if it is possible in some cases: why should submitted packets
be delayed at all?  The UAC driver wants all packets at their correct
position in time (or else to be dropped), and the UVC driver does not
want delays which could make it lose some data sent by the device.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Ming Lei wrote:
 On Mon, Jul 1, 2013 at 9:27 PM, Clemens Ladisch clem...@ladisch.de wrote:
 Ming Lei wrote:
 On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
 Ming Lei wrote:
 I understand the latency is effected by packet count in one URB,
 and it shouldn't be related with URB count,

 This is true only when capturing.  For playback, the latency is the
 length of the entire pipeline.

 For playback, every URB submitted is added into hw table
 immediately, then the data will be played to speaker. I don't
 understand why the latency is the entire pipeline.

 A submitted packet will be transmitted only after all the other packets
 in the pipeline have been transmitted.

 Yes, that is always true since EHCI HW will send out data(packet) to
 device one by one at the scheduled frame/uframe according to the
 order of URB submitting , so the USB audio driver can submit URBs
 in advance, can't it?

Latency is defined as the time interval between the software generating
the sample data, and the data actually being played.  When the driver
submits URBs in advance, latency increases accordingly.

(The Linux USB API does not allow changing a URB's buffer after it has
been submitted.)

 Also could you provide the typical time one URB in audio driver may
 span?

Anything between one microframe and twenty milliseconds.  Most software
ends up using eight milliseconds.


Regards,
Clemens
--
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: How should we handle isochronous underruns?

[Clemens Ladisch]

Alan Stern wrote:
 (1) Reject the submission with -EXDEV -- this is what we do now.

 (2) Accept the submission, but have the URB complete immediately
   with urb-status and all the packet statuses set to -EXDEV.

 (3) Accept the submission, but have the URB complete immediately
   with urb-status set to 0 and all the packet statuses set to
   -EXDEV.

The audio driver currently ignores submission errors (because of a bug:
the obvious way to stop the PCM stream would result in a deadlock; the
tasklet completion might change this), and checks urb-status only to
detect if the stream should be stopped (because of a 'normal' stop or
unplugging.)  So (2) or (3) (no matter which) is preferrable over (1).

 For (1), the only way to recover is to submit an URB with URB_ISO_ASAP
 set.  This is essentially the same as shutting down the stream and
 restarting it.

 For (2) and (3), the stream's next time slot value would be updated
 in the usual way.  For example, if 10 slots had expired and the driver
 was submitting URBs containing 4 packets each, then the first and
 second URBs would complete right away with errors, and the first two
 packets of the third URB would get errors, but the last two packets of
 the third URB would be assigned to the two upcoming time slots and
 would be treated normally.  Thus recovery would be automatic,

This is certainly preferrable over (1).

 at the cost of wasting two URBs.  Since we expect underruns to be
 rare, maybe this is acceptable.

And it's the only recovery mechanism that can preserves the timing/
synchronization of the overall stream.


Regards,
Clemens
--
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: Sound Blaster USB X-Fi configuration problem

[Clemens Ladisch]

Mariusz Grecki wrote:
 Does anybody have any idea what can be the problem? The card is
 recognized as HS device by MS Windows (XP version) without any exceptions...

When you say Windows, do you mean Windows itself, or a driver from
Creative that you installed and that can send some vendor-specific
initialization command?


Regards,
Clemens
--
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: BUG: USB audio discontinuities with 'UHCI: implement new semantics for URB_ISO_ASAP'

[Clemens Ladisch]

Alan Stern wrote:
 It is now unnecessary to set urb-start_frame; in fact that field is
 now output-only.

 (To be fair, I haven't checked _all_ the HCDs in this regard, just
 uhci-hcd, ohci-hcd, and ehci-hcd.  However, if any other HCD requires
 urb-start_frame to be set then that HCD should be considered buggy and
 should be fixed.)

That would be fhci, imx21, isp1362, and musb.  The other HCDs (including
xhci) often have obsolete comments of the form always assume
URB_ISO_ASAP and do not implement the new semantics either.


Regards,
Clemens
--
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: BUG: USB audio discontinuities with 'UHCI: implement new semantics for URB_ISO_ASAP'

[Clemens Ladisch]

Alan Stern wrote:
 On Fri, 19 Apr 2013, Joe Rayhawk wrote:
 On Fri, Apr 19, 2013 at 02:18:24PM -0400, Alan Stern wrote:
 On Fri, 19 Apr 2013, Clemens Ladisch wrote:
 Alan Stern wrote:
 + next = uhci-frame_number + 2;

 That 2 is the minimum latency, in frames (one frame per ms).

 One frame worked fine with the old driver.  What is the reason for
 this regression?

 Perhaps that was a mistake.  Joe, you can try changing the 2 above to a
 1 to see if it fixes the problem.

 Hey, that worked great! Audio's coming through continuously, now.

 This change could be added to the driver, but I would prefer not to.

Why do you think it is necessary to have a minimum latency of 2 ms?

Again, the old algorithm worked fine.  While such short queues are not
used by default, they are necessary to get low latencies for real-time
audio applications.  Keeping this change would keep this regression for
quite a few people.

 In any case, it would be best

What criteria are you using to evaluate the benefit of this?  Do you
want to reduce the chance of queue underruns?  Interrupts?  Power usage?

 if the usb-audio driver were changed to keep the pipeline length at
 least 2 ms at all times.

Why is having a queue of two URB with one packet each suddenly not
allowed?


Regards,
Clemens
--
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