[alsa-devel] [PATCH 3.1]ASoC: ad193x: add spi_hw_read, fix sysclk and register definition
asoc cache layer can't support this kind of spi registers, so bypass cache and read regiters directly
Signed-off-by: Scott Jiang scott.jiang.linux@gmail.com --- sound/soc/blackfin/bf5xx-ad193x.c | 2 +- sound/soc/codecs/ad193x.c | 8 ++++++-- sound/soc/codecs/ad193x.h | 5 +++-- sound/soc/soc-io.c | 23 +++++++++++++++++++++++ 4 files changed, 33 insertions(+), 5 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; }
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index 2374ca5..1447cc8 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); @@ -361,7 +362,10 @@ static int ad193x_probe(struct snd_soc_codec *codec) dev_err(codec->dev, "failed to set cache I/O: %d\n", ret); return ret; } - +#if defined(CONFIG_SPI_MASTER) + /* asoc cache layer can't support this kind of spi registers now */ + codec->cache_bypass = 1; +#endif /* default setting for ad193x */
/* unmute dac channels */ diff --git a/sound/soc/codecs/ad193x.h b/sound/soc/codecs/ad193x.h index 9747b54..cccc2e8 100644 --- a/sound/soc/codecs/ad193x.h +++ b/sound/soc/codecs/ad193x.h @@ -34,7 +34,8 @@ #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_SHFT 3 +#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 +64,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 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,
On Thu, 2011-08-11 at 17:04 +0800, Scott Jiang wrote:
@@ -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;
Whatever this is for it should be a separate change.
- 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);
This needs some documentation as it's not clear what it's supposed to do.
+#if defined(CONFIG_SPI_MASTER)
- /* asoc cache layer can't support this kind of spi registers now */
- codec->cache_bypass = 1;
+#endif
That's not what cache_bypass is for. Just mark the registers as volatile, or remove the cache entirely.
#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;
So, if we've got an ifdef on the write...
2011/8/11 Mark Brown broonie@opensource.wolfsonmicro.com:
On Thu, 2011-08-11 at 17:04 +0800, Scott Jiang wrote:
@@ -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; }
Whatever this is for it should be a separate change.
- 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);
This needs some documentation as it's not clear what it's supposed to do.
the word_len value forgot to shift, I think it's clear.
+#if defined(CONFIG_SPI_MASTER)
- /* asoc cache layer can't support this kind of spi registers now */
- codec->cache_bypass = 1;
+#endif
That's not what cache_bypass is for. Just mark the registers as volatile, or remove the cache entirely.
volatile usually used on the read-only registers, so I think removing cache is better.
#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;
So, if we've got an ifdef on the write...
sorry, I can't understand what you mean. I just follow i2c code above.
On Thu, 2011-08-11 at 17:52 +0800, Scott Jiang wrote:
2011/8/11 Mark Brown broonie@opensource.wolfsonmicro.com:
On Thu, 2011-08-11 at 17:04 +0800, Scott Jiang wrote:
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);
This needs some documentation as it's not clear what it's supposed to do.
the word_len value forgot to shift, I think it's clear.
This is an unrelated change and should be split out - the major reason it's confusing is that it's got nothing to do with the rest of the change.
+#if defined(CONFIG_SPI_MASTER)
/* asoc cache layer can't support this kind of spi registers now */codec->cache_bypass = 1;+#endif
That's not what cache_bypass is for. Just mark the registers as volatile, or remove the cache entirely.
volatile usually used on the read-only registers, so I think removing cache is better.
No, not at all. All volatile means is that a register can't be cached. For example, many interrupt status registers are write to clear so they're volatile because the device can change them but we still need to write to clear them.
participants (2)
-
Mark Brown -
Scott Jiang