[alsa-devel] [PATCH 0/4] ALSA: oxfw: localize stuffs of old firewire-speakers module
Hi,
ALSA oxfw module inherits some stuffs from old firewire-speakers module. Currently, the module's common structure has some members for the stuffs. This is not good because these stuffs are not common between all of supported models. It's better to localize them.
The purpose of this patchset is to enable the module to have memory block and push model-specific stuffs to each structure. This patchset is a part of my previous RFCv2 (patch 06-09) and a preparation for rest of the 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.ht...
Takashi Sakamoto (4): ALSA: oxfw: enable to keep memory block for model-specific structure ALSA: oxfw: move model-specific members from common structure ALSA: oxfw: move model-specific parameters from common structure ALSA: oxfw: rename a structure so that it means backward compatibility to old drivers
sound/firewire/oxfw/oxfw-spkr.c | 96 ++++++++++++++++++++++++++--------------- sound/firewire/oxfw/oxfw.c | 31 +++++++------ sound/firewire/oxfw/oxfw.h | 17 +------- 3 files changed, 79 insertions(+), 65 deletions(-)
ALSA oxfw driver should have backward compatibility to old firewire-speakers driver. Additionally, in future commit, scs1x driver will be merged. It's nice to add a pointer to have a memory block for model-specific structures.
This commit adds a member to 'struct snd_oxfw' for this aim. Deallocation is done at freeing ALSA card structure.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/oxfw/oxfw.c | 1 + sound/firewire/oxfw/oxfw.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c index d4fb3c1..7e50a4f 100644 --- a/sound/firewire/oxfw/oxfw.c +++ b/sound/firewire/oxfw/oxfw.c @@ -132,6 +132,7 @@ static void oxfw_card_free(struct snd_card *card) kfree(oxfw->rx_stream_formats[i]); }
+ kfree(oxfw->spec); mutex_destroy(&oxfw->mutex); }
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h index f3e14ff..9625661 100644 --- a/sound/firewire/oxfw/oxfw.h +++ b/sound/firewire/oxfw/oxfw.h @@ -74,6 +74,7 @@ struct snd_oxfw { wait_queue_head_t hwdep_wait;
const struct ieee1394_device_id *entry; + void *spec; };
/*
Currently, 'struct snd_oxfw' has some members for models supported by old firewire-speakers driver, while these members are useless to the other models.
This commit allocates new memory block and moves these members to model-specific structure.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/oxfw/oxfw-spkr.c | 48 +++++++++++++++++++++++++++++------------ sound/firewire/oxfw/oxfw.h | 5 ----- 2 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw-spkr.c b/sound/firewire/oxfw/oxfw-spkr.c index d733a15..fbdd432 100644 --- a/sound/firewire/oxfw/oxfw-spkr.c +++ b/sound/firewire/oxfw/oxfw-spkr.c @@ -7,6 +7,13 @@
#include "oxfw.h"
+struct fw_spkr { + bool mute; + s16 volume[6]; + s16 volume_min; + s16 volume_max; +}; + enum control_action { CTL_READ, CTL_WRITE }; enum control_attribute { CTL_MIN = 0x02, @@ -135,8 +142,9 @@ static int spkr_mute_get(struct snd_kcontrol *control, struct snd_ctl_elem_value *value) { struct snd_oxfw *oxfw = control->private_data; + struct fw_spkr *spkr = oxfw->spec;
- value->value.integer.value[0] = !oxfw->mute; + value->value.integer.value[0] = !spkr->mute;
return 0; } @@ -145,19 +153,20 @@ static int spkr_mute_put(struct snd_kcontrol *control, struct snd_ctl_elem_value *value) { struct snd_oxfw *oxfw = control->private_data; + struct fw_spkr *spkr = oxfw->spec; bool mute; int err;
mute = !value->value.integer.value[0];
- if (mute == oxfw->mute) + if (mute == spkr->mute) return 0;
err = avc_audio_feature_mute(oxfw->unit, oxfw->device_info->mute_fb_id, &mute, CTL_WRITE); if (err < 0) return err; - oxfw->mute = mute; + spkr->mute = mute;
return 1; } @@ -166,11 +175,12 @@ static int spkr_volume_info(struct snd_kcontrol *control, struct snd_ctl_elem_info *info) { struct snd_oxfw *oxfw = control->private_data; + struct fw_spkr *spkr = oxfw->spec;
info->type = SNDRV_CTL_ELEM_TYPE_INTEGER; info->count = oxfw->device_info->mixer_channels; - info->value.integer.min = oxfw->volume_min; - info->value.integer.max = oxfw->volume_max; + info->value.integer.min = spkr->volume_min; + info->value.integer.max = spkr->volume_max;
return 0; } @@ -181,10 +191,11 @@ static int spkr_volume_get(struct snd_kcontrol *control, struct snd_ctl_elem_value *value) { struct snd_oxfw *oxfw = control->private_data; + struct fw_spkr *spkr = oxfw->spec; unsigned int i;
for (i = 0; i < oxfw->device_info->mixer_channels; ++i) - value->value.integer.value[channel_map[i]] = oxfw->volume[i]; + value->value.integer.value[channel_map[i]] = spkr->volume[i];
return 0; } @@ -193,14 +204,15 @@ static int spkr_volume_put(struct snd_kcontrol *control, struct snd_ctl_elem_value *value) { struct snd_oxfw *oxfw = control->private_data; + struct fw_spkr *spkr = oxfw->spec; unsigned int i, changed_channels; bool equal_values = true; s16 volume; int err;
for (i = 0; i < oxfw->device_info->mixer_channels; ++i) { - if (value->value.integer.value[i] < oxfw->volume_min || - value->value.integer.value[i] > oxfw->volume_max) + if (value->value.integer.value[i] < spkr->volume_min || + value->value.integer.value[i] > spkr->volume_max) return -EINVAL; if (value->value.integer.value[i] != value->value.integer.value[0]) @@ -210,7 +222,7 @@ static int spkr_volume_put(struct snd_kcontrol *control, changed_channels = 0; for (i = 0; i < oxfw->device_info->mixer_channels; ++i) if (value->value.integer.value[channel_map[i]] != - oxfw->volume[i]) + spkr->volume[i]) changed_channels |= 1 << (i + 1);
if (equal_values && changed_channels != 0) @@ -227,7 +239,7 @@ static int spkr_volume_put(struct snd_kcontrol *control, return err; } if (i > 0) - oxfw->volume[i - 1] = volume; + spkr->volume[i - 1] = volume; }
return changed_channels != 0; @@ -251,22 +263,30 @@ int snd_oxfw_add_spkr(struct snd_oxfw *oxfw) .put = spkr_volume_put, }, }; + struct fw_spkr *spkr; unsigned int i, first_ch; int err;
+ spkr = kzalloc(sizeof(struct fw_spkr), GFP_KERNEL); + if (spkr == NULL) + return -ENOMEM; + oxfw->spec = spkr; + err = avc_audio_feature_volume(oxfw->unit, oxfw->device_info->volume_fb_id, - &oxfw->volume_min, 0, CTL_MIN, CTL_READ); + &spkr->volume_min, + 0, CTL_MIN, CTL_READ); if (err < 0) return err; err = avc_audio_feature_volume(oxfw->unit, oxfw->device_info->volume_fb_id, - &oxfw->volume_max, 0, CTL_MAX, CTL_READ); + &spkr->volume_max, + 0, CTL_MAX, CTL_READ); if (err < 0) return err;
err = avc_audio_feature_mute(oxfw->unit, oxfw->device_info->mute_fb_id, - &oxfw->mute, CTL_READ); + &spkr->mute, CTL_READ); if (err < 0) return err;
@@ -274,7 +294,7 @@ int snd_oxfw_add_spkr(struct snd_oxfw *oxfw) for (i = 0; i < oxfw->device_info->mixer_channels; ++i) { err = avc_audio_feature_volume(oxfw->unit, oxfw->device_info->volume_fb_id, - &oxfw->volume[i], + &spkr->volume[i], first_ch + i, CTL_CURRENT, CTL_READ); if (err < 0) return err; diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h index 9625661..046cd33 100644 --- a/sound/firewire/oxfw/oxfw.h +++ b/sound/firewire/oxfw/oxfw.h @@ -64,11 +64,6 @@ struct snd_oxfw { unsigned int midi_input_ports; unsigned int midi_output_ports;
- bool mute; - s16 volume[6]; - s16 volume_min; - s16 volume_max; - int dev_lock_count; bool dev_lock_changed; wait_queue_head_t hwdep_wait;
In previous commit, some members are moved from 'struct snd_oxfw' because they're model-specific. There are also the other model-specific parameters in 'struct device_info'.
This commit moves these members to model-specific structure.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/oxfw/oxfw-spkr.c | 60 +++++++++++++++++++++++------------------ sound/firewire/oxfw/oxfw.c | 16 +++-------- sound/firewire/oxfw/oxfw.h | 5 +--- 3 files changed, 39 insertions(+), 42 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw-spkr.c b/sound/firewire/oxfw/oxfw-spkr.c index fbdd432..cb905af 100644 --- a/sound/firewire/oxfw/oxfw-spkr.c +++ b/sound/firewire/oxfw/oxfw-spkr.c @@ -12,6 +12,10 @@ struct fw_spkr { s16 volume[6]; s16 volume_min; s16 volume_max; + + unsigned int mixer_channels; + u8 mute_fb_id; + u8 volume_fb_id; };
enum control_action { CTL_READ, CTL_WRITE }; @@ -162,8 +166,8 @@ static int spkr_mute_put(struct snd_kcontrol *control, if (mute == spkr->mute) return 0;
- err = avc_audio_feature_mute(oxfw->unit, oxfw->device_info->mute_fb_id, - &mute, CTL_WRITE); + err = avc_audio_feature_mute(oxfw->unit, spkr->mute_fb_id, &mute, + CTL_WRITE); if (err < 0) return err; spkr->mute = mute; @@ -178,7 +182,7 @@ static int spkr_volume_info(struct snd_kcontrol *control, struct fw_spkr *spkr = oxfw->spec;
info->type = SNDRV_CTL_ELEM_TYPE_INTEGER; - info->count = oxfw->device_info->mixer_channels; + info->count = spkr->mixer_channels; info->value.integer.min = spkr->volume_min; info->value.integer.max = spkr->volume_max;
@@ -194,7 +198,7 @@ static int spkr_volume_get(struct snd_kcontrol *control, struct fw_spkr *spkr = oxfw->spec; unsigned int i;
- for (i = 0; i < oxfw->device_info->mixer_channels; ++i) + for (i = 0; i < spkr->mixer_channels; ++i) value->value.integer.value[channel_map[i]] = spkr->volume[i];
return 0; @@ -210,7 +214,7 @@ static int spkr_volume_put(struct snd_kcontrol *control, s16 volume; int err;
- for (i = 0; i < oxfw->device_info->mixer_channels; ++i) { + for (i = 0; i < spkr->mixer_channels; ++i) { if (value->value.integer.value[i] < spkr->volume_min || value->value.integer.value[i] > spkr->volume_max) return -EINVAL; @@ -220,7 +224,7 @@ static int spkr_volume_put(struct snd_kcontrol *control, }
changed_channels = 0; - for (i = 0; i < oxfw->device_info->mixer_channels; ++i) + for (i = 0; i < spkr->mixer_channels; ++i) if (value->value.integer.value[channel_map[i]] != spkr->volume[i]) changed_channels |= 1 << (i + 1); @@ -228,12 +232,11 @@ static int spkr_volume_put(struct snd_kcontrol *control, if (equal_values && changed_channels != 0) changed_channels = 1 << 0;
- for (i = 0; i <= oxfw->device_info->mixer_channels; ++i) { + for (i = 0; i <= spkr->mixer_channels; ++i) { volume = value->value.integer.value[channel_map[i ? i - 1 : 0]]; if (changed_channels & (1 << i)) { err = avc_audio_feature_volume(oxfw->unit, - oxfw->device_info->mute_fb_id, - &volume, + spkr->volume_fb_id, &volume, i, CTL_CURRENT, CTL_WRITE); if (err < 0) return err; @@ -245,7 +248,7 @@ static int spkr_volume_put(struct snd_kcontrol *control, return changed_channels != 0; }
-int snd_oxfw_add_spkr(struct snd_oxfw *oxfw) +int snd_oxfw_add_spkr(struct snd_oxfw *oxfw, bool is_lacie) { static const struct snd_kcontrol_new controls[] = { { @@ -272,30 +275,35 @@ int snd_oxfw_add_spkr(struct snd_oxfw *oxfw) return -ENOMEM; oxfw->spec = spkr;
- err = avc_audio_feature_volume(oxfw->unit, - oxfw->device_info->volume_fb_id, - &spkr->volume_min, - 0, CTL_MIN, CTL_READ); + if (is_lacie) { + spkr->mixer_channels = 1; + spkr->mute_fb_id = 0x01; + spkr->volume_fb_id = 0x01; + } else { + spkr->mixer_channels = 6; + spkr->mute_fb_id = 0x01; + spkr->volume_fb_id = 0x02; + } + + err = avc_audio_feature_volume(oxfw->unit, spkr->volume_fb_id, + &spkr->volume_min, 0, CTL_MIN, CTL_READ); if (err < 0) return err; - err = avc_audio_feature_volume(oxfw->unit, - oxfw->device_info->volume_fb_id, - &spkr->volume_max, - 0, CTL_MAX, CTL_READ); + err = avc_audio_feature_volume(oxfw->unit, spkr->volume_fb_id, + &spkr->volume_max, 0, CTL_MAX, CTL_READ); if (err < 0) return err;
- err = avc_audio_feature_mute(oxfw->unit, oxfw->device_info->mute_fb_id, - &spkr->mute, CTL_READ); + err = avc_audio_feature_mute(oxfw->unit, spkr->mute_fb_id, &spkr->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 = avc_audio_feature_volume(oxfw->unit, - oxfw->device_info->volume_fb_id, - &spkr->volume[i], - first_ch + i, CTL_CURRENT, CTL_READ); + first_ch = spkr->mixer_channels == 1 ? 0 : 1; + for (i = 0; i < spkr->mixer_channels; ++i) { + err = avc_audio_feature_volume(oxfw->unit, spkr->volume_fb_id, + &spkr->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 7e50a4f..16ee6ea 100644 --- a/sound/firewire/oxfw/oxfw.c +++ b/sound/firewire/oxfw/oxfw.c @@ -147,12 +147,10 @@ static int detect_quirks(struct snd_oxfw *oxfw) * 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); - } + if (oxfw->entry->vendor_id == VENDOR_GRIFFIN) + return snd_oxfw_add_spkr(oxfw, false); + if (oxfw->entry->vendor_id == VENDOR_LACIE) + return snd_oxfw_add_spkr(oxfw, true);
/* * TASCAM FireOne has physical control and requires a pair of additional @@ -285,18 +283,12 @@ static const struct device_info griffin_firewave = { .driver_name = "FireWave", .vendor_name = "Griffin", .model_name = "FireWave", - .mixer_channels = 6, - .mute_fb_id = 0x01, - .volume_fb_id = 0x02, };
static const struct device_info lacie_speakers = { .driver_name = "FWSpeakers", .vendor_name = "LaCie", .model_name = "FireWire Speakers", - .mixer_channels = 1, - .mute_fb_id = 0x01, - .volume_fb_id = 0x01, };
static const struct ieee1394_device_id oxfw_id_table[] = { diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h index 046cd33..6038150 100644 --- a/sound/firewire/oxfw/oxfw.h +++ b/sound/firewire/oxfw/oxfw.h @@ -35,9 +35,6 @@ struct device_info { const char *driver_name; const char *vendor_name; const char *model_name; - unsigned int mixer_channels; - u8 mute_fb_id; - u8 volume_fb_id; };
/* This is an arbitrary number for convinience. */ @@ -142,4 +139,4 @@ 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); +int snd_oxfw_add_spkr(struct snd_oxfw *oxfw, bool is_lacie);
In former commits, some model-specific members are split from the structure. The structure is just to keep names for compatibility to old drivers.
This commit arranges name of the structure and localize it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/oxfw/oxfw.c | 14 ++++++++++---- sound/firewire/oxfw/oxfw.h | 6 ------ 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c index 16ee6ea..96fbb78 100644 --- a/sound/firewire/oxfw/oxfw.c +++ b/sound/firewire/oxfw/oxfw.c @@ -30,6 +30,12 @@ MODULE_AUTHOR("Clemens Ladisch clemens@ladisch.de"); MODULE_LICENSE("GPL v2"); MODULE_ALIAS("snd-firewire-speakers");
+struct compat_info { + const char *driver_name; + const char *vendor_name; + const char *model_name; +}; + static bool detect_loud_models(struct fw_unit *unit) { const char *const models[] = { @@ -59,7 +65,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; + const struct compat_info *info; char vendor[24]; char model[32]; const char *d, *v, *m; @@ -87,7 +93,7 @@ static int name_card(struct snd_oxfw *oxfw) /* to apply card definitions */ if (oxfw->entry->vendor_id == VENDOR_GRIFFIN || oxfw->entry->vendor_id == VENDOR_LACIE) { - info = (const struct device_info *)oxfw->entry->driver_data; + info = (const struct compat_info *)oxfw->entry->driver_data; d = info->driver_name; v = info->vendor_name; m = info->model_name; @@ -279,13 +285,13 @@ static void oxfw_remove(struct fw_unit *unit) snd_card_free_when_closed(oxfw->card); }
-static const struct device_info griffin_firewave = { +static const struct compat_info griffin_firewave = { .driver_name = "FireWave", .vendor_name = "Griffin", .model_name = "FireWave", };
-static const struct device_info lacie_speakers = { +static const struct compat_info lacie_speakers = { .driver_name = "FWSpeakers", .vendor_name = "LaCie", .model_name = "FireWire Speakers", diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h index 6038150..1c9844a 100644 --- a/sound/firewire/oxfw/oxfw.h +++ b/sound/firewire/oxfw/oxfw.h @@ -31,12 +31,6 @@ #include "../amdtp-am824.h" #include "../cmp.h"
-struct device_info { - const char *driver_name; - const char *vendor_name; - const char *model_name; -}; - /* This is an arbitrary number for convinience. */ #define SND_OXFW_STREAM_FORMAT_ENTRIES 10 struct snd_oxfw {
participants (1)
-
Takashi Sakamoto