On Thu, 04 Oct 2018 21:17:43 +0200, Jussi Laako wrote:
Adds several vendor specific mixer quirks for RME's Class Compliant USB devices. These provide extra status information from the device otherwise not available.
These include AES/SPDIF rate and status information, current system sampling rate and measured frequency. This information is especially useful in cases where device's clock is slaved to external clock source.
Signed-off-by: Jussi Laako jussi@sonarnerd.net
The implementations look mostly OK, but just some nitpicking:
sound/usb/mixer_quirks.c | 349 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 349 insertions(+)
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index cbfb48bdea51..d75e1f025b3b 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -27,6 +27,8 @@
- Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#include <asm/div64.h>
Rather include <linux/math64.h>, and put in the appropriate order.
#include <linux/hid.h> #include <linux/init.h> #include <linux/slab.h> @@ -1817,6 +1819,347 @@ static int dell_dock_mixer_init(struct usb_mixer_interface *mixer) return 0; }
+/* RME Class Compliant device quirks */
+static const u32 snd_rme_rate_table[] = {
- 32000, 44100, 48000, 50000,
- 64000, 88200, 96000, 100000,
- 128000, 176400, 192000, 200000,
- 256000, 352800, 384000, 400000,
- 512000, 705600, 768000, 800000
+};
+static int snd_rme_get_status1(struct snd_kcontrol *kcontrol,
u32 *status1)+{
- struct usb_mixer_elem_list *list = snd_kcontrol_chip(kcontrol);
- struct snd_usb_audio *chip = list->mixer->chip;
- struct usb_device *dev = chip->dev;
- int err;
- err = snd_usb_lock_shutdown(chip);
- if (err < 0)
return err;- err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
23, /* GET_STATUS1 */
This number (and the other later ones) should be defined instead of putting in each commit.
USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,0, 0,status1, sizeof(*status1));- if (err < 0) {
dev_err(&dev->dev,"unable to issue vendor read request (ret = %d)", err);goto end;- }
Since you're calling in the same pattern, it's better to consider to provide a helper to call snd_usb_ctl_msg() with the given type (and shows the error there). The helper can take snd_usb_lock_shutdown() and unlock in it, so it'll simplify a lot.
+end:
- snd_usb_unlock_shutdown(chip);
- return err;
+}
+static int snd_rme_rate_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- u32 status1;
- u32 rate = 0;
- int idx;
- int err;
- err = snd_rme_get_status1(kcontrol, &status1);
- if (err < 0)
return err;- switch (kcontrol->private_value)
- {
- case 0: /* System */
Let's use enum.
idx = (status1 >> 16) & 0x1f;
And, define this magic number 16.
if (idx < ARRAY_SIZE(snd_rme_rate_table))rate = snd_rme_rate_table[idx];break;- case 1: /* AES */
idx = (status1 >> 8) & 0xf;
Ditto (also for the rest).
+static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- u32 status1;
- int idx = 0;
- int err;
- err = snd_rme_get_status1(kcontrol, &status1);
- if (err < 0)
return err;- switch (kcontrol->private_value)
- {
- case 1: /* AES */
if (status1 & 0x4)idx = 2;else if (status1 & 0x1)idx = 1;
All these magic numbers should be defined as well.
+static int snd_rme_current_freq_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct usb_mixer_elem_list *list = snd_kcontrol_chip(kcontrol);
- struct snd_usb_audio *chip = list->mixer->chip;
- struct usb_device *dev = chip->dev;
- u32 status1;
- const u64 num = 104857600000000;
The 64bit const should be with ULL suffix.
+static int snd_rme_rate_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)+{
- uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
- uinfo->count = 1;
- uinfo->value.integer.min =
(kcontrol->private_value > 0) ? 0 : 32000;- uinfo->value.integer.max =
(kcontrol->private_value > 0) ? 200000 : 800000;
This better to be like switch (kcontrol->private_value) { case FOO: uinfo->value.integer.min = 0; uinfo->value.integer.min = 200000; break; case BAR: uinfo->value.integer.min = 32000; uinfo->value.integer.min = 800000; break; }
thanks,
Takashi