[PATCH 0/5] ALSA: oxfw: code refactoring for model-dependent quirks

[PATCH 0/5] ALSA: oxfw: code refactoring for model-dependent quirks

[Takashi Sakamoto]

Hi,

ALSA oxfw driver has got some patches to expand its functionalities. As a
result, current code includes some inefficiencies and confusions.

This patchset is a code refactoring about model-dependent quirks. This
patchset is a part of my previous RFCv2 (patch 01-05) and a preparation for
the rest of patches:

[alsa-devel] [RFC][PATCH 00/17 v2] ALSA: oxfw: refactoring and merging scs1x module
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101558.html

Takashi Sakamoto (5):
  ALSA: oxfw: rename a file for control elements so that it's for
    model-specific
  ALSA: oxfw: rename local functions for control elements so that they
    represent as local
  ALSA: oxfw: change function prototype for AV/C Audio Subunit command
  ALSA: oxfw: reuse driver entry to detect quirk
  ALSA: oxfw: gather model-dependent conditions to a function

 sound/firewire/oxfw/Makefile                       |  4 +-
 .../firewire/oxfw/{oxfw-control.c => oxfw-spkr.c}  | 80 ++++++++++++----------
 sound/firewire/oxfw/oxfw.c                         | 59 +++++++++-------
 sound/firewire/oxfw/oxfw.h                         |  6 +-
 4 files changed, 86 insertions(+), 63 deletions(-)
 rename sound/firewire/oxfw/{oxfw-control.c => oxfw-spkr.c} (71%)

-- 
2.5.0

[PATCH 1/5] ALSA: oxfw: rename a file for control elements so that it's for model-specific

[Takashi Sakamoto]

In ALSA firewire stack, drivers basically has no control elements. This
is due to the fact that each model has own functionality even if they use
the same communication chipset. Implementing all of the functionalities in
kernel space unreasonably increases our efforts to maintain the stack. In
most case, these functionalities can be implemented in userspace via Linux
fw character devices.

However, ALSA OXFW driver has control elements comes from old
firewire-speakers driver. Adding the elements is in a file names as
'oxfw-control.c', while the elements are really model-specific. The
name is confusing because it gives an idea to handle control elements
for all of OXFW-based models.

This commit renames the file so that it's just for models supported by
old firewire-speakers driver.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/oxfw/Makefile                        | 4 ++--
 sound/firewire/oxfw/{oxfw-control.c => oxfw-spkr.c} | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
 rename sound/firewire/oxfw/{oxfw-control.c => oxfw-spkr.c} (99%)

diff --git a/sound/firewire/oxfw/Makefile b/sound/firewire/oxfw/Makefile
index 06ff50f..4e54ba9 100644
--- a/sound/firewire/oxfw/Makefile
+++ b/sound/firewire/oxfw/Makefile
@@ -1,3 +1,3 @@
-snd-oxfw-objs := oxfw-command.o oxfw-stream.o oxfw-control.o oxfw-pcm.o \
-		 oxfw-proc.o oxfw-midi.o oxfw-hwdep.o oxfw.o
+snd-oxfw-objs := oxfw-command.o oxfw-stream.o oxfw-pcm.o oxfw-proc.o \
+		 oxfw-midi.o oxfw-hwdep.o oxfw-spkr.o oxfw.o
 obj-$(CONFIG_SND_OXFW) += snd-oxfw.o
diff --git a/sound/firewire/oxfw/oxfw-control.c b/sound/firewire/oxfw/oxfw-spkr.c
similarity index 99%
rename from sound/firewire/oxfw/oxfw-control.c
rename to sound/firewire/oxfw/oxfw-spkr.c
index 02a1cb9..22d8536 100644
--- a/sound/firewire/oxfw/oxfw-control.c
+++ b/sound/firewire/oxfw/oxfw-spkr.c
@@ -1,5 +1,5 @@
 /*
- * oxfw_stream.c - a part of driver for OXFW970/971 based devices
+ * oxfw-spkr.c - a part of driver for OXFW970/971 based devices
  *
  * Copyright (c) Clemens Ladisch <clemens@ladisch.de>
  * Licensed under the terms of the GNU General Public License, version 2.
-- 
2.5.0

[PATCH 2/5] ALSA: oxfw: rename local functions for control elements so that they represent as local

[Takashi Sakamoto]

This commit renames local functions with prefix 'spkr_', so that they're
for firewire-speakers.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/oxfw/oxfw-spkr.c | 40 ++++++++++++++++++++--------------------
 sound/firewire/oxfw/oxfw.c      |  2 +-
 sound/firewire/oxfw/oxfw.h      |  4 ++--
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/sound/firewire/oxfw/oxfw-spkr.c b/sound/firewire/oxfw/oxfw-spkr.c
index 22d8536..fde6b76 100644
--- a/sound/firewire/oxfw/oxfw-spkr.c
+++ b/sound/firewire/oxfw/oxfw-spkr.c
@@ -14,7 +14,7 @@ enum control_attribute {
 	CTL_CURRENT	= 0x10,
 };
 
-static int oxfw_mute_command(struct snd_oxfw *oxfw, bool *value,
+static int spkr_mute_command(struct snd_oxfw *oxfw, bool *value,
 			     enum control_action action)
 {
 	u8 *buf;
@@ -70,7 +70,7 @@ error:
 	return err;
 }
 
-static int oxfw_volume_command(struct snd_oxfw *oxfw, s16 *value,
+static int spkr_volume_command(struct snd_oxfw *oxfw, s16 *value,
 			       unsigned int channel,
 			       enum control_attribute attribute,
 			       enum control_action action)
@@ -131,7 +131,7 @@ error:
 	return err;
 }
 
-static int oxfw_mute_get(struct snd_kcontrol *control,
+static int spkr_mute_get(struct snd_kcontrol *control,
 			 struct snd_ctl_elem_value *value)
 {
 	struct snd_oxfw *oxfw = control->private_data;
@@ -141,7 +141,7 @@ static int oxfw_mute_get(struct snd_kcontrol *control,
 	return 0;
 }
 
-static int oxfw_mute_put(struct snd_kcontrol *control,
+static int spkr_mute_put(struct snd_kcontrol *control,
 			 struct snd_ctl_elem_value *value)
 {
 	struct snd_oxfw *oxfw = control->private_data;
@@ -153,7 +153,7 @@ static int oxfw_mute_put(struct snd_kcontrol *control,
 	if (mute == oxfw->mute)
 		return 0;
 
-	err = oxfw_mute_command(oxfw, &mute, CTL_WRITE);
+	err = spkr_mute_command(oxfw, &mute, CTL_WRITE);
 	if (err < 0)
 		return err;
 	oxfw->mute = mute;
@@ -161,7 +161,7 @@ static int oxfw_mute_put(struct snd_kcontrol *control,
 	return 1;
 }
 
-static int oxfw_volume_info(struct snd_kcontrol *control,
+static int spkr_volume_info(struct snd_kcontrol *control,
 			    struct snd_ctl_elem_info *info)
 {
 	struct snd_oxfw *oxfw = control->private_data;
@@ -176,7 +176,7 @@ static int oxfw_volume_info(struct snd_kcontrol *control,
 
 static const u8 channel_map[6] = { 0, 1, 4, 5, 2, 3 };
 
-static int oxfw_volume_get(struct snd_kcontrol *control,
+static int spkr_volume_get(struct snd_kcontrol *control,
 			   struct snd_ctl_elem_value *value)
 {
 	struct snd_oxfw *oxfw = control->private_data;
@@ -188,7 +188,7 @@ static int oxfw_volume_get(struct snd_kcontrol *control,
 	return 0;
 }
 
-static int oxfw_volume_put(struct snd_kcontrol *control,
+static int spkr_volume_put(struct snd_kcontrol *control,
 			   struct snd_ctl_elem_value *value)
 {
 	struct snd_oxfw *oxfw = control->private_data;
@@ -218,7 +218,7 @@ static int oxfw_volume_put(struct snd_kcontrol *control,
 	for (i = 0; i <= oxfw->device_info->mixer_channels; ++i) {
 		volume = value->value.integer.value[channel_map[i ? i - 1 : 0]];
 		if (changed_channels & (1 << i)) {
-			err = oxfw_volume_command(oxfw, &volume, i,
+			err = spkr_volume_command(oxfw, &volume, i,
 						   CTL_CURRENT, CTL_WRITE);
 			if (err < 0)
 				return err;
@@ -230,44 +230,44 @@ static int oxfw_volume_put(struct snd_kcontrol *control,
 	return changed_channels != 0;
 }
 
-int snd_oxfw_create_mixer(struct snd_oxfw *oxfw)
+int snd_oxfw_add_spkr(struct snd_oxfw *oxfw)
 {
 	static const struct snd_kcontrol_new controls[] = {
 		{
 			.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 			.name = "PCM Playback Switch",
 			.info = snd_ctl_boolean_mono_info,
-			.get = oxfw_mute_get,
-			.put = oxfw_mute_put,
+			.get = spkr_mute_get,
+			.put = spkr_mute_put,
 		},
 		{
 			.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 			.name = "PCM Playback Volume",
-			.info = oxfw_volume_info,
-			.get = oxfw_volume_get,
-			.put = oxfw_volume_put,
+			.info = spkr_volume_info,
+			.get = spkr_volume_get,
+			.put = spkr_volume_put,
 		},
 	};
 	unsigned int i, first_ch;
 	int err;
 
-	err = oxfw_volume_command(oxfw, &oxfw->volume_min,
+	err = spkr_volume_command(oxfw, &oxfw->volume_min,
 				   0, CTL_MIN, CTL_READ);
 	if (err < 0)
 		return err;
-	err = oxfw_volume_command(oxfw, &oxfw->volume_max,
+	err = spkr_volume_command(oxfw, &oxfw->volume_max,
 				   0, CTL_MAX, CTL_READ);
 	if (err < 0)
 		return err;
 
-	err = oxfw_mute_command(oxfw, &oxfw->mute, CTL_READ);
+	err = spkr_mute_command(oxfw, &oxfw->mute, CTL_READ);
 	if (err < 0)
 		return err;
 
 	first_ch = oxfw->device_info->mixer_channels == 1 ? 0 : 1;
 	for (i = 0; i < oxfw->device_info->mixer_channels; ++i) {
-		err = oxfw_volume_command(oxfw, &oxfw->volume[i],
-					   first_ch + i, CTL_CURRENT, CTL_READ);
+		err = spkr_volume_command(oxfw, &oxfw->volume[i],
+					  first_ch + i, CTL_CURRENT, CTL_READ);
 		if (err < 0)
 			return err;
 	}
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 588b93f..0304d45 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -205,7 +205,7 @@ static int oxfw_probe(struct fw_unit *unit,
 		goto error;
 
 	if (oxfw->device_info) {
-		err = snd_oxfw_create_mixer(oxfw);
+		err = snd_oxfw_add_spkr(oxfw);
 		if (err < 0)
 			goto error;
 	}
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h
index 8392c42..9efdc02 100644
--- a/sound/firewire/oxfw/oxfw.h
+++ b/sound/firewire/oxfw/oxfw.h
@@ -138,10 +138,10 @@ void snd_oxfw_stream_lock_release(struct snd_oxfw *oxfw);
 
 int snd_oxfw_create_pcm(struct snd_oxfw *oxfw);
 
-int snd_oxfw_create_mixer(struct snd_oxfw *oxfw);
-
 void snd_oxfw_proc_init(struct snd_oxfw *oxfw);
 
 int snd_oxfw_create_midi(struct snd_oxfw *oxfw);
 
 int snd_oxfw_create_hwdep(struct snd_oxfw *oxfw);
+
+int snd_oxfw_add_spkr(struct snd_oxfw *oxfw);
-- 
2.5.0

[PATCH 3/5] ALSA: oxfw: change function prototype for AV/C Audio Subunit command

[Takashi Sakamoto]

ALSA OXFW driver uses AV/C Audio Subunit commands to control some models.
The commands get/set the state of Feature function block of the subunit.
The commands are not specific to OXFW, thus there's a possibility to use
them in the other drivers.

Currently, helper functions for the commands require 'struct snd_oxfw',
although, it's not necessarily required. It's better to change prototype
of the functions without the structure for future use.

This commit changes the prototype.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/oxfw/oxfw-spkr.c | 54 +++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/sound/firewire/oxfw/oxfw-spkr.c b/sound/firewire/oxfw/oxfw-spkr.c
index fde6b76..d733a15 100644
--- a/sound/firewire/oxfw/oxfw-spkr.c
+++ b/sound/firewire/oxfw/oxfw-spkr.c
@@ -14,8 +14,8 @@ enum control_attribute {
 	CTL_CURRENT	= 0x10,
 };
 
-static int spkr_mute_command(struct snd_oxfw *oxfw, bool *value,
-			     enum control_action action)
+static int avc_audio_feature_mute(struct fw_unit *unit, u8 fb_id, bool *value,
+				  enum control_action action)
 {
 	u8 *buf;
 	u8 response_ok;
@@ -35,7 +35,7 @@ static int spkr_mute_command(struct snd_oxfw *oxfw, bool *value,
 	buf[1] = 0x08;			/* audio unit 0 */
 	buf[2] = 0xb8;			/* FUNCTION BLOCK */
 	buf[3] = 0x81;			/* function block type: feature */
-	buf[4] = oxfw->device_info->mute_fb_id; /* function block ID */
+	buf[4] = fb_id;			/* function block ID */
 	buf[5] = 0x10;			/* control attribute: current */
 	buf[6] = 0x02;			/* selector length */
 	buf[7] = 0x00;			/* audio channel number */
@@ -46,16 +46,16 @@ static int spkr_mute_command(struct snd_oxfw *oxfw, bool *value,
 	else
 		buf[10] = *value ? 0x70 : 0x60;
 
-	err = fcp_avc_transaction(oxfw->unit, buf, 11, buf, 11, 0x3fe);
+	err = fcp_avc_transaction(unit, buf, 11, buf, 11, 0x3fe);
 	if (err < 0)
 		goto error;
 	if (err < 11) {
-		dev_err(&oxfw->unit->device, "short FCP response\n");
+		dev_err(&unit->device, "short FCP response\n");
 		err = -EIO;
 		goto error;
 	}
 	if (buf[0] != response_ok) {
-		dev_err(&oxfw->unit->device, "mute command failed\n");
+		dev_err(&unit->device, "mute command failed\n");
 		err = -EIO;
 		goto error;
 	}
@@ -70,10 +70,10 @@ error:
 	return err;
 }
 
-static int spkr_volume_command(struct snd_oxfw *oxfw, s16 *value,
-			       unsigned int channel,
-			       enum control_attribute attribute,
-			       enum control_action action)
+static int avc_audio_feature_volume(struct fw_unit *unit, u8 fb_id, s16 *value,
+				    unsigned int channel,
+				    enum control_attribute attribute,
+				    enum control_action action)
 {
 	u8 *buf;
 	u8 response_ok;
@@ -93,7 +93,7 @@ static int spkr_volume_command(struct snd_oxfw *oxfw, s16 *value,
 	buf[1] = 0x08;			/* audio unit 0 */
 	buf[2] = 0xb8;			/* FUNCTION BLOCK */
 	buf[3] = 0x81;			/* function block type: feature */
-	buf[4] = oxfw->device_info->volume_fb_id; /* function block ID */
+	buf[4] = fb_id;			/* function block ID */
 	buf[5] = attribute;		/* control attribute */
 	buf[6] = 0x02;			/* selector length */
 	buf[7] = channel;		/* audio channel number */
@@ -107,16 +107,16 @@ static int spkr_volume_command(struct snd_oxfw *oxfw, s16 *value,
 		buf[11] = *value;
 	}
 
-	err = fcp_avc_transaction(oxfw->unit, buf, 12, buf, 12, 0x3fe);
+	err = fcp_avc_transaction(unit, buf, 12, buf, 12, 0x3fe);
 	if (err < 0)
 		goto error;
 	if (err < 12) {
-		dev_err(&oxfw->unit->device, "short FCP response\n");
+		dev_err(&unit->device, "short FCP response\n");
 		err = -EIO;
 		goto error;
 	}
 	if (buf[0] != response_ok) {
-		dev_err(&oxfw->unit->device, "volume command failed\n");
+		dev_err(&unit->device, "volume command failed\n");
 		err = -EIO;
 		goto error;
 	}
@@ -153,7 +153,8 @@ static int spkr_mute_put(struct snd_kcontrol *control,
 	if (mute == oxfw->mute)
 		return 0;
 
-	err = spkr_mute_command(oxfw, &mute, CTL_WRITE);
+	err = avc_audio_feature_mute(oxfw->unit, oxfw->device_info->mute_fb_id,
+				     &mute, CTL_WRITE);
 	if (err < 0)
 		return err;
 	oxfw->mute = mute;
@@ -218,8 +219,10 @@ static int spkr_volume_put(struct snd_kcontrol *control,
 	for (i = 0; i <= oxfw->device_info->mixer_channels; ++i) {
 		volume = value->value.integer.value[channel_map[i ? i - 1 : 0]];
 		if (changed_channels & (1 << i)) {
-			err = spkr_volume_command(oxfw, &volume, i,
-						   CTL_CURRENT, CTL_WRITE);
+			err = avc_audio_feature_volume(oxfw->unit,
+						  oxfw->device_info->mute_fb_id,
+						  &volume,
+						  i, CTL_CURRENT, CTL_WRITE);
 			if (err < 0)
 				return err;
 		}
@@ -251,22 +254,27 @@ int snd_oxfw_add_spkr(struct snd_oxfw *oxfw)
 	unsigned int i, first_ch;
 	int err;
 
-	err = spkr_volume_command(oxfw, &oxfw->volume_min,
-				   0, CTL_MIN, CTL_READ);
+	err = avc_audio_feature_volume(oxfw->unit,
+				       oxfw->device_info->volume_fb_id,
+				       &oxfw->volume_min, 0, CTL_MIN, CTL_READ);
 	if (err < 0)
 		return err;
-	err = spkr_volume_command(oxfw, &oxfw->volume_max,
-				   0, CTL_MAX, CTL_READ);
+	err = avc_audio_feature_volume(oxfw->unit,
+				       oxfw->device_info->volume_fb_id,
+				       &oxfw->volume_max, 0, CTL_MAX, CTL_READ);
 	if (err < 0)
 		return err;
 
-	err = spkr_mute_command(oxfw, &oxfw->mute, CTL_READ);
+	err = avc_audio_feature_mute(oxfw->unit, oxfw->device_info->mute_fb_id,
+				     &oxfw->mute, CTL_READ);
 	if (err < 0)
 		return err;
 
 	first_ch = oxfw->device_info->mixer_channels == 1 ? 0 : 1;
 	for (i = 0; i < oxfw->device_info->mixer_channels; ++i) {
-		err = spkr_volume_command(oxfw, &oxfw->volume[i],
+		err = avc_audio_feature_volume(oxfw->unit,
+					  oxfw->device_info->volume_fb_id,
+					  &oxfw->volume[i],
 					  first_ch + i, CTL_CURRENT, CTL_READ);
 		if (err < 0)
 			return err;
-- 
2.5.0

[PATCH 4/5] ALSA: oxfw: reuse driver entry to detect quirks

[Takashi Sakamoto]

Currently, assignment to model-dependent quirk is corresponding to
asynchronous transactions on IEEE 1394 bus. This is also achieved with
device entry.

This commit changes the processing of model-dependent quirk with the
entry. As a result, the transactions are sent only for Loud models.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/oxfw/oxfw.c | 38 ++++++++++++++++++++++----------------
 sound/firewire/oxfw/oxfw.h |  2 ++
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 0304d45..836d757 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -59,6 +59,7 @@ static bool detect_loud_models(struct fw_unit *unit)
 static int name_card(struct snd_oxfw *oxfw)
 {
 	struct fw_device *fw_dev = fw_parent_device(oxfw->unit);
+	const struct device_info *info;
 	char vendor[24];
 	char model[32];
 	const char *d, *v, *m;
@@ -84,10 +85,12 @@ static int name_card(struct snd_oxfw *oxfw)
 	be32_to_cpus(&firmware);
 
 	/* to apply card definitions */
-	if (oxfw->device_info) {
-		d = oxfw->device_info->driver_name;
-		v = oxfw->device_info->vendor_name;
-		m = oxfw->device_info->model_name;
+	if (oxfw->entry->vendor_id == VENDOR_GRIFFIN ||
+	    oxfw->entry->vendor_id == VENDOR_LACIE) {
+		info = (const struct device_info *)oxfw->entry->driver_data;
+		d = info->driver_name;
+		v = info->vendor_name;
+		m = info->model_name;
 	} else {
 		d = "OXFW";
 		v = vendor;
@@ -139,6 +142,16 @@ static void detect_quirks(struct snd_oxfw *oxfw)
 	int key, val;
 	int vendor, model;
 
+	/*
+	 * TASCAM FireOne has physical control and requires a pair of additional
+	 * MIDI ports.
+	 */
+	if (oxfw->entry->vendor_id == VENDOR_TASCAM) {
+		oxfw->midi_input_ports++;
+		oxfw->midi_output_ports++;
+		return;
+	}
+
 	/* Seek from Root Directory of Config ROM. */
 	vendor = model = 0;
 	fw_csr_iterator_init(&it, fw_dev->config_rom + 5);
@@ -155,25 +168,16 @@ static void detect_quirks(struct snd_oxfw *oxfw)
 	 */
 	if (vendor == VENDOR_LOUD && model == MODEL_SATELLITE)
 		oxfw->wrong_dbs = true;
-
-	/*
-	 * TASCAM FireOne has physical control and requires a pair of additional
-	 * MIDI ports.
-	 */
-	if (vendor == VENDOR_TASCAM) {
-		oxfw->midi_input_ports++;
-		oxfw->midi_output_ports++;
-	}
 }
 
 static int oxfw_probe(struct fw_unit *unit,
-		       const struct ieee1394_device_id *id)
+		      const struct ieee1394_device_id *entry)
 {
 	struct snd_card *card;
 	struct snd_oxfw *oxfw;
 	int err;
 
-	if ((id->vendor_id == VENDOR_LOUD) && !detect_loud_models(unit))
+	if (entry->vendor_id == VENDOR_LOUD && !detect_loud_models(unit))
 		return -ENODEV;
 
 	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
@@ -186,7 +190,7 @@ static int oxfw_probe(struct fw_unit *unit,
 	oxfw->card = card;
 	mutex_init(&oxfw->mutex);
 	oxfw->unit = fw_unit_get(unit);
-	oxfw->device_info = (const struct device_info *)id->driver_data;
+	oxfw->entry = entry;
 	spin_lock_init(&oxfw->lock);
 	init_waitqueue_head(&oxfw->hwdep_wait);
 
@@ -205,6 +209,8 @@ static int oxfw_probe(struct fw_unit *unit,
 		goto error;
 
 	if (oxfw->device_info) {
+		oxfw->device_info =
+				(const struct device_info *)entry->driver_data;
 		err = snd_oxfw_add_spkr(oxfw);
 		if (err < 0)
 			goto error;
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h
index 9efdc02..f3e14ff 100644
--- a/sound/firewire/oxfw/oxfw.h
+++ b/sound/firewire/oxfw/oxfw.h
@@ -72,6 +72,8 @@ struct snd_oxfw {
 	int dev_lock_count;
 	bool dev_lock_changed;
 	wait_queue_head_t hwdep_wait;
+
+	const struct ieee1394_device_id *entry;
 };
 
 /*
-- 
2.5.0

[PATCH 5/5] ALSA: oxfw: gather model-dependent conditions to a function

[Takashi Sakamoto]

Adding control elements is just for models supported by old
firewire-speakers modules. The processing should be in a function to add
model-dependent quirk.

This commit moves the codes to the function. As a result, the function
should handle error state, thus this commit also changes prototype of
the function.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/oxfw/oxfw.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 836d757..d4fb3c1 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -135,7 +135,7 @@ static void oxfw_card_free(struct snd_card *card)
 	mutex_destroy(&oxfw->mutex);
 }
 
-static void detect_quirks(struct snd_oxfw *oxfw)
+static int detect_quirks(struct snd_oxfw *oxfw)
 {
 	struct fw_device *fw_dev = fw_parent_device(oxfw->unit);
 	struct fw_csr_iterator it;
@@ -143,13 +143,24 @@ static void detect_quirks(struct snd_oxfw *oxfw)
 	int vendor, model;
 
 	/*
+	 * Add ALSA control elements for two models to keep compatibility to
+	 * old firewire-speaker module.
+	 */
+	if (oxfw->entry->vendor_id == VENDOR_GRIFFIN ||
+	    oxfw->entry->vendor_id == VENDOR_LACIE) {
+		oxfw->device_info =
+			(const struct device_info *)oxfw->entry->driver_data;
+		return snd_oxfw_add_spkr(oxfw);
+	}
+
+	/*
 	 * TASCAM FireOne has physical control and requires a pair of additional
 	 * MIDI ports.
 	 */
 	if (oxfw->entry->vendor_id == VENDOR_TASCAM) {
 		oxfw->midi_input_ports++;
 		oxfw->midi_output_ports++;
-		return;
+		return 0;
 	}
 
 	/* Seek from Root Directory of Config ROM. */
@@ -168,6 +179,8 @@ static void detect_quirks(struct snd_oxfw *oxfw)
 	 */
 	if (vendor == VENDOR_LOUD && model == MODEL_SATELLITE)
 		oxfw->wrong_dbs = true;
+
+	return 0;
 }
 
 static int oxfw_probe(struct fw_unit *unit,
@@ -198,7 +211,9 @@ static int oxfw_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
-	detect_quirks(oxfw);
+	err = detect_quirks(oxfw);
+	if (err < 0)
+		goto error;
 
 	err = name_card(oxfw);
 	if (err < 0)
@@ -208,14 +223,6 @@ static int oxfw_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
-	if (oxfw->device_info) {
-		oxfw->device_info =
-				(const struct device_info *)entry->driver_data;
-		err = snd_oxfw_add_spkr(oxfw);
-		if (err < 0)
-			goto error;
-	}
-
 	snd_oxfw_proc_init(oxfw);
 
 	err = snd_oxfw_create_midi(oxfw);
-- 
2.5.0

Re: [PATCH 0/5] ALSA: oxfw: code refactoring for model-dependent quirks

[Takashi Iwai]

On Tue, 15 Dec 2015 15:56:16 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> ALSA oxfw driver has got some patches to expand its functionalities. As a
> result, current code includes some inefficiencies and confusions.
> 
> This patchset is a code refactoring about model-dependent quirks. This
> patchset is a part of my previous RFCv2 (patch 01-05) and a preparation for
> the rest of patches:
> 
> [alsa-devel] [RFC][PATCH 00/17 v2] ALSA: oxfw: refactoring and merging scs1x module
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101558.html
> 
> Takashi Sakamoto (5):
>   ALSA: oxfw: rename a file for control elements so that it's for
>     model-specific
>   ALSA: oxfw: rename local functions for control elements so that they
>     represent as local
>   ALSA: oxfw: change function prototype for AV/C Audio Subunit command
>   ALSA: oxfw: reuse driver entry to detect quirk
>   ALSA: oxfw: gather model-dependent conditions to a function

Applied all five patches.  Thanks.


Takashi

Re: [PATCH 0/5] ALSA: oxfw: code refactoring for model-dependent quirks

[Takashi Sakamoto]

On Dec 16 2015 17:46, Takashi Iwai wrote:
> On Tue, 15 Dec 2015 15:56:16 +0100,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> ALSA oxfw driver has got some patches to expand its functionalities. As a
>> result, current code includes some inefficiencies and confusions.
>>
>> This patchset is a code refactoring about model-dependent quirks. This
>> patchset is a part of my previous RFCv2 (patch 01-05) and a preparation for
>> the rest of patches:
>>
>> [alsa-devel] [RFC][PATCH 00/17 v2] ALSA: oxfw: refactoring and merging scs1x module
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101558.html
>>
>> Takashi Sakamoto (5):
>>   ALSA: oxfw: rename a file for control elements so that it's for
>>     model-specific
>>   ALSA: oxfw: rename local functions for control elements so that they
>>     represent as local
>>   ALSA: oxfw: change function prototype for AV/C Audio Subunit command
>>   ALSA: oxfw: reuse driver entry to detect quirk
>>   ALSA: oxfw: gather model-dependent conditions to a function
> 
> Applied all five patches.  Thanks.

Thanks.

If possible, would you please apply fireworks patches on
'topic/firewire-update' to 'for-next' branch, too?


Regards

Takashi Sakamoto

Re: [PATCH 0/5] ALSA: oxfw: code refactoring for model-dependent quirks

[Takashi Iwai]

On Wed, 16 Dec 2015 12:03:16 +0100,
Takashi Sakamoto wrote:
> 
> On Dec 16 2015 17:46, Takashi Iwai wrote:
> > On Tue, 15 Dec 2015 15:56:16 +0100,
> > Takashi Sakamoto wrote:
> >>
> >> Hi,
> >>
> >> ALSA oxfw driver has got some patches to expand its functionalities. As a
> >> result, current code includes some inefficiencies and confusions.
> >>
> >> This patchset is a code refactoring about model-dependent quirks. This
> >> patchset is a part of my previous RFCv2 (patch 01-05) and a preparation for
> >> the rest of patches:
> >>
> >> [alsa-devel] [RFC][PATCH 00/17 v2] ALSA: oxfw: refactoring and merging scs1x module
> >> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101558.html
> >>
> >> Takashi Sakamoto (5):
> >>   ALSA: oxfw: rename a file for control elements so that it's for
> >>     model-specific
> >>   ALSA: oxfw: rename local functions for control elements so that they
> >>     represent as local
> >>   ALSA: oxfw: change function prototype for AV/C Audio Subunit command
> >>   ALSA: oxfw: reuse driver entry to detect quirk
> >>   ALSA: oxfw: gather model-dependent conditions to a function
> > 
> > Applied all five patches.  Thanks.
> 
> Thanks.
> 
> If possible, would you please apply fireworks patches on
> 'topic/firewire-update' to 'for-next' branch, too?

Oh, missed that one.  Merged now.


thanks,

Takashi