linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Rockchip I2S/TDM controller
@ 2021-08-20 18:27 Nicolas Frattaroli
  2021-08-20 18:27 ` [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller Nicolas Frattaroli
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Nicolas Frattaroli @ 2021-08-20 18:27 UTC (permalink / raw)
  Cc: Nicolas Frattaroli, alsa-devel, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Changes in v2:
 - remove ad-hoc writeq and needless (and broken) optimisation in
   reset assert/deassert. This wouldn't have worked on Big Endian,
   and would've been pointless on any other platform, as the
   overhead for saving one write was comparatively big
 - fix various checkpatch issues
 - get rid of leftover clk-trcm in schema
 - set status = "okay" in example in schema instead of "disabled"
 - change dma-names so rx is first, adjust device trees as necessary
 - properly reference uint32-array for rx-route and tx-route
   instead of uint32
 - replace trcm-sync with two boolean properties, adjust DT changes
   accordingly and also get rid of the header file
 - get rid of rockchip,no-dmaengine. This was only needed for
   some downstream driver and shouldn't be in the DT
 - get rid of rockchip,capture-only/playback-only. Rationale being
   that I have no way to test whether they're needed, and
   unconditionally setting channels_min to 0 breaks everything
 - change hclk description in "clocks"

Additionally, I have been made aware that registers and code seem
to be an extension of the rockchip_i2s driver. This is true,
though merging the two seems like more effort than just slapping
a few compatibles and properties into the mix. I'll investigate
it further in the coming weeks, but would likely still prefer
basing any unified driver on mine rather than the other way
around, since I think it has better code quality now.

Original cover letter:

Hello,

I come bearing four patches for your consideration.

The first of these four patches adds a driver for the Rockchip
I2S/TDM controller, used in interfacing between the AHB bus and the
I2S bus on some Rockchip SoCs. This allows for audio playback with
a matching codec.

The controller has three different modes: I2S, I2S/TDM and PCM.
It is distinct from the earlier Rockchip I2S controller, and
therefore not just an extension of that driver.

The driver is based on the downstream version, though various
changes have been made to hopefully make it more palatable to
upstream. Some needless code duplication has been refactored, and
the probe function will no longer let wrong device tree values
write nonsense to hardware registers. Properties have been renamed
and had their semantics changed. I won't bore you with the details
of what downstream did, since that's not what I'm submitting, but
the changes are significant enough that I've added myself to the
list of authors.

The second patch adds the YAML device tree bindings for this, which
have been written from scratch by yours truly. Since I didn't like
having random integers mean things, I defined them as constants in
a header file for the bindings.

The third patch adds the i2s1 controller to the rk356x device tree.
I didn't add any of the other i2s controllers on that SoC for now as
I have no way of testing them; in particular, i2s0 is tied to HDMI,
so needs a functioning VOP2 driver to even have a chance of working.

The fourth patch makes use of the i2s1 controller to enable analog
audio output on the Quartz64 Model A through its RK817 codec. I've
tested this to work properly at both 44.1 kHz and 96 kHz, so both
mclk_root0 and mclk_root1 are definitely functioning.

This is my first kernel contribution, so I most likely did
something horribly wrong. That's why I'm more than happy to receive
any criticisms and concerns over how the driver is implemented,
because I've run out of ideas on how to make it clearly better
myself.

I'd also like to extend my thanks to Peter Geis, who has been
acting as somewhat of a mentor and gave me occasional feedback
and ideas during the writing of this patch series.

Regards,
Nicolas Frattaroli

Nicolas Frattaroli (4):
  ASoC: rockchip: add support for i2s-tdm controller
  dt-bindings: sound: add rockchip i2s-tdm binding
  arm64: dts: rockchip: add i2s1 on rk356x
  arm64: dts: rockchip: add analog audio on Quartz64

 .../bindings/sound/rockchip,i2s-tdm.yaml      |  193 ++
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   |   35 +-
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |   26 +
 sound/soc/rockchip/Kconfig                    |   11 +
 sound/soc/rockchip/Makefile                   |    2 +
 sound/soc/rockchip/rockchip_i2s_tdm.c         | 1737 +++++++++++++++++
 sound/soc/rockchip/rockchip_i2s_tdm.h         |  398 ++++
 7 files changed, 2401 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
 create mode 100644 sound/soc/rockchip/rockchip_i2s_tdm.c
 create mode 100644 sound/soc/rockchip/rockchip_i2s_tdm.h

-- 
2.32.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-20 18:27 [PATCH v2 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
@ 2021-08-20 18:27 ` Nicolas Frattaroli
  2021-08-20 19:02   ` Pierre-Louis Bossart
  2021-08-23 12:03   ` Philipp Zabel
  2021-08-20 18:27 ` [PATCH v2 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Nicolas Frattaroli @ 2021-08-20 18:27 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Heiko Stuebner, Philipp Zabel
  Cc: Nicolas Frattaroli, linux-kernel, alsa-devel, linux-arm-kernel,
	linux-rockchip

This commit adds support for the rockchip i2s-tdm controller,
which enables audio output on the following rockchip SoCs:
- px30
- rk1808
- rk3308
- rk3566
- rk3568
- rv1126

This is a cleaned up version of the downstream vendor kernel's
driver. It can be enabled through the SND_SOC_ROCKCHIP_I2S_TDM
configuration option.

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 sound/soc/rockchip/Kconfig            |   11 +
 sound/soc/rockchip/Makefile           |    2 +
 sound/soc/rockchip/rockchip_i2s_tdm.c | 1737 +++++++++++++++++++++++++
 sound/soc/rockchip/rockchip_i2s_tdm.h |  398 ++++++
 4 files changed, 2148 insertions(+)
 create mode 100644 sound/soc/rockchip/rockchip_i2s_tdm.c
 create mode 100644 sound/soc/rockchip/rockchip_i2s_tdm.h

diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index 053097b73e28..42f76bc0fb02 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -16,6 +16,17 @@ config SND_SOC_ROCKCHIP_I2S
 	  Rockchip I2S device. The device supports upto maximum of
 	  8 channels each for play and record.
 
+config SND_SOC_ROCKCHIP_I2S_TDM
+	tristate "Rockchip I2S/TDM Device Driver"
+	depends on HAVE_CLK && SND_SOC_ROCKCHIP
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	help
+	  Say Y or M if you want to add support for the I2S/TDM driver for
+	  Rockchip I2S/TDM devices, found in Rockchip SoCs. These devices
+	  interface between the AHB bus and the I2S bus, and support up to a
+	  maximum of 8 channels each for playback and recording.
+
+
 config SND_SOC_ROCKCHIP_PDM
 	tristate "Rockchip PDM Controller Driver"
 	depends on HAVE_CLK && SND_SOC_ROCKCHIP
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index 65e814d46006..b10f5e7b136d 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -1,11 +1,13 @@
 # SPDX-License-Identifier: GPL-2.0
 # ROCKCHIP Platform Support
 snd-soc-rockchip-i2s-objs := rockchip_i2s.o
+snd-soc-rockchip-i2s-tdm-objs := rockchip_i2s_tdm.o
 snd-soc-rockchip-pcm-objs := rockchip_pcm.o
 snd-soc-rockchip-pdm-objs := rockchip_pdm.o
 snd-soc-rockchip-spdif-objs := rockchip_spdif.o
 
 obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-rockchip-i2s.o snd-soc-rockchip-pcm.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S_TDM) += snd-soc-rockchip-i2s-tdm.o
 obj-$(CONFIG_SND_SOC_ROCKCHIP_PDM) += snd-soc-rockchip-pdm.o
 obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-rockchip-spdif.o
 
diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
new file mode 100644
index 000000000000..c02b66f3c913
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
@@ -0,0 +1,1737 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ALSA SoC Audio Layer - Rockchip I2S/TDM Controller driver
+ *
+ * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
+ * Author: Sugar Zhang <sugar.zhang@rock-chips.com>
+ * Author: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/delay.h>
+#include <linux/of_gpio.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+#include <sound/pcm_params.h>
+#include <sound/dmaengine_pcm.h>
+
+#include "rockchip_i2s_tdm.h"
+
+#define DRV_NAME "rockchip-i2s-tdm"
+
+#define DEFAULT_MCLK_FS				256
+#define CH_GRP_MAX				4  /* The max channel 8 / 2 */
+#define MULTIPLEX_CH_MAX			10
+#define CLK_PPM_MIN				(-1000)
+#define CLK_PPM_MAX				(1000)
+
+#define TRCM_TXRX 0
+#define TRCM_TX 1
+#define TRCM_RX 2
+
+struct txrx_config {
+	u32 addr;
+	u32 reg;
+	u32 txonly;
+	u32 rxonly;
+};
+
+struct rk_i2s_soc_data {
+	u32 softrst_offset;
+	u32 grf_reg_offset;
+	u32 grf_shift;
+	int config_count;
+	const struct txrx_config *configs;
+	int (*init)(struct device *dev, u32 addr);
+};
+
+struct rk_i2s_tdm_dev {
+	struct device *dev;
+	struct clk *hclk;
+	struct clk *mclk_tx;
+	struct clk *mclk_rx;
+	/* The mclk_tx_src is parent of mclk_tx */
+	struct clk *mclk_tx_src;
+	/* The mclk_rx_src is parent of mclk_rx */
+	struct clk *mclk_rx_src;
+	/*
+	 * The mclk_root0 and mclk_root1 are root parent and supplies for
+	 * the different FS.
+	 *
+	 * e.g:
+	 * mclk_root0 is VPLL0, used for FS=48000Hz
+	 * mclk_root1 is VPLL1, used for FS=44100Hz
+	 */
+	struct clk *mclk_root0;
+	struct clk *mclk_root1;
+	struct regmap *regmap;
+	struct regmap *grf;
+	struct snd_dmaengine_dai_dma_data capture_dma_data;
+	struct snd_dmaengine_dai_dma_data playback_dma_data;
+	struct reset_control *tx_reset;
+	struct reset_control *rx_reset;
+	struct rk_i2s_soc_data *soc_data;
+	void __iomem *cru_base;
+	bool is_master_mode;
+	bool io_multiplex;
+	bool mclk_calibrate;
+	bool tdm_mode;
+	bool tdm_fsync_half_frame;
+	unsigned int mclk_rx_freq;
+	unsigned int mclk_tx_freq;
+	unsigned int mclk_root0_freq;
+	unsigned int mclk_root1_freq;
+	unsigned int mclk_root0_initial_freq;
+	unsigned int mclk_root1_initial_freq;
+	unsigned int frame_width;
+	unsigned int clk_trcm;
+	unsigned int i2s_sdis[CH_GRP_MAX];
+	unsigned int i2s_sdos[CH_GRP_MAX];
+	int clk_ppm;
+	int tx_reset_id;
+	int rx_reset_id;
+	atomic_t refcount;
+	spinlock_t lock; /* xfer lock */
+};
+
+static int to_ch_num(unsigned int val)
+{
+	switch (val) {
+	case I2S_CHN_4:
+		return 4;
+	case I2S_CHN_6:
+		return 6;
+	case I2S_CHN_8:
+		return 8;
+	default:
+		return 2;
+	}
+}
+
+static int i2s_tdm_runtime_suspend(struct device *dev)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
+
+	regcache_cache_only(i2s_tdm->regmap, true);
+	if (!IS_ERR(i2s_tdm->mclk_tx))
+		clk_disable_unprepare(i2s_tdm->mclk_tx);
+	if (!IS_ERR(i2s_tdm->mclk_rx))
+		clk_disable_unprepare(i2s_tdm->mclk_rx);
+
+	return 0;
+}
+
+static int i2s_tdm_runtime_resume(struct device *dev)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
+	int ret;
+
+	if (!IS_ERR(i2s_tdm->mclk_tx))
+		clk_prepare_enable(i2s_tdm->mclk_tx);
+	if (!IS_ERR(i2s_tdm->mclk_rx))
+		clk_prepare_enable(i2s_tdm->mclk_rx);
+
+	regcache_cache_only(i2s_tdm->regmap, false);
+	regcache_mark_dirty(i2s_tdm->regmap);
+
+	ret = regcache_sync(i2s_tdm->regmap);
+	if (ret) {
+		if (!IS_ERR(i2s_tdm->mclk_tx))
+			clk_disable_unprepare(i2s_tdm->mclk_tx);
+		if (!IS_ERR(i2s_tdm->mclk_rx))
+			clk_disable_unprepare(i2s_tdm->mclk_rx);
+	}
+
+	return ret;
+}
+
+static inline struct rk_i2s_tdm_dev *to_info(struct snd_soc_dai *dai)
+{
+	return snd_soc_dai_get_drvdata(dai);
+}
+
+static void rockchip_snd_xfer_reset_assert(struct rk_i2s_tdm_dev *i2s_tdm,
+					   int tx_bank, int tx_offset,
+					   int rx_bank, int rx_offset)
+{
+	void __iomem *cru_reset;
+	unsigned long flags;
+
+	cru_reset = i2s_tdm->cru_base + i2s_tdm->soc_data->softrst_offset;
+
+	if (tx_bank == rx_bank) {
+		writel(BIT(tx_offset) | BIT(rx_offset) |
+		       (BIT(tx_offset) << 16) | (BIT(rx_offset) << 16),
+		       cru_reset + (tx_bank * 4));
+	} else {
+		local_irq_save(flags);
+		writel(BIT(tx_offset) | (BIT(tx_offset) << 16),
+		       cru_reset + (tx_bank * 4));
+		writel(BIT(rx_offset) | (BIT(rx_offset) << 16),
+		       cru_reset + (rx_bank * 4));
+		local_irq_restore(flags);
+	}
+}
+
+static void rockchip_snd_xfer_reset_deassert(struct rk_i2s_tdm_dev *i2s_tdm,
+					     int tx_bank, int tx_offset,
+					     int rx_bank, int rx_offset)
+{
+	void __iomem *cru_reset;
+	unsigned long flags;
+
+	cru_reset = i2s_tdm->cru_base + i2s_tdm->soc_data->softrst_offset;
+
+	if (tx_bank == rx_bank) {
+		writel((BIT(tx_offset) << 16) | (BIT(rx_offset) << 16),
+		       cru_reset + (tx_bank * 4));
+	} else {
+		local_irq_save(flags);
+		writel((BIT(tx_offset) << 16),
+		       cru_reset + (tx_bank * 4));
+		writel((BIT(rx_offset) << 16),
+		       cru_reset + (rx_bank * 4));
+		local_irq_restore(flags);
+	}
+}
+
+/*
+ * Makes sure that both tx and rx are reset at the same time to sync lrck
+ * when clk_trcm > 0.
+ */
+static void rockchip_snd_xfer_sync_reset(struct rk_i2s_tdm_dev *i2s_tdm)
+{
+	int tx_id, rx_id;
+	int tx_bank, rx_bank, tx_offset, rx_offset;
+
+	if (!i2s_tdm->cru_base || !i2s_tdm->soc_data)
+		return;
+
+	tx_id = i2s_tdm->tx_reset_id;
+	rx_id = i2s_tdm->rx_reset_id;
+	if (tx_id < 0 || rx_id < 0)
+		return;
+
+	tx_bank = tx_id / 16;
+	tx_offset = tx_id % 16;
+	rx_bank = rx_id / 16;
+	rx_offset = rx_id % 16;
+	dev_dbg(i2s_tdm->dev,
+		"tx_bank: %d, rx_bank: %d, tx_offset: %d, rx_offset: %d\n",
+		tx_bank, rx_bank, tx_offset, rx_offset);
+
+	rockchip_snd_xfer_reset_assert(i2s_tdm, tx_bank, tx_offset,
+				       rx_bank, rx_offset);
+
+	udelay(150);
+
+	rockchip_snd_xfer_reset_deassert(i2s_tdm, tx_bank, tx_offset,
+					 rx_bank, rx_offset);
+}
+
+static void rockchip_snd_reset(struct reset_control *rc)
+{
+	if (IS_ERR(rc))
+		return;
+
+	reset_control_assert(rc);
+	udelay(1);
+	reset_control_deassert(rc);
+}
+
+static void rockchip_snd_xfer_clear(struct rk_i2s_tdm_dev *i2s_tdm,
+				    unsigned int clr)
+{
+	unsigned int xfer_mask;
+	unsigned int xfer_val;
+	unsigned int val;
+	int retry = 10;
+	bool tx = clr & I2S_CLR_TXC;
+	bool rx = clr & I2S_CLR_RXC;
+
+	if (!(rx || tx))
+		return;
+
+	xfer_mask = (tx ? I2S_XFER_TXS_START : 0) |
+		    (rx ? I2S_XFER_RXS_START : 0);
+	xfer_val = (tx ? I2S_XFER_TXS_STOP : 0) |
+		   (rx ? I2S_XFER_RXS_STOP : 0);
+
+	regmap_update_bits(i2s_tdm->regmap, I2S_XFER, xfer_mask, xfer_val);
+	udelay(150);
+	regmap_update_bits(i2s_tdm->regmap, I2S_CLR, clr, clr);
+
+	regmap_read(i2s_tdm->regmap, I2S_CLR, &val);
+	/* Wait on the clear operation to finish */
+	while (val) {
+		regmap_read(i2s_tdm->regmap, I2S_CLR, &val);
+		retry--;
+		if (!retry) {
+			dev_warn(i2s_tdm->dev, "clear failed, reset %s%s\n",
+				 tx ? "tx" : "", rx ? "rx" : "");
+			if (rx && tx)
+				rockchip_snd_xfer_sync_reset(i2s_tdm);
+			else if (tx)
+				rockchip_snd_reset(i2s_tdm->tx_reset);
+			else if (rx)
+				rockchip_snd_reset(i2s_tdm->rx_reset);
+			break;
+		}
+	}
+}
+
+/* only used when clk_trcm > 0 */
+static void rockchip_snd_txrxctrl(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai, int on)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai);
+
+	spin_lock(&i2s_tdm->lock);
+	if (on) {
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+					   I2S_DMACR_TDE_ENABLE,
+					   I2S_DMACR_TDE_ENABLE);
+		else
+			regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+					   I2S_DMACR_RDE_ENABLE,
+					   I2S_DMACR_RDE_ENABLE);
+
+		if (atomic_inc_return(&i2s_tdm->refcount) == 1) {
+			rockchip_snd_xfer_sync_reset(i2s_tdm);
+			regmap_update_bits(i2s_tdm->regmap, I2S_XFER,
+					   I2S_XFER_TXS_START |
+					   I2S_XFER_RXS_START,
+					   I2S_XFER_TXS_START |
+					   I2S_XFER_RXS_START);
+		}
+	} else {
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+					   I2S_DMACR_TDE_ENABLE,
+					   I2S_DMACR_TDE_DISABLE);
+		else
+			regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+					   I2S_DMACR_RDE_ENABLE,
+					   I2S_DMACR_RDE_DISABLE);
+
+		if (atomic_dec_and_test(&i2s_tdm->refcount)) {
+			rockchip_snd_xfer_clear(i2s_tdm,
+						I2S_CLR_TXC | I2S_CLR_RXC);
+		}
+	}
+	spin_unlock(&i2s_tdm->lock);
+}
+
+static void rockchip_snd_txctrl(struct rk_i2s_tdm_dev *i2s_tdm, int on)
+{
+	if (on) {
+		regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+				   I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
+
+		regmap_update_bits(i2s_tdm->regmap, I2S_XFER,
+				   I2S_XFER_TXS_START,
+				   I2S_XFER_TXS_START);
+	} else {
+		regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+				   I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE);
+
+		rockchip_snd_xfer_clear(i2s_tdm, I2S_CLR_TXC);
+	}
+}
+
+static void rockchip_snd_rxctrl(struct rk_i2s_tdm_dev *i2s_tdm, int on)
+{
+	if (on) {
+		regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+				   I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
+
+		regmap_update_bits(i2s_tdm->regmap, I2S_XFER,
+				   I2S_XFER_RXS_START,
+				   I2S_XFER_RXS_START);
+	} else {
+		rockchip_snd_xfer_clear(i2s_tdm, I2S_CLR_RXC);
+	}
+}
+
+static int rockchip_i2s_tdm_set_fmt(struct snd_soc_dai *cpu_dai,
+				    unsigned int fmt)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = to_info(cpu_dai);
+	unsigned int mask = 0, val = 0, tdm_val = 0, txcr_val = 0, rxcr_val = 0;
+	int ret = 0;
+	bool is_tdm = i2s_tdm->tdm_mode;
+
+	pm_runtime_get_sync(cpu_dai->dev);
+	mask = I2S_CKR_MSS_MASK;
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		val = I2S_CKR_MSS_MASTER;
+		i2s_tdm->is_master_mode = true;
+		break;
+	case SND_SOC_DAIFMT_CBM_CFM:
+		val = I2S_CKR_MSS_SLAVE;
+		i2s_tdm->is_master_mode = false;
+		break;
+	default:
+		ret = -EINVAL;
+		goto err_pm_put;
+	}
+
+	regmap_update_bits(i2s_tdm->regmap, I2S_CKR, mask, val);
+
+	mask = I2S_CKR_CKP_MASK | I2S_CKR_TLP_MASK | I2S_CKR_RLP_MASK;
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		val = I2S_CKR_CKP_NORMAL |
+		      I2S_CKR_TLP_NORMAL |
+		      I2S_CKR_RLP_NORMAL;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		val = I2S_CKR_CKP_NORMAL |
+		      I2S_CKR_TLP_INVERTED |
+		      I2S_CKR_RLP_INVERTED;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		val = I2S_CKR_CKP_INVERTED |
+		      I2S_CKR_TLP_NORMAL |
+		      I2S_CKR_RLP_NORMAL;
+		break;
+	case SND_SOC_DAIFMT_IB_IF:
+		val = I2S_CKR_CKP_INVERTED |
+		      I2S_CKR_TLP_INVERTED |
+		      I2S_CKR_RLP_INVERTED;
+		break;
+	default:
+		ret = -EINVAL;
+		goto err_pm_put;
+	}
+
+	regmap_update_bits(i2s_tdm->regmap, I2S_CKR, mask, val);
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_RIGHT_J:
+		txcr_val = I2S_TXCR_IBM_RSJM;
+		rxcr_val = I2S_RXCR_IBM_RSJM;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		txcr_val = I2S_TXCR_IBM_LSJM;
+		rxcr_val = I2S_RXCR_IBM_LSJM;
+		break;
+	case SND_SOC_DAIFMT_I2S:
+		txcr_val = I2S_TXCR_IBM_NORMAL;
+		rxcr_val = I2S_RXCR_IBM_NORMAL;
+		break;
+	case SND_SOC_DAIFMT_DSP_A: /* PCM no delay mode */
+		txcr_val = I2S_TXCR_TFS_PCM;
+		rxcr_val = I2S_RXCR_TFS_PCM;
+		break;
+	case SND_SOC_DAIFMT_DSP_B: /* PCM delay 1 mode */
+		txcr_val = I2S_TXCR_TFS_PCM | I2S_TXCR_PBM_MODE(1);
+		rxcr_val = I2S_RXCR_TFS_PCM | I2S_RXCR_PBM_MODE(1);
+		break;
+	default:
+		ret = -EINVAL;
+		goto err_pm_put;
+	}
+
+	mask = I2S_TXCR_IBM_MASK | I2S_TXCR_TFS_MASK | I2S_TXCR_PBM_MASK;
+	regmap_update_bits(i2s_tdm->regmap, I2S_TXCR, mask, txcr_val);
+
+	mask = I2S_RXCR_IBM_MASK | I2S_RXCR_TFS_MASK | I2S_RXCR_PBM_MASK;
+	regmap_update_bits(i2s_tdm->regmap, I2S_RXCR, mask, rxcr_val);
+
+	if (is_tdm) {
+		switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+		case SND_SOC_DAIFMT_RIGHT_J:
+			val = I2S_TXCR_TFS_TDM_I2S;
+			tdm_val = TDM_SHIFT_CTRL(2);
+			break;
+		case SND_SOC_DAIFMT_LEFT_J:
+			val = I2S_TXCR_TFS_TDM_I2S;
+			tdm_val = TDM_SHIFT_CTRL(1);
+			break;
+		case SND_SOC_DAIFMT_I2S:
+			val = I2S_TXCR_TFS_TDM_I2S;
+			tdm_val = TDM_SHIFT_CTRL(0);
+			break;
+		case SND_SOC_DAIFMT_DSP_A:
+			val = I2S_TXCR_TFS_TDM_PCM;
+			tdm_val = TDM_SHIFT_CTRL(0);
+			break;
+		case SND_SOC_DAIFMT_DSP_B:
+			val = I2S_TXCR_TFS_TDM_PCM;
+			tdm_val = TDM_SHIFT_CTRL(2);
+			break;
+		default:
+			ret = -EINVAL;
+			goto err_pm_put;
+		}
+
+		tdm_val |= TDM_FSYNC_WIDTH_SEL1(1);
+		if (i2s_tdm->tdm_fsync_half_frame)
+			tdm_val |= TDM_FSYNC_WIDTH_HALF_FRAME;
+		else
+			tdm_val |= TDM_FSYNC_WIDTH_ONE_FRAME;
+
+		mask = I2S_TXCR_TFS_MASK;
+		regmap_update_bits(i2s_tdm->regmap, I2S_TXCR, mask, val);
+		regmap_update_bits(i2s_tdm->regmap, I2S_RXCR, mask, val);
+
+		mask = TDM_FSYNC_WIDTH_SEL1_MSK | TDM_FSYNC_WIDTH_SEL0_MSK |
+		       TDM_SHIFT_CTRL_MSK;
+		regmap_update_bits(i2s_tdm->regmap, I2S_TDM_TXCR,
+				   mask, tdm_val);
+		regmap_update_bits(i2s_tdm->regmap, I2S_TDM_RXCR,
+				   mask, tdm_val);
+	}
+
+err_pm_put:
+	pm_runtime_put(cpu_dai->dev);
+
+	return ret;
+}
+
+static void rockchip_i2s_tdm_xfer_pause(struct snd_pcm_substream *substream,
+					struct rk_i2s_tdm_dev *i2s_tdm)
+{
+	int stream;
+
+	stream = SNDRV_PCM_STREAM_LAST - substream->stream;
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+				   I2S_DMACR_TDE_ENABLE,
+				   I2S_DMACR_TDE_DISABLE);
+	else
+		regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+				   I2S_DMACR_RDE_ENABLE,
+				   I2S_DMACR_RDE_DISABLE);
+
+	rockchip_snd_xfer_clear(i2s_tdm, I2S_CLR_TXC | I2S_CLR_RXC);
+}
+
+static void rockchip_i2s_tdm_xfer_resume(struct snd_pcm_substream *substream,
+					 struct rk_i2s_tdm_dev *i2s_tdm)
+{
+	int stream;
+
+	stream = SNDRV_PCM_STREAM_LAST - substream->stream;
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+				   I2S_DMACR_TDE_ENABLE,
+				   I2S_DMACR_TDE_ENABLE);
+	else
+		regmap_update_bits(i2s_tdm->regmap, I2S_DMACR,
+				   I2S_DMACR_RDE_ENABLE,
+				   I2S_DMACR_RDE_ENABLE);
+
+	regmap_update_bits(i2s_tdm->regmap, I2S_XFER,
+			   I2S_XFER_TXS_START |
+			   I2S_XFER_RXS_START,
+			   I2S_XFER_TXS_START |
+			   I2S_XFER_RXS_START);
+}
+
+static int rockchip_i2s_tdm_clk_set_rate(struct rk_i2s_tdm_dev *i2s_tdm,
+					 struct clk *clk, unsigned long rate,
+					 int ppm)
+{
+	unsigned long rate_target;
+	int delta, ret;
+
+	if (ppm == i2s_tdm->clk_ppm)
+		return 0;
+
+	delta = (ppm < 0) ? -1 : 1;
+	delta *= (int)div64_u64((u64)rate * (u64)abs(ppm) + 500000,
+				1000000);
+
+	rate_target = rate + delta;
+
+	if (!rate_target)
+		return -EINVAL;
+
+	ret = clk_set_rate(clk, rate_target);
+	if (ret)
+		return ret;
+
+	i2s_tdm->clk_ppm = ppm;
+
+	return ret;
+}
+
+static int rockchip_i2s_tdm_calibrate_mclk(struct rk_i2s_tdm_dev *i2s_tdm,
+					   struct snd_pcm_substream *substream,
+					   unsigned int lrck_freq)
+{
+	struct clk *mclk_root;
+	struct clk *mclk_parent;
+	unsigned int mclk_root_freq;
+	unsigned int mclk_root_initial_freq;
+	unsigned int mclk_parent_freq;
+	unsigned int div, delta;
+	u64 ppm;
+	int ret;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		mclk_parent = i2s_tdm->mclk_tx_src;
+	else
+		mclk_parent = i2s_tdm->mclk_rx_src;
+
+	switch (lrck_freq) {
+	case 8000:
+	case 16000:
+	case 24000:
+	case 32000:
+	case 48000:
+	case 64000:
+	case 96000:
+	case 192000:
+		mclk_root = i2s_tdm->mclk_root0;
+		mclk_root_freq = i2s_tdm->mclk_root0_freq;
+		mclk_root_initial_freq = i2s_tdm->mclk_root0_initial_freq;
+		mclk_parent_freq = DEFAULT_MCLK_FS * 192000;
+		break;
+	case 11025:
+	case 22050:
+	case 44100:
+	case 88200:
+	case 176400:
+		mclk_root = i2s_tdm->mclk_root1;
+		mclk_root_freq = i2s_tdm->mclk_root1_freq;
+		mclk_root_initial_freq = i2s_tdm->mclk_root1_initial_freq;
+		mclk_parent_freq = DEFAULT_MCLK_FS * 176400;
+		break;
+	default:
+		dev_err(i2s_tdm->dev, "Invalid LRCK freq: %u Hz\n",
+			lrck_freq);
+		return -EINVAL;
+	}
+
+	ret = clk_set_parent(mclk_parent, mclk_root);
+	if (ret)
+		return ret;
+
+	ret = rockchip_i2s_tdm_clk_set_rate(i2s_tdm, mclk_root,
+					    mclk_root_freq, 0);
+	if (ret)
+		return ret;
+
+	delta = abs(mclk_root_freq % mclk_parent_freq - mclk_parent_freq);
+	ppm = div64_u64((uint64_t)delta * 1000000, (uint64_t)mclk_root_freq);
+
+	if (ppm) {
+		div = DIV_ROUND_CLOSEST(mclk_root_initial_freq, mclk_parent_freq);
+		if (!div)
+			return -EINVAL;
+
+		mclk_root_freq = mclk_parent_freq * round_up(div, 2);
+
+		ret = clk_set_rate(mclk_root, mclk_root_freq);
+		if (ret)
+			return ret;
+
+		i2s_tdm->mclk_root0_freq = clk_get_rate(i2s_tdm->mclk_root0);
+		i2s_tdm->mclk_root1_freq = clk_get_rate(i2s_tdm->mclk_root1);
+	}
+
+	ret = clk_set_rate(mclk_parent, mclk_parent_freq);
+
+	return ret;
+}
+
+static int rockchip_i2s_tdm_set_mclk(struct rk_i2s_tdm_dev *i2s_tdm,
+				     struct snd_pcm_substream *substream,
+				     struct clk **mclk)
+{
+	unsigned int mclk_freq;
+	int ret;
+
+	if (i2s_tdm->clk_trcm) {
+		if (i2s_tdm->mclk_tx_freq != i2s_tdm->mclk_rx_freq) {
+			dev_err(i2s_tdm->dev,
+				"clk_trcm, tx: %d and rx: %d should be same\n",
+				i2s_tdm->mclk_tx_freq,
+				i2s_tdm->mclk_rx_freq);
+			ret = -EINVAL;
+			return ret;
+		}
+
+		ret = clk_set_rate(i2s_tdm->mclk_tx, i2s_tdm->mclk_tx_freq);
+		if (ret)
+			return ret;
+
+		ret = clk_set_rate(i2s_tdm->mclk_rx, i2s_tdm->mclk_rx_freq);
+		if (ret)
+			return ret;
+
+		/* mclk_rx is also ok. */
+		*mclk = i2s_tdm->mclk_tx;
+	} else {
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			*mclk = i2s_tdm->mclk_tx;
+			mclk_freq = i2s_tdm->mclk_tx_freq;
+		} else {
+			*mclk = i2s_tdm->mclk_rx;
+			mclk_freq = i2s_tdm->mclk_rx_freq;
+		}
+
+		ret = clk_set_rate(*mclk, mclk_freq);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int rockchip_i2s_io_multiplex(struct snd_pcm_substream *substream,
+				     struct snd_soc_dai *dai)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai);
+	int usable_chs = MULTIPLEX_CH_MAX;
+	unsigned int val = 0;
+
+	if (!i2s_tdm->io_multiplex)
+		return 0;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		struct snd_pcm_str *playback_str =
+			&substream->pcm->streams[SNDRV_PCM_STREAM_PLAYBACK];
+
+		if (playback_str->substream_opened) {
+			regmap_read(i2s_tdm->regmap, I2S_TXCR, &val);
+			val &= I2S_TXCR_CSR_MASK;
+			usable_chs = MULTIPLEX_CH_MAX - to_ch_num(val);
+		}
+
+		regmap_read(i2s_tdm->regmap, I2S_RXCR, &val);
+		val &= I2S_RXCR_CSR_MASK;
+
+		if (to_ch_num(val) > usable_chs) {
+			dev_err(i2s_tdm->dev,
+				"Capture chs(%d) > usable chs(%d)\n",
+				to_ch_num(val), usable_chs);
+			return -EINVAL;
+		}
+
+		switch (val) {
+		case I2S_CHN_4:
+			val = I2S_IO_6CH_OUT_4CH_IN;
+			break;
+		case I2S_CHN_6:
+			val = I2S_IO_4CH_OUT_6CH_IN;
+			break;
+		case I2S_CHN_8:
+			val = I2S_IO_2CH_OUT_8CH_IN;
+			break;
+		default:
+			val = I2S_IO_8CH_OUT_2CH_IN;
+			break;
+		}
+	} else {
+		struct snd_pcm_str *capture_str =
+			&substream->pcm->streams[SNDRV_PCM_STREAM_CAPTURE];
+
+		if (capture_str->substream_opened) {
+			regmap_read(i2s_tdm->regmap, I2S_RXCR, &val);
+			val &= I2S_RXCR_CSR_MASK;
+			usable_chs = MULTIPLEX_CH_MAX - to_ch_num(val);
+		}
+
+		regmap_read(i2s_tdm->regmap, I2S_TXCR, &val);
+		val &= I2S_TXCR_CSR_MASK;
+
+		if (to_ch_num(val) > usable_chs) {
+			dev_err(i2s_tdm->dev,
+				"Playback chs(%d) > usable chs(%d)\n",
+				to_ch_num(val), usable_chs);
+			return -EINVAL;
+		}
+
+		switch (val) {
+		case I2S_CHN_4:
+			val = I2S_IO_4CH_OUT_6CH_IN;
+			break;
+		case I2S_CHN_6:
+			val = I2S_IO_6CH_OUT_4CH_IN;
+			break;
+		case I2S_CHN_8:
+			val = I2S_IO_8CH_OUT_2CH_IN;
+			break;
+		default:
+			val = I2S_IO_2CH_OUT_8CH_IN;
+			break;
+		}
+	}
+
+	val <<= i2s_tdm->soc_data->grf_shift;
+	val |= (I2S_IO_DIRECTION_MASK << i2s_tdm->soc_data->grf_shift) << 16;
+	regmap_write(i2s_tdm->grf, i2s_tdm->soc_data->grf_reg_offset, val);
+
+	return 0;
+}
+
+static int rockchip_i2s_trcm_mode(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai,
+				  unsigned int div_bclk,
+				  unsigned int div_lrck,
+				  unsigned int fmt)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai);
+
+	if (!i2s_tdm->clk_trcm)
+		return 0;
+
+	spin_lock(&i2s_tdm->lock);
+	if (atomic_read(&i2s_tdm->refcount))
+		rockchip_i2s_tdm_xfer_pause(substream, i2s_tdm);
+
+	regmap_update_bits(i2s_tdm->regmap, I2S_CLKDIV,
+			   I2S_CLKDIV_TXM_MASK | I2S_CLKDIV_RXM_MASK,
+			   I2S_CLKDIV_TXM(div_bclk) | I2S_CLKDIV_RXM(div_bclk));
+	regmap_update_bits(i2s_tdm->regmap, I2S_CKR,
+			   I2S_CKR_TSD_MASK | I2S_CKR_RSD_MASK,
+			   I2S_CKR_TSD(div_lrck) | I2S_CKR_RSD(div_lrck));
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		regmap_update_bits(i2s_tdm->regmap, I2S_TXCR,
+				   I2S_TXCR_VDW_MASK | I2S_TXCR_CSR_MASK,
+				   fmt);
+	else
+		regmap_update_bits(i2s_tdm->regmap, I2S_RXCR,
+				   I2S_RXCR_VDW_MASK | I2S_RXCR_CSR_MASK,
+				   fmt);
+
+	if (atomic_read(&i2s_tdm->refcount))
+		rockchip_i2s_tdm_xfer_resume(substream, i2s_tdm);
+	spin_unlock(&i2s_tdm->lock);
+
+	return 0;
+}
+
+static int rockchip_i2s_tdm_hw_params(struct snd_pcm_substream *substream,
+				      struct snd_pcm_hw_params *params,
+				      struct snd_soc_dai *dai)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai);
+	struct clk *mclk;
+	int ret = 0;
+	unsigned int val = 0;
+	unsigned int mclk_rate, bclk_rate, div_bclk = 4, div_lrck = 64;
+
+	if (i2s_tdm->is_master_mode) {
+		if (i2s_tdm->mclk_calibrate)
+			rockchip_i2s_tdm_calibrate_mclk(i2s_tdm, substream,
+							params_rate(params));
+
+		ret = rockchip_i2s_tdm_set_mclk(i2s_tdm, substream, &mclk);
+		if (ret)
+			return ret;
+
+		mclk_rate = clk_get_rate(mclk);
+		bclk_rate = i2s_tdm->frame_width * params_rate(params);
+		if (!bclk_rate)
+			return -EINVAL;
+
+		div_bclk = DIV_ROUND_CLOSEST(mclk_rate, bclk_rate);
+		div_lrck = bclk_rate / params_rate(params);
+	}
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S8:
+		val |= I2S_TXCR_VDW(8);
+		break;
+	case SNDRV_PCM_FORMAT_S16_LE:
+		val |= I2S_TXCR_VDW(16);
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		val |= I2S_TXCR_VDW(20);
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		val |= I2S_TXCR_VDW(24);
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		val |= I2S_TXCR_VDW(32);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (params_channels(params)) {
+	case 8:
+		val |= I2S_CHN_8;
+		break;
+	case 6:
+		val |= I2S_CHN_6;
+		break;
+	case 4:
+		val |= I2S_CHN_4;
+		break;
+	case 2:
+		val |= I2S_CHN_2;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (i2s_tdm->clk_trcm) {
+		rockchip_i2s_trcm_mode(substream, dai, div_bclk, div_lrck, val);
+	} else if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		regmap_update_bits(i2s_tdm->regmap, I2S_CLKDIV,
+				   I2S_CLKDIV_TXM_MASK,
+				   I2S_CLKDIV_TXM(div_bclk));
+		regmap_update_bits(i2s_tdm->regmap, I2S_CKR,
+				   I2S_CKR_TSD_MASK,
+				   I2S_CKR_TSD(div_lrck));
+		regmap_update_bits(i2s_tdm->regmap, I2S_TXCR,
+				   I2S_TXCR_VDW_MASK | I2S_TXCR_CSR_MASK,
+				   val);
+	} else {
+		regmap_update_bits(i2s_tdm->regmap, I2S_CLKDIV,
+				   I2S_CLKDIV_RXM_MASK,
+				   I2S_CLKDIV_RXM(div_bclk));
+		regmap_update_bits(i2s_tdm->regmap, I2S_CKR,
+				   I2S_CKR_RSD_MASK,
+				   I2S_CKR_RSD(div_lrck));
+		regmap_update_bits(i2s_tdm->regmap, I2S_RXCR,
+				   I2S_RXCR_VDW_MASK | I2S_RXCR_CSR_MASK,
+				   val);
+	}
+
+	return rockchip_i2s_io_multiplex(substream, dai);
+}
+
+static int rockchip_i2s_tdm_trigger(struct snd_pcm_substream *substream,
+				    int cmd, struct snd_soc_dai *dai)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		if (i2s_tdm->clk_trcm)
+			rockchip_snd_txrxctrl(substream, dai, 1);
+		else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+			rockchip_snd_rxctrl(i2s_tdm, 1);
+		else
+			rockchip_snd_txctrl(i2s_tdm, 1);
+		break;
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		if (i2s_tdm->clk_trcm)
+			rockchip_snd_txrxctrl(substream, dai, 0);
+		else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+			rockchip_snd_rxctrl(i2s_tdm, 0);
+		else
+			rockchip_snd_txctrl(i2s_tdm, 0);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rockchip_i2s_tdm_set_sysclk(struct snd_soc_dai *cpu_dai, int stream,
+				       unsigned int freq, int dir)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = to_info(cpu_dai);
+
+	/* Put set mclk rate into rockchip_i2s_tdm_set_mclk() */
+	if (i2s_tdm->clk_trcm) {
+		i2s_tdm->mclk_tx_freq = freq;
+		i2s_tdm->mclk_rx_freq = freq;
+	} else {
+		if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+			i2s_tdm->mclk_tx_freq = freq;
+		else
+			i2s_tdm->mclk_rx_freq = freq;
+	}
+
+	dev_dbg(i2s_tdm->dev, "The target mclk_%s freq is: %d\n",
+		stream ? "rx" : "tx", freq);
+
+	return 0;
+}
+
+static int rockchip_i2s_tdm_clk_compensation_info(struct snd_kcontrol *kcontrol,
+						  struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = CLK_PPM_MIN;
+	uinfo->value.integer.max = CLK_PPM_MAX;
+	uinfo->value.integer.step = 1;
+
+	return 0;
+}
+
+static int rockchip_i2s_tdm_clk_compensation_get(struct snd_kcontrol *kcontrol,
+						 struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
+	struct rk_i2s_tdm_dev *i2s_tdm = snd_soc_dai_get_drvdata(dai);
+
+	ucontrol->value.integer.value[0] = i2s_tdm->clk_ppm;
+
+	return 0;
+}
+
+static int rockchip_i2s_tdm_clk_compensation_put(struct snd_kcontrol *kcontrol,
+						 struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
+	struct rk_i2s_tdm_dev *i2s_tdm = snd_soc_dai_get_drvdata(dai);
+	int ret = 0, ppm = 0;
+
+	if ((ucontrol->value.integer.value[0] < CLK_PPM_MIN) ||
+	    (ucontrol->value.integer.value[0] > CLK_PPM_MAX))
+		return -EINVAL;
+
+	ppm = ucontrol->value.integer.value[0];
+
+	ret = rockchip_i2s_tdm_clk_set_rate(i2s_tdm, i2s_tdm->mclk_root0,
+					    i2s_tdm->mclk_root0_freq, ppm);
+	if (ret)
+		return ret;
+
+	if (clk_is_match(i2s_tdm->mclk_root0, i2s_tdm->mclk_root1))
+		return 0;
+
+	ret = rockchip_i2s_tdm_clk_set_rate(i2s_tdm, i2s_tdm->mclk_root1,
+					    i2s_tdm->mclk_root1_freq, ppm);
+
+	return ret;
+}
+
+static struct snd_kcontrol_new rockchip_i2s_tdm_compensation_control = {
+	.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+	.name = "PCM Clk Compensation In PPM",
+	.info = rockchip_i2s_tdm_clk_compensation_info,
+	.get = rockchip_i2s_tdm_clk_compensation_get,
+	.put = rockchip_i2s_tdm_clk_compensation_put,
+};
+
+static int rockchip_i2s_tdm_dai_probe(struct snd_soc_dai *dai)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = snd_soc_dai_get_drvdata(dai);
+
+	dai->capture_dma_data = &i2s_tdm->capture_dma_data;
+	dai->playback_dma_data = &i2s_tdm->playback_dma_data;
+
+	if (i2s_tdm->mclk_calibrate)
+		snd_soc_add_dai_controls(dai, &rockchip_i2s_tdm_compensation_control, 1);
+
+	return 0;
+}
+
+static int rockchip_dai_tdm_slot(struct snd_soc_dai *dai,
+				 unsigned int tx_mask, unsigned int rx_mask,
+				 int slots, int slot_width)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask, val;
+
+	i2s_tdm->tdm_mode = true;
+	i2s_tdm->frame_width = slots * slot_width;
+	mask = TDM_SLOT_BIT_WIDTH_MSK | TDM_FRAME_WIDTH_MSK;
+	val = TDM_SLOT_BIT_WIDTH(slot_width) |
+	      TDM_FRAME_WIDTH(slots * slot_width);
+	regmap_update_bits(i2s_tdm->regmap, I2S_TDM_TXCR,
+			   mask, val);
+	regmap_update_bits(i2s_tdm->regmap, I2S_TDM_RXCR,
+			   mask, val);
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops rockchip_i2s_tdm_dai_ops = {
+	.hw_params = rockchip_i2s_tdm_hw_params,
+	.set_sysclk = rockchip_i2s_tdm_set_sysclk,
+	.set_fmt = rockchip_i2s_tdm_set_fmt,
+	.set_tdm_slot = rockchip_dai_tdm_slot,
+	.trigger = rockchip_i2s_tdm_trigger,
+};
+
+static const struct snd_soc_component_driver rockchip_i2s_tdm_component = {
+	.name = DRV_NAME,
+};
+
+static bool rockchip_i2s_tdm_wr_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case I2S_TXCR:
+	case I2S_RXCR:
+	case I2S_CKR:
+	case I2S_DMACR:
+	case I2S_INTCR:
+	case I2S_XFER:
+	case I2S_CLR:
+	case I2S_TXDR:
+	case I2S_TDM_TXCR:
+	case I2S_TDM_RXCR:
+	case I2S_CLKDIV:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool rockchip_i2s_tdm_rd_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case I2S_TXCR:
+	case I2S_RXCR:
+	case I2S_CKR:
+	case I2S_DMACR:
+	case I2S_INTCR:
+	case I2S_XFER:
+	case I2S_CLR:
+	case I2S_TXDR:
+	case I2S_RXDR:
+	case I2S_TXFIFOLR:
+	case I2S_INTSR:
+	case I2S_RXFIFOLR:
+	case I2S_TDM_TXCR:
+	case I2S_TDM_RXCR:
+	case I2S_CLKDIV:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool rockchip_i2s_tdm_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case I2S_TXFIFOLR:
+	case I2S_INTSR:
+	case I2S_CLR:
+	case I2S_TXDR:
+	case I2S_RXDR:
+	case I2S_RXFIFOLR:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool rockchip_i2s_tdm_precious_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case I2S_RXDR:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct reg_default rockchip_i2s_tdm_reg_defaults[] = {
+	{0x00, 0x7200000f},
+	{0x04, 0x01c8000f},
+	{0x08, 0x00001f1f},
+	{0x10, 0x001f0000},
+	{0x14, 0x01f00000},
+	{0x30, 0x00003eff},
+	{0x34, 0x00003eff},
+	{0x38, 0x00000707},
+};
+
+static const struct regmap_config rockchip_i2s_tdm_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = I2S_CLKDIV,
+	.reg_defaults = rockchip_i2s_tdm_reg_defaults,
+	.num_reg_defaults = ARRAY_SIZE(rockchip_i2s_tdm_reg_defaults),
+	.writeable_reg = rockchip_i2s_tdm_wr_reg,
+	.readable_reg = rockchip_i2s_tdm_rd_reg,
+	.volatile_reg = rockchip_i2s_tdm_volatile_reg,
+	.precious_reg = rockchip_i2s_tdm_precious_reg,
+	.cache_type = REGCACHE_FLAT,
+};
+
+static int common_soc_init(struct device *dev, u32 addr)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
+	const struct txrx_config *configs = i2s_tdm->soc_data->configs;
+	u32 reg = 0, val = 0, trcm = i2s_tdm->clk_trcm;
+	int i;
+
+	if (trcm == TRCM_TXRX)
+		return 0;
+
+	for (i = 0; i < i2s_tdm->soc_data->config_count; i++) {
+		if (addr != configs[i].addr)
+			continue;
+		reg = configs[i].reg;
+		if (trcm == TRCM_TX)
+			val = configs[i].txonly;
+		else
+			val = configs[i].rxonly;
+
+		if (reg)
+			regmap_write(i2s_tdm->grf, reg, val);
+	}
+
+	return 0;
+}
+
+static const struct txrx_config px30_txrx_config[] = {
+	{ 0xff060000, 0x184, PX30_I2S0_CLK_TXONLY, PX30_I2S0_CLK_RXONLY },
+};
+
+static const struct txrx_config rk1808_txrx_config[] = {
+	{ 0xff7e0000, 0x190, RK1808_I2S0_CLK_TXONLY, RK1808_I2S0_CLK_RXONLY },
+};
+
+static const struct txrx_config rk3308_txrx_config[] = {
+	{ 0xff300000, 0x308, RK3308_I2S0_CLK_TXONLY, RK3308_I2S0_CLK_RXONLY },
+	{ 0xff310000, 0x308, RK3308_I2S1_CLK_TXONLY, RK3308_I2S1_CLK_RXONLY },
+};
+
+static const struct txrx_config rk3568_txrx_config[] = {
+	{ 0xfe410000, 0x504, RK3568_I2S1_CLK_TXONLY, RK3568_I2S1_CLK_RXONLY },
+	{ 0xfe410000, 0x508, RK3568_I2S1_MCLK_TX_OE, RK3568_I2S1_MCLK_RX_OE },
+	{ 0xfe420000, 0x508, RK3568_I2S2_MCLK_OE, RK3568_I2S2_MCLK_OE },
+	{ 0xfe430000, 0x504, RK3568_I2S3_CLK_TXONLY, RK3568_I2S3_CLK_RXONLY },
+	{ 0xfe430000, 0x508, RK3568_I2S3_MCLK_TXONLY, RK3568_I2S3_MCLK_RXONLY },
+	{ 0xfe430000, 0x508, RK3568_I2S3_MCLK_OE, RK3568_I2S3_MCLK_OE },
+};
+
+static const struct txrx_config rv1126_txrx_config[] = {
+	{ 0xff800000, 0x10260, RV1126_I2S0_CLK_TXONLY, RV1126_I2S0_CLK_RXONLY },
+};
+
+static struct rk_i2s_soc_data px30_i2s_soc_data = {
+	.softrst_offset = 0x0300,
+	.configs = px30_txrx_config,
+	.config_count = ARRAY_SIZE(px30_txrx_config),
+	.init = common_soc_init,
+};
+
+static struct rk_i2s_soc_data rk1808_i2s_soc_data = {
+	.softrst_offset = 0x0300,
+	.configs = rk1808_txrx_config,
+	.config_count = ARRAY_SIZE(rk1808_txrx_config),
+	.init = common_soc_init,
+};
+
+static struct rk_i2s_soc_data rk3308_i2s_soc_data = {
+	.softrst_offset = 0x0400,
+	.grf_reg_offset = 0x0308,
+	.grf_shift = 5,
+	.configs = rk3308_txrx_config,
+	.config_count = ARRAY_SIZE(rk3308_txrx_config),
+	.init = common_soc_init,
+};
+
+static struct rk_i2s_soc_data rk3568_i2s_soc_data = {
+	.softrst_offset = 0x0400,
+	.configs = rk3568_txrx_config,
+	.config_count = ARRAY_SIZE(rk3568_txrx_config),
+	.init = common_soc_init,
+};
+
+static struct rk_i2s_soc_data rv1126_i2s_soc_data = {
+	.softrst_offset = 0x0300,
+	.configs = rv1126_txrx_config,
+	.config_count = ARRAY_SIZE(rv1126_txrx_config),
+	.init = common_soc_init,
+};
+
+static const struct of_device_id rockchip_i2s_tdm_match[] = {
+	{ .compatible = "rockchip,px30-i2s-tdm", .data = &px30_i2s_soc_data },
+	{ .compatible = "rockchip,rk1808-i2s-tdm", .data = &rk1808_i2s_soc_data },
+	{ .compatible = "rockchip,rk3308-i2s-tdm", .data = &rk3308_i2s_soc_data },
+	{ .compatible = "rockchip,rk3568-i2s-tdm", .data = &rk3568_i2s_soc_data },
+	{ .compatible = "rockchip,rv1126-i2s-tdm", .data = &rv1126_i2s_soc_data },
+	{},
+};
+
+static int of_i2s_resetid_get(struct device_node *node,
+			      const char *id)
+{
+	struct of_phandle_args args;
+	int index = 0;
+	int ret;
+
+	if (id)
+		index = of_property_match_string(node,
+						 "reset-names", id);
+	ret = of_parse_phandle_with_args(node, "resets", "#reset-cells",
+					 index, &args);
+	if (ret)
+		return ret;
+
+	return args.args[0];
+}
+
+static struct snd_soc_dai_driver i2s_tdm_dai = {
+	.probe = rockchip_i2s_tdm_dai_probe,
+	.playback = {
+		.stream_name  = "Playback",
+		.channels_min = 2,
+		.channels_max = 8,
+		.rates        = SNDRV_PCM_RATE_8000_192000,
+		.formats      = (SNDRV_PCM_FMTBIT_S8 |
+				SNDRV_PCM_FMTBIT_S16_LE |
+				SNDRV_PCM_FMTBIT_S20_3LE |
+				SNDRV_PCM_FMTBIT_S24_LE |
+				SNDRV_PCM_FMTBIT_S32_LE),
+	},
+	.capture = {
+		.stream_name  = "Capture",
+		.channels_min = 2,
+		.channels_max = 8,
+		.rates        = SNDRV_PCM_RATE_8000_192000,
+		.formats      = (SNDRV_PCM_FMTBIT_S8 |
+				SNDRV_PCM_FMTBIT_S16_LE |
+				SNDRV_PCM_FMTBIT_S20_3LE |
+				SNDRV_PCM_FMTBIT_S24_LE |
+				SNDRV_PCM_FMTBIT_S32_LE),
+	},
+	.ops = &rockchip_i2s_tdm_dai_ops,
+};
+
+static int rockchip_i2s_tdm_path_check(struct rk_i2s_tdm_dev *i2s_tdm,
+				       int num,
+				       bool is_rx_path)
+{
+	unsigned int *i2s_data;
+	int i, j;
+
+	if (is_rx_path)
+		i2s_data = i2s_tdm->i2s_sdis;
+	else
+		i2s_data = i2s_tdm->i2s_sdos;
+
+	for (i = 0; i < num; i++) {
+		if (i2s_data[i] > CH_GRP_MAX - 1) {
+			dev_err(i2s_tdm->dev,
+				"%s path i2s_data[%d]: %d is overflow, max is: %d\n",
+				is_rx_path ? "RX" : "TX",
+				i, i2s_data[i], CH_GRP_MAX);
+			return -EINVAL;
+		}
+
+		for (j = 0; j < num; j++) {
+			if (i == j)
+				continue;
+
+			if (i2s_data[i] == i2s_data[j]) {
+				dev_err(i2s_tdm->dev,
+					"%s path invalid routed i2s_data: [%d]%d == [%d]%d\n",
+					is_rx_path ? "RX" : "TX",
+					i, i2s_data[i],
+					j, i2s_data[j]);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void rockchip_i2s_tdm_tx_path_config(struct rk_i2s_tdm_dev *i2s_tdm,
+					    int num)
+{
+	int idx;
+
+	for (idx = 0; idx < num; idx++) {
+		regmap_update_bits(i2s_tdm->regmap, I2S_TXCR,
+				   I2S_TXCR_PATH_MASK(idx),
+				   I2S_TXCR_PATH(idx, i2s_tdm->i2s_sdos[idx]));
+	}
+}
+
+static void rockchip_i2s_tdm_rx_path_config(struct rk_i2s_tdm_dev *i2s_tdm,
+					    int num)
+{
+	int idx;
+
+	for (idx = 0; idx < num; idx++) {
+		regmap_update_bits(i2s_tdm->regmap, I2S_RXCR,
+				   I2S_RXCR_PATH_MASK(idx),
+				   I2S_RXCR_PATH(idx, i2s_tdm->i2s_sdis[idx]));
+	}
+}
+
+static void rockchip_i2s_tdm_path_config(struct rk_i2s_tdm_dev *i2s_tdm,
+					 int num, bool is_rx_path)
+{
+	if (is_rx_path)
+		rockchip_i2s_tdm_rx_path_config(i2s_tdm, num);
+	else
+		rockchip_i2s_tdm_tx_path_config(i2s_tdm, num);
+}
+
+static int rockchip_i2s_tdm_path_prepare(struct rk_i2s_tdm_dev *i2s_tdm,
+					 struct device_node *np,
+					 bool is_rx_path)
+{
+	char *i2s_tx_path_prop = "rockchip,i2s-tx-route";
+	char *i2s_rx_path_prop = "rockchip,i2s-rx-route";
+	char *i2s_path_prop;
+	unsigned int *i2s_data;
+	int num, ret = 0;
+
+	if (is_rx_path) {
+		i2s_path_prop = i2s_rx_path_prop;
+		i2s_data = i2s_tdm->i2s_sdis;
+	} else {
+		i2s_path_prop = i2s_tx_path_prop;
+		i2s_data = i2s_tdm->i2s_sdos;
+	}
+
+	num = of_count_phandle_with_args(np, i2s_path_prop, NULL);
+	if (num < 0) {
+		if (num != -ENOENT) {
+			dev_err(i2s_tdm->dev,
+				"Failed to read '%s' num: %d\n",
+				i2s_path_prop, num);
+			ret = num;
+		}
+		return ret;
+	} else if (num != CH_GRP_MAX) {
+		dev_err(i2s_tdm->dev,
+			"The num: %d should be: %d\n", num, CH_GRP_MAX);
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_array(np, i2s_path_prop,
+					 i2s_data, num);
+	if (ret < 0) {
+		dev_err(i2s_tdm->dev,
+			"Failed to read '%s': %d\n",
+			i2s_path_prop, ret);
+		return ret;
+	}
+
+	ret = rockchip_i2s_tdm_path_check(i2s_tdm, num, is_rx_path);
+	if (ret < 0) {
+		dev_err(i2s_tdm->dev,
+			"Failed to check i2s data bus: %d\n", ret);
+		return ret;
+	}
+
+	rockchip_i2s_tdm_path_config(i2s_tdm, num, is_rx_path);
+
+	return 0;
+}
+
+static int rockchip_i2s_tdm_tx_path_prepare(struct rk_i2s_tdm_dev *i2s_tdm,
+					    struct device_node *np)
+{
+	return rockchip_i2s_tdm_path_prepare(i2s_tdm, np, 0);
+}
+
+static int rockchip_i2s_tdm_rx_path_prepare(struct rk_i2s_tdm_dev *i2s_tdm,
+					    struct device_node *np)
+{
+	return rockchip_i2s_tdm_path_prepare(i2s_tdm, np, 1);
+}
+
+static int rockchip_i2s_tdm_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *cru_node;
+	const struct of_device_id *of_id;
+	struct rk_i2s_tdm_dev *i2s_tdm;
+	struct resource *res;
+	void __iomem *regs;
+	int ret;
+	int val;
+
+	i2s_tdm = devm_kzalloc(&pdev->dev, sizeof(*i2s_tdm), GFP_KERNEL);
+	if (!i2s_tdm)
+		return -ENOMEM;
+
+	i2s_tdm->dev = &pdev->dev;
+
+	of_id = of_match_device(rockchip_i2s_tdm_match, &pdev->dev);
+	if (!of_id || !of_id->data)
+		return -EINVAL;
+
+	spin_lock_init(&i2s_tdm->lock);
+	i2s_tdm->soc_data = (struct rk_i2s_soc_data *)of_id->data;
+
+	i2s_tdm->frame_width = 64;
+	if (!of_property_read_u32(node, "rockchip,frame-width", &val)) {
+		if (val >= 32 && (val % 2 == 0) && val <= 512) {
+			i2s_tdm->frame_width = val;
+		} else {
+			dev_err(i2s_tdm->dev, "unsupported frame width: '%d'\n",
+				val);
+			return -EINVAL;
+		}
+	}
+
+	i2s_tdm->clk_trcm = TRCM_TXRX;
+	if (of_property_read_bool(node, "rockchip,trcm-sync-tx-only"))
+		i2s_tdm->clk_trcm = TRCM_TX;
+	if (of_property_read_bool(node, "rockchip,trcm-sync-rx-only")) {
+		if (i2s_tdm->clk_trcm) {
+			dev_err(i2s_tdm->dev, "invalid trcm-sync configuration\n");
+			return -EINVAL;
+		}
+		i2s_tdm->clk_trcm = TRCM_RX;
+	}
+	if (i2s_tdm->clk_trcm != TRCM_TXRX)
+		i2s_tdm_dai.symmetric_rate = 1;
+
+	i2s_tdm->tdm_fsync_half_frame =
+		of_property_read_bool(node, "rockchip,tdm-fsync-half-frame");
+
+	i2s_tdm->grf = syscon_regmap_lookup_by_phandle(node, "rockchip,grf");
+	if (IS_ERR(i2s_tdm->grf))
+		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->grf),
+				     "Error in rockchip,grf\n");
+
+	if (i2s_tdm->clk_trcm != TRCM_TXRX) {
+		cru_node = of_parse_phandle(node, "rockchip,cru", 0);
+		i2s_tdm->cru_base = of_iomap(cru_node, 0);
+		of_node_put(cru_node);
+		if (!i2s_tdm->cru_base) {
+			dev_err(i2s_tdm->dev,
+				"Missing or unsupported rockchip,cru node\n");
+			return -ENOENT;
+		}
+
+		i2s_tdm->tx_reset_id = of_i2s_resetid_get(node, "tx-m");
+		i2s_tdm->rx_reset_id = of_i2s_resetid_get(node, "rx-m");
+	}
+
+	i2s_tdm->tx_reset = devm_reset_control_get(&pdev->dev, "tx-m");
+	if (IS_ERR(i2s_tdm->tx_reset)) {
+		ret = PTR_ERR(i2s_tdm->tx_reset);
+		if (ret != -ENOENT)
+			return dev_err_probe(i2s_tdm->dev, ret,
+					     "Error in tx-m reset control\n");
+	}
+
+	i2s_tdm->rx_reset = devm_reset_control_get(&pdev->dev, "rx-m");
+	if (IS_ERR(i2s_tdm->rx_reset)) {
+		ret = PTR_ERR(i2s_tdm->rx_reset);
+		if (ret != -ENOENT)
+			return dev_err_probe(i2s_tdm->dev, ret,
+					     "Error in rx-m reset control\n");
+	}
+
+	i2s_tdm->hclk = devm_clk_get(&pdev->dev, "hclk");
+	if (IS_ERR(i2s_tdm->hclk)) {
+		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->hclk),
+				     "Failed to get clock hclk\n");
+	}
+
+	ret = clk_prepare_enable(i2s_tdm->hclk);
+	if (ret) {
+		return dev_err_probe(i2s_tdm->dev, ret,
+				     "Failed to enable clock hclk\n");
+	}
+
+	i2s_tdm->mclk_tx = devm_clk_get(&pdev->dev, "mclk_tx");
+	if (IS_ERR(i2s_tdm->mclk_tx)) {
+		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->mclk_tx),
+				     "Failed to get clock mclk_tx\n");
+	}
+
+	i2s_tdm->mclk_rx = devm_clk_get(&pdev->dev, "mclk_rx");
+	if (IS_ERR(i2s_tdm->mclk_rx)) {
+		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->mclk_rx),
+				     "Failed to get clock mclk_rx\n");
+	}
+
+	i2s_tdm->io_multiplex =
+		of_property_read_bool(node, "rockchip,io-multiplex");
+
+	i2s_tdm->mclk_calibrate =
+		of_property_read_bool(node, "rockchip,mclk-calibrate");
+	if (i2s_tdm->mclk_calibrate) {
+		i2s_tdm->mclk_tx_src = devm_clk_get(&pdev->dev, "mclk_tx_src");
+		if (IS_ERR(i2s_tdm->mclk_tx_src)) {
+			return dev_err_probe(i2s_tdm->dev,
+					     PTR_ERR(i2s_tdm->mclk_tx_src),
+					     "Failed to get clock mclk_tx_src\n");
+		}
+		i2s_tdm->mclk_rx_src = devm_clk_get(&pdev->dev, "mclk_rx_src");
+		if (IS_ERR(i2s_tdm->mclk_rx_src)) {
+			return dev_err_probe(i2s_tdm->dev,
+					     PTR_ERR(i2s_tdm->mclk_rx_src),
+					     "Failed to get clock mclk_rx_src\n");
+		}
+		i2s_tdm->mclk_root0 = devm_clk_get(&pdev->dev, "mclk_root0");
+		if (IS_ERR(i2s_tdm->mclk_root0)) {
+			return dev_err_probe(i2s_tdm->dev,
+					     PTR_ERR(i2s_tdm->mclk_root0),
+					     "Failed to get clock mclk_root0\n");
+		}
+		i2s_tdm->mclk_root1 = devm_clk_get(&pdev->dev, "mclk_root1");
+		if (IS_ERR(i2s_tdm->mclk_root1)) {
+			return dev_err_probe(i2s_tdm->dev,
+					     PTR_ERR(i2s_tdm->mclk_root1),
+					     "Failed to get clock mclk_root1\n");
+		}
+
+		i2s_tdm->mclk_root0_initial_freq = clk_get_rate(i2s_tdm->mclk_root0);
+		i2s_tdm->mclk_root1_initial_freq = clk_get_rate(i2s_tdm->mclk_root1);
+		i2s_tdm->mclk_root0_freq = i2s_tdm->mclk_root0_initial_freq;
+		i2s_tdm->mclk_root1_freq = i2s_tdm->mclk_root1_initial_freq;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(regs)) {
+		return dev_err_probe(i2s_tdm->dev, PTR_ERR(regs),
+				     "Failed to get resource IORESOURCE_MEM\n");
+	}
+
+	i2s_tdm->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
+					    &rockchip_i2s_tdm_regmap_config);
+	if (IS_ERR(i2s_tdm->regmap)) {
+		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->regmap),
+				     "Failed to initialise regmap\n");
+	}
+
+	i2s_tdm->playback_dma_data.addr = res->start + I2S_TXDR;
+	i2s_tdm->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	i2s_tdm->playback_dma_data.maxburst = 8;
+
+	i2s_tdm->capture_dma_data.addr = res->start + I2S_RXDR;
+	i2s_tdm->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	i2s_tdm->capture_dma_data.maxburst = 8;
+
+	ret = rockchip_i2s_tdm_tx_path_prepare(i2s_tdm, node);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "I2S TX path prepare failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = rockchip_i2s_tdm_rx_path_prepare(i2s_tdm, node);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "I2S RX path prepare failed: %d\n", ret);
+		return ret;
+	}
+
+	atomic_set(&i2s_tdm->refcount, 0);
+	dev_set_drvdata(&pdev->dev, i2s_tdm);
+
+	pm_runtime_enable(&pdev->dev);
+	if (!pm_runtime_enabled(&pdev->dev)) {
+		ret = i2s_tdm_runtime_resume(&pdev->dev);
+		if (ret)
+			goto err_pm_disable;
+	}
+
+	regmap_update_bits(i2s_tdm->regmap, I2S_DMACR, I2S_DMACR_TDL_MASK,
+			   I2S_DMACR_TDL(16));
+	regmap_update_bits(i2s_tdm->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK,
+			   I2S_DMACR_RDL(16));
+	regmap_update_bits(i2s_tdm->regmap, I2S_CKR, I2S_CKR_TRCM_MASK,
+			   i2s_tdm->clk_trcm << I2S_CKR_TRCM_SHIFT);
+
+	if (i2s_tdm->soc_data && i2s_tdm->soc_data->init)
+		i2s_tdm->soc_data->init(&pdev->dev, res->start);
+
+	ret = devm_snd_soc_register_component(&pdev->dev,
+					      &rockchip_i2s_tdm_component,
+					      &i2s_tdm_dai, 1);
+
+	if (ret) {
+		dev_err(&pdev->dev, "Could not register DAI\n");
+		goto err_suspend;
+	}
+
+	ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not register PCM\n");
+		return ret;
+	}
+
+	return 0;
+
+err_suspend:
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		i2s_tdm_runtime_suspend(&pdev->dev);
+err_pm_disable:
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
+}
+
+static int rockchip_i2s_tdm_remove(struct platform_device *pdev)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(&pdev->dev);
+
+	pm_runtime_disable(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		i2s_tdm_runtime_suspend(&pdev->dev);
+
+	if (!IS_ERR(i2s_tdm->mclk_tx))
+		clk_prepare_enable(i2s_tdm->mclk_tx);
+	if (!IS_ERR(i2s_tdm->mclk_rx))
+		clk_prepare_enable(i2s_tdm->mclk_rx);
+	if (!IS_ERR(i2s_tdm->hclk))
+		clk_disable_unprepare(i2s_tdm->hclk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int rockchip_i2s_tdm_suspend(struct device *dev)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
+
+	regcache_mark_dirty(i2s_tdm->regmap);
+
+	return 0;
+}
+
+static int rockchip_i2s_tdm_resume(struct device *dev)
+{
+	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		return ret;
+	ret = regcache_sync(i2s_tdm->regmap);
+	pm_runtime_put(dev);
+
+	return ret;
+}
+#endif
+
+static const struct dev_pm_ops rockchip_i2s_tdm_pm_ops = {
+	SET_RUNTIME_PM_OPS(i2s_tdm_runtime_suspend, i2s_tdm_runtime_resume,
+			   NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(rockchip_i2s_tdm_suspend,
+				rockchip_i2s_tdm_resume)
+};
+
+static struct platform_driver rockchip_i2s_tdm_driver = {
+	.probe = rockchip_i2s_tdm_probe,
+	.remove = rockchip_i2s_tdm_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = of_match_ptr(rockchip_i2s_tdm_match),
+		.pm = &rockchip_i2s_tdm_pm_ops,
+	},
+};
+module_platform_driver(rockchip_i2s_tdm_driver);
+
+MODULE_DESCRIPTION("ROCKCHIP I2S/TDM ASoC Interface");
+MODULE_AUTHOR("Sugar Zhang <sugar.zhang@rock-chips.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DEVICE_TABLE(of, rockchip_i2s_tdm_match);
diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.h b/sound/soc/rockchip/rockchip_i2s_tdm.h
new file mode 100644
index 000000000000..3a9d9e8db262
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_i2s_tdm.h
@@ -0,0 +1,398 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ALSA SoC Audio Layer - Rockchip I2S/TDM Controller driver
+ *
+ * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
+ * Author: Sugar Zhang <sugar.zhang@rock-chips.com>
+ *
+ */
+
+#ifndef _ROCKCHIP_I2S_TDM_H
+#define _ROCKCHIP_I2S_TDM_H
+
+/*
+ * TXCR
+ * transmit operation control register
+ */
+#define I2S_TXCR_PATH_SHIFT(x)	(23 + (x) * 2)
+#define I2S_TXCR_PATH_MASK(x)	(0x3 << I2S_TXCR_PATH_SHIFT(x))
+#define I2S_TXCR_PATH(x, v)	((v) << I2S_TXCR_PATH_SHIFT(x))
+#define I2S_TXCR_RCNT_SHIFT	17
+#define I2S_TXCR_RCNT_MASK	(0x3f << I2S_TXCR_RCNT_SHIFT)
+#define I2S_TXCR_CSR_SHIFT	15
+#define I2S_TXCR_CSR(x)		(x << I2S_TXCR_CSR_SHIFT)
+#define I2S_TXCR_CSR_MASK	(3 << I2S_TXCR_CSR_SHIFT)
+#define I2S_TXCR_HWT		BIT(14)
+#define I2S_TXCR_SJM_SHIFT	12
+#define I2S_TXCR_SJM_R		(0 << I2S_TXCR_SJM_SHIFT)
+#define I2S_TXCR_SJM_L		(1 << I2S_TXCR_SJM_SHIFT)
+#define I2S_TXCR_FBM_SHIFT	11
+#define I2S_TXCR_FBM_MSB	(0 << I2S_TXCR_FBM_SHIFT)
+#define I2S_TXCR_FBM_LSB	(1 << I2S_TXCR_FBM_SHIFT)
+#define I2S_TXCR_IBM_SHIFT	9
+#define I2S_TXCR_IBM_NORMAL	(0 << I2S_TXCR_IBM_SHIFT)
+#define I2S_TXCR_IBM_LSJM	(1 << I2S_TXCR_IBM_SHIFT)
+#define I2S_TXCR_IBM_RSJM	(2 << I2S_TXCR_IBM_SHIFT)
+#define I2S_TXCR_IBM_MASK	(3 << I2S_TXCR_IBM_SHIFT)
+#define I2S_TXCR_PBM_SHIFT	7
+#define I2S_TXCR_PBM_MODE(x)	(x << I2S_TXCR_PBM_SHIFT)
+#define I2S_TXCR_PBM_MASK	(3 << I2S_TXCR_PBM_SHIFT)
+#define I2S_TXCR_TFS_SHIFT	5
+#define I2S_TXCR_TFS_I2S	(0 << I2S_TXCR_TFS_SHIFT)
+#define I2S_TXCR_TFS_PCM	(1 << I2S_TXCR_TFS_SHIFT)
+#define I2S_TXCR_TFS_TDM_PCM	(2 << I2S_TXCR_TFS_SHIFT)
+#define I2S_TXCR_TFS_TDM_I2S	(3 << I2S_TXCR_TFS_SHIFT)
+#define I2S_TXCR_TFS_MASK	(3 << I2S_TXCR_TFS_SHIFT)
+#define I2S_TXCR_VDW_SHIFT	0
+#define I2S_TXCR_VDW(x)		((x - 1) << I2S_TXCR_VDW_SHIFT)
+#define I2S_TXCR_VDW_MASK	(0x1f << I2S_TXCR_VDW_SHIFT)
+
+/*
+ * RXCR
+ * receive operation control register
+ */
+#define I2S_RXCR_PATH_SHIFT(x)	(17 + (x) * 2)
+#define I2S_RXCR_PATH_MASK(x)	(0x3 << I2S_RXCR_PATH_SHIFT(x))
+#define I2S_RXCR_PATH(x, v)	((v) << I2S_RXCR_PATH_SHIFT(x))
+#define I2S_RXCR_CSR_SHIFT	15
+#define I2S_RXCR_CSR(x)		(x << I2S_RXCR_CSR_SHIFT)
+#define I2S_RXCR_CSR_MASK	(3 << I2S_RXCR_CSR_SHIFT)
+#define I2S_RXCR_HWT		BIT(14)
+#define I2S_RXCR_SJM_SHIFT	12
+#define I2S_RXCR_SJM_R		(0 << I2S_RXCR_SJM_SHIFT)
+#define I2S_RXCR_SJM_L		(1 << I2S_RXCR_SJM_SHIFT)
+#define I2S_RXCR_FBM_SHIFT	11
+#define I2S_RXCR_FBM_MSB	(0 << I2S_RXCR_FBM_SHIFT)
+#define I2S_RXCR_FBM_LSB	(1 << I2S_RXCR_FBM_SHIFT)
+#define I2S_RXCR_IBM_SHIFT	9
+#define I2S_RXCR_IBM_NORMAL	(0 << I2S_RXCR_IBM_SHIFT)
+#define I2S_RXCR_IBM_LSJM	(1 << I2S_RXCR_IBM_SHIFT)
+#define I2S_RXCR_IBM_RSJM	(2 << I2S_RXCR_IBM_SHIFT)
+#define I2S_RXCR_IBM_MASK	(3 << I2S_RXCR_IBM_SHIFT)
+#define I2S_RXCR_PBM_SHIFT	7
+#define I2S_RXCR_PBM_MODE(x)	(x << I2S_RXCR_PBM_SHIFT)
+#define I2S_RXCR_PBM_MASK	(3 << I2S_RXCR_PBM_SHIFT)
+#define I2S_RXCR_TFS_SHIFT	5
+#define I2S_RXCR_TFS_I2S	(0 << I2S_RXCR_TFS_SHIFT)
+#define I2S_RXCR_TFS_PCM	(1 << I2S_RXCR_TFS_SHIFT)
+#define I2S_RXCR_TFS_TDM_PCM	(2 << I2S_RXCR_TFS_SHIFT)
+#define I2S_RXCR_TFS_TDM_I2S	(3 << I2S_RXCR_TFS_SHIFT)
+#define I2S_RXCR_TFS_MASK	(3 << I2S_RXCR_TFS_SHIFT)
+#define I2S_RXCR_VDW_SHIFT	0
+#define I2S_RXCR_VDW(x)		((x - 1) << I2S_RXCR_VDW_SHIFT)
+#define I2S_RXCR_VDW_MASK	(0x1f << I2S_RXCR_VDW_SHIFT)
+
+/*
+ * CKR
+ * clock generation register
+ */
+#define I2S_CKR_TRCM_SHIFT	28
+#define I2S_CKR_TRCM(x)	(x << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_TXRX	(0 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_TXONLY	(1 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_RXONLY	(2 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_MASK	(3 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_MSS_SHIFT	27
+#define I2S_CKR_MSS_MASTER	(0 << I2S_CKR_MSS_SHIFT)
+#define I2S_CKR_MSS_SLAVE	(1 << I2S_CKR_MSS_SHIFT)
+#define I2S_CKR_MSS_MASK	(1 << I2S_CKR_MSS_SHIFT)
+#define I2S_CKR_CKP_SHIFT	26
+#define I2S_CKR_CKP_NORMAL	(0 << I2S_CKR_CKP_SHIFT)
+#define I2S_CKR_CKP_INVERTED	(1 << I2S_CKR_CKP_SHIFT)
+#define I2S_CKR_CKP_MASK	(1 << I2S_CKR_CKP_SHIFT)
+#define I2S_CKR_RLP_SHIFT	25
+#define I2S_CKR_RLP_NORMAL	(0 << I2S_CKR_RLP_SHIFT)
+#define I2S_CKR_RLP_INVERTED	(1 << I2S_CKR_RLP_SHIFT)
+#define I2S_CKR_RLP_MASK	(1 << I2S_CKR_RLP_SHIFT)
+#define I2S_CKR_TLP_SHIFT	24
+#define I2S_CKR_TLP_NORMAL	(0 << I2S_CKR_TLP_SHIFT)
+#define I2S_CKR_TLP_INVERTED	(1 << I2S_CKR_TLP_SHIFT)
+#define I2S_CKR_TLP_MASK	(1 << I2S_CKR_TLP_SHIFT)
+#define I2S_CKR_MDIV_SHIFT	16
+#define I2S_CKR_MDIV(x)		((x - 1) << I2S_CKR_MDIV_SHIFT)
+#define I2S_CKR_MDIV_MASK	(0xff << I2S_CKR_MDIV_SHIFT)
+#define I2S_CKR_RSD_SHIFT	8
+#define I2S_CKR_RSD(x)		((x - 1) << I2S_CKR_RSD_SHIFT)
+#define I2S_CKR_RSD_MASK	(0xff << I2S_CKR_RSD_SHIFT)
+#define I2S_CKR_TSD_SHIFT	0
+#define I2S_CKR_TSD(x)		((x - 1) << I2S_CKR_TSD_SHIFT)
+#define I2S_CKR_TSD_MASK	(0xff << I2S_CKR_TSD_SHIFT)
+
+/*
+ * FIFOLR
+ * FIFO level register
+ */
+#define I2S_FIFOLR_RFL_SHIFT	24
+#define I2S_FIFOLR_RFL_MASK	(0x3f << I2S_FIFOLR_RFL_SHIFT)
+#define I2S_FIFOLR_TFL3_SHIFT	18
+#define I2S_FIFOLR_TFL3_MASK	(0x3f << I2S_FIFOLR_TFL3_SHIFT)
+#define I2S_FIFOLR_TFL2_SHIFT	12
+#define I2S_FIFOLR_TFL2_MASK	(0x3f << I2S_FIFOLR_TFL2_SHIFT)
+#define I2S_FIFOLR_TFL1_SHIFT	6
+#define I2S_FIFOLR_TFL1_MASK	(0x3f << I2S_FIFOLR_TFL1_SHIFT)
+#define I2S_FIFOLR_TFL0_SHIFT	0
+#define I2S_FIFOLR_TFL0_MASK	(0x3f << I2S_FIFOLR_TFL0_SHIFT)
+
+/*
+ * DMACR
+ * DMA control register
+ */
+#define I2S_DMACR_RDE_SHIFT	24
+#define I2S_DMACR_RDE_DISABLE	(0 << I2S_DMACR_RDE_SHIFT)
+#define I2S_DMACR_RDE_ENABLE	(1 << I2S_DMACR_RDE_SHIFT)
+#define I2S_DMACR_RDL_SHIFT	16
+#define I2S_DMACR_RDL(x)	((x - 1) << I2S_DMACR_RDL_SHIFT)
+#define I2S_DMACR_RDL_MASK	(0x1f << I2S_DMACR_RDL_SHIFT)
+#define I2S_DMACR_TDE_SHIFT	8
+#define I2S_DMACR_TDE_DISABLE	(0 << I2S_DMACR_TDE_SHIFT)
+#define I2S_DMACR_TDE_ENABLE	(1 << I2S_DMACR_TDE_SHIFT)
+#define I2S_DMACR_TDL_SHIFT	0
+#define I2S_DMACR_TDL(x)	((x) << I2S_DMACR_TDL_SHIFT)
+#define I2S_DMACR_TDL_MASK	(0x1f << I2S_DMACR_TDL_SHIFT)
+
+/*
+ * INTCR
+ * interrupt control register
+ */
+#define I2S_INTCR_RFT_SHIFT	20
+#define I2S_INTCR_RFT(x)	((x - 1) << I2S_INTCR_RFT_SHIFT)
+#define I2S_INTCR_RXOIC		BIT(18)
+#define I2S_INTCR_RXOIE_SHIFT	17
+#define I2S_INTCR_RXOIE_DISABLE	(0 << I2S_INTCR_RXOIE_SHIFT)
+#define I2S_INTCR_RXOIE_ENABLE	(1 << I2S_INTCR_RXOIE_SHIFT)
+#define I2S_INTCR_RXFIE_SHIFT	16
+#define I2S_INTCR_RXFIE_DISABLE	(0 << I2S_INTCR_RXFIE_SHIFT)
+#define I2S_INTCR_RXFIE_ENABLE	(1 << I2S_INTCR_RXFIE_SHIFT)
+#define I2S_INTCR_TFT_SHIFT	4
+#define I2S_INTCR_TFT(x)	((x - 1) << I2S_INTCR_TFT_SHIFT)
+#define I2S_INTCR_TFT_MASK	(0x1f << I2S_INTCR_TFT_SHIFT)
+#define I2S_INTCR_TXUIC		BIT(2)
+#define I2S_INTCR_TXUIE_SHIFT	1
+#define I2S_INTCR_TXUIE_DISABLE	(0 << I2S_INTCR_TXUIE_SHIFT)
+#define I2S_INTCR_TXUIE_ENABLE	(1 << I2S_INTCR_TXUIE_SHIFT)
+
+/*
+ * INTSR
+ * interrupt status register
+ */
+#define I2S_INTSR_TXEIE_SHIFT	0
+#define I2S_INTSR_TXEIE_DISABLE	(0 << I2S_INTSR_TXEIE_SHIFT)
+#define I2S_INTSR_TXEIE_ENABLE	(1 << I2S_INTSR_TXEIE_SHIFT)
+#define I2S_INTSR_RXOI_SHIFT	17
+#define I2S_INTSR_RXOI_INA	(0 << I2S_INTSR_RXOI_SHIFT)
+#define I2S_INTSR_RXOI_ACT	(1 << I2S_INTSR_RXOI_SHIFT)
+#define I2S_INTSR_RXFI_SHIFT	16
+#define I2S_INTSR_RXFI_INA	(0 << I2S_INTSR_RXFI_SHIFT)
+#define I2S_INTSR_RXFI_ACT	(1 << I2S_INTSR_RXFI_SHIFT)
+#define I2S_INTSR_TXUI_SHIFT	1
+#define I2S_INTSR_TXUI_INA	(0 << I2S_INTSR_TXUI_SHIFT)
+#define I2S_INTSR_TXUI_ACT	(1 << I2S_INTSR_TXUI_SHIFT)
+#define I2S_INTSR_TXEI_SHIFT	0
+#define I2S_INTSR_TXEI_INA	(0 << I2S_INTSR_TXEI_SHIFT)
+#define I2S_INTSR_TXEI_ACT	(1 << I2S_INTSR_TXEI_SHIFT)
+
+/*
+ * XFER
+ * Transfer start register
+ */
+#define I2S_XFER_RXS_SHIFT	1
+#define I2S_XFER_RXS_STOP	(0 << I2S_XFER_RXS_SHIFT)
+#define I2S_XFER_RXS_START	(1 << I2S_XFER_RXS_SHIFT)
+#define I2S_XFER_TXS_SHIFT	0
+#define I2S_XFER_TXS_STOP	(0 << I2S_XFER_TXS_SHIFT)
+#define I2S_XFER_TXS_START	(1 << I2S_XFER_TXS_SHIFT)
+
+/*
+ * CLR
+ * clear SCLK domain logic register
+ */
+#define I2S_CLR_RXC	BIT(1)
+#define I2S_CLR_TXC	BIT(0)
+
+/*
+ * TXDR
+ * Transimt FIFO data register, write only.
+ */
+#define I2S_TXDR_MASK	(0xff)
+
+/*
+ * RXDR
+ * Receive FIFO data register, write only.
+ */
+#define I2S_RXDR_MASK	(0xff)
+
+/*
+ * TDM_CTRL
+ * TDM ctrl register
+ */
+#define TDM_FSYNC_WIDTH_SEL1_MSK	GENMASK(20, 18)
+#define TDM_FSYNC_WIDTH_SEL1(x)		((x - 1) << 18)
+#define TDM_FSYNC_WIDTH_SEL0_MSK	BIT(17)
+#define TDM_FSYNC_WIDTH_HALF_FRAME	0
+#define TDM_FSYNC_WIDTH_ONE_FRAME	BIT(17)
+#define TDM_SHIFT_CTRL_MSK		GENMASK(16, 14)
+#define TDM_SHIFT_CTRL(x)		((x) << 14)
+#define TDM_SLOT_BIT_WIDTH_MSK		GENMASK(13, 9)
+#define TDM_SLOT_BIT_WIDTH(x)		((x - 1) << 9)
+#define TDM_FRAME_WIDTH_MSK		GENMASK(8, 0)
+#define TDM_FRAME_WIDTH(x)		((x - 1) << 0)
+
+/*
+ * CLKDIV
+ * Mclk div register
+ */
+#define I2S_CLKDIV_TXM_SHIFT	0
+#define I2S_CLKDIV_TXM(x)		((x - 1) << I2S_CLKDIV_TXM_SHIFT)
+#define I2S_CLKDIV_TXM_MASK	(0xff << I2S_CLKDIV_TXM_SHIFT)
+#define I2S_CLKDIV_RXM_SHIFT	8
+#define I2S_CLKDIV_RXM(x)		((x - 1) << I2S_CLKDIV_RXM_SHIFT)
+#define I2S_CLKDIV_RXM_MASK	(0xff << I2S_CLKDIV_RXM_SHIFT)
+
+/* Clock divider id */
+enum {
+	ROCKCHIP_DIV_MCLK = 0,
+	ROCKCHIP_DIV_BCLK,
+};
+
+/* channel select */
+#define I2S_CSR_SHIFT	15
+#define I2S_CHN_2	(0 << I2S_CSR_SHIFT)
+#define I2S_CHN_4	(1 << I2S_CSR_SHIFT)
+#define I2S_CHN_6	(2 << I2S_CSR_SHIFT)
+#define I2S_CHN_8	(3 << I2S_CSR_SHIFT)
+
+/* io direction cfg register */
+#define I2S_IO_DIRECTION_MASK	(7)
+#define I2S_IO_8CH_OUT_2CH_IN	(7)
+#define I2S_IO_6CH_OUT_4CH_IN	(3)
+#define I2S_IO_4CH_OUT_6CH_IN	(1)
+#define I2S_IO_2CH_OUT_8CH_IN	(0)
+
+/* I2S REGS */
+#define I2S_TXCR	(0x0000)
+#define I2S_RXCR	(0x0004)
+#define I2S_CKR		(0x0008)
+#define I2S_TXFIFOLR	(0x000c)
+#define I2S_DMACR	(0x0010)
+#define I2S_INTCR	(0x0014)
+#define I2S_INTSR	(0x0018)
+#define I2S_XFER	(0x001c)
+#define I2S_CLR		(0x0020)
+#define I2S_TXDR	(0x0024)
+#define I2S_RXDR	(0x0028)
+#define I2S_RXFIFOLR	(0x002c)
+#define I2S_TDM_TXCR	(0x0030)
+#define I2S_TDM_RXCR	(0x0034)
+#define I2S_CLKDIV	(0x0038)
+
+#define HIWORD_UPDATE(v, h, l)	(((v) << (l)) | (GENMASK((h), (l)) << 16))
+
+/* PX30 GRF CONFIGS */
+#define PX30_I2S0_CLK_IN_SRC_FROM_TX		HIWORD_UPDATE(1, 13, 12)
+#define PX30_I2S0_CLK_IN_SRC_FROM_RX		HIWORD_UPDATE(2, 13, 12)
+#define PX30_I2S0_MCLK_OUT_SRC_FROM_TX		HIWORD_UPDATE(1, 5, 5)
+#define PX30_I2S0_MCLK_OUT_SRC_FROM_RX		HIWORD_UPDATE(0, 5, 5)
+
+#define PX30_I2S0_CLK_TXONLY \
+	(PX30_I2S0_MCLK_OUT_SRC_FROM_TX | PX30_I2S0_CLK_IN_SRC_FROM_TX)
+
+#define PX30_I2S0_CLK_RXONLY \
+	(PX30_I2S0_MCLK_OUT_SRC_FROM_RX | PX30_I2S0_CLK_IN_SRC_FROM_RX)
+
+/* RK1808 GRF CONFIGS */
+#define RK1808_I2S0_MCLK_OUT_SRC_FROM_RX	HIWORD_UPDATE(1, 2, 2)
+#define RK1808_I2S0_MCLK_OUT_SRC_FROM_TX	HIWORD_UPDATE(0, 2, 2)
+#define RK1808_I2S0_CLK_IN_SRC_FROM_TX		HIWORD_UPDATE(1, 1, 0)
+#define RK1808_I2S0_CLK_IN_SRC_FROM_RX		HIWORD_UPDATE(2, 1, 0)
+
+#define RK1808_I2S0_CLK_TXONLY \
+	(RK1808_I2S0_MCLK_OUT_SRC_FROM_TX | RK1808_I2S0_CLK_IN_SRC_FROM_TX)
+
+#define RK1808_I2S0_CLK_RXONLY \
+	(RK1808_I2S0_MCLK_OUT_SRC_FROM_RX | RK1808_I2S0_CLK_IN_SRC_FROM_RX)
+
+/* RK3308 GRF CONFIGS */
+#define RK3308_I2S0_8CH_MCLK_OUT_SRC_FROM_RX	HIWORD_UPDATE(1, 10, 10)
+#define RK3308_I2S0_8CH_MCLK_OUT_SRC_FROM_TX	HIWORD_UPDATE(0, 10, 10)
+#define RK3308_I2S0_8CH_CLK_IN_RX_SRC_FROM_TX	HIWORD_UPDATE(1, 9, 9)
+#define RK3308_I2S0_8CH_CLK_IN_RX_SRC_FROM_RX	HIWORD_UPDATE(0, 9, 9)
+#define RK3308_I2S0_8CH_CLK_IN_TX_SRC_FROM_RX	HIWORD_UPDATE(1, 8, 8)
+#define RK3308_I2S0_8CH_CLK_IN_TX_SRC_FROM_TX	HIWORD_UPDATE(0, 8, 8)
+#define RK3308_I2S1_8CH_MCLK_OUT_SRC_FROM_RX	HIWORD_UPDATE(1, 2, 2)
+#define RK3308_I2S1_8CH_MCLK_OUT_SRC_FROM_TX	HIWORD_UPDATE(0, 2, 2)
+#define RK3308_I2S1_8CH_CLK_IN_RX_SRC_FROM_TX	HIWORD_UPDATE(1, 1, 1)
+#define RK3308_I2S1_8CH_CLK_IN_RX_SRC_FROM_RX	HIWORD_UPDATE(0, 1, 1)
+#define RK3308_I2S1_8CH_CLK_IN_TX_SRC_FROM_RX	HIWORD_UPDATE(1, 0, 0)
+#define RK3308_I2S1_8CH_CLK_IN_TX_SRC_FROM_TX	HIWORD_UPDATE(0, 0, 0)
+
+#define RK3308_I2S0_CLK_TXONLY \
+	(RK3308_I2S0_8CH_MCLK_OUT_SRC_FROM_TX | \
+	RK3308_I2S0_8CH_CLK_IN_RX_SRC_FROM_TX | \
+	RK3308_I2S0_8CH_CLK_IN_TX_SRC_FROM_TX)
+
+#define RK3308_I2S0_CLK_RXONLY \
+	(RK3308_I2S0_8CH_MCLK_OUT_SRC_FROM_RX | \
+	RK3308_I2S0_8CH_CLK_IN_RX_SRC_FROM_RX | \
+	RK3308_I2S0_8CH_CLK_IN_TX_SRC_FROM_RX)
+
+#define RK3308_I2S1_CLK_TXONLY \
+	(RK3308_I2S1_8CH_MCLK_OUT_SRC_FROM_TX | \
+	RK3308_I2S1_8CH_CLK_IN_RX_SRC_FROM_TX | \
+	RK3308_I2S1_8CH_CLK_IN_TX_SRC_FROM_TX)
+
+#define RK3308_I2S1_CLK_RXONLY \
+	(RK3308_I2S1_8CH_MCLK_OUT_SRC_FROM_RX | \
+	RK3308_I2S1_8CH_CLK_IN_RX_SRC_FROM_RX | \
+	RK3308_I2S1_8CH_CLK_IN_TX_SRC_FROM_RX)
+
+/* RK3568 GRF CONFIGS */
+#define RK3568_I2S1_MCLK_OUT_SRC_FROM_TX	HIWORD_UPDATE(1, 5, 5)
+#define RK3568_I2S1_MCLK_OUT_SRC_FROM_RX	HIWORD_UPDATE(0, 5, 5)
+
+#define RK3568_I2S1_CLK_TXONLY \
+	RK3568_I2S1_MCLK_OUT_SRC_FROM_TX
+
+#define RK3568_I2S1_CLK_RXONLY \
+	RK3568_I2S1_MCLK_OUT_SRC_FROM_RX
+
+#define RK3568_I2S3_MCLK_OUT_SRC_FROM_TX	HIWORD_UPDATE(1, 15, 15)
+#define RK3568_I2S3_MCLK_OUT_SRC_FROM_RX	HIWORD_UPDATE(0, 15, 15)
+#define RK3568_I2S3_SCLK_SRC_FROM_TX		HIWORD_UPDATE(1, 7, 7)
+#define RK3568_I2S3_SCLK_SRC_FROM_RX		HIWORD_UPDATE(0, 7, 7)
+#define RK3568_I2S3_LRCK_SRC_FROM_TX		HIWORD_UPDATE(1, 6, 6)
+#define RK3568_I2S3_LRCK_SRC_FROM_RX		HIWORD_UPDATE(0, 6, 6)
+
+#define RK3568_I2S3_MCLK_TXONLY \
+	RK3568_I2S3_MCLK_OUT_SRC_FROM_TX
+
+#define RK3568_I2S3_CLK_TXONLY \
+	(RK3568_I2S3_SCLK_SRC_FROM_TX | \
+	RK3568_I2S3_LRCK_SRC_FROM_TX)
+
+#define RK3568_I2S3_MCLK_RXONLY \
+	RK3568_I2S3_MCLK_OUT_SRC_FROM_RX
+
+#define RK3568_I2S3_CLK_RXONLY \
+	(RK3568_I2S3_SCLK_SRC_FROM_RX | \
+	RK3568_I2S3_LRCK_SRC_FROM_RX)
+
+#define RK3568_I2S3_MCLK_IE			HIWORD_UPDATE(0, 3, 3)
+#define RK3568_I2S3_MCLK_OE			HIWORD_UPDATE(1, 3, 3)
+#define RK3568_I2S2_MCLK_IE			HIWORD_UPDATE(0, 2, 2)
+#define RK3568_I2S2_MCLK_OE			HIWORD_UPDATE(1, 2, 2)
+#define RK3568_I2S1_MCLK_TX_IE			HIWORD_UPDATE(0, 1, 1)
+#define RK3568_I2S1_MCLK_TX_OE			HIWORD_UPDATE(1, 1, 1)
+#define RK3568_I2S1_MCLK_RX_IE			HIWORD_UPDATE(0, 0, 0)
+#define RK3568_I2S1_MCLK_RX_OE			HIWORD_UPDATE(1, 0, 0)
+
+/* RV1126 GRF CONFIGS */
+#define RV1126_I2S0_MCLK_OUT_SRC_FROM_TX	HIWORD_UPDATE(0, 9, 9)
+#define RV1126_I2S0_MCLK_OUT_SRC_FROM_RX	HIWORD_UPDATE(1, 9, 9)
+
+#define RV1126_I2S0_CLK_TXONLY \
+	RV1126_I2S0_MCLK_OUT_SRC_FROM_TX
+
+#define RV1126_I2S0_CLK_RXONLY \
+	RV1126_I2S0_MCLK_OUT_SRC_FROM_RX
+
+#endif /* _ROCKCHIP_I2S_TDM_H */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-08-20 18:27 [PATCH v2 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
  2021-08-20 18:27 ` [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller Nicolas Frattaroli
@ 2021-08-20 18:27 ` Nicolas Frattaroli
  2021-08-24 16:42   ` Rob Herring
  2021-08-20 18:27 ` [PATCH v2 3/4] arm64: dts: rockchip: add i2s1 on rk356x Nicolas Frattaroli
  2021-08-20 18:27 ` [PATCH v2 4/4] arm64: dts: rockchip: add analog audio on Quartz64 Nicolas Frattaroli
  3 siblings, 1 reply; 13+ messages in thread
From: Nicolas Frattaroli @ 2021-08-20 18:27 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Heiko Stuebner,
	Nicolas Frattaroli
  Cc: alsa-devel, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

This adds the YAML bindings for the Rockchip I2S/TDM audio driver.

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 .../bindings/sound/rockchip,i2s-tdm.yaml      | 193 ++++++++++++++++++
 1 file changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml

diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
new file mode 100644
index 000000000000..1e8c1500ba40
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
@@ -0,0 +1,193 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/rockchip,i2s-tdm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip I2S/TDM Controller
+
+description:
+  The Rockchip I2S/TDM Controller is a Time Division Multiplexed
+  audio interface found in various Rockchip SoCs, allowing up
+  to 8 channels of audio over a serial interface.
+
+maintainers:
+  - Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - rockchip,px30-i2s-tdm
+      - rockchip,rk1808-i2s-tdm
+      - rockchip,rk3308-i2s-tdm
+      - rockchip,rk3568-i2s-tdm
+      - rockchip,rv1126-i2s-tdm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  dmas:
+    minItems: 1
+    maxItems: 2
+
+  dma-names:
+    minItems: 1
+    items:
+      - const: rx
+      - const: tx
+
+  clocks:
+    items:
+      - description: clock for TX
+      - description: clock for RX
+      - description: AHB clock driving the interface
+
+  clock-names:
+    items:
+      - const: mclk_tx
+      - const: mclk_rx
+      - const: hclk
+
+  rockchip,frame-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 64
+    minimum: 32
+    maximum: 512
+    description:
+      Width of a frame, usually slot width multiplied by number of slots.
+      Must be even.
+
+  resets:
+    items:
+      - description: reset for TX
+      - description: reset for RX
+
+  reset-names:
+    items:
+      - const: tx-m
+      - const: rx-m
+
+  rockchip,cru:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the cru.
+      Required if both playback and capture are used, i.e. if rockchip,clk-trcm
+      is 0.
+
+  rockchip,grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the syscon node for the GRF register.
+
+  rockchip,mclk-calibrate:
+    description:
+      Enable mclk source calibration.
+    type: boolean
+
+  rockchip,trcm-sync-tx-only:
+    type: boolean
+    description: Use TX BCLK/LRCK for both TX and RX.
+
+  rockchip,trcm-sync-rx-only:
+    type: boolean
+    description: Use RX BCLK/LRCK for both TX and RX.
+
+  "#sound-dai-cells":
+    const: 0
+
+  rockchip,i2s-rx-route:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Defines the mapping of I2S RX sdis to I2S data bus lines.
+      By default, they are mapped one-to-one.
+    items:
+      - description: which sdi to connect to data line 0
+      - description: which sdi to connect to data line 1
+      - description: which sdi to connect to data line 2
+      - description: which sdi to connect to data line 3
+
+  rockchip,i2s-tx-route:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Defines the mapping of I2S TX sdos to I2S data bus lines.
+      By default, they are mapped one-to-one.
+    items:
+      - description: which sdo to connect to data line 0
+      - description: which sdo to connect to data line 1
+      - description: which sdo to connect to data line 2
+      - description: which sdo to connect to data line 3
+
+  rockchip,tdm-fsync-half-frame:
+    description: Whether to use half frame fsync.
+    type: boolean
+
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - dmas
+  - dma-names
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - rockchip,grf
+  - "#sound-dai-cells"
+
+allOf:
+  - if:
+      properties:
+        rockchip,trcm-sync-tx-only: false
+        rockchip,trcm-sync-rx-only: false
+    then:
+      required:
+        - rockchip,cru
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3568-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/pinctrl/rockchip.h>
+
+    foo {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        i2s@fe410000 {
+            compatible = "rockchip,rk3568-i2s-tdm";
+            reg = <0x0 0xfe410000 0x0 0x1000>;
+            interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cru MCLK_I2S1_8CH_TX>, <&cru MCLK_I2S1_8CH_RX>,
+                     <&cru HCLK_I2S1_8CH>;
+            clock-names = "mclk_tx", "mclk_rx", "hclk";
+            dmas = <&dmac1 3>, <&dmac1 2>;
+            dma-names = "rx", "tx";
+            resets = <&cru SRST_M_I2S1_8CH_TX>, <&cru SRST_M_I2S1_8CH_RX>;
+            reset-names = "tx-m", "rx-m";
+            rockchip,trcm-sync-tx-only;
+            rockchip,cru = <&cru>;
+            rockchip,grf = <&grf>;
+            #sound-dai-cells = <0>;
+            pinctrl-names = "default";
+            pinctrl-0 =
+                <&i2s1m0_sclktx
+                &i2s1m0_sclkrx
+                &i2s1m0_lrcktx
+                &i2s1m0_lrckrx
+                &i2s1m0_sdi0
+                &i2s1m0_sdi1
+                &i2s1m0_sdi2
+                &i2s1m0_sdi3
+                &i2s1m0_sdo0
+                &i2s1m0_sdo1
+                &i2s1m0_sdo2
+                &i2s1m0_sdo3>;
+            status = "okay";
+        };
+    };
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/4] arm64: dts: rockchip: add i2s1 on rk356x
  2021-08-20 18:27 [PATCH v2 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
  2021-08-20 18:27 ` [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller Nicolas Frattaroli
  2021-08-20 18:27 ` [PATCH v2 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
@ 2021-08-20 18:27 ` Nicolas Frattaroli
  2021-08-20 18:27 ` [PATCH v2 4/4] arm64: dts: rockchip: add analog audio on Quartz64 Nicolas Frattaroli
  3 siblings, 0 replies; 13+ messages in thread
From: Nicolas Frattaroli @ 2021-08-20 18:27 UTC (permalink / raw)
  To: Rob Herring, Heiko Stuebner
  Cc: Nicolas Frattaroli, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

This adds the necessary device tree node on rk3566 and rk3568
to enable the I2S1 TDM audio controller.

I2S0 has not been added, as it is connected to HDMI and there is
no way to test that it's working without a functioning video
clock (read: VOP2 driver).

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 26 ++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 13b185e1dc5f..fce760996a64 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -559,6 +559,32 @@ sdhci: mmc@fe310000 {
 		status = "disabled";
 	};
 
+	i2s1_8ch: i2s@fe410000 {
+		compatible = "rockchip,rk3568-i2s-tdm";
+		reg = <0x0 0xfe410000 0x0 0x1000>;
+		interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
+		assigned-clocks = <&cru CLK_I2S1_8CH_TX_SRC>, <&cru CLK_I2S1_8CH_RX_SRC>;
+		assigned-clock-rates = <1188000000>, <1188000000>;
+		clocks = <&cru MCLK_I2S1_8CH_TX>, <&cru MCLK_I2S1_8CH_RX>,
+		<&cru HCLK_I2S1_8CH>;
+		clock-names = "mclk_tx", "mclk_rx", "hclk";
+		dmas = <&dmac1 3>, <&dmac1 2>;
+		dma-names = "rx", "tx";
+		resets = <&cru SRST_M_I2S1_8CH_TX>, <&cru SRST_M_I2S1_8CH_RX>;
+		reset-names = "tx-m", "rx-m";
+		rockchip,cru = <&cru>;
+		rockchip,grf = <&grf>;
+		#sound-dai-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_sclkrx
+			     &i2s1m0_lrcktx &i2s1m0_lrckrx
+			     &i2s1m0_sdi0   &i2s1m0_sdi1
+			     &i2s1m0_sdi2   &i2s1m0_sdi3
+			     &i2s1m0_sdo0   &i2s1m0_sdo1
+			     &i2s1m0_sdo2   &i2s1m0_sdo3>;
+		status = "disabled";
+	};
+
 	dmac0: dmac@fe530000 {
 		compatible = "arm,pl330", "arm,primecell";
 		reg = <0x0 0xfe530000 0x0 0x4000>;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/4] arm64: dts: rockchip: add analog audio on Quartz64
  2021-08-20 18:27 [PATCH v2 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
                   ` (2 preceding siblings ...)
  2021-08-20 18:27 ` [PATCH v2 3/4] arm64: dts: rockchip: add i2s1 on rk356x Nicolas Frattaroli
@ 2021-08-20 18:27 ` Nicolas Frattaroli
  3 siblings, 0 replies; 13+ messages in thread
From: Nicolas Frattaroli @ 2021-08-20 18:27 UTC (permalink / raw)
  To: Rob Herring, Heiko Stuebner
  Cc: Nicolas Frattaroli, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On the Quartz64 Model A, the I2S1 TDM controller is connected
to the rk817 codec in I2S mode. Enabling it and adding the
necessary simple-sound-card and codec nodes allows for analog
audio output on the PINE64 Quartz64 Model A SBC.

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   | 35 ++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
index b239f314b38a..c974b0e22ac0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
@@ -50,6 +50,20 @@ led-diy {
 		};
 	};
 
+	rk817-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,name = "Analog RK817";
+		simple-audio-card,mclk-fs = <256>;
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s1_8ch>;
+		};
+		simple-audio-card,codec {
+			sound-dai = <&rk817>;
+		};
+	};
+
 	vcc12v_dcin: vcc12v_dcin {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc12v_dcin";
@@ -174,8 +188,13 @@ rk817: pmic@20 {
 		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
 		clock-output-names = "rk808-clkout1", "rk808-clkout2";
 
+		#sound-dai-cells = <0>;
+		clock-names = "mclk";
+		clocks = <&cru I2S1_MCLKOUT_TX>;
+		assigned-clocks = <&cru I2S1_MCLKOUT_TX>;
+		assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&pmic_int_l>;
+		pinctrl-0 = <&pmic_int_l>, <&i2s1m0_mclk>;
 		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
@@ -364,9 +383,23 @@ regulator-state-mem {
 				};
 			};
 		};
+
+		rk817_codec: codec {
+		};
+
 	};
 };
 
+&i2s1_8ch {
+	status = "okay";
+	rockchip,trcm-sync-tx-only;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2s1m0_sclktx
+		     &i2s1m0_lrcktx
+		     &i2s1m0_sdi0
+		     &i2s1m0_sdo0>;
+};
+
 &mdio1 {
 	rgmii_phy1: ethernet-phy@0 {
 		compatible = "ethernet-phy-ieee802.3-c22";
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-20 18:27 ` [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller Nicolas Frattaroli
@ 2021-08-20 19:02   ` Pierre-Louis Bossart
  2021-08-21 20:45     ` Nicolas Frattaroli
  2021-08-23 12:03   ` Philipp Zabel
  1 sibling, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-08-20 19:02 UTC (permalink / raw)
  To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Heiko Stuebner, Philipp Zabel
  Cc: linux-rockchip, alsa-devel, linux-kernel, linux-arm-kernel


> diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
> new file mode 100644
> index 000000000000..c02b66f3c913
> --- /dev/null
> +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
> @@ -0,0 +1,1737 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ALSA SoC Audio Layer - Rockchip I2S/TDM Controller driver
> + *
> + * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
> + * Author: Sugar Zhang <sugar.zhang@rock-chips.com>
> + * Author: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/delay.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +#include <sound/pcm_params.h>
> +#include <sound/dmaengine_pcm.h>

alphabetical order?

> +static void rockchip_snd_xfer_clear(struct rk_i2s_tdm_dev *i2s_tdm,
> +				    unsigned int clr)
> +{
> +	unsigned int xfer_mask;
> +	unsigned int xfer_val;
> +	unsigned int val;
> +	int retry = 10;
> +	bool tx = clr & I2S_CLR_TXC;
> +	bool rx = clr & I2S_CLR_RXC;
> +
> +	if (!(rx || tx))
> +		return;
> +
> +	xfer_mask = (tx ? I2S_XFER_TXS_START : 0) |
> +		    (rx ? I2S_XFER_RXS_START : 0);
> +	xfer_val = (tx ? I2S_XFER_TXS_STOP : 0) |
> +		   (rx ? I2S_XFER_RXS_STOP : 0);
> +
> +	regmap_update_bits(i2s_tdm->regmap, I2S_XFER, xfer_mask, xfer_val);
> +	udelay(150);
> +	regmap_update_bits(i2s_tdm->regmap, I2S_CLR, clr, clr);
> +
> +	regmap_read(i2s_tdm->regmap, I2S_CLR, &val);
> +	/* Wait on the clear operation to finish */
> +	while (val) {

delay needed here?

> +		regmap_read(i2s_tdm->regmap, I2S_CLR, &val);
> +		retry--;
> +		if (!retry) {
> +			dev_warn(i2s_tdm->dev, "clear failed, reset %s%s\n",
> +				 tx ? "tx" : "", rx ? "rx" : "");
> +			if (rx && tx)
> +				rockchip_snd_xfer_sync_reset(i2s_tdm);
> +			else if (tx)
> +				rockchip_snd_reset(i2s_tdm->tx_reset);
> +			else if (rx)
> +				rockchip_snd_reset(i2s_tdm->rx_reset);
> +			break;
> +		}
> +	}
> +}

> +static int rockchip_i2s_tdm_set_fmt(struct snd_soc_dai *cpu_dai,
> +				    unsigned int fmt)
> +{
> +	struct rk_i2s_tdm_dev *i2s_tdm = to_info(cpu_dai);
> +	unsigned int mask = 0, val = 0, tdm_val = 0, txcr_val = 0, rxcr_val = 0;

not all initializations are required. Run e.g. cppcheck.

> +	int ret = 0;
> +	bool is_tdm = i2s_tdm->tdm_mode;
> +
> +	pm_runtime_get_sync(cpu_dai->dev);

test if this worked?

Also if this fails you need to call pm_runtime_put_noidle()

this is what we use.

ret = pm_runtime_get_sync(dev);
if (ret < 0 && ret != -EACCES) {
      pm_runtime_put_noidle(dev);
      return ret;
}


> +	mask = I2S_CKR_MSS_MASK;
> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +	case SND_SOC_DAIFMT_CBS_CFS:
> +		val = I2S_CKR_MSS_MASTER;
> +		i2s_tdm->is_master_mode = true;
> +		break;
> +	case SND_SOC_DAIFMT_CBM_CFM:
> +		val = I2S_CKR_MSS_SLAVE;
> +		i2s_tdm->is_master_mode = false;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto err_pm_put;
> +	}
> +
> +	regmap_update_bits(i2s_tdm->regmap, I2S_CKR, mask, val);
> +
> +	mask = I2S_CKR_CKP_MASK | I2S_CKR_TLP_MASK | I2S_CKR_RLP_MASK;
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_NB_NF:
> +		val = I2S_CKR_CKP_NORMAL |
> +		      I2S_CKR_TLP_NORMAL |
> +		      I2S_CKR_RLP_NORMAL;
> +		break;
> +	case SND_SOC_DAIFMT_NB_IF:
> +		val = I2S_CKR_CKP_NORMAL |
> +		      I2S_CKR_TLP_INVERTED |
> +		      I2S_CKR_RLP_INVERTED;
> +		break;
> +	case SND_SOC_DAIFMT_IB_NF:
> +		val = I2S_CKR_CKP_INVERTED |
> +		      I2S_CKR_TLP_NORMAL |
> +		      I2S_CKR_RLP_NORMAL;
> +		break;
> +	case SND_SOC_DAIFMT_IB_IF:
> +		val = I2S_CKR_CKP_INVERTED |
> +		      I2S_CKR_TLP_INVERTED |
> +		      I2S_CKR_RLP_INVERTED;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto err_pm_put;
> +	}
> +
> +	regmap_update_bits(i2s_tdm->regmap, I2S_CKR, mask, val);
> +
> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_RIGHT_J:
> +		txcr_val = I2S_TXCR_IBM_RSJM;
> +		rxcr_val = I2S_RXCR_IBM_RSJM;
> +		break;
> +	case SND_SOC_DAIFMT_LEFT_J:
> +		txcr_val = I2S_TXCR_IBM_LSJM;
> +		rxcr_val = I2S_RXCR_IBM_LSJM;
> +		break;
> +	case SND_SOC_DAIFMT_I2S:
> +		txcr_val = I2S_TXCR_IBM_NORMAL;
> +		rxcr_val = I2S_RXCR_IBM_NORMAL;
> +		break;
> +	case SND_SOC_DAIFMT_DSP_A: /* PCM no delay mode */
> +		txcr_val = I2S_TXCR_TFS_PCM;
> +		rxcr_val = I2S_RXCR_TFS_PCM;
> +		break;
> +	case SND_SOC_DAIFMT_DSP_B: /* PCM delay 1 mode */
> +		txcr_val = I2S_TXCR_TFS_PCM | I2S_TXCR_PBM_MODE(1);
> +		rxcr_val = I2S_RXCR_TFS_PCM | I2S_RXCR_PBM_MODE(1);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto err_pm_put;
> +	}
> +
> +	mask = I2S_TXCR_IBM_MASK | I2S_TXCR_TFS_MASK | I2S_TXCR_PBM_MASK;
> +	regmap_update_bits(i2s_tdm->regmap, I2S_TXCR, mask, txcr_val);
> +
> +	mask = I2S_RXCR_IBM_MASK | I2S_RXCR_TFS_MASK | I2S_RXCR_PBM_MASK;
> +	regmap_update_bits(i2s_tdm->regmap, I2S_RXCR, mask, rxcr_val);
> +
> +	if (is_tdm) {
> +		switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +		case SND_SOC_DAIFMT_RIGHT_J:
> +			val = I2S_TXCR_TFS_TDM_I2S;
> +			tdm_val = TDM_SHIFT_CTRL(2);
> +			break;
> +		case SND_SOC_DAIFMT_LEFT_J:
> +			val = I2S_TXCR_TFS_TDM_I2S;
> +			tdm_val = TDM_SHIFT_CTRL(1);
> +			break;
> +		case SND_SOC_DAIFMT_I2S:
> +			val = I2S_TXCR_TFS_TDM_I2S;
> +			tdm_val = TDM_SHIFT_CTRL(0);
> +			break;
> +		case SND_SOC_DAIFMT_DSP_A:
> +			val = I2S_TXCR_TFS_TDM_PCM;
> +			tdm_val = TDM_SHIFT_CTRL(0);
> +			break;
> +		case SND_SOC_DAIFMT_DSP_B:
> +			val = I2S_TXCR_TFS_TDM_PCM;
> +			tdm_val = TDM_SHIFT_CTRL(2);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +			goto err_pm_put;
> +		}
> +
> +		tdm_val |= TDM_FSYNC_WIDTH_SEL1(1);
> +		if (i2s_tdm->tdm_fsync_half_frame)
> +			tdm_val |= TDM_FSYNC_WIDTH_HALF_FRAME;
> +		else
> +			tdm_val |= TDM_FSYNC_WIDTH_ONE_FRAME;
> +
> +		mask = I2S_TXCR_TFS_MASK;
> +		regmap_update_bits(i2s_tdm->regmap, I2S_TXCR, mask, val);
> +		regmap_update_bits(i2s_tdm->regmap, I2S_RXCR, mask, val);
> +
> +		mask = TDM_FSYNC_WIDTH_SEL1_MSK | TDM_FSYNC_WIDTH_SEL0_MSK |
> +		       TDM_SHIFT_CTRL_MSK;
> +		regmap_update_bits(i2s_tdm->regmap, I2S_TDM_TXCR,
> +				   mask, tdm_val);
> +		regmap_update_bits(i2s_tdm->regmap, I2S_TDM_RXCR,
> +				   mask, tdm_val);
> +	}
> +
> +err_pm_put:
> +	pm_runtime_put(cpu_dai->dev);
> +
> +	return ret;
> +}
> +

> +static int rockchip_i2s_tdm_clk_set_rate(struct rk_i2s_tdm_dev *i2s_tdm,
> +					 struct clk *clk, unsigned long rate,
> +					 int ppm)
> +{
> +	unsigned long rate_target;
> +	int delta, ret;
> +
> +	if (ppm == i2s_tdm->clk_ppm)
> +		return 0;
> +
> +	delta = (ppm < 0) ? -1 : 1;
> +	delta *= (int)div64_u64((u64)rate * (u64)abs(ppm) + 500000,
> +				1000000);

formula looks odd? looks like you are implementing a round to nearest
operation, but that shouldn't require this multiplication?

> +
> +	rate_target = rate + delta;
> +
> +	if (!rate_target)
> +		return -EINVAL;
> +
> +	ret = clk_set_rate(clk, rate_target);
> +	if (ret)
> +		return ret;
> +
> +	i2s_tdm->clk_ppm = ppm;
> +
> +	return ret;
> +}
> +
> +static int rockchip_i2s_tdm_calibrate_mclk(struct rk_i2s_tdm_dev *i2s_tdm,
> +					   struct snd_pcm_substream *substream,
> +					   unsigned int lrck_freq)
> +{
> +	struct clk *mclk_root;
> +	struct clk *mclk_parent;
> +	unsigned int mclk_root_freq;
> +	unsigned int mclk_root_initial_freq;
> +	unsigned int mclk_parent_freq;
> +	unsigned int div, delta;
> +	u64 ppm;
> +	int ret;
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		mclk_parent = i2s_tdm->mclk_tx_src;
> +	else
> +		mclk_parent = i2s_tdm->mclk_rx_src;
> +
> +	switch (lrck_freq) {
> +	case 8000:
> +	case 16000:
> +	case 24000:
> +	case 32000:
> +	case 48000:
> +	case 64000:
> +	case 96000:
> +	case 192000:
> +		mclk_root = i2s_tdm->mclk_root0;
> +		mclk_root_freq = i2s_tdm->mclk_root0_freq;
> +		mclk_root_initial_freq = i2s_tdm->mclk_root0_initial_freq;
> +		mclk_parent_freq = DEFAULT_MCLK_FS * 192000;
> +		break;
> +	case 11025:
> +	case 22050:
> +	case 44100:
> +	case 88200:
> +	case 176400:
> +		mclk_root = i2s_tdm->mclk_root1;
> +		mclk_root_freq = i2s_tdm->mclk_root1_freq;
> +		mclk_root_initial_freq = i2s_tdm->mclk_root1_initial_freq;
> +		mclk_parent_freq = DEFAULT_MCLK_FS * 176400;
> +		break;
> +	default:
> +		dev_err(i2s_tdm->dev, "Invalid LRCK freq: %u Hz\n",
> +			lrck_freq);
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_set_parent(mclk_parent, mclk_root);
> +	if (ret)
> +		return ret;
> +
> +	ret = rockchip_i2s_tdm_clk_set_rate(i2s_tdm, mclk_root,
> +					    mclk_root_freq, 0);
> +	if (ret)
> +		return ret;
> +
> +	delta = abs(mclk_root_freq % mclk_parent_freq - mclk_parent_freq);
> +	ppm = div64_u64((uint64_t)delta * 1000000, (uint64_t)mclk_root_freq);
> +
> +	if (ppm) {
> +		div = DIV_ROUND_CLOSEST(mclk_root_initial_freq, mclk_parent_freq);
> +		if (!div)
> +			return -EINVAL;
> +
> +		mclk_root_freq = mclk_parent_freq * round_up(div, 2);
> +
> +		ret = clk_set_rate(mclk_root, mclk_root_freq);
> +		if (ret)
> +			return ret;
> +
> +		i2s_tdm->mclk_root0_freq = clk_get_rate(i2s_tdm->mclk_root0);
> +		i2s_tdm->mclk_root1_freq = clk_get_rate(i2s_tdm->mclk_root1);
> +	}
> +
> +	ret = clk_set_rate(mclk_parent, mclk_parent_freq);
> +
> +	return ret;

return clk_set_rate(mclk_parent, mclk_parent_freq); ?

> +}
> +
> +static int rockchip_i2s_tdm_set_mclk(struct rk_i2s_tdm_dev *i2s_tdm,
> +				     struct snd_pcm_substream *substream,
> +				     struct clk **mclk)
> +{
> +	unsigned int mclk_freq;
> +	int ret;
> +
> +	if (i2s_tdm->clk_trcm) {
> +		if (i2s_tdm->mclk_tx_freq != i2s_tdm->mclk_rx_freq) {
> +			dev_err(i2s_tdm->dev,
> +				"clk_trcm, tx: %d and rx: %d should be same\n",
> +				i2s_tdm->mclk_tx_freq,
> +				i2s_tdm->mclk_rx_freq);
> +			ret = -EINVAL;
> +			return ret;

return -EINVAL?

> +		}
> +
> +		ret = clk_set_rate(i2s_tdm->mclk_tx, i2s_tdm->mclk_tx_freq);
> +		if (ret)
> +			return ret;
> +
> +		ret = clk_set_rate(i2s_tdm->mclk_rx, i2s_tdm->mclk_rx_freq);
> +		if (ret)
> +			return ret;
> +
> +		/* mclk_rx is also ok. */
> +		*mclk = i2s_tdm->mclk_tx;
> +	} else {
> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +			*mclk = i2s_tdm->mclk_tx;
> +			mclk_freq = i2s_tdm->mclk_tx_freq;
> +		} else {
> +			*mclk = i2s_tdm->mclk_rx;
> +			mclk_freq = i2s_tdm->mclk_rx_freq;
> +		}
> +
> +		ret = clk_set_rate(*mclk, mclk_freq);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +

> +	pm_runtime_enable(&pdev->dev);
> +	if (!pm_runtime_enabled(&pdev->dev)) {
> +		ret = i2s_tdm_runtime_resume(&pdev->dev);

that looks like dead code? you've just enabled pm_runtime, why would
this fail?

> +		if (ret)
> +			goto err_pm_disable;
> +	}
> +
> +	regmap_update_bits(i2s_tdm->regmap, I2S_DMACR, I2S_DMACR_TDL_MASK,
> +			   I2S_DMACR_TDL(16));
> +	regmap_update_bits(i2s_tdm->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK,
> +			   I2S_DMACR_RDL(16));
> +	regmap_update_bits(i2s_tdm->regmap, I2S_CKR, I2S_CKR_TRCM_MASK,
> +			   i2s_tdm->clk_trcm << I2S_CKR_TRCM_SHIFT);
> +
> +	if (i2s_tdm->soc_data && i2s_tdm->soc_data->init)
> +		i2s_tdm->soc_data->init(&pdev->dev, res->start);
> +
> +	ret = devm_snd_soc_register_component(&pdev->dev,
> +					      &rockchip_i2s_tdm_component,
> +					      &i2s_tdm_dai, 1);
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "Could not register DAI\n");
> +		goto err_suspend;
> +	}
> +
> +	ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Could not register PCM\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +
> +err_suspend:
> +	if (!pm_runtime_status_suspended(&pdev->dev))
> +		i2s_tdm_runtime_suspend(&pdev->dev);

why is this necessary?

> +err_pm_disable:
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return ret;
> +}
> +
> +static int rockchip_i2s_tdm_remove(struct platform_device *pdev)
> +{
> +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(&pdev->dev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +	if (!pm_runtime_status_suspended(&pdev->dev))
> +		i2s_tdm_runtime_suspend(&pdev->dev);

this looks backwards, if you disable pm_runtime first what is the
expectation for the rest.
> +
> +	if (!IS_ERR(i2s_tdm->mclk_tx))
> +		clk_prepare_enable(i2s_tdm->mclk_tx);
> +	if (!IS_ERR(i2s_tdm->mclk_rx))
> +		clk_prepare_enable(i2s_tdm->mclk_rx);
> +	if (!IS_ERR(i2s_tdm->hclk))
> +		clk_disable_unprepare(i2s_tdm->hclk);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP

use __maybe_unused

> +static int rockchip_i2s_tdm_suspend(struct device *dev)
> +{
> +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
> +
> +	regcache_mark_dirty(i2s_tdm->regmap);
> +
> +	return 0;
> +}
> +
> +static int rockchip_i2s_tdm_resume(struct device *dev)
> +{
> +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0)
> +		return ret;
> +	ret = regcache_sync(i2s_tdm->regmap);
> +	pm_runtime_put(dev);
> +
> +	return ret;
> +}
> +#endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-20 19:02   ` Pierre-Louis Bossart
@ 2021-08-21 20:45     ` Nicolas Frattaroli
  2021-08-23 11:19       ` Mark Brown
  2021-08-23 15:02       ` Pierre-Louis Bossart
  0 siblings, 2 replies; 13+ messages in thread
From: Nicolas Frattaroli @ 2021-08-21 20:45 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Heiko Stuebner, Philipp Zabel, Pierre-Louis Bossart
  Cc: linux-rockchip, alsa-devel, linux-kernel, linux-arm-kernel

On Freitag, 20. August 2021 21:02:16 CEST Pierre-Louis Bossart wrote:
> > +	regmap_read(i2s_tdm->regmap, I2S_CLR, &val);
> > +	/* Wait on the clear operation to finish */
> > +	while (val) {
> 
> delay needed here?
> 

The rockchip_i2s.c code doesn't have a delay here either, but I can
add one of 1 usec for good measure, it seems weird to retry the
read as fast as it can.

> > +static int rockchip_i2s_tdm_clk_set_rate(struct rk_i2s_tdm_dev *i2s_tdm,
> > +					 struct clk *clk, unsigned long rate,
> > +					 int ppm)
> > +{
> > +	unsigned long rate_target;
> > +	int delta, ret;
> > +
> > +	if (ppm == i2s_tdm->clk_ppm)
> > +		return 0;
> > +
> > +	delta = (ppm < 0) ? -1 : 1;
> > +	delta *= (int)div64_u64((u64)rate * (u64)abs(ppm) + 500000,
> > +				1000000);
> 
> formula looks odd? looks like you are implementing a round to nearest
> operation, but that shouldn't require this multiplication?
> 

I believe the multiplication is there to compensate for clock drift.
ppm is a value between -1000 and 1000 that specifies the clock drift
in presumably parts per million, going by the variable name.

> > +	pm_runtime_enable(&pdev->dev);
> > +	if (!pm_runtime_enabled(&pdev->dev)) {
> > +		ret = i2s_tdm_runtime_resume(&pdev->dev);
> 
> that looks like dead code? you've just enabled pm_runtime, why would
> this fail?
> 

I've had a look at the upstream rockchip_i2s.c code which does the
same thing, and I believe the idea here is that we need to manually
prepare and enable the master clocks (mclk_rx/mclk_tx) if pm_runtime
is not available. Otherwise, pm_runtime will presumably call our
resume callback at some point.

If runtime power management is disabled in the kernel config then 
pm_runtime_enabled is always going to return false.

> > +err_suspend:
> > +	if (!pm_runtime_status_suspended(&pdev->dev))
> > +		i2s_tdm_runtime_suspend(&pdev->dev);
> 
> why is this necessary?

I believe this is the same kind of situation as before, and the
other driver does this too: if pm_runtime is not available, we
need to stop our clocks manually on probe failure.
 
> > +err_pm_disable:
> > +	pm_runtime_disable(&pdev->dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static int rockchip_i2s_tdm_remove(struct platform_device *pdev)
> > +{
> > +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(&pdev->dev);
> > +
> > +	pm_runtime_disable(&pdev->dev);
> > +	if (!pm_runtime_status_suspended(&pdev->dev))
> > +		i2s_tdm_runtime_suspend(&pdev->dev);
> 
> this looks backwards, if you disable pm_runtime first what is the
> expectation for the rest.

I'm not well versed in the PM code but if my theory of this being
related to unavailable PM is correct, then my best guess is that
pm_runtime_disable does suspend the device, so if it's not
suspended then we don't have pm_runtime and therefore need to call
it manually.

> > +
> > +	if (!IS_ERR(i2s_tdm->mclk_tx))
> > +		clk_prepare_enable(i2s_tdm->mclk_tx);
> > +	if (!IS_ERR(i2s_tdm->mclk_rx))
> > +		clk_prepare_enable(i2s_tdm->mclk_rx);
> > +	if (!IS_ERR(i2s_tdm->hclk))
> > +		clk_disable_unprepare(i2s_tdm->hclk);
> > +
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> 
> use __maybe_unused

You mean instead of the ifdef stuff to just add this attribute to
the following functions like this?

static int rockchip_i2s_tdm_suspend(struct device *dev) __maybe_unused

> 
> > +static int rockchip_i2s_tdm_suspend(struct device *dev)
> > +{
> > +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
> > +
> > +	regcache_mark_dirty(i2s_tdm->regmap);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rockchip_i2s_tdm_resume(struct device *dev)
> > +{
> > +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = pm_runtime_get_sync(dev);
> > +	if (ret < 0)
> > +		return ret;
> > +	ret = regcache_sync(i2s_tdm->regmap);
> > +	pm_runtime_put(dev);
> > +
> > +	return ret;
> > +}
> > +#endif

Thank you for your review!

Regards,
Nicolas Frattaroli




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-21 20:45     ` Nicolas Frattaroli
@ 2021-08-23 11:19       ` Mark Brown
  2021-08-23 15:02       ` Pierre-Louis Bossart
  1 sibling, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-08-23 11:19 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Heiko Stuebner,
	Philipp Zabel, Pierre-Louis Bossart, linux-rockchip, alsa-devel,
	linux-kernel, linux-arm-kernel

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

On Sat, Aug 21, 2021 at 10:45:52PM +0200, Nicolas Frattaroli wrote:
> On Freitag, 20. August 2021 21:02:16 CEST Pierre-Louis Bossart wrote:
> > > +	regmap_read(i2s_tdm->regmap, I2S_CLR, &val);
> > > +	/* Wait on the clear operation to finish */
> > > +	while (val) {

> > delay needed here?

> The rockchip_i2s.c code doesn't have a delay here either, but I can
> add one of 1 usec for good measure, it seems weird to retry the
> read as fast as it can.

You should probably have a cpu_relax() in there at least.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-20 18:27 ` [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller Nicolas Frattaroli
  2021-08-20 19:02   ` Pierre-Louis Bossart
@ 2021-08-23 12:03   ` Philipp Zabel
  2021-08-23 14:35     ` Nicolas Frattaroli
  1 sibling, 1 reply; 13+ messages in thread
From: Philipp Zabel @ 2021-08-23 12:03 UTC (permalink / raw)
  To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Heiko Stuebner
  Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-rockchip

Hi Nicolas,

On Fri, 2021-08-20 at 20:27 +0200, Nicolas Frattaroli wrote:
[...]
> diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
> new file mode 100644
> index 000000000000..c02b66f3c913
> --- /dev/null
> +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
> @@ -0,0 +1,1737 @@
[...]
> +static void rockchip_snd_xfer_reset_assert(struct rk_i2s_tdm_dev *i2s_tdm,
> +					   int tx_bank, int tx_offset,
> +					   int rx_bank, int rx_offset)
> +{
> +	void __iomem *cru_reset;
> +	unsigned long flags;
> +
> +	cru_reset = i2s_tdm->cru_base + i2s_tdm->soc_data->softrst_offset;
> +
> +	if (tx_bank == rx_bank) {
> +		writel(BIT(tx_offset) | BIT(rx_offset) |
> +		       (BIT(tx_offset) << 16) | (BIT(rx_offset) << 16),
> +		       cru_reset + (tx_bank * 4));
> +	} else {
> +		local_irq_save(flags);
> +		writel(BIT(tx_offset) | (BIT(tx_offset) << 16),
> +		       cru_reset + (tx_bank * 4));
> +		writel(BIT(rx_offset) | (BIT(rx_offset) << 16),
> +		       cru_reset + (rx_bank * 4));
> +		local_irq_restore(flags);
> +	}
> +}
> +
> +static void rockchip_snd_xfer_reset_deassert(struct rk_i2s_tdm_dev *i2s_tdm,
> +					     int tx_bank, int tx_offset,
> +					     int rx_bank, int rx_offset)
> +{
> +	void __iomem *cru_reset;
> +	unsigned long flags;
> +
> +	cru_reset = i2s_tdm->cru_base + i2s_tdm->soc_data->softrst_offset;
> +
> +	if (tx_bank == rx_bank) {
> +		writel((BIT(tx_offset) << 16) | (BIT(rx_offset) << 16),
> +		       cru_reset + (tx_bank * 4));
> +	} else {
> +		local_irq_save(flags);
> +		writel((BIT(tx_offset) << 16),
> +		       cru_reset + (tx_bank * 4));
> +		writel((BIT(rx_offset) << 16),
> +		       cru_reset + (rx_bank * 4));
> +		local_irq_restore(flags);
> +	}
> +}
> +
> +/*
> + * Makes sure that both tx and rx are reset at the same time to sync lrck
> + * when clk_trcm > 0.
> + */
> +static void rockchip_snd_xfer_sync_reset(struct rk_i2s_tdm_dev *i2s_tdm)
> +{
> +	int tx_id, rx_id;
> +	int tx_bank, rx_bank, tx_offset, rx_offset;
> +
> +	if (!i2s_tdm->cru_base || !i2s_tdm->soc_data)
> +		return;
> +
> +	tx_id = i2s_tdm->tx_reset_id;
> +	rx_id = i2s_tdm->rx_reset_id;
> +	if (tx_id < 0 || rx_id < 0)
> +		return;
> +
> +	tx_bank = tx_id / 16;
> +	tx_offset = tx_id % 16;
> +	rx_bank = rx_id / 16;
> +	rx_offset = rx_id % 16;
> +	dev_dbg(i2s_tdm->dev,
> +		"tx_bank: %d, rx_bank: %d, tx_offset: %d, rx_offset: %d\n",
> +		tx_bank, rx_bank, tx_offset, rx_offset);
> +
> +	rockchip_snd_xfer_reset_assert(i2s_tdm, tx_bank, tx_offset,
> +				       rx_bank, rx_offset);
> +
> +	udelay(150);
> +
> +	rockchip_snd_xfer_reset_deassert(i2s_tdm, tx_bank, tx_offset,
> +					 rx_bank, rx_offset);
> +}

I'm not too fond of reimplementing half a reset controller in here.
The reset framework does not support synchronized (de)assertion of
multiple reset controls, I wonder if this would be useful to add.
Without having thought about this too hard, I could imagine this as an
extension to the bulk API, or possibly a call to join multiple reset
controls into a reset control array.

> +static void rockchip_snd_reset(struct reset_control *rc)
> +{
> +	if (IS_ERR(rc))
> +		return;

This shouldn't be called with an error code, see the comment about
optional reset controls below.

> +
> +	reset_control_assert(rc);
> +	udelay(1);

What is the reason for the different delays in
rockchip_snd_xfer_sync_reset() and rockchip_snd_reset()?

> +	reset_control_deassert(rc);
> +}
[...]
> +static int rockchip_i2s_tdm_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct device_node *cru_node;
> +	const struct of_device_id *of_id;
> +	struct rk_i2s_tdm_dev *i2s_tdm;
> +	struct resource *res;
> +	void __iomem *regs;
> +	int ret;
> +	int val;
> +
> +	i2s_tdm = devm_kzalloc(&pdev->dev, sizeof(*i2s_tdm), GFP_KERNEL);
> +	if (!i2s_tdm)
> +		return -ENOMEM;
> +
> +	i2s_tdm->dev = &pdev->dev;
> +
> +	of_id = of_match_device(rockchip_i2s_tdm_match, &pdev->dev);
> +	if (!of_id || !of_id->data)
> +		return -EINVAL;
> +
> +	spin_lock_init(&i2s_tdm->lock);
> +	i2s_tdm->soc_data = (struct rk_i2s_soc_data *)of_id->data;
> +
> +	i2s_tdm->frame_width = 64;
> +	if (!of_property_read_u32(node, "rockchip,frame-width", &val)) {
> +		if (val >= 32 && (val % 2 == 0) && val <= 512) {
> +			i2s_tdm->frame_width = val;
> +		} else {
> +			dev_err(i2s_tdm->dev, "unsupported frame width: '%d'\n",
> +				val);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	i2s_tdm->clk_trcm = TRCM_TXRX;
> +	if (of_property_read_bool(node, "rockchip,trcm-sync-tx-only"))
> +		i2s_tdm->clk_trcm = TRCM_TX;
> +	if (of_property_read_bool(node, "rockchip,trcm-sync-rx-only")) {
> +		if (i2s_tdm->clk_trcm) {
> +			dev_err(i2s_tdm->dev, "invalid trcm-sync configuration\n");
> +			return -EINVAL;
> +		}
> +		i2s_tdm->clk_trcm = TRCM_RX;
> +	}
> +	if (i2s_tdm->clk_trcm != TRCM_TXRX)
> +		i2s_tdm_dai.symmetric_rate = 1;
> +
> +	i2s_tdm->tdm_fsync_half_frame =
> +		of_property_read_bool(node, "rockchip,tdm-fsync-half-frame");
> +
> +	i2s_tdm->grf = syscon_regmap_lookup_by_phandle(node, "rockchip,grf");
> +	if (IS_ERR(i2s_tdm->grf))
> +		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->grf),
> +				     "Error in rockchip,grf\n");
> +
> +	if (i2s_tdm->clk_trcm != TRCM_TXRX) {
> +		cru_node = of_parse_phandle(node, "rockchip,cru", 0);
> +		i2s_tdm->cru_base = of_iomap(cru_node, 0);

This is a bit ugly if there is another driver sitting on the
rockchip,cru compatible node. Which reset controller driver is backing
the reset controls below?

> +		of_node_put(cru_node);
> +		if (!i2s_tdm->cru_base) {
> +			dev_err(i2s_tdm->dev,
> +				"Missing or unsupported rockchip,cru node\n");
> +			return -ENOENT;
> +		}
> +
> +		i2s_tdm->tx_reset_id = of_i2s_resetid_get(node, "tx-m");
> +		i2s_tdm->rx_reset_id = of_i2s_resetid_get(node, "rx-m");
> +	}
> +
> +	i2s_tdm->tx_reset = devm_reset_control_get(&pdev->dev, "tx-m");

Please use devm_reset_control_get_exclusive() instead.

> +	if (IS_ERR(i2s_tdm->tx_reset)) {
> +		ret = PTR_ERR(i2s_tdm->tx_reset);
> +		if (ret != -ENOENT)
> +			return dev_err_probe(i2s_tdm->dev, ret,
> +					     "Error in tx-m reset control\n");
> +	}
> +
> +	i2s_tdm->rx_reset = devm_reset_control_get(&pdev->dev, "rx-m");
> +	if (IS_ERR(i2s_tdm->rx_reset)) {
> +		ret = PTR_ERR(i2s_tdm->rx_reset);
> +		if (ret != -ENOENT)

Why is -ENOENT acceptable? If you want these to be optional, use
devm_reset_control_get_optional_exclusive() [1] instead. That will
return NULL if the reset is not specified in the device tree, which will
be ignored by reset_control_(de)assert().

[1] https://www.kernel.org/doc/html/latest/driver-api/reset.html#c.devm_reset_control_get_optional_exclusive

> +			return dev_err_probe(i2s_tdm->dev, ret,
> +					     "Error in rx-m reset control\n");
> +	}
> +
> +	i2s_tdm->hclk = devm_clk_get(&pdev->dev, "hclk");
> +	if (IS_ERR(i2s_tdm->hclk)) {
> +		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->hclk),
> +				     "Failed to get clock hclk\n");
> +	}
> +
> +	ret = clk_prepare_enable(i2s_tdm->hclk);
> +	if (ret) {
> +		return dev_err_probe(i2s_tdm->dev, ret,
> +				     "Failed to enable clock hclk\n");
> +	}

This clock is left enabled in all the error paths below. You could
install a cleanup action with devm_add_action_or_reset(), or better
start enabling the hardware only after acquisition of all required
resources has succeeded.

[...]
> +static int rockchip_i2s_tdm_remove(struct platform_device *pdev)
> +{
> +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(&pdev->dev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +	if (!pm_runtime_status_suspended(&pdev->dev))
> +		i2s_tdm_runtime_suspend(&pdev->dev);
> +
> +	if (!IS_ERR(i2s_tdm->mclk_tx))
> +		clk_prepare_enable(i2s_tdm->mclk_tx);
> +	if (!IS_ERR(i2s_tdm->mclk_rx))
> +		clk_prepare_enable(i2s_tdm->mclk_rx);

Why are we enabling these clocks now?

> +	if (!IS_ERR(i2s_tdm->hclk))
> +		clk_disable_unprepare(i2s_tdm->hclk);
> +
> +	return 0;
> +}

regards
Philipp

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-23 12:03   ` Philipp Zabel
@ 2021-08-23 14:35     ` Nicolas Frattaroli
  2021-08-23 16:03       ` Philipp Zabel
  0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Frattaroli @ 2021-08-23 14:35 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Heiko Stuebner, Philipp Zabel
  Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-rockchip

Hi Philipp,

On Montag, 23. August 2021 14:03:25 CEST Philipp Zabel wrote:
> Hi Nicolas,
> 
> On Fri, 2021-08-20 at 20:27 +0200, Nicolas Frattaroli wrote:
> [...]
> 
> > diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c
> > b/sound/soc/rockchip/rockchip_i2s_tdm.c new file mode 100644
> > index 000000000000..c02b66f3c913
> > --- /dev/null
> > +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
> > @@ -0,0 +1,1737 @@
> 
> [...]
> 
> > +static void rockchip_snd_xfer_reset_assert(struct rk_i2s_tdm_dev
> > *i2s_tdm,
> > +					   int tx_bank, int tx_offset,
> > +					   int rx_bank, int rx_offset)
> > +{
> > +	void __iomem *cru_reset;
> > +	unsigned long flags;
> > +
> > +	cru_reset = i2s_tdm->cru_base + i2s_tdm->soc_data->softrst_offset;
> > +
> > +	if (tx_bank == rx_bank) {
> > +		writel(BIT(tx_offset) | BIT(rx_offset) |
> > +		       (BIT(tx_offset) << 16) | (BIT(rx_offset) << 16),
> > +		       cru_reset + (tx_bank * 4));
> > +	} else {
> > +		local_irq_save(flags);
> > +		writel(BIT(tx_offset) | (BIT(tx_offset) << 16),
> > +		       cru_reset + (tx_bank * 4));
> > +		writel(BIT(rx_offset) | (BIT(rx_offset) << 16),
> > +		       cru_reset + (rx_bank * 4));
> > +		local_irq_restore(flags);
> > +	}
> > +}
> > +
> > +static void rockchip_snd_xfer_reset_deassert(struct rk_i2s_tdm_dev
> > *i2s_tdm, +					     int tx_bank, int 
tx_offset,
> > +					     int rx_bank, int rx_offset)
> > +{
> > +	void __iomem *cru_reset;
> > +	unsigned long flags;
> > +
> > +	cru_reset = i2s_tdm->cru_base + i2s_tdm->soc_data->softrst_offset;
> > +
> > +	if (tx_bank == rx_bank) {
> > +		writel((BIT(tx_offset) << 16) | (BIT(rx_offset) << 16),
> > +		       cru_reset + (tx_bank * 4));
> > +	} else {
> > +		local_irq_save(flags);
> > +		writel((BIT(tx_offset) << 16),
> > +		       cru_reset + (tx_bank * 4));
> > +		writel((BIT(rx_offset) << 16),
> > +		       cru_reset + (rx_bank * 4));
> > +		local_irq_restore(flags);
> > +	}
> > +}
> > +
> > +/*
> > + * Makes sure that both tx and rx are reset at the same time to sync lrck
> > + * when clk_trcm > 0.
> > + */
> > +static void rockchip_snd_xfer_sync_reset(struct rk_i2s_tdm_dev *i2s_tdm)
> > +{
> > +	int tx_id, rx_id;
> > +	int tx_bank, rx_bank, tx_offset, rx_offset;
> > +
> > +	if (!i2s_tdm->cru_base || !i2s_tdm->soc_data)
> > +		return;
> > +
> > +	tx_id = i2s_tdm->tx_reset_id;
> > +	rx_id = i2s_tdm->rx_reset_id;
> > +	if (tx_id < 0 || rx_id < 0)
> > +		return;
> > +
> > +	tx_bank = tx_id / 16;
> > +	tx_offset = tx_id % 16;
> > +	rx_bank = rx_id / 16;
> > +	rx_offset = rx_id % 16;
> > +	dev_dbg(i2s_tdm->dev,
> > +		"tx_bank: %d, rx_bank: %d, tx_offset: %d, rx_offset: %d\n",
> > +		tx_bank, rx_bank, tx_offset, rx_offset);
> > +
> > +	rockchip_snd_xfer_reset_assert(i2s_tdm, tx_bank, tx_offset,
> > +				       rx_bank, rx_offset);
> > +
> > +	udelay(150);
> > +
> > +	rockchip_snd_xfer_reset_deassert(i2s_tdm, tx_bank, tx_offset,
> > +					 rx_bank, rx_offset);
> > +}
> 
> I'm not too fond of reimplementing half a reset controller in here.
> The reset framework does not support synchronized (de)assertion of
> multiple reset controls, I wonder if this would be useful to add.
> Without having thought about this too hard, I could imagine this as an
> extension to the bulk API, or possibly a call to join multiple reset
> controls into a reset control array.

I agree, the code required for synchronised reset seems to be a good
candidate for a generalised solution elsewhere.
However, I'm not sure if I'm the right candidate to design this API
as my first kernel contribution when the board I'm currently testing
this with doesn't even utilise the synchronized reset.

If I come across an opportunity to test synchronised resets, I'll
definitely look into refactoring this. I'd also be happy to hear of
any other driver which is currently implementing its own synchronised
reset.

[...]
 
> > +
> > +	reset_control_assert(rc);
> > +	udelay(1);
> 
> What is the reason for the different delays in
> rockchip_snd_xfer_sync_reset() and rockchip_snd_reset()?
> 

Simply put: I have no idea. This is what downstream does, and it
appears to work for me. The git history of the downstream kernel
also doesn't tell me anything about why the sync reset is 150
and this one is 1.

I've got no device to test the sync reset with at the moment so
I'm wary of playing with the delay value.

> > +	reset_control_deassert(rc);
> > +}
> 
> [...]
> 
> > +static int rockchip_i2s_tdm_probe(struct platform_device *pdev)
> > +{
> > +	struct device_node *node = pdev->dev.of_node;
> > +	struct device_node *cru_node;
> > +	const struct of_device_id *of_id;
> > +	struct rk_i2s_tdm_dev *i2s_tdm;
> > +	struct resource *res;
> > +	void __iomem *regs;
> > +	int ret;
> > +	int val;
> > +
> > +	i2s_tdm = devm_kzalloc(&pdev->dev, sizeof(*i2s_tdm), GFP_KERNEL);
> > +	if (!i2s_tdm)
> > +		return -ENOMEM;
> > +
> > +	i2s_tdm->dev = &pdev->dev;
> > +
> > +	of_id = of_match_device(rockchip_i2s_tdm_match, &pdev->dev);
> > +	if (!of_id || !of_id->data)
> > +		return -EINVAL;
> > +
> > +	spin_lock_init(&i2s_tdm->lock);
> > +	i2s_tdm->soc_data = (struct rk_i2s_soc_data *)of_id->data;
> > +
> > +	i2s_tdm->frame_width = 64;
> > +	if (!of_property_read_u32(node, "rockchip,frame-width", &val)) {
> > +		if (val >= 32 && (val % 2 == 0) && val <= 512) {
> > +			i2s_tdm->frame_width = val;
> > +		} else {
> > +			dev_err(i2s_tdm->dev, "unsupported frame width: 
'%d'\n",
> > +				val);
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	i2s_tdm->clk_trcm = TRCM_TXRX;
> > +	if (of_property_read_bool(node, "rockchip,trcm-sync-tx-only"))
> > +		i2s_tdm->clk_trcm = TRCM_TX;
> > +	if (of_property_read_bool(node, "rockchip,trcm-sync-rx-only")) {
> > +		if (i2s_tdm->clk_trcm) {
> > +			dev_err(i2s_tdm->dev, "invalid trcm-sync 
configuration\n");
> > +			return -EINVAL;
> > +		}
> > +		i2s_tdm->clk_trcm = TRCM_RX;
> > +	}
> > +	if (i2s_tdm->clk_trcm != TRCM_TXRX)
> > +		i2s_tdm_dai.symmetric_rate = 1;
> > +
> > +	i2s_tdm->tdm_fsync_half_frame =
> > +		of_property_read_bool(node, "rockchip,tdm-fsync-half-frame");
> > +
> > +	i2s_tdm->grf = syscon_regmap_lookup_by_phandle(node, "rockchip,grf");
> > +	if (IS_ERR(i2s_tdm->grf))
> > +		return dev_err_probe(i2s_tdm->dev, PTR_ERR(i2s_tdm->grf),
> > +				     "Error in rockchip,grf\n");
> > +
> > +	if (i2s_tdm->clk_trcm != TRCM_TXRX) {
> > +		cru_node = of_parse_phandle(node, "rockchip,cru", 0);
> > +		i2s_tdm->cru_base = of_iomap(cru_node, 0);
> 
> This is a bit ugly if there is another driver sitting on the
> rockchip,cru compatible node. Which reset controller driver is backing
> the reset controls below?

I'm not sure if I understand the question (I only just today learned that
reset controls have drivers) but I think the reset it is using in the
Quartz64 dts is backed by rk3568-cru. Let me know if I misunderstood you.

[...]

> > +static int rockchip_i2s_tdm_remove(struct platform_device *pdev)
> > +{
> > +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(&pdev->dev);
> > +
> > +	pm_runtime_disable(&pdev->dev);
> > +	if (!pm_runtime_status_suspended(&pdev->dev))
> > +		i2s_tdm_runtime_suspend(&pdev->dev);
> > +
> > +	if (!IS_ERR(i2s_tdm->mclk_tx))
> > +		clk_prepare_enable(i2s_tdm->mclk_tx);
> > +	if (!IS_ERR(i2s_tdm->mclk_rx))
> > +		clk_prepare_enable(i2s_tdm->mclk_rx);
> 
> Why are we enabling these clocks now?
> 

I wondered that too while I was looking into the pm_runtime stuff,
and decided to just throw these two calls out. They don't seem to
make sense to me, and nothing I tested broke when I removed them.

If left in (and the code works as I hypothesise it works) then
this would simply re-enable clocks we have just disabled, which
would make the number of enable and disable calls unbalanced.
Highly sus, as the kids would say, and completely omits the other
mclk_tx_src etc. clocks. (Though those are forgotten elsewhere in
the code as well, which I have since fixed, along with any of the
review points I don't directly respond to.)

> > +	if (!IS_ERR(i2s_tdm->hclk))
> > +		clk_disable_unprepare(i2s_tdm->hclk);
> > +
> > +	return 0;
> > +}
> 
> regards
> Philipp

Regards,
Nicolas Frattaroli



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-21 20:45     ` Nicolas Frattaroli
  2021-08-23 11:19       ` Mark Brown
@ 2021-08-23 15:02       ` Pierre-Louis Bossart
  1 sibling, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-08-23 15:02 UTC (permalink / raw)
  To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Heiko Stuebner, Philipp Zabel
  Cc: linux-rockchip, alsa-devel, linux-kernel, linux-arm-kernel



On 8/21/21 3:45 PM, Nicolas Frattaroli wrote:
> On Freitag, 20. August 2021 21:02:16 CEST Pierre-Louis Bossart wrote:
>>> +	regmap_read(i2s_tdm->regmap, I2S_CLR, &val);
>>> +	/* Wait on the clear operation to finish */
>>> +	while (val) {
>>
>> delay needed here?
>>
> 
> The rockchip_i2s.c code doesn't have a delay here either, but I can
> add one of 1 usec for good measure, it seems weird to retry the
> read as fast as it can.

yep.

>>> +static int rockchip_i2s_tdm_clk_set_rate(struct rk_i2s_tdm_dev *i2s_tdm,
>>> +					 struct clk *clk, unsigned long rate,
>>> +					 int ppm)
>>> +{
>>> +	unsigned long rate_target;
>>> +	int delta, ret;
>>> +
>>> +	if (ppm == i2s_tdm->clk_ppm)
>>> +		return 0;
>>> +
>>> +	delta = (ppm < 0) ? -1 : 1;
>>> +	delta *= (int)div64_u64((u64)rate * (u64)abs(ppm) + 500000,
>>> +				1000000);
>>
>> formula looks odd? looks like you are implementing a round to nearest
>> operation, but that shouldn't require this multiplication?
>>
> 
> I believe the multiplication is there to compensate for clock drift.
> ppm is a value between -1000 and 1000 that specifies the clock drift
> in presumably parts per million, going by the variable name.

I meant using a signed division with lsb round-to-nearest, something like:

delta = (int)div64_u64((u64)rate * (u64)(ppm) + 500000,
			1000000);

> 
>>> +	pm_runtime_enable(&pdev->dev);
>>> +	if (!pm_runtime_enabled(&pdev->dev)) {
>>> +		ret = i2s_tdm_runtime_resume(&pdev->dev);
>>
>> that looks like dead code? you've just enabled pm_runtime, why would
>> this fail?
>>
> 
> I've had a look at the upstream rockchip_i2s.c code which does the
> same thing, and I believe the idea here is that we need to manually
> prepare and enable the master clocks (mclk_rx/mclk_tx) if pm_runtime
> is not available. Otherwise, pm_runtime will presumably call our
> resume callback at some point.
> 
> If runtime power management is disabled in the kernel config then 
> pm_runtime_enabled is always going to return false.

that seems very odd. why not enable the clocks by default and let them
stop in suspend.

>>> +err_suspend:
>>> +	if (!pm_runtime_status_suspended(&pdev->dev))
>>> +		i2s_tdm_runtime_suspend(&pdev->dev);
>>
>> why is this necessary?
> 
> I believe this is the same kind of situation as before, and the
> other driver does this too: if pm_runtime is not available, we
> need to stop our clocks manually on probe failure.

then use pm_runtime_disable() and manually stop the clocks...

>>> +err_pm_disable:
>>> +	pm_runtime_disable(&pdev->dev);>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int rockchip_i2s_tdm_remove(struct platform_device *pdev)
>>> +{
>>> +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(&pdev->dev);
>>> +
>>> +	pm_runtime_disable(&pdev->dev);
>>> +	if (!pm_runtime_status_suspended(&pdev->dev))
>>> +		i2s_tdm_runtime_suspend(&pdev->dev);
>>
>> this looks backwards, if you disable pm_runtime first what is the
>> expectation for the rest.
> 
> I'm not well versed in the PM code but if my theory of this being
> related to unavailable PM is correct, then my best guess is that
> pm_runtime_disable does suspend the device, so if it's not
> suspended then we don't have pm_runtime and therefore need to call
> it manually.

I think this is really doing things backwards. You want to
unconditionally enable all resources on probe, and let them go to idle
when no one needs them - or if pm_runtime is disabled.

>>> +
>>> +	if (!IS_ERR(i2s_tdm->mclk_tx))
>>> +		clk_prepare_enable(i2s_tdm->mclk_tx);
>>> +	if (!IS_ERR(i2s_tdm->mclk_rx))
>>> +		clk_prepare_enable(i2s_tdm->mclk_rx);
>>> +	if (!IS_ERR(i2s_tdm->hclk))
>>> +		clk_disable_unprepare(i2s_tdm->hclk);
>>> +
>>> +	return 0;>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>
>> use __maybe_unused
> 
> You mean instead of the ifdef stuff to just add this attribute to
> the following functions like this?
> 
> static int rockchip_i2s_tdm_suspend(struct device *dev) __maybe_unused

yes

> 
>>
>>> +static int rockchip_i2s_tdm_suspend(struct device *dev)
>>> +{
>>> +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
>>> +
>>> +	regcache_mark_dirty(i2s_tdm->regmap);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int rockchip_i2s_tdm_resume(struct device *dev)
>>> +{
>>> +	struct rk_i2s_tdm_dev *i2s_tdm = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = pm_runtime_get_sync(dev);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +	ret = regcache_sync(i2s_tdm->regmap);
>>> +	pm_runtime_put(dev);
>>> +
>>> +	return ret;
>>> +}
>>> +#endif
> 
> Thank you for your review!
> 
> Regards,
> Nicolas Frattaroli
> 
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller
  2021-08-23 14:35     ` Nicolas Frattaroli
@ 2021-08-23 16:03       ` Philipp Zabel
  0 siblings, 0 replies; 13+ messages in thread
From: Philipp Zabel @ 2021-08-23 16:03 UTC (permalink / raw)
  To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Heiko Stuebner
  Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-rockchip

Hi Nicolas,

On Mon, 2021-08-23 at 16:35 +0200, Nicolas Frattaroli wrote:
[...]
> > I'm not too fond of reimplementing half a reset controller in here.
> > The reset framework does not support synchronized (de)assertion of
> > multiple reset controls, I wonder if this would be useful to add.
> > Without having thought about this too hard, I could imagine this as an
> > extension to the bulk API, or possibly a call to join multiple reset
> > controls into a reset control array.
> 
> I agree, the code required for synchronised reset seems to be a good
> candidate for a generalised solution elsewhere.
> However, I'm not sure if I'm the right candidate to design this API
> as my first kernel contribution when the board I'm currently testing
> this with doesn't even utilise the synchronized reset.
> 
> If I come across an opportunity to test synchronised resets, I'll
> definitely look into refactoring this. I'd also be happy to hear of
> any other driver which is currently implementing its own synchronised
> reset.
[...] 
> > What is the reason for the different delays in
> > rockchip_snd_xfer_sync_reset() and rockchip_snd_reset()?
> > 
> 
> Simply put: I have no idea. This is what downstream does, and it
> appears to work for me. The git history of the downstream kernel
> also doesn't tell me anything about why the sync reset is 150
> and this one is 1.
> 
> I've got no device to test the sync reset with at the moment so
> I'm wary of playing with the delay value.

Fair points. You could remove the untested synchronized reset support
for now; that would allow to get rid of the rockchip,cru device tree
property, which is only required to let this driver access the CRU
registers behind the clock/reset controller driver's back.

[...]
> > > +	if (i2s_tdm->clk_trcm != TRCM_TXRX) {
> > > +		cru_node = of_parse_phandle(node, "rockchip,cru", 0);
> > > +		i2s_tdm->cru_base = of_iomap(cru_node, 0);
> > 
> > This is a bit ugly if there is another driver sitting on the
> > rockchip,cru compatible node. Which reset controller driver is backing
> > the reset controls below?
> 
> I'm not sure if I understand the question (I only just today learned that
> reset controls have drivers) but I think the reset it is using in the
> Quartz64 dts is backed by rk3568-cru. Let me know if I misunderstood you.

That's the one, thanks.

The devm_reset_control_get() calls below follow the "reset-names"
and "resets" device tree properties. Those point (&cru) to a
clock-controller node with compatible = "rockchip,rk3568-cru".

The corresponding driver is drivers/clk/rockchip/clk-rk3568.c,
so the reset controller is provided by the reset_controller_register()
call in drivers/clk/rockchip/softrst.c.

regards
Philipp

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-08-20 18:27 ` [PATCH v2 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
@ 2021-08-24 16:42   ` Rob Herring
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2021-08-24 16:42 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: Liam Girdwood, Mark Brown, Heiko Stuebner, alsa-devel,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

On Fri, Aug 20, 2021 at 08:27:29PM +0200, Nicolas Frattaroli wrote:
> This adds the YAML bindings for the Rockchip I2S/TDM audio driver.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
>  .../bindings/sound/rockchip,i2s-tdm.yaml      | 193 ++++++++++++++++++
>  1 file changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> new file mode 100644
> index 000000000000..1e8c1500ba40
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> @@ -0,0 +1,193 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/rockchip,i2s-tdm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip I2S/TDM Controller
> +
> +description:
> +  The Rockchip I2S/TDM Controller is a Time Division Multiplexed
> +  audio interface found in various Rockchip SoCs, allowing up
> +  to 8 channels of audio over a serial interface.
> +
> +maintainers:
> +  - Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,px30-i2s-tdm
> +      - rockchip,rk1808-i2s-tdm
> +      - rockchip,rk3308-i2s-tdm
> +      - rockchip,rk3568-i2s-tdm
> +      - rockchip,rv1126-i2s-tdm
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  dmas:
> +    minItems: 1
> +    maxItems: 2
> +
> +  dma-names:
> +    minItems: 1
> +    items:
> +      - const: rx
> +      - const: tx
> +
> +  clocks:
> +    items:
> +      - description: clock for TX
> +      - description: clock for RX
> +      - description: AHB clock driving the interface
> +
> +  clock-names:
> +    items:
> +      - const: mclk_tx
> +      - const: mclk_rx
> +      - const: hclk
> +
> +  rockchip,frame-width:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 64
> +    minimum: 32
> +    maximum: 512
> +    description:
> +      Width of a frame, usually slot width multiplied by number of slots.
> +      Must be even.
> +
> +  resets:
> +    items:
> +      - description: reset for TX
> +      - description: reset for RX
> +
> +  reset-names:
> +    items:
> +      - const: tx-m
> +      - const: rx-m
> +
> +  rockchip,cru:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      The phandle of the cru.
> +      Required if both playback and capture are used, i.e. if rockchip,clk-trcm
> +      is 0.

Another case of the nonexistent rockchip,clk-trcm.


> +
> +  rockchip,grf:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      The phandle of the syscon node for the GRF register.
> +
> +  rockchip,mclk-calibrate:
> +    description:
> +      Enable mclk source calibration.
> +    type: boolean
> +
> +  rockchip,trcm-sync-tx-only:
> +    type: boolean
> +    description: Use TX BCLK/LRCK for both TX and RX.
> +
> +  rockchip,trcm-sync-rx-only:
> +    type: boolean
> +    description: Use RX BCLK/LRCK for both TX and RX.
> +
> +  "#sound-dai-cells":
> +    const: 0
> +
> +  rockchip,i2s-rx-route:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Defines the mapping of I2S RX sdis to I2S data bus lines.
> +      By default, they are mapped one-to-one.
> +    items:
> +      - description: which sdi to connect to data line 0
> +      - description: which sdi to connect to data line 1
> +      - description: which sdi to connect to data line 2
> +      - description: which sdi to connect to data line 3
> +
> +  rockchip,i2s-tx-route:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Defines the mapping of I2S TX sdos to I2S data bus lines.
> +      By default, they are mapped one-to-one.
> +    items:
> +      - description: which sdo to connect to data line 0
> +      - description: which sdo to connect to data line 1
> +      - description: which sdo to connect to data line 2
> +      - description: which sdo to connect to data line 3
> +
> +  rockchip,tdm-fsync-half-frame:
> +    description: Whether to use half frame fsync.
> +    type: boolean
> +
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - dmas
> +  - dma-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - rockchip,grf
> +  - "#sound-dai-cells"
> +
> +allOf:
> +  - if:
> +      properties:
> +        rockchip,trcm-sync-tx-only: false
> +        rockchip,trcm-sync-rx-only: false
> +    then:
> +      required:
> +        - rockchip,cru
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rk3568-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/pinctrl/rockchip.h>
> +
> +    foo {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        i2s@fe410000 {
> +            compatible = "rockchip,rk3568-i2s-tdm";
> +            reg = <0x0 0xfe410000 0x0 0x1000>;
> +            interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&cru MCLK_I2S1_8CH_TX>, <&cru MCLK_I2S1_8CH_RX>,
> +                     <&cru HCLK_I2S1_8CH>;
> +            clock-names = "mclk_tx", "mclk_rx", "hclk";
> +            dmas = <&dmac1 3>, <&dmac1 2>;
> +            dma-names = "rx", "tx";
> +            resets = <&cru SRST_M_I2S1_8CH_TX>, <&cru SRST_M_I2S1_8CH_RX>;
> +            reset-names = "tx-m", "rx-m";
> +            rockchip,trcm-sync-tx-only;
> +            rockchip,cru = <&cru>;
> +            rockchip,grf = <&grf>;
> +            #sound-dai-cells = <0>;
> +            pinctrl-names = "default";
> +            pinctrl-0 =
> +                <&i2s1m0_sclktx
> +                &i2s1m0_sclkrx
> +                &i2s1m0_lrcktx
> +                &i2s1m0_lrckrx
> +                &i2s1m0_sdi0
> +                &i2s1m0_sdi1
> +                &i2s1m0_sdi2
> +                &i2s1m0_sdi3
> +                &i2s1m0_sdo0
> +                &i2s1m0_sdo1
> +                &i2s1m0_sdo2
> +                &i2s1m0_sdo3>;
> +            status = "okay";

Sorry if I wasn't clear, but you don't need status at all in examples as 
okay is the default.

Rob

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-08-24 16:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 18:27 [PATCH v2 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
2021-08-20 18:27 ` [PATCH v2 1/4] ASoC: rockchip: add support for i2s-tdm controller Nicolas Frattaroli
2021-08-20 19:02   ` Pierre-Louis Bossart
2021-08-21 20:45     ` Nicolas Frattaroli
2021-08-23 11:19       ` Mark Brown
2021-08-23 15:02       ` Pierre-Louis Bossart
2021-08-23 12:03   ` Philipp Zabel
2021-08-23 14:35     ` Nicolas Frattaroli
2021-08-23 16:03       ` Philipp Zabel
2021-08-20 18:27 ` [PATCH v2 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
2021-08-24 16:42   ` Rob Herring
2021-08-20 18:27 ` [PATCH v2 3/4] arm64: dts: rockchip: add i2s1 on rk356x Nicolas Frattaroli
2021-08-20 18:27 ` [PATCH v2 4/4] arm64: dts: rockchip: add analog audio on Quartz64 Nicolas Frattaroli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).