[alsa-devel] [PATCH 01/20] ASoC: S3C: I2Sv2: Add missing semicolon
Add missing semicolon after s3c2412_i2s_delay
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index aa84f4c..8f08508 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -753,7 +753,7 @@ int s3c_i2sv2_register_dai(struct snd_soc_dai *dai)
/* Allow overriding by (for example) IISv4 */ if (!ops->delay) - ops->delay = s3c2412_i2s_delay, + ops->delay = s3c2412_i2s_delay;
dai->suspend = s3c2412_i2s_suspend; dai->resume = s3c2412_i2s_resume;
Towards generalizing CPU driver interface, do not accept direct field values for the BCLK and RCLK. The machine driver should simply request the FS-multiple and not provide the value to be set in divide field of IISMOD.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.c | 68 +++++++++++++++++---------------------- 1 files changed, 30 insertions(+), 38 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index 8f08508..fd5c842 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -469,29 +469,25 @@ static int s3c2412_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai,
switch (div_id) { case S3C_I2SV2_DIV_BCLK: - if (div > 3) { - /* convert value to bit field */ - - switch (div) { - case 16: - div = S3C2412_IISMOD_BCLK_16FS; - break; + switch (div) { + case 16: + div = S3C2412_IISMOD_BCLK_16FS; + break;
- case 32: - div = S3C2412_IISMOD_BCLK_32FS; - break; + case 32: + div = S3C2412_IISMOD_BCLK_32FS; + break;
- case 24: - div = S3C2412_IISMOD_BCLK_24FS; - break; + case 24: + div = S3C2412_IISMOD_BCLK_24FS; + break;
- case 48: - div = S3C2412_IISMOD_BCLK_48FS; - break; + case 48: + div = S3C2412_IISMOD_BCLK_48FS; + break;
- default: - return -EINVAL; - } + default: + return -EINVAL; }
reg = readl(i2s->regs + S3C2412_IISMOD); @@ -502,29 +498,25 @@ static int s3c2412_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai, break;
case S3C_I2SV2_DIV_RCLK: - if (div > 3) { - /* convert value to bit field */ - - switch (div) { - case 256: - div = S3C2412_IISMOD_RCLK_256FS; - break; + switch (div) { + case 256: + div = S3C2412_IISMOD_RCLK_256FS; + break;
- case 384: - div = S3C2412_IISMOD_RCLK_384FS; - break; + case 384: + div = S3C2412_IISMOD_RCLK_384FS; + break;
- case 512: - div = S3C2412_IISMOD_RCLK_512FS; - break; + case 512: + div = S3C2412_IISMOD_RCLK_512FS; + break;
- case 768: - div = S3C2412_IISMOD_RCLK_768FS; - break; + case 768: + div = S3C2412_IISMOD_RCLK_768FS; + break;
- default: - return -EINVAL; - } + default: + return -EINVAL; }
reg = readl(i2s->regs + S3C2412_IISMOD);
For some CPU-CODEC and source clock combination we might need to set BCLK to N*Sample_size*LRCLK, where N may be even 3 or 4, not just 2.
We can simply remove the dependency of BCLK on sample size as there is already a callback(S3C_I2SV2_DIV_BCLK) available to set required BCLK.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index fd5c842..b01f50e 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -364,19 +364,16 @@ static int s3c2412_i2s_hw_params(struct snd_pcm_substream *substream, #endif
#ifdef CONFIG_PLAT_S3C64XX - iismod &= ~(S3C64XX_IISMOD_BLC_MASK | S3C2412_IISMOD_BCLK_MASK); + iismod &= ~S3C64XX_IISMOD_BLC_MASK; /* Sample size */ switch (params_format(params)) { case SNDRV_PCM_FORMAT_S8: - /* 8 bit sample, 16fs BCLK */ - iismod |= (S3C64XX_IISMOD_BLC_8BIT | S3C2412_IISMOD_BCLK_16FS); + iismod |= S3C64XX_IISMOD_BLC_8BIT; break; case SNDRV_PCM_FORMAT_S16_LE: - /* 16 bit sample, 32fs BCLK */ break; case SNDRV_PCM_FORMAT_S24_LE: - /* 24 bit sample, 48fs BCLK */ - iismod |= (S3C64XX_IISMOD_BLC_24BIT | S3C2412_IISMOD_BCLK_48FS); + iismod |= S3C64XX_IISMOD_BLC_24BIT; break; } #endif
The code in s3c-i2s-v2.c is compiled via Makefile and not implicitly by include'ing. Also, the driver developer anyways has to refer to the manual to see if the code can be reused for the SoC under consideration. That makes the S3C_IIS_V2_SUPPORTED retrospective rather than a check.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.c | 14 -------------- 1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index b01f50e..2812b41 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -37,20 +37,6 @@ #include "s3c-i2s-v2.h" #include "s3c-dma.h"
-#undef S3C_IIS_V2_SUPPORTED - -#if defined(CONFIG_CPU_S3C2412) || defined(CONFIG_CPU_S3C2413) -#define S3C_IIS_V2_SUPPORTED -#endif - -#ifdef CONFIG_PLAT_S3C64XX -#define S3C_IIS_V2_SUPPORTED -#endif - -#ifndef S3C_IIS_V2_SUPPORTED -#error Unsupported CPU model -#endif - #define S3C2412_I2S_DEBUG_CON 0
static inline struct s3c_i2sv2_info *to_info(struct snd_soc_dai *cpu_dai)
Rather than having the multiple definitions of the same clocks, define them in one common place and refer by SoC specific names.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.h | 4 ++++ sound/soc/s3c24xx/s3c2412-i2s.h | 4 ++-- sound/soc/s3c24xx/s3c64xx-i2s.h | 6 +++--- 3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.h b/sound/soc/s3c24xx/s3c-i2s-v2.h index ecf8eaa..b094d3c 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.h +++ b/sound/soc/s3c24xx/s3c-i2s-v2.h @@ -25,6 +25,10 @@ #define S3C_I2SV2_DIV_RCLK (2) #define S3C_I2SV2_DIV_PRESCALER (3)
+#define S3C_I2SV2_CLKSRC_PCLK 0 +#define S3C_I2SV2_CLKSRC_AUDIOBUS 1 +#define S3C_I2SV2_CLKSRC_CDCLK 2 + /** * struct s3c_i2sv2_info - S3C I2S-V2 information * @dev: The parent device passed to use from the probe. diff --git a/sound/soc/s3c24xx/s3c2412-i2s.h b/sound/soc/s3c24xx/s3c2412-i2s.h index 92848e5..60cac00 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.h +++ b/sound/soc/s3c24xx/s3c2412-i2s.h @@ -21,8 +21,8 @@ #define S3C2412_DIV_RCLK S3C_I2SV2_DIV_RCLK #define S3C2412_DIV_PRESCALER S3C_I2SV2_DIV_PRESCALER
-#define S3C2412_CLKSRC_PCLK (0) -#define S3C2412_CLKSRC_I2SCLK (1) +#define S3C2412_CLKSRC_PCLK S3C_I2SV2_CLKSRC_PCLK +#define S3C2412_CLKSRC_I2SCLK S3C_I2SV2_CLKSRC_AUDIOBUS
extern struct clk *s3c2412_get_iisclk(void);
diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.h b/sound/soc/s3c24xx/s3c64xx-i2s.h index abe7253..e350d28 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.h +++ b/sound/soc/s3c24xx/s3c64xx-i2s.h @@ -23,9 +23,9 @@ struct clk; #define S3C64XX_DIV_RCLK S3C_I2SV2_DIV_RCLK #define S3C64XX_DIV_PRESCALER S3C_I2SV2_DIV_PRESCALER
-#define S3C64XX_CLKSRC_PCLK (0) -#define S3C64XX_CLKSRC_MUX (1) -#define S3C64XX_CLKSRC_CDCLK (2) +#define S3C64XX_CLKSRC_PCLK S3C_I2SV2_CLKSRC_PCLK +#define S3C64XX_CLKSRC_MUX S3C_I2SV2_CLKSRC_AUDIOBUS +#define S3C64XX_CLKSRC_CDCLK S3C_I2SV2_CLKSRC_CDCLK
extern struct snd_soc_dai s3c64xx_i2s_dai[];
s3c-i2s-v2 remove unnecessary headers
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index 2812b41..170b3c1 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -16,18 +16,12 @@ * option) any later version. */
-#include <linux/init.h> -#include <linux/module.h> -#include <linux/device.h> #include <linux/delay.h> #include <linux/clk.h> -#include <linux/kernel.h> #include <linux/io.h>
-#include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> -#include <sound/initval.h> #include <sound/soc.h>
#include <plat/regs-s3c2412-iis.h>
s3c64xx-i2s remove unncessary headers
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c64xx-i2s.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c index 93ed3aa..39563ed 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.c +++ b/sound/soc/s3c24xx/s3c64xx-i2s.c @@ -12,9 +12,6 @@ * published by the Free Software Foundation. */
-#include <linux/init.h> -#include <linux/module.h> -#include <linux/device.h> #include <linux/clk.h> #include <linux/gpio.h> #include <linux/io.h>
In order for the RATE and FMT defines to be reuseable in future by the i2sv4 driver, move the MACROs out to the header file.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c64xx-i2s.c | 9 --------- sound/soc/s3c24xx/s3c64xx-i2s.h | 9 +++++++++ 2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c index 39563ed..6552894 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.c +++ b/sound/soc/s3c24xx/s3c64xx-i2s.c @@ -127,15 +127,6 @@ static int s3c64xx_i2s_probe(struct platform_device *pdev, }
-#define S3C64XX_I2S_RATES \ - (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 | SNDRV_PCM_RATE_16000 | \ - SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \ - SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000) - -#define S3C64XX_I2S_FMTS \ - (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE |\ - SNDRV_PCM_FMTBIT_S24_LE) - static struct snd_soc_dai_ops s3c64xx_i2s_dai_ops = { .set_sysclk = s3c64xx_i2s_set_sysclk, }; diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.h b/sound/soc/s3c24xx/s3c64xx-i2s.h index e350d28..53d2a0a 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.h +++ b/sound/soc/s3c24xx/s3c64xx-i2s.h @@ -27,6 +27,15 @@ struct clk; #define S3C64XX_CLKSRC_MUX S3C_I2SV2_CLKSRC_AUDIOBUS #define S3C64XX_CLKSRC_CDCLK S3C_I2SV2_CLKSRC_CDCLK
+#define S3C64XX_I2S_RATES \ + (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 | SNDRV_PCM_RATE_16000 | \ + SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \ + SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000) + +#define S3C64XX_I2S_FMTS \ + (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE |\ + SNDRV_PCM_FMTBIT_S24_LE) + extern struct snd_soc_dai s3c64xx_i2s_dai[];
extern struct clk *s3c64xx_i2s_get_clock(struct snd_soc_dai *dai);
Towards having build for multiple SoCs segregate hw_params callback for s3c2412 and s3c64xx. Since, all new SoCs have s3c64xx like register map, we keep that as default implementation if no SoC specific callback is already defined.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.c | 18 +++--------------- sound/soc/s3c24xx/s3c2412-i2s.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index 170b3c1..0d655ab 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -312,7 +312,7 @@ static int s3c2412_i2s_set_fmt(struct snd_soc_dai *cpu_dai, return 0; }
-static int s3c2412_i2s_hw_params(struct snd_pcm_substream *substream, +static int s3c_i2sv2_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *socdai) { @@ -332,18 +332,6 @@ static int s3c2412_i2s_hw_params(struct snd_pcm_substream *substream, iismod = readl(i2s->regs + S3C2412_IISMOD); pr_debug("%s: r: IISMOD: %x\n", __func__, iismod);
-#if defined(CONFIG_CPU_S3C2412) || defined(CONFIG_CPU_S3C2413) - switch (params_format(params)) { - case SNDRV_PCM_FORMAT_S8: - iismod |= S3C2412_IISMOD_8BIT; - break; - case SNDRV_PCM_FORMAT_S16_LE: - iismod &= ~S3C2412_IISMOD_8BIT; - break; - } -#endif - -#ifdef CONFIG_PLAT_S3C64XX iismod &= ~S3C64XX_IISMOD_BLC_MASK; /* Sample size */ switch (params_format(params)) { @@ -356,7 +344,6 @@ static int s3c2412_i2s_hw_params(struct snd_pcm_substream *substream, iismod |= S3C64XX_IISMOD_BLC_24BIT; break; } -#endif
writel(iismod, i2s->regs + S3C2412_IISMOD); pr_debug("%s: w: IISMOD: %x\n", __func__, iismod); @@ -716,7 +703,8 @@ int s3c_i2sv2_register_dai(struct snd_soc_dai *dai) struct snd_soc_dai_ops *ops = dai->ops;
ops->trigger = s3c2412_i2s_trigger; - ops->hw_params = s3c2412_i2s_hw_params; + if (!ops->hw_params) + ops->hw_params = s3c_i2sv2_hw_params; ops->set_fmt = s3c2412_i2s_set_fmt; ops->set_clkdiv = s3c2412_i2s_set_clkdiv;
diff --git a/sound/soc/s3c24xx/s3c2412-i2s.c b/sound/soc/s3c24xx/s3c2412-i2s.c index 359e593..a5b21f6 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.c +++ b/sound/soc/s3c24xx/s3c2412-i2s.c @@ -103,6 +103,10 @@ struct clk *s3c2412_get_iisclk(void) } EXPORT_SYMBOL_GPL(s3c2412_get_iisclk);
+static inline struct s3c_i2sv2_info *to_info(struct snd_soc_dai *cpu_dai) +{ + return cpu_dai->private_data; +}
static int s3c2412_i2s_probe(struct platform_device *pdev, struct snd_soc_dai *dai) @@ -142,6 +146,38 @@ static int s3c2412_i2s_probe(struct platform_device *pdev, return 0; }
+static int s3c2412_i2s_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *cpu_dai) +{ + struct s3c_i2sv2_info *i2s = to_info(cpu_dai); + u32 iismod; + + pr_debug("Entered %s\n", __func__); + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + cpu_dai->dma_data = i2s->dma_playback; + else + cpu_dai->dma_data = i2s->dma_capture; + + iismod = readl(i2s->regs + S3C2412_IISMOD); + pr_debug("%s: r: IISMOD: %x\n", __func__, iismod); + + switch (params_format(params)) { + case SNDRV_PCM_FORMAT_S8: + iismod |= S3C2412_IISMOD_8BIT; + break; + case SNDRV_PCM_FORMAT_S16_LE: + iismod &= ~S3C2412_IISMOD_8BIT; + break; + } + + writel(iismod, i2s->regs + S3C2412_IISMOD); + pr_debug("%s: w: IISMOD: %x\n", __func__, iismod); + + return 0; +} + #define S3C2412_I2S_RATES \ (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 | SNDRV_PCM_RATE_16000 | \ SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \ @@ -149,6 +185,7 @@ static int s3c2412_i2s_probe(struct platform_device *pdev,
static struct snd_soc_dai_ops s3c2412_i2s_dai_ops = { .set_sysclk = s3c2412_i2s_set_sysclk, + .hw_params = s3c2412_i2s_hw_params, };
struct snd_soc_dai s3c2412_i2s_dai = {
The header for I2Sv2 (linux/arch/arm/plat-s3c/include/plat/regs-s3c2412-iis.h) contains only controller specific definitions and nothing SoC specific. So, it could be moved to sound/soc/s3c24xx/ That will also help avoid delays in accepting patches that touch ASOC and ARCH parts.
For now, we simply copy to regs-i2s-v2.h without any material changes, and render the regs-s3c2412-iis.h useless in the kernel. A separate patch will be submitted to S3C ARCH tree to remove the regs-s3c2412-iis.h
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/regs-i2s-v2.h | 79 +++++++++++++++++++++++++++++++++++++++ sound/soc/s3c24xx/s3c-i2s-v2.c | 3 +- sound/soc/s3c24xx/s3c2412-i2s.c | 3 +- sound/soc/s3c24xx/s3c64xx-i2s.c | 2 +- 4 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 sound/soc/s3c24xx/regs-i2s-v2.h
diff --git a/sound/soc/s3c24xx/regs-i2s-v2.h b/sound/soc/s3c24xx/regs-i2s-v2.h new file mode 100644 index 0000000..8e1a585 --- /dev/null +++ b/sound/soc/s3c24xx/regs-i2s-v2.h @@ -0,0 +1,79 @@ +/* linux/sound/soc/s3c24xx/regs-i2s-v2.h + * + * Copyright 2007 Simtec Electronics linux@simtec.co.uk + * http://armlinux.simtec.co.uk/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Register definitions for controllers that share the I2Sv2 part +*/ + +#ifndef __REGS_IISV2_H +#define __REGS_IISV2_H + +#define S3C2412_IISCON (0x00) +#define S3C2412_IISMOD (0x04) +#define S3C2412_IISFIC (0x08) +#define S3C2412_IISPSR (0x0C) +#define S3C2412_IISTXD (0x10) +#define S3C2412_IISRXD (0x14) + +#define S3C2412_IISCON_LRINDEX (1 << 11) +#define S3C2412_IISCON_TXFIFO_EMPTY (1 << 10) +#define S3C2412_IISCON_RXFIFO_EMPTY (1 << 9) +#define S3C2412_IISCON_TXFIFO_FULL (1 << 8) +#define S3C2412_IISCON_RXFIFO_FULL (1 << 7) +#define S3C2412_IISCON_TXDMA_PAUSE (1 << 6) +#define S3C2412_IISCON_RXDMA_PAUSE (1 << 5) +#define S3C2412_IISCON_TXCH_PAUSE (1 << 4) +#define S3C2412_IISCON_RXCH_PAUSE (1 << 3) +#define S3C2412_IISCON_TXDMA_ACTIVE (1 << 2) +#define S3C2412_IISCON_RXDMA_ACTIVE (1 << 1) +#define S3C2412_IISCON_IIS_ACTIVE (1 << 0) + +#define S3C64XX_IISMOD_BLC_16BIT (0 << 13) +#define S3C64XX_IISMOD_BLC_8BIT (1 << 13) +#define S3C64XX_IISMOD_BLC_24BIT (2 << 13) +#define S3C64XX_IISMOD_BLC_MASK (3 << 13) + +#define S3C64XX_IISMOD_IMS_PCLK (0 << 10) +#define S3C64XX_IISMOD_IMS_SYSMUX (1 << 10) + +#define S3C2412_IISMOD_MASTER_INTERNAL (0 << 10) +#define S3C2412_IISMOD_MASTER_EXTERNAL (1 << 10) +#define S3C2412_IISMOD_SLAVE (2 << 10) +#define S3C2412_IISMOD_MASTER_MASK (3 << 10) +#define S3C2412_IISMOD_MODE_TXONLY (0 << 8) +#define S3C2412_IISMOD_MODE_RXONLY (1 << 8) +#define S3C2412_IISMOD_MODE_TXRX (2 << 8) +#define S3C2412_IISMOD_MODE_MASK (3 << 8) +#define S3C2412_IISMOD_LR_LLOW (0 << 7) +#define S3C2412_IISMOD_LR_RLOW (1 << 7) +#define S3C2412_IISMOD_SDF_IIS (0 << 5) +#define S3C2412_IISMOD_SDF_MSB (1 << 5) +#define S3C2412_IISMOD_SDF_LSB (2 << 5) +#define S3C2412_IISMOD_SDF_MASK (3 << 5) +#define S3C2412_IISMOD_RCLK_256FS (0 << 3) +#define S3C2412_IISMOD_RCLK_512FS (1 << 3) +#define S3C2412_IISMOD_RCLK_384FS (2 << 3) +#define S3C2412_IISMOD_RCLK_768FS (3 << 3) +#define S3C2412_IISMOD_RCLK_MASK (3 << 3) +#define S3C2412_IISMOD_BCLK_32FS (0 << 1) +#define S3C2412_IISMOD_BCLK_48FS (1 << 1) +#define S3C2412_IISMOD_BCLK_16FS (2 << 1) +#define S3C2412_IISMOD_BCLK_24FS (3 << 1) +#define S3C2412_IISMOD_BCLK_MASK (3 << 1) +#define S3C2412_IISMOD_8BIT (1 << 0) + +#define S3C64XX_IISMOD_CDCLKCON (1 << 12) + +#define S3C2412_IISPSR_PSREN (1 << 15) + +#define S3C2412_IISFIC_TXFLUSH (1 << 15) +#define S3C2412_IISFIC_RXFLUSH (1 << 7) +#define S3C2412_IISFIC_TXCOUNT(x) (((x) >> 8) & 0xf) +#define S3C2412_IISFIC_RXCOUNT(x) (((x) >> 0) & 0xf) + +#endif /* __REGS_IISV2_H */ diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index 0d655ab..b690078 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -24,10 +24,9 @@ #include <sound/pcm_params.h> #include <sound/soc.h>
-#include <plat/regs-s3c2412-iis.h> - #include <mach/dma.h>
+#include "regs-i2s-v2.h" #include "s3c-i2s-v2.h" #include "s3c-dma.h"
diff --git a/sound/soc/s3c24xx/s3c2412-i2s.c b/sound/soc/s3c24xx/s3c2412-i2s.c index a5b21f6..42fb663 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.c +++ b/sound/soc/s3c24xx/s3c2412-i2s.c @@ -32,12 +32,11 @@ #include <sound/soc.h> #include <mach/hardware.h>
-#include <plat/regs-s3c2412-iis.h> - #include <mach/regs-gpio.h> #include <mach/dma.h>
#include "s3c-dma.h" +#include "regs-i2s-v2.h" #include "s3c2412-i2s.h"
#define S3C2412_I2S_DEBUG 0 diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c index 6552894..9bcc99d 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.c +++ b/sound/soc/s3c24xx/s3c64xx-i2s.c @@ -18,7 +18,6 @@
#include <sound/soc.h>
-#include <plat/regs-s3c2412-iis.h> #include <plat/gpio-bank-d.h> #include <plat/gpio-bank-e.h> #include <plat/gpio-cfg.h> @@ -27,6 +26,7 @@ #include <mach/dma.h>
#include "s3c-dma.h" +#include "regs-i2s-v2.h" #include "s3c64xx-i2s.h"
/* The value should be set to maximum of the total number
Define more bit definitions in the order of mainline support for the SoC.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/regs-i2s-v2.h | 41 +++++++++++++++++++++++++++++++++++++++ 1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/sound/soc/s3c24xx/regs-i2s-v2.h b/sound/soc/s3c24xx/regs-i2s-v2.h index 8e1a585..dea199e 100644 --- a/sound/soc/s3c24xx/regs-i2s-v2.h +++ b/sound/soc/s3c24xx/regs-i2s-v2.h @@ -20,6 +20,24 @@ #define S3C2412_IISTXD (0x10) #define S3C2412_IISRXD (0x14)
+#define S5PC1XX_IISFICS 0x18 +#define S5PC1XX_IISTXDS 0x1C + +#define S5PC1XX_IISCON_SW_RST (1 << 31) +#define S5PC1XX_IISCON_FRXOFSTATUS (1 << 26) +#define S5PC1XX_IISCON_FRXORINTEN (1 << 25) +#define S5PC1XX_IISCON_FTXSURSTAT (1 << 24) +#define S5PC1XX_IISCON_FTXSURINTEN (1 << 23) +#define S5PC1XX_IISCON_TXSDMAPAUSE (1 << 20) +#define S5PC1XX_IISCON_TXSDMACTIVE (1 << 18) + +#define S3C64XX_IISCON_FTXURSTATUS (1 << 17) +#define S3C64XX_IISCON_FTXURINTEN (1 << 16) +#define S3C64XX_IISCON_TXFIFO2_EMPTY (1 << 15) +#define S3C64XX_IISCON_TXFIFO1_EMPTY (1 << 14) +#define S3C64XX_IISCON_TXFIFO2_FULL (1 << 13) +#define S3C64XX_IISCON_TXFIFO1_FULL (1 << 12) + #define S3C2412_IISCON_LRINDEX (1 << 11) #define S3C2412_IISCON_TXFIFO_EMPTY (1 << 10) #define S3C2412_IISCON_RXFIFO_EMPTY (1 << 9) @@ -33,6 +51,23 @@ #define S3C2412_IISCON_RXDMA_ACTIVE (1 << 1) #define S3C2412_IISCON_IIS_ACTIVE (1 << 0)
+#define S5PC1XX_IISMOD_OPCLK_CDCLK_OUT (0 << 30) +#define S5PC1XX_IISMOD_OPCLK_CDCLK_IN (1 << 30) +#define S5PC1XX_IISMOD_OPCLK_BCLK_OUT (2 << 30) +#define S5PC1XX_IISMOD_OPCLK_PCLK (3 << 30) +#define S5PC1XX_IISMOD_OPCLK_MASK (3 << 30) +#define S5PC1XX_IISMOD_TXS_IDMA (1 << 28) /* Sec_TXFIFO use I-DMA */ +#define S5PC1XX_IISMOD_BLCS_MASK 0x3 +#define S5PC1XX_IISMOD_BLCS_SHIFT 26 +#define S5PC1XX_IISMOD_BLCP_MASK 0x3 +#define S5PC1XX_IISMOD_BLCP_SHIFT 24 + +#define S3C64XX_IISMOD_C2DD_HHALF (1 << 21) /* Discard Higher-half */ +#define S3C64XX_IISMOD_C2DD_LHALF (1 << 20) /* Discard Lower-half */ +#define S3C64XX_IISMOD_C1DD_HHALF (1 << 19) +#define S3C64XX_IISMOD_C1DD_LHALF (1 << 18) +#define S3C64XX_IISMOD_DC2_EN (1 << 17) +#define S3C64XX_IISMOD_DC1_EN (1 << 16) #define S3C64XX_IISMOD_BLC_16BIT (0 << 13) #define S3C64XX_IISMOD_BLC_8BIT (1 << 13) #define S3C64XX_IISMOD_BLC_24BIT (2 << 13) @@ -71,9 +106,15 @@
#define S3C2412_IISPSR_PSREN (1 << 15)
+#define S3C64XX_IISFIC_TX2COUNT(x) (((x) >> 24) & 0xf) +#define S3C64XX_IISFIC_TX1COUNT(x) (((x) >> 16) & 0xf) + #define S3C2412_IISFIC_TXFLUSH (1 << 15) #define S3C2412_IISFIC_RXFLUSH (1 << 7) #define S3C2412_IISFIC_TXCOUNT(x) (((x) >> 8) & 0xf) #define S3C2412_IISFIC_RXCOUNT(x) (((x) >> 0) & 0xf)
+#define S5PC1XX_IISFICS_TXFLUSH (1 << 15) +#define S5PC1XX_IISFICS_TXCOUNT(x) (((x) >> 8) & 0x7f) + #endif /* __REGS_IISV2_H */
The IMS field of s3c2412/13 is essentially the same as that of s3c64xx. That is, the IISMOD[11] bit decides Master/Slave mode and IISMOD[10] bit selects source clock for signal generation. For that reason, remove improper defines for IISMOD[11:10] field mask and define two 1bit fields that can be set independent of each other. As a consequence, corresponding fields for PLAT_S3C64XX too get to use these new defines.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/regs-i2s-v2.h | 6 ++---- sound/soc/s3c24xx/s3c-i2s-v2.c | 25 ++----------------------- sound/soc/s3c24xx/s3c2412-i2s.c | 8 ++------ 3 files changed, 6 insertions(+), 33 deletions(-)
diff --git a/sound/soc/s3c24xx/regs-i2s-v2.h b/sound/soc/s3c24xx/regs-i2s-v2.h index dea199e..f612468 100644 --- a/sound/soc/s3c24xx/regs-i2s-v2.h +++ b/sound/soc/s3c24xx/regs-i2s-v2.h @@ -76,10 +76,8 @@ #define S3C64XX_IISMOD_IMS_PCLK (0 << 10) #define S3C64XX_IISMOD_IMS_SYSMUX (1 << 10)
-#define S3C2412_IISMOD_MASTER_INTERNAL (0 << 10) -#define S3C2412_IISMOD_MASTER_EXTERNAL (1 << 10) -#define S3C2412_IISMOD_SLAVE (2 << 10) -#define S3C2412_IISMOD_MASTER_MASK (3 << 10) +#define S3C2412_IISMOD_IMS_SYSMUX (1 << 10) +#define S3C2412_IISMOD_SLAVE (1 << 11) #define S3C2412_IISMOD_MODE_TXONLY (0 << 8) #define S3C2412_IISMOD_MODE_RXONLY (1 << 8) #define S3C2412_IISMOD_MODE_TXRX (2 << 8) diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index b690078..1b29b4b 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -251,35 +251,14 @@ static int s3c2412_i2s_set_fmt(struct snd_soc_dai *cpu_dai, iismod = readl(i2s->regs + S3C2412_IISMOD); pr_debug("hw_params r: IISMOD: %x \n", iismod);
-#if defined(CONFIG_CPU_S3C2412) || defined(CONFIG_CPU_S3C2413) -#define IISMOD_MASTER_MASK S3C2412_IISMOD_MASTER_MASK -#define IISMOD_SLAVE S3C2412_IISMOD_SLAVE -#define IISMOD_MASTER S3C2412_IISMOD_MASTER_INTERNAL -#endif - -#if defined(CONFIG_PLAT_S3C64XX) -/* From Rev1.1 datasheet, we have two master and two slave modes: - * IMS[11:10]: - * 00 = master mode, fed from PCLK - * 01 = master mode, fed from CLKAUDIO - * 10 = slave mode, using PCLK - * 11 = slave mode, using I2SCLK - */ -#define IISMOD_MASTER_MASK (1 << 11) -#define IISMOD_SLAVE (1 << 11) -#define IISMOD_MASTER (0 << 11) -#endif - switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBM_CFM: i2s->master = 0; - iismod &= ~IISMOD_MASTER_MASK; - iismod |= IISMOD_SLAVE; + iismod |= S3C2412_IISMOD_SLAVE; break; case SND_SOC_DAIFMT_CBS_CFS: i2s->master = 1; - iismod &= ~IISMOD_MASTER_MASK; - iismod |= IISMOD_MASTER; + iismod &= ~S3C2412_IISMOD_SLAVE; break; default: pr_err("unknwon master/slave format\n"); diff --git a/sound/soc/s3c24xx/s3c2412-i2s.c b/sound/soc/s3c24xx/s3c2412-i2s.c index 42fb663..e6871f7 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.c +++ b/sound/soc/s3c24xx/s3c2412-i2s.c @@ -78,14 +78,10 @@ static int s3c2412_i2s_set_sysclk(struct snd_soc_dai *cpu_dai,
switch (clk_id) { case S3C2412_CLKSRC_PCLK: - s3c2412_i2s.master = 1; - iismod &= ~S3C2412_IISMOD_MASTER_MASK; - iismod |= S3C2412_IISMOD_MASTER_INTERNAL; + iismod &= ~S3C2412_IISMOD_IMS_SYSMUX; break; case S3C2412_CLKSRC_I2SCLK: - s3c2412_i2s.master = 0; - iismod &= ~S3C2412_IISMOD_MASTER_MASK; - iismod |= S3C2412_IISMOD_MASTER_EXTERNAL; + iismod |= S3C2412_IISMOD_IMS_SYSMUX; break; default: return -EINVAL;
Until now, s3c2412_get_iisclk would return NULL since iis_clk was never initialized. Return appropriate pointer as per the selection made for source clock.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c2412-i2s.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c2412-i2s.c b/sound/soc/s3c24xx/s3c2412-i2s.c index e6871f7..4996843 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.c +++ b/sound/soc/s3c24xx/s3c2412-i2s.c @@ -94,7 +94,13 @@ static int s3c2412_i2s_set_sysclk(struct snd_soc_dai *cpu_dai,
struct clk *s3c2412_get_iisclk(void) { - return s3c2412_i2s.iis_clk; + struct s3c_i2sv2_info *i2s = &s3c2412_i2s; + u32 iismod = readl(i2s->regs + S3C2412_IISMOD); + + if (iismod & S3C2412_IISMOD_IMS_SYSMUX) + return i2s->iis_cclk; + else + return i2s->iis_pclk; } EXPORT_SYMBOL_GPL(s3c2412_get_iisclk);
No need to keep redundant field iis_clk in s3c_i2sv2_info. iis_cclk and iis_pclk is all we need.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.h | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.h b/sound/soc/s3c24xx/s3c-i2s-v2.h index b094d3c..ea56467 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.h +++ b/sound/soc/s3c24xx/s3c-i2s-v2.h @@ -49,7 +49,6 @@ struct s3c_i2sv2_info {
struct clk *iis_pclk; struct clk *iis_cclk; - struct clk *iis_clk;
unsigned char master;
Now that we have two callbacks s3c2412_i2s_get_clock & s3c64xx_i2s_get_clock doing exactly the same thing, we can define one generic s3c_i2sv2_get_clock and discard other two copies. Also, switch the users to make calls to the newly defined and generic s3c_i2sv2_get_clock
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/jive_wm8750.c | 2 +- sound/soc/s3c24xx/s3c-i2s-v2.c | 12 ++++++++++++ sound/soc/s3c24xx/s3c-i2s-v2.h | 2 ++ sound/soc/s3c24xx/s3c2412-i2s.c | 13 ------------- sound/soc/s3c24xx/s3c2412-i2s.h | 2 -- sound/soc/s3c24xx/s3c64xx-i2s.c | 12 ------------ sound/soc/s3c24xx/s3c64xx-i2s.h | 2 -- 7 files changed, 15 insertions(+), 30 deletions(-)
diff --git a/sound/soc/s3c24xx/jive_wm8750.c b/sound/soc/s3c24xx/jive_wm8750.c index 59dc2c6..304240d 100644 --- a/sound/soc/s3c24xx/jive_wm8750.c +++ b/sound/soc/s3c24xx/jive_wm8750.c @@ -70,7 +70,7 @@ static int jive_hw_params(struct snd_pcm_substream *substream, }
s3c_i2sv2_iis_calc_rate(&div, NULL, params_rate(params), - s3c2412_get_iisclk()); + s3c_i2sv2_get_clock(cpu_dai));
/* set codec DAI configuration */ ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S | diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index 1b29b4b..ecb3054 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -499,6 +499,18 @@ static snd_pcm_sframes_t s3c2412_i2s_delay(struct snd_pcm_substream *substream, return delay; }
+struct clk *s3c_i2sv2_get_clock(struct snd_soc_dai *cpu_dai) +{ + struct s3c_i2sv2_info *i2s = to_info(cpu_dai); + u32 iismod = readl(i2s->regs + S3C2412_IISMOD); + + if (iismod & S3C2412_IISMOD_IMS_SYSMUX) + return i2s->iis_cclk; + else + return i2s->iis_pclk; +} +EXPORT_SYMBOL_GPL(s3c_i2sv2_get_clock); + /* default table of all avaialable root fs divisors */ static unsigned int iis_fs_tab[] = { 256, 512, 384, 768 };
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.h b/sound/soc/s3c24xx/s3c-i2s-v2.h index ea56467..39a6db6 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.h +++ b/sound/soc/s3c24xx/s3c-i2s-v2.h @@ -60,6 +60,8 @@ struct s3c_i2sv2_info { u32 suspend_iispsr; };
+extern struct clk *s3c_i2sv2_get_clock(struct snd_soc_dai *cpu_dai); + struct s3c_i2sv2_rate_calc { unsigned int clk_div; /* for prescaler */ unsigned int fs_div; /* for root frame clock */ diff --git a/sound/soc/s3c24xx/s3c2412-i2s.c b/sound/soc/s3c24xx/s3c2412-i2s.c index 4996843..fe90867 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.c +++ b/sound/soc/s3c24xx/s3c2412-i2s.c @@ -91,19 +91,6 @@ static int s3c2412_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, return 0; }
- -struct clk *s3c2412_get_iisclk(void) -{ - struct s3c_i2sv2_info *i2s = &s3c2412_i2s; - u32 iismod = readl(i2s->regs + S3C2412_IISMOD); - - if (iismod & S3C2412_IISMOD_IMS_SYSMUX) - return i2s->iis_cclk; - else - return i2s->iis_pclk; -} -EXPORT_SYMBOL_GPL(s3c2412_get_iisclk); - static inline struct s3c_i2sv2_info *to_info(struct snd_soc_dai *cpu_dai) { return cpu_dai->private_data; diff --git a/sound/soc/s3c24xx/s3c2412-i2s.h b/sound/soc/s3c24xx/s3c2412-i2s.h index 60cac00..0b5686b 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.h +++ b/sound/soc/s3c24xx/s3c2412-i2s.h @@ -24,8 +24,6 @@ #define S3C2412_CLKSRC_PCLK S3C_I2SV2_CLKSRC_PCLK #define S3C2412_CLKSRC_I2SCLK S3C_I2SV2_CLKSRC_AUDIOBUS
-extern struct clk *s3c2412_get_iisclk(void); - extern struct snd_soc_dai s3c2412_i2s_dai;
#endif /* __SND_SOC_S3C24XX_S3C2412_I2S_H */ diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c index 9bcc99d..9343349 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.c +++ b/sound/soc/s3c24xx/s3c64xx-i2s.c @@ -91,18 +91,6 @@ static int s3c64xx_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, return 0; }
-struct clk *s3c64xx_i2s_get_clock(struct snd_soc_dai *dai) -{ - struct s3c_i2sv2_info *i2s = to_info(dai); - u32 iismod = readl(i2s->regs + S3C2412_IISMOD); - - if (iismod & S3C64XX_IISMOD_IMS_SYSMUX) - return i2s->iis_cclk; - else - return i2s->iis_pclk; -} -EXPORT_SYMBOL_GPL(s3c64xx_i2s_get_clock); - static int s3c64xx_i2s_probe(struct platform_device *pdev, struct snd_soc_dai *dai) { diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.h b/sound/soc/s3c24xx/s3c64xx-i2s.h index 53d2a0a..f27ed50 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.h +++ b/sound/soc/s3c24xx/s3c64xx-i2s.h @@ -38,6 +38,4 @@ struct clk;
extern struct snd_soc_dai s3c64xx_i2s_dai[];
-extern struct clk *s3c64xx_i2s_get_clock(struct snd_soc_dai *dai); - #endif /* __SND_SOC_S3C24XX_S3C64XX_I2S_H */
Now that the fields are defined for s3c2412, use them and avoid having multiple copies of same defines.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/regs-i2s-v2.h | 3 --- sound/soc/s3c24xx/s3c64xx-i2s.c | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/soc/s3c24xx/regs-i2s-v2.h b/sound/soc/s3c24xx/regs-i2s-v2.h index f612468..ad06937 100644 --- a/sound/soc/s3c24xx/regs-i2s-v2.h +++ b/sound/soc/s3c24xx/regs-i2s-v2.h @@ -73,9 +73,6 @@ #define S3C64XX_IISMOD_BLC_24BIT (2 << 13) #define S3C64XX_IISMOD_BLC_MASK (3 << 13)
-#define S3C64XX_IISMOD_IMS_PCLK (0 << 10) -#define S3C64XX_IISMOD_IMS_SYSMUX (1 << 10) - #define S3C2412_IISMOD_IMS_SYSMUX (1 << 10) #define S3C2412_IISMOD_SLAVE (1 << 11) #define S3C2412_IISMOD_MODE_TXONLY (0 << 8) diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c index 9343349..1a29564 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.c +++ b/sound/soc/s3c24xx/s3c64xx-i2s.c @@ -62,11 +62,11 @@ static int s3c64xx_i2s_set_sysclk(struct snd_soc_dai *cpu_dai,
switch (clk_id) { case S3C64XX_CLKSRC_PCLK: - iismod &= ~S3C64XX_IISMOD_IMS_SYSMUX; + iismod &= ~S3C2412_IISMOD_IMS_SYSMUX; break;
case S3C64XX_CLKSRC_MUX: - iismod |= S3C64XX_IISMOD_IMS_SYSMUX; + iismod |= S3C2412_IISMOD_IMS_SYSMUX; break;
case S3C64XX_CLKSRC_CDCLK:
In order to make s3c-i2s-v2.c manage controllers with minor quirks and variation in features, we define a per-block flag that indicates the availability/lack of a particular feature to the s3c-i2s-v2.c
While adding support for new SoCs' I2S, check for the blocks of older SoCs that have similar feature and set the flag for that feature.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.h | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.h b/sound/soc/s3c24xx/s3c-i2s-v2.h index 39a6db6..766f43a 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.h +++ b/sound/soc/s3c24xx/s3c-i2s-v2.h @@ -29,10 +29,16 @@ #define S3C_I2SV2_CLKSRC_AUDIOBUS 1 #define S3C_I2SV2_CLKSRC_CDCLK 2
+/* Set this flag for I2S controllers that have the bit IISMOD[12] + * bridge/break RCLK signal and external Xi2sCDCLK pin. + */ +#define S3C_FEATURE_CDCLKCON (1 << 0) + /** * struct s3c_i2sv2_info - S3C I2S-V2 information * @dev: The parent device passed to use from the probe. * @regs: The pointer to the device registe block. + * @feature: Set of bit-flags indicating features of the controller. * @master: True if the I2S core is the I2S bit clock master. * @dma_playback: DMA information for playback channel. * @dma_capture: DMA information for capture channel. @@ -47,6 +53,8 @@ struct s3c_i2sv2_info { struct device *dev; void __iomem *regs;
+ u32 feature; + struct clk *iis_pclk; struct clk *iis_cclk;
Now that we can specify feature of a particular controller, we can avoid multiple copies of same code by defining the CDCLKCON bit feature in controller specific code and detecting that flag in the code common to all controllers.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/s3c-i2s-v2.c | 47 +++++++++++++++++++++++++++++++++++++++ sound/soc/s3c24xx/s3c2412-i2s.c | 27 ---------------------- sound/soc/s3c24xx/s3c64xx-i2s.c | 43 ++--------------------------------- 3 files changed, 50 insertions(+), 67 deletions(-)
diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index ecb3054..7d2edc0 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -325,6 +325,52 @@ static int s3c_i2sv2_hw_params(struct snd_pcm_substream *substream,
writel(iismod, i2s->regs + S3C2412_IISMOD); pr_debug("%s: w: IISMOD: %x\n", __func__, iismod); + + return 0; +} + +static int s3c_i2sv2_set_sysclk(struct snd_soc_dai *cpu_dai, + int clk_id, unsigned int freq, int dir) +{ + struct s3c_i2sv2_info *i2s = to_info(cpu_dai); + u32 iismod = readl(i2s->regs + S3C2412_IISMOD); + + pr_debug("Entered %s\n", __func__); + pr_debug("%s r: IISMOD: %x \n", __func__, iismod); + + switch (clk_id) { + case S3C_I2SV2_CLKSRC_PCLK: + iismod &= ~S3C2412_IISMOD_IMS_SYSMUX; + break; + + case S3C_I2SV2_CLKSRC_AUDIOBUS: + iismod |= S3C2412_IISMOD_IMS_SYSMUX; + break; + + case S3C_I2SV2_CLKSRC_CDCLK: + /* Error if controller doesn't have the CDCLKCON bit */ + if (!(i2s->feature & S3C_FEATURE_CDCLKCON)) + return -EINVAL; + + switch (dir) { + case SND_SOC_CLOCK_IN: + iismod |= S3C64XX_IISMOD_CDCLKCON; + break; + case SND_SOC_CLOCK_OUT: + iismod &= ~S3C64XX_IISMOD_CDCLKCON; + break; + default: + return -EINVAL; + } + break; + + default: + return -EINVAL; + } + + writel(iismod, i2s->regs + S3C2412_IISMOD); + pr_debug("%s w: IISMOD: %x \n", __func__, iismod); + return 0; }
@@ -697,6 +743,7 @@ int s3c_i2sv2_register_dai(struct snd_soc_dai *dai) ops->hw_params = s3c_i2sv2_hw_params; ops->set_fmt = s3c2412_i2s_set_fmt; ops->set_clkdiv = s3c2412_i2s_set_clkdiv; + ops->set_sysclk = s3c_i2sv2_set_sysclk;
/* Allow overriding by (for example) IISv4 */ if (!ops->delay) diff --git a/sound/soc/s3c24xx/s3c2412-i2s.c b/sound/soc/s3c24xx/s3c2412-i2s.c index fe90867..e351376 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.c +++ b/sound/soc/s3c24xx/s3c2412-i2s.c @@ -65,32 +65,6 @@ static struct s3c_dma_params s3c2412_i2s_pcm_stereo_in = {
static struct s3c_i2sv2_info s3c2412_i2s;
-/* - * Set S3C2412 Clock source - */ -static int s3c2412_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, - int clk_id, unsigned int freq, int dir) -{ - u32 iismod = readl(s3c2412_i2s.regs + S3C2412_IISMOD); - - pr_debug("%s(%p, %d, %u, %d)\n", __func__, cpu_dai, clk_id, - freq, dir); - - switch (clk_id) { - case S3C2412_CLKSRC_PCLK: - iismod &= ~S3C2412_IISMOD_IMS_SYSMUX; - break; - case S3C2412_CLKSRC_I2SCLK: - iismod |= S3C2412_IISMOD_IMS_SYSMUX; - break; - default: - return -EINVAL; - } - - writel(iismod, s3c2412_i2s.regs + S3C2412_IISMOD); - return 0; -} - static inline struct s3c_i2sv2_info *to_info(struct snd_soc_dai *cpu_dai) { return cpu_dai->private_data; @@ -172,7 +146,6 @@ static int s3c2412_i2s_hw_params(struct snd_pcm_substream *substream, SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
static struct snd_soc_dai_ops s3c2412_i2s_dai_ops = { - .set_sysclk = s3c2412_i2s_set_sysclk, .hw_params = s3c2412_i2s_hw_params, };
diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c index 1a29564..03cce8a 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.c +++ b/sound/soc/s3c24xx/s3c64xx-i2s.c @@ -54,43 +54,6 @@ static inline struct s3c_i2sv2_info *to_info(struct snd_soc_dai *cpu_dai) return cpu_dai->private_data; }
-static int s3c64xx_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, - int clk_id, unsigned int freq, int dir) -{ - struct s3c_i2sv2_info *i2s = to_info(cpu_dai); - u32 iismod = readl(i2s->regs + S3C2412_IISMOD); - - switch (clk_id) { - case S3C64XX_CLKSRC_PCLK: - iismod &= ~S3C2412_IISMOD_IMS_SYSMUX; - break; - - case S3C64XX_CLKSRC_MUX: - iismod |= S3C2412_IISMOD_IMS_SYSMUX; - break; - - case S3C64XX_CLKSRC_CDCLK: - switch (dir) { - case SND_SOC_CLOCK_IN: - iismod |= S3C64XX_IISMOD_CDCLKCON; - break; - case SND_SOC_CLOCK_OUT: - iismod &= ~S3C64XX_IISMOD_CDCLKCON; - break; - default: - return -EINVAL; - } - break; - - default: - return -EINVAL; - } - - writel(iismod, i2s->regs + S3C2412_IISMOD); - - return 0; -} - static int s3c64xx_i2s_probe(struct platform_device *pdev, struct snd_soc_dai *dai) { @@ -115,9 +78,7 @@ static int s3c64xx_i2s_probe(struct platform_device *pdev, }
-static struct snd_soc_dai_ops s3c64xx_i2s_dai_ops = { - .set_sysclk = s3c64xx_i2s_set_sysclk, -}; +static struct snd_soc_dai_ops s3c64xx_i2s_dai_ops;
static __devinit int s3c64xx_iis_dev_probe(struct platform_device *pdev) { @@ -147,6 +108,8 @@ static __devinit int s3c64xx_iis_dev_probe(struct platform_device *pdev) dai->probe = s3c64xx_i2s_probe; dai->ops = &s3c64xx_i2s_dai_ops;
+ i2s->feature |= S3C_FEATURE_CDCLKCON; + i2s->dma_capture = &s3c64xx_i2s_pcm_stereo_in[pdev->id]; i2s->dma_playback = &s3c64xx_i2s_pcm_stereo_out[pdev->id];
Add the CPU driver for the IISv4 block found on S3C6410. For now, the driver is almost a copy of s3c64xx-i2s.c but it should diverge as more IISv4 specific stuff is added.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/Kconfig | 5 + sound/soc/s3c24xx/Makefile | 2 + sound/soc/s3c24xx/s3c64xx-i2s-v4.c | 206 ++++++++++++++++++++++++++++++++++++ sound/soc/s3c24xx/s3c64xx-i2s.h | 1 + 4 files changed, 214 insertions(+), 0 deletions(-) create mode 100644 sound/soc/s3c24xx/s3c64xx-i2s-v4.c
diff --git a/sound/soc/s3c24xx/Kconfig b/sound/soc/s3c24xx/Kconfig index 15fe57e..c0c7edf 100644 --- a/sound/soc/s3c24xx/Kconfig +++ b/sound/soc/s3c24xx/Kconfig @@ -24,6 +24,11 @@ config SND_S3C64XX_SOC_I2S select SND_S3C_I2SV2_SOC select S3C64XX_DMA
+config SND_S3C64XX_SOC_I2S_V4 + tristate + select SND_S3C_I2SV2_SOC + select S3C64XX_DMA + config SND_S3C_SOC_PCM tristate
diff --git a/sound/soc/s3c24xx/Makefile b/sound/soc/s3c24xx/Makefile index df071a3..81d8dc5 100644 --- a/sound/soc/s3c24xx/Makefile +++ b/sound/soc/s3c24xx/Makefile @@ -4,6 +4,7 @@ snd-soc-s3c24xx-i2s-objs := s3c24xx-i2s.o snd-soc-s3c2412-i2s-objs := s3c2412-i2s.o snd-soc-s3c64xx-i2s-objs := s3c64xx-i2s.o snd-soc-s3c-ac97-objs := s3c-ac97.o +snd-soc-s3c64xx-i2s-v4-objs := s3c64xx-i2s-v4.o snd-soc-s3c-i2s-v2-objs := s3c-i2s-v2.o snd-soc-s3c-pcm-objs := s3c-pcm.o
@@ -12,6 +13,7 @@ obj-$(CONFIG_SND_S3C24XX_SOC_I2S) += snd-soc-s3c24xx-i2s.o obj-$(CONFIG_SND_S3C_SOC_AC97) += snd-soc-s3c-ac97.o obj-$(CONFIG_SND_S3C2412_SOC_I2S) += snd-soc-s3c2412-i2s.o obj-$(CONFIG_SND_S3C64XX_SOC_I2S) += snd-soc-s3c64xx-i2s.o +obj-$(CONFIG_SND_S3C64XX_SOC_I2S_V4) += snd-soc-s3c64xx-i2s-v4.o obj-$(CONFIG_SND_S3C_I2SV2_SOC) += snd-soc-s3c-i2s-v2.o obj-$(CONFIG_SND_S3C_SOC_PCM) += snd-soc-s3c-pcm.o
diff --git a/sound/soc/s3c24xx/s3c64xx-i2s-v4.c b/sound/soc/s3c24xx/s3c64xx-i2s-v4.c new file mode 100644 index 0000000..d1fa837 --- /dev/null +++ b/sound/soc/s3c24xx/s3c64xx-i2s-v4.c @@ -0,0 +1,206 @@ +/* sound/soc/s3c24xx/s3c64xx-i2s-v4.c + * + * ALSA SoC Audio Layer - S3C64XX I2Sv4 driver + * Copyright (c) 2010 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 version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/clk.h> +#include <linux/gpio.h> +#include <linux/io.h> + +#include <sound/soc.h> +#include <sound/pcm_params.h> + +#include <plat/gpio-bank-c.h> +#include <plat/gpio-bank-h.h> +#include <plat/gpio-cfg.h> + +#include <mach/map.h> +#include <mach/dma.h> + +#include "s3c-dma.h" +#include "regs-i2s-v2.h" +#include "s3c64xx-i2s.h" + +static struct s3c2410_dma_client s3c64xx_dma_client_out = { + .name = "I2Sv4 PCM Stereo out" +}; + +static struct s3c2410_dma_client s3c64xx_dma_client_in = { + .name = "I2Sv4 PCM Stereo in" +}; + +static struct s3c_dma_params s3c64xx_i2sv4_pcm_stereo_out; +static struct s3c_dma_params s3c64xx_i2sv4_pcm_stereo_in; +static struct s3c_i2sv2_info s3c64xx_i2sv4; + +struct snd_soc_dai s3c64xx_i2s_v4_dai; +EXPORT_SYMBOL_GPL(s3c64xx_i2s_v4_dai); + +static inline struct s3c_i2sv2_info *to_info(struct snd_soc_dai *cpu_dai) +{ + return cpu_dai->private_data; +} + +static int s3c64xx_i2sv4_probe(struct platform_device *pdev, + struct snd_soc_dai *dai) +{ + /* configure GPIO for i2s port */ + s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C64XX_GPC4_I2S_V40_DO0); + s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C64XX_GPC5_I2S_V40_DO1); + s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C64XX_GPC7_I2S_V40_DO2); + s3c_gpio_cfgpin(S3C64XX_GPH(6), S3C64XX_GPH6_I2S_V40_BCLK); + s3c_gpio_cfgpin(S3C64XX_GPH(7), S3C64XX_GPH7_I2S_V40_CDCLK); + s3c_gpio_cfgpin(S3C64XX_GPH(8), S3C64XX_GPH8_I2S_V40_LRCLK); + s3c_gpio_cfgpin(S3C64XX_GPH(9), S3C64XX_GPH9_I2S_V40_DI); + + return 0; +} + +static int s3c_i2sv4_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *cpu_dai) +{ + struct s3c_i2sv2_info *i2s = to_info(cpu_dai); + u32 iismod; + + dev_dbg(cpu_dai->dev, "Entered %s\n", __func__); + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + cpu_dai->dma_data = i2s->dma_playback; + else + cpu_dai->dma_data = i2s->dma_capture; + + iismod = readl(i2s->regs + S3C2412_IISMOD); + dev_dbg(cpu_dai->dev, "%s: r: IISMOD: %x\n", __func__, iismod); + + iismod &= ~S3C64XX_IISMOD_BLC_MASK; + switch (params_format(params)) { + case SNDRV_PCM_FORMAT_S8: + iismod |= S3C64XX_IISMOD_BLC_8BIT; + break; + case SNDRV_PCM_FORMAT_S16_LE: + break; + case SNDRV_PCM_FORMAT_S24_LE: + iismod |= S3C64XX_IISMOD_BLC_24BIT; + break; + } + + writel(iismod, i2s->regs + S3C2412_IISMOD); + dev_dbg(cpu_dai->dev, "%s: w: IISMOD: %x\n", __func__, iismod); + + return 0; +} + +static struct snd_soc_dai_ops s3c64xx_i2sv4_dai_ops = { + .hw_params = s3c_i2sv4_hw_params, +}; + +static __devinit int s3c64xx_i2sv4_dev_probe(struct platform_device *pdev) +{ + struct s3c_i2sv2_info *i2s; + struct snd_soc_dai *dai; + int ret; + + i2s = &s3c64xx_i2sv4; + dai = &s3c64xx_i2s_v4_dai; + + if (dai->dev) { + dev_dbg(dai->dev, "%s: \ + I2Sv4 instance already registered!\n", __func__); + return -EBUSY; + } + + dai->dev = &pdev->dev; + dai->name = "s3c64xx-i2s-v4"; + dai->id = 0; + dai->symmetric_rates = 1; + dai->playback.channels_min = 2; + dai->playback.channels_max = 2; + dai->playback.rates = S3C64XX_I2S_RATES; + dai->playback.formats = S3C64XX_I2S_FMTS; + dai->capture.channels_min = 2; + dai->capture.channels_max = 2; + dai->capture.rates = S3C64XX_I2S_RATES; + dai->capture.formats = S3C64XX_I2S_FMTS; + dai->probe = s3c64xx_i2sv4_probe; + dai->ops = &s3c64xx_i2sv4_dai_ops; + + i2s->feature |= S3C_FEATURE_CDCLKCON; + + i2s->dma_capture = &s3c64xx_i2sv4_pcm_stereo_in; + i2s->dma_playback = &s3c64xx_i2sv4_pcm_stereo_out; + + i2s->dma_capture->channel = DMACH_HSI_I2SV40_RX; + i2s->dma_capture->dma_addr = S3C64XX_PA_IISV4 + S3C2412_IISRXD; + i2s->dma_playback->channel = DMACH_HSI_I2SV40_TX; + i2s->dma_playback->dma_addr = S3C64XX_PA_IISV4 + S3C2412_IISTXD; + + i2s->dma_capture->client = &s3c64xx_dma_client_in; + i2s->dma_capture->dma_size = 4; + i2s->dma_playback->client = &s3c64xx_dma_client_out; + i2s->dma_playback->dma_size = 4; + + i2s->iis_cclk = clk_get(&pdev->dev, "audio-bus"); + if (IS_ERR(i2s->iis_cclk)) { + dev_err(&pdev->dev, "failed to get audio-bus\n"); + ret = PTR_ERR(i2s->iis_cclk); + goto err; + } + + clk_enable(i2s->iis_cclk); + + ret = s3c_i2sv2_probe(pdev, dai, i2s, 0); + if (ret) + goto err_clk; + + ret = s3c_i2sv2_register_dai(dai); + if (ret != 0) + goto err_i2sv2; + + return 0; + +err_i2sv2: + /* Not implemented for I2Sv2 core yet */ +err_clk: + clk_put(i2s->iis_cclk); +err: + return ret; +} + +static __devexit int s3c64xx_i2sv4_dev_remove(struct platform_device *pdev) +{ + dev_err(&pdev->dev, "Device removal not yet supported\n"); + return 0; +} + +static struct platform_driver s3c64xx_i2sv4_driver = { + .probe = s3c64xx_i2sv4_dev_probe, + .remove = s3c64xx_i2sv4_dev_remove, + .driver = { + .name = "s3c64xx-iis-v4", + .owner = THIS_MODULE, + }, +}; + +static int __init s3c64xx_i2sv4_init(void) +{ + return platform_driver_register(&s3c64xx_i2sv4_driver); +} +module_init(s3c64xx_i2sv4_init); + +static void __exit s3c64xx_i2sv4_exit(void) +{ + platform_driver_unregister(&s3c64xx_i2sv4_driver); +} +module_exit(s3c64xx_i2sv4_exit); + +/* Module information */ +MODULE_AUTHOR("Jaswinder Singh, jassi.brar@samsung.com"); +MODULE_DESCRIPTION("S3C64XX I2Sv4 SoC Interface"); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.h b/sound/soc/s3c24xx/s3c64xx-i2s.h index f27ed50..7a40f43 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.h +++ b/sound/soc/s3c24xx/s3c64xx-i2s.h @@ -37,5 +37,6 @@ struct clk; SNDRV_PCM_FMTBIT_S24_LE)
extern struct snd_soc_dai s3c64xx_i2s_dai[]; +extern struct snd_soc_dai s3c64xx_i2s_v4_dai;
#endif /* __SND_SOC_S3C24XX_S3C64XX_I2S_H */
Switch the MACHINE driver to use IISv4 CPU dai. Remove BROKEN dependency now that we have proper CPU driver available. Also, disable build for SMDK6400, since the S3C6400 doesn't have IISv4 controller.
Signed-off-by: Jassi Brar jassi.brar@samsung.com --- sound/soc/s3c24xx/Kconfig | 7 +++---- sound/soc/s3c24xx/smdk64xx_wm8580.c | 6 ++---- 2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/sound/soc/s3c24xx/Kconfig b/sound/soc/s3c24xx/Kconfig index c0c7edf..2ac1f33 100644 --- a/sound/soc/s3c24xx/Kconfig +++ b/sound/soc/s3c24xx/Kconfig @@ -64,12 +64,11 @@ config SND_S3C24XX_SOC_JIVE_WM8750
config SND_S3C64XX_SOC_WM8580 tristate "SoC I2S Audio support for WM8580 on SMDK64XX" - depends on SND_S3C24XX_SOC && (MACH_SMDK6400 || MACH_SMDK6410) - depends on BROKEN + depends on SND_S3C24XX_SOC && MACH_SMDK6410 select SND_SOC_WM8580 - select SND_S3C64XX_SOC_I2S + select SND_S3C64XX_SOC_I2S_V4 help - Sat Y if you want to add support for SoC audio on the SMDK64XX. + Say 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" diff --git a/sound/soc/s3c24xx/smdk64xx_wm8580.c b/sound/soc/s3c24xx/smdk64xx_wm8580.c index efe4901..07e8e51 100644 --- a/sound/soc/s3c24xx/smdk64xx_wm8580.c +++ b/sound/soc/s3c24xx/smdk64xx_wm8580.c @@ -22,8 +22,6 @@ #include "s3c-dma.h" #include "s3c64xx-i2s.h"
-#define S3C64XX_I2S_V4 2 - /* SMDK64XX has a 12MHZ crystal attached to WM8580 */ #define SMDK64XX_WM8580_FREQ 12000000
@@ -215,7 +213,7 @@ 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], + .cpu_dai = &s3c64xx_i2s_v4_dai, .codec_dai = &wm8580_dai[WM8580_DAI_PAIFRX], .init = smdk64xx_wm8580_init_paifrx, .ops = &smdk64xx_ops, @@ -223,7 +221,7 @@ static struct snd_soc_dai_link smdk64xx_dai[] = { { /* Primary Capture i/f */ .name = "WM8580 PAIF TX", .stream_name = "Capture", - .cpu_dai = &s3c64xx_i2s_dai[S3C64XX_I2S_V4], + .cpu_dai = &s3c64xx_i2s_v4_dai, .codec_dai = &wm8580_dai[WM8580_DAI_PAIFTX], .init = smdk64xx_wm8580_init_paiftx, .ops = &smdk64xx_ops,
On Wed, Mar 10, 2010 at 04:49:09PM +0900, Jassi Brar wrote:
Switch the MACHINE driver to use IISv4 CPU dai. Remove BROKEN dependency now that we have proper CPU driver available. Also, disable build for SMDK6400, since the S3C6400 doesn't have IISv4 controller.
Will the driver actually probe with mainline?
On Wed, Mar 10, 2010 at 8:40 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:49:09PM +0900, Jassi Brar wrote:
Switch the MACHINE driver to use IISv4 CPU dai. Remove BROKEN dependency now that we have proper CPU driver available. Also, disable build for SMDK6400, since the S3C6400 doesn't have IISv4 controller.
Will the driver actually probe with mainline?
Need to check.
On Wed, Mar 10, 2010 at 04:49:08PM +0900, Jassi Brar wrote:
+static int s3c64xx_i2sv4_probe(struct platform_device *pdev,
struct snd_soc_dai *dai)+{
- /* configure GPIO for i2s port */
- s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C64XX_GPC4_I2S_V40_DO0);
- s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C64XX_GPC5_I2S_V40_DO1);
- s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C64XX_GPC7_I2S_V40_DO2);
- s3c_gpio_cfgpin(S3C64XX_GPH(6), S3C64XX_GPH6_I2S_V40_BCLK);
- s3c_gpio_cfgpin(S3C64XX_GPH(7), S3C64XX_GPH7_I2S_V40_CDCLK);
- s3c_gpio_cfgpin(S3C64XX_GPH(8), S3C64XX_GPH8_I2S_V40_LRCLK);
- s3c_gpio_cfgpin(S3C64XX_GPH(9), S3C64XX_GPH9_I2S_V40_DI);
It seems retrograde to add this rather than using the platform data callback that you added. We'll need some arch/arm changes for the driver to probe anyway so it't not a big deal to add the dependency.
On Wed, Mar 10, 2010 at 8:39 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:49:08PM +0900, Jassi Brar wrote:
+static int s3c64xx_i2sv4_probe(struct platform_device *pdev,
- struct snd_soc_dai *dai)
+{
- /* configure GPIO for i2s port */
- s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C64XX_GPC4_I2S_V40_DO0);
- s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C64XX_GPC5_I2S_V40_DO1);
- s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C64XX_GPC7_I2S_V40_DO2);
- s3c_gpio_cfgpin(S3C64XX_GPH(6), S3C64XX_GPH6_I2S_V40_BCLK);
- s3c_gpio_cfgpin(S3C64XX_GPH(7), S3C64XX_GPH7_I2S_V40_CDCLK);
- s3c_gpio_cfgpin(S3C64XX_GPH(8), S3C64XX_GPH8_I2S_V40_LRCLK);
- s3c_gpio_cfgpin(S3C64XX_GPH(9), S3C64XX_GPH9_I2S_V40_DI);
It seems retrograde to add this rather than using the platform data callback that you added. We'll need some arch/arm changes for the driver to probe anyway so it't not a big deal to add the dependency.
I know, but that way I would have had to wait on S3C ARCH tree reflect the patch(for which s3c64xx-i2s.c depends already). When my latest ARCH patches are visible in your tree, we can easily switch to using cfg_gpio in both the drivers.
On Wed, Mar 10, 2010 at 04:49:06PM +0900, Jassi Brar wrote:
While adding support for new SoCs' I2S, check for the blocks of older SoCs that have similar feature and set the flag for that feature.
As a result of this...
+/* Set this flag for I2S controllers that have the bit IISMOD[12]
- bridge/break RCLK signal and external Xi2sCDCLK pin.
- */
+#define S3C_FEATURE_CDCLKCON (1 << 0)
...I'd suggest moving this into the patch that adds support for CDCLKCON (or merging that patch in with this one).
Probably worth namespacing these defines a bit more (eg, S3C_IIS_) since I'd expect they'll end up being used by the arch/arm code too in the platform data definition. They ought to be in include/sound or arch/arm for visibility to arch/arm code too.
On Wed, Mar 10, 2010 at 11:09:59AM +0000, Mark Brown wrote:
Probably worth namespacing these defines a bit more (eg, S3C_IIS_) since I'd expect they'll end up being used by the arch/arm code too in the platform data definition. They ought to be in include/sound or arch/arm for visibility to arch/arm code too.
Meh, scratch that - I see where this is being used now.
On Wed, Mar 10, 2010 at 04:49:00PM +0900, Jassi Brar wrote:
Define more bit definitions in the order of mainline support for the SoC.
For changes like this it'd be better if the changelog said something like "Add register bit definitions for S5PC1xx" - this is all mainline code so it's much clearer to say that this is adding support for a new SoC.
One other thing I'd suggest is that when constructing a patch series it'd be better to put the more invasive or controversial changes (like moving the headers) last. This makes it easer to apply bits of the series if there is any controversy.
On Wed, Mar 10, 2010 at 8:00 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:49:00PM +0900, Jassi Brar wrote:
Define more bit definitions in the order of mainline support for the SoC.
For changes like this it'd be better if the changelog said something like "Add register bit definitions for S5PC1xx" - this is all mainline code so it's much clearer to say that this is adding support for a new SoC.
I thought of adding definitions in 'chronological order of support' in kernel rather than SoC wise. Ok, will divide in two patches.
One other thing I'd suggest is that when constructing a patch series it'd be better to put the more invasive or controversial changes (like moving the headers) last. This makes it easer to apply bits of the series if there is any controversy.
I thought I already made that sure. In my opinion, the only controversial patch was 'header-copying' and I couldn't move that any further down the series or other changes without that.
On Wed, Mar 10, 2010 at 09:24:45PM +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 8:00 PM, Mark Brown
One other thing I'd suggest is that when constructing a patch series it'd be better to put the more invasive or controversial changes (like moving the headers) last. This makes it easer to apply bits of the series if there is any controversy.
I thought I already made that sure. In my opinion, the only controversial patch was 'header-copying' and I couldn't move that any further down the series or other changes without that.
I agree that this is the only really controversial change, but it seemed like pretty much all the changes that added stuff to the headers or used them could've been done pre-move so that they didn't depend on it.
On Wed, Mar 10, 2010 at 9:38 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 09:24:45PM +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 8:00 PM, Mark Brown
One other thing I'd suggest is that when constructing a patch series it'd be better to put the more invasive or controversial changes (like moving the headers) last. This makes it easer to apply bits of the series if there is any controversy.
I thought I already made that sure. In my opinion, the only controversial patch was 'header-copying' and I couldn't move that any further down the series or other changes without that.
I agree that this is the only really controversial change, but it seemed like pretty much all the changes that added stuff to the headers or used them could've been done pre-move so that they didn't depend on it.
Not sure which patch you point, but it was decision of logical build-up and successful compilation after each patch that made the patch series as it is now.
On Wed, Mar 10, 2010 at 09:49:03PM +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 9:38 PM, Mark Brown
I agree that this is the only really controversial change, but it seemed like pretty much all the changes that added stuff to the headers or used them could've been done pre-move so that they didn't depend on it.
Not sure which patch you point, but it was decision of logical build-up and successful compilation after each patch that made the patch series as it is now.
Pretty much all of them - for example, patch 11 adds a bunch of new bitfield definitions to the header. This change didn't really need the header to have been moved before it was made, it could have been done pre-move. Keeping things building is important and some of the changes that affected both S3C24xx and S3C64xx might've needed the move for that but not all of them.
On Wed, Mar 10, 2010 at 9:56 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 09:49:03PM +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 9:38 PM, Mark Brown
I agree that this is the only really controversial change, but it seemed like pretty much all the changes that added stuff to the headers or used them could've been done pre-move so that they didn't depend on it.
Not sure which patch you point, but it was decision of logical build-up and successful compilation after each patch that made the patch series as it is now.
Pretty much all of them - for example, patch 11 adds a bunch of new bitfield definitions to the header. This change didn't really need the header to have been moved before it was made, it could have been done pre-move.
Dear that wud have defeated the very purpose of header moving -- don't wanna depend on changes surfacing via ARCH tree after too long.
On Wed, Mar 10, 2010 at 10:03:28PM +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 9:56 PM, Mark Brown
Pretty much all of them - for example, patch 11 adds a bunch of new bitfield definitions to the header. This change didn't really need the header to have been moved before it was made, it could have been done pre-move.
Dear that wud have defeated the very purpose of header moving -- don't wanna depend on changes surfacing via ARCH tree after too long.
Right, but the header move is itself an arch change so you're blocked on one anyway until it goes in :/
On Wed, Mar 10, 2010 at 04:48:59PM +0900, Jassi Brar wrote:
The header for I2Sv2 (linux/arch/arm/plat-s3c/include/plat/regs-s3c2412-iis.h) contains only controller specific definitions and nothing SoC specific. So, it could be moved to sound/soc/s3c24xx/ That will also help avoid delays in accepting patches that touch ASOC and ARCH parts.
For now, we simply copy to regs-i2s-v2.h without any material changes, and render the regs-s3c2412-iis.h useless in the kernel. A separate patch will be submitted to S3C ARCH tree to remove the regs-s3c2412-iis.h
could you please reformat the header to wrap nicely?
I'd ack a patch moving this in one go since there shouldn't be any updates to this file going via my tree.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
sound/soc/s3c24xx/regs-i2s-v2.h | 79 +++++++++++++++++++++++++++++++++++++++ sound/soc/s3c24xx/s3c-i2s-v2.c | 3 +- sound/soc/s3c24xx/s3c2412-i2s.c | 3 +- sound/soc/s3c24xx/s3c64xx-i2s.c | 2 +- 4 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 sound/soc/s3c24xx/regs-i2s-v2.h
diff --git a/sound/soc/s3c24xx/regs-i2s-v2.h b/sound/soc/s3c24xx/regs-i2s-v2.h new file mode 100644 index 0000000..8e1a585 --- /dev/null +++ b/sound/soc/s3c24xx/regs-i2s-v2.h @@ -0,0 +1,79 @@ +/* linux/sound/soc/s3c24xx/regs-i2s-v2.h
- Copyright 2007 Simtec Electronics linux@simtec.co.uk
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- Register definitions for controllers that share the I2Sv2 part
+*/
+#ifndef __REGS_IISV2_H +#define __REGS_IISV2_H
+#define S3C2412_IISCON (0x00) +#define S3C2412_IISMOD (0x04) +#define S3C2412_IISFIC (0x08) +#define S3C2412_IISPSR (0x0C) +#define S3C2412_IISTXD (0x10) +#define S3C2412_IISRXD (0x14)
+#define S3C2412_IISCON_LRINDEX (1 << 11) +#define S3C2412_IISCON_TXFIFO_EMPTY (1 << 10) +#define S3C2412_IISCON_RXFIFO_EMPTY (1 << 9) +#define S3C2412_IISCON_TXFIFO_FULL (1 << 8) +#define S3C2412_IISCON_RXFIFO_FULL (1 << 7) +#define S3C2412_IISCON_TXDMA_PAUSE (1 << 6) +#define S3C2412_IISCON_RXDMA_PAUSE (1 << 5) +#define S3C2412_IISCON_TXCH_PAUSE (1 << 4) +#define S3C2412_IISCON_RXCH_PAUSE (1 << 3) +#define S3C2412_IISCON_TXDMA_ACTIVE (1 << 2) +#define S3C2412_IISCON_RXDMA_ACTIVE (1 << 1) +#define S3C2412_IISCON_IIS_ACTIVE (1 << 0)
+#define S3C64XX_IISMOD_BLC_16BIT (0 << 13) +#define S3C64XX_IISMOD_BLC_8BIT (1 << 13) +#define S3C64XX_IISMOD_BLC_24BIT (2 << 13) +#define S3C64XX_IISMOD_BLC_MASK (3 << 13)
+#define S3C64XX_IISMOD_IMS_PCLK (0 << 10) +#define S3C64XX_IISMOD_IMS_SYSMUX (1 << 10)
+#define S3C2412_IISMOD_MASTER_INTERNAL (0 << 10) +#define S3C2412_IISMOD_MASTER_EXTERNAL (1 << 10) +#define S3C2412_IISMOD_SLAVE (2 << 10) +#define S3C2412_IISMOD_MASTER_MASK (3 << 10) +#define S3C2412_IISMOD_MODE_TXONLY (0 << 8) +#define S3C2412_IISMOD_MODE_RXONLY (1 << 8) +#define S3C2412_IISMOD_MODE_TXRX (2 << 8) +#define S3C2412_IISMOD_MODE_MASK (3 << 8) +#define S3C2412_IISMOD_LR_LLOW (0 << 7) +#define S3C2412_IISMOD_LR_RLOW (1 << 7) +#define S3C2412_IISMOD_SDF_IIS (0 << 5) +#define S3C2412_IISMOD_SDF_MSB (1 << 5) +#define S3C2412_IISMOD_SDF_LSB (2 << 5) +#define S3C2412_IISMOD_SDF_MASK (3 << 5) +#define S3C2412_IISMOD_RCLK_256FS (0 << 3) +#define S3C2412_IISMOD_RCLK_512FS (1 << 3) +#define S3C2412_IISMOD_RCLK_384FS (2 << 3) +#define S3C2412_IISMOD_RCLK_768FS (3 << 3) +#define S3C2412_IISMOD_RCLK_MASK (3 << 3) +#define S3C2412_IISMOD_BCLK_32FS (0 << 1) +#define S3C2412_IISMOD_BCLK_48FS (1 << 1) +#define S3C2412_IISMOD_BCLK_16FS (2 << 1) +#define S3C2412_IISMOD_BCLK_24FS (3 << 1) +#define S3C2412_IISMOD_BCLK_MASK (3 << 1) +#define S3C2412_IISMOD_8BIT (1 << 0)
+#define S3C64XX_IISMOD_CDCLKCON (1 << 12)
+#define S3C2412_IISPSR_PSREN (1 << 15)
+#define S3C2412_IISFIC_TXFLUSH (1 << 15) +#define S3C2412_IISFIC_RXFLUSH (1 << 7) +#define S3C2412_IISFIC_TXCOUNT(x) (((x) >> 8) & 0xf) +#define S3C2412_IISFIC_RXCOUNT(x) (((x) >> 0) & 0xf)
+#endif /* __REGS_IISV2_H */ diff --git a/sound/soc/s3c24xx/s3c-i2s-v2.c b/sound/soc/s3c24xx/s3c-i2s-v2.c index 0d655ab..b690078 100644 --- a/sound/soc/s3c24xx/s3c-i2s-v2.c +++ b/sound/soc/s3c24xx/s3c-i2s-v2.c @@ -24,10 +24,9 @@ #include <sound/pcm_params.h> #include <sound/soc.h>
-#include <plat/regs-s3c2412-iis.h>
#include <mach/dma.h>
+#include "regs-i2s-v2.h" #include "s3c-i2s-v2.h" #include "s3c-dma.h"
diff --git a/sound/soc/s3c24xx/s3c2412-i2s.c b/sound/soc/s3c24xx/s3c2412-i2s.c index a5b21f6..42fb663 100644 --- a/sound/soc/s3c24xx/s3c2412-i2s.c +++ b/sound/soc/s3c24xx/s3c2412-i2s.c @@ -32,12 +32,11 @@ #include <sound/soc.h> #include <mach/hardware.h>
-#include <plat/regs-s3c2412-iis.h>
#include <mach/regs-gpio.h> #include <mach/dma.h>
#include "s3c-dma.h" +#include "regs-i2s-v2.h" #include "s3c2412-i2s.h"
#define S3C2412_I2S_DEBUG 0 diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c index 6552894..9bcc99d 100644 --- a/sound/soc/s3c24xx/s3c64xx-i2s.c +++ b/sound/soc/s3c24xx/s3c64xx-i2s.c @@ -18,7 +18,6 @@
#include <sound/soc.h>
-#include <plat/regs-s3c2412-iis.h> #include <plat/gpio-bank-d.h> #include <plat/gpio-bank-e.h> #include <plat/gpio-cfg.h> @@ -27,6 +26,7 @@ #include <mach/dma.h>
#include "s3c-dma.h" +#include "regs-i2s-v2.h" #include "s3c64xx-i2s.h"
/* The value should be set to maximum of the total number
1.6.2.5
On Wed, Mar 10, 2010 at 04:48:58PM +0900, Jassi Brar wrote:
Towards having build for multiple SoCs segregate hw_params callback for s3c2412 and s3c64xx. Since, all new SoCs have s3c64xx like register map, we keep that as default implementation if no SoC specific callback is already defined.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
This is OK but appears to depend on one or more of the other patches that didn't get applied yet.
On Wed, Mar 10, 2010 at 02:14:17PM +0000, Mark Brown wrote:
On Wed, Mar 10, 2010 at 04:48:58PM +0900, Jassi Brar wrote:
Towards having build for multiple SoCs segregate hw_params callback for s3c2412 and s3c64xx. Since, all new SoCs have s3c64xx like register map, we keep that as default implementation if no SoC specific callback is already defined.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
This is OK but appears to depend on one or more of the other patches that didn't get applied yet.
Actually it does apply - patch 3 hadn't actually made it into the queue I was applying and that was the dependency. I've now got them both applied, though they were missing from the pull I sent to Takashi earlier so I've not pushed them up to kernel.org yet.
On Wed, Mar 10, 2010 at 04:48:57PM +0900, Jassi Brar wrote:
In order for the RATE and FMT defines to be reuseable in future by the i2sv4 driver, move the MACROs out to the header file.
I'll apply this but for the rates...
-#define S3C64XX_I2S_RATES \
- (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 | SNDRV_PCM_RATE_16000 | \
- SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \
- SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
This is just SNDRV_PCM_RATE_8000_96000 anyway so may as well use it directly.
On Wed, Mar 10, 2010 at 7:51 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:48:57PM +0900, Jassi Brar wrote:
In order for the RATE and FMT defines to be reuseable in future by the i2sv4 driver, move the MACROs out to the header file.
I'll apply this but for the rates...
-#define S3C64XX_I2S_RATES \
- (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 | SNDRV_PCM_RATE_16000 | \
- SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \
- SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
This is just SNDRV_PCM_RATE_8000_96000 anyway so may as well use it directly.
Of course. Btw, I was thinking of adding RATE_KNOT too, as it is usually possible to achieve 'not so standard' 12000 and 24000 rates, besides the flag doesn't 'promise' anything about availability of rates.
On Wed, Mar 10, 2010 at 09:52:49PM +0900, jassi brar wrote:
Btw, I was thinking of adding RATE_KNOT too, as it is usually possible to achieve 'not so standard' 12000 and 24000 rates, besides the flag doesn't 'promise' anything about availability of rates.
Yeah, _KNOT support needs more work in the core - at the minute it's just doing a bitmask match on the standard rates and someone needs to sit down and see how things will work when you've got two different drivers providing the constraints. There's not actually been much demand for this since most people just use the standard audio rates (and a lot of devices aren't properly rated/supported at non-standard ones) but it'd definitely be good if you could implement it.
On Wed, Mar 10, 2010 at 04:48:56PM +0900, Jassi Brar wrote:
s3c64xx-i2s remove unncessary headers
Signed-off-by: Jassi Brar jassi.brar@samsung.com
Applied, thanks.
On Wed, Mar 10, 2010 at 04:48:55PM +0900, Jassi Brar wrote:
s3c-i2s-v2 remove unnecessary headers
Signed-off-by: Jassi Brar jassi.brar@samsung.com
Applied, thanks.
On Wed, Mar 10, 2010 at 04:48:54PM +0900, Jassi Brar wrote:
Rather than having the multiple definitions of the same clocks, define them in one common place and refer by SoC specific names.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
Applied, thanks.
On Wed, Mar 10, 2010 at 04:48:53PM +0900, Jassi Brar wrote:
The code in s3c-i2s-v2.c is compiled via Makefile and not implicitly by include'ing. Also, the driver developer anyways has to refer to the manual to see if the code can be reused for the SoC under consideration. That makes the S3C_IIS_V2_SUPPORTED retrospective rather than a check.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
This code is in there for pure defensiveness - since at least the S3C64xx and S3C24xx have slightly different register maps for the block (the ones that are currently handled are the IISMOD master settings, there may be more) the code needs to have explicit support for the processors. The goal of this block is to print a #error saying exactly what's going on rather than just have the build fail with missing defines in order to help make it more immediately clear to users what is going on if they try to select a processor combination they don't know how to handle.
Users often have a strong expectation that the driver will just work and so relying on them consulting the processor manual isn't always realistic.
It may be that these differences are due to some being IISv2 and some being IISv3 or something and could easily be handled via mandatory platform data but since none of the documentation refers to any of this stuff and it has to be reverse engineered we've got this system based on build time processor selection.
On Wed, Mar 10, 2010 at 7:49 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:48:53PM +0900, Jassi Brar wrote:
The code in s3c-i2s-v2.c is compiled via Makefile and not implicitly by include'ing. Also, the driver developer anyways has to refer to the manual to see if the code can be reused for the SoC under consideration. That makes the S3C_IIS_V2_SUPPORTED retrospective rather than a check.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
This code is in there for pure defensiveness - since at least the S3C64xx and S3C24xx have slightly different register maps for the block (the ones that are currently handled are the IISMOD master settings, there may be more) the code needs to have explicit support for the processors. The goal of this block is to print a #error saying exactly what's going on rather than just have the build fail with missing defines in order to help make it more immediately clear to users what is going on if they try to select a processor combination they don't know how to handle.
The way I see it, if we keep it we will have to add every supported SoC to the list which is kinda redundant since that information could already be extracted from Makefile. The list of supported SoC could be useful if we somehow knew the future SoCs' characteristics to tell if they wud work or not, but the developer has to first check the Manual and see if the s3c-i2s-v2.c could be made use of or not. Besides, I assume 'user' to be the MACHINE driver writer and 'developer' to be the CPU driver writer.
Users often have a strong expectation that the driver will just work and so relying on them consulting the processor manual isn't always realistic.
well, I am not sure if we should sympathize with such a developer :)
It may be that these differences are due to some being IISv2 and some being IISv3 or something and could easily be handled via mandatory platform data but since none of the documentation refers to any of this stuff and it has to be reverse engineered we've got this system based on build time processor selection.
Yes, maybe some more documentation is what is needed.
On Wed, Mar 10, 2010 at 09:38:16PM +0900, jassi brar wrote:
The way I see it, if we keep it we will have to add every supported SoC to the list
I agree that the current situation sucks. Like I say, I suspect that we can handle this better with platform data if we knew what the underlying changes in the blocks are but right now this is all being reverse engineered from the datasheets which don't explicitly mention any differences or similarities between the IIS controllers of the various CPUs that people happen to have been able to obtain datasheets for - you're in a much better position to be able to do something about this than most since you have access to Samsung internal information.
What I'm saying here is that while the driver needs to have per processor ifdefs I'd rather keep the matching plain text #error - if we can get those removed then this block could also go but it should happen in that order.
which is kinda redundant since that information could already be extracted from Makefile. The list of supported SoC could be useful if we somehow knew the future SoCs' characteristics to tell if they wud work or not, but the developer has to first check the Manual and see if the s3c-i2s-v2.c could be made use of or not. Besides, I assume 'user' to be the MACHINE driver writer and 'developer' to be the CPU driver writer.
Thing is that people writing machine drivers will frequently try building the driver if they're working on a SoC that isn't supported yet - they'll see a generic Samsung IIS controller driver and so (reasonably enough) try to use it. The error message is there to try to make it clearer to them that they either need to turn into a CPU driver developer and make sure support is OK or get someone else to do the required work.
On Wed, Mar 10, 2010 at 04:48:52PM +0900, Jassi Brar wrote:
For some CPU-CODEC and source clock combination we might need to set BCLK to N*Sample_size*LRCLK, where N may be even 3 or 4, not just 2.
We can simply remove the dependency of BCLK on sample size as there is already a callback(S3C_I2SV2_DIV_BCLK) available to set required BCLK.
I've applied this but I'd rather see the code changed so that the BCLK is set automatically by default and the explict divider configuration disables that. This way we get the best of both worlds - most users won't need to worry about the BCLK configuration but those that need to configure it can do so. An awful lot of users don't really understand audio hardware and find having to learn about which clock rates they need and so on a bit of a learning curve.
On Wed, Mar 10, 2010 at 9:31 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:48:52PM +0900, Jassi Brar wrote:
For some CPU-CODEC and source clock combination we might need to set BCLK to N*Sample_size*LRCLK, where N may be even 3 or 4, not just 2.
We can simply remove the dependency of BCLK on sample size as there is already a callback(S3C_I2SV2_DIV_BCLK) available to set required BCLK.
I've applied this but I'd rather see the code changed so that the BCLK is set automatically by default and the explict divider configuration disables that. This way we get the best of both worlds - most users won't need to worry about the BCLK configuration but those that need to configure it can do so. An awful lot of users don't really understand audio hardware and find having to learn about which clock rates they need and so on a bit of a learning curve.
I was thinking of calculating all register field values within the CPU driver and let MACHINE driver specify only general parameters of audio(as in many CODEC drivers). After this patch series, though.
On Wed, Mar 10, 2010 at 09:46:15PM +0900, jassi brar wrote:
I was thinking of calculating all register field values within the CPU driver and let MACHINE driver specify only general parameters of audio(as in many CODEC drivers). After this patch series, though.
Yes, like I say I've applied the patch as-is - this was just a suggestion for future work and it's good to hear that you're already planning to implement such improvements.
On Wed, Mar 10, 2010 at 04:48:51PM +0900, Jassi Brar wrote:
Towards generalizing CPU driver interface, do not accept direct field values for the BCLK and RCLK. The machine driver should simply request the FS-multiple and not provide the value to be set in divide field of IISMOD.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
This is good but it needs to also update all the machine drivers that use the direct register values. A brief grep through suggests that the uda134x machine and the OpenMoko machines need a look, though I didn't actually check which DAI driver they're all using. If nothing uses this feature then just note that in the changelog (if the feature is unused that's even more reason to remove it anyway).
On Wed, Mar 10, 2010 at 7:32 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:48:51PM +0900, Jassi Brar wrote:
Towards generalizing CPU driver interface, do not accept direct field values for the BCLK and RCLK. The machine driver should simply request the FS-multiple and not provide the value to be set in divide field of IISMOD.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
This is good but it needs to also update all the machine drivers that use the direct register values. A brief grep through suggests that the uda134x machine and the OpenMoko machines need a look, though I didn't actually check which DAI driver they're all using. If nothing uses this feature then just note that in the changelog (if the feature is unused that's even more reason to remove it anyway).
s3c-i2s-v2.c is shared only by s3c2412 and s3c64xx. smdk64xx_wm8580 and jive_wm8750 already use appropriate values. Will add the note in changelog.
On Wed, Mar 10, 2010 at 09:27:10PM +0900, jassi brar wrote:
s3c-i2s-v2.c is shared only by s3c2412 and s3c64xx. smdk64xx_wm8580 and jive_wm8750 already use appropriate values. Will add the note in changelog.
That's OK, I'll apply it and add the note myself - no need to resend. Thanks for checking.
On Wed, Mar 10, 2010 at 04:48:50PM +0900, Jassi Brar wrote:
Add missing semicolon after s3c2412_i2s_delay
Signed-off-by: Jassi Brar jassi.brar@samsung.com
Applied, thanks.
On Wed, Mar 10, 2010 at 04:48:50PM +0900, Jassi Brar wrote:
Add missing semicolon after s3c2412_i2s_delay
Signed-off-by: Jassi Brar jassi.brar@samsung.com
Applied, thanks.
participants (4)
-
Ben Dooks -
Jassi Brar -
jassi brar -
Mark Brown