[PATCH v2 0/4] RME HDSP fixes

[PATCH v2 0/4] RME HDSP fixes

[Adrian Knoth]

Hi,

here's v2 of my previous series of patches for the RME HDSP.

Changes to the previous version:
   - correct time value in verbose output ("iobox found after %d ms")
   - define and use constants for iobox detection (no more 0x10a0)
   - hdsp_toggle_setting: only change register if newval != oldval

Adrian Knoth (4):
  ALSA: hdsp - Fix detection for RME RPM/Multiface/Digiface ioboxes
  ALSA: hdsp - Implement generic function to toggle settings
  ALSA: hdsp - Use HDSP_TOGGLE_SETTING to alter settings
  ALSA: hdsp - Remove obsolete settings functions

 sound/pci/rme9652/hdsp.c |  461 ++++++++++++----------------------------------
 1 file changed, 116 insertions(+), 345 deletions(-)

-- 
1.7.10.4

[PATCH 1/4] ALSA: hdsp - Fix detection for RME RPM/Multiface/Digiface ioboxes

[Adrian Knoth]

The current iobox detection code reportedly fails for various users, so
simply do what the Win32 driver does instead.

Patch originally by Karl Grill <kgrill@chello.at> and then modified to
comply with kernel coding guidelines + current HEAD.

Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index 4fae81f..ba190d5 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -154,10 +154,12 @@ MODULE_FIRMWARE("digiface_firmware_rev11.bin");
 #define HDSP_BIGENDIAN_MODE     0x200
 #define HDSP_RD_MULTIPLE        0x400
 #define HDSP_9652_ENABLE_MIXER  0x800
+#define HDSP_S200		0x800
+#define HDSP_CYCLIC_MODE	0x1000
 #define HDSP_TDO                0x10000000
 
-#define HDSP_S_PROGRAM     	(HDSP_PROGRAM|HDSP_CONFIG_MODE_0)
-#define HDSP_S_LOAD		(HDSP_PROGRAM|HDSP_CONFIG_MODE_1)
+#define HDSP_S_PROGRAM	    (HDSP_CYCLIC_MODE|HDSP_PROGRAM|HDSP_CONFIG_MODE_0)
+#define HDSP_S_LOAD	    (HDSP_CYCLIC_MODE|HDSP_PROGRAM|HDSP_CONFIG_MODE_1)
 
 /* Control Register bits */
 
@@ -671,13 +673,23 @@ static unsigned int hdsp_read(struct hdsp *hdsp, int reg)
 
 static int hdsp_check_for_iobox (struct hdsp *hdsp)
 {
+	int i;
+
 	if (hdsp->io_type == H9652 || hdsp->io_type == H9632) return 0;
-	if (hdsp_read (hdsp, HDSP_statusRegister) & HDSP_ConfigError) {
-		snd_printk("Hammerfall-DSP: no IO box connected!\n");
-		hdsp->state &= ~HDSP_FirmwareLoaded;
-		return -EIO;
+	for (i = 0; i < 500; i++) {
+		if (0 == (hdsp_read(hdsp, HDSP_statusRegister) &
+					HDSP_ConfigError)) {
+			if (i) {
+				snd_printk("Hammerfall-DSP: IO box found after %d ms\n",
+						(20 * i));
+			}
+			return 0;
+		}
+		msleep(20);
 	}
-	return 0;
+	snd_printk("Hammerfall-DSP: no IO box connected!\n");
+	hdsp->state &= ~HDSP_FirmwareLoaded;
+	return -EIO;
 }
 
 static int hdsp_wait_for_iobox(struct hdsp *hdsp, unsigned int loops,
@@ -728,6 +740,7 @@ static int snd_hdsp_load_firmware_from_cache(struct hdsp *hdsp) {
 
 		if (hdsp_fifo_wait (hdsp, 0, HDSP_LONG_WAIT)) {
 			snd_printk ("Hammerfall-DSP: timeout waiting for download preparation\n");
+			hdsp_write(hdsp, HDSP_control2Reg, HDSP_S200);
 			return -EIO;
 		}
 
@@ -737,17 +750,15 @@ static int snd_hdsp_load_firmware_from_cache(struct hdsp *hdsp) {
 			hdsp_write(hdsp, HDSP_fifoData, cache[i]);
 			if (hdsp_fifo_wait (hdsp, 127, HDSP_LONG_WAIT)) {
 				snd_printk ("Hammerfall-DSP: timeout during firmware loading\n");
+				hdsp_write(hdsp, HDSP_control2Reg, HDSP_S200);
 				return -EIO;
 			}
 		}
 
-		ssleep(3);
-
-		if (hdsp_fifo_wait (hdsp, 0, HDSP_LONG_WAIT)) {
-			snd_printk ("Hammerfall-DSP: timeout at end of firmware loading\n");
-		    	return -EIO;
-		}
+		hdsp_fifo_wait(hdsp, 3, HDSP_LONG_WAIT);
+		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S200);
 
+		ssleep(3);
 #ifdef SNDRV_BIG_ENDIAN
 		hdsp->control2_register = HDSP_BIGENDIAN_MODE;
 #else
@@ -773,24 +784,51 @@ static int hdsp_get_iobox_version (struct hdsp *hdsp)
 {
 	if ((hdsp_read (hdsp, HDSP_statusRegister) & HDSP_DllError) != 0) {
 
-		hdsp_write (hdsp, HDSP_control2Reg, HDSP_PROGRAM);
-		hdsp_write (hdsp, HDSP_fifoData, 0);
-		if (hdsp_fifo_wait (hdsp, 0, HDSP_SHORT_WAIT) < 0)
-			return -EIO;
+		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S_LOAD);
+		hdsp_write(hdsp, HDSP_fifoData, 0);
 
-		hdsp_write (hdsp, HDSP_control2Reg, HDSP_S_LOAD);
+		if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT) < 0) {
+			hdsp_write(hdsp, HDSP_control2Reg, 0x100 | HDSP_S200);
+			hdsp_write(hdsp, HDSP_control2Reg, HDSP_S_LOAD);
+		}
+
+		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S200 | HDSP_PROGRAM);
 		hdsp_write (hdsp, HDSP_fifoData, 0);
+		if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT) < 0) {
+			hdsp->io_type = Multiface;
+			snd_printk("Hammerfall-DSP: Multiface found\n");
+			return 0;
+		}
 
-		if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT)) {
-			hdsp_write(hdsp, HDSP_control2Reg, HDSP_VERSION_BIT);
-			hdsp_write(hdsp, HDSP_control2Reg, HDSP_S_LOAD);
-			if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT))
-				hdsp->io_type = RPM;
-			else
-				hdsp->io_type = Multiface;
-		} else {
+		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S_LOAD);
+		hdsp_write(hdsp, HDSP_fifoData, 0);
+		if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT) == 0) {
 			hdsp->io_type = Digiface;
+			snd_printk("Hammerfall-DSP: Digiface found\n");
+			return 0;
 		}
+
+		hdsp_write(hdsp, HDSP_control2Reg, 0x100 | HDSP_S200);
+		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S_LOAD);
+		hdsp_write(hdsp, HDSP_fifoData, 0);
+		if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT) == 0) {
+			hdsp->io_type = Multiface;
+			snd_printk("Hammerfall-DSP: Multiface found\n");
+			return 0;
+		}
+
+		hdsp_write(hdsp, HDSP_control2Reg, 0x100 | HDSP_S200);
+		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S_LOAD);
+		hdsp_write(hdsp, HDSP_fifoData, 0);
+		if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT) < 0) {
+			hdsp->io_type = Multiface;
+			snd_printk("Hammerfall-DSP: Multiface found\n");
+			return 0;
+		}
+
+		hdsp->io_type = RPM;
+		snd_printk("Hammerfall-DSP: RPM found\n");
+		return 0;
 	} else {
 		/* firmware was already loaded, get iobox type */
 		if (hdsp_read(hdsp, HDSP_status2Register) & HDSP_version2)
-- 
1.7.10.4

[PATCH 2/4] ALSA: hdsp - Implement generic function to toggle settings

[Adrian Knoth]

The driver contains multiple similar functions that change only a single
bit in the control register, only the bit position varies.

This patch implements a generic function to toggle a certain bit
position that will be used to replace the old code.

Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index ba190d5..cdc2b20 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -1712,6 +1712,65 @@ static int snd_hdsp_put_spdif_in(struct snd_kcontrol *kcontrol, struct snd_ctl_e
 	return change;
 }
 
+#define HDSP_TOGGLE_SETTING(xname, xindex) \
+{   .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
+	.name = xname, \
+	.private_value = xindex, \
+	.info = snd_hdsp_info_toggle_setting, \
+	.get = snd_hdsp_get_toggle_setting, \
+	.put = snd_hdsp_put_toggle_setting \
+}
+
+static int hdsp_toggle_setting(struct hdsp *hdsp, u32 regmask)
+{
+	return (hdsp->control_register & regmask) ? 1 : 0;
+}
+
+static int hdsp_set_toggle_setting(struct hdsp *hdsp, u32 regmask, int out)
+{
+	if (out)
+		hdsp->control_register |= regmask;
+	else
+		hdsp->control_register &= ~regmask;
+	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
+
+	return 0;
+}
+
+#define snd_hdsp_info_toggle_setting		   snd_ctl_boolean_mono_info
+
+static int snd_hdsp_get_toggle_setting(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
+	u32 regmask = kcontrol->private_value;
+
+	spin_lock_irq(&hdsp->lock);
+	ucontrol->value.integer.value[0] = hdsp_toggle_setting(hdsp, regmask);
+	spin_unlock_irq(&hdsp->lock);
+	return 0;
+}
+
+static int snd_hdsp_put_toggle_setting(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
+	u32 regmask = kcontrol->private_value;
+	int change;
+	unsigned int val;
+
+	if (!snd_hdsp_use_is_exclusive(hdsp))
+		return -EBUSY;
+	val = ucontrol->value.integer.value[0] & 1;
+	spin_lock_irq(&hdsp->lock);
+	change = (int) val != hdsp_toggle_setting(hdsp, regmask);
+	if (change)
+		hdsp_set_toggle_setting(hdsp, regmask, val);
+	spin_unlock_irq(&hdsp->lock);
+	return change;
+}
+
+
 #define HDSP_SPDIF_OUT(xname, xindex) \
 { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xindex, \
   .info = snd_hdsp_info_spdif_bits, \
-- 
1.7.10.4

[PATCH 3/4] ALSA: hdsp - Use HDSP_TOGGLE_SETTING to alter settings

[Adrian Knoth]

HDSP_TOGGLE_SETTING and its corresponding functions allow to change
settings in the control register. Instead of using the specialised
functions, use the generic code to make the code DRY.

Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index cdc2b20..45a101a 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -3287,7 +3287,7 @@ static struct snd_kcontrol_new snd_hdsp_9632_controls[] = {
 HDSP_DA_GAIN("DA Gain", 0),
 HDSP_AD_GAIN("AD Gain", 0),
 HDSP_PHONE_GAIN("Phones Gain", 0),
-HDSP_XLR_BREAKOUT_CABLE("XLR Breakout Cable", 0),
+HDSP_TOGGLE_SETTING("XLR Breakout Cable", HDSP_XLRBreakoutCable),
 HDSP_DDS_OFFSET("DDS Sample Rate Offset", 0)
 };
 
@@ -3329,10 +3329,10 @@ static struct snd_kcontrol_new snd_hdsp_controls[] = {
 },
 HDSP_MIXER("Mixer", 0),
 HDSP_SPDIF_IN("IEC958 Input Connector", 0),
-HDSP_SPDIF_OUT("IEC958 Output also on ADAT1", 0),
-HDSP_SPDIF_PROFESSIONAL("IEC958 Professional Bit", 0),
-HDSP_SPDIF_EMPHASIS("IEC958 Emphasis Bit", 0),
-HDSP_SPDIF_NON_AUDIO("IEC958 Non-audio Bit", 0),
+HDSP_TOGGLE_SETTING("IEC958 Output also on ADAT1", HDSP_SPDIFOpticalOut),
+HDSP_TOGGLE_SETTING("IEC958 Professional Bit", HDSP_SPDIFProfessional),
+HDSP_TOGGLE_SETTING("IEC958 Emphasis Bit", HDSP_SPDIFEmphasis),
+HDSP_TOGGLE_SETTING("IEC958 Non-audio Bit", HDSP_SPDIFNonAudio),
 /* 'Sample Clock Source' complies with the alsa control naming scheme */
 HDSP_CLOCK_SOURCE("Sample Clock Source", 0),
 {
@@ -3352,7 +3352,7 @@ HDSP_AUTOSYNC_SAMPLE_RATE("External Rate", 0),
 HDSP_WC_SYNC_CHECK("Word Clock Lock Status", 0),
 HDSP_SPDIF_SYNC_CHECK("SPDIF Lock Status", 0),
 HDSP_ADATSYNC_SYNC_CHECK("ADAT Sync Lock Status", 0),
-HDSP_LINE_OUT("Line Out", 0),
+HDSP_TOGGLE_SETTING("Line Out", HDSP_LineOut),
 HDSP_PRECISE_POINTER("Precise Pointer", 0),
 HDSP_USE_MIDI_TASKLET("Use Midi Tasklet", 0),
 };
@@ -3669,7 +3669,9 @@ static struct snd_kcontrol_new snd_hdsp_rpm_controls[] = {
 	HDSP_MIXER("Mixer", 0)
 };
 
-static struct snd_kcontrol_new snd_hdsp_96xx_aeb = HDSP_AEB("Analog Extension Board", 0);
+static struct snd_kcontrol_new snd_hdsp_96xx_aeb =
+	HDSP_TOGGLE_SETTING("Analog Extension Board",
+			HDSP_AnalogExtensionBoard);
 static struct snd_kcontrol_new snd_hdsp_adat_sync_check = HDSP_ADAT_SYNC_CHECK;
 
 static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
@@ -4092,7 +4094,9 @@ snd_hdsp_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
 		}
 		snd_iprintf(buffer, "Phones Gain : %s\n", tmp);
 
-		snd_iprintf(buffer, "XLR Breakout Cable : %s\n", hdsp_xlr_breakout_cable(hdsp) ? "yes" : "no");
+		snd_iprintf(buffer, "XLR Breakout Cable : %s\n",
+			hdsp_toggle_setting(hdsp, HDSP_XLRBreakoutCable) ?
+			"yes" : "no");
 
 		if (hdsp->control_register & HDSP_AnalogExtensionBoard)
 			snd_iprintf(buffer, "AEB : on (ADAT1 internal)\n");
@@ -5123,29 +5127,38 @@ static int snd_hdsp_hwdep_ioctl(struct snd_hwdep *hw, struct file *file, unsigne
 		for (i = 0; i < ((hdsp->io_type != Multiface && hdsp->io_type != RPM && hdsp->io_type != H9632) ? 3 : 1); ++i)
 			info.adat_sync_check[i] = (unsigned char)hdsp_adat_sync_check(hdsp, i);
 		info.spdif_in = (unsigned char)hdsp_spdif_in(hdsp);
-		info.spdif_out = (unsigned char)hdsp_spdif_out(hdsp);
-		info.spdif_professional = (unsigned char)hdsp_spdif_professional(hdsp);
-		info.spdif_emphasis = (unsigned char)hdsp_spdif_emphasis(hdsp);
-		info.spdif_nonaudio = (unsigned char)hdsp_spdif_nonaudio(hdsp);
+		info.spdif_out = (unsigned char)hdsp_toggle_setting(hdsp,
+				HDSP_SPDIFOpticalOut);
+		info.spdif_professional = (unsigned char)
+			hdsp_toggle_setting(hdsp, HDSP_SPDIFProfessional);
+		info.spdif_emphasis = (unsigned char)
+			hdsp_toggle_setting(hdsp, HDSP_SPDIFEmphasis);
+		info.spdif_nonaudio = (unsigned char)
+			hdsp_toggle_setting(hdsp, HDSP_SPDIFNonAudio);
 		info.spdif_sample_rate = hdsp_spdif_sample_rate(hdsp);
 		info.system_sample_rate = hdsp->system_sample_rate;
 		info.autosync_sample_rate = hdsp_external_sample_rate(hdsp);
 		info.system_clock_mode = (unsigned char)hdsp_system_clock_mode(hdsp);
 		info.clock_source = (unsigned char)hdsp_clock_source(hdsp);
 		info.autosync_ref = (unsigned char)hdsp_autosync_ref(hdsp);
-		info.line_out = (unsigned char)hdsp_line_out(hdsp);
+		info.line_out = (unsigned char)
+			hdsp_toggle_setting(hdsp, HDSP_LineOut);
 		if (hdsp->io_type == H9632) {
 			info.da_gain = (unsigned char)hdsp_da_gain(hdsp);
 			info.ad_gain = (unsigned char)hdsp_ad_gain(hdsp);
 			info.phone_gain = (unsigned char)hdsp_phone_gain(hdsp);
-			info.xlr_breakout_cable = (unsigned char)hdsp_xlr_breakout_cable(hdsp);
+			info.xlr_breakout_cable =
+				(unsigned char)hdsp_toggle_setting(hdsp,
+					HDSP_XLRBreakoutCable);
 
 		} else if (hdsp->io_type == RPM) {
 			info.da_gain = (unsigned char) hdsp_rpm_input12(hdsp);
 			info.ad_gain = (unsigned char) hdsp_rpm_input34(hdsp);
 		}
 		if (hdsp->io_type == H9632 || hdsp->io_type == H9652)
-			info.analog_extension_board = (unsigned char)hdsp_aeb(hdsp);
+			info.analog_extension_board =
+				(unsigned char)hdsp_toggle_setting(hdsp,
+					    HDSP_AnalogExtensionBoard);
 		spin_unlock_irqrestore(&hdsp->lock, flags);
 		if (copy_to_user(argp, &info, sizeof(info)))
 			return -EFAULT;
-- 
1.7.10.4

[PATCH 4/4] ALSA: hdsp - Remove obsolete settings functions

[Adrian Knoth]

With HDSP_TOGGLE_SETTING in place, these functions are no longer
required. Removing them makes the code DRY and considerably shorter.

Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index 45a101a..6cce259 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -1770,185 +1770,6 @@ static int snd_hdsp_put_toggle_setting(struct snd_kcontrol *kcontrol,
 	return change;
 }
 
-
-#define HDSP_SPDIF_OUT(xname, xindex) \
-{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xindex, \
-  .info = snd_hdsp_info_spdif_bits, \
-  .get = snd_hdsp_get_spdif_out, .put = snd_hdsp_put_spdif_out }
-
-static int hdsp_spdif_out(struct hdsp *hdsp)
-{
-	return (hdsp->control_register & HDSP_SPDIFOpticalOut) ? 1 : 0;
-}
-
-static int hdsp_set_spdif_output(struct hdsp *hdsp, int out)
-{
-	if (out)
-		hdsp->control_register |= HDSP_SPDIFOpticalOut;
-	else
-		hdsp->control_register &= ~HDSP_SPDIFOpticalOut;
-	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
-	return 0;
-}
-
-#define snd_hdsp_info_spdif_bits	snd_ctl_boolean_mono_info
-
-static int snd_hdsp_get_spdif_out(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-
-	ucontrol->value.integer.value[0] = hdsp_spdif_out(hdsp);
-	return 0;
-}
-
-static int snd_hdsp_put_spdif_out(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-	int change;
-	unsigned int val;
-
-	if (!snd_hdsp_use_is_exclusive(hdsp))
-		return -EBUSY;
-	val = ucontrol->value.integer.value[0] & 1;
-	spin_lock_irq(&hdsp->lock);
-	change = (int)val != hdsp_spdif_out(hdsp);
-	hdsp_set_spdif_output(hdsp, val);
-	spin_unlock_irq(&hdsp->lock);
-	return change;
-}
-
-#define HDSP_SPDIF_PROFESSIONAL(xname, xindex) \
-{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xindex, \
-  .info = snd_hdsp_info_spdif_bits, \
-  .get = snd_hdsp_get_spdif_professional, .put = snd_hdsp_put_spdif_professional }
-
-static int hdsp_spdif_professional(struct hdsp *hdsp)
-{
-	return (hdsp->control_register & HDSP_SPDIFProfessional) ? 1 : 0;
-}
-
-static int hdsp_set_spdif_professional(struct hdsp *hdsp, int val)
-{
-	if (val)
-		hdsp->control_register |= HDSP_SPDIFProfessional;
-	else
-		hdsp->control_register &= ~HDSP_SPDIFProfessional;
-	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
-	return 0;
-}
-
-static int snd_hdsp_get_spdif_professional(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-
-	ucontrol->value.integer.value[0] = hdsp_spdif_professional(hdsp);
-	return 0;
-}
-
-static int snd_hdsp_put_spdif_professional(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-	int change;
-	unsigned int val;
-
-	if (!snd_hdsp_use_is_exclusive(hdsp))
-		return -EBUSY;
-	val = ucontrol->value.integer.value[0] & 1;
-	spin_lock_irq(&hdsp->lock);
-	change = (int)val != hdsp_spdif_professional(hdsp);
-	hdsp_set_spdif_professional(hdsp, val);
-	spin_unlock_irq(&hdsp->lock);
-	return change;
-}
-
-#define HDSP_SPDIF_EMPHASIS(xname, xindex) \
-{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xindex, \
-  .info = snd_hdsp_info_spdif_bits, \
-  .get = snd_hdsp_get_spdif_emphasis, .put = snd_hdsp_put_spdif_emphasis }
-
-static int hdsp_spdif_emphasis(struct hdsp *hdsp)
-{
-	return (hdsp->control_register & HDSP_SPDIFEmphasis) ? 1 : 0;
-}
-
-static int hdsp_set_spdif_emphasis(struct hdsp *hdsp, int val)
-{
-	if (val)
-		hdsp->control_register |= HDSP_SPDIFEmphasis;
-	else
-		hdsp->control_register &= ~HDSP_SPDIFEmphasis;
-	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
-	return 0;
-}
-
-static int snd_hdsp_get_spdif_emphasis(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-
-	ucontrol->value.integer.value[0] = hdsp_spdif_emphasis(hdsp);
-	return 0;
-}
-
-static int snd_hdsp_put_spdif_emphasis(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-	int change;
-	unsigned int val;
-
-	if (!snd_hdsp_use_is_exclusive(hdsp))
-		return -EBUSY;
-	val = ucontrol->value.integer.value[0] & 1;
-	spin_lock_irq(&hdsp->lock);
-	change = (int)val != hdsp_spdif_emphasis(hdsp);
-	hdsp_set_spdif_emphasis(hdsp, val);
-	spin_unlock_irq(&hdsp->lock);
-	return change;
-}
-
-#define HDSP_SPDIF_NON_AUDIO(xname, xindex) \
-{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xindex, \
-  .info = snd_hdsp_info_spdif_bits, \
-  .get = snd_hdsp_get_spdif_nonaudio, .put = snd_hdsp_put_spdif_nonaudio }
-
-static int hdsp_spdif_nonaudio(struct hdsp *hdsp)
-{
-	return (hdsp->control_register & HDSP_SPDIFNonAudio) ? 1 : 0;
-}
-
-static int hdsp_set_spdif_nonaudio(struct hdsp *hdsp, int val)
-{
-	if (val)
-		hdsp->control_register |= HDSP_SPDIFNonAudio;
-	else
-		hdsp->control_register &= ~HDSP_SPDIFNonAudio;
-	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
-	return 0;
-}
-
-static int snd_hdsp_get_spdif_nonaudio(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-
-	ucontrol->value.integer.value[0] = hdsp_spdif_nonaudio(hdsp);
-	return 0;
-}
-
-static int snd_hdsp_put_spdif_nonaudio(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-	int change;
-	unsigned int val;
-
-	if (!snd_hdsp_use_is_exclusive(hdsp))
-		return -EBUSY;
-	val = ucontrol->value.integer.value[0] & 1;
-	spin_lock_irq(&hdsp->lock);
-	change = (int)val != hdsp_spdif_nonaudio(hdsp);
-	hdsp_set_spdif_nonaudio(hdsp, val);
-	spin_unlock_irq(&hdsp->lock);
-	return change;
-}
-
 #define HDSP_SPDIF_SAMPLE_RATE(xname, xindex) \
 { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
   .name = xname, \
@@ -2548,114 +2369,6 @@ static int snd_hdsp_put_phone_gain(struct snd_kcontrol *kcontrol, struct snd_ctl
 	return change;
 }
 
-#define HDSP_XLR_BREAKOUT_CABLE(xname, xindex) \
-{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
-  .name = xname, \
-  .index = xindex, \
-  .info = snd_hdsp_info_xlr_breakout_cable, \
-  .get = snd_hdsp_get_xlr_breakout_cable, \
-  .put = snd_hdsp_put_xlr_breakout_cable \
-}
-
-static int hdsp_xlr_breakout_cable(struct hdsp *hdsp)
-{
-	if (hdsp->control_register & HDSP_XLRBreakoutCable)
-		return 1;
-	return 0;
-}
-
-static int hdsp_set_xlr_breakout_cable(struct hdsp *hdsp, int mode)
-{
-	if (mode)
-		hdsp->control_register |= HDSP_XLRBreakoutCable;
-	else
-		hdsp->control_register &= ~HDSP_XLRBreakoutCable;
-	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
-	return 0;
-}
-
-#define snd_hdsp_info_xlr_breakout_cable	snd_ctl_boolean_mono_info
-
-static int snd_hdsp_get_xlr_breakout_cable(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-
-	ucontrol->value.enumerated.item[0] = hdsp_xlr_breakout_cable(hdsp);
-	return 0;
-}
-
-static int snd_hdsp_put_xlr_breakout_cable(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-	int change;
-	int val;
-
-	if (!snd_hdsp_use_is_exclusive(hdsp))
-		return -EBUSY;
-	val = ucontrol->value.integer.value[0] & 1;
-	spin_lock_irq(&hdsp->lock);
-	change = (int)val != hdsp_xlr_breakout_cable(hdsp);
-	hdsp_set_xlr_breakout_cable(hdsp, val);
-	spin_unlock_irq(&hdsp->lock);
-	return change;
-}
-
-/* (De)activates old RME Analog Extension Board
-   These are connected to the internal ADAT connector
-   Switching this on desactivates external ADAT
-*/
-#define HDSP_AEB(xname, xindex) \
-{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
-  .name = xname, \
-  .index = xindex, \
-  .info = snd_hdsp_info_aeb, \
-  .get = snd_hdsp_get_aeb, \
-  .put = snd_hdsp_put_aeb \
-}
-
-static int hdsp_aeb(struct hdsp *hdsp)
-{
-	if (hdsp->control_register & HDSP_AnalogExtensionBoard)
-		return 1;
-	return 0;
-}
-
-static int hdsp_set_aeb(struct hdsp *hdsp, int mode)
-{
-	if (mode)
-		hdsp->control_register |= HDSP_AnalogExtensionBoard;
-	else
-		hdsp->control_register &= ~HDSP_AnalogExtensionBoard;
-	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
-	return 0;
-}
-
-#define snd_hdsp_info_aeb		snd_ctl_boolean_mono_info
-
-static int snd_hdsp_get_aeb(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-
-	ucontrol->value.enumerated.item[0] = hdsp_aeb(hdsp);
-	return 0;
-}
-
-static int snd_hdsp_put_aeb(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-	int change;
-	int val;
-
-	if (!snd_hdsp_use_is_exclusive(hdsp))
-		return -EBUSY;
-	val = ucontrol->value.integer.value[0] & 1;
-	spin_lock_irq(&hdsp->lock);
-	change = (int)val != hdsp_aeb(hdsp);
-	hdsp_set_aeb(hdsp, val);
-	spin_unlock_irq(&hdsp->lock);
-	return change;
-}
-
 #define HDSP_PREF_SYNC_REF(xname, xindex) \
 { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
   .name = xname, \
@@ -2844,58 +2557,6 @@ static int snd_hdsp_get_autosync_ref(struct snd_kcontrol *kcontrol, struct snd_c
 	return 0;
 }
 
-#define HDSP_LINE_OUT(xname, xindex) \
-{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
-  .name = xname, \
-  .index = xindex, \
-  .info = snd_hdsp_info_line_out, \
-  .get = snd_hdsp_get_line_out, \
-  .put = snd_hdsp_put_line_out \
-}
-
-static int hdsp_line_out(struct hdsp *hdsp)
-{
-	return (hdsp->control_register & HDSP_LineOut) ? 1 : 0;
-}
-
-static int hdsp_set_line_output(struct hdsp *hdsp, int out)
-{
-	if (out)
-		hdsp->control_register |= HDSP_LineOut;
-	else
-		hdsp->control_register &= ~HDSP_LineOut;
-	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
-	return 0;
-}
-
-#define snd_hdsp_info_line_out		snd_ctl_boolean_mono_info
-
-static int snd_hdsp_get_line_out(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-
-	spin_lock_irq(&hdsp->lock);
-	ucontrol->value.integer.value[0] = hdsp_line_out(hdsp);
-	spin_unlock_irq(&hdsp->lock);
-	return 0;
-}
-
-static int snd_hdsp_put_line_out(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
-{
-	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
-	int change;
-	unsigned int val;
-
-	if (!snd_hdsp_use_is_exclusive(hdsp))
-		return -EBUSY;
-	val = ucontrol->value.integer.value[0] & 1;
-	spin_lock_irq(&hdsp->lock);
-	change = (int)val != hdsp_line_out(hdsp);
-	hdsp_set_line_output(hdsp, val);
-	spin_unlock_irq(&hdsp->lock);
-	return change;
-}
-
 #define HDSP_PRECISE_POINTER(xname, xindex) \
 { .iface = SNDRV_CTL_ELEM_IFACE_CARD, \
   .name = xname, \
-- 
1.7.10.4

Re: [PATCH 1/4] ALSA: hdsp - Fix detection for RME RPM/Multiface/Digiface ioboxes

[Takashi Iwai]

At Tue, 15 Jan 2013 15:18:10 +0100,
Adrian Knoth wrote:
> 
> The current iobox detection code reportedly fails for various users, so
> simply do what the Win32 driver does instead.
> 
> Patch originally by Karl Grill <kgrill@chello.at> and then modified to
> comply with kernel coding guidelines + current HEAD.
> 
> Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>
> 
> diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
> index 4fae81f..ba190d5 100644
> --- a/sound/pci/rme9652/hdsp.c
> +++ b/sound/pci/rme9652/hdsp.c
> @@ -154,10 +154,12 @@ MODULE_FIRMWARE("digiface_firmware_rev11.bin");
>  #define HDSP_BIGENDIAN_MODE     0x200
>  #define HDSP_RD_MULTIPLE        0x400
>  #define HDSP_9652_ENABLE_MIXER  0x800
> +#define HDSP_S200		0x800
> +#define HDSP_CYCLIC_MODE	0x1000
>  #define HDSP_TDO                0x10000000
>  
> -#define HDSP_S_PROGRAM     	(HDSP_PROGRAM|HDSP_CONFIG_MODE_0)
> -#define HDSP_S_LOAD		(HDSP_PROGRAM|HDSP_CONFIG_MODE_1)
> +#define HDSP_S_PROGRAM	    (HDSP_CYCLIC_MODE|HDSP_PROGRAM|HDSP_CONFIG_MODE_0)
> +#define HDSP_S_LOAD	    (HDSP_CYCLIC_MODE|HDSP_PROGRAM|HDSP_CONFIG_MODE_1)
>  
>  /* Control Register bits */
>  
> @@ -671,13 +673,23 @@ static unsigned int hdsp_read(struct hdsp *hdsp, int reg)
>  
>  static int hdsp_check_for_iobox (struct hdsp *hdsp)
>  {
> +	int i;
> +
>  	if (hdsp->io_type == H9652 || hdsp->io_type == H9632) return 0;
> -	if (hdsp_read (hdsp, HDSP_statusRegister) & HDSP_ConfigError) {
> -		snd_printk("Hammerfall-DSP: no IO box connected!\n");
> -		hdsp->state &= ~HDSP_FirmwareLoaded;
> -		return -EIO;
> +	for (i = 0; i < 500; i++) {
> +		if (0 == (hdsp_read(hdsp, HDSP_statusRegister) &
> +					HDSP_ConfigError)) {
> +			if (i) {
> +				snd_printk("Hammerfall-DSP: IO box found after %d ms\n",
> +						(20 * i));

One more favor: please make it only as a debug print.
It's not good to print this positive result always.
That is, simply replace it with snd_printd().

> +			}
> +			return 0;
> +		}
> +		msleep(20);
>  	}
> -	return 0;
> +	snd_printk("Hammerfall-DSP: no IO box connected!\n");

Also, recommended to put KERN_ERR prefix here.

> +	hdsp->state &= ~HDSP_FirmwareLoaded;
> +	return -EIO;
>  }
>  
>  static int hdsp_wait_for_iobox(struct hdsp *hdsp, unsigned int loops,
> @@ -728,6 +740,7 @@ static int snd_hdsp_load_firmware_from_cache(struct hdsp *hdsp) {
>  
>  		if (hdsp_fifo_wait (hdsp, 0, HDSP_LONG_WAIT)) {
>  			snd_printk ("Hammerfall-DSP: timeout waiting for download preparation\n");
> +			hdsp_write(hdsp, HDSP_control2Reg, HDSP_S200);
>  			return -EIO;
>  		}
>  
> @@ -737,17 +750,15 @@ static int snd_hdsp_load_firmware_from_cache(struct hdsp *hdsp) {
>  			hdsp_write(hdsp, HDSP_fifoData, cache[i]);
>  			if (hdsp_fifo_wait (hdsp, 127, HDSP_LONG_WAIT)) {
>  				snd_printk ("Hammerfall-DSP: timeout during firmware loading\n");
> +				hdsp_write(hdsp, HDSP_control2Reg, HDSP_S200);
>  				return -EIO;
>  			}
>  		}
>  
> -		ssleep(3);
> -
> -		if (hdsp_fifo_wait (hdsp, 0, HDSP_LONG_WAIT)) {
> -			snd_printk ("Hammerfall-DSP: timeout at end of firmware loading\n");
> -		    	return -EIO;
> -		}
> +		hdsp_fifo_wait(hdsp, 3, HDSP_LONG_WAIT);
> +		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S200);
>  
> +		ssleep(3);
>  #ifdef SNDRV_BIG_ENDIAN
>  		hdsp->control2_register = HDSP_BIGENDIAN_MODE;
>  #else
> @@ -773,24 +784,51 @@ static int hdsp_get_iobox_version (struct hdsp *hdsp)
>  {
>  	if ((hdsp_read (hdsp, HDSP_statusRegister) & HDSP_DllError) != 0) {
>  
> -		hdsp_write (hdsp, HDSP_control2Reg, HDSP_PROGRAM);
> -		hdsp_write (hdsp, HDSP_fifoData, 0);
> -		if (hdsp_fifo_wait (hdsp, 0, HDSP_SHORT_WAIT) < 0)
> -			return -EIO;
> +		hdsp_write(hdsp, HDSP_control2Reg, HDSP_S_LOAD);
> +		hdsp_write(hdsp, HDSP_fifoData, 0);
>  
> -		hdsp_write (hdsp, HDSP_control2Reg, HDSP_S_LOAD);
> +		if (hdsp_fifo_wait(hdsp, 0, HDSP_SHORT_WAIT) < 0) {
> +			hdsp_write(hdsp, HDSP_control2Reg, 0x100 | HDSP_S200);

0x100 isn't replaced with a literal?
Maybe worth to define (0x100 | HDSP_S200) to something meaningful, as
this appears repeatedly?


thanks,

Takashi

Re: [PATCH 2/4] ALSA: hdsp - Implement generic function to toggle settings

[Takashi Iwai]

At Tue, 15 Jan 2013 07:48:52 +0100,
Takashi Iwai wrote:
> 
> At Mon, 14 Jan 2013 21:37:18 +0100,
> Adrian Knoth wrote:
> > 
> > The driver contains multiple similar functions that change only a single
> > bit in the control register, only the bit position varies.
> > 
> > This patch implements a generic function to toggle a certain bit
> > position that will be used to replace the old code.
> > 
> > Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>
> > 
> > diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
> > index bcad9ba..f51726a 100644
> > --- a/sound/pci/rme9652/hdsp.c
> > +++ b/sound/pci/rme9652/hdsp.c
> > @@ -1711,6 +1711,64 @@ static int snd_hdsp_put_spdif_in(struct snd_kcontrol *kcontrol, struct snd_ctl_e
> >  	return change;
> >  }
> >  
> > +#define HDSP_TOGGLE_SETTING(xname, xindex) \
> > +{   .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
> > +	.name = xname, \
> > +	.private_value = xindex, \
> > +	.info = snd_hdsp_info_toggle_setting, \
> > +	.get = snd_hdsp_get_toggle_setting, \
> > +	.put = snd_hdsp_put_toggle_setting \
> > +}
> > +
> > +static int hdsp_toggle_setting(struct hdsp *hdsp, u32 regmask)
> > +{
> > +	return (hdsp->control_register & regmask) ? 1 : 0;
> > +}
> > +
> > +static int hdsp_set_toggle_setting(struct hdsp *hdsp, u32 regmask, int out)
> > +{
> > +	if (out)
> > +		hdsp->control_register |= regmask;
> > +	else
> > +		hdsp->control_register &= ~regmask;
> > +	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
> > +
> > +	return 0;
> > +}
> > +
> > +#define snd_hdsp_info_toggle_setting		   snd_ctl_boolean_mono_info
> > +
> > +static int snd_hdsp_get_toggle_setting(struct snd_kcontrol *kcontrol,
> > +		struct snd_ctl_elem_value *ucontrol)
> > +{
> > +	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
> > +	u32 regmask = kcontrol->private_value;
> > +
> > +	spin_lock_irq(&hdsp->lock);
> > +	ucontrol->value.integer.value[0] = hdsp_toggle_setting(hdsp, regmask);
> > +	spin_unlock_irq(&hdsp->lock);
> > +	return 0;
> > +}
> > +
> > +static int snd_hdsp_put_toggle_setting(struct snd_kcontrol *kcontrol,
> > +		struct snd_ctl_elem_value *ucontrol)
> > +{
> > +	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
> > +	u32 regmask = kcontrol->private_value;
> > +	int change;
> > +	unsigned int val;
> > +
> > +	if (!snd_hdsp_use_is_exclusive(hdsp))
> > +		return -EBUSY;
> > +	val = ucontrol->value.integer.value[0] & 1;
> > +	spin_lock_irq(&hdsp->lock);
> > +	change = (int) val != hdsp_toggle_setting(hdsp, regmask);
> > +	hdsp_set_toggle_setting(hdsp, regmask, val);
> 
> If it doesn't change, you don't have to call
> hdsp_set_toggle_setting().

I know the old code does it so, but we don't have to follow it in the
new code, too.


Takashi

Re: [PATCH 2/4] ALSA: hdsp - Implement generic function to toggle settings

[Takashi Iwai]

At Mon, 14 Jan 2013 21:37:18 +0100,
Adrian Knoth wrote:
> 
> The driver contains multiple similar functions that change only a single
> bit in the control register, only the bit position varies.
> 
> This patch implements a generic function to toggle a certain bit
> position that will be used to replace the old code.
> 
> Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>
> 
> diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
> index bcad9ba..f51726a 100644
> --- a/sound/pci/rme9652/hdsp.c
> +++ b/sound/pci/rme9652/hdsp.c
> @@ -1711,6 +1711,64 @@ static int snd_hdsp_put_spdif_in(struct snd_kcontrol *kcontrol, struct snd_ctl_e
>  	return change;
>  }
>  
> +#define HDSP_TOGGLE_SETTING(xname, xindex) \
> +{   .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
> +	.name = xname, \
> +	.private_value = xindex, \
> +	.info = snd_hdsp_info_toggle_setting, \
> +	.get = snd_hdsp_get_toggle_setting, \
> +	.put = snd_hdsp_put_toggle_setting \
> +}
> +
> +static int hdsp_toggle_setting(struct hdsp *hdsp, u32 regmask)
> +{
> +	return (hdsp->control_register & regmask) ? 1 : 0;
> +}
> +
> +static int hdsp_set_toggle_setting(struct hdsp *hdsp, u32 regmask, int out)
> +{
> +	if (out)
> +		hdsp->control_register |= regmask;
> +	else
> +		hdsp->control_register &= ~regmask;
> +	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
> +
> +	return 0;
> +}
> +
> +#define snd_hdsp_info_toggle_setting		   snd_ctl_boolean_mono_info
> +
> +static int snd_hdsp_get_toggle_setting(struct snd_kcontrol *kcontrol,
> +		struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
> +	u32 regmask = kcontrol->private_value;
> +
> +	spin_lock_irq(&hdsp->lock);
> +	ucontrol->value.integer.value[0] = hdsp_toggle_setting(hdsp, regmask);
> +	spin_unlock_irq(&hdsp->lock);
> +	return 0;
> +}
> +
> +static int snd_hdsp_put_toggle_setting(struct snd_kcontrol *kcontrol,
> +		struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
> +	u32 regmask = kcontrol->private_value;
> +	int change;
> +	unsigned int val;
> +
> +	if (!snd_hdsp_use_is_exclusive(hdsp))
> +		return -EBUSY;
> +	val = ucontrol->value.integer.value[0] & 1;
> +	spin_lock_irq(&hdsp->lock);
> +	change = (int) val != hdsp_toggle_setting(hdsp, regmask);
> +	hdsp_set_toggle_setting(hdsp, regmask, val);

If it doesn't change, you don't have to call
hdsp_set_toggle_setting().


Takashi

> +	spin_unlock_irq(&hdsp->lock);
> +	return change;
> +}
> +
> +
>  #define HDSP_SPDIF_OUT(xname, xindex) \
>  { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xindex, \
>    .info = snd_hdsp_info_spdif_bits, \
> -- 
> 1.7.10.4
> 

[PATCH 2/4] ALSA: hdsp - Implement generic function to toggle settings

[Adrian Knoth]

The driver contains multiple similar functions that change only a single
bit in the control register, only the bit position varies.

This patch implements a generic function to toggle a certain bit
position that will be used to replace the old code.

Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de>

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index bcad9ba..f51726a 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -1711,6 +1711,64 @@ static int snd_hdsp_put_spdif_in(struct snd_kcontrol *kcontrol, struct snd_ctl_e
 	return change;
 }
 
+#define HDSP_TOGGLE_SETTING(xname, xindex) \
+{   .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
+	.name = xname, \
+	.private_value = xindex, \
+	.info = snd_hdsp_info_toggle_setting, \
+	.get = snd_hdsp_get_toggle_setting, \
+	.put = snd_hdsp_put_toggle_setting \
+}
+
+static int hdsp_toggle_setting(struct hdsp *hdsp, u32 regmask)
+{
+	return (hdsp->control_register & regmask) ? 1 : 0;
+}
+
+static int hdsp_set_toggle_setting(struct hdsp *hdsp, u32 regmask, int out)
+{
+	if (out)
+		hdsp->control_register |= regmask;
+	else
+		hdsp->control_register &= ~regmask;
+	hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
+
+	return 0;
+}
+
+#define snd_hdsp_info_toggle_setting		   snd_ctl_boolean_mono_info
+
+static int snd_hdsp_get_toggle_setting(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
+	u32 regmask = kcontrol->private_value;
+
+	spin_lock_irq(&hdsp->lock);
+	ucontrol->value.integer.value[0] = hdsp_toggle_setting(hdsp, regmask);
+	spin_unlock_irq(&hdsp->lock);
+	return 0;
+}
+
+static int snd_hdsp_put_toggle_setting(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	struct hdsp *hdsp = snd_kcontrol_chip(kcontrol);
+	u32 regmask = kcontrol->private_value;
+	int change;
+	unsigned int val;
+
+	if (!snd_hdsp_use_is_exclusive(hdsp))
+		return -EBUSY;
+	val = ucontrol->value.integer.value[0] & 1;
+	spin_lock_irq(&hdsp->lock);
+	change = (int) val != hdsp_toggle_setting(hdsp, regmask);
+	hdsp_set_toggle_setting(hdsp, regmask, val);
+	spin_unlock_irq(&hdsp->lock);
+	return change;
+}
+
+
 #define HDSP_SPDIF_OUT(xname, xindex) \
 { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xindex, \
   .info = snd_hdsp_info_spdif_bits, \
-- 
1.7.10.4