Re: [PATCH v2 1/5] ASoC: ad193x: fix registers definition

Re: [PATCH v2 1/5] ASoC: ad193x: fix registers definition

[Paul Menzel]

[-- Attachment #1.1: Type: text/plain, Size: 164 bytes --]

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

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2 1/5] ASoC: ad193x: fix registers definition

[Scott Jiang]

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
-- 
1.7.0.4

[PATCH v2 2/5] ASoC: ad193x: fix dac word len setting

[Scott Jiang]

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
-- 
1.7.0.4

[PATCH v2 3/5] ASoC: ad193x: fix system clock

[Scott Jiang]

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;
 	}
 
-- 
1.7.0.4

[PATCH v2 4/5] ASoC: add spi hw read function for 16 addr 8 data mode

[Scott Jiang]

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, &reg, 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

[PATCH v2 5/5] ASoC: ad193x: remove cache support

[Scott Jiang]

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)
-- 
1.7.0.4

Re: [PATCH v2 2/5] ASoC: ad193x: fix dac word len setting

[Barry Song]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 4/5] ASoC: add spi hw read function for 16 addr 8 data mode

[Barry Song]

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, &reg, 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 4/5] ASoC: add spi hw read function for 16 addr 8 data mode

[Scott Jiang]

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.

Re: [PATCH v2 1/5] ASoC: ad193x: fix registers definition

[Mark Brown]

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.

Re: [PATCH v2 2/5] ASoC: ad193x: fix dac word len setting

[Mark Brown]

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.

Re: [PATCH v2 3/5] ASoC: ad193x: fix system clock

[Mark Brown]

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.

Re: [PATCH v2 4/5] ASoC: add spi hw read function for 16 addr 8 data mode

[Mark Brown]

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.

Re: [PATCH v2 5/5] ASoC: ad193x: remove cache support

[Mark Brown]

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.