[alsa-devel] [PATCH] sound: usb-audio: full capture/playback/spdif support for Digidesign Mbox 2

[Takashi Iwai]

At Sun, 16 Dec 2012 22:06:25 +1100,
Damien Zammit wrote:
> 
> Support for Digidesign Mbox 2 USB sound card:
> 
> This patch is the result of a lot of trial and error, since there are no specs
> available for the device.
> 
> Full duplex support is provided, i.e. playback and recording in stereo.
> The format is hardcoded at 48000Hz @ 24 bit, which is the maximum that the
> device supports.  Also, MIDI in and MIDI out both work.
> 
> Users will notice that the S/PDIF light also flashes when playback or recording
> is active.  I believe this means that S/PDIF input/output is simultaneously
> activated with the analogue i/o during use.
> But this particular functionality remains untested.
> 
> Note that this particular version of the patch is so far untested on the
> physical hardware because I have not compiled a full kernel with the changes.
> However, extensive testing has been done by many users of the hardware
> who believe other versions of my patch have worked since circa 2009.
> 
> Signed-off-by: Damien Zammit <damien at zamaudio.com>

Thanks for the revised patch.
It looks almost good, but just a few things below:

> @@ -761,6 +766,108 @@ void snd_usb_set_format_quirk(struct snd_usb_substream *subs,
>  	}
>  }
>  
> +void mbox2_setup_48_24_magic(struct usb_device *dev)

This doesn't have to be global.  Make it static.

> +int snd_usb_mbox2_boot_quirk(struct usb_device *dev)
> +{
> +	struct usb_host_config *config = dev->actconfig;
> +	int err;
> +	u8 enablemagic[3];
> +	u8 bootresponse;
> +	u8 temp[12];

enablemagic[] and temp[] aren't used in this function.
Remove them.

> +	if (bootresponse != MBOX2_BOOT_READY) {
> +		snd_printk(KERN_ERR "usb-audio: Unknown bootresponse=%d, or timed out, ignoring device.\n", bootresponse);
> +		return -ENODEV;
> +	}
> +
> +	snd_printd("usb-audio: device initialised!\n");

I'd say this could be snd_printdd() (as you'll see anyway the message
below at each time), and...

> +	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0,
> +		&dev->descriptor, sizeof(dev->descriptor));
> +	config = dev->actconfig;
> +	if (err < 0)
> +		snd_printdd("error usb_get_descriptor: %d\n", err);

... make it to snd_printd(), as this is good to know for debugging,


> +	err = usb_reset_configuration(dev);
> +	if (err < 0)
> +		snd_printdd("error usb_reset_configuration: %d\n", err);

... ditto, make snd_printd().

> +	snd_printdd("mbox2_boot: new boot length = %d\n",
> +		le16_to_cpu(get_cfg_desc(config)->wTotalLength));
> +
> +	mbox2_setup_48_24_magic(dev);
> +
> +	snd_printk(KERN_INFO "usb-audio: Digidesign Mbox 2: 24bit 48kHz");
> +
> +	return 0; /* Successful boot */
> +}
> +
> +int mbox2_skip_setting_quirk(struct snd_usb_audio *chip,
> +					int iface, int altno)

Where is this function called?  I don't see the caller in your latest
patch.


thanks,

Takashi