[PATCH 0/4] ASoC: sunxi: Add i2s controller support

[PATCH 0/4] ASoC: sunxi: Add i2s controller support

[Maxime Ripard]

Hi everyone,

This is the first version of the I2S support for the controller found
in the Allwinner A10 and later SoCs.

Playback has been tested with an UDA1380 on an A20-Olinuxino. Capture
is not implemented yet, but will come eventually.

Let me know what you think,
Maxime

Emilio López (1):
  ARM: sun7i: Add mod1 clock nodes

Maxime Ripard (3):
  dt-bindings: Add A10 I2S controller binding documentation
  ASoC: sunxi: Add Allwinner A10 Digital Audio driver
  ARM: sun7i: Add DAI nodes

 .../devicetree/bindings/sound/sun4i-i2s.txt        |  33 +
 arch/arm/boot/dts/sun7i-a20.dtsi                   |  87 ++-
 sound/soc/sunxi/Kconfig                            |  10 +
 sound/soc/sunxi/Makefile                           |   2 +-
 sound/soc/sunxi/sun4i-i2s.c                        | 714 +++++++++++++++++++++
 5 files changed, 843 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/sun4i-i2s.txt
 create mode 100644 sound/soc/sunxi/sun4i-i2s.c

-- 
2.8.3

[PATCH 1/4] dt-bindings: Add A10 I2S controller binding documentation

[Maxime Ripard]

Introduce the device tree binding for the I2S controller found in the
Allwinner A10 and later SoCs.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/sound/sun4i-i2s.txt        | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/sun4i-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
new file mode 100644
index 000000000000..365ca4eede5f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -0,0 +1,33 @@
+* Allwinner A10 I2S controller
+
+The I2S bus (Inter-IC sound bus) is a serial link for digital
+audio data transfer between devices in the system.
+
+Required properties:
+
+- compatible: should be one of the followings
+   - "allwinner,sun4i-a10-i2s"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: should contain the I2S interrupt.
+- dmas: DMA specifiers for tx and rx dma. See the DMA client binding,
+	Documentation/devicetree/bindings/dma/dma.txt
+- dma-names: should include "tx" and "rx".
+- clocks: a list of phandle + clock-specifer pairs, one for each entry in clock-names.
+- clock-names: should contain followings:
+   - "apb" : clock for the I2S bus interface
+   - "mod" : module clock for the I2S controller
+
+Example:
+
+i2s0: i2s@01c22400 {
+	#sound-dai-cells = <0>;
+	compatible = "allwinner,sun4i-a10-i2s";
+	reg = <0x01c22400 0x400>;
+	interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&apb0_gates 3>, <&i2s0_clk>;
+	clock-names = "apb", "mod";
+	dmas = <&dma SUN4I_DMA_NORMAL 3>,
+	       <&dma SUN4I_DMA_NORMAL 3>;
+	dma-names = "rx", "tx";
+};
-- 
2.8.3

[PATCH 2/4] ASoC: sunxi: Add Allwinner A10 Digital Audio driver

[Maxime Ripard]

The Allwinner A10 and later come with a hardware block that used for the
PCM and I2S interfaces.

Add a driver for it in ASoC.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 sound/soc/sunxi/Kconfig     |  10 +
 sound/soc/sunxi/Makefile    |   2 +-
 sound/soc/sunxi/sun4i-i2s.c | 714 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 725 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/sunxi/sun4i-i2s.c

diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index ae42294ef688..0a2a2ec2b580 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -8,6 +8,15 @@ config SND_SUN4I_CODEC
 	  Select Y or M to add support for the Codec embedded in the Allwinner
 	  A10 and affiliated SoCs.
 
+config SND_SUN4I_I2S
+	tristate "Allwinner A10 I2S Support"
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	select REGMAP_MMIO
+	help
+	  Say Y or M if you want to add support for codecs attached to
+	  the Allwinner A10 I2S. You will also need to select the
+	  individual machine drivers to support below.
+
 config SND_SUN4I_SPDIF
 	tristate "Allwinner A10 SPDIF Support"
 	depends on OF
@@ -16,4 +25,5 @@ config SND_SUN4I_SPDIF
 	help
 	  Say Y or M to add support for the S/PDIF audio block in the Allwinner
 	  A10 and affiliated SoCs.
+
 endmenu
diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
index 8f5e889667f1..604c7b842837 100644
--- a/sound/soc/sunxi/Makefile
+++ b/sound/soc/sunxi/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_SND_SUN4I_CODEC) += sun4i-codec.o
-
+obj-$(CONFIG_SND_SUN4I_I2S) += sun4i-i2s.o
 obj-$(CONFIG_SND_SUN4I_SPDIF) += sun4i-spdif.o
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
new file mode 100644
index 000000000000..f6f3b2e03def
--- /dev/null
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -0,0 +1,714 @@
+/*
+ * Copyright (C) 2015 Andrea Venturi
+ * Andrea Venturi <be17068@iperbole.bo.it>
+ *
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+
+#define SUN4I_I2S_CTRL_REG		0x00
+#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 SUN4I_I2S_CTRL_MODE_SLAVE			(1 << 5)
+#define SUN4I_I2S_CTRL_MODE_MASTER			(0 << 5)
+#define SUN4I_I2S_CTRL_TX_EN			BIT(2)
+#define SUN4I_I2S_CTRL_RX_EN			BIT(1)
+#define SUN4I_I2S_CTRL_GL_EN			BIT(0)
+
+#define SUN4I_I2S_FMT0_REG		0x04
+#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 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 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 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)
+#define SUN4I_I2S_FMT0_FMT_I2S				(0 << 0)
+
+#define SUN4I_I2S_FMT1_REG		0x08
+#define SUN4I_I2S_FIFO_TX_REG		0x0c
+#define SUN4I_I2S_FIFO_RX_REG		0x10
+
+#define SUN4I_I2S_FIFO_CTRL_REG		0x14
+#define SUN4I_I2S_FIFO_CTRL_FLUSH_TX		BIT(25)
+#define SUN4I_I2S_FIFO_CTRL_FLUSH_RX		BIT(24)
+#define SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK	BIT(2)
+#define SUN4I_I2S_FIFO_CTRL_TX_MODE(mode)		((mode) << 2)
+#define SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK	GENMASK(1, 0)
+#define SUN4I_I2S_FIFO_CTRL_RX_MODE(mode)		(mode)
+
+#define SUN4I_I2S_FIFO_STA_REG		0x18
+
+#define SUN4I_I2S_DMA_INT_CTRL_REG	0x1c
+#define SUN4I_I2S_DMA_INT_CTRL_TX_DRQ_EN	BIT(7)
+#define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN	BIT(3)
+
+#define SUN4I_I2S_INT_STA_REG		0x20
+
+#define SUN4I_I2S_CLK_DIV_REG		0x24
+#define SUN4I_I2S_CLK_DIV_MCLK_EN		BIT(7)
+#define SUN4I_I2S_CLK_DIV_BCLK_MASK		GENMASK(6, 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)
+
+#define SUN4I_I2S_RX_CNT_REG		0x28
+#define SUN4I_I2S_TX_CNT_REG		0x2c
+
+#define SUN4I_I2S_TX_CHAN_SEL_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 SUN4I_I2S_RX_CHAN_SEL_REG	0x38
+#define SUN4I_I2S_RX_CHAN_MAP_REG	0x3c
+
+struct sun4i_i2s {
+	struct clk	*bus_clk;
+	struct clk	*mod_clk;
+	struct regmap	*regmap;
+
+	struct snd_dmaengine_dai_dma_data	playback_dma_data;
+};
+
+struct sun4i_i2s_clk_div {
+	u8	div;
+	u8	val;
+};
+
+static const struct sun4i_i2s_clk_div sun4i_i2s_bclk_div[] = {
+	{ .div = 2, .val = 0 },
+	{ .div = 4, .val = 1 },
+	{ .div = 6, .val = 2 },
+	{ .div = 8, .val = 3 },
+	{ .div = 12, .val = 4 },
+	{ .div = 16, .val = 5 },
+	{ /* Sentinel */ },
+};
+
+static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
+	{ .div = 1, .val = 0 },
+	{ .div = 2, .val = 1 },
+	{ .div = 4, .val = 2 },
+	{ .div = 6, .val = 3 },
+	{ .div = 8, .val = 4 },
+	{ .div = 12, .val = 5 },
+	{ .div = 16, .val = 6 },
+	{ .div = 24, .val = 7 },
+	{ /* Sentinel */ },
+};
+
+static int sun4i_i2s_params_to_sr(struct snd_pcm_hw_params *params)
+{
+	switch (params_width(params)) {
+	case 16:
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static u8 sun4i_i2s_params_to_wss(struct snd_pcm_hw_params *params)
+{
+	switch (params_width(params)) {
+	case 16:
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int sun4i_i2s_get_bclk_div(struct sun4i_i2s *i2s,
+				  unsigned int oversample_rate,
+				  unsigned int word_size)
+{
+	int div = oversample_rate / word_size / 2;
+	int i;
+
+	for (i = 0; sun4i_i2s_bclk_div[i].div; i++) {
+		const struct sun4i_i2s_clk_div *bdiv = sun4i_i2s_bclk_div + i;
+
+		if (bdiv->div == div)
+			return bdiv->val;
+	}
+
+	return -EINVAL;
+}
+
+static int sun4i_i2s_get_mclk_div(struct sun4i_i2s *i2s,
+				  unsigned int oversample_rate,
+				  unsigned int module_rate,
+				  unsigned int sampling_rate)
+{
+	int div = module_rate / sampling_rate / oversample_rate;
+	int i;
+
+	for (i = 0; sun4i_i2s_mclk_div[i].div; i++) {
+		const struct sun4i_i2s_clk_div *mdiv = sun4i_i2s_mclk_div + i;
+
+		if (mdiv->div == div)
+			return mdiv->val;
+	}
+
+	return -EINVAL;
+}
+
+static int sun4i_i2s_oversample_rates[] = { 128, 192, 256, 384, 512, 768 };
+
+static int sun4i_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 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;
+        }
+
+	clk_set_rate(i2s->mod_clk, clk_rate);
+
+	/* 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) |
+		     SUN4I_I2S_CLK_DIV_MCLK_EN);
+
+	return 0;
+}
+
+static int sun4i_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;
+
+	if (params_channels(params) != 2)
+		return -EINVAL;
+
+	/* Enable the first output line */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   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));
+
+	switch (params_physical_width(params)) {
+	case 16:
+		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		break;
+	default:
+		return -EINVAL;
+	}
+	i2s->playback_dma_data.addr_width = width;
+
+	sr = sun4i_i2s_params_to_sr(params);
+	if (sr < 0)
+		return -EINVAL;
+
+	wss = sun4i_i2s_params_to_wss(params);
+	if (wss < 0)
+		return -EINVAL;
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+			   SUN4I_I2S_FMT0_WSS_MASK | SUN4I_I2S_FMT0_SR_MASK,
+			   SUN4I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
+
+	return sun4i_i2s_set_clk_rate(i2s, params_rate(params),
+				      params_width(params));
+}
+
+static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+        struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	u32 val;
+
+	/* DAI Mode */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		val = SUN4I_I2S_FMT0_FMT_I2S;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		val = SUN4I_I2S_FMT0_FMT_LEFT_J;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		val = SUN4I_I2S_FMT0_FMT_RIGHT_J;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+			   SUN4I_I2S_FMT0_FMT_MASK,
+			   val);
+
+	/* DAI clock polarity */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_IB_IF:
+		/* Invert both clocks */
+		val = SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+			SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		/* Invert bit clock */
+		val = SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+			SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		/* Invert frame clock */
+		val = SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
+			SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL;
+		break;
+	case SND_SOC_DAIFMT_NB_NF:
+		/* Nothing to do for both normal cases */
+		val = SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL |
+			SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+			   SUN4I_I2S_FMT0_BCLK_POLARITY_MASK |
+			   SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK,
+			   val);
+
+	/* DAI clock master masks */
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		/* BCLK and LRCLK master */
+		val = SUN4I_I2S_CTRL_MODE_MASTER;
+		break;
+	case SND_SOC_DAIFMT_CBM_CFM:
+		/* BCLK and LRCLK slave */
+		val = SUN4I_I2S_CTRL_MODE_SLAVE;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN4I_I2S_CTRL_MODE_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_playback(struct sun4i_i2s *i2s)
+{
+	/* Flush TX FIFO */
+        regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+			   SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
+			   SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
+
+        /* Clear TX counter */
+	regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
+
+        /* Enable TX Block */
+        regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN4I_I2S_CTRL_TX_EN,
+			   SUN4I_I2S_CTRL_TX_EN);
+
+        /* Enable TX DRQ */
+        regmap_update_bits(i2s->regmap, SUN4I_I2S_DMA_INT_CTRL_REG,
+			   SUN4I_I2S_DMA_INT_CTRL_TX_DRQ_EN,
+			   SUN4I_I2S_DMA_INT_CTRL_TX_DRQ_EN);
+}
+
+
+static void sun4i_i2s_stop_playback(struct sun4i_i2s *i2s)
+{
+        /* Disable TX Block */
+        regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN4I_I2S_CTRL_TX_EN,
+			   0);
+
+        /* Disable TX DRQ */
+        regmap_update_bits(i2s->regmap, SUN4I_I2S_DMA_INT_CTRL_REG,
+			   SUN4I_I2S_DMA_INT_CTRL_TX_DRQ_EN,
+			   0);
+}
+
+static int sun4i_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
+			     struct snd_soc_dai *dai)
+{
+	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			sun4i_i2s_start_playback(i2s);
+		else
+			return -EINVAL;
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			sun4i_i2s_stop_playback(i2s);
+		else
+			return -EINVAL;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+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);
+
+	return clk_prepare_enable(i2s->mod_clk);
+}
+
+static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai)
+{
+	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+
+	return clk_disable_unprepare(i2s->mod_clk);
+}
+
+static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
+	.hw_params	= sun4i_i2s_hw_params,
+	.set_fmt	= sun4i_i2s_set_fmt,
+	.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);
+
+	/* Enable the whole hardware block */
+	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
+		     SUN4I_I2S_CTRL_GL_EN);
+
+	snd_soc_dai_init_dma_data(dai, &i2s->playback_dma_data, NULL);
+
+	snd_soc_dai_set_drvdata(dai, i2s);
+
+	return 0;
+}
+
+static struct snd_soc_dai_driver sun4i_i2s_dai = {
+	.probe = sun4i_i2s_dai_probe,
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_8000_192000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+	.ops = &sun4i_i2s_dai_ops,
+	.symmetric_rates = 1,
+};
+
+static const struct snd_soc_component_driver sun4i_i2s_component = {
+	.name	= "sun4i-dai",
+};
+
+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;
+	}
+}
+
+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:
+		return true;
+	}
+}
+
+static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SUN4I_I2S_FIFO_RX_REG:
+	case SUN4I_I2S_INT_STA_REG:
+	case SUN4I_I2S_RX_CNT_REG:
+	case SUN4I_I2S_TX_CNT_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 },
+	{ SUN4I_I2S_FMT1_REG, 0x00004020 },
+	{ SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
+	{ SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
+	{ SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
+	{ SUN4I_I2S_TX_CHAN_SEL_REG, 0x00000001 },
+	{ SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210 },
+	{ SUN4I_I2S_RX_CHAN_SEL_REG, 0x00000001 },
+	{ SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
+};
+
+static const struct regmap_config sun4i_i2s_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= SUN4I_I2S_RX_CHAN_MAP_REG,
+
+	.cache_type	= REGCACHE_FLAT,
+	.reg_defaults	= sun4i_i2s_reg_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(sun4i_i2s_reg_defaults),
+	.writeable_reg	= sun4i_i2s_wr_reg,
+	.readable_reg	= sun4i_i2s_rd_reg,
+	.volatile_reg	= sun4i_i2s_volatile_reg,
+};
+
+static int sun4i_i2s_runtime_resume(struct device *dev)
+{
+	struct sun4i_i2s *i2s = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(i2s->bus_clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable bus clock\n");
+		return ret;
+	}
+
+	regcache_cache_only(i2s->regmap, false);
+	regcache_mark_dirty(i2s->regmap);
+
+	ret = regcache_sync(i2s->regmap);
+	if (ret) {
+		dev_err(dev, "Failed to sync regmap cache\n");
+		goto err_disable_clk;
+	}
+
+	return 0;
+
+err_disable_clk:
+	clk_disable_unprepare(i2s->bus_clk);
+	return ret;
+}
+
+static int sun4i_i2s_runtime_suspend(struct device *dev)
+{
+	struct sun4i_i2s *i2s = dev_get_drvdata(dev);
+
+	regcache_cache_only(i2s->regmap, true);
+
+	clk_disable_unprepare(i2s->bus_clk);
+
+	return 0;
+}
+
+static int sun4i_i2s_probe(struct platform_device *pdev)
+{
+	struct sun4i_i2s *i2s;
+	struct resource *res;
+	void __iomem *regs;
+	int irq, ret;
+
+	i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
+	if (!i2s)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, i2s);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(regs)) {
+		dev_err(&pdev->dev, "Can't request IO region\n");
+		return PTR_ERR(regs);
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Can't retrieve our interrupt\n");
+		return irq;
+	}
+
+	i2s->bus_clk = devm_clk_get(&pdev->dev, "apb");
+	if (IS_ERR(i2s->bus_clk)) {
+		dev_err(&pdev->dev, "Can't get our bus clock\n");
+		return PTR_ERR(i2s->bus_clk);
+	}
+
+	i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
+					     &sun4i_i2s_regmap_config);
+	if (IS_ERR(i2s->regmap)) {
+		dev_err(&pdev->dev, "Regmap initialisation failed\n");
+		return PTR_ERR(i2s->regmap);
+	};
+
+	i2s->mod_clk = devm_clk_get(&pdev->dev, "mod");
+	if (IS_ERR(i2s->mod_clk)) {
+		dev_err(&pdev->dev, "Can't get our mod clock\n");
+		return PTR_ERR(i2s->mod_clk);
+	}
+	
+	i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG;
+	i2s->playback_dma_data.maxburst = 4;
+
+	pm_runtime_enable(&pdev->dev);
+	if (!pm_runtime_enabled(&pdev->dev)) {
+		ret = sun4i_i2s_runtime_resume(&pdev->dev);
+		if (ret)
+			goto err_pm_disable;
+	}
+
+	ret = devm_snd_soc_register_component(&pdev->dev,
+					      &sun4i_i2s_component,
+					      &sun4i_i2s_dai, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not register DAI\n");
+		goto err_suspend;
+	}
+
+	ret = snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not register PCM\n");
+		goto err_suspend;
+	}
+
+	return 0;
+
+err_suspend:
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		sun4i_i2s_runtime_suspend(&pdev->dev);
+err_pm_disable:
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
+}
+
+static int sun4i_i2s_remove(struct platform_device *pdev)
+{
+	snd_dmaengine_pcm_unregister(&pdev->dev);
+
+	pm_runtime_disable(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		sun4i_i2s_runtime_suspend(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id sun4i_i2s_match[] = {
+	{ .compatible = "allwinner,sun4i-a10-i2s", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
+
+static const struct dev_pm_ops sun4i_i2s_pm_ops = {
+	.runtime_resume		= sun4i_i2s_runtime_resume,
+	.runtime_suspend	= sun4i_i2s_runtime_suspend,
+};
+
+static struct platform_driver sun4i_i2s_driver = {
+	.probe	= sun4i_i2s_probe,
+	.remove	= sun4i_i2s_remove,
+	.driver	= {
+		.name		= "sun4i-i2s",
+		.of_match_table	= sun4i_i2s_match,
+		.pm		= &sun4i_i2s_pm_ops,
+	},
+};
+module_platform_driver(sun4i_i2s_driver);
+
+MODULE_AUTHOR("Andrea Venturi <be17068@iperbole.bo.it>");
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_DESCRIPTION("Allwinner A10 I2S driver");
+MODULE_LICENSE("GPL");
-- 
2.8.3

[PATCH 3/4] ARM: sun7i: Add mod1 clock nodes

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

This commit adds all the mod1 clocks available on A20 to its device
tree. This list was created by looking at the A20 user manual.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 48 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index febdf4c72fb0..603fda5b7f94 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -366,9 +366,9 @@
 					<5>, <6>, <7>,
 					<8>, <10>;
 			clock-output-names = "apb0_codec", "apb0_spdif",
-				"apb0_ac97", "apb0_iis0", "apb0_iis1",
+				"apb0_ac97", "apb0_i2s0", "apb0_i2s1",
 				"apb0_pio", "apb0_ir0", "apb0_ir1",
-				"apb0_iis2", "apb0_keypad";
+				"apb0_i2s2", "apb0_keypad";
 		};
 
 		apb1: clk@01c20058 {
@@ -518,6 +518,28 @@
 			clock-output-names = "ir1";
 		};
 
+		i2s0_clk: clk@01c200b8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200b8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "i2s0";
+		};
+
+		ac97_clk: clk@01c200bc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200bc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "ac97";
+		};
+
 		spdif_clk: clk@01c200c0 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod1-clk";
@@ -555,6 +577,28 @@
 			clock-output-names = "spi3";
 		};
 
+		i2s1_clk: clk@01c200d8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200d8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "i2s1";
+		};
+
+		i2s2_clk: clk@01c200dc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200dc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "i2s2";
+		};
+
 		dram_gates: clk@01c20100 {
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-dram-gates-clk";
-- 
2.8.3

[PATCH 4/4] ARM: sun7i: Add DAI nodes

[Maxime Ripard]

Add the new DAI blocks to the device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 603fda5b7f94..b656bc20ce59 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -1361,6 +1361,32 @@
 			status = "disabled";
 		};
 
+		i2s1: i2s@01c22000 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun4i-a10-i2s";
+			reg = <0x01c22000 0x400>;
+			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&apb0_gates 4>, <&i2s1_clk>;
+			clock-names = "apb", "mod";
+			dmas = <&dma SUN4I_DMA_NORMAL 4>,
+			       <&dma SUN4I_DMA_NORMAL 4>;
+			dma-names = "rx", "tx";
+			status = "disabled";
+		};
+
+		i2s0: i2s@01c22400 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun4i-a10-i2s";
+			reg = <0x01c22400 0x400>;
+			interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&apb0_gates 3>, <&i2s0_clk>;
+			clock-names = "apb", "mod";
+			dmas = <&dma SUN4I_DMA_NORMAL 3>,
+			       <&dma SUN4I_DMA_NORMAL 3>;
+			dma-names = "rx", "tx";
+			status = "disabled";
+		};
+
 		lradc: lradc@01c22800 {
 			compatible = "allwinner,sun4i-a10-lradc-keys";
 			reg = <0x01c22800 0x100>;
@@ -1386,6 +1412,19 @@
 			reg = <0x01c23800 0x200>;
 		};
 
+		i2s2: i2s@01c24400 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun4i-a10-i2s";
+			reg = <0x01c24400 0x400>;
+			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&apb0_gates 8>, <&i2s2_clk>;
+			clock-names = "apb", "mod";
+			dmas = <&dma SUN4I_DMA_NORMAL 6>,
+			       <&dma SUN4I_DMA_NORMAL 6>;
+			dma-names = "rx", "tx";
+			status = "disabled";
+		};
+
 		rtp: rtp@01c25000 {
 			compatible = "allwinner,sun5i-a13-ts";
 			reg = <0x01c25000 0x100>;
-- 
2.8.3

Re: [PATCH 2/4] ASoC: sunxi: Add Allwinner A10 Digital Audio driver

[Code Kipper]

snip
> +
> +       /* 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)
Hi Maxime,
this wouldn't work if one of the divs returns a valid value and I saw
this when working with this driver. Use if ((bclk_div < 0) ||
(mclk_div < 0)). Rest of the code looks code and I will have a go at
testing with it ASAP,
Great work,
CK
> +               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) |
> +                    SUN4I_I2S_CLK_DIV_MCLK_EN);
> +
> +       return 0;
> +}
> +
> +static int sun4i_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;
> +
> +       if (params_channels(params) != 2)
> +               return -EINVAL;
> +
> +       /* Enable the first output line */
> +       regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +                          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));
> +
> +       switch (params_physical_width(params)) {
> +       case 16:
> +               width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +       i2s->playback_dma_data.addr_width = width;
> +
> +       sr = sun4i_i2s_params_to_sr(params);
> +       if (sr < 0)
> +               return -EINVAL;
> +
> +       wss = sun4i_i2s_params_to_wss(params);
> +       if (wss < 0)
> +               return -EINVAL;
> +
> +       regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> +                          SUN4I_I2S_FMT0_WSS_MASK | SUN4I_I2S_FMT0_SR_MASK,
> +                          SUN4I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
> +
> +       return sun4i_i2s_set_clk_rate(i2s, params_rate(params),
> +                                     params_width(params));
> +}
> +
> +static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> +        struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> +       u32 val;
> +
> +       /* DAI Mode */
> +       switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +       case SND_SOC_DAIFMT_I2S:
> +               val = SUN4I_I2S_FMT0_FMT_I2S;
> +               break;
> +       case SND_SOC_DAIFMT_LEFT_J:
> +               val = SUN4I_I2S_FMT0_FMT_LEFT_J;
> +               break;
> +       case SND_SOC_DAIFMT_RIGHT_J:
> +               val = SUN4I_I2S_FMT0_FMT_RIGHT_J;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> +                          SUN4I_I2S_FMT0_FMT_MASK,
> +                          val);
> +
> +       /* DAI clock polarity */
> +       switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +       case SND_SOC_DAIFMT_IB_IF:
> +               /* Invert both clocks */
> +               val = SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED |
> +                       SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
> +               break;
> +       case SND_SOC_DAIFMT_IB_NF:
> +               /* Invert bit clock */
> +               val = SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED |
> +                       SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
> +               break;
> +       case SND_SOC_DAIFMT_NB_IF:
> +               /* Invert frame clock */
> +               val = SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
> +                       SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL;
> +               break;
> +       case SND_SOC_DAIFMT_NB_NF:
> +               /* Nothing to do for both normal cases */
> +               val = SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL |
> +                       SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> +                          SUN4I_I2S_FMT0_BCLK_POLARITY_MASK |
> +                          SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK,
> +                          val);
> +
> +       /* DAI clock master masks */
> +       switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +       case SND_SOC_DAIFMT_CBS_CFS:
> +               /* BCLK and LRCLK master */
> +               val = SUN4I_I2S_CTRL_MODE_MASTER;
> +               break;
> +       case SND_SOC_DAIFMT_CBM_CFM:
> +               /* BCLK and LRCLK slave */
> +               val = SUN4I_I2S_CTRL_MODE_SLAVE;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +                          SUN4I_I2S_CTRL_MODE_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_playback(struct sun4i_i2s *i2s)
> +{
> +       /* Flush TX FIFO */
> +        regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> +                          SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
> +                          SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
> +
> +        /* Clear TX counter */
> +       regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
> +
> +        /* Enable TX Block */
> +        regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +                          SUN4I_I2S_CTRL_TX_EN,
> +                          SUN4I_I2S_CTRL_TX_EN);
> +
> +        /* Enable TX DRQ */
> +        regmap_update_bits(i2s->regmap, SUN4I_I2S_DMA_INT_CTRL_REG,
> +                          SUN4I_I2S_DMA_INT_CTRL_TX_DRQ_EN,
> +                          SUN4I_I2S_DMA_INT_CTRL_TX_DRQ_EN);
> +}
> +
> +
> +static void sun4i_i2s_stop_playback(struct sun4i_i2s *i2s)
> +{
> +        /* Disable TX Block */
> +        regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +                          SUN4I_I2S_CTRL_TX_EN,
> +                          0);
> +
> +        /* Disable TX DRQ */
> +        regmap_update_bits(i2s->regmap, SUN4I_I2S_DMA_INT_CTRL_REG,
> +                          SUN4I_I2S_DMA_INT_CTRL_TX_DRQ_EN,
> +                          0);
> +}
> +
> +static int sun4i_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> +                            struct snd_soc_dai *dai)
> +{
> +       struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> +
> +       switch (cmd) {
> +       case SNDRV_PCM_TRIGGER_START:
> +       case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +       case SNDRV_PCM_TRIGGER_RESUME:
> +               if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +                       sun4i_i2s_start_playback(i2s);
> +               else
> +                       return -EINVAL;
> +               break;
> +
> +       case SNDRV_PCM_TRIGGER_STOP:
> +       case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +       case SNDRV_PCM_TRIGGER_SUSPEND:
> +               if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +                       sun4i_i2s_stop_playback(i2s);
> +               else
> +                       return -EINVAL;
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +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);
> +
> +       return clk_prepare_enable(i2s->mod_clk);
> +}
> +
> +static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
> +                              struct snd_soc_dai *dai)
> +{
> +       struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> +
> +       return clk_disable_unprepare(i2s->mod_clk);
> +}
> +
> +static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
> +       .hw_params      = sun4i_i2s_hw_params,
> +       .set_fmt        = sun4i_i2s_set_fmt,
> +       .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);
> +
> +       /* Enable the whole hardware block */
> +       regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +                    SUN4I_I2S_CTRL_GL_EN);
> +
> +       snd_soc_dai_init_dma_data(dai, &i2s->playback_dma_data, NULL);
> +
> +       snd_soc_dai_set_drvdata(dai, i2s);
> +
> +       return 0;
> +}
> +
> +static struct snd_soc_dai_driver sun4i_i2s_dai = {
> +       .probe = sun4i_i2s_dai_probe,
> +       .playback = {
> +               .stream_name = "Playback",
> +               .channels_min = 2,
> +               .channels_max = 2,
> +               .rates = SNDRV_PCM_RATE_8000_192000,
> +               .formats = SNDRV_PCM_FMTBIT_S16_LE,
> +       },
> +       .ops = &sun4i_i2s_dai_ops,
> +       .symmetric_rates = 1,
> +};
> +
> +static const struct snd_soc_component_driver sun4i_i2s_component = {
> +       .name   = "sun4i-dai",
> +};
> +
> +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;
> +       }
> +}
> +
> +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:
> +               return true;
> +       }
> +}
> +
> +static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +       switch (reg) {
> +       case SUN4I_I2S_FIFO_RX_REG:
> +       case SUN4I_I2S_INT_STA_REG:
> +       case SUN4I_I2S_RX_CNT_REG:
> +       case SUN4I_I2S_TX_CNT_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 },
> +       { SUN4I_I2S_FMT1_REG, 0x00004020 },
> +       { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
> +       { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
> +       { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
> +       { SUN4I_I2S_TX_CHAN_SEL_REG, 0x00000001 },
> +       { SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210 },
> +       { SUN4I_I2S_RX_CHAN_SEL_REG, 0x00000001 },
> +       { SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
> +};
> +
> +static const struct regmap_config sun4i_i2s_regmap_config = {
> +       .reg_bits       = 32,
> +       .reg_stride     = 4,
> +       .val_bits       = 32,
> +       .max_register   = SUN4I_I2S_RX_CHAN_MAP_REG,
> +
> +       .cache_type     = REGCACHE_FLAT,
> +       .reg_defaults   = sun4i_i2s_reg_defaults,
> +       .num_reg_defaults       = ARRAY_SIZE(sun4i_i2s_reg_defaults),
> +       .writeable_reg  = sun4i_i2s_wr_reg,
> +       .readable_reg   = sun4i_i2s_rd_reg,
> +       .volatile_reg   = sun4i_i2s_volatile_reg,
> +};
> +
> +static int sun4i_i2s_runtime_resume(struct device *dev)
> +{
> +       struct sun4i_i2s *i2s = dev_get_drvdata(dev);
> +       int ret;
> +
> +       ret = clk_prepare_enable(i2s->bus_clk);
> +       if (ret) {
> +               dev_err(dev, "Failed to enable bus clock\n");
> +               return ret;
> +       }
> +
> +       regcache_cache_only(i2s->regmap, false);
> +       regcache_mark_dirty(i2s->regmap);
> +
> +       ret = regcache_sync(i2s->regmap);
> +       if (ret) {
> +               dev_err(dev, "Failed to sync regmap cache\n");
> +               goto err_disable_clk;
> +       }
> +
> +       return 0;
> +
> +err_disable_clk:
> +       clk_disable_unprepare(i2s->bus_clk);
> +       return ret;
> +}
> +
> +static int sun4i_i2s_runtime_suspend(struct device *dev)
> +{
> +       struct sun4i_i2s *i2s = dev_get_drvdata(dev);
> +
> +       regcache_cache_only(i2s->regmap, true);
> +
> +       clk_disable_unprepare(i2s->bus_clk);
> +
> +       return 0;
> +}
> +
> +static int sun4i_i2s_probe(struct platform_device *pdev)
> +{
> +       struct sun4i_i2s *i2s;
> +       struct resource *res;
> +       void __iomem *regs;
> +       int irq, ret;
> +
> +       i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
> +       if (!i2s)
> +               return -ENOMEM;
> +       platform_set_drvdata(pdev, i2s);
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       regs = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(regs)) {
> +               dev_err(&pdev->dev, "Can't request IO region\n");
> +               return PTR_ERR(regs);
> +       }
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0) {
> +               dev_err(&pdev->dev, "Can't retrieve our interrupt\n");
> +               return irq;
> +       }
> +
> +       i2s->bus_clk = devm_clk_get(&pdev->dev, "apb");
> +       if (IS_ERR(i2s->bus_clk)) {
> +               dev_err(&pdev->dev, "Can't get our bus clock\n");
> +               return PTR_ERR(i2s->bus_clk);
> +       }
> +
> +       i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
> +                                            &sun4i_i2s_regmap_config);
> +       if (IS_ERR(i2s->regmap)) {
> +               dev_err(&pdev->dev, "Regmap initialisation failed\n");
> +               return PTR_ERR(i2s->regmap);
> +       };
> +
> +       i2s->mod_clk = devm_clk_get(&pdev->dev, "mod");
> +       if (IS_ERR(i2s->mod_clk)) {
> +               dev_err(&pdev->dev, "Can't get our mod clock\n");
> +               return PTR_ERR(i2s->mod_clk);
> +       }
> +
> +       i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG;
> +       i2s->playback_dma_data.maxburst = 4;
> +
> +       pm_runtime_enable(&pdev->dev);
> +       if (!pm_runtime_enabled(&pdev->dev)) {
> +               ret = sun4i_i2s_runtime_resume(&pdev->dev);
> +               if (ret)
> +                       goto err_pm_disable;
> +       }
> +
> +       ret = devm_snd_soc_register_component(&pdev->dev,
> +                                             &sun4i_i2s_component,
> +                                             &sun4i_i2s_dai, 1);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Could not register DAI\n");
> +               goto err_suspend;
> +       }
> +
> +       ret = snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Could not register PCM\n");
> +               goto err_suspend;
> +       }
> +
> +       return 0;
> +
> +err_suspend:
> +       if (!pm_runtime_status_suspended(&pdev->dev))
> +               sun4i_i2s_runtime_suspend(&pdev->dev);
> +err_pm_disable:
> +       pm_runtime_disable(&pdev->dev);
> +
> +       return ret;
> +}
> +
> +static int sun4i_i2s_remove(struct platform_device *pdev)
> +{
> +       snd_dmaengine_pcm_unregister(&pdev->dev);
> +
> +       pm_runtime_disable(&pdev->dev);
> +       if (!pm_runtime_status_suspended(&pdev->dev))
> +               sun4i_i2s_runtime_suspend(&pdev->dev);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id sun4i_i2s_match[] = {
> +       { .compatible = "allwinner,sun4i-a10-i2s", },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
> +
> +static const struct dev_pm_ops sun4i_i2s_pm_ops = {
> +       .runtime_resume         = sun4i_i2s_runtime_resume,
> +       .runtime_suspend        = sun4i_i2s_runtime_suspend,
> +};
> +
> +static struct platform_driver sun4i_i2s_driver = {
> +       .probe  = sun4i_i2s_probe,
> +       .remove = sun4i_i2s_remove,
> +       .driver = {
> +               .name           = "sun4i-i2s",
> +               .of_match_table = sun4i_i2s_match,
> +               .pm             = &sun4i_i2s_pm_ops,
> +       },
> +};
> +module_platform_driver(sun4i_i2s_driver);
> +
> +MODULE_AUTHOR("Andrea Venturi <be17068@iperbole.bo.it>");
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Allwinner A10 I2S driver");
> +MODULE_LICENSE("GPL");
> --
> 2.8.3
>

Re: [PATCH 1/4] dt-bindings: Add A10 I2S controller binding documentation

[Mark Brown]

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

On Wed, Jun 01, 2016 at 07:54:27PM +0200, Maxime Ripard wrote:
> Introduce the device tree binding for the I2S controller found in the
> Allwinner A10 and later SoCs.

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

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

Re: [PATCH 2/4] ASoC: sunxi: Add Allwinner A10 Digital Audio driver

[Mark Brown]

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

On Wed, Jun 01, 2016 at 07:54:28PM +0200, Maxime Ripard wrote:

> @@ -16,4 +25,5 @@ config SND_SUN4I_SPDIF
>  	help
>  	  Say Y or M to add support for the S/PDIF audio block in the Allwinner
>  	  A10 and affiliated SoCs.
> +
>  endmenu

Unrelated whitespace change.

> +static int sun4i_i2s_params_to_sr(struct snd_pcm_hw_params *params)
> +{
> +	switch (params_width(params)) {
> +	case 16:
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}

The switch statement here and in the _wss() function look weird because
they don't have default cases.  Since there's only one user of both
functions it seems better to have the switch statements inline anyway.

> +	for (i = 0; sun4i_i2s_mclk_div[i].div; i++) {
> +		const struct sun4i_i2s_clk_div *mdiv = sun4i_i2s_mclk_div + i;

Why not just write these as normal array lookups?

> +	switch (rate) {
> +        case 176400:
> +        case 88200:

Weird indentation here...  it also seems spaces not tabs are being
used for the case labels.

> +	clk_set_rate(i2s->mod_clk, clk_rate);

Should really check the return value.

> +	/* Enable the first output line */
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +			   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));

We don't undo these if setup fails...  do them once on probe?

> +static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> +        struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> +	u32 val;

More tab/space damage, there seems to be quite a bit in the rest of the
driver.

> +static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
> +{
> +	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> +
> +	/* Enable the whole hardware block */
> +	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +		     SUN4I_I2S_CTRL_GL_EN);

Runtime PM?  It also seems like this is something that ought to be
covered in the suspend and resume callbacks.

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

Re: [PATCH 1/4] dt-bindings: Add A10 I2S controller binding documentation

[Rob Herring]

On Wed, Jun 01, 2016 at 07:54:27PM +0200, Maxime Ripard wrote:
> Introduce the device tree binding for the I2S controller found in the
> Allwinner A10 and later SoCs.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/sound/sun4i-i2s.txt        | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> new file mode 100644
> index 000000000000..365ca4eede5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -0,0 +1,33 @@
> +* Allwinner A10 I2S controller
> +
> +The I2S bus (Inter-IC sound bus) is a serial link for digital
> +audio data transfer between devices in the system.
> +
> +Required properties:
> +
> +- compatible: should be one of the followings
> +   - "allwinner,sun4i-a10-i2s"
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +- interrupts: should contain the I2S interrupt.
> +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding,
> +	Documentation/devicetree/bindings/dma/dma.txt
> +- dma-names: should include "tx" and "rx".
> +- clocks: a list of phandle + clock-specifer pairs, one for each entry in clock-names.
> +- clock-names: should contain followings:
> +   - "apb" : clock for the I2S bus interface
> +   - "mod" : module clock for the I2S controller
> +
> +Example:
> +
> +i2s0: i2s@01c22400 {
> +	#sound-dai-cells = <0>;

This is missing from the property list.

> +	compatible = "allwinner,sun4i-a10-i2s";
> +	reg = <0x01c22400 0x400>;
> +	interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +	clocks = <&apb0_gates 3>, <&i2s0_clk>;
> +	clock-names = "apb", "mod";
> +	dmas = <&dma SUN4I_DMA_NORMAL 3>,
> +	       <&dma SUN4I_DMA_NORMAL 3>;
> +	dma-names = "rx", "tx";
> +};
> -- 
> 2.8.3
> 

Re: [PATCH 1/4] dt-bindings: Add A10 I2S controller binding documentation

[Maxime Ripard]

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

Hi Rob,

On Mon, Jun 06, 2016 at 08:08:18AM -0500, Rob Herring wrote:
> On Wed, Jun 01, 2016 at 07:54:27PM +0200, Maxime Ripard wrote:
> > Introduce the device tree binding for the I2S controller found in the
> > Allwinner A10 and later SoCs.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  .../devicetree/bindings/sound/sun4i-i2s.txt        | 33 ++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> > new file mode 100644
> > index 000000000000..365ca4eede5f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> > @@ -0,0 +1,33 @@
> > +* Allwinner A10 I2S controller
> > +
> > +The I2S bus (Inter-IC sound bus) is a serial link for digital
> > +audio data transfer between devices in the system.
> > +
> > +Required properties:
> > +
> > +- compatible: should be one of the followings
> > +   - "allwinner,sun4i-a10-i2s"
> > +- reg: physical base address of the controller and length of memory mapped
> > +  region.
> > +- interrupts: should contain the I2S interrupt.
> > +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding,
> > +	Documentation/devicetree/bindings/dma/dma.txt
> > +- dma-names: should include "tx" and "rx".
> > +- clocks: a list of phandle + clock-specifer pairs, one for each entry in clock-names.
> > +- clock-names: should contain followings:
> > +   - "apb" : clock for the I2S bus interface
> > +   - "mod" : module clock for the I2S controller
> > +
> > +Example:
> > +
> > +i2s0: i2s@01c22400 {
> > +	#sound-dai-cells = <0>;
> 
> This is missing from the property list.

Indeed, I'll add it.

Thanks!
Maxime

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

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

Re: [PATCH 2/4] ASoC: sunxi: Add Allwinner A10 Digital Audio driver

[Maxime Ripard]

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

Hi Mark,

On Thu, Jun 02, 2016 at 11:26:51AM +0100, Mark Brown wrote:
> > +static int sun4i_i2s_params_to_sr(struct snd_pcm_hw_params *params)
> > +{
> > +	switch (params_width(params)) {
> > +	case 16:
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> 
> The switch statement here and in the _wss() function look weird because
> they don't have default cases.  Since there's only one user of both
> functions it seems better to have the switch statements inline anyway.

I don't know, maybe it's just me, but I really find it cleaner that
way, and the compiler will probably inline it anyway. If you insist,
I'll change it though.


> > +	for (i = 0; sun4i_i2s_mclk_div[i].div; i++) {
> > +		const struct sun4i_i2s_clk_div *mdiv = sun4i_i2s_mclk_div + i;
> 
> Why not just write these as normal array lookups?

By normal, you mean using ARRAY_SIZE()?

> > +	/* Enable the first output line */
> > +	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> > +			   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));
> 
> We don't undo these if setup fails...  do them once on probe?

Yep

> > +static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> > +{
> > +        struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> > +	u32 val;
> 
> More tab/space damage, there seems to be quite a bit in the rest of the
> driver.

Indeed, sorry for that.

> > +static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
> > +{
> > +	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> > +
> > +	/* Enable the whole hardware block */
> > +	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
> > +		     SUN4I_I2S_CTRL_GL_EN);
> 
> Runtime PM?  It also seems like this is something that ought to be
> covered in the suspend and resume callbacks.

runtime_pm is supported, and uses the regmap cache to keep those
changes.

suspend and resume is not supported for that SoC yet, so I couldn't
test it properly (and isn't that redundant with runtime_pm?)

Thanks!
Maxime

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

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

Re: [PATCH 2/4] ASoC: sunxi: Add Allwinner A10 Digital Audio driver

[Maxime Ripard]

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

Hi Markus,

On Thu, Jun 02, 2016 at 10:03:21AM +0200, Code Kipper wrote:
> snip
> > +
> > +       /* 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)
>
> this wouldn't work if one of the divs returns a valid value and I saw
> this when working with this driver. Use if ((bclk_div < 0) ||
> (mclk_div < 0)). Rest of the code looks code and I will have a go at
> testing with it ASAP,

Ack, thanks!
Maxime

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

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

Re: [PATCH 2/4] ASoC: sunxi: Add Allwinner A10 Digital Audio driver

[Mark Brown]

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

On Thu, Jun 09, 2016 at 11:05:15PM +0200, Maxime Ripard wrote:
> On Thu, Jun 02, 2016 at 11:26:51AM +0100, Mark Brown wrote:
> > > +static int sun4i_i2s_params_to_sr(struct snd_pcm_hw_params *params)
> > > +{
> > > +	switch (params_width(params)) {
> > > +	case 16:
> > > +		return 0;
> > > +	}
> > > +
> > > +	return -EINVAL;
> > > +}

> > The switch statement here and in the _wss() function look weird because
> > they don't have default cases.  Since there's only one user of both
> > functions it seems better to have the switch statements inline anyway.

> I don't know, maybe it's just me, but I really find it cleaner that
> way, and the compiler will probably inline it anyway. If you insist,
> I'll change it though.

Yes, it's really not helping here.

> > > +	for (i = 0; sun4i_i2s_mclk_div[i].div; i++) {
> > > +		const struct sun4i_i2s_clk_div *mdiv = sun4i_i2s_mclk_div + i;

> > Why not just write these as normal array lookups?

> By normal, you mean using ARRAY_SIZE()?

array[index].

> > > +static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
> > > +{
> > > +	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);

> > > +	/* Enable the whole hardware block */
> > > +	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
> > > +		     SUN4I_I2S_CTRL_GL_EN);

> > Runtime PM?  It also seems like this is something that ought to be
> > covered in the suspend and resume callbacks.

> runtime_pm is supported, and uses the regmap cache to keep those
> changes.

No, my point is that I'd expect to see the block powered off on suspend.

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

Re: [PATCH 2/4] ASoC: sunxi: Add Allwinner A10 Digital Audio driver

[Maxime Ripard]

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

Hi Mark,

On Fri, Jun 10, 2016 at 01:37:55AM +0100, Mark Brown wrote:
> > > > +static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
> > > > +{
> > > > +	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> 
> > > > +	/* Enable the whole hardware block */
> > > > +	regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
> > > > +		     SUN4I_I2S_CTRL_GL_EN);
> 
> > > Runtime PM?  It also seems like this is something that ought to be
> > > covered in the suspend and resume callbacks.
> 
> > runtime_pm is supported, and uses the regmap cache to keep those
> > changes.
> 
> No, my point is that I'd expect to see the block powered off on suspend.

It is. The bus clock is disabled, which also puts the IP in reset.

Maxime

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

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