On Wed, 27 Feb 2019 15:26:01 +0100,
Manuel Reinhardt wrote:
>
> Add an entry to the quirks-table to for usb-audio to recognize the
> Microbook II (although it only exposes vendor interfaces). A simple boot
> quirk is also implemented to set up the sample rate and make sure that
> no audio urbs are sent before the device is ready.
>
> This patch only provides audio playback and capture at 96kHz sample
> rate. Notice the following shortcomings:
>
> - The sample rate is currently hardcoded to 96k although the device also
> supports 48k and 44.1k.
>
> - The various mixer controls of the MicroBook are not made available.
>
> - The keep-iface control should be on by default because the device
> shuts down whenever the altsetting is reset which is usually unwanted.
> (I don't know the best way to do this)
Can be set either in the quirk callback you write.
Or it'd be not bad to create a white list, too.
> +static int snd_usb_motu_microbookii_boot_quirk(struct usb_device *dev)
> +{
> + int err, actual_length, poll_attempts = 0;
> + bool setup_finished = false;
> + static const u8 set_samplerate_seq[] = { 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x0b, 0x14,
> + 0x00, 0x00, 0x00, 0x01 };
> + static const u8 poll_ready_seq[] = { 0x00, 0x04, 0x00, 0x00,
> + 0x00, 0x00, 0x0b, 0x18 };
> + u8 *buf = kzalloc(MICROBOOK_BUF_SIZE, GFP_KERNEL);
> +
> + if (!buf)
> + return -ENOMEM;
> +
> + dev_info(&dev->dev, "Waiting for MOTU Microbook II to boot up...\n");
> +
> + /* First we tell the device which sample rate to use. */
> + memcpy(buf, set_samplerate_seq, ARRAY_SIZE(set_samplerate_seq));
This is about byte-size, so better to use sizeof().
ARRAY_SIZE() confuses readers as if he needs to care about the
type.
> + actual_length = ARRAY_SIZE(set_samplerate_seq);
Ditto here. Also some other codes below, too.
> + err = snd_usb_motu_microbookii_communicate(dev, buf, MICROBOOK_BUF_SIZE,
> + &actual_length);
> +
> + if (err < 0) {
> + kfree(buf);
> + dev_err(&dev->dev,
> + "failed setting the sample rate for Motu MicroBook II: %d\n",
> + err);
> + return err;
Since kfree() is mandatory in the error path, it's better to use goto
and kfree() there, instead of spreading over all places.
> + }
> +
> + /* Then we poll every 100 ms until the device informs of its readiness. */
> + while (!setup_finished) {
> + if (++poll_attempts > 100) {
> + kfree(buf);
> + dev_err(&dev->dev,
> + "failed booting Motu MicroBook II: timeout\n");
> + return -ENODEV;
> + }
> +
> + memset(buf, 0, MICROBOOK_BUF_SIZE);
> + memcpy(buf, poll_ready_seq, ARRAY_SIZE(poll_ready_seq));
> +
> + actual_length = ARRAY_SIZE(poll_ready_seq);
> + err = snd_usb_motu_microbookii_communicate(
> + dev, buf, MICROBOOK_BUF_SIZE, &actual_length);
> + if (err < 0) {
> + kfree(buf);
> + dev_err(&dev->dev,
> + "failed booting Motu MicroBook II: communication error\n");
> + return err;
> + }
> +
> + if (buf[actual_length - 1] == 1)
> + setup_finished = true;
I wouldn't trust the returned value. Let's be paranoid and check
whether the actual_length is within the expected range.
> +
> + msleep(100);
BTW, setup_finished is superfluous. You can just write like
msleep(100);
if (buf[actual_lenght - 1] == 1)
break;
Or if 100ms sleep isn't needed in the successful case, it can be
if (buf[actual_lenght - 1] == 1)
break;
msleep(100);
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
