[PATCH 0/5] ASoC/MFD/OMAP: TWL4030: APLL_CTL handling change

[PATCH 0/5] ASoC/MFD/OMAP: TWL4030: APLL_CTL handling change

[Peter Ujfalusi]

Hello,

The following series changes the way how the APLL_CTL register is handled by
the twl4030 codec related drivers.
Moving the configuration of APLL_CTL register from child drivers (specifically
from soc codec driver) to the twl4030-codec MFD driver.
Because the audio_mclk is static, board specific and can not be changed in
runtime, the codec MFD is the right place to configure it early enough, so
child can use the hardware correctly.
Before this patch for example the APLL_INFREQ was configured first, when the
user played or recorded audio via the twl4030.
The digital bypass needs correct APLL_INFREQ to be configured in order to work
properly. If the digital bypass enabled before any audio activity, in some
cases the bypassed audio was not correct.

The series also addresses the issue, that after boot-up, neither of the
loopbacks was working, because the codec was actually off.
Setting the codec->bias_level to _OFF, before setting the codec bias level to
STANDBY fixes this issue.

This series is on top of Takashi's sound-2.6 topic/asoc branch, which has the
twl403-codec MFD and related patches for 2.6.33.

It would be really nice, if this series would make it also to the same branch,
since this fixes some real anomalies brought in by the cleanup of the twl4030
codec related drivers.
These problems were hidden before because of the inconsistent handling of bits
and states in the codec driver in the past.

Thank you,

---
Peter Ujfalusi (5):
  MFD: TWL4030: Add audio_mclk to the codec platform data
  OMAP: Configure audio_mclk for twl4030-codec MFD
  MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver
  ASoC: TWL4030: Make sure, that the codec is powered on startup
  ASoC: TWL4030: Do not modify the APLL_CTL register

 arch/arm/mach-omap2/board-3430sdp.c      |    1 +
 arch/arm/mach-omap2/board-omap3beagle.c  |    1 +
 arch/arm/mach-omap2/board-omap3evm.c     |    1 +
 arch/arm/mach-omap2/board-omap3pandora.c |    1 +
 arch/arm/mach-omap2/board-overo.c        |    1 +
 arch/arm/mach-omap2/board-zoom2.c        |    1 +
 drivers/mfd/twl4030-codec.c              |   40 +++++++++++++++++
 include/linux/i2c/twl4030.h              |    1 +
 include/linux/mfd/twl4030-codec.h        |    1 +
 sound/soc/codecs/twl4030.c               |   70 +++++++++++------------------
 10 files changed, 75 insertions(+), 43 deletions(-)

[PATCH 0/5] ASoC/MFD/OMAP: TWL4030: APLL_CTL handling change

[Peter Ujfalusi]

Hello,

The following series changes the way how the APLL_CTL register is handled by
the twl4030 codec related drivers.
Moving the configuration of APLL_CTL register from child drivers (specifically
from soc codec driver) to the twl4030-codec MFD driver.
Because the audio_mclk is static, board specific and can not be changed in
runtime, the codec MFD is the right place to configure it early enough, so
child can use the hardware correctly.
Before this patch for example the APLL_INFREQ was configured first, when the
user played or recorded audio via the twl4030.
The digital bypass needs correct APLL_INFREQ to be configured in order to work
properly. If the digital bypass enabled before any audio activity, in some
cases the bypassed audio was not correct.

The series also addresses the issue, that after boot-up, neither of the
loopbacks was working, because the codec was actually off.
Setting the codec->bias_level to _OFF, before setting the codec bias level to
STANDBY fixes this issue.

This series is on top of Takashi's sound-2.6 topic/asoc branch, which has the
twl403-codec MFD and related patches for 2.6.33.

It would be really nice, if this series would make it also to the same branch,
since this fixes some real anomalies brought in by the cleanup of the twl4030
codec related drivers.
These problems were hidden before because of the inconsistent handling of bits
and states in the codec driver in the past.

Thank you,

---
Peter Ujfalusi (5):
  MFD: TWL4030: Add audio_mclk to the codec platform data
  OMAP: Configure audio_mclk for twl4030-codec MFD
  MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver
  ASoC: TWL4030: Make sure, that the codec is powered on startup
  ASoC: TWL4030: Do not modify the APLL_CTL register

 arch/arm/mach-omap2/board-3430sdp.c      |    1 +
 arch/arm/mach-omap2/board-omap3beagle.c  |    1 +
 arch/arm/mach-omap2/board-omap3evm.c     |    1 +
 arch/arm/mach-omap2/board-omap3pandora.c |    1 +
 arch/arm/mach-omap2/board-overo.c        |    1 +
 arch/arm/mach-omap2/board-zoom2.c        |    1 +
 drivers/mfd/twl4030-codec.c              |   40 +++++++++++++++++
 include/linux/i2c/twl4030.h              |    1 +
 include/linux/mfd/twl4030-codec.h        |    1 +
 sound/soc/codecs/twl4030.c               |   70 +++++++++++------------------
 10 files changed, 75 insertions(+), 43 deletions(-)

[PATCH 1/5] MFD: TWL4030: Add audio_mclk to the codec platform data

[Peter Ujfalusi]

Add audio_mclk to the twl4030-codec MFD

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 include/linux/i2c/twl4030.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/i2c/twl4030.h b/include/linux/i2c/twl4030.h
index 42d6c72..c188961 100644
--- a/include/linux/i2c/twl4030.h
+++ b/include/linux/i2c/twl4030.h
@@ -414,6 +414,7 @@ struct twl4030_codec_vibra_data {
 };
 
 struct twl4030_codec_data {
+	unsigned int	audio_mclk;
 	struct twl4030_codec_audio_data		*audio;
 	struct twl4030_codec_vibra_data		*vibra;
 };
-- 
1.6.5.2

[PATCH 1/5] MFD: TWL4030: Add audio_mclk to the codec platform data

[Peter Ujfalusi]

Add audio_mclk to the twl4030-codec MFD

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 include/linux/i2c/twl4030.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/i2c/twl4030.h b/include/linux/i2c/twl4030.h
index 42d6c72..c188961 100644
--- a/include/linux/i2c/twl4030.h
+++ b/include/linux/i2c/twl4030.h
@@ -414,6 +414,7 @@ struct twl4030_codec_vibra_data {
 };
 
 struct twl4030_codec_data {
+	unsigned int	audio_mclk;
 	struct twl4030_codec_audio_data		*audio;
 	struct twl4030_codec_vibra_data		*vibra;
 };
-- 
1.6.5.2

[PATCH 2/5] OMAP: Configure audio_mclk for twl4030-codec MFD

[Peter Ujfalusi]

audio_mclk value is going to be handled by the
twl4030-codec MFD driver, configure the correct
value for boards, which is using the twl4030 audio.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 arch/arm/mach-omap2/board-3430sdp.c      |    1 +
 arch/arm/mach-omap2/board-omap3beagle.c  |    1 +
 arch/arm/mach-omap2/board-omap3evm.c     |    1 +
 arch/arm/mach-omap2/board-omap3pandora.c |    1 +
 arch/arm/mach-omap2/board-overo.c        |    1 +
 arch/arm/mach-omap2/board-zoom2.c        |    1 +
 6 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index 4f91f7a..9afd957 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -415,6 +415,7 @@ static struct twl4030_codec_audio_data sdp3430_audio = {
 };
 
 static struct twl4030_codec_data sdp3430_codec = {
+	.audio_mclk = 26000000,
 	.audio = &sdp3430_audio,
 };
 
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 2161d85..8f0c106 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -259,6 +259,7 @@ static struct twl4030_codec_audio_data beagle_audio_data = {
 };
 
 static struct twl4030_codec_data beagle_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &beagle_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index d9a6103..5bb30cb 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -199,6 +199,7 @@ static struct twl4030_codec_audio_data omap3evm_audio_data = {
 };
 
 static struct twl4030_codec_data omap3evm_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &omap3evm_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c
index 5036b56..77790ee 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -286,6 +286,7 @@ static struct twl4030_codec_audio_data omap3pandora_audio_data = {
 };
 
 static struct twl4030_codec_data omap3pandora_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &omap3pandora_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
index dc55008..e1fb504 100644
--- a/arch/arm/mach-omap2/board-overo.c
+++ b/arch/arm/mach-omap2/board-overo.c
@@ -334,6 +334,7 @@ static struct twl4030_codec_audio_data overo_audio_data = {
 };
 
 static struct twl4030_codec_data overo_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &overo_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-zoom2.c b/arch/arm/mach-omap2/board-zoom2.c
index f1b4e7c..de3a38d 100644
--- a/arch/arm/mach-omap2/board-zoom2.c
+++ b/arch/arm/mach-omap2/board-zoom2.c
@@ -234,6 +234,7 @@ static struct twl4030_codec_audio_data zoom2_audio_data = {
 };
 
 static struct twl4030_codec_data zoom2_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &zoom2_audio_data,
 };
 
-- 
1.6.5.2

[PATCH 2/5] OMAP: Configure audio_mclk for twl4030-codec MFD

[Peter Ujfalusi]

audio_mclk value is going to be handled by the
twl4030-codec MFD driver, configure the correct
value for boards, which is using the twl4030 audio.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 arch/arm/mach-omap2/board-3430sdp.c      |    1 +
 arch/arm/mach-omap2/board-omap3beagle.c  |    1 +
 arch/arm/mach-omap2/board-omap3evm.c     |    1 +
 arch/arm/mach-omap2/board-omap3pandora.c |    1 +
 arch/arm/mach-omap2/board-overo.c        |    1 +
 arch/arm/mach-omap2/board-zoom2.c        |    1 +
 6 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index 4f91f7a..9afd957 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -415,6 +415,7 @@ static struct twl4030_codec_audio_data sdp3430_audio = {
 };
 
 static struct twl4030_codec_data sdp3430_codec = {
+	.audio_mclk = 26000000,
 	.audio = &sdp3430_audio,
 };
 
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 2161d85..8f0c106 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -259,6 +259,7 @@ static struct twl4030_codec_audio_data beagle_audio_data = {
 };
 
 static struct twl4030_codec_data beagle_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &beagle_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index d9a6103..5bb30cb 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -199,6 +199,7 @@ static struct twl4030_codec_audio_data omap3evm_audio_data = {
 };
 
 static struct twl4030_codec_data omap3evm_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &omap3evm_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c
index 5036b56..77790ee 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -286,6 +286,7 @@ static struct twl4030_codec_audio_data omap3pandora_audio_data = {
 };
 
 static struct twl4030_codec_data omap3pandora_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &omap3pandora_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
index dc55008..e1fb504 100644
--- a/arch/arm/mach-omap2/board-overo.c
+++ b/arch/arm/mach-omap2/board-overo.c
@@ -334,6 +334,7 @@ static struct twl4030_codec_audio_data overo_audio_data = {
 };
 
 static struct twl4030_codec_data overo_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &overo_audio_data,
 };
 
diff --git a/arch/arm/mach-omap2/board-zoom2.c b/arch/arm/mach-omap2/board-zoom2.c
index f1b4e7c..de3a38d 100644
--- a/arch/arm/mach-omap2/board-zoom2.c
+++ b/arch/arm/mach-omap2/board-zoom2.c
@@ -234,6 +234,7 @@ static struct twl4030_codec_audio_data zoom2_audio_data = {
 };
 
 static struct twl4030_codec_data zoom2_codec_data = {
+	.audio_mclk = 26000000,
 	.audio = &zoom2_audio_data,
 };
 
-- 
1.6.5.2

[PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Peter Ujfalusi]

Move the APLL_CTL register configuration to the twl4030-codec
MFD driver.
Provide also a function for childs to query the audio_mclk
frequency.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 drivers/mfd/twl4030-codec.c       |   40 +++++++++++++++++++++++++++++++++++++
 include/linux/mfd/twl4030-codec.h |    1 +
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/twl4030-codec.c b/drivers/mfd/twl4030-codec.c
index 9710307..6e77f57 100644
--- a/drivers/mfd/twl4030-codec.c
+++ b/drivers/mfd/twl4030-codec.c
@@ -41,6 +41,7 @@ struct twl4030_codec_resource {
 };
 
 struct twl4030_codec {
+	unsigned int audio_mclk;
 	struct mutex mutex;
 	struct twl4030_codec_resource resource[TWL4030_CODEC_RES_MAX];
 	struct mfd_cell cells[TWL4030_CODEC_CELLS];
@@ -145,12 +146,32 @@ int twl4030_codec_disable_resource(unsigned id)
 }
 EXPORT_SYMBOL_GPL(twl4030_codec_disable_resource);
 
+unsigned int twl4030_codec_get_mclk(void)
+{
+	struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
+
+	return codec->audio_mclk;
+}
+EXPORT_SYMBOL_GPL(twl4030_codec_get_mclk);
+
 static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 {
 	struct twl4030_codec *codec;
 	struct twl4030_codec_data *pdata = pdev->dev.platform_data;
 	struct mfd_cell *cell = NULL;
 	int ret, childs = 0;
+	u8 val;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "Platform data is missing\n");
+		return -EINVAL;
+	}
+	if (!(pdata->audio_mclk == 19200000 ||
+	      pdata->audio_mclk == 26000000 ||
+	      pdata->audio_mclk == 38400000)) {
+		dev_err(&pdev->dev, "Invalid audio_mclk\n");
+		return -EINVAL;
+	}
 
 	codec = kzalloc(sizeof(struct twl4030_codec), GFP_KERNEL);
 	if (!codec)
@@ -160,6 +181,25 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 
 	twl4030_codec_dev = pdev;
 	mutex_init(&codec->mutex);
+	codec->audio_mclk = pdata->audio_mclk;
+
+	/* Configure APLL_INFREQ */
+	twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &val,
+			TWL4030_REG_APLL_CTL);
+	val &= ~TWL4030_APLL_INFREQ;
+	switch (codec->audio_mclk) {
+	case 19200000:
+		val |= TWL4030_APLL_INFREQ_19200KHZ;
+		break;
+	case 26000000:
+		val |= TWL4030_APLL_INFREQ_26000KHZ;
+		break;
+	case 38400000:
+		val |= TWL4030_APLL_INFREQ_38400KHZ;
+		break;
+	}
+	twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE,
+					val, TWL4030_REG_APLL_CTL);
 
 	/* Codec power */
 	codec->resource[TWL4030_CODEC_RES_POWER].reg = TWL4030_REG_CODEC_MODE;
diff --git a/include/linux/mfd/twl4030-codec.h b/include/linux/mfd/twl4030-codec.h
index ef0a304..2ec317c 100644
--- a/include/linux/mfd/twl4030-codec.h
+++ b/include/linux/mfd/twl4030-codec.h
@@ -267,5 +267,6 @@ enum twl4030_codec_res {
 
 int twl4030_codec_disable_resource(enum twl4030_codec_res id);
 int twl4030_codec_enable_resource(enum twl4030_codec_res id);
+unsigned int twl4030_codec_get_mclk(void);
 
 #endif	/* End of __TWL4030_CODEC_H__ */
-- 
1.6.5.2

[PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Peter Ujfalusi]

Move the APLL_CTL register configuration to the twl4030-codec
MFD driver.
Provide also a function for childs to query the audio_mclk
frequency.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 drivers/mfd/twl4030-codec.c       |   40 +++++++++++++++++++++++++++++++++++++
 include/linux/mfd/twl4030-codec.h |    1 +
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/twl4030-codec.c b/drivers/mfd/twl4030-codec.c
index 9710307..6e77f57 100644
--- a/drivers/mfd/twl4030-codec.c
+++ b/drivers/mfd/twl4030-codec.c
@@ -41,6 +41,7 @@ struct twl4030_codec_resource {
 };
 
 struct twl4030_codec {
+	unsigned int audio_mclk;
 	struct mutex mutex;
 	struct twl4030_codec_resource resource[TWL4030_CODEC_RES_MAX];
 	struct mfd_cell cells[TWL4030_CODEC_CELLS];
@@ -145,12 +146,32 @@ int twl4030_codec_disable_resource(unsigned id)
 }
 EXPORT_SYMBOL_GPL(twl4030_codec_disable_resource);
 
+unsigned int twl4030_codec_get_mclk(void)
+{
+	struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
+
+	return codec->audio_mclk;
+}
+EXPORT_SYMBOL_GPL(twl4030_codec_get_mclk);
+
 static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 {
 	struct twl4030_codec *codec;
 	struct twl4030_codec_data *pdata = pdev->dev.platform_data;
 	struct mfd_cell *cell = NULL;
 	int ret, childs = 0;
+	u8 val;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "Platform data is missing\n");
+		return -EINVAL;
+	}
+	if (!(pdata->audio_mclk == 19200000 ||
+	      pdata->audio_mclk == 26000000 ||
+	      pdata->audio_mclk == 38400000)) {
+		dev_err(&pdev->dev, "Invalid audio_mclk\n");
+		return -EINVAL;
+	}
 
 	codec = kzalloc(sizeof(struct twl4030_codec), GFP_KERNEL);
 	if (!codec)
@@ -160,6 +181,25 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 
 	twl4030_codec_dev = pdev;
 	mutex_init(&codec->mutex);
+	codec->audio_mclk = pdata->audio_mclk;
+
+	/* Configure APLL_INFREQ */
+	twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &val,
+			TWL4030_REG_APLL_CTL);
+	val &= ~TWL4030_APLL_INFREQ;
+	switch (codec->audio_mclk) {
+	case 19200000:
+		val |= TWL4030_APLL_INFREQ_19200KHZ;
+		break;
+	case 26000000:
+		val |= TWL4030_APLL_INFREQ_26000KHZ;
+		break;
+	case 38400000:
+		val |= TWL4030_APLL_INFREQ_38400KHZ;
+		break;
+	}
+	twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE,
+					val, TWL4030_REG_APLL_CTL);
 
 	/* Codec power */
 	codec->resource[TWL4030_CODEC_RES_POWER].reg = TWL4030_REG_CODEC_MODE;
diff --git a/include/linux/mfd/twl4030-codec.h b/include/linux/mfd/twl4030-codec.h
index ef0a304..2ec317c 100644
--- a/include/linux/mfd/twl4030-codec.h
+++ b/include/linux/mfd/twl4030-codec.h
@@ -267,5 +267,6 @@ enum twl4030_codec_res {
 
 int twl4030_codec_disable_resource(enum twl4030_codec_res id);
 int twl4030_codec_enable_resource(enum twl4030_codec_res id);
+unsigned int twl4030_codec_get_mclk(void);
 
 #endif	/* End of __TWL4030_CODEC_H__ */
-- 
1.6.5.2

[PATCH 4/5] ASoC: TWL4030: Make sure, that the codec is powered on startup

[Peter Ujfalusi]

Set the codec->bias_level to SND_SOC_BIAS_OFF before changing
the initial bias level to STANDBY.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/codecs/twl4030.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index f9121ef..c0b47df 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -2234,6 +2234,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 
 	/* Set the defaults, and power up the codec */
 	twl4030_init_chip(codec);
+	codec->bias_level = SND_SOC_BIAS_OFF;
 	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
 	ret = snd_soc_register_codec(codec);
-- 
1.6.5.2

[PATCH 4/5] ASoC: TWL4030: Make sure, that the codec is powered on startup

[Peter Ujfalusi]

Set the codec->bias_level to SND_SOC_BIAS_OFF before changing
the initial bias level to STANDBY.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/codecs/twl4030.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index f9121ef..c0b47df 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -2234,6 +2234,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 
 	/* Set the defaults, and power up the codec */
 	twl4030_init_chip(codec);
+	codec->bias_level = SND_SOC_BIAS_OFF;
 	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
 	ret = snd_soc_register_codec(codec);
-- 
1.6.5.2

[PATCH 5/5] ASoC: TWL4030: Do not modify the APLL_CTL register

[Peter Ujfalusi]

APLL_CTL register is configured by the twl4030-codec MFD
driver.
Remove code, which makes changes in the APLL_CTL register,
and replace those with checks against the configured
audio_mclk configuration done in the MFD driver.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/codecs/twl4030.c |   69 ++++++++++++++++---------------------------
 1 files changed, 26 insertions(+), 43 deletions(-)

diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index c0b47df..67cde72 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -214,7 +214,8 @@ static void twl4030_init_chip(struct snd_soc_codec *codec)
 
 	/* set all audio section registers to reasonable defaults */
 	for (i = TWL4030_REG_OPTION; i <= TWL4030_REG_MISC_SET_2; i++)
-		twl4030_write(codec, i,	cache[i]);
+		if (i != TWL4030_REG_APLL_CTL)
+			twl4030_write(codec, i,	cache[i]);
 
 }
 
@@ -1753,30 +1754,23 @@ static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
 	struct twl4030_priv *twl4030 = codec->private_data;
-	u8 apll_ctrl;
 
-	apll_ctrl = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL);
-	apll_ctrl &= ~TWL4030_APLL_INFREQ;
 	switch (freq) {
 	case 19200000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_19200KHZ;
-		twl4030->sysclk = 19200;
-		break;
 	case 26000000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_26000KHZ;
-		twl4030->sysclk = 26000;
-		break;
 	case 38400000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_38400KHZ;
-		twl4030->sysclk = 38400;
 		break;
 	default:
-		printk(KERN_ERR "TWL4030 set sysclk: unknown rate %d\n",
-			freq);
+		dev_err(codec->dev, "Unsupported frequency %u\n", freq);
 		return -EINVAL;
 	}
 
-	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
+	if ((freq / 1000) != twl4030->sysclk) {
+		dev_err(codec->dev,
+			"Mismatch in APLL infrequency %u (configred: %u)\n",
+			freq, twl4030->sysclk * 1000);
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -1874,16 +1868,13 @@ static int twl4030_voice_startup(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_device *socdev = rtd->socdev;
 	struct snd_soc_codec *codec = socdev->card->codec;
-	u8 infreq;
+	struct twl4030_priv *twl4030 = codec->private_data;
 	u8 mode;
 
 	/* If the system master clock is not 26MHz, the voice PCM interface is
 	 * not avilable.
 	 */
-	infreq = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL)
-		& TWL4030_APLL_INFREQ;
-
-	if (infreq != TWL4030_APLL_INFREQ_26000KHZ) {
+	if (twl4030->sysclk != 26000) {
 		printk(KERN_ERR "TWL4030 voice startup: "
 			"MCLK is not 26MHz, call set_sysclk() on init\n");
 		return -EINVAL;
@@ -1958,22 +1949,18 @@ static int twl4030_voice_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 		int clk_id, unsigned int freq, int dir)
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
-	u8 apll_ctrl;
+	struct twl4030_priv *twl4030 = codec->private_data;
 
-	apll_ctrl = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL);
-	apll_ctrl &= ~TWL4030_APLL_INFREQ;
-	switch (freq) {
-	case 26000000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_26000KHZ;
-		break;
-	default:
-		printk(KERN_ERR "TWL4030 voice set sysclk: unknown rate %d\n",
-			freq);
+	if (freq != 26000000) {
+		dev_err(codec->dev, "Unsupported frequency %u\n", freq);
+		return -EINVAL;
+	}
+	if ((freq / 1000) != twl4030->sysclk) {
+		dev_err(codec->dev,
+			"Mismatch in APLL infrequency %u (configred: %u)\n",
+			freq, twl4030->sysclk * 1000);
 		return -EINVAL;
 	}
-
-	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
-
 	return 0;
 }
 
@@ -2131,17 +2118,14 @@ static int twl4030_soc_probe(struct platform_device *pdev)
 	if (setup) {
 		unsigned char hs_pop;
 
-		if (setup->sysclk)
-			twl4030->sysclk = setup->sysclk;
-		else
-			twl4030->sysclk = 26000;
+		if (setup->sysclk != twl4030->sysclk)
+			dev_warn(&pdev->dev, "sysclk mismatch (%u vs %u)\n",
+				 twl4030->sysclk, setup->sysclk);
 
 		hs_pop = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET);
 		hs_pop &= ~TWL4030_RAMP_DELAY;
 		hs_pop |= (setup->ramp_delay_value << 2);
 		twl4030_write_reg_cache(codec, TWL4030_REG_HS_POPN_SET, hs_pop);
-	} else {
-		twl4030->sysclk = 26000;
 	}
 
 	/* register pcms */
@@ -2191,10 +2175,8 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 	struct twl4030_priv *twl4030;
 	int ret;
 
-	if (!pdata || !(pdata->audio_mclk == 19200000 ||
-			pdata->audio_mclk == 26000000 ||
-			pdata->audio_mclk == 38400000)) {
-		dev_err(&pdev->dev, "Invalid platform_data\n");
+	if (!pdata) {
+		dev_err(&pdev->dev, "platform_data is missing\n");
 		return -EINVAL;
 	}
 
@@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 	twl4030_codec = codec;
 
 	/* Set the defaults, and power up the codec */
+	twl4030->sysclk = twl4030_codec_get_mclk() / 1000;
 	twl4030_init_chip(codec);
 	codec->bias_level = SND_SOC_BIAS_OFF;
 	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
-- 
1.6.5.2

[PATCH 5/5] ASoC: TWL4030: Do not modify the APLL_CTL register

[Peter Ujfalusi]

APLL_CTL register is configured by the twl4030-codec MFD
driver.
Remove code, which makes changes in the APLL_CTL register,
and replace those with checks against the configured
audio_mclk configuration done in the MFD driver.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/codecs/twl4030.c |   69 ++++++++++++++++---------------------------
 1 files changed, 26 insertions(+), 43 deletions(-)

diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index c0b47df..67cde72 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -214,7 +214,8 @@ static void twl4030_init_chip(struct snd_soc_codec *codec)
 
 	/* set all audio section registers to reasonable defaults */
 	for (i = TWL4030_REG_OPTION; i <= TWL4030_REG_MISC_SET_2; i++)
-		twl4030_write(codec, i,	cache[i]);
+		if (i != TWL4030_REG_APLL_CTL)
+			twl4030_write(codec, i,	cache[i]);
 
 }
 
@@ -1753,30 +1754,23 @@ static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
 	struct twl4030_priv *twl4030 = codec->private_data;
-	u8 apll_ctrl;
 
-	apll_ctrl = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL);
-	apll_ctrl &= ~TWL4030_APLL_INFREQ;
 	switch (freq) {
 	case 19200000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_19200KHZ;
-		twl4030->sysclk = 19200;
-		break;
 	case 26000000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_26000KHZ;
-		twl4030->sysclk = 26000;
-		break;
 	case 38400000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_38400KHZ;
-		twl4030->sysclk = 38400;
 		break;
 	default:
-		printk(KERN_ERR "TWL4030 set sysclk: unknown rate %d\n",
-			freq);
+		dev_err(codec->dev, "Unsupported frequency %u\n", freq);
 		return -EINVAL;
 	}
 
-	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
+	if ((freq / 1000) != twl4030->sysclk) {
+		dev_err(codec->dev,
+			"Mismatch in APLL infrequency %u (configred: %u)\n",
+			freq, twl4030->sysclk * 1000);
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -1874,16 +1868,13 @@ static int twl4030_voice_startup(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_device *socdev = rtd->socdev;
 	struct snd_soc_codec *codec = socdev->card->codec;
-	u8 infreq;
+	struct twl4030_priv *twl4030 = codec->private_data;
 	u8 mode;
 
 	/* If the system master clock is not 26MHz, the voice PCM interface is
 	 * not avilable.
 	 */
-	infreq = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL)
-		& TWL4030_APLL_INFREQ;
-
-	if (infreq != TWL4030_APLL_INFREQ_26000KHZ) {
+	if (twl4030->sysclk != 26000) {
 		printk(KERN_ERR "TWL4030 voice startup: "
 			"MCLK is not 26MHz, call set_sysclk() on init\n");
 		return -EINVAL;
@@ -1958,22 +1949,18 @@ static int twl4030_voice_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 		int clk_id, unsigned int freq, int dir)
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
-	u8 apll_ctrl;
+	struct twl4030_priv *twl4030 = codec->private_data;
 
-	apll_ctrl = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL);
-	apll_ctrl &= ~TWL4030_APLL_INFREQ;
-	switch (freq) {
-	case 26000000:
-		apll_ctrl |= TWL4030_APLL_INFREQ_26000KHZ;
-		break;
-	default:
-		printk(KERN_ERR "TWL4030 voice set sysclk: unknown rate %d\n",
-			freq);
+	if (freq != 26000000) {
+		dev_err(codec->dev, "Unsupported frequency %u\n", freq);
+		return -EINVAL;
+	}
+	if ((freq / 1000) != twl4030->sysclk) {
+		dev_err(codec->dev,
+			"Mismatch in APLL infrequency %u (configred: %u)\n",
+			freq, twl4030->sysclk * 1000);
 		return -EINVAL;
 	}
-
-	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
-
 	return 0;
 }
 
@@ -2131,17 +2118,14 @@ static int twl4030_soc_probe(struct platform_device *pdev)
 	if (setup) {
 		unsigned char hs_pop;
 
-		if (setup->sysclk)
-			twl4030->sysclk = setup->sysclk;
-		else
-			twl4030->sysclk = 26000;
+		if (setup->sysclk != twl4030->sysclk)
+			dev_warn(&pdev->dev, "sysclk mismatch (%u vs %u)\n",
+				 twl4030->sysclk, setup->sysclk);
 
 		hs_pop = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET);
 		hs_pop &= ~TWL4030_RAMP_DELAY;
 		hs_pop |= (setup->ramp_delay_value << 2);
 		twl4030_write_reg_cache(codec, TWL4030_REG_HS_POPN_SET, hs_pop);
-	} else {
-		twl4030->sysclk = 26000;
 	}
 
 	/* register pcms */
@@ -2191,10 +2175,8 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 	struct twl4030_priv *twl4030;
 	int ret;
 
-	if (!pdata || !(pdata->audio_mclk == 19200000 ||
-			pdata->audio_mclk == 26000000 ||
-			pdata->audio_mclk == 38400000)) {
-		dev_err(&pdev->dev, "Invalid platform_data\n");
+	if (!pdata) {
+		dev_err(&pdev->dev, "platform_data is missing\n");
 		return -EINVAL;
 	}
 
@@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
 	twl4030_codec = codec;
 
 	/* Set the defaults, and power up the codec */
+	twl4030->sysclk = twl4030_codec_get_mclk() / 1000;
 	twl4030_init_chip(codec);
 	codec->bias_level = SND_SOC_BIAS_OFF;
 	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
-- 
1.6.5.2

Re: [PATCH 5/5] ASoC: TWL4030: Do not modify the APLL_CTL register

[Mark Brown]

On Mon, Nov 02, 2009 at 02:34:55PM +0200, Peter Ujfalusi wrote:
> APLL_CTL register is configured by the twl4030-codec MFD
> driver.
> Remove code, which makes changes in the APLL_CTL register,
> and replace those with checks against the configured
> audio_mclk configuration done in the MFD driver.

This all looks mostly OK.  It might be nice to combine this patch with
the change to the MFD driver, simply because when the previous patch
said "move" but didn't remove anything from the CODEC driver it seemed a
bit surprising.  The MFD part of this is queued in ASoC ATM so it
shouldn't cause cross tree issues.

> -	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
> +	if ((freq / 1000) != twl4030->sysclk) {
> +		dev_err(codec->dev,
> +			"Mismatch in APLL infrequency %u (configred: %u)\n",
> +			freq, twl4030->sysclk * 1000);

Typos in the log message here (and similarly for the other DAI).

> -	if (infreq != TWL4030_APLL_INFREQ_26000KHZ) {
> +	if (twl4030->sysclk != 26000) {
>  		printk(KERN_ERR "TWL4030 voice startup: "
>  			"MCLK is not 26MHz, call set_sysclk() on init\n");
>  		return -EINVAL;

Is that advice still appropriate if things are specified via the
codec device now?

> @@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
>  	twl4030_codec = codec;
>  
>  	/* Set the defaults, and power up the codec */
> +	twl4030->sysclk = twl4030_codec_get_mclk() / 1000;
>  	twl4030_init_chip(codec);
>  	codec->bias_level = SND_SOC_BIAS_OFF;
>  	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);

twl4030_codec_get_mclk() feels like it ought to take a parameter saying
which twl4030, though the chances of having multiple twl4030s in one
system are remote so it's not a real issue.

Re: [PATCH 5/5] ASoC: TWL4030: Do not modify the APLL_CTL register

[Mark Brown]

On Mon, Nov 02, 2009 at 02:34:55PM +0200, Peter Ujfalusi wrote:
> APLL_CTL register is configured by the twl4030-codec MFD
> driver.
> Remove code, which makes changes in the APLL_CTL register,
> and replace those with checks against the configured
> audio_mclk configuration done in the MFD driver.

This all looks mostly OK.  It might be nice to combine this patch with
the change to the MFD driver, simply because when the previous patch
said "move" but didn't remove anything from the CODEC driver it seemed a
bit surprising.  The MFD part of this is queued in ASoC ATM so it
shouldn't cause cross tree issues.

> -	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
> +	if ((freq / 1000) != twl4030->sysclk) {
> +		dev_err(codec->dev,
> +			"Mismatch in APLL infrequency %u (configred: %u)\n",
> +			freq, twl4030->sysclk * 1000);

Typos in the log message here (and similarly for the other DAI).

> -	if (infreq != TWL4030_APLL_INFREQ_26000KHZ) {
> +	if (twl4030->sysclk != 26000) {
>  		printk(KERN_ERR "TWL4030 voice startup: "
>  			"MCLK is not 26MHz, call set_sysclk() on init\n");
>  		return -EINVAL;

Is that advice still appropriate if things are specified via the
codec device now?

> @@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
>  	twl4030_codec = codec;
>  
>  	/* Set the defaults, and power up the codec */
> +	twl4030->sysclk = twl4030_codec_get_mclk() / 1000;
>  	twl4030_init_chip(codec);
>  	codec->bias_level = SND_SOC_BIAS_OFF;
>  	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);

twl4030_codec_get_mclk() feels like it ought to take a parameter saying
which twl4030, though the chances of having multiple twl4030s in one
system are remote so it's not a real issue.

Re: [PATCH 4/5] ASoC: TWL4030: Make sure, that the codec is powered on startup

[Mark Brown]

On Mon, Nov 02, 2009 at 02:34:54PM +0200, Peter Ujfalusi wrote:
> Set the codec->bias_level to SND_SOC_BIAS_OFF before changing
> the initial bias level to STANDBY.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>

I've applied this one immediately since it doesn't need any of the other
stuff.

Re: [PATCH 4/5] ASoC: TWL4030: Make sure, that the codec is powered on startup

[Mark Brown]

On Mon, Nov 02, 2009 at 02:34:54PM +0200, Peter Ujfalusi wrote:
> Set the codec->bias_level to SND_SOC_BIAS_OFF before changing
> the initial bias level to STANDBY.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>

I've applied this one immediately since it doesn't need any of the other
stuff.

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Mark Brown]

On Mon, Nov 02, 2009 at 02:34:53PM +0200, Peter Ujfalusi wrote:
> Move the APLL_CTL register configuration to the twl4030-codec
> MFD driver.
> Provide also a function for childs to query the audio_mclk
> frequency.

This all looks good to me, some nitpicks below.

> +unsigned int twl4030_codec_get_mclk(void)
> +{
> +	struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
> +
> +	return codec->audio_mclk;
> +}
> +EXPORT_SYMBOL_GPL(twl4030_codec_get_mclk);

As I said in my followup to patch 5 this feels like it should have a
parameter to specify the twl4030 though in practical systems it won't
matter.

> +	if (!(pdata->audio_mclk == 19200000 ||
> +	      pdata->audio_mclk == 26000000 ||
> +	      pdata->audio_mclk == 38400000)) {
> +		dev_err(&pdev->dev, "Invalid audio_mclk\n");
> +		return -EINVAL;
> +	}

Might flow more naturally with a switch statement?

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Mark Brown]

On Mon, Nov 02, 2009 at 02:34:53PM +0200, Peter Ujfalusi wrote:
> Move the APLL_CTL register configuration to the twl4030-codec
> MFD driver.
> Provide also a function for childs to query the audio_mclk
> frequency.

This all looks good to me, some nitpicks below.

> +unsigned int twl4030_codec_get_mclk(void)
> +{
> +	struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
> +
> +	return codec->audio_mclk;
> +}
> +EXPORT_SYMBOL_GPL(twl4030_codec_get_mclk);

As I said in my followup to patch 5 this feels like it should have a
parameter to specify the twl4030 though in practical systems it won't
matter.

> +	if (!(pdata->audio_mclk == 19200000 ||
> +	      pdata->audio_mclk == 26000000 ||
> +	      pdata->audio_mclk == 38400000)) {
> +		dev_err(&pdev->dev, "Invalid audio_mclk\n");
> +		return -EINVAL;
> +	}

Might flow more naturally with a switch statement?

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Peter Ujfalusi]

On Monday 02 November 2009 19:30:35 ext Mark Brown wrote:
> On Mon, Nov 02, 2009 at 02:34:53PM +0200, Peter Ujfalusi wrote:
> > Move the APLL_CTL register configuration to the twl4030-codec
> > MFD driver.
> > Provide also a function for childs to query the audio_mclk
> > frequency.
> 
> This all looks good to me, some nitpicks below.
> 
> > +unsigned int twl4030_codec_get_mclk(void)
> > +{
> > +	struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
> > +
> > +	return codec->audio_mclk;
> > +}
> > +EXPORT_SYMBOL_GPL(twl4030_codec_get_mclk);
> 
> As I said in my followup to patch 5 this feels like it should have a
> parameter to specify the twl4030 though in practical systems it won't
> matter.

I agree that this does not look quite nice, but as you already mentioned, it is 
highly unlikely that one system would have more than one twl series of PM chip 
on board. Another reason is that we have other part of the twl, which needs 
resources from the codec part, but it is not loaded through the codec MFD, so 
providing the needed information is kind of tricky with that setup.

> 
> > +	if (!(pdata->audio_mclk == 19200000 ||
> > +	      pdata->audio_mclk == 26000000 ||
> > +	      pdata->audio_mclk == 38400000)) {
> > +		dev_err(&pdev->dev, "Invalid audio_mclk\n");
> > +		return -EINVAL;
> > +	}
> 
> Might flow more naturally with a switch statement?

Yes, true. This looks weird, I'll change it.

> 

-- 
Péter

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Peter Ujfalusi]

On Monday 02 November 2009 19:30:35 ext Mark Brown wrote:
> On Mon, Nov 02, 2009 at 02:34:53PM +0200, Peter Ujfalusi wrote:
> > Move the APLL_CTL register configuration to the twl4030-codec
> > MFD driver.
> > Provide also a function for childs to query the audio_mclk
> > frequency.
> 
> This all looks good to me, some nitpicks below.
> 
> > +unsigned int twl4030_codec_get_mclk(void)
> > +{
> > +	struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
> > +
> > +	return codec->audio_mclk;
> > +}
> > +EXPORT_SYMBOL_GPL(twl4030_codec_get_mclk);
> 
> As I said in my followup to patch 5 this feels like it should have a
> parameter to specify the twl4030 though in practical systems it won't
> matter.

I agree that this does not look quite nice, but as you already mentioned, it is 
highly unlikely that one system would have more than one twl series of PM chip 
on board. Another reason is that we have other part of the twl, which needs 
resources from the codec part, but it is not loaded through the codec MFD, so 
providing the needed information is kind of tricky with that setup.

> 
> > +	if (!(pdata->audio_mclk == 19200000 ||
> > +	      pdata->audio_mclk == 26000000 ||
> > +	      pdata->audio_mclk == 38400000)) {
> > +		dev_err(&pdev->dev, "Invalid audio_mclk\n");
> > +		return -EINVAL;
> > +	}
> 
> Might flow more naturally with a switch statement?

Yes, true. This looks weird, I'll change it.

> 

-- 
Péter

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Mark Brown]

> on board. Another reason is that we have other part of the twl, which needs 
> resources from the codec part, but it is not loaded through the codec MFD, so 
> providing the needed information is kind of tricky with that setup.

I had thought that the sub-device was supposed to handle the arbitration
and could be used here.  But like we said, it's not going to make any
difference in the grand scheme of things.

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Mark Brown]

> on board. Another reason is that we have other part of the twl, which needs 
> resources from the codec part, but it is not loaded through the codec MFD, so 
> providing the needed information is kind of tricky with that setup.

I had thought that the sub-device was supposed to handle the arbitration
and could be used here.  But like we said, it's not going to make any
difference in the grand scheme of things.

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Peter Ujfalusi]

On Tuesday 03 November 2009 15:44:38 ext Mark Brown wrote:
> > on board. Another reason is that we have other part of the twl, which
> > needs resources from the codec part, but it is not loaded through the
> > codec MFD, so providing the needed information is kind of tricky with
> > that setup.
> 
> I had thought that the sub-device was supposed to handle the arbitration
> and could be used here.

That would have been ideal. As such the audio and the Vibra interface is the 
only one directly inside of the codec part of twl, but there is a need for other 
modules to actually control resources within the codec part in order to operate, 
but they are not actually sub-devices of the codec MFD.

-- 
Péter

Re: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver

[Peter Ujfalusi]

On Tuesday 03 November 2009 15:44:38 ext Mark Brown wrote:
> > on board. Another reason is that we have other part of the twl, which
> > needs resources from the codec part, but it is not loaded through the
> > codec MFD, so providing the needed information is kind of tricky with
> > that setup.
> 
> I had thought that the sub-device was supposed to handle the arbitration
> and could be used here.

That would have been ideal. As such the audio and the Vibra interface is the 
only one directly inside of the codec part of twl, but there is a need for other 
modules to actually control resources within the codec part in order to operate, 
but they are not actually sub-devices of the codec MFD.

-- 
Péter

Re: [PATCH 5/5] ASoC: TWL4030: Do not modify the APLL_CTL register

[Peter Ujfalusi]

On Monday 02 November 2009 19:27:20 ext Mark Brown wrote:
> On Mon, Nov 02, 2009 at 02:34:55PM +0200, Peter Ujfalusi wrote:
> > APLL_CTL register is configured by the twl4030-codec MFD
> > driver.
> > Remove code, which makes changes in the APLL_CTL register,
> > and replace those with checks against the configured
> > audio_mclk configuration done in the MFD driver.
> 
> This all looks mostly OK.  It might be nice to combine this patch with
> the change to the MFD driver, simply because when the previous patch
> said "move" but didn't remove anything from the CODEC driver it seemed a
> bit surprising.  The MFD part of this is queued in ASoC ATM so it
> shouldn't cause cross tree issues.

I just wanted to keep patches for different subsystems separate, but I can 
either merge those two, or change the commit message in patch 3, that it will be 
not misleading. 

> 
> > -	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
> > +	if ((freq / 1000) != twl4030->sysclk) {
> > +		dev_err(codec->dev,
> > +			"Mismatch in APLL infrequency %u (configred: %u)\n",
> > +			freq, twl4030->sysclk * 1000);
> 
> Typos in the log message here (and similarly for the other DAI).

Will fix those.

> 
> > -	if (infreq != TWL4030_APLL_INFREQ_26000KHZ) {
> > +	if (twl4030->sysclk != 26000) {
> >  		printk(KERN_ERR "TWL4030 voice startup: "
> >  			"MCLK is not 26MHz, call set_sysclk() on init\n");
> >  		return -EINVAL;
> 
> Is that advice still appropriate if things are specified via the
> codec device now?

Correct, it is not correct.

> 
> > @@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct
> > platform_device *pdev) twl4030_codec = codec;
> >
> >  	/* Set the defaults, and power up the codec */
> > +	twl4030->sysclk = twl4030_codec_get_mclk() / 1000;
> >  	twl4030_init_chip(codec);
> >  	codec->bias_level = SND_SOC_BIAS_OFF;
> >  	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
> 
> twl4030_codec_get_mclk() feels like it ought to take a parameter saying
> which twl4030, though the chances of having multiple twl4030s in one
> system are remote so it's not a real issue.

We have discussed this, but, yes the chances of having more than one twl in a 
system is unlikely.



-- 
Péter

Re: [PATCH 5/5] ASoC: TWL4030: Do not modify the APLL_CTL register

[Peter Ujfalusi]

On Monday 02 November 2009 19:27:20 ext Mark Brown wrote:
> On Mon, Nov 02, 2009 at 02:34:55PM +0200, Peter Ujfalusi wrote:
> > APLL_CTL register is configured by the twl4030-codec MFD
> > driver.
> > Remove code, which makes changes in the APLL_CTL register,
> > and replace those with checks against the configured
> > audio_mclk configuration done in the MFD driver.
> 
> This all looks mostly OK.  It might be nice to combine this patch with
> the change to the MFD driver, simply because when the previous patch
> said "move" but didn't remove anything from the CODEC driver it seemed a
> bit surprising.  The MFD part of this is queued in ASoC ATM so it
> shouldn't cause cross tree issues.

I just wanted to keep patches for different subsystems separate, but I can 
either merge those two, or change the commit message in patch 3, that it will be 
not misleading. 

> 
> > -	twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl);
> > +	if ((freq / 1000) != twl4030->sysclk) {
> > +		dev_err(codec->dev,
> > +			"Mismatch in APLL infrequency %u (configred: %u)\n",
> > +			freq, twl4030->sysclk * 1000);
> 
> Typos in the log message here (and similarly for the other DAI).

Will fix those.

> 
> > -	if (infreq != TWL4030_APLL_INFREQ_26000KHZ) {
> > +	if (twl4030->sysclk != 26000) {
> >  		printk(KERN_ERR "TWL4030 voice startup: "
> >  			"MCLK is not 26MHz, call set_sysclk() on init\n");
> >  		return -EINVAL;
> 
> Is that advice still appropriate if things are specified via the
> codec device now?

Correct, it is not correct.

> 
> > @@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct
> > platform_device *pdev) twl4030_codec = codec;
> >
> >  	/* Set the defaults, and power up the codec */
> > +	twl4030->sysclk = twl4030_codec_get_mclk() / 1000;
> >  	twl4030_init_chip(codec);
> >  	codec->bias_level = SND_SOC_BIAS_OFF;
> >  	twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
> 
> twl4030_codec_get_mclk() feels like it ought to take a parameter saying
> which twl4030, though the chances of having multiple twl4030s in one
> system are remote so it's not a real issue.

We have discussed this, but, yes the chances of having more than one twl in a 
system is unlikely.



-- 
Péter