[alsa-devel] [PATCH -next 0/3] mop500_ab8500 fixes
Hi,
these are small fixes to the mop500_ab8500 asoc driver, developed on top of today's ASoC-next.
The first patch is actually non ab8500 specific, but as ab8500 seems to be the only codec to use the SND_SOC_DAPM_CLOCK_SUPPLY widget I'm posting it here anyway.
The other two patches sort out a mixup of card and codec controls in mop500_ab8500.
Thanks, Fabio
Fabio Baltieri (3): ASoC: dapm: use clk_prepare_enable and clk_disable_unprepare ASoC: ux500: move clock controls to ab8500-codec ASoC: ux500: register controls to card instead of codec
sound/soc/codecs/ab8500-codec.c | 10 ++++++++++ sound/soc/soc-dapm.c | 4 ++-- sound/soc/ux500/mop500_ab8500.c | 20 +++++--------------- 3 files changed, 17 insertions(+), 17 deletions(-)
Update dapm_clock_event to use clk_prepare_enable and clk_disable_unprepare.
Signed-off-by: Fabio Baltieri fabio.baltieri@linaro.org --- sound/soc/soc-dapm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 21779a6..a80c883 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -1095,9 +1095,9 @@ int dapm_clock_event(struct snd_soc_dapm_widget *w,
#ifdef CONFIG_HAVE_CLK if (SND_SOC_DAPM_EVENT_ON(event)) { - return clk_enable(w->clk); + return clk_prepare_enable(w->clk); } else { - clk_disable(w->clk); + clk_disable_unprepare(w->clk); return 0; } #endif
Move ab8500 clock control definitions to the ab8500 codec driver, leaving only card specific setting in mop500_ab8500_ctrls.
Signed-off-by: Fabio Baltieri fabio.baltieri@linaro.org --- sound/soc/codecs/ab8500-codec.c | 10 ++++++++++ sound/soc/ux500/mop500_ab8500.c | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c index a153b16..925e625 100644 --- a/sound/soc/codecs/ab8500-codec.c +++ b/sound/soc/codecs/ab8500-codec.c @@ -1615,6 +1615,16 @@ static SOC_ENUM_SINGLE_EXT_DECL(soc_enum_sidstate, enum_sid_state); static SOC_ENUM_SINGLE_EXT_DECL(soc_enum_ancstate, enum_anc_state);
static struct snd_kcontrol_new ab8500_ctrls[] = { + /* Digital interface - Clocks */ + SOC_SINGLE("Digital Interface Master Generator Switch", + AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN, + 1, 0), + SOC_SINGLE("Digital Interface 0 Bit-clock Switch", + AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0, + 1, 0), + SOC_SINGLE("Digital Interface 1 Bit-clock Switch", + AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1, + 1, 0), /* Charge pump */ SOC_ENUM("Charge Pump High Threshold For Low Voltage", soc_enum_envdeththre), diff --git a/sound/soc/ux500/mop500_ab8500.c b/sound/soc/ux500/mop500_ab8500.c index 78cce23..1a5c6b9 100644 --- a/sound/soc/ux500/mop500_ab8500.c +++ b/sound/soc/ux500/mop500_ab8500.c @@ -162,16 +162,6 @@ static struct snd_kcontrol_new mop500_ab8500_ctrls[] = { SOC_ENUM_EXT("Master Clock Select", soc_enum_mclk, mclk_input_control_get, mclk_input_control_put), - /* Digital interface - Clocks */ - SOC_SINGLE("Digital Interface Master Generator Switch", - AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN, - 1, 0), - SOC_SINGLE("Digital Interface 0 Bit-clock Switch", - AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0, - 1, 0), - SOC_SINGLE("Digital Interface 1 Bit-clock Switch", - AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1, - 1, 0), SOC_DAPM_PIN_SWITCH("Headset Left"), SOC_DAPM_PIN_SWITCH("Headset Right"), SOC_DAPM_PIN_SWITCH("Earpiece"),
On 04/30/2013 04:09 PM, Fabio Baltieri wrote:
Move ab8500 clock control definitions to the ab8500 codec driver, leaving only card specific setting in mop500_ab8500_ctrls.
Signed-off-by: Fabio Baltieri fabio.baltieri@linaro.org
sound/soc/codecs/ab8500-codec.c | 10 ++++++++++ sound/soc/ux500/mop500_ab8500.c | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c index a153b16..925e625 100644 --- a/sound/soc/codecs/ab8500-codec.c +++ b/sound/soc/codecs/ab8500-codec.c @@ -1615,6 +1615,16 @@ static SOC_ENUM_SINGLE_EXT_DECL(soc_enum_sidstate, enum_sid_state); static SOC_ENUM_SINGLE_EXT_DECL(soc_enum_ancstate, enum_anc_state);
static struct snd_kcontrol_new ab8500_ctrls[] = {
- /* Digital interface - Clocks */
- SOC_SINGLE("Digital Interface Master Generator Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN,1, 0),- SOC_SINGLE("Digital Interface 0 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0,1, 0),- SOC_SINGLE("Digital Interface 1 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1,1, 0),
I think this patch as it is is fine. But those three controls looks like they should be converted to DAPM widgets.
- Lars
On Tue, Apr 30, 2013 at 04:09:53PM +0200, Fabio Baltieri wrote:
Move ab8500 clock control definitions to the ab8500 codec driver, leaving only card specific setting in mop500_ab8500_ctrls.
So, if this is some generic thing and not some weird stuff for the card this really reopens the question about why this is done with user visible controls...
static struct snd_kcontrol_new ab8500_ctrls[] = {
- /* Digital interface - Clocks */
- SOC_SINGLE("Digital Interface Master Generator Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN,1, 0),- SOC_SINGLE("Digital Interface 0 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0,1, 0),- SOC_SINGLE("Digital Interface 1 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1,1, 0),
...this is all stuff that is normally figured out automatically by the drivers, we know when the audio interface is in use and hence when it needs to be clocked.
On Tue, Apr 30, 2013 at 07:30:35PM +0100, Mark Brown wrote:
On Tue, Apr 30, 2013 at 04:09:53PM +0200, Fabio Baltieri wrote:
Move ab8500 clock control definitions to the ab8500 codec driver, leaving only card specific setting in mop500_ab8500_ctrls.
So, if this is some generic thing and not some weird stuff for the card this really reopens the question about why this is done with user visible controls...
static struct snd_kcontrol_new ab8500_ctrls[] = {
- /* Digital interface - Clocks */
- SOC_SINGLE("Digital Interface Master Generator Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN,1, 0),- SOC_SINGLE("Digital Interface 0 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0,1, 0),- SOC_SINGLE("Digital Interface 1 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1,1, 0),...this is all stuff that is normally figured out automatically by the drivers, we know when the audio interface is in use and hence when it needs to be clocked.
It makes sense, I'll poke the documentation I have and try to figure out how to control those bits from a more appropriate place.
Thanks, Fabio
On Thu, May 02, 2013 at 09:54:21AM +0200, Fabio Baltieri wrote:
On Tue, Apr 30, 2013 at 07:30:35PM +0100, Mark Brown wrote:
On Tue, Apr 30, 2013 at 04:09:53PM +0200, Fabio Baltieri wrote:
Move ab8500 clock control definitions to the ab8500 codec driver, leaving only card specific setting in mop500_ab8500_ctrls.
So, if this is some generic thing and not some weird stuff for the card this really reopens the question about why this is done with user visible controls...
static struct snd_kcontrol_new ab8500_ctrls[] = {
- /* Digital interface - Clocks */
- SOC_SINGLE("Digital Interface Master Generator Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN,1, 0),- SOC_SINGLE("Digital Interface 0 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0,1, 0),- SOC_SINGLE("Digital Interface 1 Bit-clock Switch",
AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1,1, 0),...this is all stuff that is normally figured out automatically by the drivers, we know when the audio interface is in use and hence when it needs to be clocked.
It makes sense, I'll poke the documentation I have and try to figure out how to control those bits from a more appropriate place.
Well, it looks like this is *already* handled automatically by the ab8500 codec driver in ab8500_codec_set_dai_clock_gate(), and these controls just allow some messy degree of overriding after the audio stream is started. At this point the only thing that comes to my mind is that this is some debug leftover and I'm replying with a v2 to just drop these three widgets altogether.
In the meantime, I'm also observing some funny behavior of other alsamixer controls, but I'll fix those on a separate series. This is enough to bring the driver back on a working state.
Thanks, Fabio
Drop ab8500 clock gating widgets from mop500_ab8500_ctrls, as these bits are already controlled by ab8500 codec driver and should not be exposed to the user.
Signed-off-by: Fabio Baltieri fabio.baltieri@linaro.org ---
Was: ASoC: ux500: move clock controls to ab8500-codec
Changes from v1: - rewrote the patch to just drop the widgets, as those allowed the user to control some bits already handled by the codec driver.
sound/soc/ux500/mop500_ab8500.c | 10 ---------- 1 file changed, 10 deletions(-)
diff --git a/sound/soc/ux500/mop500_ab8500.c b/sound/soc/ux500/mop500_ab8500.c index 2c122de..4606194 100644 --- a/sound/soc/ux500/mop500_ab8500.c +++ b/sound/soc/ux500/mop500_ab8500.c @@ -162,16 +162,6 @@ static struct snd_kcontrol_new mop500_ab8500_ctrls[] = { SOC_ENUM_EXT("Master Clock Select", soc_enum_mclk, mclk_input_control_get, mclk_input_control_put), - /* Digital interface - Clocks */ - SOC_SINGLE("Digital Interface Master Generator Switch", - AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN, - 1, 0), - SOC_SINGLE("Digital Interface 0 Bit-clock Switch", - AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0, - 1, 0), - SOC_SINGLE("Digital Interface 1 Bit-clock Switch", - AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1, - 1, 0), SOC_DAPM_PIN_SWITCH("Headset Left"), SOC_DAPM_PIN_SWITCH("Headset Right"), SOC_DAPM_PIN_SWITCH("Earpiece"),
On Thu, May 02, 2013 at 11:52:51AM +0200, Fabio Baltieri wrote:
Drop ab8500 clock gating widgets from mop500_ab8500_ctrls, as these bits are already controlled by ab8500 codec driver and should not be exposed to the user.
Applied, thanks.
Update mop500_ab8500_machine_init to register mop500_ab8500_ctrls as card control instead of codec control, as it only contains SOC_DAPM_PIN_SWITCH definitions.
Signed-off-by: Fabio Baltieri fabio.baltieri@linaro.org --- sound/soc/ux500/mop500_ab8500.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/ux500/mop500_ab8500.c b/sound/soc/ux500/mop500_ab8500.c index 1a5c6b9..4606194 100644 --- a/sound/soc/ux500/mop500_ab8500.c +++ b/sound/soc/ux500/mop500_ab8500.c @@ -127,9 +127,9 @@ static int mop500_ab8500_set_mclk(struct device *dev, static int mclk_input_control_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); + struct snd_soc_card *card = snd_kcontrol_chip(kcontrol); struct mop500_ab8500_drvdata *drvdata = - snd_soc_card_get_drvdata(codec->card); + snd_soc_card_get_drvdata(card);
ucontrol->value.enumerated.item[0] = drvdata->mclk_sel;
@@ -139,9 +139,9 @@ static int mclk_input_control_get(struct snd_kcontrol *kcontrol, static int mclk_input_control_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); + struct snd_soc_card *card = snd_kcontrol_chip(kcontrol); struct mop500_ab8500_drvdata *drvdata = - snd_soc_card_get_drvdata(codec->card); + snd_soc_card_get_drvdata(card); unsigned int val = ucontrol->value.enumerated.item[0];
if (val > (unsigned int)MCLK_ULPCLK) @@ -377,7 +377,7 @@ int mop500_ab8500_machine_init(struct snd_soc_pcm_runtime *rtd) drvdata->mclk_sel = MCLK_ULPCLK;
/* Add controls */ - ret = snd_soc_add_codec_controls(codec, mop500_ab8500_ctrls, + ret = snd_soc_add_card_controls(codec->card, mop500_ab8500_ctrls, ARRAY_SIZE(mop500_ab8500_ctrls)); if (ret < 0) { pr_err("%s: Failed to add machine-controls (%d)!\n",
On Tue, Apr 30, 2013 at 04:09:54PM +0200, Fabio Baltieri wrote:
Update mop500_ab8500_machine_init to register mop500_ab8500_ctrls as card control instead of codec control, as it only contains SOC_DAPM_PIN_SWITCH definitions.
Applied, thanks.
participants (3)
-
Fabio Baltieri -
Lars-Peter Clausen -
Mark Brown