[PATCH] S3C64XX I2S: Added machine driver for WM8580

[PATCH] S3C64XX I2S: Added machine driver for WM8580

[Jassi]

New machine driver for WM8580 I2S i/f on SMDK64XX.
By default SoC-Slave is set and WM8580 is configured to use it's
PLLA to generate clocks from a 12MHz crystal attached to WM8580.
I2S controller-0 is set as default, set S3C64XX_I2S_CNTRLR to 2
inorder to use I2S_v4 controller of S3C6410.

Signed-off-by: Jassi <jassi.brar@samsung.com>
---
 sound/soc/s3c24xx/Kconfig           |    8 +
 sound/soc/s3c24xx/Makefile          |    2 +
 sound/soc/s3c24xx/smdk64xx_wm8580.c |  288 +++++++++++++++++++++++++++++++++++
 3 files changed, 298 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/s3c24xx/smdk64xx_wm8580.c

diff --git a/sound/soc/s3c24xx/Kconfig b/sound/soc/s3c24xx/Kconfig
index 923428f..769697e 100644
--- a/sound/soc/s3c24xx/Kconfig
+++ b/sound/soc/s3c24xx/Kconfig
@@ -56,6 +56,14 @@ config SND_S3C24XX_SOC_JIVE_WM8750
 	help
 	  Sat Y if you want to add support for SoC audio on the Jive.
 
+config SND_S3C64XX_SOC_WM8580
+	tristate "SoC I2S Audio support for WM8580 on SMDK64XX"
+	depends on SND_S3C24XX_SOC && (MACH_SMDK6400 || MACH_SMDK6410)
+	select SND_SOC_WM8580
+	select SND_S3C64XX_SOC_I2S
+	help
+	  Sat Y if you want to add support for SoC audio on the SMDK64XX.
+
 config SND_S3C24XX_SOC_SMDK2443_WM9710
 	tristate "SoC AC97 Audio support for SMDK2443 - WM9710"
 	depends on SND_S3C24XX_SOC && MACH_SMDK2443
diff --git a/sound/soc/s3c24xx/Makefile b/sound/soc/s3c24xx/Makefile
index 99f5a7d..7790406 100644
--- a/sound/soc/s3c24xx/Makefile
+++ b/sound/soc/s3c24xx/Makefile
@@ -23,6 +23,7 @@ snd-soc-s3c24xx-uda134x-objs := s3c24xx_uda134x.o
 snd-soc-s3c24xx-simtec-objs := s3c24xx_simtec.o
 snd-soc-s3c24xx-simtec-hermes-objs := s3c24xx_simtec_hermes.o
 snd-soc-s3c24xx-simtec-tlv320aic23-objs := s3c24xx_simtec_tlv320aic23.o
+snd-soc-smdk64xx-wm8580-objs := smdk64xx_wm8580.o
 
 obj-$(CONFIG_SND_S3C24XX_SOC_JIVE_WM8750) += snd-soc-jive-wm8750.o
 obj-$(CONFIG_SND_S3C24XX_SOC_NEO1973_WM8753) += snd-soc-neo1973-wm8753.o
@@ -33,4 +34,5 @@ obj-$(CONFIG_SND_S3C24XX_SOC_S3C24XX_UDA134X) += snd-soc-s3c24xx-uda134x.o
 obj-$(CONFIG_SND_S3C24XX_SOC_SIMTEC) += snd-soc-s3c24xx-simtec.o
 obj-$(CONFIG_SND_S3C24XX_SOC_SIMTEC_HERMES) += snd-soc-s3c24xx-simtec-hermes.o
 obj-$(CONFIG_SND_S3C24XX_SOC_SIMTEC_TLV320AIC23) += snd-soc-s3c24xx-simtec-tlv320aic23.o
+obj-$(CONFIG_SND_S3C64XX_SOC_WM8580) += snd-soc-smdk64xx-wm8580.o
 
diff --git a/sound/soc/s3c24xx/smdk64xx_wm8580.c b/sound/soc/s3c24xx/smdk64xx_wm8580.c
new file mode 100644
index 0000000..6489e52
--- /dev/null
+++ b/sound/soc/s3c24xx/smdk64xx_wm8580.c
@@ -0,0 +1,288 @@
+/*
+ *  smdk64xx_wm8580.c
+ *
+ *  Copyright (c) 2009 Samsung Electronics Co. Ltd
+ *  Author: Jaswinder Singh <jassi.brar@samsung.com>
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+#include "../codecs/wm8580.h"
+#include "s3c24xx-pcm.h"
+#include "s3c64xx-i2s.h"
+
+#define S3C64XX_I2S_CNTRLR 0
+
+/* SMDK64XX has a 12MHZ crystal attached to WM8580 */
+#define SMDK64XX_WM8580_FREQ (12000000)
+
+static int smdk64xx_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
+	unsigned int pll_out;
+	int bfs, rfs, ret;
+
+	switch (params_rate(params)) {
+	case 8000:
+		pll_out = 8192000 / 4; break;
+	case 11025:
+		pll_out = 11289600 / 4; break;
+	case 16000:
+		pll_out = 8192000 / 2; break;
+	case 22050:
+		pll_out = 11289600 / 2; break;
+	case 32000:
+		pll_out = 8192000; break;
+	case 44100:
+		pll_out = 11289600; break;
+	case 48000:
+		pll_out = 12288000; break;
+	case 64000:
+		pll_out = 24576000; break;
+	case 88200:
+		pll_out = 11289600 * 2; break;
+	case 96000:
+		pll_out = 12288000 * 2; break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Set the Codec DAI configuration */
+	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S
+					 | SND_SOC_DAIFMT_NB_NF
+					 | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0)
+		return ret;
+
+	/* Set the AP DAI configuration */
+	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S
+					 | SND_SOC_DAIFMT_NB_NF
+					 | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0)
+		return ret;
+
+	/* We block CODCLK from MCLK of WM8580 */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, S3C64XX_CODCLKSRC_EXT,
+					0, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	/* We use PCLK for basic ops in SoC-Slave mode */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, S3C64XX_CLKSRC_PCLK,
+					0, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	/* Set WM8580 to drive MCLK from it's PLLA */
+	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_MCLK,
+					WM8580_CLKSRC_PLLA);
+	if (ret < 0)
+		return ret;
+
+	/* Explicitly set WM8580-ADC/DAC to source from MCLK */
+	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_ADC_CLKSEL,
+					WM8580_CLKSRC_MCLK);
+	if (ret < 0)
+		return ret;
+	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_DAC_CLKSEL,
+					WM8580_CLKSRC_MCLK);
+	if (ret < 0)
+		return ret;
+
+	/* Don't output to the disconnected pin */
+	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_CLKOUTSRC,
+					WM8580_CLKSRC_NONE);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_pll(codec_dai, WM8580_PLLA,
+					SMDK64XX_WM8580_FREQ, pll_out);
+	if (ret < 0)
+		return ret;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_U8:
+	case SNDRV_PCM_FORMAT_S8:
+		bfs = 16;
+		rfs = 256;
+		break;
+	case SNDRV_PCM_FORMAT_U16_LE:
+	case SNDRV_PCM_FORMAT_S16_LE:
+		bfs = 32;
+		rfs = 256;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C_I2SV2_DIV_BCLK, bfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C_I2SV2_DIV_RCLK, rfs);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int smdk64xx_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
+
+	/* disable the PLL */
+	return snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 0, 0);
+}
+
+/*
+ * SMDK64XX WM8580 DAI operations.
+ */
+static struct snd_soc_ops smdk64xx_ops = {
+	.hw_params = smdk64xx_hw_params,
+	.hw_free = smdk64xx_hw_free,
+};
+
+/* SMDK64xx Playback widgets */
+static const struct snd_soc_dapm_widget wm8580_dapm_widgets_pbk[] = {
+	SND_SOC_DAPM_HP("Front-L/R", NULL),
+	SND_SOC_DAPM_HP("Center/Sub", NULL),
+	SND_SOC_DAPM_HP("Rear-L/R", NULL),
+};
+
+/* SMDK64xx Capture widgets */
+static const struct snd_soc_dapm_widget wm8580_dapm_widgets_cpt[] = {
+	SND_SOC_DAPM_MIC("MicIn", NULL),
+	SND_SOC_DAPM_LINE("LineIn", NULL),
+};
+
+/* SMDK-PAIFTX connections */
+static const struct snd_soc_dapm_route audio_map_tx[] = {
+	/* MicIn feeds AINL */
+	{"AINL", NULL, "MicIn"},
+
+	/* LineIn feeds AINL/R */
+	{"AINL", NULL, "LineIn"},
+	{"AINR", NULL, "LineIn"},
+};
+
+/* SMDK-PAIFRX connections */
+static const struct snd_soc_dapm_route audio_map_rx[] = {
+	/* Front Left/Right are fed VOUT1L/R */
+	{"Front-L/R", NULL, "VOUT1L"},
+	{"Front-L/R", NULL, "VOUT1R"},
+
+	/* Center/Sub are fed VOUT2L/R */
+	{"Center/Sub", NULL, "VOUT2L"},
+	{"Center/Sub", NULL, "VOUT2R"},
+
+	/* Rear Left/Right are fed VOUT3L/R */
+	{"Rear-L/R", NULL, "VOUT3L"},
+	{"Rear-L/R", NULL, "VOUT3R"},
+};
+
+static int smdk64xx_wm8580_init_paiftx(struct snd_soc_codec *codec)
+{
+	/* Add smdk64xx specific Capture widgets */
+	snd_soc_dapm_new_controls(codec, wm8580_dapm_widgets_cpt,
+				  ARRAY_SIZE(wm8580_dapm_widgets_cpt));
+
+	/* Set up PAIFTX audio path */
+	snd_soc_dapm_add_routes(codec, audio_map_tx, ARRAY_SIZE(audio_map_tx));
+
+	/* LineIn enabled by default */
+	snd_soc_dapm_disable_pin(codec, "MicIn");
+	snd_soc_dapm_enable_pin(codec, "LineIn");
+
+	/* signal a DAPM event */
+	snd_soc_dapm_sync(codec);
+
+	return 0;
+}
+
+static int smdk64xx_wm8580_init_paifrx(struct snd_soc_codec *codec)
+{
+	/* Add smdk64xx specific Playback widgets */
+	snd_soc_dapm_new_controls(codec, wm8580_dapm_widgets_pbk,
+				  ARRAY_SIZE(wm8580_dapm_widgets_pbk));
+
+	/* Set up PAIFRX audio path */
+	snd_soc_dapm_add_routes(codec, audio_map_rx, ARRAY_SIZE(audio_map_rx));
+
+	/* Stereo enabled by default */
+	snd_soc_dapm_enable_pin(codec, "Front-L/R");
+	snd_soc_dapm_disable_pin(codec, "Center/Sub");
+	snd_soc_dapm_disable_pin(codec, "Rear-L/R");
+
+	/* signal a DAPM event */
+	snd_soc_dapm_sync(codec);
+
+	return 0;
+}
+
+static struct snd_soc_dai_link smdk64xx_dai[] = {
+{ /* Primary Playback i/f */
+	.name = "WM8580 PAIF RX",
+	.stream_name = "Playback",
+	.cpu_dai = &s3c64xx_i2s_dai[S3C64XX_I2S_CNTRLR],
+	.codec_dai = &wm8580_dai[WM8580_DAI_PAIFRX],
+	.init = smdk64xx_wm8580_init_paifrx,
+	.ops = &smdk64xx_ops,
+},
+{ /* Primary Capture i/f */
+	.name = "WM8580 PAIF TX",
+	.stream_name = "Capture",
+	.cpu_dai = &s3c64xx_i2s_dai[S3C64XX_I2S_CNTRLR],
+	.codec_dai = &wm8580_dai[WM8580_DAI_PAIFTX],
+	.init = smdk64xx_wm8580_init_paiftx,
+	.ops = &smdk64xx_ops,
+},
+};
+
+static struct snd_soc_card smdk64xx = {
+	.name = "smdk64xx",
+	.platform = &s3c24xx_soc_platform,
+	.dai_link = smdk64xx_dai,
+	.num_links = ARRAY_SIZE(smdk64xx_dai),
+};
+
+static struct snd_soc_device smdk64xx_snd_devdata = {
+	.card = &smdk64xx,
+	.codec_dev = &soc_codec_dev_wm8580,
+};
+
+static struct platform_device *smdk64xx_snd_device;
+
+static int __init smdk64xx_audio_init(void)
+{
+	int ret;
+
+	smdk64xx_snd_device = platform_device_alloc("soc-audio", -1);
+	if (!smdk64xx_snd_device)
+		return -ENOMEM;
+
+	platform_set_drvdata(smdk64xx_snd_device, &smdk64xx_snd_devdata);
+	smdk64xx_snd_devdata.dev = &smdk64xx_snd_device->dev;
+	ret = platform_device_add(smdk64xx_snd_device);
+
+	if (ret)
+		platform_device_put(smdk64xx_snd_device);
+
+	return ret;
+}
+module_init(smdk64xx_audio_init);
+
+static void __exit smdk64xx_audio_exit(void)
+{
+	platform_device_unregister(smdk64xx_snd_device);
+}
+module_exit(smdk64xx_audio_exit);
+
+/* Module information */
+MODULE_DESCRIPTION("ALSA SoC SMDK64XX WM8580");
+MODULE_LICENSE("GPL");
-- 
1.6.2.5

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[Mark Brown]

On Wed, Sep 16, 2009 at 07:02:42PM +0900, Jassi wrote:

> New machine driver for WM8580 I2S i/f on SMDK64XX.
> By default SoC-Slave is set and WM8580 is configured to use it's
> PLLA to generate clocks from a 12MHz crystal attached to WM8580.

This looks sane as a starting point and most of the driver looks OK.

> I2S controller-0 is set as default, set S3C64XX_I2S_CNTRLR to 2
> inorder to use I2S_v4 controller of S3C6410.

I'm not entirely sure what's going on in this configuration - the
primary AIFs of the CODEC appear to be hard wired to the IISv4 interface
in the schematics I have, IIS port 0 is only wired to the secondary AIF
of the WM8580 so will need support for that implementing.  The end
result should be an extra set of DAI links for the secondary AIF.  Could
you clarify, please?

> +/* SMDK64XX has a 12MHZ crystal attached to WM8580 */
> +#define SMDK64XX_WM8580_FREQ (12000000)

No need for the () here.

> +	switch (params_rate(params)) {
> +	case 8000:
> +		pll_out = 8192000 / 4; break;

These are all a bit odd - you're taking two magic numbers and performing
an operation on them, giving another magic number.  For the most part
the resulting magic number is always 256fs (I suspect the cases where
they aren't should be 256fs).  Probably replacing all of this with

	pll_out = params_rate(params) * 256

would work.

> +	/* We block CODCLK from MCLK of WM8580 */
> +	ret = snd_soc_dai_set_sysclk(cpu_dai, S3C64XX_CODCLKSRC_EXT,
> +					0, SND_SOC_CLOCK_IN);
> +	if (ret < 0)
> +		return ret;

The code is OK (modulo the issues with the previous patch) but the
comment is exceptionally difficult to parse.

> +	/* Explicitly set WM8580-ADC/DAC to source from MCLK */
> +	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_ADC_CLKSEL,
> +					WM8580_CLKSRC_MCLK);

Obviously this will need updating for the actual patch for the WM8580
driver.

> +	if (ret < 0)
> +		return ret;
> +	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_DAC_CLKSEL,

Missing blank line here.

> +	/* Don't output to the disconnected pin */
> +	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_CLKOUTSRC,
> +					WM8580_CLKSRC_NONE);
> +	if (ret < 0)
> +		return ret;

If you're going to do this do it on init, it's nothing to do with
starting the audio stream.

> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_U8:
> +	case SNDRV_PCM_FORMAT_S8:
> +		bfs = 16;
> +		rfs = 256;
> +		break;
> +	case SNDRV_PCM_FORMAT_U16_LE:
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		bfs = 32;
> +		rfs = 256;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

This ought to get factored out into the CPU DAI driver, at least as a
library function - there's nothing board specific going on here.

> +	/* LineIn enabled by default */
> +	snd_soc_dapm_disable_pin(codec, "MicIn");
> +	snd_soc_dapm_enable_pin(codec, "LineIn");

Please add a comment explaining that if this is changed it needs to be
done with a resistor fit mod - this begs the question "How do I change
away from the default?", especially given the many switches for things
like this on the board.

For completeness if you're going to handle the mapping to the external
jacks there's also the possiblity that the signals will be switched out
to the PMIC card or WM9713.  It's probably better to just not bother,
it's not really adding anything since there's no jack detect or anything
on the board.  All that'll happen is that people are forced to rebuild
for DIP switch changes.

The other option would be to add controls mirroring the DIP switch
settings, but that'd not really cover the resistor fit stuff.

> +	/* Stereo enabled by default */
> +	snd_soc_dapm_enable_pin(codec, "Front-L/R");
> +	snd_soc_dapm_disable_pin(codec, "Center/Sub");
> +	snd_soc_dapm_disable_pin(codec, "Rear-L/R");

Similar issues here - putting a fixed configuration from this in the
driver doesn't buy anything.

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[jassi brar]

seems my original email to alsa-devel got stuck.
lemme resend the patch with some of ur suggestions incorporated and
continue from there.

On Thu, Sep 17, 2009 at 5:00 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Sep 16, 2009 at 07:02:42PM +0900, Jassi wrote:
>
>> New machine driver for WM8580 I2S i/f on SMDK64XX.
>> By default SoC-Slave is set and WM8580 is configured to use it's
>> PLLA to generate clocks from a 12MHz crystal attached to WM8580.
>
> This looks sane as a starting point and most of the driver looks OK.
>
>> I2S controller-0 is set as default, set S3C64XX_I2S_CNTRLR to 2
>> inorder to use I2S_v4 controller of S3C6410.
>
> I'm not entirely sure what's going on in this configuration - the
> primary AIFs of the CODEC appear to be hard wired to the IISv4 interface
> in the schematics I have, IIS port 0 is only wired to the secondary AIF
> of the WM8580 so will need support for that implementing.  The end
> result should be an extra set of DAI links for the secondary AIF.  Could
> you clarify, please?
>
>> +/* SMDK64XX has a 12MHZ crystal attached to WM8580 */
>> +#define SMDK64XX_WM8580_FREQ (12000000)
>
> No need for the () here.
>
>> +     switch (params_rate(params)) {
>> +     case 8000:
>> +             pll_out = 8192000 / 4; break;
>
> These are all a bit odd - you're taking two magic numbers and performing
> an operation on them, giving another magic number.  For the most part
> the resulting magic number is always 256fs (I suspect the cases where
> they aren't should be 256fs).  Probably replacing all of this with
>
>        pll_out = params_rate(params) * 256
>
> would work.
>
>> +     /* We block CODCLK from MCLK of WM8580 */
>> +     ret = snd_soc_dai_set_sysclk(cpu_dai, S3C64XX_CODCLKSRC_EXT,
>> +                                     0, SND_SOC_CLOCK_IN);
>> +     if (ret < 0)
>> +             return ret;
>
> The code is OK (modulo the issues with the previous patch) but the
> comment is exceptionally difficult to parse.
>
>> +     /* Explicitly set WM8580-ADC/DAC to source from MCLK */
>> +     ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_ADC_CLKSEL,
>> +                                     WM8580_CLKSRC_MCLK);
>
> Obviously this will need updating for the actual patch for the WM8580
> driver.
>
>> +     if (ret < 0)
>> +             return ret;
>> +     ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_DAC_CLKSEL,
>
> Missing blank line here.
>
>> +     /* Don't output to the disconnected pin */
>> +     ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_CLKOUTSRC,
>> +                                     WM8580_CLKSRC_NONE);
>> +     if (ret < 0)
>> +             return ret;
>
> If you're going to do this do it on init, it's nothing to do with
> starting the audio stream.
>
>> +     switch (params_format(params)) {
>> +     case SNDRV_PCM_FORMAT_U8:
>> +     case SNDRV_PCM_FORMAT_S8:
>> +             bfs = 16;
>> +             rfs = 256;
>> +             break;
>> +     case SNDRV_PCM_FORMAT_U16_LE:
>> +     case SNDRV_PCM_FORMAT_S16_LE:
>> +             bfs = 32;
>> +             rfs = 256;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>
> This ought to get factored out into the CPU DAI driver, at least as a
> library function - there's nothing board specific going on here.
>
>> +     /* LineIn enabled by default */
>> +     snd_soc_dapm_disable_pin(codec, "MicIn");
>> +     snd_soc_dapm_enable_pin(codec, "LineIn");
>
> Please add a comment explaining that if this is changed it needs to be
> done with a resistor fit mod - this begs the question "How do I change
> away from the default?", especially given the many switches for things
> like this on the board.
>
> For completeness if you're going to handle the mapping to the external
> jacks there's also the possiblity that the signals will be switched out
> to the PMIC card or WM9713.  It's probably better to just not bother,
> it's not really adding anything since there's no jack detect or anything
> on the board.  All that'll happen is that people are forced to rebuild
> for DIP switch changes.
>
> The other option would be to add controls mirroring the DIP switch
> settings, but that'd not really cover the resistor fit stuff.
>
>> +     /* Stereo enabled by default */
>> +     snd_soc_dapm_enable_pin(codec, "Front-L/R");
>> +     snd_soc_dapm_disable_pin(codec, "Center/Sub");
>> +     snd_soc_dapm_disable_pin(codec, "Rear-L/R");
>
> Similar issues here - putting a fixed configuration from this in the
> driver doesn't buy anything.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH] S3C64XX I2S: Added machine driver for WM8580

[jassi brar]

New machine driver for WM8580 I2S i/f on SMDK64XX.
By default SoC-Slave is set and WM8580 is configured to use it's
PLLA to generate clocks from a 12MHz crystal attached to WM8580.

Signed-off-by: Jassi <jassi.brar@samsung.com>
---
 sound/soc/s3c24xx/Kconfig           |    8 +
 sound/soc/s3c24xx/Makefile          |    2 +
 sound/soc/s3c24xx/smdk64xx_wm8580.c |  284 +++++++++++++++++++++++++++++++++++
 3 files changed, 294 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/s3c24xx/smdk64xx_wm8580.c

diff --git a/sound/soc/s3c24xx/Kconfig b/sound/soc/s3c24xx/Kconfig
index 923428f..769697e 100644
--- a/sound/soc/s3c24xx/Kconfig
+++ b/sound/soc/s3c24xx/Kconfig
@@ -56,6 +56,14 @@ config SND_S3C24XX_SOC_JIVE_WM8750
 	help
 	  Sat Y if you want to add support for SoC audio on the Jive.
 
+config SND_S3C64XX_SOC_WM8580
+	tristate "SoC I2S Audio support for WM8580 on SMDK64XX"
+	depends on SND_S3C24XX_SOC && (MACH_SMDK6400 || MACH_SMDK6410)
+	select SND_SOC_WM8580
+	select SND_S3C64XX_SOC_I2S
+	help
+	  Sat Y if you want to add support for SoC audio on the SMDK64XX.
+
 config SND_S3C24XX_SOC_SMDK2443_WM9710
 	tristate "SoC AC97 Audio support for SMDK2443 - WM9710"
 	depends on SND_S3C24XX_SOC && MACH_SMDK2443
diff --git a/sound/soc/s3c24xx/Makefile b/sound/soc/s3c24xx/Makefile
index 99f5a7d..7790406 100644
--- a/sound/soc/s3c24xx/Makefile
+++ b/sound/soc/s3c24xx/Makefile
@@ -23,6 +23,7 @@ snd-soc-s3c24xx-uda134x-objs := s3c24xx_uda134x.o
 snd-soc-s3c24xx-simtec-objs := s3c24xx_simtec.o
 snd-soc-s3c24xx-simtec-hermes-objs := s3c24xx_simtec_hermes.o
 snd-soc-s3c24xx-simtec-tlv320aic23-objs := s3c24xx_simtec_tlv320aic23.o
+snd-soc-smdk64xx-wm8580-objs := smdk64xx_wm8580.o
 
 obj-$(CONFIG_SND_S3C24XX_SOC_JIVE_WM8750) += snd-soc-jive-wm8750.o
 obj-$(CONFIG_SND_S3C24XX_SOC_NEO1973_WM8753) += snd-soc-neo1973-wm8753.o
@@ -33,4 +34,5 @@ obj-$(CONFIG_SND_S3C24XX_SOC_S3C24XX_UDA134X) += snd-soc-s3c24xx-uda134x.o
 obj-$(CONFIG_SND_S3C24XX_SOC_SIMTEC) += snd-soc-s3c24xx-simtec.o
 obj-$(CONFIG_SND_S3C24XX_SOC_SIMTEC_HERMES) += snd-soc-s3c24xx-simtec-hermes.o
 obj-$(CONFIG_SND_S3C24XX_SOC_SIMTEC_TLV320AIC23) += snd-soc-s3c24xx-simtec-tlv320aic23.o
+obj-$(CONFIG_SND_S3C64XX_SOC_WM8580) += snd-soc-smdk64xx-wm8580.o
 
diff --git a/sound/soc/s3c24xx/smdk64xx_wm8580.c b/sound/soc/s3c24xx/smdk64xx_wm8580.c
new file mode 100644
index 0000000..186e1de
--- /dev/null
+++ b/sound/soc/s3c24xx/smdk64xx_wm8580.c
@@ -0,0 +1,284 @@
+/*
+ *  smdk64xx_wm8580.c
+ *
+ *  Copyright (c) 2009 Samsung Electronics Co. Ltd
+ *  Author: Jaswinder Singh <jassi.brar@samsung.com>
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+#include "../codecs/wm8580.h"
+#include "s3c24xx-pcm.h"
+#include "s3c64xx-i2s.h"
+
+#define S3C64XX_I2S_V4 2
+
+/* SMDK64XX has a 12MHZ crystal attached to WM8580 */
+#define SMDK64XX_WM8580_FREQ 12000000
+
+static int smdk64xx_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
+	unsigned int pll_out;
+	int bfs, rfs, ret;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_U8:
+	case SNDRV_PCM_FORMAT_S8:
+		bfs = 16;
+		break;
+	case SNDRV_PCM_FORMAT_U16_LE:
+	case SNDRV_PCM_FORMAT_S16_LE:
+		bfs = 32;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Currently, WM8580 driver doesn't support PLL-out rates
+	 * other than those mentioned in Table-52 Page-58 of WM8580A
+	 * manual. Some of these unavailable rates include
+	 * {8000x256, 64000x256, 11025x256}Hz.
+	 * As a wayout, we rather change rfs to a value that results in
+	 * (params_rate(params) * rfs), and itself, acceptable to the both
+	 * the CODEC and the CPU.
+	 */
+	switch (params_rate(params)) {
+	case 16000:
+	case 22050:
+	case 32000:
+	case 44100:
+	case 48000:
+	case 88200:
+	case 96000:
+		rfs = 256;
+		break;
+	/* Exceptions for this CPU-CODEC combination */
+	case 64000:
+		rfs = 384;
+		break;
+	case 8000:
+	case 11025:
+		rfs = 512;
+		break;
+	default:
+		return -EINVAL;
+	}
+	pll_out = params_rate(params) * rfs;
+
+	/* Set the Codec DAI configuration */
+	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S
+					 | SND_SOC_DAIFMT_NB_NF
+					 | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0)
+		return ret;
+
+	/* Set the AP DAI configuration */
+	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S
+					 | SND_SOC_DAIFMT_NB_NF
+					 | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_sysclk(cpu_dai, S3C64XX_CLKSRC_CDCLK,
+					0, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	/* We use PCLK for basic ops in SoC-Slave mode */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, S3C64XX_CLKSRC_PCLK,
+					0, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	/* Set WM8580 to drive MCLK from it's PLLA */
+	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_MCLK,
+					WM8580_CLKSRC_PLLA);
+	if (ret < 0)
+		return ret;
+
+	/* Explicitly set WM8580-DAC to source from MCLK */
+	ret = snd_soc_dai_set_clkdiv(codec_dai, WM8580_DAC_CLKSEL,
+					WM8580_CLKSRC_MCLK);
+	if (ret < 0)
+		return ret;
+
+	/* Assuming the CODEC driver evaluates it's rfs too from this call */
+	ret = snd_soc_dai_set_pll(codec_dai, WM8580_PLLA,
+					SMDK64XX_WM8580_FREQ, pll_out);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C_I2SV2_DIV_BCLK, bfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C_I2SV2_DIV_RCLK, rfs);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int smdk64xx_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
+
+	/* disable the PLL */
+	return snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 0, 0);
+}
+
+/*
+ * SMDK64XX WM8580 DAI operations.
+ */
+static struct snd_soc_ops smdk64xx_ops = {
+	.hw_params = smdk64xx_hw_params,
+	.hw_free = smdk64xx_hw_free,
+};
+
+/* SMDK64xx Playback widgets */
+static const struct snd_soc_dapm_widget wm8580_dapm_widgets_pbk[] = {
+	SND_SOC_DAPM_HP("Front-L/R", NULL),
+	SND_SOC_DAPM_HP("Center/Sub", NULL),
+	SND_SOC_DAPM_HP("Rear-L/R", NULL),
+};
+
+/* SMDK64xx Capture widgets */
+static const struct snd_soc_dapm_widget wm8580_dapm_widgets_cpt[] = {
+	SND_SOC_DAPM_MIC("MicIn", NULL),
+	SND_SOC_DAPM_LINE("LineIn", NULL),
+};
+
+/* SMDK-PAIFTX connections */
+static const struct snd_soc_dapm_route audio_map_tx[] = {
+	/* MicIn feeds AINL */
+	{"AINL", NULL, "MicIn"},
+
+	/* LineIn feeds AINL/R */
+	{"AINL", NULL, "LineIn"},
+	{"AINR", NULL, "LineIn"},
+};
+
+/* SMDK-PAIFRX connections */
+static const struct snd_soc_dapm_route audio_map_rx[] = {
+	/* Front Left/Right are fed VOUT1L/R */
+	{"Front-L/R", NULL, "VOUT1L"},
+	{"Front-L/R", NULL, "VOUT1R"},
+
+	/* Center/Sub are fed VOUT2L/R */
+	{"Center/Sub", NULL, "VOUT2L"},
+	{"Center/Sub", NULL, "VOUT2R"},
+
+	/* Rear Left/Right are fed VOUT3L/R */
+	{"Rear-L/R", NULL, "VOUT3L"},
+	{"Rear-L/R", NULL, "VOUT3R"},
+};
+
+static int smdk64xx_wm8580_init_paiftx(struct snd_soc_codec *codec)
+{
+	/* Add smdk64xx specific Capture widgets */
+	snd_soc_dapm_new_controls(codec, wm8580_dapm_widgets_cpt,
+				  ARRAY_SIZE(wm8580_dapm_widgets_cpt));
+
+	/* Set up PAIFTX audio path */
+	snd_soc_dapm_add_routes(codec, audio_map_tx, ARRAY_SIZE(audio_map_tx));
+
+	/* LineIn enabled by default */
+	snd_soc_dapm_disable_pin(codec, "MicIn");
+	snd_soc_dapm_enable_pin(codec, "LineIn");
+
+	/* signal a DAPM event */
+	snd_soc_dapm_sync(codec);
+
+	return 0;
+}
+
+static int smdk64xx_wm8580_init_paifrx(struct snd_soc_codec *codec)
+{
+	/* Add smdk64xx specific Playback widgets */
+	snd_soc_dapm_new_controls(codec, wm8580_dapm_widgets_pbk,
+				  ARRAY_SIZE(wm8580_dapm_widgets_pbk));
+
+	/* Set up PAIFRX audio path */
+	snd_soc_dapm_add_routes(codec, audio_map_rx, ARRAY_SIZE(audio_map_rx));
+
+	/* Stereo enabled by default */
+	snd_soc_dapm_enable_pin(codec, "Front-L/R");
+	snd_soc_dapm_disable_pin(codec, "Center/Sub");
+	snd_soc_dapm_disable_pin(codec, "Rear-L/R");
+
+	/* signal a DAPM event */
+	snd_soc_dapm_sync(codec);
+
+	return 0;
+}
+static struct snd_soc_dai_link smdk64xx_dai[] = {
+{ /* Primary Playback i/f */
+	.name = "WM8580 PAIF RX",
+	.stream_name = "Playback",
+	.cpu_dai = &s3c64xx_i2s_dai[S3C64XX_I2S_V4],
+	.codec_dai = &wm8580_dai[WM8580_DAI_PAIFRX],
+	.init = smdk64xx_wm8580_init_paifrx,
+	.ops = &smdk64xx_ops,
+},
+{ /* Primary Capture i/f */
+	.name = "WM8580 PAIF TX",
+	.stream_name = "Capture",
+	.cpu_dai = &s3c64xx_i2s_dai[S3C64XX_I2S_V4],
+	.codec_dai = &wm8580_dai[WM8580_DAI_PAIFTX],
+	.init = smdk64xx_wm8580_init_paiftx,
+	.ops = &smdk64xx_ops,
+},
+};
+
+static struct snd_soc_card smdk64xx = {
+	.name = "smdk64xx",
+	.platform = &s3c24xx_soc_platform,
+	.dai_link = smdk64xx_dai,
+	.num_links = ARRAY_SIZE(smdk64xx_dai),
+};
+
+static struct snd_soc_device smdk64xx_snd_devdata = {
+	.card = &smdk64xx,
+	.codec_dev = &soc_codec_dev_wm8580,
+};
+
+static struct platform_device *smdk64xx_snd_device;
+
+static int __init smdk64xx_audio_init(void)
+{
+	int ret;
+
+	smdk64xx_snd_device = platform_device_alloc("soc-audio", -1);
+	if (!smdk64xx_snd_device)
+		return -ENOMEM;
+
+	platform_set_drvdata(smdk64xx_snd_device, &smdk64xx_snd_devdata);
+	smdk64xx_snd_devdata.dev = &smdk64xx_snd_device->dev;
+	ret = platform_device_add(smdk64xx_snd_device);
+
+	if (ret)
+		platform_device_put(smdk64xx_snd_device);
+
+	return ret;
+}
+module_init(smdk64xx_audio_init);
+
+MODULE_AUTHOR("Jaswinder Singh, jassi.brar@samsung.com");
+MODULE_DESCRIPTION("ALSA SoC SMDK64XX WM8580");
+MODULE_LICENSE("GPL");
-- 
1.6.2.5

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[Mark Brown]

On Thu, Sep 17, 2009 at 01:54:13PM +0900, jassi brar wrote:

> +	/* Exceptions for this CPU-CODEC combination */
> +	case 64000:
> +		rfs = 384;
> +		break;

This needs to explain what the exceptions are for as well, otherwise
anyone working with the code is going to have to work it out from
scratch.

> +/* SMDK64xx Playback widgets */
> +static const struct snd_soc_dapm_widget wm8580_dapm_widgets_pbk[] = {
> +	SND_SOC_DAPM_HP("Front-L/R", NULL),
> +	SND_SOC_DAPM_HP("Center/Sub", NULL),
> +	SND_SOC_DAPM_HP("Rear-L/R", NULL),
> +};

All my previous comments about these and the way they're being
configured still apply.

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[jassi brar]

On Thu, Sep 17, 2009 at 8:02 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Sep 17, 2009 at 01:54:13PM +0900, jassi brar wrote:
>
>> +     /* Exceptions for this CPU-CODEC combination */
>> +     case 64000:
>> +             rfs = 384;
>> +             break;
>
> This needs to explain what the exceptions are for as well, otherwise
> anyone working with the code is going to have to work it out from
> scratch.
This note means stands for the forthcoming cases. The explanation is already
given above the switch.
/* Currently, WM8580 driver doesn't support PLL-out rates
 * other than those mentioned in Table-52 Page-58 of WM8580A
 * manual. Some of these unavailable rates include
 * {8000x256, 64000x256, 11025x256}Hz.
 * As a wayout, we rather change rfs to a value that results in
 * (params_rate(params) * rfs), and itself, acceptable to the both
 * the CODEC and the CPU.
 */
The rates 64000, 8000 and 11025 are thus 'exceptions'.

Infact, these were the real cases that I pointed during
MCLK/BCLK-RATIO discussions.
Currently, other cases work because 256fs RCLK is selected by default
and these three don't
because there is no way to indicate it to the CODEC driver.

>> +/* SMDK64xx Playback widgets */
>> +static const struct snd_soc_dapm_widget wm8580_dapm_widgets_pbk[] = {
>> +     SND_SOC_DAPM_HP("Front-L/R", NULL),
>> +     SND_SOC_DAPM_HP("Center/Sub", NULL),
>> +     SND_SOC_DAPM_HP("Rear-L/R", NULL),
>> +};
>
> All my previous comments about these and the way they're being
> configured still apply.
Once 6channel support is up, i will submit a patch to implement audio
routing options
using snd_soc_add_controls. Whereby user can set audio route on the device.
For now, as we have only 2channels enabled, i decided to enable only Front-L/R
and keep others disabled. Of the Mic and LineIn, we enable LineIn by
default. How is that?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[Mark Brown]

On Thu, Sep 17, 2009 at 08:35:56PM +0900, jassi brar wrote:

> This note means stands for the forthcoming cases. The explanation is already
> given above the switch.

> /* Currently, WM8580 driver doesn't support PLL-out rates
>  * other than those mentioned in Table-52 Page-58 of WM8580A
>  * manual. Some of these unavailable rates include
>  * {8000x256, 64000x256, 11025x256}Hz.

Which version of the datasheet are you looking at?  Table 52 in the
current version 4.7 is (as it says) a list of example configurations,
not an exhaustive list of supported configurations.  Anything within the
operating characteristics of the PLL is supported, set_pll() should
enforce this where required.

> > All my previous comments about these and the way they're being
> > configured still apply.

> Once 6channel support is up, i will submit a patch to implement audio
> routing options
> using snd_soc_add_controls. Whereby user can set audio route on the device.
> For now, as we have only 2channels enabled, i decided to enable only Front-L/R
> and keep others disabled. Of the Mic and LineIn, we enable LineIn by
> default. How is that?

Please just remove this for now or at most enable everything by default,
like I say it's not buying anything except confusion as it stands.

If it's not present then the machine driver will immediately work when
six channel support is added.

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[jassi brar]

On Thu, Sep 17, 2009 at 9:03 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Sep 17, 2009 at 08:35:56PM +0900, jassi brar wrote:
>
>> This note means stands for the forthcoming cases. The explanation is already
>> given above the switch.
>
>> /* Currently, WM8580 driver doesn't support PLL-out rates
>>  * other than those mentioned in Table-52 Page-58 of WM8580A
>>  * manual. Some of these unavailable rates include
>>  * {8000x256, 64000x256, 11025x256}Hz.
>
> Which version of the datasheet are you looking at?  Table 52 in the
> current version 4.7 is (as it says) a list of example configurations,
> not an exhaustive list of supported configurations.  Anything within the
> operating characteristics of the PLL is supported, set_pll() should
> enforce this where required.
That very manual. And i don't say the WM8580 doesn't support, I said
the WM8580 driver doesn't support: which can be verified looking at the
CODEC driver.
Theoretically all output clocks are possible but usually the PLL coefficients
have limits on their value and thus final output. WM8580 driver too seems
to enforce that.
Though, you wud know better of WM8580.

>> > All my previous comments about these and the way they're being
>> > configured still apply.
>
>> Once 6channel support is up, i will submit a patch to implement audio
>> routing options
>> using snd_soc_add_controls. Whereby user can set audio route on the device.
>> For now, as we have only 2channels enabled, i decided to enable only Front-L/R
>> and keep others disabled. Of the Mic and LineIn, we enable LineIn by
>> default. How is that?
>
> Please just remove this for now or at most enable everything by default,
Will enable all and submit again.
Thanks.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[Mark Brown]

On Thu, Sep 17, 2009 at 09:26:28PM +0900, jassi brar wrote:

> > > +       /* Currently, WM8580 driver doesn't support PLL-out rates
> > > +        * other than those mentioned in Table-52 Page-58 of WM8580A

> That very manual. And i don't say the WM8580 doesn't support, I said
> the WM8580 driver doesn't support: which can be verified looking at the
> CODEC driver.

Your comment says that only the output frequencies in table 52 are
supported.  Could you please provide more specific references to where
this is done in the driver?  I think you're confusing the fact that the
example table lists most of the common audio frequencies with what the
driver supports here.

> Theoretically all output clocks are possible but usually the PLL coefficients
> have limits on their value and thus final output. WM8580 driver too seems
> to enforce that.
> Though, you wud know better of WM8580.

So what you're actually saying is that 256fs doesn't give us the option
of an an in-range Fvco for the FLL at those frequencies (which is the
check I think you're talking about here)?  That's a limitation of the
chip.

Certainly, the comment is not accurate - the contents of table 52 aren't
relevant here, the driver is calculating what it can do dynamically so
the restrictions are a combination of the limits on Fvco and the pre and
post scaling dividers available.

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[jassi brar]

On Thu, Sep 17, 2009 at 10:11 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Sep 17, 2009 at 09:26:28PM +0900, jassi brar wrote:
>
>> > > +       /* Currently, WM8580 driver doesn't support PLL-out rates
>> > > +        * other than those mentioned in Table-52 Page-58 of WM8580A
>
>> That very manual. And i don't say the WM8580 doesn't support, I said
>> the WM8580 driver doesn't support: which can be verified looking at the
>> CODEC driver.
>
> Your comment says that only the output frequencies in table 52 are
> supported.  Could you please provide more specific references to where
> this is done in the driver?  I think you're confusing the fact that the
> example table lists most of the common audio frequencies with what the
> driver supports here.
Let me be precise.
With this machine driver and the WM8580 CODEC driver from origin/for-2.6.32
if we do any of the following:-
   snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 12000000, 8000*256);
   snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 12000000, 11025*256);
   snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 12000000, 64000*256);
we get the error: "wm8580: Unable to scale output frequency"

>> Theoretically all output clocks are possible but usually the PLL coefficients
>> have limits on their value and thus final output. WM8580 driver too seems
>> to enforce that.
>> Though, you wud know better of WM8580.
>
> So what you're actually saying is that 256fs doesn't give us the option
> of an an in-range Fvco for the FLL at those frequencies (which is the
> check I think you're talking about here)?  That's a limitation of the
> chip.
> Certainly, the comment is not accurate - the contents of table 52 aren't
> relevant here, the driver is calculating what it can do dynamically so
> the restrictions are a combination of the limits on Fvco and the pre and
> post scaling dividers available.
Of course, there just might be a set of values for PLL coeffs that output
256fs for 8000, 11025 and 64000. If that set is within acceptable range of
of the chip then it's just a matter of driver upgrade otherwise its a
limitation
of the chip.
You wud know which case is it.

I apologize if my inappropriate selection of expression bothered you.
In future, please feel free to suggest modification of any comment in the
code. Regards.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] S3C64XX I2S: Added machine driver for WM8580

[Mark Brown]

On Thu, Sep 17, 2009 at 10:36:10PM +0900, jassi brar wrote:
> On Thu, Sep 17, 2009 at 10:11 PM, Mark Brown

> > Your comment says that only the output frequencies in table 52 are
> > supported.  Could you please provide more specific references to where
> > this is done in the driver?  I think you're confusing the fact that the
> > example table lists most of the common audio frequencies with what the
> > driver supports here.

> Let me be precise.
> With this machine driver and the WM8580 CODEC driver from origin/for-2.6.32
> if we do any of the following:-
>    snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 12000000, 8000*256);
>    snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 12000000, 11025*256);
>    snd_soc_dai_set_pll(codec_dai, WM8580_PLLA, 12000000, 64000*256);
> we get the error: "wm8580: Unable to scale output frequency"

Right, this is nothing to do with the list of example configurations and
everything to do with the fact that those options are just not supported
by the hardware.  For example, 256fs for 8kHz is only 2.048MHz and even
the largest postscale divider of 24 can't get anywhere near that from
90MHz which is the minimum Fvco for the PLL.

> I apologize if my inappropriate selection of expression bothered you.
> In future, please feel free to suggest modification of any comment in the
> code. Regards.

Part of the issue here is that my review is in part based on the
comments.  If the code (including comments) seems obviously incorrect
from reading then that's an issue in itself.