[PATCH 0/3] ASoC: Add I2S support for Allwinner H3 SoCs

[PATCH 0/3] ASoC: Add I2S support for Allwinner H3 SoCs

[codekipper-Re5JQEeQqe8AvxtiuMwx3w]

From: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi All,
please find attached a series of patches to bring i2s support to the
Allwinner H3 SoC. This has been tested with the following setups:

A20 Olimex EVB connected to a pcm5102
Orange Pi 2 connected to a uda1380
Orange Pi 2 hdmi audio playback
Pine 64 connected to the audio DAC board

To get i2s working some additional patches are required which will be
delivered later. For now they have been pushed here

https://github.com/codekipper/linux-sunxi/commits/sunxi-audio-h3

Thanks in advance,
CK

Marcus Cooper (3):
  ASoC: sun4i-i2s: Add more quirks for newer SoCs
  ASoC: sun4i-i2s: Get startup to call set_fmt
  ASoC: sun4i-i2s: Add support for H3

 .../devicetree/bindings/sound/sun4i-i2s.txt        |   2 +
 sound/soc/sunxi/sun4i-i2s.c                        | 391 ++++++++++++++++++++-
 2 files changed, 376 insertions(+), 17 deletions(-)

-- 
2.13.2

[PATCH 1/3] ASoC: sun4i-i2s: Add more quirks for newer SoCs

[codekipper-Re5JQEeQqe8AvxtiuMwx3w]

From: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The Allwinner H3 has some differences to its I2S block such as a
different address for the TXFIFO and various register changes.

This patch prepares the driver for these changes.

Signed-off-by: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/sunxi/sun4i-i2s.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 3635bbc72cbc..38ab0144f897 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -544,7 +544,6 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = {
 		.rates = SNDRV_PCM_RATE_8000_192000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	},
-	.ops = &sun4i_i2s_dai_ops,
 	.symmetric_rates = 1,
 };
 
@@ -655,15 +654,24 @@ static int sun4i_i2s_runtime_suspend(struct device *dev)
 }
 
 struct sun4i_i2s_quirks {
-	bool has_reset;
+	bool				has_reset;
+	unsigned int			reg_offset_txdata;	/* TX FIFO */
+	const struct regmap_config	*sun4i_i2s_regmap;
+	const struct snd_soc_dai_ops	*ops;
 };
 
 static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = {
-	.has_reset	= false,
+	.has_reset		= false,
+	.reg_offset_txdata	= SUN4I_I2S_FIFO_TX_REG,
+	.sun4i_i2s_regmap	= &sun4i_i2s_regmap_config,
+	.ops			= &sun4i_i2s_dai_ops,
 };
 
 static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
-	.has_reset	= true,
+	.has_reset		= true,
+	.reg_offset_txdata	= SUN4I_I2S_FIFO_TX_REG,
+	.sun4i_i2s_regmap	= &sun4i_i2s_regmap_config,
+	.ops			= &sun4i_i2s_dai_ops,
 };
 
 static int sun4i_i2s_probe(struct platform_device *pdev)
@@ -702,8 +710,14 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 		return PTR_ERR(i2s->bus_clk);
 	}
 
+	quirks = of_device_get_match_data(&pdev->dev);
+	if (quirks == NULL) {
+		dev_err(&pdev->dev, "Failed to determine the quirks to use\n");
+		return -ENODEV;
+	}
+
 	i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
-					    &sun4i_i2s_regmap_config);
+					    quirks->sun4i_i2s_regmap);
 	if (IS_ERR(i2s->regmap)) {
 		dev_err(&pdev->dev, "Regmap initialisation failed\n");
 		return PTR_ERR(i2s->regmap);
@@ -732,7 +746,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 		}
 	}
 
-	i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG;
+	i2s->playback_dma_data.addr = res->start + quirks->reg_offset_txdata;
 	i2s->playback_dma_data.maxburst = 8;
 
 	i2s->capture_dma_data.addr = res->start + SUN4I_I2S_FIFO_RX_REG;
@@ -745,6 +759,8 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 			goto err_pm_disable;
 	}
 
+	/* Register ops with dai */
+	sun4i_i2s_dai.ops = quirks->ops;
 	ret = devm_snd_soc_register_component(&pdev->dev,
 					      &sun4i_i2s_component,
 					      &sun4i_i2s_dai, 1);
-- 
2.13.2

[PATCH 2/3] ASoC: sun4i-i2s: Get startup to call set_fmt

[codekipper-Re5JQEeQqe8AvxtiuMwx3w]

From: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The set_fmt function pointer is called during probing and this is whilst
the block is disabled. It is over writing the default register values with
the same settings so isn't noticed.
This wasn't a problem with the older SoCs but with the desire to reuse as
much functionlity as possible for the newer devices then set_fmt needs to
be called whilst the block is enabled.

Signed-off-by: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/sunxi/sun4i-i2s.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 38ab0144f897..bb7affd53002 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -351,6 +351,15 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 			   SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
 			   SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
 			   SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+
+	/* Enable the first two channels */
+	regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_SEL_REG,
+		     SUN4I_I2S_TX_CHAN_SEL(2));
+
+	/* Map them to the two first samples coming in */
+	regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG,
+		     SUN4I_I2S_TX_CHAN_MAP(0, 0) | SUN4I_I2S_TX_CHAN_MAP(1, 1));
+
 	return 0;
 }
 
@@ -457,6 +466,9 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
 	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct device *dev = rtd->card->dev;
+	int ret = 0;
 
 	/* Enable the whole hardware block */
 	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
@@ -467,13 +479,11 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
 			   SUN4I_I2S_CTRL_SDO_EN_MASK,
 			   SUN4I_I2S_CTRL_SDO_EN(0));
 
-	/* Enable the first two channels */
-	regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_SEL_REG,
-		     SUN4I_I2S_TX_CHAN_SEL(2));
-
-	/* Map them to the two first samples coming in */
-	regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG,
-		     SUN4I_I2S_TX_CHAN_MAP(0, 0) | SUN4I_I2S_TX_CHAN_MAP(1, 1));
+	ret = snd_soc_dai_set_fmt(rtd->cpu_dai, rtd->dai_link->dai_fmt);
+	if (ret < 0) {
+		dev_err(dev, "can't set cpu_dai set fmt: %d\n", ret);
+		return ret;
+	}
 
 	return clk_prepare_enable(i2s->mod_clk);
 }
-- 
2.13.2

Re: [PATCH 2/3] ASoC: sun4i-i2s: Get startup to call set_fmt

[Maxime Ripard]

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

On Wed, Jul 05, 2017 at 05:43:23PM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
> 
> The set_fmt function pointer is called during probing and this is whilst
> the block is disabled. It is over writing the default register values with
> the same settings so isn't noticed.
> This wasn't a problem with the older SoCs but with the desire to reuse as
> much functionlity as possible for the newer devices then set_fmt needs to
> be called whilst the block is enabled.

The driver while the block is disabled will use the cache, and will
flush it when the block is enabled so it won't make any difference.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[PATCH 3/3] ASoC: sun4i-i2s: Add support for H3

[codekipper-Re5JQEeQqe8AvxtiuMwx3w]

From: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

There are a lot of changes to the sun8i-h3 i2s block but not enough
to warrant to a new driver.

Signed-off-by: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/sound/sun4i-i2s.txt        |   2 +
 sound/soc/sunxi/sun4i-i2s.c                        | 339 ++++++++++++++++++++-
 2 files changed, 337 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
index ee21da865771..fc5da6080759 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -8,6 +8,7 @@ Required properties:
 - compatible: should be one of the following:
    - "allwinner,sun4i-a10-i2s"
    - "allwinner,sun6i-a31-i2s"
+   - "allwinner,sun8i-h3-i2s"
 - reg: physical base address of the controller and length of memory mapped
   region.
 - interrupts: should contain the I2S interrupt.
@@ -22,6 +23,7 @@ Required properties:
 
 Required properties for the following compatibles:
 	- "allwinner,sun6i-a31-i2s"
+	- "allwinner,sun8i-h3-i2s"
 - resets: phandle to the reset line for this codec
 
 Example:
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index bb7affd53002..0b853fe320e0 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -26,9 +26,16 @@
 #include <sound/soc-dai.h>
 
 #define SUN4I_I2S_CTRL_REG		0x00
+#define SUN4I_I2S_CTRL_BCLK_OUT			BIT(18)
+#define SUN4I_I2S_CTRL_LRCK_OUT			BIT(17)
+#define SUN4I_I2S_CTRL_LRCKR_OUT		BIT(16)
 #define SUN4I_I2S_CTRL_SDO_EN_MASK		GENMASK(11, 8)
 #define SUN4I_I2S_CTRL_SDO_EN(sdo)			BIT(8 + (sdo))
 #define SUN4I_I2S_CTRL_MODE_MASK		BIT(5)
+#define SUN8I_I2S_CTRL_MODE_MASK		GENMASK(5, 4)
+#define SUN8I_I2S_CTRL_MODE_RIGHT_J			(2 << 4)
+#define SUN8I_I2S_CTRL_MODE_I2S			(1 << 4)
+#define SUN8I_I2S_CTRL_MODE_PCM			(0 << 4)
 #define SUN4I_I2S_CTRL_MODE_SLAVE			(1 << 5)
 #define SUN4I_I2S_CTRL_MODE_MASTER			(0 << 5)
 #define SUN4I_I2S_CTRL_TX_EN			BIT(2)
@@ -36,16 +43,27 @@
 #define SUN4I_I2S_CTRL_GL_EN			BIT(0)
 
 #define SUN4I_I2S_FMT0_REG		0x04
+#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK	BIT(19)
+#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED		(1 << 19)
+#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL		(0 << 19)
+#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK		GENMASK(17, 8)
+#define SUN8I_I2S_FMT0_LRCK_PERIOD(period)		((period) << 8)
 #define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK	BIT(7)
 #define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED		(1 << 7)
 #define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL		(0 << 7)
+#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK	BIT(7)
+#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED		(1 << 7)
+#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL		(0 << 7)
 #define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK	BIT(6)
 #define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED		(1 << 6)
 #define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL		(0 << 6)
 #define SUN4I_I2S_FMT0_SR_MASK			GENMASK(5, 4)
+#define SUN8I_I2S_FMT0_SR_MASK			GENMASK(6, 4)
 #define SUN4I_I2S_FMT0_SR(sr)				((sr) << 4)
 #define SUN4I_I2S_FMT0_WSS_MASK			GENMASK(3, 2)
 #define SUN4I_I2S_FMT0_WSS(wss)				((wss) << 2)
+#define SUN8I_I2S_FMT0_WSS_MASK			GENMASK(2, 0)
+#define SUN8I_I2S_FMT0_WSS(wss)				(wss)
 #define SUN4I_I2S_FMT0_FMT_MASK			GENMASK(1, 0)
 #define SUN4I_I2S_FMT0_FMT_RIGHT_J			(2 << 0)
 #define SUN4I_I2S_FMT0_FMT_LEFT_J			(1 << 0)
@@ -53,6 +71,7 @@
 
 #define SUN4I_I2S_FMT1_REG		0x08
 #define SUN4I_I2S_FIFO_TX_REG		0x0c
+#define SUN8I_I2S_INT_STA_REG		0x0c
 #define SUN4I_I2S_FIFO_RX_REG		0x10
 
 #define SUN4I_I2S_FIFO_CTRL_REG		0x14
@@ -70,10 +89,13 @@
 #define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN	BIT(3)
 
 #define SUN4I_I2S_INT_STA_REG		0x20
+#define SUN8I_I2S_FIFO_TX_REG		0x20
 
 #define SUN4I_I2S_CLK_DIV_REG		0x24
+#define SUN8I_I2S_CLK_DIV_MCLK_EN		BIT(8)
 #define SUN4I_I2S_CLK_DIV_MCLK_EN		BIT(7)
 #define SUN4I_I2S_CLK_DIV_BCLK_MASK		GENMASK(6, 4)
+#define SUN8I_I2S_CLK_DIV_BCLK_MASK		GENMASK(7, 4)
 #define SUN4I_I2S_CLK_DIV_BCLK(bclk)			((bclk) << 4)
 #define SUN4I_I2S_CLK_DIV_MCLK_MASK		GENMASK(3, 0)
 #define SUN4I_I2S_CLK_DIV_MCLK(mclk)			((mclk) << 0)
@@ -82,14 +104,27 @@
 #define SUN4I_I2S_TX_CNT_REG		0x2c
 
 #define SUN4I_I2S_TX_CHAN_SEL_REG	0x30
+#define SUN8I_I2S_TX_CHAN_CFG_REG	0x30
 #define SUN4I_I2S_TX_CHAN_SEL(num_chan)		(((num_chan) - 1) << 0)
 
 #define SUN4I_I2S_TX_CHAN_MAP_REG	0x34
 #define SUN4I_I2S_TX_CHAN_MAP(chan, sample)	((sample) << (chan << 2))
+#define SUN8I_I2S_TX_CHAN_SEL_REG	0x34
+#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)
+#define SUN8I_I2S_TX_CHAN_SEL_MASK		GENMASK(2, 0)
+#define SUN8I_I2S_TX_CHAN_SEL(num_chan)		(((num_chan) - 1) << 0)
 
 #define SUN4I_I2S_RX_CHAN_SEL_REG	0x38
 #define SUN4I_I2S_RX_CHAN_MAP_REG	0x3c
 
+#define SUN8I_I2S_TX_CHAN_MAP_REG	0x44
+
+#define SUN8I_I2S_RX_CHAN_SEL_REG      0x54
+#define SUN8I_I2S_RX_CHAN_MAP_REG      0x58
+
 struct sun4i_i2s {
 	struct clk	*bus_clk;
 	struct clk	*mod_clk;
@@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 	return 0;
 }
 
+static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
+				  unsigned int rate,
+				  unsigned int word_size)
+{
+	unsigned int clk_rate;
+	int bclk_div, mclk_div;
+	int ret, i;
+
+	switch (rate) {
+	case 176400:
+	case 88200:
+	case 44100:
+	case 22050:
+	case 11025:
+		clk_rate = 22579200;
+		break;
+
+	case 192000:
+	case 128000:
+	case 96000:
+	case 64000:
+	case 48000:
+	case 32000:
+	case 24000:
+	case 16000:
+	case 12000:
+	case 8000:
+		clk_rate = 24576000;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	ret = clk_set_rate(i2s->mod_clk, clk_rate);
+	if (ret)
+		return ret;
+
+	/* Always favor the highest oversampling rate */
+	for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
+		unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
+
+		bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
+						  word_size);
+		mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
+						  clk_rate,
+						  rate);
+
+		if ((bclk_div >= 0) && (mclk_div >= 0))
+			break;
+	}
+
+	if ((bclk_div < 0) || (mclk_div < 0))
+		return -EINVAL;
+
+	regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
+		     SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
+		     SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
+		     SUN8I_I2S_CLK_DIV_MCLK_EN);
+
+	/* Set sync period */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+		     SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
+		     SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
+	regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);
+
+	return 0;
+}
+
+static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
+			       struct snd_pcm_hw_params *params,
+			       struct snd_soc_dai *dai)
+{
+	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	int sr, wss;
+	u32 width, channels = 0;
+
+	switch (params_channels(params)) {
+	case 2:
+		channels |= SUN4I_I2S_CTRL_SDO_EN(0);
+		break;
+	default:
+		dev_err(dai->dev, "invalid channel: %d\n",
+			params_channels(params));
+		return -EINVAL;
+	}
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN4I_I2S_CTRL_SDO_EN_MASK, channels);
+
+	/* Configure the channels */
+	regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
+		     SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));
+
+	/* Select the channels */
+	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+			   SUN8I_I2S_TX_CHAN_EN_MASK |
+			   SUN8I_I2S_TX_CHAN_SEL_MASK,
+			   SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
+			   SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
+
+	/* Map the channels for stereo playback */
+	regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
+		     SUN4I_I2S_TX_CHAN_MAP(1, 1));
+
+	/* Select the channels */
+	regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
+		     SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
+
+	/* Map the channels for stereo capture */
+	regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
+		     SUN4I_I2S_TX_CHAN_MAP(1, 1));
+
+	switch (params_physical_width(params)) {
+	case 16:
+		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		break;
+	case 24:
+	case 32:
+		width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		break;
+	default:
+		return -EINVAL;
+	}
+	i2s->playback_dma_data.addr_width = width;
+
+	switch (params_width(params)) {
+	case 16:
+		sr = 3;
+		wss = 3;
+		break;
+	case 20:
+		sr = 4;
+		wss = 4;
+		break;
+	case 24:
+		sr = 5;
+		wss = 5;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+			   SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
+			   SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
+
+	return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
+				      params_width(params));
+}
+
+static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	u32 val = SUN8I_I2S_CTRL_MODE_I2S;
+	u32 offset = 0;
+
+	/* DAI Mode */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		offset = 1;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN8I_I2S_CTRL_MODE_MASK,
+			   val);
+
+	/* blck offset determines whether i2s or LJ */
+	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+			   SUN8I_I2S_TX_CHAN_OFFSET_MASK,
+			   SUN8I_I2S_TX_CHAN_OFFSET(offset));
+
+	/* DAI clock polarity */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_IB_IF:
+		/* Invert both clocks */
+		val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+			SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		/* Invert bit clock */
+		val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+			SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		/* Invert frame clock */
+		val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
+			SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
+		break;
+	case SND_SOC_DAIFMT_NB_NF:
+		/* Nothing to do for both normal cases */
+		val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
+			SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+			   SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
+			   SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
+			   val);
+
+	/* Set significant bits in our FIFOs */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+			   SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
+			   SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
+			   SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
+			   SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+	return 0;
+}
+
 static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
 {
 	/* Flush RX FIFO */
@@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
 	int ret = 0;
 
 	/* Enable the whole hardware block */
-	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
-		     SUN4I_I2S_CTRL_GL_EN);
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
 
 	/* Enable the first output line */
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
@@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
 			   SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
 
 	/* Disable the whole hardware block */
-	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN4I_I2S_CTRL_GL_EN, 0);
 }
 
 static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
@@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
 	.trigger	= sun4i_i2s_trigger,
 };
 
+static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
+	.hw_params	= sun8i_i2s_hw_params,
+	.set_fmt	= sun8i_i2s_set_fmt,
+	.set_sysclk	= sun4i_i2s_set_sysclk,
+	.shutdown	= sun4i_i2s_shutdown,
+	.startup	= sun4i_i2s_startup,
+	.trigger	= sun4i_i2s_trigger,
+};
+
 static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
 {
 	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
@@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
 	}
 }
 
+
 static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
 	case SUN4I_I2S_FIFO_RX_REG:
-	case SUN4I_I2S_FIFO_STA_REG:
 		return false;
 
 	default:
@@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
 	case SUN4I_I2S_FIFO_RX_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:
@@ -598,6 +863,34 @@ 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)
+{
+	switch (reg) {
+	case SUN4I_I2S_FIFO_RX_REG:
+	case SUN4I_I2S_FIFO_CTRL_REG:
+	case SUN4I_I2S_FIFO_STA_REG:
+	case SUN8I_I2S_INT_STA_REG:
+	case SUN4I_I2S_RX_CNT_REG:
+	case SUN4I_I2S_TX_CNT_REG:
+	case SUN4I_I2S_TX_CHAN_MAP_REG:
+		return true;
+
+	default:
+		return false;
+	}
+}
+
 static const struct reg_default sun4i_i2s_reg_defaults[] = {
 	{ SUN4I_I2S_CTRL_REG, 0x00000000 },
 	{ SUN4I_I2S_FMT0_REG, 0x0000000c },
@@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
 	{ SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
 };
 
+static const struct reg_default sun8i_i2s_reg_defaults[] = {
+	{ SUN4I_I2S_CTRL_REG, 0x00060000 },
+	{ SUN4I_I2S_FMT0_REG, 0x00000033 },
+	{ SUN4I_I2S_FMT1_REG, 0x00000030 },
+	{ SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
+	{ SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
+	{ SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
+	{ SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
+	{ SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
+	{ SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
+	{ SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
+	{ SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
+};
+
 static const struct regmap_config sun4i_i2s_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,
@@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
 	.volatile_reg	= sun4i_i2s_volatile_reg,
 };
 
+static const struct regmap_config sun8i_i2s_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= SUN8I_I2S_RX_CHAN_MAP_REG,
+	.cache_type	= REGCACHE_FLAT,
+	.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,
+	.volatile_reg	= sun8i_i2s_volatile_reg,
+};
+
 static int sun4i_i2s_runtime_resume(struct device *dev)
 {
 	struct sun4i_i2s *i2s = dev_get_drvdata(dev);
@@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 	.ops			= &sun4i_i2s_dai_ops,
 };
 
+static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
+	.has_reset		= true,
+	.reg_offset_txdata	= SUN8I_I2S_FIFO_TX_REG,
+	.sun4i_i2s_regmap	= &sun8i_i2s_regmap_config,
+	.ops			= &sun8i_i2s_dai_ops,
+};
+
 static int sun4i_i2s_probe(struct platform_device *pdev)
 {
 	struct sun4i_i2s *i2s;
@@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
 		.compatible = "allwinner,sun6i-a31-i2s",
 		.data = &sun6i_a31_i2s_quirks,
 	},
+	{
+		.compatible = "allwinner,sun8i-h3-i2s",
+		.data = &sun8i_h3_i2s_quirks,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
-- 
2.13.2

Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 17706 bytes --]

On Wed, Jul 05, 2017 at 05:43:24PM +0200, codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> There are a lot of changes to the sun8i-h3 i2s block but not enough
> to warrant to a new driver.
> 
> Signed-off-by: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/sound/sun4i-i2s.txt        |   2 +
>  sound/soc/sunxi/sun4i-i2s.c                        | 339 ++++++++++++++++++++-
>  2 files changed, 337 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> index ee21da865771..fc5da6080759 100644
> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -8,6 +8,7 @@ Required properties:
>  - compatible: should be one of the following:
>     - "allwinner,sun4i-a10-i2s"
>     - "allwinner,sun6i-a31-i2s"
> +   - "allwinner,sun8i-h3-i2s"
>  - reg: physical base address of the controller and length of memory mapped
>    region.
>  - interrupts: should contain the I2S interrupt.
> @@ -22,6 +23,7 @@ Required properties:
>  
>  Required properties for the following compatibles:
>  	- "allwinner,sun6i-a31-i2s"
> +	- "allwinner,sun8i-h3-i2s"
>  - resets: phandle to the reset line for this codec
>  
>  Example:
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index bb7affd53002..0b853fe320e0 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -26,9 +26,16 @@
>  #include <sound/soc-dai.h>
>  
>  #define SUN4I_I2S_CTRL_REG		0x00
> +#define SUN4I_I2S_CTRL_BCLK_OUT			BIT(18)
> +#define SUN4I_I2S_CTRL_LRCK_OUT			BIT(17)
> +#define SUN4I_I2S_CTRL_LRCKR_OUT		BIT(16)
>  #define SUN4I_I2S_CTRL_SDO_EN_MASK		GENMASK(11, 8)
>  #define SUN4I_I2S_CTRL_SDO_EN(sdo)			BIT(8 + (sdo))
>  #define SUN4I_I2S_CTRL_MODE_MASK		BIT(5)
> +#define SUN8I_I2S_CTRL_MODE_MASK		GENMASK(5, 4)
> +#define SUN8I_I2S_CTRL_MODE_RIGHT_J			(2 << 4)
> +#define SUN8I_I2S_CTRL_MODE_I2S			(1 << 4)
> +#define SUN8I_I2S_CTRL_MODE_PCM			(0 << 4)
>  #define SUN4I_I2S_CTRL_MODE_SLAVE			(1 << 5)
>  #define SUN4I_I2S_CTRL_MODE_MASTER			(0 << 5)
>  #define SUN4I_I2S_CTRL_TX_EN			BIT(2)
> @@ -36,16 +43,27 @@
>  #define SUN4I_I2S_CTRL_GL_EN			BIT(0)
>  
>  #define SUN4I_I2S_FMT0_REG		0x04
> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK	BIT(19)
> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED		(1 << 19)
> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL		(0 << 19)
> +#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK		GENMASK(17, 8)
> +#define SUN8I_I2S_FMT0_LRCK_PERIOD(period)		((period) << 8)
>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK	BIT(7)
>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED		(1 << 7)
>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL		(0 << 7)
> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK	BIT(7)
> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED		(1 << 7)
> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL		(0 << 7)
>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK	BIT(6)
>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED		(1 << 6)
>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL		(0 << 6)
>  #define SUN4I_I2S_FMT0_SR_MASK			GENMASK(5, 4)
> +#define SUN8I_I2S_FMT0_SR_MASK			GENMASK(6, 4)
>  #define SUN4I_I2S_FMT0_SR(sr)				((sr) << 4)
>  #define SUN4I_I2S_FMT0_WSS_MASK			GENMASK(3, 2)
>  #define SUN4I_I2S_FMT0_WSS(wss)				((wss) << 2)
> +#define SUN8I_I2S_FMT0_WSS_MASK			GENMASK(2, 0)
> +#define SUN8I_I2S_FMT0_WSS(wss)				(wss)
>  #define SUN4I_I2S_FMT0_FMT_MASK			GENMASK(1, 0)
>  #define SUN4I_I2S_FMT0_FMT_RIGHT_J			(2 << 0)
>  #define SUN4I_I2S_FMT0_FMT_LEFT_J			(1 << 0)
> @@ -53,6 +71,7 @@
>  
>  #define SUN4I_I2S_FMT1_REG		0x08
>  #define SUN4I_I2S_FIFO_TX_REG		0x0c
> +#define SUN8I_I2S_INT_STA_REG		0x0c
>  #define SUN4I_I2S_FIFO_RX_REG		0x10
>  
>  #define SUN4I_I2S_FIFO_CTRL_REG		0x14
> @@ -70,10 +89,13 @@
>  #define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN	BIT(3)
>  
>  #define SUN4I_I2S_INT_STA_REG		0x20
> +#define SUN8I_I2S_FIFO_TX_REG		0x20
>  
>  #define SUN4I_I2S_CLK_DIV_REG		0x24
> +#define SUN8I_I2S_CLK_DIV_MCLK_EN		BIT(8)
>  #define SUN4I_I2S_CLK_DIV_MCLK_EN		BIT(7)
>  #define SUN4I_I2S_CLK_DIV_BCLK_MASK		GENMASK(6, 4)
> +#define SUN8I_I2S_CLK_DIV_BCLK_MASK		GENMASK(7, 4)
>  #define SUN4I_I2S_CLK_DIV_BCLK(bclk)			((bclk) << 4)
>  #define SUN4I_I2S_CLK_DIV_MCLK_MASK		GENMASK(3, 0)
>  #define SUN4I_I2S_CLK_DIV_MCLK(mclk)			((mclk) << 0)
> @@ -82,14 +104,27 @@
>  #define SUN4I_I2S_TX_CNT_REG		0x2c
>  
>  #define SUN4I_I2S_TX_CHAN_SEL_REG	0x30
> +#define SUN8I_I2S_TX_CHAN_CFG_REG	0x30
>  #define SUN4I_I2S_TX_CHAN_SEL(num_chan)		(((num_chan) - 1) << 0)
>  
>  #define SUN4I_I2S_TX_CHAN_MAP_REG	0x34
>  #define SUN4I_I2S_TX_CHAN_MAP(chan, sample)	((sample) << (chan << 2))
> +#define SUN8I_I2S_TX_CHAN_SEL_REG	0x34
> +#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)
> +#define SUN8I_I2S_TX_CHAN_SEL_MASK		GENMASK(2, 0)
> +#define SUN8I_I2S_TX_CHAN_SEL(num_chan)		(((num_chan) - 1) << 0)
>  
>  #define SUN4I_I2S_RX_CHAN_SEL_REG	0x38
>  #define SUN4I_I2S_RX_CHAN_MAP_REG	0x3c
>  
> +#define SUN8I_I2S_TX_CHAN_MAP_REG	0x44
> +
> +#define SUN8I_I2S_RX_CHAN_SEL_REG      0x54
> +#define SUN8I_I2S_RX_CHAN_MAP_REG      0x58
> +
>  struct sun4i_i2s {
>  	struct clk	*bus_clk;
>  	struct clk	*mod_clk;
> @@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  	return 0;
>  }
>  
> +static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
> +				  unsigned int rate,
> +				  unsigned int word_size)
> +{
> +	unsigned int clk_rate;
> +	int bclk_div, mclk_div;
> +	int ret, i;
> +
> +	switch (rate) {
> +	case 176400:
> +	case 88200:
> +	case 44100:
> +	case 22050:
> +	case 11025:
> +		clk_rate = 22579200;
> +		break;
> +
> +	case 192000:
> +	case 128000:
> +	case 96000:
> +	case 64000:
> +	case 48000:
> +	case 32000:
> +	case 24000:
> +	case 16000:
> +	case 12000:
> +	case 8000:
> +		clk_rate = 24576000;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_set_rate(i2s->mod_clk, clk_rate);
> +	if (ret)
> +		return ret;
> +
> +	/* Always favor the highest oversampling rate */
> +	for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
> +		unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
> +
> +		bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
> +						  word_size);
> +		mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
> +						  clk_rate,
> +						  rate);
> +
> +		if ((bclk_div >= 0) && (mclk_div >= 0))
> +			break;
> +	}
> +
> +	if ((bclk_div < 0) || (mclk_div < 0))
> +		return -EINVAL;
> +
> +	regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
> +		     SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
> +		     SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
> +		     SUN8I_I2S_CLK_DIV_MCLK_EN);

Duplicating all that code just for a single bit difference doesn't
seem very wise. You can handle that bit difference through
regmap_fields.


> +	/* Set sync period */
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> +		     SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> +		     SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
> +	regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);

It seems to be applicable for the old flavour too, why isn't it there?

> +
> +	return 0;
> +}
> +
> +static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
> +			       struct snd_pcm_hw_params *params,
> +			       struct snd_soc_dai *dai)
> +{
> +	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> +	int sr, wss;
> +	u32 width, channels = 0;
> +
> +	switch (params_channels(params)) {
> +	case 2:
> +		channels |= SUN4I_I2S_CTRL_SDO_EN(0);
> +		break;
> +	default:
> +		dev_err(dai->dev, "invalid channel: %d\n",
> +			params_channels(params));
> +		return -EINVAL;
> +	}
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +			   SUN4I_I2S_CTRL_SDO_EN_MASK, channels);

This seems applicable to the old generation.

> +	/* Configure the channels */
> +	regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
> +		     SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));

This can be handled by regmap_fields.

> +	/* Select the channels */
> +	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> +			   SUN8I_I2S_TX_CHAN_EN_MASK |
> +			   SUN8I_I2S_TX_CHAN_SEL_MASK,
> +			   SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
> +			   SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));

Ditto.

>
> +	/* Map the channels for stereo playback */
> +	regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
> +		     SUN4I_I2S_TX_CHAN_MAP(1, 1));

Ditto.

> +	/* Select the channels */
> +	regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
> +		     SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));

Ditto.

> +	/* Map the channels for stereo capture */
> +	regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
> +		     SUN4I_I2S_TX_CHAN_MAP(1, 1));

Ditto.

> +	switch (params_physical_width(params)) {
> +	case 16:
> +		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +		break;
> +	case 24:
> +	case 32:
> +		width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	i2s->playback_dma_data.addr_width = width;

This is applicable to the old generation.

> +
> +	switch (params_width(params)) {
> +	case 16:
> +		sr = 3;
> +		wss = 3;
> +		break;
> +	case 20:
> +		sr = 4;
> +		wss = 4;
> +		break;
> +	case 24:
> +		sr = 5;
> +		wss = 5;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

It looks like this changed, maybe we can split it into a separate
function?

> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> +			   SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
> +			   SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
> +
> +	return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
> +				      params_width(params));
> +}
> +
> +static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> +	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> +	u32 val = SUN8I_I2S_CTRL_MODE_I2S;
> +	u32 offset = 0;
> +
> +	/* DAI Mode */
> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		offset = 1;
> +		break;
> +	case SND_SOC_DAIFMT_LEFT_J:
> +		break;
> +	case SND_SOC_DAIFMT_RIGHT_J:
> +		val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +			   SUN8I_I2S_CTRL_MODE_MASK,
> +			   val);
> +
> +	/* blck offset determines whether i2s or LJ */
> +	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> +			   SUN8I_I2S_TX_CHAN_OFFSET_MASK,
> +			   SUN8I_I2S_TX_CHAN_OFFSET(offset));
> +
> +	/* DAI clock polarity */
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_IB_IF:
> +		/* Invert both clocks */
> +		val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
> +			SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
> +		break;
> +	case SND_SOC_DAIFMT_IB_NF:
> +		/* Invert bit clock */
> +		val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
> +			SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
> +		break;
> +	case SND_SOC_DAIFMT_NB_IF:
> +		/* Invert frame clock */
> +		val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
> +			SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
> +		break;
> +	case SND_SOC_DAIFMT_NB_NF:
> +		/* Nothing to do for both normal cases */
> +		val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
> +			SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> +			   SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
> +			   SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
> +			   val);
> +
> +	/* Set significant bits in our FIFOs */
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> +			   SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
> +			   SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
> +			   SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
> +			   SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
> +	return 0;
> +}
> +
>  static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
>  {
>  	/* Flush RX FIFO */
> @@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
>  	int ret = 0;
>  
>  	/* Enable the whole hardware block */
> -	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
> -		     SUN4I_I2S_CTRL_GL_EN);
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +			   SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);

Why? This needs to be explained.

>  
>  	/* Enable the first output line */
>  	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> @@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
>  			   SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
>  
>  	/* Disable the whole hardware block */
> -	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +			   SUN4I_I2S_CTRL_GL_EN, 0);

Ditto.

>  }
>  
>  static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> @@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
>  	.trigger	= sun4i_i2s_trigger,
>  };
>  
> +static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
> +	.hw_params	= sun8i_i2s_hw_params,
> +	.set_fmt	= sun8i_i2s_set_fmt,
> +	.set_sysclk	= sun4i_i2s_set_sysclk,
> +	.shutdown	= sun4i_i2s_shutdown,
> +	.startup	= sun4i_i2s_startup,
> +	.trigger	= sun4i_i2s_trigger,
> +};
> +
>  static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
>  {
>  	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> @@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
>  	}
>  }
>  
> +

This one looks unnecessary

>  static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
>  	case SUN4I_I2S_FIFO_RX_REG:
> -	case SUN4I_I2S_FIFO_STA_REG:

This needs to be explained

>  		return false;
>  
>  	default:
> @@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
>  	case SUN4I_I2S_FIFO_RX_REG:
> +	case SUN4I_I2S_FIFO_STA_REG:

Ditto.

>  	case SUN4I_I2S_INT_STA_REG:
>  	case SUN4I_I2S_RX_CNT_REG:
>  	case SUN4I_I2S_TX_CNT_REG:
> @@ -598,6 +863,34 @@ 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;
> +	}
> +}

It also applies to the old generation.

> +static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case SUN4I_I2S_FIFO_RX_REG:
> +	case SUN4I_I2S_FIFO_CTRL_REG:
> +	case SUN4I_I2S_FIFO_STA_REG:
> +	case SUN8I_I2S_INT_STA_REG:
> +	case SUN4I_I2S_RX_CNT_REG:
> +	case SUN4I_I2S_TX_CNT_REG:
> +	case SUN4I_I2S_TX_CHAN_MAP_REG:
> +		return true;
> +
> +	default:
> +		return false;
> +	}
> +}

Besides the H3 interrupt status, it looks duplicated. Can't we share
both implementations with a condition for that register?

> +
>  static const struct reg_default sun4i_i2s_reg_defaults[] = {
>  	{ SUN4I_I2S_CTRL_REG, 0x00000000 },
>  	{ SUN4I_I2S_FMT0_REG, 0x0000000c },
> @@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
>  	{ SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
>  };
>  
> +static const struct reg_default sun8i_i2s_reg_defaults[] = {
> +	{ SUN4I_I2S_CTRL_REG, 0x00060000 },
> +	{ SUN4I_I2S_FMT0_REG, 0x00000033 },
> +	{ SUN4I_I2S_FMT1_REG, 0x00000030 },
> +	{ SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
> +	{ SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
> +	{ SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
> +	{ SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
> +	{ SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
> +	{ SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
> +	{ SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
> +	{ SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
> +};
> +
>  static const struct regmap_config sun4i_i2s_regmap_config = {
>  	.reg_bits	= 32,
>  	.reg_stride	= 4,
> @@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
>  	.volatile_reg	= sun4i_i2s_volatile_reg,
>  };
>  
> +static const struct regmap_config sun8i_i2s_regmap_config = {
> +	.reg_bits	= 32,
> +	.reg_stride	= 4,
> +	.val_bits	= 32,
> +	.max_register	= SUN8I_I2S_RX_CHAN_MAP_REG,
> +	.cache_type	= REGCACHE_FLAT,
> +	.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,
> +	.volatile_reg	= sun8i_i2s_volatile_reg,
> +};
> +
>  static int sun4i_i2s_runtime_resume(struct device *dev)
>  {
>  	struct sun4i_i2s *i2s = dev_get_drvdata(dev);
> @@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
>  	.ops			= &sun4i_i2s_dai_ops,
>  };
>  
> +static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
> +	.has_reset		= true,
> +	.reg_offset_txdata	= SUN8I_I2S_FIFO_TX_REG,
> +	.sun4i_i2s_regmap	= &sun8i_i2s_regmap_config,
> +	.ops			= &sun8i_i2s_dai_ops,
> +};
> +
>  static int sun4i_i2s_probe(struct platform_device *pdev)
>  {
>  	struct sun4i_i2s *i2s;
> @@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
>  		.compatible = "allwinner,sun6i-a31-i2s",
>  		.data = &sun6i_a31_i2s_quirks,
>  	},
> +	{
> +		.compatible = "allwinner,sun8i-h3-i2s",
> +		.data = &sun8i_h3_i2s_quirks,
> +	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
> -- 
> 2.13.2
> 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3

[Code Kipper]

On 5 July 2017 at 18:20, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Wed, Jul 05, 2017 at 05:43:24PM +0200, codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> From: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> There are a lot of changes to the sun8i-h3 i2s block but not enough
>> to warrant to a new driver.
>>
>> Signed-off-by: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/sound/sun4i-i2s.txt        |   2 +
>>  sound/soc/sunxi/sun4i-i2s.c                        | 339 ++++++++++++++++++++-
>>  2 files changed, 337 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> index ee21da865771..fc5da6080759 100644
>> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> @@ -8,6 +8,7 @@ Required properties:
>>  - compatible: should be one of the following:
>>     - "allwinner,sun4i-a10-i2s"
>>     - "allwinner,sun6i-a31-i2s"
>> +   - "allwinner,sun8i-h3-i2s"
>>  - reg: physical base address of the controller and length of memory mapped
>>    region.
>>  - interrupts: should contain the I2S interrupt.
>> @@ -22,6 +23,7 @@ Required properties:
>>
>>  Required properties for the following compatibles:
>>       - "allwinner,sun6i-a31-i2s"
>> +     - "allwinner,sun8i-h3-i2s"
>>  - resets: phandle to the reset line for this codec
>>
>>  Example:
>> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
>> index bb7affd53002..0b853fe320e0 100644
>> --- a/sound/soc/sunxi/sun4i-i2s.c
>> +++ b/sound/soc/sunxi/sun4i-i2s.c
>> @@ -26,9 +26,16 @@
>>  #include <sound/soc-dai.h>
>>
>>  #define SUN4I_I2S_CTRL_REG           0x00
>> +#define SUN4I_I2S_CTRL_BCLK_OUT                      BIT(18)
>> +#define SUN4I_I2S_CTRL_LRCK_OUT                      BIT(17)
>> +#define SUN4I_I2S_CTRL_LRCKR_OUT             BIT(16)
>>  #define SUN4I_I2S_CTRL_SDO_EN_MASK           GENMASK(11, 8)
>>  #define SUN4I_I2S_CTRL_SDO_EN(sdo)                   BIT(8 + (sdo))
>>  #define SUN4I_I2S_CTRL_MODE_MASK             BIT(5)
>> +#define SUN8I_I2S_CTRL_MODE_MASK             GENMASK(5, 4)
>> +#define SUN8I_I2S_CTRL_MODE_RIGHT_J                  (2 << 4)
>> +#define SUN8I_I2S_CTRL_MODE_I2S                      (1 << 4)
>> +#define SUN8I_I2S_CTRL_MODE_PCM                      (0 << 4)
>>  #define SUN4I_I2S_CTRL_MODE_SLAVE                    (1 << 5)
>>  #define SUN4I_I2S_CTRL_MODE_MASTER                   (0 << 5)
>>  #define SUN4I_I2S_CTRL_TX_EN                 BIT(2)
>> @@ -36,16 +43,27 @@
>>  #define SUN4I_I2S_CTRL_GL_EN                 BIT(0)
>>
>>  #define SUN4I_I2S_FMT0_REG           0x04
>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK   BIT(19)
>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED               (1 << 19)
>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL         (0 << 19)
>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK              GENMASK(17, 8)
>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD(period)           ((period) << 8)
>>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK   BIT(7)
>>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED               (1 << 7)
>>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL         (0 << 7)
>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK    BIT(7)
>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED                (1 << 7)
>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL          (0 << 7)
>>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK    BIT(6)
>>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED                (1 << 6)
>>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL          (0 << 6)
>>  #define SUN4I_I2S_FMT0_SR_MASK                       GENMASK(5, 4)
>> +#define SUN8I_I2S_FMT0_SR_MASK                       GENMASK(6, 4)
>>  #define SUN4I_I2S_FMT0_SR(sr)                                ((sr) << 4)
>>  #define SUN4I_I2S_FMT0_WSS_MASK                      GENMASK(3, 2)
>>  #define SUN4I_I2S_FMT0_WSS(wss)                              ((wss) << 2)
>> +#define SUN8I_I2S_FMT0_WSS_MASK                      GENMASK(2, 0)
>> +#define SUN8I_I2S_FMT0_WSS(wss)                              (wss)
>>  #define SUN4I_I2S_FMT0_FMT_MASK                      GENMASK(1, 0)
>>  #define SUN4I_I2S_FMT0_FMT_RIGHT_J                   (2 << 0)
>>  #define SUN4I_I2S_FMT0_FMT_LEFT_J                    (1 << 0)
>> @@ -53,6 +71,7 @@
>>
>>  #define SUN4I_I2S_FMT1_REG           0x08
>>  #define SUN4I_I2S_FIFO_TX_REG                0x0c
>> +#define SUN8I_I2S_INT_STA_REG                0x0c
>>  #define SUN4I_I2S_FIFO_RX_REG                0x10
>>
>>  #define SUN4I_I2S_FIFO_CTRL_REG              0x14
>> @@ -70,10 +89,13 @@
>>  #define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN     BIT(3)
>>
>>  #define SUN4I_I2S_INT_STA_REG                0x20
>> +#define SUN8I_I2S_FIFO_TX_REG                0x20
>>
>>  #define SUN4I_I2S_CLK_DIV_REG                0x24
>> +#define SUN8I_I2S_CLK_DIV_MCLK_EN            BIT(8)
>>  #define SUN4I_I2S_CLK_DIV_MCLK_EN            BIT(7)
>>  #define SUN4I_I2S_CLK_DIV_BCLK_MASK          GENMASK(6, 4)
>> +#define SUN8I_I2S_CLK_DIV_BCLK_MASK          GENMASK(7, 4)
>>  #define SUN4I_I2S_CLK_DIV_BCLK(bclk)                 ((bclk) << 4)
>>  #define SUN4I_I2S_CLK_DIV_MCLK_MASK          GENMASK(3, 0)
>>  #define SUN4I_I2S_CLK_DIV_MCLK(mclk)                 ((mclk) << 0)
>> @@ -82,14 +104,27 @@
>>  #define SUN4I_I2S_TX_CNT_REG         0x2c
>>
>>  #define SUN4I_I2S_TX_CHAN_SEL_REG    0x30
>> +#define SUN8I_I2S_TX_CHAN_CFG_REG    0x30
>>  #define SUN4I_I2S_TX_CHAN_SEL(num_chan)              (((num_chan) - 1) << 0)
>>
>>  #define SUN4I_I2S_TX_CHAN_MAP_REG    0x34
>>  #define SUN4I_I2S_TX_CHAN_MAP(chan, sample)  ((sample) << (chan << 2))
>> +#define SUN8I_I2S_TX_CHAN_SEL_REG    0x34
>> +#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)
>> +#define SUN8I_I2S_TX_CHAN_SEL_MASK           GENMASK(2, 0)
>> +#define SUN8I_I2S_TX_CHAN_SEL(num_chan)              (((num_chan) - 1) << 0)
>>
>>  #define SUN4I_I2S_RX_CHAN_SEL_REG    0x38
>>  #define SUN4I_I2S_RX_CHAN_MAP_REG    0x3c
>>
>> +#define SUN8I_I2S_TX_CHAN_MAP_REG    0x44
>> +
>> +#define SUN8I_I2S_RX_CHAN_SEL_REG      0x54
>> +#define SUN8I_I2S_RX_CHAN_MAP_REG      0x58
>> +
>>  struct sun4i_i2s {
>>       struct clk      *bus_clk;
>>       struct clk      *mod_clk;
>> @@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>       return 0;
>>  }
>>
>> +static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
>> +                               unsigned int rate,
>> +                               unsigned int word_size)
>> +{
>> +     unsigned int clk_rate;
>> +     int bclk_div, mclk_div;
>> +     int ret, i;
>> +
>> +     switch (rate) {
>> +     case 176400:
>> +     case 88200:
>> +     case 44100:
>> +     case 22050:
>> +     case 11025:
>> +             clk_rate = 22579200;
>> +             break;
>> +
>> +     case 192000:
>> +     case 128000:
>> +     case 96000:
>> +     case 64000:
>> +     case 48000:
>> +     case 32000:
>> +     case 24000:
>> +     case 16000:
>> +     case 12000:
>> +     case 8000:
>> +             clk_rate = 24576000;
>> +             break;
>> +
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     ret = clk_set_rate(i2s->mod_clk, clk_rate);
>> +     if (ret)
>> +             return ret;
>> +
>> +     /* Always favor the highest oversampling rate */
>> +     for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
>> +             unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
>> +
>> +             bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
>> +                                               word_size);
>> +             mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
>> +                                               clk_rate,
>> +                                               rate);
>> +
>> +             if ((bclk_div >= 0) && (mclk_div >= 0))
>> +                     break;
>> +     }
>> +
>> +     if ((bclk_div < 0) || (mclk_div < 0))
>> +             return -EINVAL;
>> +
>> +     regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
>> +                  SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
>> +                  SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
>> +                  SUN8I_I2S_CLK_DIV_MCLK_EN);
>
> Duplicating all that code just for a single bit difference doesn't
> seem very wise. You can handle that bit difference through
> regmap_fields.

Thanks Maxime for the quick review...I'll ack all these and get back
to you if I get stuck. Just had a quick look at reusing the original
function of the above and with some additional quirks it seems to
work. Wasn't aware of the regmap_fields so I will need to investiagte
their usage, do you know of any good examples?
BR,
CK
>
>
>> +     /* Set sync period */
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>> +                  SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
>> +                  SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
>> +     regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);
>
> It seems to be applicable for the old flavour too, why isn't it there?
>
>> +
>> +     return 0;
>> +}
>> +
>> +static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
>> +                            struct snd_pcm_hw_params *params,
>> +                            struct snd_soc_dai *dai)
>> +{
>> +     struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> +     int sr, wss;
>> +     u32 width, channels = 0;
>> +
>> +     switch (params_channels(params)) {
>> +     case 2:
>> +             channels |= SUN4I_I2S_CTRL_SDO_EN(0);
>> +             break;
>> +     default:
>> +             dev_err(dai->dev, "invalid channel: %d\n",
>> +                     params_channels(params));
>> +             return -EINVAL;
>> +     }
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> +                        SUN4I_I2S_CTRL_SDO_EN_MASK, channels);
>
> This seems applicable to the old generation.
>
>> +     /* Configure the channels */
>> +     regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
>> +                  SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));
>
> This can be handled by regmap_fields.
>
>> +     /* Select the channels */
>> +     regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>> +                        SUN8I_I2S_TX_CHAN_EN_MASK |
>> +                        SUN8I_I2S_TX_CHAN_SEL_MASK,
>> +                        SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
>> +                        SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>
> Ditto.
>
>>
>> +     /* Map the channels for stereo playback */
>> +     regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
>> +                  SUN4I_I2S_TX_CHAN_MAP(1, 1));
>
> Ditto.
>
>> +     /* Select the channels */
>> +     regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
>> +                  SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>
> Ditto.
>
>> +     /* Map the channels for stereo capture */
>> +     regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
>> +                  SUN4I_I2S_TX_CHAN_MAP(1, 1));
>
> Ditto.
>
>> +     switch (params_physical_width(params)) {
>> +     case 16:
>> +             width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> +             break;
>> +     case 24:
>> +     case 32:
>> +             width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +     i2s->playback_dma_data.addr_width = width;
>
> This is applicable to the old generation.
>
>> +
>> +     switch (params_width(params)) {
>> +     case 16:
>> +             sr = 3;
>> +             wss = 3;
>> +             break;
>> +     case 20:
>> +             sr = 4;
>> +             wss = 4;
>> +             break;
>> +     case 24:
>> +             sr = 5;
>> +             wss = 5;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>
> It looks like this changed, maybe we can split it into a separate
> function?
>
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>> +                        SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
>> +                        SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
>> +
>> +     return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
>> +                                   params_width(params));
>> +}
>> +
>> +static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>> +{
>> +     struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> +     u32 val = SUN8I_I2S_CTRL_MODE_I2S;
>> +     u32 offset = 0;
>> +
>> +     /* DAI Mode */
>> +     switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>> +     case SND_SOC_DAIFMT_I2S:
>> +             offset = 1;
>> +             break;
>> +     case SND_SOC_DAIFMT_LEFT_J:
>> +             break;
>> +     case SND_SOC_DAIFMT_RIGHT_J:
>> +             val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> +                        SUN8I_I2S_CTRL_MODE_MASK,
>> +                        val);
>> +
>> +     /* blck offset determines whether i2s or LJ */
>> +     regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>> +                        SUN8I_I2S_TX_CHAN_OFFSET_MASK,
>> +                        SUN8I_I2S_TX_CHAN_OFFSET(offset));
>> +
>> +     /* DAI clock polarity */
>> +     switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>> +     case SND_SOC_DAIFMT_IB_IF:
>> +             /* Invert both clocks */
>> +             val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>> +                     SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
>> +             break;
>> +     case SND_SOC_DAIFMT_IB_NF:
>> +             /* Invert bit clock */
>> +             val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>> +                     SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>> +             break;
>> +     case SND_SOC_DAIFMT_NB_IF:
>> +             /* Invert frame clock */
>> +             val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
>> +                     SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
>> +             break;
>> +     case SND_SOC_DAIFMT_NB_NF:
>> +             /* Nothing to do for both normal cases */
>> +             val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
>> +                     SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>> +                        SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
>> +                        SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
>> +                        val);
>> +
>> +     /* Set significant bits in our FIFOs */
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>> +                        SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
>> +                        SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
>> +                        SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
>> +                        SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
>> +     return 0;
>> +}
>> +
>>  static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
>>  {
>>       /* Flush RX FIFO */
>> @@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
>>       int ret = 0;
>>
>>       /* Enable the whole hardware block */
>> -     regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> -                  SUN4I_I2S_CTRL_GL_EN);
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> +                        SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
>
> Why? This needs to be explained.
>
>>
>>       /* Enable the first output line */
>>       regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> @@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
>>                          SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
>>
>>       /* Disable the whole hardware block */
>> -     regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> +                        SUN4I_I2S_CTRL_GL_EN, 0);
>
> Ditto.
>
>>  }
>>
>>  static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>> @@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
>>       .trigger        = sun4i_i2s_trigger,
>>  };
>>
>> +static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
>> +     .hw_params      = sun8i_i2s_hw_params,
>> +     .set_fmt        = sun8i_i2s_set_fmt,
>> +     .set_sysclk     = sun4i_i2s_set_sysclk,
>> +     .shutdown       = sun4i_i2s_shutdown,
>> +     .startup        = sun4i_i2s_startup,
>> +     .trigger        = sun4i_i2s_trigger,
>> +};
>> +
>>  static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
>>  {
>>       struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> @@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
>>       }
>>  }
>>
>> +
>
> This one looks unnecessary
>
>>  static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
>>  {
>>       switch (reg) {
>>       case SUN4I_I2S_FIFO_RX_REG:
>> -     case SUN4I_I2S_FIFO_STA_REG:
>
> This needs to be explained
>
>>               return false;
>>
>>       default:
>> @@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>>  {
>>       switch (reg) {
>>       case SUN4I_I2S_FIFO_RX_REG:
>> +     case SUN4I_I2S_FIFO_STA_REG:
>
> Ditto.
>
>>       case SUN4I_I2S_INT_STA_REG:
>>       case SUN4I_I2S_RX_CNT_REG:
>>       case SUN4I_I2S_TX_CNT_REG:
>> @@ -598,6 +863,34 @@ 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;
>> +     }
>> +}
>
> It also applies to the old generation.
>
>> +static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> +     switch (reg) {
>> +     case SUN4I_I2S_FIFO_RX_REG:
>> +     case SUN4I_I2S_FIFO_CTRL_REG:
>> +     case SUN4I_I2S_FIFO_STA_REG:
>> +     case SUN8I_I2S_INT_STA_REG:
>> +     case SUN4I_I2S_RX_CNT_REG:
>> +     case SUN4I_I2S_TX_CNT_REG:
>> +     case SUN4I_I2S_TX_CHAN_MAP_REG:
>> +             return true;
>> +
>> +     default:
>> +             return false;
>> +     }
>> +}
>
> Besides the H3 interrupt status, it looks duplicated. Can't we share
> both implementations with a condition for that register?
>
>> +
>>  static const struct reg_default sun4i_i2s_reg_defaults[] = {
>>       { SUN4I_I2S_CTRL_REG, 0x00000000 },
>>       { SUN4I_I2S_FMT0_REG, 0x0000000c },
>> @@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
>>       { SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
>>  };
>>
>> +static const struct reg_default sun8i_i2s_reg_defaults[] = {
>> +     { SUN4I_I2S_CTRL_REG, 0x00060000 },
>> +     { SUN4I_I2S_FMT0_REG, 0x00000033 },
>> +     { SUN4I_I2S_FMT1_REG, 0x00000030 },
>> +     { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
>> +     { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
>> +     { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
>> +     { SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
>> +     { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
>> +     { SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
>> +     { SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
>> +     { SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
>> +};
>> +
>>  static const struct regmap_config sun4i_i2s_regmap_config = {
>>       .reg_bits       = 32,
>>       .reg_stride     = 4,
>> @@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
>>       .volatile_reg   = sun4i_i2s_volatile_reg,
>>  };
>>
>> +static const struct regmap_config sun8i_i2s_regmap_config = {
>> +     .reg_bits       = 32,
>> +     .reg_stride     = 4,
>> +     .val_bits       = 32,
>> +     .max_register   = SUN8I_I2S_RX_CHAN_MAP_REG,
>> +     .cache_type     = REGCACHE_FLAT,
>> +     .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,
>> +     .volatile_reg   = sun8i_i2s_volatile_reg,
>> +};
>> +
>>  static int sun4i_i2s_runtime_resume(struct device *dev)
>>  {
>>       struct sun4i_i2s *i2s = dev_get_drvdata(dev);
>> @@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
>>       .ops                    = &sun4i_i2s_dai_ops,
>>  };
>>
>> +static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
>> +     .has_reset              = true,
>> +     .reg_offset_txdata      = SUN8I_I2S_FIFO_TX_REG,
>> +     .sun4i_i2s_regmap       = &sun8i_i2s_regmap_config,
>> +     .ops                    = &sun8i_i2s_dai_ops,
>> +};
>> +
>>  static int sun4i_i2s_probe(struct platform_device *pdev)
>>  {
>>       struct sun4i_i2s *i2s;
>> @@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
>>               .compatible = "allwinner,sun6i-a31-i2s",
>>               .data = &sun6i_a31_i2s_quirks,
>>       },
>> +     {
>> +             .compatible = "allwinner,sun8i-h3-i2s",
>> +             .data = &sun8i_h3_i2s_quirks,
>> +     },
>>       {}
>>  };
>>  MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
>> --
>> 2.13.2
>>
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3

[Chen-Yu Tsai]

On Thu, Jul 6, 2017 at 1:49 AM, Code Kipper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 5 July 2017 at 18:20, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> On Wed, Jul 05, 2017 at 05:43:24PM +0200, codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>> From: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>
>>> There are a lot of changes to the sun8i-h3 i2s block but not enough
>>> to warrant to a new driver.
>>>
>>> Signed-off-by: Marcus Cooper <codekipper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/sound/sun4i-i2s.txt        |   2 +
>>>  sound/soc/sunxi/sun4i-i2s.c                        | 339 ++++++++++++++++++++-
>>>  2 files changed, 337 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>>> index ee21da865771..fc5da6080759 100644
>>> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>>> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>>> @@ -8,6 +8,7 @@ Required properties:
>>>  - compatible: should be one of the following:
>>>     - "allwinner,sun4i-a10-i2s"
>>>     - "allwinner,sun6i-a31-i2s"
>>> +   - "allwinner,sun8i-h3-i2s"
>>>  - reg: physical base address of the controller and length of memory mapped
>>>    region.
>>>  - interrupts: should contain the I2S interrupt.
>>> @@ -22,6 +23,7 @@ Required properties:
>>>
>>>  Required properties for the following compatibles:
>>>       - "allwinner,sun6i-a31-i2s"
>>> +     - "allwinner,sun8i-h3-i2s"
>>>  - resets: phandle to the reset line for this codec
>>>
>>>  Example:
>>> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
>>> index bb7affd53002..0b853fe320e0 100644
>>> --- a/sound/soc/sunxi/sun4i-i2s.c
>>> +++ b/sound/soc/sunxi/sun4i-i2s.c
>>> @@ -26,9 +26,16 @@
>>>  #include <sound/soc-dai.h>
>>>
>>>  #define SUN4I_I2S_CTRL_REG           0x00
>>> +#define SUN4I_I2S_CTRL_BCLK_OUT                      BIT(18)
>>> +#define SUN4I_I2S_CTRL_LRCK_OUT                      BIT(17)
>>> +#define SUN4I_I2S_CTRL_LRCKR_OUT             BIT(16)
>>>  #define SUN4I_I2S_CTRL_SDO_EN_MASK           GENMASK(11, 8)
>>>  #define SUN4I_I2S_CTRL_SDO_EN(sdo)                   BIT(8 + (sdo))
>>>  #define SUN4I_I2S_CTRL_MODE_MASK             BIT(5)
>>> +#define SUN8I_I2S_CTRL_MODE_MASK             GENMASK(5, 4)
>>> +#define SUN8I_I2S_CTRL_MODE_RIGHT_J                  (2 << 4)
>>> +#define SUN8I_I2S_CTRL_MODE_I2S                      (1 << 4)
>>> +#define SUN8I_I2S_CTRL_MODE_PCM                      (0 << 4)
>>>  #define SUN4I_I2S_CTRL_MODE_SLAVE                    (1 << 5)
>>>  #define SUN4I_I2S_CTRL_MODE_MASTER                   (0 << 5)
>>>  #define SUN4I_I2S_CTRL_TX_EN                 BIT(2)
>>> @@ -36,16 +43,27 @@
>>>  #define SUN4I_I2S_CTRL_GL_EN                 BIT(0)
>>>
>>>  #define SUN4I_I2S_FMT0_REG           0x04
>>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK   BIT(19)
>>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED               (1 << 19)
>>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL         (0 << 19)
>>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK              GENMASK(17, 8)
>>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD(period)           ((period) << 8)
>>>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK   BIT(7)
>>>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED               (1 << 7)
>>>  #define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL         (0 << 7)
>>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK    BIT(7)
>>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED                (1 << 7)
>>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL          (0 << 7)
>>>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK    BIT(6)
>>>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED                (1 << 6)
>>>  #define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL          (0 << 6)
>>>  #define SUN4I_I2S_FMT0_SR_MASK                       GENMASK(5, 4)
>>> +#define SUN8I_I2S_FMT0_SR_MASK                       GENMASK(6, 4)
>>>  #define SUN4I_I2S_FMT0_SR(sr)                                ((sr) << 4)
>>>  #define SUN4I_I2S_FMT0_WSS_MASK                      GENMASK(3, 2)
>>>  #define SUN4I_I2S_FMT0_WSS(wss)                              ((wss) << 2)
>>> +#define SUN8I_I2S_FMT0_WSS_MASK                      GENMASK(2, 0)
>>> +#define SUN8I_I2S_FMT0_WSS(wss)                              (wss)
>>>  #define SUN4I_I2S_FMT0_FMT_MASK                      GENMASK(1, 0)
>>>  #define SUN4I_I2S_FMT0_FMT_RIGHT_J                   (2 << 0)
>>>  #define SUN4I_I2S_FMT0_FMT_LEFT_J                    (1 << 0)
>>> @@ -53,6 +71,7 @@
>>>
>>>  #define SUN4I_I2S_FMT1_REG           0x08
>>>  #define SUN4I_I2S_FIFO_TX_REG                0x0c
>>> +#define SUN8I_I2S_INT_STA_REG                0x0c
>>>  #define SUN4I_I2S_FIFO_RX_REG                0x10
>>>
>>>  #define SUN4I_I2S_FIFO_CTRL_REG              0x14
>>> @@ -70,10 +89,13 @@
>>>  #define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN     BIT(3)
>>>
>>>  #define SUN4I_I2S_INT_STA_REG                0x20
>>> +#define SUN8I_I2S_FIFO_TX_REG                0x20
>>>
>>>  #define SUN4I_I2S_CLK_DIV_REG                0x24
>>> +#define SUN8I_I2S_CLK_DIV_MCLK_EN            BIT(8)
>>>  #define SUN4I_I2S_CLK_DIV_MCLK_EN            BIT(7)
>>>  #define SUN4I_I2S_CLK_DIV_BCLK_MASK          GENMASK(6, 4)
>>> +#define SUN8I_I2S_CLK_DIV_BCLK_MASK          GENMASK(7, 4)
>>>  #define SUN4I_I2S_CLK_DIV_BCLK(bclk)                 ((bclk) << 4)
>>>  #define SUN4I_I2S_CLK_DIV_MCLK_MASK          GENMASK(3, 0)
>>>  #define SUN4I_I2S_CLK_DIV_MCLK(mclk)                 ((mclk) << 0)
>>> @@ -82,14 +104,27 @@
>>>  #define SUN4I_I2S_TX_CNT_REG         0x2c
>>>
>>>  #define SUN4I_I2S_TX_CHAN_SEL_REG    0x30
>>> +#define SUN8I_I2S_TX_CHAN_CFG_REG    0x30
>>>  #define SUN4I_I2S_TX_CHAN_SEL(num_chan)              (((num_chan) - 1) << 0)
>>>
>>>  #define SUN4I_I2S_TX_CHAN_MAP_REG    0x34
>>>  #define SUN4I_I2S_TX_CHAN_MAP(chan, sample)  ((sample) << (chan << 2))
>>> +#define SUN8I_I2S_TX_CHAN_SEL_REG    0x34
>>> +#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)
>>> +#define SUN8I_I2S_TX_CHAN_SEL_MASK           GENMASK(2, 0)
>>> +#define SUN8I_I2S_TX_CHAN_SEL(num_chan)              (((num_chan) - 1) << 0)
>>>
>>>  #define SUN4I_I2S_RX_CHAN_SEL_REG    0x38
>>>  #define SUN4I_I2S_RX_CHAN_MAP_REG    0x3c
>>>
>>> +#define SUN8I_I2S_TX_CHAN_MAP_REG    0x44
>>> +
>>> +#define SUN8I_I2S_RX_CHAN_SEL_REG      0x54
>>> +#define SUN8I_I2S_RX_CHAN_MAP_REG      0x58
>>> +
>>>  struct sun4i_i2s {
>>>       struct clk      *bus_clk;
>>>       struct clk      *mod_clk;
>>> @@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>>       return 0;
>>>  }
>>>
>>> +static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
>>> +                               unsigned int rate,
>>> +                               unsigned int word_size)
>>> +{
>>> +     unsigned int clk_rate;
>>> +     int bclk_div, mclk_div;
>>> +     int ret, i;
>>> +
>>> +     switch (rate) {
>>> +     case 176400:
>>> +     case 88200:
>>> +     case 44100:
>>> +     case 22050:
>>> +     case 11025:
>>> +             clk_rate = 22579200;
>>> +             break;
>>> +
>>> +     case 192000:
>>> +     case 128000:
>>> +     case 96000:
>>> +     case 64000:
>>> +     case 48000:
>>> +     case 32000:
>>> +     case 24000:
>>> +     case 16000:
>>> +     case 12000:
>>> +     case 8000:
>>> +             clk_rate = 24576000;
>>> +             break;
>>> +
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     ret = clk_set_rate(i2s->mod_clk, clk_rate);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     /* Always favor the highest oversampling rate */
>>> +     for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
>>> +             unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
>>> +
>>> +             bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
>>> +                                               word_size);
>>> +             mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
>>> +                                               clk_rate,
>>> +                                               rate);
>>> +
>>> +             if ((bclk_div >= 0) && (mclk_div >= 0))
>>> +                     break;
>>> +     }
>>> +
>>> +     if ((bclk_div < 0) || (mclk_div < 0))
>>> +             return -EINVAL;
>>> +
>>> +     regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
>>> +                  SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
>>> +                  SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
>>> +                  SUN8I_I2S_CLK_DIV_MCLK_EN);
>>
>> Duplicating all that code just for a single bit difference doesn't
>> seem very wise. You can handle that bit difference through
>> regmap_fields.
>
> Thanks Maxime for the quick review...I'll ack all these and get back
> to you if I get stuck. Just had a quick look at reusing the original
> function of the above and with some additional quirks it seems to
> work. Wasn't aware of the regmap_fields so I will need to investiagte
> their usage, do you know of any good examples?
> BR,
> CK

See https://wens.tw/hdmi-i2c-regmap.txt for an example of what I'm
doing for the HDMI driver. Note that this is a bit extreme as some
regmap_fields are only 1 bit wide. This example uses regmap_fields
for all register bitfields that are different.

In other cases where only the register offset differs I would just
have regmap_field point to the whole register.

ChenYu

>>
>>
>>> +     /* Set sync period */
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>>> +                  SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
>>> +                  SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
>>> +     regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);
>>
>> It seems to be applicable for the old flavour too, why isn't it there?
>>
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
>>> +                            struct snd_pcm_hw_params *params,
>>> +                            struct snd_soc_dai *dai)
>>> +{
>>> +     struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> +     int sr, wss;
>>> +     u32 width, channels = 0;
>>> +
>>> +     switch (params_channels(params)) {
>>> +     case 2:
>>> +             channels |= SUN4I_I2S_CTRL_SDO_EN(0);
>>> +             break;
>>> +     default:
>>> +             dev_err(dai->dev, "invalid channel: %d\n",
>>> +                     params_channels(params));
>>> +             return -EINVAL;
>>> +     }
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> +                        SUN4I_I2S_CTRL_SDO_EN_MASK, channels);
>>
>> This seems applicable to the old generation.
>>
>>> +     /* Configure the channels */
>>> +     regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
>>> +                  SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));
>>
>> This can be handled by regmap_fields.
>>
>>> +     /* Select the channels */
>>> +     regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>>> +                        SUN8I_I2S_TX_CHAN_EN_MASK |
>>> +                        SUN8I_I2S_TX_CHAN_SEL_MASK,
>>> +                        SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
>>> +                        SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>>
>> Ditto.
>>
>>>
>>> +     /* Map the channels for stereo playback */
>>> +     regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
>>> +                  SUN4I_I2S_TX_CHAN_MAP(1, 1));
>>
>> Ditto.
>>
>>> +     /* Select the channels */
>>> +     regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
>>> +                  SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>>
>> Ditto.
>>
>>> +     /* Map the channels for stereo capture */
>>> +     regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
>>> +                  SUN4I_I2S_TX_CHAN_MAP(1, 1));
>>
>> Ditto.
>>
>>> +     switch (params_physical_width(params)) {
>>> +     case 16:
>>> +             width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>>> +             break;
>>> +     case 24:
>>> +     case 32:
>>> +             width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>>> +             break;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +     i2s->playback_dma_data.addr_width = width;
>>
>> This is applicable to the old generation.
>>
>>> +
>>> +     switch (params_width(params)) {
>>> +     case 16:
>>> +             sr = 3;
>>> +             wss = 3;
>>> +             break;
>>> +     case 20:
>>> +             sr = 4;
>>> +             wss = 4;
>>> +             break;
>>> +     case 24:
>>> +             sr = 5;
>>> +             wss = 5;
>>> +             break;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>
>> It looks like this changed, maybe we can split it into a separate
>> function?
>>
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>>> +                        SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
>>> +                        SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
>>> +
>>> +     return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
>>> +                                   params_width(params));
>>> +}
>>> +
>>> +static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>> +{
>>> +     struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> +     u32 val = SUN8I_I2S_CTRL_MODE_I2S;
>>> +     u32 offset = 0;
>>> +
>>> +     /* DAI Mode */
>>> +     switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>> +     case SND_SOC_DAIFMT_I2S:
>>> +             offset = 1;
>>> +             break;
>>> +     case SND_SOC_DAIFMT_LEFT_J:
>>> +             break;
>>> +     case SND_SOC_DAIFMT_RIGHT_J:
>>> +             val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
>>> +             break;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> +                        SUN8I_I2S_CTRL_MODE_MASK,
>>> +                        val);
>>> +
>>> +     /* blck offset determines whether i2s or LJ */
>>> +     regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>>> +                        SUN8I_I2S_TX_CHAN_OFFSET_MASK,
>>> +                        SUN8I_I2S_TX_CHAN_OFFSET(offset));
>>> +
>>> +     /* DAI clock polarity */
>>> +     switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>>> +     case SND_SOC_DAIFMT_IB_IF:
>>> +             /* Invert both clocks */
>>> +             val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>>> +                     SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
>>> +             break;
>>> +     case SND_SOC_DAIFMT_IB_NF:
>>> +             /* Invert bit clock */
>>> +             val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>>> +                     SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>>> +             break;
>>> +     case SND_SOC_DAIFMT_NB_IF:
>>> +             /* Invert frame clock */
>>> +             val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
>>> +                     SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
>>> +             break;
>>> +     case SND_SOC_DAIFMT_NB_NF:
>>> +             /* Nothing to do for both normal cases */
>>> +             val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
>>> +                     SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>>> +             break;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>>> +                        SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
>>> +                        SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
>>> +                        val);
>>> +
>>> +     /* Set significant bits in our FIFOs */
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>>> +                        SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
>>> +                        SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
>>> +                        SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
>>> +                        SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
>>> +     return 0;
>>> +}
>>> +
>>>  static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
>>>  {
>>>       /* Flush RX FIFO */
>>> @@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
>>>       int ret = 0;
>>>
>>>       /* Enable the whole hardware block */
>>> -     regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> -                  SUN4I_I2S_CTRL_GL_EN);
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> +                        SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
>>
>> Why? This needs to be explained.
>>
>>>
>>>       /* Enable the first output line */
>>>       regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> @@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
>>>                          SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
>>>
>>>       /* Disable the whole hardware block */
>>> -     regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
>>> +     regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> +                        SUN4I_I2S_CTRL_GL_EN, 0);
>>
>> Ditto.
>>
>>>  }
>>>
>>>  static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>>> @@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
>>>       .trigger        = sun4i_i2s_trigger,
>>>  };
>>>
>>> +static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
>>> +     .hw_params      = sun8i_i2s_hw_params,
>>> +     .set_fmt        = sun8i_i2s_set_fmt,
>>> +     .set_sysclk     = sun4i_i2s_set_sysclk,
>>> +     .shutdown       = sun4i_i2s_shutdown,
>>> +     .startup        = sun4i_i2s_startup,
>>> +     .trigger        = sun4i_i2s_trigger,
>>> +};
>>> +
>>>  static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
>>>  {
>>>       struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> @@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
>>>       }
>>>  }
>>>
>>> +
>>
>> This one looks unnecessary
>>
>>>  static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
>>>  {
>>>       switch (reg) {
>>>       case SUN4I_I2S_FIFO_RX_REG:
>>> -     case SUN4I_I2S_FIFO_STA_REG:
>>
>> This needs to be explained
>>
>>>               return false;
>>>
>>>       default:
>>> @@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>>>  {
>>>       switch (reg) {
>>>       case SUN4I_I2S_FIFO_RX_REG:
>>> +     case SUN4I_I2S_FIFO_STA_REG:
>>
>> Ditto.
>>
>>>       case SUN4I_I2S_INT_STA_REG:
>>>       case SUN4I_I2S_RX_CNT_REG:
>>>       case SUN4I_I2S_TX_CNT_REG:
>>> @@ -598,6 +863,34 @@ 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;
>>> +     }
>>> +}
>>
>> It also applies to the old generation.
>>
>>> +static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>>> +{
>>> +     switch (reg) {
>>> +     case SUN4I_I2S_FIFO_RX_REG:
>>> +     case SUN4I_I2S_FIFO_CTRL_REG:
>>> +     case SUN4I_I2S_FIFO_STA_REG:
>>> +     case SUN8I_I2S_INT_STA_REG:
>>> +     case SUN4I_I2S_RX_CNT_REG:
>>> +     case SUN4I_I2S_TX_CNT_REG:
>>> +     case SUN4I_I2S_TX_CHAN_MAP_REG:
>>> +             return true;
>>> +
>>> +     default:
>>> +             return false;
>>> +     }
>>> +}
>>
>> Besides the H3 interrupt status, it looks duplicated. Can't we share
>> both implementations with a condition for that register?
>>
>>> +
>>>  static const struct reg_default sun4i_i2s_reg_defaults[] = {
>>>       { SUN4I_I2S_CTRL_REG, 0x00000000 },
>>>       { SUN4I_I2S_FMT0_REG, 0x0000000c },
>>> @@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
>>>       { SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
>>>  };
>>>
>>> +static const struct reg_default sun8i_i2s_reg_defaults[] = {
>>> +     { SUN4I_I2S_CTRL_REG, 0x00060000 },
>>> +     { SUN4I_I2S_FMT0_REG, 0x00000033 },
>>> +     { SUN4I_I2S_FMT1_REG, 0x00000030 },
>>> +     { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
>>> +     { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
>>> +     { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
>>> +     { SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
>>> +     { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
>>> +     { SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
>>> +     { SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
>>> +     { SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
>>> +};
>>> +
>>>  static const struct regmap_config sun4i_i2s_regmap_config = {
>>>       .reg_bits       = 32,
>>>       .reg_stride     = 4,
>>> @@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
>>>       .volatile_reg   = sun4i_i2s_volatile_reg,
>>>  };
>>>
>>> +static const struct regmap_config sun8i_i2s_regmap_config = {
>>> +     .reg_bits       = 32,
>>> +     .reg_stride     = 4,
>>> +     .val_bits       = 32,
>>> +     .max_register   = SUN8I_I2S_RX_CHAN_MAP_REG,
>>> +     .cache_type     = REGCACHE_FLAT,
>>> +     .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,
>>> +     .volatile_reg   = sun8i_i2s_volatile_reg,
>>> +};
>>> +
>>>  static int sun4i_i2s_runtime_resume(struct device *dev)
>>>  {
>>>       struct sun4i_i2s *i2s = dev_get_drvdata(dev);
>>> @@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
>>>       .ops                    = &sun4i_i2s_dai_ops,
>>>  };
>>>
>>> +static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
>>> +     .has_reset              = true,
>>> +     .reg_offset_txdata      = SUN8I_I2S_FIFO_TX_REG,
>>> +     .sun4i_i2s_regmap       = &sun8i_i2s_regmap_config,
>>> +     .ops                    = &sun8i_i2s_dai_ops,
>>> +};
>>> +
>>>  static int sun4i_i2s_probe(struct platform_device *pdev)
>>>  {
>>>       struct sun4i_i2s *i2s;
>>> @@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
>>>               .compatible = "allwinner,sun6i-a31-i2s",
>>>               .data = &sun6i_a31_i2s_quirks,
>>>       },
>>> +     {
>>> +             .compatible = "allwinner,sun8i-h3-i2s",
>>> +             .data = &sun8i_h3_i2s_quirks,
>>> +     },
>>>       {}
>>>  };
>>>  MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
>>> --
>>> 2.13.2
>>>
>>
>> Thanks!
>> Maxime
>>
>> --
>> Maxime Ripard, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.