diff mbox series

[v3,4/9] ASoC: sun4i-i2s: Fix offset mask

Message ID 20181221152110.17982-5-codekipper@gmail.com (mailing list archive)
State New, archived
Headers show
Series ASoC: sun4i-i2s: Updates to the driver | expand

Commit Message

Code Kipper Dec. 21, 2018, 3:21 p.m. UTC
From: Marcus Cooper <codekipper@gmail.com>

Also add offset to RX channel select

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

Comments

Chen-Yu Tsai Dec. 21, 2018, 4:54 p.m. UTC | #1
On Fri, Dec 21, 2018 at 11:21 PM <codekipper@gmail.com> wrote:
>
> From: Marcus Cooper <codekipper@gmail.com>
>
> Also add offset to RX channel select
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>

Commit log seems a bit lacking. You could probably explain how you
found this, either when comparing datasheet macros, or some actual
error manifested and you "heard" it. You could also state what the
fixed outcome should be.

Also please add a fixes tag.

The RX channel select offset should be added as a separate patch,
also with an explanation on how you found this missing, and a fixes
tag.

ChenYu

> ---
>  sound/soc/sunxi/sun4i-i2s.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index adb988ae9ac5..93a484d7e228 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -110,7 +110,7 @@
>
>  #define SUN8I_I2S_TX_CHAN_MAP_REG      0x44
>  #define SUN8I_I2S_TX_CHAN_SEL_REG      0x34
> -#define SUN8I_I2S_TX_CHAN_OFFSET_MASK          GENMASK(13, 11)
> +#define SUN8I_I2S_TX_CHAN_OFFSET_MASK          GENMASK(13, 12)
>  #define SUN8I_I2S_TX_CHAN_OFFSET(offset)       (offset << 12)
>  #define SUN8I_I2S_TX_CHAN_EN_MASK              GENMASK(11, 4)
>  #define SUN8I_I2S_TX_CHAN_EN(num_chan)         (((1 << num_chan) - 1) << 4)
> @@ -482,6 +482,10 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>                 regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>                                    SUN8I_I2S_TX_CHAN_OFFSET_MASK,
>                                    SUN8I_I2S_TX_CHAN_OFFSET(offset));
> +
> +               regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
> +                                  SUN8I_I2S_TX_CHAN_OFFSET_MASK,
> +                                  SUN8I_I2S_TX_CHAN_OFFSET(offset));
>         }
>
>         regmap_field_write(i2s->field_fmt_mode, val);
> --
> 2.20.1
>
Mark Brown Jan. 9, 2019, 6:50 p.m. UTC | #2
On Sat, Dec 22, 2018 at 12:54:48AM +0800, Chen-Yu Tsai wrote:
> On Fri, Dec 21, 2018 at 11:21 PM <codekipper@gmail.com> wrote:

> > Also add offset to RX channel select

> Commit log seems a bit lacking. You could probably explain how you
> found this, either when comparing datasheet macros, or some actual
> error manifested and you "heard" it. You could also state what the
> fixed outcome should be.

> Also please add a fixes tag.

Plus send this at the start of the series so it can be applied as a fix
and sent to Linus during the development cycle.
diff mbox series

Patch

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index adb988ae9ac5..93a484d7e228 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -110,7 +110,7 @@ 
 
 #define SUN8I_I2S_TX_CHAN_MAP_REG	0x44
 #define SUN8I_I2S_TX_CHAN_SEL_REG	0x34
-#define SUN8I_I2S_TX_CHAN_OFFSET_MASK		GENMASK(13, 11)
+#define SUN8I_I2S_TX_CHAN_OFFSET_MASK		GENMASK(13, 12)
 #define SUN8I_I2S_TX_CHAN_OFFSET(offset)	(offset << 12)
 #define SUN8I_I2S_TX_CHAN_EN_MASK		GENMASK(11, 4)
 #define SUN8I_I2S_TX_CHAN_EN(num_chan)		(((1 << num_chan) - 1) << 4)
@@ -482,6 +482,10 @@  static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
 				   SUN8I_I2S_TX_CHAN_OFFSET_MASK,
 				   SUN8I_I2S_TX_CHAN_OFFSET(offset));
+
+		regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
+				   SUN8I_I2S_TX_CHAN_OFFSET_MASK,
+				   SUN8I_I2S_TX_CHAN_OFFSET(offset));
 	}
 
 	regmap_field_write(i2s->field_fmt_mode, val);