* [PATCH] usb-audio: Input source control - digidesign mbox
@ 2021-08-08 5:09 Damien Zammit
2021-08-11 15:26 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: Damien Zammit @ 2021-08-08 5:09 UTC (permalink / raw)
To: alsa-devel; +Cc: Damien Zammit
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.
---
sound/usb/mixer_quirks.c | 197 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 175 insertions(+), 22 deletions(-)
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index 0a3cb8fd7..ef771bb3e 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -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];
+
+ /* 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);
+
+ if (!buff[0] && !buff[1] && !buff[2]) {
+ /* SPDIF synced */
+ kctl->private_value = 1;
+ } else {
+ /* Internally clocked */
+ kctl->private_value = 0;
+ }
+
ucontrol->value.enumerated.item[0] = kctl->private_value;
return 0;
}
-static int snd_mbox1_switch_update(struct usb_mixer_interface *mixer, int val)
+static int snd_mbox1_clk_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;
- /* Prepare for magic command to toggle clock source */
+ /* 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, buff, 1);
+ USB_RECIP_INTERFACE, 0x00, 0x500, source, 1);
if (err < 0)
goto err;
+
+ /* Read clock source */
err = snd_usb_ctl_msg(chip->dev,
usb_rcvctrlpipe(chip->dev, 0), 0x81,
USB_DIR_IN |
@@ -644,13 +666,15 @@ static int snd_mbox1_switch_update(struct usb_mixer_interface *mixer, int val)
buff[0] = buff[1] = buff[2] = 0x00;
}
- /* Send the magic command to toggle the clock source */
+ /* Toggle clock source */
err = snd_usb_ctl_msg(chip->dev,
usb_sndctrlpipe(chip->dev, 0), 0x1,
USB_TYPE_CLASS |
USB_RECIP_ENDPOINT, 0x100, 0x81, buff, 3);
if (err < 0)
goto err;
+
+ /* Read clock source */
err = snd_usb_ctl_msg(chip->dev,
usb_rcvctrlpipe(chip->dev, 0), 0x81,
USB_DIR_IN |
@@ -671,8 +695,8 @@ static int snd_mbox1_switch_update(struct usb_mixer_interface *mixer, int val)
return err;
}
-static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
- struct snd_ctl_elem_value *ucontrol)
+static int snd_mbox1_clk_switch_put(struct snd_kcontrol *kctl,
+ struct snd_ctl_elem_value *ucontrol)
{
struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
struct usb_mixer_interface *mixer = list->mixer;
@@ -685,12 +709,12 @@ static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
return 0;
kctl->private_value = new_val;
- err = snd_mbox1_switch_update(mixer, new_val);
+ err = snd_mbox1_clk_switch_update(mixer, new_val);
return err < 0 ? err : 1;
}
-static int snd_mbox1_switch_info(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_info *uinfo)
+static int snd_mbox1_clk_switch_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
{
static const char *const texts[2] = {
"Internal",
@@ -700,27 +724,156 @@ static int snd_mbox1_switch_info(struct snd_kcontrol *kcontrol,
return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
}
-static int snd_mbox1_switch_resume(struct usb_mixer_elem_list *list)
+static int snd_mbox1_clk_switch_resume(struct usb_mixer_elem_list *list)
+{
+ return snd_mbox1_clk_switch_update(list->mixer, list->kctl->private_value);
+}
+
+/* Digidesign Mbox 1 input source switch (analog/spdif) */
+
+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];
+
+ /* 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);
+
+ kctl->private_value = source[0] - 1;
+ 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;
+
+ /* Toggle the input source */
+ err = snd_usb_ctl_msg(chip->dev,
+ usb_sndctrlpipe(chip->dev, 0), 0x1,
+ USB_TYPE_CLASS |
+ USB_RECIP_INTERFACE, 0x00, 0x500, source, 1);
+ if (err < 0)
+ goto 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;
+
+ /* Read clock source */
+ err = 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);
+ if (err < 0)
+ goto err;
+err:
+ snd_usb_unlock_shutdown(chip);
+ return err;
+}
+
+static int snd_mbox1_src_switch_put(struct snd_kcontrol *kctl,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
+ struct usb_mixer_interface *mixer = list->mixer;
+ int err;
+ bool cur_val, new_val;
+
+ cur_val = kctl->private_value;
+ new_val = ucontrol->value.enumerated.item[0];
+ if (cur_val == new_val)
+ return 0;
+
+ kctl->private_value = new_val;
+ err = snd_mbox1_src_switch_update(mixer, new_val);
+ return err < 0 ? err : 1;
+}
+
+static int snd_mbox1_src_switch_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char *const texts[2] = {
+ "Analog",
+ "S/PDIF"
+ };
+
+ return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
+}
+
+static int snd_mbox1_src_switch_resume(struct usb_mixer_elem_list *list)
{
- return snd_mbox1_switch_update(list->mixer, list->kctl->private_value);
+ return snd_mbox1_src_switch_update(list->mixer, list->kctl->private_value);
}
-static const struct snd_kcontrol_new snd_mbox1_switch = {
+static const struct snd_kcontrol_new snd_mbox1_clk_switch = {
.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
.name = "Clock Source",
.index = 0,
.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
- .info = snd_mbox1_switch_info,
- .get = snd_mbox1_switch_get,
- .put = snd_mbox1_switch_put,
+ .info = snd_mbox1_clk_switch_info,
+ .get = snd_mbox1_clk_switch_get,
+ .put = snd_mbox1_clk_switch_put,
+ .private_value = 0
+};
+
+static const struct snd_kcontrol_new snd_mbox1_src_switch = {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Input Source",
+ .index = 1,
+ .access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
+ .info = snd_mbox1_src_switch_info,
+ .get = snd_mbox1_src_switch_get,
+ .put = snd_mbox1_src_switch_put,
.private_value = 0
};
-static int snd_mbox1_create_sync_switch(struct usb_mixer_interface *mixer)
+static int snd_mbox1_controls_create(struct usb_mixer_interface *mixer)
{
- return add_single_ctl_with_resume(mixer, 0,
- snd_mbox1_switch_resume,
- &snd_mbox1_switch, NULL);
+ int err;
+ err = add_single_ctl_with_resume(mixer, 0,
+ snd_mbox1_clk_switch_resume,
+ &snd_mbox1_clk_switch, NULL);
+ if (err < 0)
+ return err;
+
+ return add_single_ctl_with_resume(mixer, 1,
+ snd_mbox1_src_switch_resume,
+ &snd_mbox1_src_switch, NULL);
}
/* Native Instruments device quirks */
@@ -3029,7 +3182,7 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
break;
case USB_ID(0x0dba, 0x1000): /* Digidesign Mbox 1 */
- err = snd_mbox1_create_sync_switch(mixer);
+ err = snd_mbox1_controls_create(mixer);
break;
case USB_ID(0x17cc, 0x1011): /* Traktor Audio 6 */
--
2.13.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] usb-audio: Input source control - digidesign mbox
2021-08-08 5:09 [PATCH] usb-audio: Input source control - digidesign mbox Damien Zammit
@ 2021-08-11 15:26 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2021-08-11 15:26 UTC (permalink / raw)
To: Damien Zammit; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-08-11 15:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-08 5:09 [PATCH] usb-audio: Input source control - digidesign mbox Damien Zammit
2021-08-11 15:26 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.