On Sun, 08 Aug 2021 07:09:31 +0200, Damien Zammit wrote:
This adds a second mixer control to Digidesign Mbox to select between Analog/SPDIF capture.
Users will note that selecting the SPDIF input source automatically switches the clock mode to sync to SPDIF, which is a feature of the hardware.
(You can change the clock source back to internal if you want to capture from spdif but not sync to its clock although this mode is probably not recommended).
Unfortunately, starting the stream resets both modes to Internally clocked and Analog input mode.
Please add your Signed-off-by line. It's mandatory.
About the code change:
@@ -596,31 +596,53 @@ static int snd_xonar_u1_controls_create(struct usb_mixer_interface *mixer)
/* Digidesign Mbox 1 clock source switch (internal/spdif) */
-static int snd_mbox1_switch_get(struct snd_kcontrol *kctl,
struct snd_ctl_elem_value *ucontrol)+static int snd_mbox1_clk_switch_get(struct snd_kcontrol *kctl,
struct snd_ctl_elem_value *ucontrol){
- struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
- struct snd_usb_audio *chip = list->mixer->chip;
- unsigned char buff[3];
You need to wrap the execution with snd_usb_lock_shutdown() / snd_usb_unlock_shutdown().
- /* Read clock source */
- snd_usb_ctl_msg(chip->dev,
usb_rcvctrlpipe(chip->dev, 0), 0x81,USB_DIR_IN |USB_TYPE_CLASS |USB_RECIP_ENDPOINT, 0x100, 0x81, buff, 3);
Please try to align the indent level. And, the execution error check is missing.
Also, I believe it'd be wise to make it a helper function, as this is used in another place, too. Ditto for other snd_usb_ctl_msg() calls, too.
+static int snd_mbox1_src_switch_get(struct snd_kcontrol *kctl,
struct snd_ctl_elem_value *ucontrol)+{
- struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
- struct snd_usb_audio *chip = list->mixer->chip;
- unsigned char source[1];
Missing snd_usb_lock_shutdown() / *_unlock_*() here, too.
- /* Read input source */
- snd_usb_ctl_msg(chip->dev,
usb_rcvctrlpipe(chip->dev, 0), 0x81,USB_DIR_IN |USB_TYPE_CLASS |USB_RECIP_INTERFACE, 0x00, 0x500, source, 1);
Check the error.
- kctl->private_value = source[0] - 1;
The value returned from the hardware isn't reliable. Check whether it's a valid value.
- ucontrol->value.enumerated.item[0] = kctl->private_value;
- return 0;
+}
+static int snd_mbox1_src_switch_update(struct usb_mixer_interface *mixer, int val) +{
- struct snd_usb_audio *chip = mixer->chip;
- int err;
- unsigned char buff[3];
- unsigned char source[1];
- err = snd_usb_lock_shutdown(chip);
- if (err < 0)
return err;- /* Read input source */
- err = snd_usb_ctl_msg(chip->dev,
usb_rcvctrlpipe(chip->dev, 0), 0x81,USB_DIR_IN |USB_TYPE_CLASS |USB_RECIP_INTERFACE, 0x00, 0x500, source, 1);- if (err < 0)
goto err;- /* 2 possibilities: ANALOG Source -> 0x01
* S/PDIF Source -> 0x02* Setting the input source to S/PDIF resets the clock source to S/PDIF*/- source[0] = (val + 1) & 0xff;
Here better to trust private_value is a valid value, and you can drop the superfluous "& 0xff". It's cut down anyway.
thanks,
Takashi