From: Takashi S. <o-t...@sa...> - 2014-07-21 02:10:28
|
This commit adds some dev_err() to help debug when avc_maudio_set_special_clk() failed. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- sound/firewire/bebob/bebob_maudio.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c index 83f8d5b..008ff2c 100644 --- a/sound/firewire/bebob/bebob_maudio.c +++ b/sound/firewire/bebob/bebob_maudio.c @@ -393,6 +393,10 @@ static int special_clk_ctl_set(struct snd_kcontrol *kctl, mutex_unlock(&bebob->mutex); + if (err < 0) + dev_err(&bebob->unit->device, + "fail to change clock source: %d\n", err); + return err >= 0; } static struct snd_kcontrol_new special_clk_ctl = { @@ -506,7 +510,14 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, dig_in_fmt, params->dig_out_fmt, params->clk_lock); - if ((err < 0) || (params->dig_in_fmt > 0)) /* ADAT */ + if (err < 0) { + dev_err(&bebob->unit->device, + "fail to change clock source: %d\n", err); + goto end; + } + + /* For ADAT, optical interface is only available. */ + if (params->dig_in_fmt > 0) goto end; err = avc_audio_set_selector(bebob->unit, 0x00, 0x04, dig_in_iface); @@ -567,10 +578,13 @@ static int special_dig_out_iface_ctl_set(struct snd_kcontrol *kctl, params->clk_src, params->dig_in_fmt, id, params->clk_lock); - if (err >= 0) - special_stream_formation_set(bebob); mutex_unlock(&bebob->mutex); + + if (err < 0) + dev_err(&bebob->unit->device, + "fail to change digital output interface: %d\n", err); + return err; } static struct snd_kcontrol_new special_dig_out_iface_ctl = { -- 1.9.1 |
From: Takashi S. <o-t...@sa...> - 2014-07-21 02:10:25
|
This series of patch improves some control elements for M-Audio specific operations. Current implementation includes dead lock and my misunderstanding about hardware specification and ALSA Control interface. Takashi Sakamoto (6): bebob: Arrangement for critical section to be shorter bebob: Reducing function callers for simplicity bebob: Add dev_err() for debugging bebob: Use different labels for digital input/output interfaces bebob: Correction for return value of .put callback functions bebob: Arrangement for a control element to which two settings relate sound/firewire/bebob/bebob_maudio.c | 181 ++++++++++++++++++++++-------------- 1 file changed, 111 insertions(+), 70 deletions(-) -- 1.9.1 |
From: Takashi S. <o-t...@sa...> - 2014-07-21 02:10:25
|
Cc: Wei Yongjun <wei...@16...> This commit move some mutex_lock() to shorten critical section in some functions. This commit is my solution for this post. [PATCH -next] ALSA: bebob: Fix missing unlock on error in special_clk_ctl_put() https://lkml.org/lkml/2014/7/20/12 This commit also renames a function for my naming consistency. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- sound/firewire/bebob/bebob_maudio.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c index 6af50eb..0a33045 100644 --- a/sound/firewire/bebob/bebob_maudio.c +++ b/sound/firewire/bebob/bebob_maudio.c @@ -372,23 +372,24 @@ static int special_clk_ctl_get(struct snd_kcontrol *kctl, uval->value.enumerated.item[0] = params->clk_src; return 0; } -static int special_clk_ctl_put(struct snd_kcontrol *kctl, +static int special_clk_ctl_set(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *uval) { struct snd_bebob *bebob = snd_kcontrol_chip(kctl); struct special_params *params = bebob->maudio_special_quirk; int err, id; - mutex_lock(&bebob->mutex); - id = uval->value.enumerated.item[0]; if (id >= ARRAY_SIZE(special_clk_labels)) return 0; + mutex_lock(&bebob->mutex); + err = avc_maudio_set_special_clk(bebob, id, params->dig_in_fmt, params->dig_out_fmt, params->clk_lock); + mutex_unlock(&bebob->mutex); return err >= 0; @@ -399,7 +400,7 @@ static struct snd_kcontrol_new special_clk_ctl = { .access = SNDRV_CTL_ELEM_ACCESS_READWRITE, .info = special_clk_ctl_info, .get = special_clk_ctl_get, - .put = special_clk_ctl_put + .put = special_clk_ctl_set }; /* Clock synchronization control for special firmware */ @@ -491,14 +492,14 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, unsigned int id, dig_in_fmt, dig_in_iface; int err; - mutex_lock(&bebob->mutex); - id = uval->value.enumerated.item[0]; /* decode user value */ dig_in_fmt = (id >> 1) & 0x01; dig_in_iface = id & 0x01; + mutex_lock(&bebob->mutex); + err = avc_maudio_set_special_clk(bebob, params->clk_src, dig_in_fmt, @@ -558,10 +559,10 @@ static int special_dig_out_iface_ctl_set(struct snd_kcontrol *kctl, unsigned int id; int err; - mutex_lock(&bebob->mutex); - id = uval->value.enumerated.item[0]; + mutex_lock(&bebob->mutex); + err = avc_maudio_set_special_clk(bebob, params->clk_src, params->dig_in_fmt, -- 1.9.1 |
From: Takashi S. <o-t...@sa...> - 2014-07-21 02:10:28
|
M-Audio Firewire 1814 and ProjectMix I/O change their stream formation according to current digital format for input/output, although drivers cannot recognize current formation by any commands. Thus the drivers need to remember the formations. The special_stream_formation_set() is for this purpose. This function is expected to be called just after current digital formats are changed. The way to change current digital format is to call avc_maudio_set_special_clk() and there is no other ways. For simplicity, this commit make the avc_maudio_set_special_clk() as a sole caller for the special_stream_formation_set(). Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- sound/firewire/bebob/bebob_maudio.c | 62 ++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c index 0a33045..83f8d5b 100644 --- a/sound/firewire/bebob/bebob_maudio.c +++ b/sound/firewire/bebob/bebob_maudio.c @@ -160,6 +160,35 @@ end: return err; } +static void +special_stream_formation_set(struct snd_bebob *bebob) +{ + static const unsigned int ch_table[2][2][3] = { + /* AMDTP_OUT_STREAM */ + { { 6, 6, 4 }, /* SPDIF */ + { 12, 8, 4 } }, /* ADAT */ + /* AMDTP_IN_STREAM */ + { { 10, 10, 2 }, /* SPDIF */ + { 16, 12, 2 } } /* ADAT */ + }; + struct special_params *params = bebob->maudio_special_quirk; + unsigned int i, max; + + max = SND_BEBOB_STRM_FMT_ENTRIES - 1; + if (!params->is1814) + max -= 2; + + for (i = 0; i < max; i++) { + bebob->tx_stream_formations[i + 1].pcm = + ch_table[AMDTP_IN_STREAM][params->dig_in_fmt][i / 2]; + bebob->tx_stream_formations[i + 1].midi = 1; + + bebob->rx_stream_formations[i + 1].pcm = + ch_table[AMDTP_OUT_STREAM][params->dig_out_fmt][i / 2]; + bebob->rx_stream_formations[i + 1].midi = 1; + } +} + /* * dig_fmt: 0x00:S/PDIF, 0x01:ADAT * clk_lock: 0x00:unlock, 0x01:lock @@ -212,6 +241,8 @@ avc_maudio_set_special_clk(struct snd_bebob *bebob, unsigned int clk_src, params->dig_out_fmt = buf[8]; params->clk_lock = buf[9]; + special_stream_formation_set(bebob); + if (params->ctl_id_sync) snd_ctl_notify(bebob->card, SNDRV_CTL_EVENT_MASK_VALUE, params->ctl_id_sync); @@ -221,34 +252,6 @@ end: kfree(buf); return err; } -static void -special_stream_formation_set(struct snd_bebob *bebob) -{ - static const unsigned int ch_table[2][2][3] = { - /* AMDTP_OUT_STREAM */ - { { 6, 6, 4 }, /* SPDIF */ - { 12, 8, 4 } }, /* ADAT */ - /* AMDTP_IN_STREAM */ - { { 10, 10, 2 }, /* SPDIF */ - { 16, 12, 2 } } /* ADAT */ - }; - struct special_params *params = bebob->maudio_special_quirk; - unsigned int i, max; - - max = SND_BEBOB_STRM_FMT_ENTRIES - 1; - if (!params->is1814) - max -= 2; - - for (i = 0; i < max; i++) { - bebob->tx_stream_formations[i + 1].pcm = - ch_table[AMDTP_IN_STREAM][params->dig_in_fmt][i / 2]; - bebob->tx_stream_formations[i + 1].midi = 1; - - bebob->rx_stream_formations[i + 1].pcm = - ch_table[AMDTP_OUT_STREAM][params->dig_out_fmt][i / 2]; - bebob->rx_stream_formations[i + 1].midi = 1; - } -} static int add_special_controls(struct snd_bebob *bebob); int @@ -280,8 +283,6 @@ snd_bebob_maudio_special_discover(struct snd_bebob *bebob, bool is1814) if (err < 0) goto end; - special_stream_formation_set(bebob); - if (params->is1814) { bebob->midi_input_ports = 1; bebob->midi_output_ports = 1; @@ -513,7 +514,6 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, dev_err(&bebob->unit->device, "fail to set digital input interface: %d\n", err); end: - special_stream_formation_set(bebob); mutex_unlock(&bebob->mutex); return err; } -- 1.9.1 |
From: Takashi S. <o-t...@sa...> - 2014-07-21 02:10:31
|
A control element for digital input interface includes two settings; one is for digital format (S/PDIF or ADAT), another is for interface (optical or coaxial). The setting for interface is meaningful just for S/PDIF, therefore to consider recovery at failure of a operation for the second setting, a operation for the first setting should be for interface, not digital format. Additionally, this commit changes special_dig_in_iface_ctl_get() to reduce needless processing for the interface when the digital format is ADAT. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- sound/firewire/bebob/bebob_maudio.c | 70 +++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c index acfe94a..7899b56 100644 --- a/sound/firewire/bebob/bebob_maudio.c +++ b/sound/firewire/bebob/bebob_maudio.c @@ -471,21 +471,24 @@ static int special_dig_in_iface_ctl_get(struct snd_kcontrol *kctl, mutex_lock(&bebob->mutex); - err = avc_audio_get_selector(bebob->unit, 0x00, 0x04, - &dig_in_iface); - if (err < 0) { - dev_err(&bebob->unit->device, - "fail to get digital input interface: %d\n", err); - goto end; + /* For ADAT, optical interface is only available. */ + if (params->dig_in_fmt > 0) { + err = 0; + dig_in_iface = 0; + } else { + err = avc_audio_get_selector(bebob->unit, 0x00, 0x04, + &dig_in_iface); + if (err < 0) { + dev_err(&bebob->unit->device, + "fail to get digital input interface: %d\n", + err); + goto end; + } } /* encoded id for user value */ val = (params->dig_in_fmt << 1) | (dig_in_iface & 0x01); - /* for ADAT Optical */ - if (val > 2) - val = 2; - uval->value.enumerated.item[0] = val; end: mutex_unlock(&bebob->mutex); @@ -497,7 +500,7 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, struct snd_bebob *bebob = snd_kcontrol_chip(kctl); struct special_params *params = bebob->maudio_special_quirk; unsigned int id, dig_in_fmt, dig_in_iface; - int err; + int err = 0; id = uval->value.enumerated.item[0]; if (id >= ARRAY_SIZE(special_dig_in_iface_labels)) @@ -509,29 +512,36 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, mutex_lock(&bebob->mutex); - err = avc_maudio_set_special_clk(bebob, - params->clk_src, - dig_in_fmt, - params->dig_out_fmt, - params->clk_lock); - if (err < 0) { - dev_err(&bebob->unit->device, - "fail to change clock source: %d\n", err); - goto end; - } - - /* For ADAT, optical interface is only available. */ - if (params->dig_in_fmt > 0) { + /* For S/PDIF, optical/coaxial interfaces are selectable. */ + if (dig_in_fmt == 0) { + if (amdtp_stream_running(&bebob->rx_stream)) + goto end; + + err = avc_audio_set_selector(bebob->unit, 0x00, 0x04, + dig_in_iface); + if (err < 0) { + dev_err(&bebob->unit->device, + "fail to change digital input interface: %d\n", + err); + goto end; + } err = 1; - goto end; } - err = avc_audio_set_selector(bebob->unit, 0x00, 0x04, dig_in_iface); - if (err < 0) - dev_err(&bebob->unit->device, - "fail to set digital input interface: %d\n", err); - else + if (params->dig_in_fmt != dig_in_fmt) { + err = avc_maudio_set_special_clk(bebob, + params->clk_src, + dig_in_fmt, + params->dig_out_fmt, + params->clk_lock); + if (err < 0) { + dev_err(&bebob->unit->device, + "fail to change digital input format: %d\n", + err); + goto end; + } err = 1; + } end: mutex_unlock(&bebob->mutex); return err; -- 1.9.1 |
From: Takashi S. <o-t...@sa...> - 2014-07-21 02:10:34
|
This commit is for correction of my misunderstanding for return value of .put callback of ALSA Control interface. According to 'Writing ALSA Driver' (*1), return value of the callback has three patterns; 1: changed, 0: not changed, an negative value: fatal error. But I misunderstood that it's boolean; zero or nonzero. *1: Writing an ALSA Driver (2005, Takashi Iwai) http://www.alsa-project.org/main/index.php/ALSA_Driver_Documentation Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- sound/firewire/bebob/bebob_maudio.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c index f2277ce..acfe94a 100644 --- a/sound/firewire/bebob/bebob_maudio.c +++ b/sound/firewire/bebob/bebob_maudio.c @@ -396,8 +396,10 @@ static int special_clk_ctl_set(struct snd_kcontrol *kctl, if (err < 0) dev_err(&bebob->unit->device, "fail to change clock source: %d\n", err); + else + err = 1; - return err >= 0; + return err; } static struct snd_kcontrol_new special_clk_ctl = { .name = "Clock Source", @@ -519,13 +521,17 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, } /* For ADAT, optical interface is only available. */ - if (params->dig_in_fmt > 0) + if (params->dig_in_fmt > 0) { + err = 1; goto end; + } err = avc_audio_set_selector(bebob->unit, 0x00, 0x04, dig_in_iface); if (err < 0) dev_err(&bebob->unit->device, "fail to set digital input interface: %d\n", err); + else + err = 1; end: mutex_unlock(&bebob->mutex); return err; @@ -592,6 +598,8 @@ static int special_dig_out_iface_ctl_set(struct snd_kcontrol *kctl, if (err < 0) dev_err(&bebob->unit->device, "fail to change digital output interface: %d\n", err); + else + err = 1; return err; } -- 1.9.1 |
From: Takashi I. <ti...@su...> - 2014-07-21 07:06:36
|
At Mon, 21 Jul 2014 11:09:59 +0900, Takashi Sakamoto wrote: > > This series of patch improves some control elements for M-Audio specific > operations. Current implementation includes dead lock and my misunderstanding > about hardware specification and ALSA Control interface. I don't want to apply such a big series at the late stage of 3.16-rc. Please create short patches "just doing fix" for 3.16 as Yongjun did, and rebase comprehensive cleanups on it. thanks, Takashi > > Takashi Sakamoto (6): > bebob: Arrangement for critical section to be shorter > bebob: Reducing function callers for simplicity > bebob: Add dev_err() for debugging > bebob: Use different labels for digital input/output interfaces > bebob: Correction for return value of .put callback functions > bebob: Arrangement for a control element to which two settings relate > > sound/firewire/bebob/bebob_maudio.c | 181 ++++++++++++++++++++++-------------- > 1 file changed, 111 insertions(+), 70 deletions(-) > > -- > 1.9.1 > |
From: Takashi S. <o-t...@sa...> - 2014-07-21 02:10:27
|
This commit use different labels for control elements of digital input/output interfaces to correct my misunderstanding about M-Audio Firewire 1814 and ProjectMix I/O. According to user manuals for these two models, they have two modes for digital input; one is S/PDIF in both of optical and coaxial interfaces, another is ADAT in optical interface only. But in current implementation, a control element for digital input uses reduced labels which a control element for digital output uses because of my misunderstanding that optical interface is not available for digital input with S/PDIF mode. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- sound/firewire/bebob/bebob_maudio.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c index 008ff2c..f2277ce 100644 --- a/sound/firewire/bebob/bebob_maudio.c +++ b/sound/firewire/bebob/bebob_maudio.c @@ -440,8 +440,8 @@ static struct snd_kcontrol_new special_sync_ctl = { .get = special_sync_ctl_get, }; -/* Digital interface control for special firmware */ -static char *const special_dig_iface_labels[] = { +/* Digital input interface control for special firmware */ +static char *const special_dig_in_iface_labels[] = { "S/PDIF Optical", "S/PDIF Coaxial", "ADAT Optical" }; static int special_dig_in_iface_ctl_info(struct snd_kcontrol *kctl, @@ -449,13 +449,13 @@ static int special_dig_in_iface_ctl_info(struct snd_kcontrol *kctl, { einf->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; einf->count = 1; - einf->value.enumerated.items = ARRAY_SIZE(special_dig_iface_labels); + einf->value.enumerated.items = ARRAY_SIZE(special_dig_in_iface_labels); if (einf->value.enumerated.item >= einf->value.enumerated.items) einf->value.enumerated.item = einf->value.enumerated.items - 1; strcpy(einf->value.enumerated.name, - special_dig_iface_labels[einf->value.enumerated.item]); + special_dig_in_iface_labels[einf->value.enumerated.item]); return 0; } @@ -498,6 +498,8 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, int err; id = uval->value.enumerated.item[0]; + if (id >= ARRAY_SIZE(special_dig_in_iface_labels)) + return 0; /* decode user value */ dig_in_fmt = (id >> 1) & 0x01; @@ -537,18 +539,22 @@ static struct snd_kcontrol_new special_dig_in_iface_ctl = { .put = special_dig_in_iface_ctl_set }; +/* Digital output interface control for special firmware */ +static char *const special_dig_out_iface_labels[] = { + "S/PDIF Optical and Coaxial", "ADAT Optical" +}; static int special_dig_out_iface_ctl_info(struct snd_kcontrol *kctl, struct snd_ctl_elem_info *einf) { einf->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; einf->count = 1; - einf->value.enumerated.items = ARRAY_SIZE(special_dig_iface_labels) - 1; + einf->value.enumerated.items = ARRAY_SIZE(special_dig_out_iface_labels); if (einf->value.enumerated.item >= einf->value.enumerated.items) einf->value.enumerated.item = einf->value.enumerated.items - 1; strcpy(einf->value.enumerated.name, - special_dig_iface_labels[einf->value.enumerated.item + 1]); + special_dig_out_iface_labels[einf->value.enumerated.item]); return 0; } @@ -571,6 +577,8 @@ static int special_dig_out_iface_ctl_set(struct snd_kcontrol *kctl, int err; id = uval->value.enumerated.item[0]; + if (id >= ARRAY_SIZE(special_dig_out_iface_labels)) + return 0; mutex_lock(&bebob->mutex); -- 1.9.1 |
From: Takashi S. <o-t...@sa...> - 2014-07-21 09:50:45
|
Hi Iwai-San, (Jul 21 2014 16:06), Takashi Iwai wrote: > I don't want to apply such a big series at the late stage of 3.16-rc. > Please create short patches "just doing fix" for 3.16 as Yongjun did, > and rebase comprehensive cleanups on it. Exactly. In this time, I want to fix three bugs: 1. Missing unlock which Yongjun reported 2. My misunderstanding about return value of .put callback 3. Wrong label for a control element of digital input interface I think these should be fixed in this release cycle. Additionally, I want to add some dev_err() to reduce my maintaining efforts, because the fireworks/bebob drivers newly supports 60-70 models in Linux 3.16, while the number of developers who can support users is a few, maybe I and Clemens. I'll post four patches for these items, instead of the six patches, and post additional patches for next cycle, 3.17. Is it OK? Regards Takashi Sakamoto o-t...@sa... |
From: Takashi I. <ti...@su...> - 2014-07-21 09:59:26
|
At Mon, 21 Jul 2014 18:50:23 +0900, Takashi Sakamoto wrote: > > Hi Iwai-San, > > (Jul 21 2014 16:06), Takashi Iwai wrote: > > I don't want to apply such a big series at the late stage of 3.16-rc. > > Please create short patches "just doing fix" for 3.16 as Yongjun did, > > and rebase comprehensive cleanups on it. > > Exactly. In this time, I want to fix three bugs: > 1. Missing unlock which Yongjun reported > 2. My misunderstanding about return value of .put callback > 3. Wrong label for a control element of digital input interface > > I think these should be fixed in this release cycle. > > Additionally, I want to add some dev_err() to reduce my maintaining > efforts, because the fireworks/bebob drivers newly supports 60-70 models > in Linux 3.16, while the number of developers who can support users is a > few, maybe I and Clemens. > > I'll post four patches for these items, instead of the six patches, and > post additional patches for next cycle, 3.17. Is it OK? Yes. Takashi |
From: Takashi I. <ti...@su...> - 2014-07-21 09:58:21
|
At Mon, 21 Jul 2014 11:10:03 +0900, Takashi Sakamoto wrote: > > This commit use different labels for control elements of digital input/output > interfaces to correct my misunderstanding about M-Audio Firewire 1814 and > ProjectMix I/O. > > According to user manuals for these two models, they have two modes for > digital input; one is S/PDIF in both of optical and coaxial interfaces, > another is ADAT in optical interface only. > > But in current implementation, a control element for digital input uses > reduced labels which a control element for digital output uses because of my > misunderstanding that optical interface is not available for digital input > with S/PDIF mode. > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > sound/firewire/bebob/bebob_maudio.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c > index 008ff2c..f2277ce 100644 > --- a/sound/firewire/bebob/bebob_maudio.c > +++ b/sound/firewire/bebob/bebob_maudio.c > @@ -440,8 +440,8 @@ static struct snd_kcontrol_new special_sync_ctl = { > .get = special_sync_ctl_get, > }; > > -/* Digital interface control for special firmware */ > -static char *const special_dig_iface_labels[] = { > +/* Digital input interface control for special firmware */ > +static char *const special_dig_in_iface_labels[] = { > "S/PDIF Optical", "S/PDIF Coaxial", "ADAT Optical" > }; > static int special_dig_in_iface_ctl_info(struct snd_kcontrol *kctl, > @@ -449,13 +449,13 @@ static int special_dig_in_iface_ctl_info(struct snd_kcontrol *kctl, > { > einf->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; > einf->count = 1; > - einf->value.enumerated.items = ARRAY_SIZE(special_dig_iface_labels); > + einf->value.enumerated.items = ARRAY_SIZE(special_dig_in_iface_labels); > > if (einf->value.enumerated.item >= einf->value.enumerated.items) > einf->value.enumerated.item = einf->value.enumerated.items - 1; > > strcpy(einf->value.enumerated.name, > - special_dig_iface_labels[einf->value.enumerated.item]); > + special_dig_in_iface_labels[einf->value.enumerated.item]); > > return 0; > } > @@ -498,6 +498,8 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, > int err; > > id = uval->value.enumerated.item[0]; > + if (id >= ARRAY_SIZE(special_dig_in_iface_labels)) > + return 0; This should return an error. > > /* decode user value */ > dig_in_fmt = (id >> 1) & 0x01; > @@ -537,18 +539,22 @@ static struct snd_kcontrol_new special_dig_in_iface_ctl = { > .put = special_dig_in_iface_ctl_set > }; > > +/* Digital output interface control for special firmware */ > +static char *const special_dig_out_iface_labels[] = { > + "S/PDIF Optical and Coaxial", "ADAT Optical" > +}; > static int special_dig_out_iface_ctl_info(struct snd_kcontrol *kctl, > struct snd_ctl_elem_info *einf) > { > einf->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; > einf->count = 1; > - einf->value.enumerated.items = ARRAY_SIZE(special_dig_iface_labels) - 1; > + einf->value.enumerated.items = ARRAY_SIZE(special_dig_out_iface_labels); > > if (einf->value.enumerated.item >= einf->value.enumerated.items) > einf->value.enumerated.item = einf->value.enumerated.items - 1; > > strcpy(einf->value.enumerated.name, > - special_dig_iface_labels[einf->value.enumerated.item + 1]); > + special_dig_out_iface_labels[einf->value.enumerated.item]); > > return 0; > } > @@ -571,6 +577,8 @@ static int special_dig_out_iface_ctl_set(struct snd_kcontrol *kctl, > int err; > > id = uval->value.enumerated.item[0]; > + if (id >= ARRAY_SIZE(special_dig_out_iface_labels)) > + return 0; Ditto. thanks, Takashi > > mutex_lock(&bebob->mutex); > > -- > 1.9.1 > |
From: Takashi S. <o-t...@sa...> - 2014-07-21 10:50:06
|
(Jul 21 2014 18:58), Takashi Iwai wrote: >> @@ -498,6 +498,8 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, >> int err; >> >> id = uval->value.enumerated.item[0]; >> + if (id >= ARRAY_SIZE(special_dig_in_iface_labels)) >> + return 0; > > This should return an error. >> @@ -571,6 +577,8 @@ static int special_dig_out_iface_ctl_set(struct snd_kcontrol *kctl, >> int err; >> >> id = uval->value.enumerated.item[0]; >> + if (id >= ARRAY_SIZE(special_dig_out_iface_labels)) >> + return 0; > > Ditto. Oops, exactly. I use -EINVAL for the error code. Thank you. Takashi Sakamoto o-t...@sa... |
From: Takashi I. <ti...@su...> - 2014-07-21 10:03:07
|
At Mon, 21 Jul 2014 11:10:02 +0900, Takashi Sakamoto wrote: > > This commit adds some dev_err() to help debug when > avc_maudio_set_special_clk() failed. > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > sound/firewire/bebob/bebob_maudio.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/sound/firewire/bebob/bebob_maudio.c b/sound/firewire/bebob/bebob_maudio.c > index 83f8d5b..008ff2c 100644 > --- a/sound/firewire/bebob/bebob_maudio.c > +++ b/sound/firewire/bebob/bebob_maudio.c > @@ -393,6 +393,10 @@ static int special_clk_ctl_set(struct snd_kcontrol *kctl, > > mutex_unlock(&bebob->mutex); > > + if (err < 0) > + dev_err(&bebob->unit->device, > + "fail to change clock source: %d\n", err); > + The error message at the end of put callback is often useless. If it's an error, it's notified to user-space and you'll see the error there. The unconditional error message via dev_err() may flood the kernel messages, too. For the debugging purpose, rather put dev_dbg() to each point receiving an error and give the individual message. thanks, Takashi > return err >= 0; > } > static struct snd_kcontrol_new special_clk_ctl = { > @@ -506,7 +510,14 @@ static int special_dig_in_iface_ctl_set(struct snd_kcontrol *kctl, > dig_in_fmt, > params->dig_out_fmt, > params->clk_lock); > - if ((err < 0) || (params->dig_in_fmt > 0)) /* ADAT */ > + if (err < 0) { > + dev_err(&bebob->unit->device, > + "fail to change clock source: %d\n", err); > + goto end; > + } > + > + /* For ADAT, optical interface is only available. */ > + if (params->dig_in_fmt > 0) > goto end; > > err = avc_audio_set_selector(bebob->unit, 0x00, 0x04, dig_in_iface); > @@ -567,10 +578,13 @@ static int special_dig_out_iface_ctl_set(struct snd_kcontrol *kctl, > params->clk_src, > params->dig_in_fmt, > id, params->clk_lock); > - if (err >= 0) > - special_stream_formation_set(bebob); > > mutex_unlock(&bebob->mutex); > + > + if (err < 0) > + dev_err(&bebob->unit->device, > + "fail to change digital output interface: %d\n", err); > + > return err; > } > static struct snd_kcontrol_new special_dig_out_iface_ctl = { > -- > 1.9.1 > |