[alsa-devel] [PATCH v2 1/5] ASoC: ad193x: fix registers definition
fix dac word len mask and adc tdm fmt shift value
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com --- sound/soc/codecs/ad193x.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/ad193x.h b/sound/soc/codecs/ad193x.h index 9747b54..c1029d2 100644 --- a/sound/soc/codecs/ad193x.h +++ b/sound/soc/codecs/ad193x.h @@ -34,7 +34,7 @@ #define AD193X_DAC_LEFT_HIGH (1 << 3) #define AD193X_DAC_BCLK_INV (1 << 7) #define AD193X_DAC_CTRL2 0x804 -#define AD193X_DAC_WORD_LEN_MASK 0xC +#define AD193X_DAC_WORD_LEN_MASK 0x18 #define AD193X_DAC_MASTER_MUTE 1 #define AD193X_DAC_CHNL_MUTE 0x805 #define AD193X_DACL1_MUTE 0 @@ -63,7 +63,7 @@ #define AD193X_ADC_CTRL1 0x80f #define AD193X_ADC_SERFMT_MASK 0x60 #define AD193X_ADC_SERFMT_STEREO (0 << 5) -#define AD193X_ADC_SERFMT_TDM (1 << 2) +#define AD193X_ADC_SERFMT_TDM (1 << 5) #define AD193X_ADC_SERFMT_AUX (2 << 5) #define AD193X_ADC_WORD_LEN_MASK 0x3 #define AD193X_ADC_CTRL2 0x810
Am Freitag, den 12.08.2011, 18:04 -0400 schrieb Scott Jiang:
[…]
Your clock is not set correctly so your patches are from the future.
Thanks,
Paul
dac word len value should left shift before setting
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com --- sound/soc/codecs/ad193x.c | 3 ++- sound/soc/codecs/ad193x.h | 1 + 2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index 2374ca5..f1a8be5 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -307,7 +307,8 @@ static int ad193x_hw_params(struct snd_pcm_substream *substream, snd_soc_write(codec, AD193X_PLL_CLK_CTRL0, reg);
reg = snd_soc_read(codec, AD193X_DAC_CTRL2); - reg = (reg & (~AD193X_DAC_WORD_LEN_MASK)) | word_len; + reg = (reg & (~AD193X_DAC_WORD_LEN_MASK)) + | (word_len << AD193X_DAC_WORD_LEN_SHFT); snd_soc_write(codec, AD193X_DAC_CTRL2, reg);
reg = snd_soc_read(codec, AD193X_ADC_CTRL1); diff --git a/sound/soc/codecs/ad193x.h b/sound/soc/codecs/ad193x.h index c1029d2..cccc2e8 100644 --- a/sound/soc/codecs/ad193x.h +++ b/sound/soc/codecs/ad193x.h @@ -34,6 +34,7 @@ #define AD193X_DAC_LEFT_HIGH (1 << 3) #define AD193X_DAC_BCLK_INV (1 << 7) #define AD193X_DAC_CTRL2 0x804 +#define AD193X_DAC_WORD_LEN_SHFT 3 #define AD193X_DAC_WORD_LEN_MASK 0x18 #define AD193X_DAC_MASTER_MUTE 1 #define AD193X_DAC_CHNL_MUTE 0x805
2011/8/13 Scott Jiang scott.jiang.linux@gmail.com:
dac word len value should left shift before setting
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com
Acked-by: Barry Song 21cnbao@gmail.com
if you can move AD193X_DAC_WORD_LEN_MASK from your another patch to this, it is better.
sound/soc/codecs/ad193x.c | 3 ++- sound/soc/codecs/ad193x.h | 1 + 2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index 2374ca5..f1a8be5 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -307,7 +307,8 @@ static int ad193x_hw_params(struct snd_pcm_substream *substream, snd_soc_write(codec, AD193X_PLL_CLK_CTRL0, reg);
reg = snd_soc_read(codec, AD193X_DAC_CTRL2);
- reg = (reg & (~AD193X_DAC_WORD_LEN_MASK)) | word_len;
- reg = (reg & (~AD193X_DAC_WORD_LEN_MASK))
- | (word_len << AD193X_DAC_WORD_LEN_SHFT);
snd_soc_write(codec, AD193X_DAC_CTRL2, reg);
reg = snd_soc_read(codec, AD193X_ADC_CTRL1); diff --git a/sound/soc/codecs/ad193x.h b/sound/soc/codecs/ad193x.h index c1029d2..cccc2e8 100644 --- a/sound/soc/codecs/ad193x.h +++ b/sound/soc/codecs/ad193x.h @@ -34,6 +34,7 @@ #define AD193X_DAC_LEFT_HIGH (1 << 3) #define AD193X_DAC_BCLK_INV (1 << 7) #define AD193X_DAC_CTRL2 0x804 +#define AD193X_DAC_WORD_LEN_SHFT 3 #define AD193X_DAC_WORD_LEN_MASK 0x18 #define AD193X_DAC_MASTER_MUTE 1 #define AD193X_DAC_CHNL_MUTE 0x805 -- 1.7.0.4
On Fri, Aug 12, 2011 at 06:04:11PM -0400, Scott Jiang wrote:
dac word len value should left shift before setting
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com
Applied, thanks.
system clock is 24.576MHz instead of 12.288MHz
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com --- sound/soc/blackfin/bf5xx-ad193x.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/blackfin/bf5xx-ad193x.c b/sound/soc/blackfin/bf5xx-ad193x.c index d6651c0..a118a0f 100644 --- a/sound/soc/blackfin/bf5xx-ad193x.c +++ b/sound/soc/blackfin/bf5xx-ad193x.c @@ -56,7 +56,7 @@ static int bf5xx_ad193x_hw_params(struct snd_pcm_substream *substream,
switch (params_rate(params)) { case 48000: - clk = 12288000; + clk = 24576000; break; }
On Fri, Aug 12, 2011 at 06:04:12PM -0400, Scott Jiang wrote:
system clock is 24.576MHz instead of 12.288MHz
sound/soc/blackfin/bf5xx-ad193x.c | 2 +-
This is not a patch for the ad193x driver at all, this is a patch to one of the Blackfin machine drivers. Please make more effort with your changelogs all round, I've applied this time but it took a bit of time to work out what you're trying to do here.
Looking at the code I suspect you want a default case in this switch statement which returns an error as nothing else really makes sense - the current code just looks broken.
some spi registers are 7bits global address + 1 bit r/w + 8 bits register address. soc cache layer can't support this kind well. so let codec driver read registers directly.
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com --- sound/soc/soc-io.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c index cca490c..a62f7dd 100644 --- a/sound/soc/soc-io.c +++ b/sound/soc/soc-io.c @@ -205,6 +205,25 @@ static unsigned int snd_soc_16_8_read_i2c(struct snd_soc_codec *codec, #define snd_soc_16_8_read_i2c NULL #endif
+#if defined(CONFIG_SPI_MASTER) +static unsigned int snd_soc_16_8_read_spi(struct snd_soc_codec *codec, + unsigned int r) +{ + struct spi_device *spi = codec->control_data; + + const u16 reg = cpu_to_be16(r | 0x100); + u8 data; + int ret; + + ret = spi_write_then_read(spi, ®, 2, &data, 1); + if (ret < 0) + return 0; + return data; +} +#else +#define snd_soc_16_8_read_spi NULL +#endif + static int snd_soc_16_8_write(struct snd_soc_codec *codec, unsigned int reg, unsigned int value) { @@ -295,6 +314,7 @@ static struct { int (*write)(struct snd_soc_codec *codec, unsigned int, unsigned int); unsigned int (*read)(struct snd_soc_codec *, unsigned int); unsigned int (*i2c_read)(struct snd_soc_codec *, unsigned int); + unsigned int (*spi_read)(struct snd_soc_codec *, unsigned int); } io_types[] = { { .addr_bits = 4, .data_bits = 12, @@ -318,6 +338,7 @@ static struct { .addr_bits = 16, .data_bits = 8, .write = snd_soc_16_8_write, .i2c_read = snd_soc_16_8_read_i2c, + .spi_read = snd_soc_16_8_read_spi, }, { .addr_bits = 16, .data_bits = 16, @@ -383,6 +404,8 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec, #ifdef CONFIG_SPI_MASTER codec->hw_write = do_spi_write; #endif + if (io_types[i].spi_read) + codec->hw_read = io_types[i].spi_read;
codec->control_data = container_of(codec->dev, struct spi_device,
2011/8/13 Scott Jiang scott.jiang.linux@gmail.com:
some spi registers are 7bits global address + 1 bit r/w + 8 bits register address. soc cache layer can't support this kind well. so let codec driver read registers directly.
i don't think your this document has any relationship with your patch. it is just making confusion. And who is the user of your this new API?
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com
sound/soc/soc-io.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c index cca490c..a62f7dd 100644 --- a/sound/soc/soc-io.c +++ b/sound/soc/soc-io.c @@ -205,6 +205,25 @@ static unsigned int snd_soc_16_8_read_i2c(struct snd_soc_codec *codec, #define snd_soc_16_8_read_i2c NULL #endif
+#if defined(CONFIG_SPI_MASTER) +static unsigned int snd_soc_16_8_read_spi(struct snd_soc_codec *codec,
- unsigned int r)
+{
- struct spi_device *spi = codec->control_data;
- const u16 reg = cpu_to_be16(r | 0x100);
- u8 data;
- int ret;
- ret = spi_write_then_read(spi, ®, 2, &data, 1);
- if (ret < 0)
- return 0;
- return data;
+} +#else +#define snd_soc_16_8_read_spi NULL +#endif
static int snd_soc_16_8_write(struct snd_soc_codec *codec, unsigned int reg, unsigned int value) { @@ -295,6 +314,7 @@ static struct { int (*write)(struct snd_soc_codec *codec, unsigned int, unsigned int); unsigned int (*read)(struct snd_soc_codec *, unsigned int); unsigned int (*i2c_read)(struct snd_soc_codec *, unsigned int);
- unsigned int (*spi_read)(struct snd_soc_codec *, unsigned int);
} io_types[] = { { .addr_bits = 4, .data_bits = 12, @@ -318,6 +338,7 @@ static struct { .addr_bits = 16, .data_bits = 8, .write = snd_soc_16_8_write, .i2c_read = snd_soc_16_8_read_i2c,
- .spi_read = snd_soc_16_8_read_spi,
}, { .addr_bits = 16, .data_bits = 16, @@ -383,6 +404,8 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec, #ifdef CONFIG_SPI_MASTER codec->hw_write = do_spi_write; #endif
- if (io_types[i].spi_read)
- codec->hw_read = io_types[i].spi_read;
codec->control_data = container_of(codec->dev, struct spi_device, -- 1.7.0.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
2011/8/15 Barry Song 21cnbao@gmail.com:
2011/8/13 Scott Jiang scott.jiang.linux@gmail.com:
some spi registers are 7bits global address + 1 bit r/w + 8 bits register address. soc cache layer can't support this kind well. so let codec driver read registers directly.
i don't think your this document has any relationship with your patch. it is just making confusion. And who is the user of your this new API?
some spi codecs such as ad1938 can't be supported by cache layer. so they need spi hw read function. This patch add this function for 16bits addr 8 bits data mode spi transfer.
On Fri, Aug 12, 2011 at 06:04:13PM -0400, Scott Jiang wrote:
some spi registers are 7bits global address + 1 bit r/w + 8 bits register address. soc cache layer can't support this kind well. so let codec driver read registers directly.
I have to agree with Barry, this changelog really doesn't explain what you're doing here at all clearly. You're adding 8, 16 read, that's it. This could also be used for reading volatile registers like interrupt status registers.
Stil, I've applied.
asoc cache layer can't support this kind of spi registers well. remove cache support and read/write registers directly
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com --- sound/soc/codecs/ad193x.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index f1a8be5..eedb6f5 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -27,11 +27,6 @@ struct ad193x_priv { int sysclk; };
-/* ad193x register cache & default register settings */ -static const u8 ad193x_reg[AD193X_NUM_REGS] = { - 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, -}; - /* * AD193X volume/mute/de-emphasis etc. controls */ @@ -390,9 +385,6 @@ static int ad193x_probe(struct snd_soc_codec *codec)
static struct snd_soc_codec_driver soc_codec_dev_ad193x = { .probe = ad193x_probe, - .reg_cache_default = ad193x_reg, - .reg_cache_size = AD193X_NUM_REGS, - .reg_word_size = sizeof(u16), };
#if defined(CONFIG_SPI_MASTER)
On Fri, Aug 12, 2011 at 06:04:14PM -0400, Scott Jiang wrote:
asoc cache layer can't support this kind of spi registers well. remove cache support and read/write registers directly
Applied, thanks.
On Fri, Aug 12, 2011 at 06:04:10PM -0400, Scott Jiang wrote:
fix dac word len mask and adc tdm fmt shift value
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com
Applied, thanks.
participants (4)
-
Barry Song -
Mark Brown -
Paul Menzel -
Scott Jiang