* [PATCH 1/6] bebob: Arrangement for critical section to be shorter
2014-07-21 2:09 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Sakamoto
@ 2014-07-21 2:10 ` Takashi Sakamoto
2014-07-21 2:10 ` [PATCH 2/6] bebob: Reducing function callers for simplicity Takashi Sakamoto
` (5 subsequent siblings)
6 siblings, 0 replies; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 2:10 UTC (permalink / raw)
To: clemens, tiwai, perex; +Cc: Wei Yongjun, alsa-devel, darrena092, ffado-devel
Cc: Wei Yongjun <weiyj_lk@163.com>
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-takashi@sakamocchi.jp>
---
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 2/6] bebob: Reducing function callers for simplicity
2014-07-21 2:09 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Sakamoto
2014-07-21 2:10 ` [PATCH 1/6] bebob: Arrangement for critical section to be shorter Takashi Sakamoto
@ 2014-07-21 2:10 ` Takashi Sakamoto
2014-07-21 2:10 ` [PATCH 3/6] bebob: Add dev_err() for debugging Takashi Sakamoto
` (4 subsequent siblings)
6 siblings, 0 replies; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 2:10 UTC (permalink / raw)
To: clemens, tiwai, perex; +Cc: alsa-devel, darrena092, ffado-devel
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-takashi@sakamocchi.jp>
---
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 3/6] bebob: Add dev_err() for debugging
2014-07-21 2:09 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Sakamoto
2014-07-21 2:10 ` [PATCH 1/6] bebob: Arrangement for critical section to be shorter Takashi Sakamoto
2014-07-21 2:10 ` [PATCH 2/6] bebob: Reducing function callers for simplicity Takashi Sakamoto
@ 2014-07-21 2:10 ` Takashi Sakamoto
2014-07-21 10:02 ` Takashi Iwai
2014-07-21 2:10 ` [PATCH 4/6] bebob: Use different labels for digital input/output interfaces Takashi Sakamoto
` (3 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 2:10 UTC (permalink / raw)
To: clemens, tiwai, perex; +Cc: alsa-devel, darrena092, ffado-devel
This commit adds some dev_err() to help debug when
avc_maudio_set_special_clk() failed.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 3/6] bebob: Add dev_err() for debugging
2014-07-21 2:10 ` [PATCH 3/6] bebob: Add dev_err() for debugging Takashi Sakamoto
@ 2014-07-21 10:02 ` Takashi Iwai
0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2014-07-21 10:02 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens, darrena092, ffado-devel
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-takashi@sakamocchi.jp>
> ---
> 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
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/6] bebob: Use different labels for digital input/output interfaces
2014-07-21 2:09 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Sakamoto
` (2 preceding siblings ...)
2014-07-21 2:10 ` [PATCH 3/6] bebob: Add dev_err() for debugging Takashi Sakamoto
@ 2014-07-21 2:10 ` Takashi Sakamoto
2014-07-21 9:58 ` Takashi Iwai
2014-07-21 2:10 ` [PATCH 5/6] bebob: Correction for return value of .put callback functions Takashi Sakamoto
` (2 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 2:10 UTC (permalink / raw)
To: clemens, tiwai, perex; +Cc: alsa-devel, darrena092, ffado-devel
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-takashi@sakamocchi.jp>
---
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 4/6] bebob: Use different labels for digital input/output interfaces
2014-07-21 2:10 ` [PATCH 4/6] bebob: Use different labels for digital input/output interfaces Takashi Sakamoto
@ 2014-07-21 9:58 ` Takashi Iwai
2014-07-21 10:49 ` Takashi Sakamoto
0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2014-07-21 9:58 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens, darrena092, ffado-devel
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-takashi@sakamocchi.jp>
> ---
> 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
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/6] bebob: Use different labels for digital input/output interfaces
2014-07-21 9:58 ` Takashi Iwai
@ 2014-07-21 10:49 ` Takashi Sakamoto
0 siblings, 0 replies; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 10:49 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, clemens, darrena092, ffado-devel
(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-takashi@sakamocchi.jp
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/6] bebob: Correction for return value of .put callback functions
2014-07-21 2:09 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Sakamoto
` (3 preceding siblings ...)
2014-07-21 2:10 ` [PATCH 4/6] bebob: Use different labels for digital input/output interfaces Takashi Sakamoto
@ 2014-07-21 2:10 ` Takashi Sakamoto
2014-07-21 2:10 ` [PATCH 6/6] bebob: Arrangement for a control element to which two settings relate Takashi Sakamoto
2014-07-21 7:06 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Iwai
6 siblings, 0 replies; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 2:10 UTC (permalink / raw)
To: clemens, tiwai, perex; +Cc: alsa-devel, darrena092, ffado-devel
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-takashi@sakamocchi.jp>
---
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 6/6] bebob: Arrangement for a control element to which two settings relate
2014-07-21 2:09 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Sakamoto
` (4 preceding siblings ...)
2014-07-21 2:10 ` [PATCH 5/6] bebob: Correction for return value of .put callback functions Takashi Sakamoto
@ 2014-07-21 2:10 ` Takashi Sakamoto
2014-07-21 7:06 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Iwai
6 siblings, 0 replies; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 2:10 UTC (permalink / raw)
To: clemens, tiwai, perex; +Cc: alsa-devel, darrena092, ffado-devel
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-takashi@sakamocchi.jp>
---
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 0/6] bebob: Improvements for M-Audio specific operations
2014-07-21 2:09 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Sakamoto
` (5 preceding siblings ...)
2014-07-21 2:10 ` [PATCH 6/6] bebob: Arrangement for a control element to which two settings relate Takashi Sakamoto
@ 2014-07-21 7:06 ` Takashi Iwai
2014-07-21 9:50 ` Takashi Sakamoto
6 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2014-07-21 7:06 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens, darrena092, ffado-devel
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
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 0/6] bebob: Improvements for M-Audio specific operations
2014-07-21 7:06 ` [PATCH 0/6] bebob: Improvements for M-Audio specific operations Takashi Iwai
@ 2014-07-21 9:50 ` Takashi Sakamoto
2014-07-21 9:59 ` Takashi Iwai
0 siblings, 1 reply; 24+ messages in thread
From: Takashi Sakamoto @ 2014-07-21 9:50 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, clemens, darrena092, ffado-devel
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-takashi@sakamocchi.jp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/6] bebob: Improvements for M-Audio specific operations
2014-07-21 9:50 ` Takashi Sakamoto
@ 2014-07-21 9:59 ` Takashi Iwai
0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2014-07-21 9:59 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens, darrena092, ffado-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread