[PATCH -next 0/3] mop500_ab8500 fixes

[PATCH -next 0/3] mop500_ab8500 fixes

[Fabio Baltieri]

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(-)

-- 
1.8.2

[PATCH 1/3] ASoC: dapm: use clk_prepare_enable and clk_disable_unprepare

[Fabio Baltieri]

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
-- 
1.8.2

Re: [PATCH 1/3] ASoC: dapm: use clk_prepare_enable and clk_disable_unprepare

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 163 bytes --]

On Tue, Apr 30, 2013 at 04:09:52PM +0200, Fabio Baltieri wrote:
> Update dapm_clock_event to use clk_prepare_enable and
> clk_disable_unprepare.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 2/3] ASoC: ux500: move clock controls to ab8500-codec

[Fabio Baltieri]

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"),
-- 
1.8.2

Re: [PATCH 2/3] ASoC: ux500: move clock controls to ab8500-codec

[Lars-Peter Clausen]

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

Re: [PATCH 2/3] ASoC: ux500: move clock controls to ab8500-codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/3] ASoC: ux500: move clock controls to ab8500-codec

[Fabio Baltieri]

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

-- 
Fabio Baltieri

Re: [PATCH 2/3] ASoC: ux500: move clock controls to ab8500-codec

[Fabio Baltieri]

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

-- 
Fabio Baltieri

[PATCH v2 2/3] ASoC: ux500: drop clock gating widgets from machine driver

[Fabio Baltieri]

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"),
-- 
1.8.2

Re: [PATCH v2 2/3] ASoC: ux500: drop clock gating widgets from machine driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/3] ASoC: ux500: register controls to card instead of codec

[Fabio Baltieri]

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",
-- 
1.8.2

Re: [PATCH 3/3] ASoC: ux500: register controls to card instead of codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 249 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]