[v5,15/15] ASoC: sun4i-i2s: Adjust regmap settings
diff mbox series

Message ID 20190814060854.26345-16-codekipper@gmail.com
State New
Headers show
Series
  • ASoC: sun4i-i2s: Updates to the driver
Related show

Commit Message

Code Kipper Aug. 14, 2019, 6:08 a.m. UTC
From: Marcus Cooper <codekipper@gmail.com>

Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
to reflect this.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

Comments

Maxime Ripard Aug. 14, 2019, 7:20 a.m. UTC | #1
On Wed, Aug 14, 2019 at 08:08:54AM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> to reflect this.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>

This patch looks like it's fixing something while the commit log
doesn't mention what is being fixed.

Having some context here would be great.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Jernej Škrabec Aug. 14, 2019, 8:37 a.m. UTC | #2
Hi!

Dne sreda, 14. avgust 2019 ob 08:08:54 CEST je codekipper@gmail.com 
napisal(a):
> From: Marcus Cooper <codekipper@gmail.com>
> 
> Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> to reflect this.
> 
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 31 ++++++++++---------------------
>  1 file changed, 10 insertions(+), 21 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index d3c8789f70bb..ecfc1ed79379 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -876,9 +876,11 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai,
> unsigned int fmt) static void sun4i_i2s_start_capture(struct sun4i_i2s
> *i2s)
>  {
>  	/* Flush RX FIFO */
> +	regcache_cache_bypass(i2s->regmap, true);
>  	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>  			   SUN4I_I2S_FIFO_CTRL_FLUSH_RX,
>  			   SUN4I_I2S_FIFO_CTRL_FLUSH_RX);
> +	regcache_cache_bypass(i2s->regmap, false);

Did you try with regmap_write_bits() instead? This function will 
unconditionally write bits so it's nicer solution, because you don't have to 
use regcache_cache_bypass().

> 
>  	/* Clear RX counter */
>  	regmap_write(i2s->regmap, SUN4I_I2S_RX_CNT_REG, 0);
> @@ -897,9 +899,11 @@ static void sun4i_i2s_start_capture(struct sun4i_i2s
> *i2s) static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
>  {
>  	/* Flush TX FIFO */
> +	regcache_cache_bypass(i2s->regmap, true);
>  	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>  			   SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
>  			   SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
> +	regcache_cache_bypass(i2s->regmap, false);

Ditto.

> 
>  	/* Clear TX counter */
>  	regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
> @@ -1053,13 +1057,7 @@ static const struct snd_soc_component_driver
> sun4i_i2s_component = {
> 
>  static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
>  {
> -	switch (reg) {
> -	case SUN4I_I2S_FIFO_TX_REG:
> -		return false;
> -
> -	default:
> -		return true;
> -	}
> +	return true;

Why did you change this? Manual mentions that SUN4I_I2S_FIFO_TX_REG is write-
only register. Even if it can be read, then it's better to remove whole 
function, which will automatically mean that all registers can be read.


>  }
> 
>  static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
> @@ -1078,6 +1076,8 @@ static bool sun4i_i2s_volatile_reg(struct device *dev,
> unsigned int reg) {
>  	switch (reg) {
>  	case SUN4I_I2S_FIFO_RX_REG:
> +	case SUN4I_I2S_FIFO_TX_REG:
> +	case SUN4I_I2S_FIFO_STA_REG:
>  	case SUN4I_I2S_INT_STA_REG:
>  	case SUN4I_I2S_RX_CNT_REG:
>  	case SUN4I_I2S_TX_CNT_REG:

SUN4I_I2S_FIFO_CTRL_REG should be put here, because it has two bits which 
returns to 0 immediately after they are set to 1.

Best regards,
Jernej

> @@ -1088,23 +1088,12 @@ static bool sun4i_i2s_volatile_reg(struct device
> *dev, unsigned int reg) }
>  }
> 
> -static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
> -{
> -	switch (reg) {
> -	case SUN8I_I2S_FIFO_TX_REG:
> -		return false;
> -
> -	default:
> -		return true;
> -	}
> -}
> -
>  static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>  {
>  	if (reg == SUN8I_I2S_INT_STA_REG)
>  		return true;
>  	if (reg == SUN8I_I2S_FIFO_TX_REG)
> -		return false;
> +		return true;
> 
>  	return sun4i_i2s_volatile_reg(dev, reg);
>  }
> @@ -1175,7 +1164,7 @@ static const struct regmap_config
> sun8i_i2s_regmap_config = { .reg_defaults	= sun8i_i2s_reg_defaults,
>  	.num_reg_defaults	= ARRAY_SIZE(sun8i_i2s_reg_defaults),
>  	.writeable_reg	= sun4i_i2s_wr_reg,
> -	.readable_reg	= sun8i_i2s_rd_reg,
> +	.readable_reg	= sun4i_i2s_rd_reg,
>  	.volatile_reg	= sun8i_i2s_volatile_reg,
>  };
> 
> @@ -1188,7 +1177,7 @@ static const struct regmap_config
> sun50i_i2s_regmap_config = { .reg_defaults	= sun50i_i2s_reg_defaults,
>  	.num_reg_defaults	= ARRAY_SIZE(sun50i_i2s_reg_defaults),
>  	.writeable_reg	= sun4i_i2s_wr_reg,
> -	.readable_reg	= sun8i_i2s_rd_reg,
> +	.readable_reg	= sun4i_i2s_rd_reg,
>  	.volatile_reg	= sun8i_i2s_volatile_reg,
>  };
Code Kipper Aug. 14, 2019, 9:02 a.m. UTC | #3
On Wed, 14 Aug 2019 at 10:38, Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Hi!
>
> Dne sreda, 14. avgust 2019 ob 08:08:54 CEST je codekipper@gmail.com
> napisal(a):
> > From: Marcus Cooper <codekipper@gmail.com>
> >
> > Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> > to reflect this.
> >
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> > ---
> >  sound/soc/sunxi/sun4i-i2s.c | 31 ++++++++++---------------------
> >  1 file changed, 10 insertions(+), 21 deletions(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index d3c8789f70bb..ecfc1ed79379 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -876,9 +876,11 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai,
> > unsigned int fmt) static void sun4i_i2s_start_capture(struct sun4i_i2s
> > *i2s)
> >  {
> >       /* Flush RX FIFO */
> > +     regcache_cache_bypass(i2s->regmap, true);
> >       regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> >                          SUN4I_I2S_FIFO_CTRL_FLUSH_RX,
> >                          SUN4I_I2S_FIFO_CTRL_FLUSH_RX);
> > +     regcache_cache_bypass(i2s->regmap, false);
>
> Did you try with regmap_write_bits() instead? This function will
> unconditionally write bits so it's nicer solution, because you don't have to
> use regcache_cache_bypass().

I didn't....with all the rework I've avoided messing with this change.
Now that the dust
has settled, I can go back to look at this.
Thanks,
CK
>
> >
> >       /* Clear RX counter */
> >       regmap_write(i2s->regmap, SUN4I_I2S_RX_CNT_REG, 0);
> > @@ -897,9 +899,11 @@ static void sun4i_i2s_start_capture(struct sun4i_i2s
> > *i2s) static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
> >  {
> >       /* Flush TX FIFO */
> > +     regcache_cache_bypass(i2s->regmap, true);
> >       regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> >                          SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
> >                          SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
> > +     regcache_cache_bypass(i2s->regmap, false);
>
> Ditto.
>
> >
> >       /* Clear TX counter */
> >       regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
> > @@ -1053,13 +1057,7 @@ static const struct snd_soc_component_driver
> > sun4i_i2s_component = {
> >
> >  static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
> >  {
> > -     switch (reg) {
> > -     case SUN4I_I2S_FIFO_TX_REG:
> > -             return false;
> > -
> > -     default:
> > -             return true;
> > -     }
> > +     return true;
>
> Why did you change this? Manual mentions that SUN4I_I2S_FIFO_TX_REG is write-
> only register. Even if it can be read, then it's better to remove whole
> function, which will automatically mean that all registers can be read.
>
>
> >  }
> >
> >  static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
> > @@ -1078,6 +1076,8 @@ static bool sun4i_i2s_volatile_reg(struct device *dev,
> > unsigned int reg) {
> >       switch (reg) {
> >       case SUN4I_I2S_FIFO_RX_REG:
> > +     case SUN4I_I2S_FIFO_TX_REG:
> > +     case SUN4I_I2S_FIFO_STA_REG:
> >       case SUN4I_I2S_INT_STA_REG:
> >       case SUN4I_I2S_RX_CNT_REG:
> >       case SUN4I_I2S_TX_CNT_REG:
>
> SUN4I_I2S_FIFO_CTRL_REG should be put here, because it has two bits which
> returns to 0 immediately after they are set to 1.
>
> Best regards,
> Jernej
>
> > @@ -1088,23 +1088,12 @@ static bool sun4i_i2s_volatile_reg(struct device
> > *dev, unsigned int reg) }
> >  }
> >
> > -static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
> > -{
> > -     switch (reg) {
> > -     case SUN8I_I2S_FIFO_TX_REG:
> > -             return false;
> > -
> > -     default:
> > -             return true;
> > -     }
> > -}
> > -
> >  static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> >  {
> >       if (reg == SUN8I_I2S_INT_STA_REG)
> >               return true;
> >       if (reg == SUN8I_I2S_FIFO_TX_REG)
> > -             return false;
> > +             return true;
> >
> >       return sun4i_i2s_volatile_reg(dev, reg);
> >  }
> > @@ -1175,7 +1164,7 @@ static const struct regmap_config
> > sun8i_i2s_regmap_config = { .reg_defaults     = sun8i_i2s_reg_defaults,
> >       .num_reg_defaults       = ARRAY_SIZE(sun8i_i2s_reg_defaults),
> >       .writeable_reg  = sun4i_i2s_wr_reg,
> > -     .readable_reg   = sun8i_i2s_rd_reg,
> > +     .readable_reg   = sun4i_i2s_rd_reg,
> >       .volatile_reg   = sun8i_i2s_volatile_reg,
> >  };
> >
> > @@ -1188,7 +1177,7 @@ static const struct regmap_config
> > sun50i_i2s_regmap_config = { .reg_defaults    = sun50i_i2s_reg_defaults,
> >       .num_reg_defaults       = ARRAY_SIZE(sun50i_i2s_reg_defaults),
> >       .writeable_reg  = sun4i_i2s_wr_reg,
> > -     .readable_reg   = sun8i_i2s_rd_reg,
> > +     .readable_reg   = sun4i_i2s_rd_reg,
> >       .volatile_reg   = sun8i_i2s_volatile_reg,
> >  };
>
>
>
>
Jernej Škrabec Aug. 14, 2019, 11:31 a.m. UTC | #4
Dne sreda, 14. avgust 2019 ob 09:20:07 CEST je Maxime Ripard napisal(a):
> On Wed, Aug 14, 2019 at 08:08:54AM +0200, codekipper@gmail.com wrote:
> > From: Marcus Cooper <codekipper@gmail.com>
> > 
> > Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> > to reflect this.
> > 
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> 
> This patch looks like it's fixing something while the commit log
> doesn't mention what is being fixed.

Main issue addressed here is that SUN4I_I2S_FIFO_CTRL_REG has two self-clear 
registers (SUN4I_I2S_FIFO_CTRL_FLUSH_RX and SUN4I_I2S_FIFO_CTRL_FLUSH_TX) and 
thus it should be marked as volatile.

Best regards,
Jernej

> 
> Having some context here would be great.
> 
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Chen-Yu Tsai Aug. 14, 2019, 11:31 a.m. UTC | #5
On Wed, Aug 14, 2019 at 2:09 PM <codekipper@gmail.com> wrote:
>
> From: Marcus Cooper <codekipper@gmail.com>
>
> Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> to reflect this.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 31 ++++++++++---------------------
>  1 file changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index d3c8789f70bb..ecfc1ed79379 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -876,9 +876,11 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
>  {
>         /* Flush RX FIFO */
> +       regcache_cache_bypass(i2s->regmap, true);
>         regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>                            SUN4I_I2S_FIFO_CTRL_FLUSH_RX,
>                            SUN4I_I2S_FIFO_CTRL_FLUSH_RX);
> +       regcache_cache_bypass(i2s->regmap, false);
>
>         /* Clear RX counter */
>         regmap_write(i2s->regmap, SUN4I_I2S_RX_CNT_REG, 0);
> @@ -897,9 +899,11 @@ static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
>  static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
>  {
>         /* Flush TX FIFO */
> +       regcache_cache_bypass(i2s->regmap, true);
>         regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>                            SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
>                            SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
> +       regcache_cache_bypass(i2s->regmap, false);
>
>         /* Clear TX counter */
>         regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
> @@ -1053,13 +1057,7 @@ static const struct snd_soc_component_driver sun4i_i2s_component = {
>
>  static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
>  {
> -       switch (reg) {
> -       case SUN4I_I2S_FIFO_TX_REG:
> -               return false;
> -
> -       default:
> -               return true;
> -       }
> +       return true;

The commit log needs to explain why this is relevant. And I'm not sure why one
would read back the TX FIFO. Also, if it's always true, just drop the callback.

ChenYu

>  }
>
>  static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
> @@ -1078,6 +1076,8 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>  {
>         switch (reg) {
>         case SUN4I_I2S_FIFO_RX_REG:
> +       case SUN4I_I2S_FIFO_TX_REG:
> +       case SUN4I_I2S_FIFO_STA_REG:
>         case SUN4I_I2S_INT_STA_REG:
>         case SUN4I_I2S_RX_CNT_REG:
>         case SUN4I_I2S_TX_CNT_REG:
> @@ -1088,23 +1088,12 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>         }
>  }
>
> -static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
> -{
> -       switch (reg) {
> -       case SUN8I_I2S_FIFO_TX_REG:
> -               return false;
> -
> -       default:
> -               return true;
> -       }
> -}
> -
>  static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>  {
>         if (reg == SUN8I_I2S_INT_STA_REG)
>                 return true;
>         if (reg == SUN8I_I2S_FIFO_TX_REG)
> -               return false;
> +               return true;
>
>         return sun4i_i2s_volatile_reg(dev, reg);
>  }
> @@ -1175,7 +1164,7 @@ static const struct regmap_config sun8i_i2s_regmap_config = {
>         .reg_defaults   = sun8i_i2s_reg_defaults,
>         .num_reg_defaults       = ARRAY_SIZE(sun8i_i2s_reg_defaults),
>         .writeable_reg  = sun4i_i2s_wr_reg,
> -       .readable_reg   = sun8i_i2s_rd_reg,
> +       .readable_reg   = sun4i_i2s_rd_reg,
>         .volatile_reg   = sun8i_i2s_volatile_reg,
>  };
>
> @@ -1188,7 +1177,7 @@ static const struct regmap_config sun50i_i2s_regmap_config = {
>         .reg_defaults   = sun50i_i2s_reg_defaults,
>         .num_reg_defaults       = ARRAY_SIZE(sun50i_i2s_reg_defaults),
>         .writeable_reg  = sun4i_i2s_wr_reg,
> -       .readable_reg   = sun8i_i2s_rd_reg,
> +       .readable_reg   = sun4i_i2s_rd_reg,
>         .volatile_reg   = sun8i_i2s_volatile_reg,
>  };
>
> --
> 2.22.0
>

Patch
diff mbox series

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index d3c8789f70bb..ecfc1ed79379 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -876,9 +876,11 @@  static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
 {
 	/* Flush RX FIFO */
+	regcache_cache_bypass(i2s->regmap, true);
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
 			   SUN4I_I2S_FIFO_CTRL_FLUSH_RX,
 			   SUN4I_I2S_FIFO_CTRL_FLUSH_RX);
+	regcache_cache_bypass(i2s->regmap, false);
 
 	/* Clear RX counter */
 	regmap_write(i2s->regmap, SUN4I_I2S_RX_CNT_REG, 0);
@@ -897,9 +899,11 @@  static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
 static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
 {
 	/* Flush TX FIFO */
+	regcache_cache_bypass(i2s->regmap, true);
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
 			   SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
 			   SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
+	regcache_cache_bypass(i2s->regmap, false);
 
 	/* Clear TX counter */
 	regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
@@ -1053,13 +1057,7 @@  static const struct snd_soc_component_driver sun4i_i2s_component = {
 
 static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
 {
-	switch (reg) {
-	case SUN4I_I2S_FIFO_TX_REG:
-		return false;
-
-	default:
-		return true;
-	}
+	return true;
 }
 
 static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
@@ -1078,6 +1076,8 @@  static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
 	case SUN4I_I2S_FIFO_RX_REG:
+	case SUN4I_I2S_FIFO_TX_REG:
+	case SUN4I_I2S_FIFO_STA_REG:
 	case SUN4I_I2S_INT_STA_REG:
 	case SUN4I_I2S_RX_CNT_REG:
 	case SUN4I_I2S_TX_CNT_REG:
@@ -1088,23 +1088,12 @@  static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
 	}
 }
 
-static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
-{
-	switch (reg) {
-	case SUN8I_I2S_FIFO_TX_REG:
-		return false;
-
-	default:
-		return true;
-	}
-}
-
 static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
 {
 	if (reg == SUN8I_I2S_INT_STA_REG)
 		return true;
 	if (reg == SUN8I_I2S_FIFO_TX_REG)
-		return false;
+		return true;
 
 	return sun4i_i2s_volatile_reg(dev, reg);
 }
@@ -1175,7 +1164,7 @@  static const struct regmap_config sun8i_i2s_regmap_config = {
 	.reg_defaults	= sun8i_i2s_reg_defaults,
 	.num_reg_defaults	= ARRAY_SIZE(sun8i_i2s_reg_defaults),
 	.writeable_reg	= sun4i_i2s_wr_reg,
-	.readable_reg	= sun8i_i2s_rd_reg,
+	.readable_reg	= sun4i_i2s_rd_reg,
 	.volatile_reg	= sun8i_i2s_volatile_reg,
 };
 
@@ -1188,7 +1177,7 @@  static const struct regmap_config sun50i_i2s_regmap_config = {
 	.reg_defaults	= sun50i_i2s_reg_defaults,
 	.num_reg_defaults	= ARRAY_SIZE(sun50i_i2s_reg_defaults),
 	.writeable_reg	= sun4i_i2s_wr_reg,
-	.readable_reg	= sun8i_i2s_rd_reg,
+	.readable_reg	= sun4i_i2s_rd_reg,
 	.volatile_reg	= sun8i_i2s_volatile_reg,
 };