snd_uaudio: provide information about the device name and attached driver

[christos]

Summary

Unlike the other sound drivers, snd_uaudio(4) doesn't provide
information about the device's description and the driver it's attached
to. A side-effect of this is that applications such as mixer(8), that
fetch these strings through the OSS API's SNDCTL_CARDINFO ioctl will
show a USB audio device as:

pcm0:mixer: <USB Audio> at ? kld snd_uaudio

This patch replaces the generic "USB Audio" description with the
device's actual manufacturer and product strings, and the "at ?" string
with the driver it's attached to:

pcm0:mixer: <Focusrite Scarlett Solo USB> at uaudio0 kld snd_uaudio

Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks

[markj]

There is also device_set_usb_desc(), which is called during attach by many USB drivers. Perhaps we should use that instead?

[christos]

In D43347#987928, @markj wrote:

There is also device_set_usb_desc(), which is called during attach by many USB drivers. Perhaps we should use that instead?

device_set_usb_desc() is called during uaudio_attach(), so we could call device_get_desc(), but it doesn't provide the manufacturer name as well, which I thought would be nice to have. Perhaps we could modify device_set_usb_desc() to also include the manufacturer string in the device_set_desc_copy() call.

[markj]

In D43347#987932, @christos wrote:

In D43347#987928, @markj wrote:

There is also device_set_usb_desc(), which is called during attach by many USB drivers. Perhaps we should use that instead?

device_set_usb_desc() is called during uaudio_attach(), so we could call device_get_desc(), but it doesn't provide the manufacturer name as well, which I thought would be nice to have. Perhaps we could modify device_set_usb_desc() to also include the manufacturer string in the device_set_desc_copy() call.

The fallback in usb_devinfo() does provide the manufacturer name. The normal case lets the device identify itself. That may or may not include the manufacturer name already though, so if you add it we might end up printing it out twice.

And I do seem to get manufacturer strings already?

ugen1.1: <AMD XHCI root HUB> at usbus1
ugen3.1: <AMD XHCI root HUB> at usbus3
ugen2.1: <AMD XHCI root HUB> at usbus2
ugen0.1: <AMD XHCI root HUB> at usbus0
ugen2.2: <Asmedia ASM107x> at usbus2
ugen2.3: <Asmedia ASM107x> at usbus2
ugen0.2: <CM Storm Side print> at usbus0
ugen0.3: <MSI MYSTIC LIGHT> at usbus0
ugen0.4: <MediaTek Inc. WirelessDevice> at usbus0

What output do you get from device_set_usb_desc() for this device?

[christos]

And I do seem to get manufacturer strings already?

ugen1.1: <AMD XHCI root HUB> at usbus1
ugen3.1: <AMD XHCI root HUB> at usbus3
ugen2.1: <AMD XHCI root HUB> at usbus2
ugen0.1: <AMD XHCI root HUB> at usbus0
ugen2.2: <Asmedia ASM107x> at usbus2
ugen2.3: <Asmedia ASM107x> at usbus2
ugen0.2: <CM Storm Side print> at usbus0
ugen0.3: <MSI MYSTIC LIGHT> at usbus0
ugen0.4: <MediaTek Inc. WirelessDevice> at usbus0

These are printed in usb_alloc_device(), which manually prints both the manufacturer and product strings as well:

	/* Announce device */
	printf("%s: <%s %s> at %s\n", udev->ugen_name,
	    usb_get_manufacturer(udev), usb_get_product(udev),
	    device_get_nameunit(udev->bus->bdev));

What output do you get from device_set_usb_desc() for this device?

Just the product string:

uaudio0: <Scarlett Solo USB> on usbus0

[imp]

sys/dev/sound/usb/uaudio.c

1174

128 looks very magic...
And where we're called from, you'd be better off using an sbuf and not imposing arbitrary limits or wasting the stack...

[markj]

sys/dev/sound/usb/uaudio.c

1174

I think what we really want is a version of device_set_desc() which takes a format string and parameters, so that one can just write:

device_set_descf("%s %s", usb_get_manufacturer(...), ...);

An sbuf is overkill for this particular case IMO.

But I don't think the use of a magic buffer here is a problem. Basically all of the device_set_desc_copy() calls in the tree have the same thing. My proposal would eliminate them.

[christos]

sys/dev/sound/usb/uaudio.c

1174

128 looks very magic...
And where we're called from, you'd be better off using an sbuf and not imposing arbitrary limits or wasting the stack...

@imp, as @markj pointed out, 128 is what most other audio drivers seem to be using for the description buffer.

What I'd propose is either a SNDCTL_DESCLEN constant in sound.h, similar to SNDCTL_STATUSLEN, and use that instead of hardcoding 128 (or whatever else) everywhere, or do what Mark suggests and implement a printf-like device_set_desc() to avoid dealing with buffers altogether. I can take care of this if we agree to go with the latter.

[christos]

sys/dev/sound/usb/uaudio.c

1174

s/SNDCTL/SND/

[grahamperrin]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271574 updated.