All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/7] Add HDMI audio support for HiKey
@ 2016-07-16  2:13 John Stultz
  2016-07-16  2:13 ` [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping John Stultz
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu

This patch set is required for HDMI audio support on HiKey.

This patchset hasn't yet seen the light of lkml, so I suspect
there will be a few revisions, but I wanted to send it out for
an initial review.

The work is mostly that of Andy Green's, but I've taking a swing
at forward porting and cleaning it up where I saw fit. So credit
to Andy and blame to me. Apologies in advance, as I'm not super
familiar with either DMA or ASoC driver.

The one bit missing to have audio fully working is changes to the
adv7511 driver, but most of those changes are still out of tree, so
I'll submit those changes once they land.

Feedback would be very much appreicated!

thanks
-john

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>

Andy Green (5):
  k3dma: Fix hisi burst clipping
  k3dma: Fix dma err offsets
  k3dma: Fix "nobody cared" message seen on any error
  k3dma: Add cyclic mode for audio
  ASoC: hisilicon:  Add hi6210 i2s audio driver for hdmi audio

John Stultz (2):
  Kconfig: Allow k3dma driver to be selected for more then HISI3xx
    platforms
  dts: hi6220: Add k3-dma and i2s/hdmi audio support

 arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  36 ++
 drivers/dma/Kconfig                       |   1 -
 drivers/dma/k3dma.c                       | 149 ++++++-
 sound/soc/Kconfig                         |   1 +
 sound/soc/Makefile                        |   1 +
 sound/soc/hisilicon/Kconfig               |   5 +
 sound/soc/hisilicon/Makefile              |   2 +
 sound/soc/hisilicon/hi6210-hdmi-card.c    | 131 ++++++
 sound/soc/hisilicon/hi6210-i2s.c          | 641 ++++++++++++++++++++++++++++++
 sound/soc/hisilicon/hi6210-i2s.h          | 276 +++++++++++++
 10 files changed, 1222 insertions(+), 21 deletions(-)
 create mode 100644 sound/soc/hisilicon/Kconfig
 create mode 100644 sound/soc/hisilicon/Makefile
 create mode 100644 sound/soc/hisilicon/hi6210-hdmi-card.c
 create mode 100644 sound/soc/hisilicon/hi6210-i2s.c
 create mode 100644 sound/soc/hisilicon/hi6210-i2s.h

-- 
1.9.1

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

* [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
@ 2016-07-16  2:13 ` John Stultz
  2016-07-18  6:37   ` zhangfei
  2016-07-16  2:13 ` [RFC][PATCH 2/7] k3dma: Fix dma err offsets John Stultz
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu, John Stultz

From: Andy Green <andy.green@linaro.org>

Max burst len is a 4-bit field, but at the moment it's clipped with
a 5-bit constant... reduce it to that which can be expressed

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
[jstultz: Forward ported to mainline]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma/k3dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index 1ba2fd7..d01a11d 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -552,7 +552,7 @@ static int k3_dma_config(struct dma_chan *chan,
 	c->ccfg |= (val << 12) | (val << 16);
 
 	if ((maxburst == 0) || (maxburst > 16))
-		val = 16;
+		val = 15;
 	else
 		val = maxburst - 1;
 	c->ccfg |= (val << 20) | (val << 24);
-- 
1.9.1

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

* [RFC][PATCH 2/7] k3dma: Fix dma err offsets
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
  2016-07-16  2:13 ` [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping John Stultz
@ 2016-07-16  2:13 ` John Stultz
  2016-07-18  6:39   ` zhangfei
  2016-07-16  2:13 ` [RFC][PATCH 3/7] k3dma: Fix "nobody cared" message seen on any error John Stultz
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu, John Stultz

From: Andy Green <andy.green@linaro.org>

The offsets for ERR1 and ERR2 are wrong actually.
That's why you can never clear an error.

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
[jstultz: Forward ported to mainline]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma/k3dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index d01a11d..8dd050c 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -34,8 +34,8 @@
 #define INT_ERR1_MASK		0x20
 #define INT_ERR2_MASK		0x24
 #define INT_TC1_RAW		0x600
-#define INT_ERR1_RAW		0x608
-#define INT_ERR2_RAW		0x610
+#define INT_ERR1_RAW		0x610
+#define INT_ERR2_RAW		0x618
 #define CH_PRI			0x688
 #define CH_STAT			0x690
 #define CX_CUR_CNT		0x704
-- 
1.9.1

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

* [RFC][PATCH 3/7] k3dma: Fix "nobody cared" message seen on any error
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
  2016-07-16  2:13 ` [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping John Stultz
  2016-07-16  2:13 ` [RFC][PATCH 2/7] k3dma: Fix dma err offsets John Stultz
@ 2016-07-16  2:13 ` John Stultz
  2016-07-18  6:40   ` zhangfei
  2016-07-16  2:13 ` [RFC][PATCH 4/7] k3dma: Add cyclic mode for audio John Stultz
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu, John Stultz

From: Andy Green <andy.green@linaro.org>

As it was before, as soon as the DMAC IP felt there was an error
he would return IRQ_NONE since no actual transfer had completed.

After spinning on that for 100K interrupts, Linux yanks the IRQ with
a "nobody cared" error.

This patch lets it handle the interrupt and keep the IRQ alive.

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
[jstultz: Forward ported to mainline]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma/k3dma.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index 8dd050c..c2906a82 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -220,11 +220,13 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id)
 	writel_relaxed(err1, d->base + INT_ERR1_RAW);
 	writel_relaxed(err2, d->base + INT_ERR2_RAW);
 
-	if (irq_chan) {
+	if (irq_chan)
 		tasklet_schedule(&d->task);
+
+	if (irq_chan || err1 || err2)
 		return IRQ_HANDLED;
-	} else
-		return IRQ_NONE;
+
+	return IRQ_NONE;
 }
 
 static int k3_dma_start_txd(struct k3_dma_chan *c)
-- 
1.9.1

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

* [RFC][PATCH 4/7] k3dma: Add cyclic mode for audio
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
                   ` (2 preceding siblings ...)
  2016-07-16  2:13 ` [RFC][PATCH 3/7] k3dma: Fix "nobody cared" message seen on any error John Stultz
@ 2016-07-16  2:13 ` John Stultz
  2016-07-18  6:43   ` zhangfei
  2016-07-16  2:13 ` [RFC][PATCH 5/7] Kconfig: Allow k3dma driver to be selected for more then HISI3xx platforms John Stultz
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu, John Stultz

From: Andy Green <andy.green@linaro.org>

Currently the k3dma driver doesn't offer the cyclic mode
necessary for handling audio.

This patch adds it.

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
[jstultz: Forward ported to mainline, removed a few
 bits of logic that didn't seem to have much effect]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma/k3dma.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 121 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index c2906a82..eff7483 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013 Linaro Ltd.
+ * Copyright (c) 2013 - 2015 Linaro Ltd.
  * Copyright (c) 2013 Hisilicon Limited.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -25,22 +25,27 @@
 
 #define DRIVER_NAME		"k3-dma"
 #define DMA_MAX_SIZE		0x1ffc
+#define DMA_CYCLIC_MAX_PERIOD	0x1000
 
 #define INT_STAT		0x00
 #define INT_TC1			0x04
+#define INT_TC2			0x08
 #define INT_ERR1		0x0c
 #define INT_ERR2		0x10
 #define INT_TC1_MASK		0x18
+#define INT_TC2_MASK		0x1c
 #define INT_ERR1_MASK		0x20
 #define INT_ERR2_MASK		0x24
 #define INT_TC1_RAW		0x600
+#define INT_TC2_RAW		0x608
 #define INT_ERR1_RAW		0x610
 #define INT_ERR2_RAW		0x618
 #define CH_PRI			0x688
 #define CH_STAT			0x690
 #define CX_CUR_CNT		0x704
 #define CX_LLI			0x800
-#define CX_CNT			0x810
+#define CX_CNT1			0x80c
+#define CX_CNT0			0x810
 #define CX_SRC			0x814
 #define CX_DST			0x818
 #define CX_CFG			0x81c
@@ -49,6 +54,7 @@
 
 #define CX_LLI_CHAIN_EN		0x2
 #define CX_CFG_EN		0x1
+#define CX_CFG_NODEIRQ		BIT(1)
 #define CX_CFG_MEM2PER		(0x1 << 2)
 #define CX_CFG_PER2MEM		(0x2 << 2)
 #define CX_CFG_SRCINCR		(0x1 << 31)
@@ -81,6 +87,7 @@ struct k3_dma_chan {
 	enum dma_transfer_direction dir;
 	dma_addr_t		dev_addr;
 	enum dma_status		status;
+	bool			cyclic;
 };
 
 struct k3_dma_phy {
@@ -134,6 +141,7 @@ static void k3_dma_terminate_chan(struct k3_dma_phy *phy, struct k3_dma_dev *d)
 
 	val = 0x1 << phy->idx;
 	writel_relaxed(val, d->base + INT_TC1_RAW);
+	writel_relaxed(val, d->base + INT_TC2_RAW);
 	writel_relaxed(val, d->base + INT_ERR1_RAW);
 	writel_relaxed(val, d->base + INT_ERR2_RAW);
 }
@@ -141,11 +149,15 @@ static void k3_dma_terminate_chan(struct k3_dma_phy *phy, struct k3_dma_dev *d)
 static void k3_dma_set_desc(struct k3_dma_phy *phy, struct k3_desc_hw *hw)
 {
 	writel_relaxed(hw->lli, phy->base + CX_LLI);
-	writel_relaxed(hw->count, phy->base + CX_CNT);
+	writel_relaxed(hw->count, phy->base + CX_CNT0);
 	writel_relaxed(hw->saddr, phy->base + CX_SRC);
 	writel_relaxed(hw->daddr, phy->base + CX_DST);
 	writel_relaxed(AXI_CFG_DEFAULT, phy->base + AXI_CFG);
 	writel_relaxed(hw->config, phy->base + CX_CFG);
+	pr_debug("%s: desc %p: ch idx = %d, lli: 0x%x, count: 0x%x,"
+		" saddr: 0x%x, daddr 0x%x, cfg: 0x%x\n", __func__,
+		(void *) hw, phy->idx, hw->lli,	hw->count, hw->saddr,
+		hw->daddr, hw->config);
 }
 
 static u32 k3_dma_get_curr_cnt(struct k3_dma_dev *d, struct k3_dma_phy *phy)
@@ -175,11 +187,13 @@ static void k3_dma_enable_dma(struct k3_dma_dev *d, bool on)
 
 		/* unmask irq */
 		writel_relaxed(0xffff, d->base + INT_TC1_MASK);
+		writel_relaxed(0xffff, d->base + INT_TC2_MASK);
 		writel_relaxed(0xffff, d->base + INT_ERR1_MASK);
 		writel_relaxed(0xffff, d->base + INT_ERR2_MASK);
 	} else {
 		/* mask irq */
 		writel_relaxed(0x0, d->base + INT_TC1_MASK);
+		writel_relaxed(0x0, d->base + INT_TC2_MASK);
 		writel_relaxed(0x0, d->base + INT_ERR1_MASK);
 		writel_relaxed(0x0, d->base + INT_ERR2_MASK);
 	}
@@ -192,24 +206,31 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id)
 	struct k3_dma_chan *c;
 	u32 stat = readl_relaxed(d->base + INT_STAT);
 	u32 tc1  = readl_relaxed(d->base + INT_TC1);
+	u32 tc2  = readl_relaxed(d->base + INT_TC2);
 	u32 err1 = readl_relaxed(d->base + INT_ERR1);
 	u32 err2 = readl_relaxed(d->base + INT_ERR2);
 	u32 i, irq_chan = 0;
 
 	while (stat) {
 		i = __ffs(stat);
-		stat &= (stat - 1);
-		if (likely(tc1 & BIT(i))) {
+		stat &= ~BIT(i);
+		if (likely(tc1 & BIT(i)) || (tc2 & BIT(i))) {
+			unsigned long flags;
+
 			p = &d->phy[i];
 			c = p->vchan;
-			if (c) {
-				unsigned long flags;
-
+			if (c && (tc1 & BIT(i))) {
 				spin_lock_irqsave(&c->vc.lock, flags);
 				vchan_cookie_complete(&p->ds_run->vd);
 				p->ds_done = p->ds_run;
 				spin_unlock_irqrestore(&c->vc.lock, flags);
 			}
+			if (c && (tc2 & BIT(i))) {
+				spin_lock_irqsave(&c->vc.lock, flags);
+				if (p->ds_run != NULL)
+					vchan_cyclic_callback(&p->ds_run->vd);
+				spin_unlock_irqrestore(&c->vc.lock, flags);
+			}
 			irq_chan |= BIT(i);
 		}
 		if (unlikely((err1 & BIT(i)) || (err2 & BIT(i))))
@@ -217,6 +238,7 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id)
 	}
 
 	writel_relaxed(irq_chan, d->base + INT_TC1_RAW);
+	writel_relaxed(irq_chan, d->base + INT_TC2_RAW);
 	writel_relaxed(err1, d->base + INT_ERR1_RAW);
 	writel_relaxed(err2, d->base + INT_ERR2_RAW);
 
@@ -352,7 +374,7 @@ static enum dma_status k3_dma_tx_status(struct dma_chan *chan,
 	 * its total size.
 	 */
 	vd = vchan_find_desc(&c->vc, cookie);
-	if (vd) {
+	if (vd && !c->cyclic) {
 		bytes = container_of(vd, struct k3_dma_desc_sw, vd)->size;
 	} else if ((!p) || (!p->ds_run)) {
 		bytes = 0;
@@ -362,7 +384,8 @@ static enum dma_status k3_dma_tx_status(struct dma_chan *chan,
 
 		bytes = k3_dma_get_curr_cnt(d, p);
 		clli = k3_dma_get_curr_lli(p);
-		index = (clli - ds->desc_hw_lli) / sizeof(struct k3_desc_hw);
+		index = ((clli - ds->desc_hw_lli) /
+				sizeof(struct k3_desc_hw)) + 1;
 		for (; index < ds->desc_num; index++) {
 			bytes += ds->desc_hw[index].count;
 			/* end of lli */
@@ -403,14 +426,22 @@ static void k3_dma_issue_pending(struct dma_chan *chan)
 static void k3_dma_fill_desc(struct k3_dma_desc_sw *ds, dma_addr_t dst,
 			dma_addr_t src, size_t len, u32 num, u32 ccfg)
 {
-	if ((num + 1) < ds->desc_num)
+	if (num != ds->desc_num - 1)
 		ds->desc_hw[num].lli = ds->desc_hw_lli + (num + 1) *
 			sizeof(struct k3_desc_hw);
+
 	ds->desc_hw[num].lli |= CX_LLI_CHAIN_EN;
 	ds->desc_hw[num].count = len;
 	ds->desc_hw[num].saddr = src;
 	ds->desc_hw[num].daddr = dst;
 	ds->desc_hw[num].config = ccfg;
+
+	pr_debug("%s: k3_dma_desc_sw = %p, desc_hw = %p (num = %d) lli: 0x%x,"
+		" count: 0x%x, saddr: 0x%x, daddr 0x%x, cfg: 0x%x\n", __func__,
+		(void *)ds, &ds->desc_hw[num], num,
+		ds->desc_hw[num].lli, ds->desc_hw[num].count,
+		ds->desc_hw[num].saddr, ds->desc_hw[num].daddr,
+		ds->desc_hw[num].config);
 }
 
 static struct dma_async_tx_descriptor *k3_dma_prep_memcpy(
@@ -431,6 +462,7 @@ static struct dma_async_tx_descriptor *k3_dma_prep_memcpy(
 		dev_dbg(chan->device->dev, "vchan %p: kzalloc fail\n", &c->vc);
 		return NULL;
 	}
+	c->cyclic = 0;
 	ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]);
 	ds->size = len;
 	ds->desc_num = num;
@@ -476,17 +508,19 @@ static struct dma_async_tx_descriptor *k3_dma_prep_slave_sg(
 	if (sgl == NULL)
 		return NULL;
 
+	c->cyclic = 0;
+
 	for_each_sg(sgl, sg, sglen, i) {
 		avail = sg_dma_len(sg);
+		pr_err("   avail=0x%x\n", (int)avail);
 		if (avail > DMA_MAX_SIZE)
 			num += DIV_ROUND_UP(avail, DMA_MAX_SIZE) - 1;
 	}
 
 	ds = kzalloc(sizeof(*ds) + num * sizeof(ds->desc_hw[0]), GFP_ATOMIC);
-	if (!ds) {
-		dev_dbg(chan->device->dev, "vchan %p: kzalloc fail\n", &c->vc);
+	if (!ds)
 		return NULL;
-	}
+
 	ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]);
 	ds->desc_num = num;
 	num = 0;
@@ -519,6 +553,77 @@ static struct dma_async_tx_descriptor *k3_dma_prep_slave_sg(
 	return vchan_tx_prep(&c->vc, &ds->vd, flags);
 }
 
+static struct dma_async_tx_descriptor *
+k3_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
+		       size_t buf_len, size_t period_len,
+		       enum dma_transfer_direction dir,
+		       unsigned long flags)
+{
+	struct k3_dma_chan *c = to_k3_chan(chan);
+	struct k3_dma_desc_sw *ds;
+	size_t len, avail, total = 0;
+	dma_addr_t addr, src = 0, dst = 0;
+	int num = 1, since = 0;
+	size_t modulo = DMA_CYCLIC_MAX_PERIOD;
+	u32 en_tc2 = 0;
+
+	pr_debug("%s: buf %p, dst %p, buf len %d, period_len = %d, dir %d\n",
+	       __func__, (void *)buf_addr, (void *)to_k3_chan(chan)->dev_addr,
+	       (int)buf_len, (int)period_len, (int)dir);
+
+	avail = buf_len;
+	if (avail > modulo)
+		num += DIV_ROUND_UP(avail, modulo) - 1;
+
+	ds = kzalloc(sizeof(*ds) + num * sizeof(ds->desc_hw[0]), GFP_ATOMIC);
+	if (!ds)
+		return NULL;
+
+	ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]);
+	ds->desc_num = num;
+
+	c->cyclic = 1;
+	addr = buf_addr;
+	avail = buf_len;
+	total = avail;
+	num = 0;
+
+	if (period_len < modulo)
+		modulo = period_len;
+
+	do {
+		len = min_t(size_t, avail, modulo);
+
+		if (dir == DMA_MEM_TO_DEV) {
+			src = addr;
+			dst = c->dev_addr;
+		} else if (dir == DMA_DEV_TO_MEM) {
+			src = c->dev_addr;
+			dst = addr;
+		}
+		since += len;
+		if (since >= period_len) {
+			/* descriptor asks for TC2 interrupt on completion */
+			en_tc2 = CX_CFG_NODEIRQ;
+			since -= period_len;
+		} else
+			en_tc2 = 0;
+
+		k3_dma_fill_desc(ds, dst, src, len, num++, c->ccfg | en_tc2);
+
+		addr += len;
+		avail -= len;
+	} while (avail);
+
+	/* "Cyclic" == end of link points back to start of link */
+	ds->desc_hw[num - 1].lli |= ds->desc_hw_lli;
+
+	ds->size = total;
+
+	return vchan_tx_prep(&c->vc, &ds->vd, flags);
+}
+
+
 static int k3_dma_config(struct dma_chan *chan,
 			 struct dma_slave_config *cfg)
 {
@@ -723,11 +828,13 @@ static int k3_dma_probe(struct platform_device *op)
 	INIT_LIST_HEAD(&d->slave.channels);
 	dma_cap_set(DMA_SLAVE, d->slave.cap_mask);
 	dma_cap_set(DMA_MEMCPY, d->slave.cap_mask);
+	dma_cap_set(DMA_CYCLIC, d->slave.cap_mask);
 	d->slave.dev = &op->dev;
 	d->slave.device_free_chan_resources = k3_dma_free_chan_resources;
 	d->slave.device_tx_status = k3_dma_tx_status;
 	d->slave.device_prep_dma_memcpy = k3_dma_prep_memcpy;
 	d->slave.device_prep_slave_sg = k3_dma_prep_slave_sg;
+	d->slave.device_prep_dma_cyclic = k3_dma_prep_dma_cyclic;
 	d->slave.device_issue_pending = k3_dma_issue_pending;
 	d->slave.device_config = k3_dma_config;
 	d->slave.device_pause = k3_dma_transfer_pause;
-- 
1.9.1

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

* [RFC][PATCH 5/7] Kconfig: Allow k3dma driver to be selected for more then HISI3xx platforms
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
                   ` (3 preceding siblings ...)
  2016-07-16  2:13 ` [RFC][PATCH 4/7] k3dma: Add cyclic mode for audio John Stultz
@ 2016-07-16  2:13 ` John Stultz
  2016-07-16 11:18   ` Mark Brown
  2016-07-16  2:13 ` [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio John Stultz
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu

This allows the k3dma driver to be selected on HiKey

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 8c98779..c5a230d 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -279,7 +279,6 @@ config INTEL_MIC_X100_DMA
 
 config K3_DMA
 	tristate "Hisilicon K3 DMA support"
-	depends on ARCH_HI3xxx
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
 	help
-- 
1.9.1

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

* [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
                   ` (4 preceding siblings ...)
  2016-07-16  2:13 ` [RFC][PATCH 5/7] Kconfig: Allow k3dma driver to be selected for more then HISI3xx platforms John Stultz
@ 2016-07-16  2:13 ` John Stultz
  2016-07-16 11:44   ` Mark Brown
  2016-07-16  2:13 ` [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support John Stultz
  2016-07-16  3:15 ` [RFC][PATCH 0/7] Add HDMI audio support for HiKey Andy Green
  7 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu, John Stultz

From: Andy Green <andy.green@linaro.org>

Add driver for hi6210 i2s controller found on hi6220 boards.

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
[jstultz: Forward ported to mainline, limit rates to 48k]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 sound/soc/Kconfig                      |   1 +
 sound/soc/Makefile                     |   1 +
 sound/soc/hisilicon/Kconfig            |   5 +
 sound/soc/hisilicon/Makefile           |   2 +
 sound/soc/hisilicon/hi6210-hdmi-card.c | 131 +++++++
 sound/soc/hisilicon/hi6210-i2s.c       | 641 +++++++++++++++++++++++++++++++++
 sound/soc/hisilicon/hi6210-i2s.h       | 276 ++++++++++++++
 7 files changed, 1057 insertions(+)
 create mode 100644 sound/soc/hisilicon/Kconfig
 create mode 100644 sound/soc/hisilicon/Makefile
 create mode 100644 sound/soc/hisilicon/hi6210-hdmi-card.c
 create mode 100644 sound/soc/hisilicon/hi6210-i2s.c
 create mode 100644 sound/soc/hisilicon/hi6210-i2s.h

diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 182d92e..9df9658 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -47,6 +47,7 @@ source "sound/soc/cirrus/Kconfig"
 source "sound/soc/davinci/Kconfig"
 source "sound/soc/dwc/Kconfig"
 source "sound/soc/fsl/Kconfig"
+source "sound/soc/hisilicon/Kconfig"
 source "sound/soc/jz4740/Kconfig"
 source "sound/soc/nuc900/Kconfig"
 source "sound/soc/omap/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 9a30f21..2f6aabb 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_SND_SOC)	+= cirrus/
 obj-$(CONFIG_SND_SOC)	+= davinci/
 obj-$(CONFIG_SND_SOC)	+= dwc/
 obj-$(CONFIG_SND_SOC)	+= fsl/
+obj-$(CONFIG_SND_SOC)	+= hisilicon/
 obj-$(CONFIG_SND_SOC)	+= jz4740/
 obj-$(CONFIG_SND_SOC)	+= img/
 obj-$(CONFIG_SND_SOC)	+= intel/
diff --git a/sound/soc/hisilicon/Kconfig b/sound/soc/hisilicon/Kconfig
new file mode 100644
index 0000000..4356d5a
--- /dev/null
+++ b/sound/soc/hisilicon/Kconfig
@@ -0,0 +1,5 @@
+config SND_I2S_HI6210_I2S
+	tristate "Hisilicon I2S controller"
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	help
+	  Hisilicon I2S
diff --git a/sound/soc/hisilicon/Makefile b/sound/soc/hisilicon/Makefile
new file mode 100644
index 0000000..43a5504
--- /dev/null
+++ b/sound/soc/hisilicon/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_SND_I2S_HI6210_I2S) += hi6210-i2s.o \
+				    hi6210-hdmi-card.o
\ No newline at end of file
diff --git a/sound/soc/hisilicon/hi6210-hdmi-card.c b/sound/soc/hisilicon/hi6210-hdmi-card.c
new file mode 100644
index 0000000..f0995a7
--- /dev/null
+++ b/sound/soc/hisilicon/hi6210-hdmi-card.c
@@ -0,0 +1,131 @@
+/*
+ * linux/sound/soc/hisilicon/hi6210-hdmi-card.c
+ *
+ * Copyright (C) 2015 Linaro, Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/module.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+
+static int hdmi_hw_params(struct snd_pcm_substream *substream,
+			  struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int ret;
+
+	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS);
+	if (ret)
+		return ret;
+
+	/* set i2s system clock */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, 24576000, SND_SOC_CLOCK_IN);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* operations of sound device */
+static struct snd_soc_ops hdmi_ops = {
+	.hw_params = hdmi_hw_params,
+};
+
+static struct snd_soc_dai_link hi6210_hdmi_dai_link = {
+	.name = "hi6210-hdmi-dai-link",  /* "codec name" */
+	.stream_name = "hdmi", /* stream name */
+
+	.cpu_dai_name = "f7118000.hi6210_i2s",
+	.codec_name = "0.hi6210_hdmi_card",
+	.id = 0,
+	.ops = &hdmi_ops,
+	.codec_dai_name = "hi6210_hdmi_dai",
+	.platform_name = "f7118000.hi6210_i2s",
+};
+
+static struct snd_soc_card snd_soc_hi6210_hdmi = {
+	.name = "hi6210-hdmi",
+	.owner = THIS_MODULE,
+	.dai_link = &hi6210_hdmi_dai_link,
+	.num_links = 1,
+};
+
+static const struct snd_soc_dai_ops hi6210_hdmi_dai_ops = {
+};
+
+static struct snd_soc_dai_driver hi6210_hdmi_dai = {
+	.name = "hi6210_hdmi_dai",
+	.playback = {
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+	.ops = &hi6210_hdmi_dai_ops,
+};
+
+static struct snd_soc_codec_driver hi6210_hdmi_codec;
+
+static int hi6210_hdmi_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &snd_soc_hi6210_hdmi;
+	int ret;
+
+	ret = snd_soc_register_codec(&pdev->dev, &hi6210_hdmi_codec,
+			&hi6210_hdmi_dai, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_codec failed (%d)\n",
+					ret);
+		return ret;
+	}
+	card->dev = &pdev->dev;
+
+	ret = snd_soc_register_card(card);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		snd_soc_unregister_codec(&pdev->dev);
+		card->dev = NULL;
+		return ret;
+	}
+	return 0;
+}
+
+static int hi6210_hdmi_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+
+	snd_soc_unregister_card(card);
+	snd_soc_unregister_codec(&pdev->dev);
+	card->dev = NULL;
+	return 0;
+}
+
+static const struct of_device_id hi6210_hdmi_dt_ids[] = {
+	{ .compatible = "hisilicon,hi6210-hdmi-audio-card" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, hi6210_hdmi_dt_ids);
+
+static struct platform_driver hi6210_hdmi_driver = {
+	.driver = {
+		.name = "hi6210-hdmi-audio",
+		.owner = THIS_MODULE,
+		.of_match_table = hi6210_hdmi_dt_ids,
+	},
+	.probe = hi6210_hdmi_probe,
+	.remove = hi6210_hdmi_remove,
+};
+
+module_platform_driver(hi6210_hdmi_driver);
+
+MODULE_AUTHOR("andy.green@linaro.org");
+MODULE_DESCRIPTION("Hisilicon HDMI machine ASoC driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:hi6210-hdmi-audio");
diff --git a/sound/soc/hisilicon/hi6210-i2s.c b/sound/soc/hisilicon/hi6210-i2s.c
new file mode 100644
index 0000000..5b7e35d
--- /dev/null
+++ b/sound/soc/hisilicon/hi6210-i2s.c
@@ -0,0 +1,641 @@
+/*
+ * linux/sound/soc/m8m/hi6210_i2s.c - I2S IP driver
+ *
+ * Copyright (C) 2015 Linaro, Ltd
+ * Author: Andy Green <andy.green@linaro.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver only deals with S2 interface (BT)
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/clk.h>
+#include <linux/jiffies.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+#include <linux/interrupt.h>
+#include <linux/reset.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/reset-controller.h>
+#include <linux/clk.h>
+
+#include "hi6210-i2s.h"
+
+struct hi6210_i2s {
+	struct device *dev;
+	struct reset_control *rc;
+	struct clk *clk[8];
+	int clocks;
+	struct snd_soc_dai_driver dai;
+	void __iomem *base;
+	void __iomem *base_syscon;
+	void __iomem *base_pmctrl;
+	phys_addr_t base_phys;
+	struct snd_dmaengine_dai_dma_data dma_data[2];
+	int clk_rate;
+	spinlock_t lock;
+	int rate;
+	int format;
+	u8 bits;
+	u8 channels;
+	u8 id;
+	u8 channel_length;
+	u8 use;
+	u32 master:1;
+	u32 status:1;
+};
+
+#define SC_PERIPH_CLKEN1	0x210
+#define SC_PERIPH_CLKDIS1	0x214
+
+#define SC_PERIPH_CLKEN3	0x230
+#define SC_PERIPH_CLKDIS3	0x234
+
+#define SC_PERIPH_CLKEN12	0x270
+#define SC_PERIPH_CLKDIS12	0x274
+
+#define SC_PERIPH_RSTEN1	0x310
+#define SC_PERIPH_RSTDIS1	0x314
+#define SC_PERIPH_RSTSTAT1	0x318
+
+#define SC_PERIPH_RSTEN2	0x320
+#define SC_PERIPH_RSTDIS2	0x324
+#define SC_PERIPH_RSTSTAT2	0x328
+
+#define SOC_PMCTRL_BBPPLLALIAS	0x48
+
+static void hi6210_bits(struct hi6210_i2s *i2s, u32 ofs, u32 reset, u32 set)
+{
+	u32 val = readl(i2s->base + ofs) & ~reset;
+
+	writel(val | set, i2s->base + ofs);
+}
+
+static int _hi6210_i2s_set_fmt(struct hi6210_i2s *i2s,
+			       struct snd_pcm_substream *substream)
+{
+	u32 u;
+
+	hi6210_bits(i2s, HII2S_ST_DL_FIFO_TH_CFG,
+		    (HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AEMPTY_MASK <<
+		     HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AEMPTY_SHIFT) |
+		    (HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AFULL_MASK <<
+		     HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AFULL_SHIFT) |
+		    (HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AEMPTY_MASK <<
+		     HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AEMPTY_SHIFT) |
+		    (HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AFULL_MASK <<
+		     HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AFULL_SHIFT),
+		    (16 << HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AEMPTY_SHIFT) |
+		    (30 << HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AFULL_SHIFT) |
+		    (16 << HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AEMPTY_SHIFT) |
+		    (30 << HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AFULL_SHIFT));
+
+	hi6210_bits(i2s, HII2S_IF_CLK_EN_CFG, 0,
+		    BIT(19) | BIT(18) | BIT(17) |
+		    HII2S_IF_CLK_EN_CFG__S2_IF_CLK_EN |
+		    HII2S_IF_CLK_EN_CFG__S2_OL_MIXER_EN |
+		    HII2S_IF_CLK_EN_CFG__S2_OL_SRC_EN |
+		    HII2S_IF_CLK_EN_CFG__ST_DL_R_EN |
+		    HII2S_IF_CLK_EN_CFG__ST_DL_L_EN);
+
+	hi6210_bits(i2s, HII2S_DIG_FILTER_CLK_EN_CFG,
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACR_SDM_EN |
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACR_HBF2I_EN |
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACR_AGC_EN |
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACL_SDM_EN |
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACL_HBF2I_EN |
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACL_AGC_EN,
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACR_MIXER_EN |
+		    HII2S_DIG_FILTER_CLK_EN_CFG__DACL_MIXER_EN
+	);
+
+	hi6210_bits(i2s, HII2S_DIG_FILTER_MODULE_CFG,
+		    HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_IN2_MUTE |
+		    HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_IN2_MUTE,
+		    0
+	);
+	hi6210_bits(i2s, HII2S_MUX_TOP_MODULE_CFG,
+		    HII2S_MUX_TOP_MODULE_CFG__S2_OL_MIXER_IN1_MUTE |
+		    HII2S_MUX_TOP_MODULE_CFG__S2_OL_MIXER_IN2_MUTE |
+		    HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_MIXER_IN1_MUTE |
+		    HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_MIXER_IN2_MUTE,
+		    0
+	);
+
+	switch (i2s->format & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBM_CFM:
+		i2s->master = false;
+		hi6210_bits(i2s, HII2S_I2S_CFG, 0, HII2S_I2S_CFG__S2_MST_SLV);
+		break;
+	case SND_SOC_DAIFMT_CBS_CFS:
+		i2s->master = true;
+		hi6210_bits(i2s, HII2S_I2S_CFG, HII2S_I2S_CFG__S2_MST_SLV, 0);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		u = HII2S_FORMAT_I2S;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		u = HII2S_FORMAT_LEFT_JUST;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		u = HII2S_FORMAT_RIGHT_JUST;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* set the i2s format */
+	hi6210_bits(i2s, HII2S_I2S_CFG,
+		    (HII2S_I2S_CFG__S2_FUNC_MODE_MASK <<
+		     HII2S_I2S_CFG__S2_FUNC_MODE_SHIFT),
+		    u << HII2S_I2S_CFG__S2_FUNC_MODE_SHIFT);
+
+	/* misc control */
+	hi6210_bits(i2s, HII2S_CLK_SEL,
+		    HII2S_CLK_SEL__I2S_BT_FM_SEL | /* BT gets the I2S */
+		    HII2S_CLK_SEL__EXT_12_288MHZ_SEL, /* internal clock src */
+		    0);
+
+	return 0;
+}
+
+int hi6210_i2s_startup(struct snd_pcm_substream *substream,
+		     struct snd_soc_dai *cpu_dai)
+{
+	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
+	int ret, n;
+
+	/* deassert reset on ABB */
+	if (readl(i2s->base_syscon + SC_PERIPH_RSTSTAT2) & BIT(4))
+		writel(BIT(4), i2s->base_syscon + SC_PERIPH_RSTDIS2);
+
+	for (n = 0; n < i2s->clocks; n++) {
+		ret = clk_prepare_enable(i2s->clk[n]);
+		if (ret)
+			return ret;
+	}
+
+	ret = clk_set_rate(i2s->clk[1], 49152000);
+	if (ret) {
+		dev_err(i2s->dev, "%s: setting 49.152MHz base rate failed %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/* enable clock before frequency division */
+	writel(BIT(9), i2s->base_syscon + SC_PERIPH_CLKEN12);
+
+	/* enable codec working clock / == "codec bus clock" */
+	writel(BIT(5), i2s->base_syscon + SC_PERIPH_CLKEN1);
+
+	/* deassert reset on codec / interface clock / working clock */
+	writel(BIT(5), i2s->base_syscon + SC_PERIPH_RSTEN1);
+	writel(BIT(5), i2s->base_syscon + SC_PERIPH_RSTDIS1);
+
+	/* not interested in i2s irqs */
+	hi6210_bits(i2s, HII2S_CODEC_IRQ_MASK, 0, 0x3f);
+
+	/* reset the stereo downlink fifo */
+	hi6210_bits(i2s, HII2S_APB_AFIFO_CFG_1, 0, BIT(5) | BIT(4));
+	hi6210_bits(i2s, HII2S_APB_AFIFO_CFG_1, BIT(5) | BIT(4), 0);
+
+	hi6210_bits(i2s, HII2S_SW_RST_N,
+		    (HII2S_SW_RST_N__ST_DL_WORDLEN_MASK <<
+		     HII2S_SW_RST_N__ST_DL_WORDLEN_SHIFT),
+		    (HII2S_BITS_16 << HII2S_SW_RST_N__ST_DL_WORDLEN_SHIFT)
+	);
+
+	hi6210_bits(i2s, HII2S_MISC_CFG,
+		    /* mux 11/12 = APB not i2s */
+		    HII2S_MISC_CFG__ST_DL_TEST_SEL |
+		    /* BT R ch  0 = mixer op of DACR ch */
+		    HII2S_MISC_CFG__S2_DOUT_RIGHT_SEL |
+		    HII2S_MISC_CFG__S2_DOUT_TEST_SEL,
+		    HII2S_MISC_CFG__S2_DOUT_RIGHT_SEL |
+		   /* BT L ch = 1 = mux 7 = "mixer output of DACL */
+		    HII2S_MISC_CFG__S2_DOUT_TEST_SEL
+	);
+
+	/* disable the local i2s reset */
+	hi6210_bits(i2s, HII2S_SW_RST_N, 0, HII2S_SW_RST_N__SW_RST_N);
+
+	return 0;
+}
+void hi6210_i2s_shutdown(struct snd_pcm_substream *substream,
+		       struct snd_soc_dai *cpu_dai)
+{
+	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
+	int n;
+
+	for (n = 0; n < i2s->clocks; n++)
+		clk_disable_unprepare(i2s->clk[n]);
+
+	writel(BIT(5), i2s->base_syscon + SC_PERIPH_RSTEN1);
+}
+
+static void hi6210_i2s_txctrl(struct snd_soc_dai *cpu_dai, int on)
+{
+	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
+
+	spin_lock(&i2s->lock);
+
+	if (on) {
+		/* enable S2 TX */
+		hi6210_bits(i2s, HII2S_I2S_CFG, 0, HII2S_I2S_CFG__S2_IF_TX_EN);
+	} else
+		/* disable S2 TX */
+		hi6210_bits(i2s, HII2S_I2S_CFG, HII2S_I2S_CFG__S2_IF_TX_EN, 0);
+
+	spin_unlock(&i2s->lock);
+}
+
+static void hi6210_i2s_rxctrl(struct snd_soc_dai *cpu_dai, int on)
+{
+	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
+
+	spin_lock(&i2s->lock);
+	if (on)
+		hi6210_bits(i2s, HII2S_I2S_CFG, 0, HII2S_I2S_CFG__S2_IF_RX_EN);
+	else
+		hi6210_bits(i2s, HII2S_I2S_CFG, HII2S_I2S_CFG__S2_IF_RX_EN, 0);
+
+	spin_unlock(&i2s->lock);
+}
+
+static int hi6210_i2s_set_sysclk(struct snd_soc_dai *cpu_dai,
+			     int clk_id, unsigned int freq, int dir)
+{
+	return 0;
+}
+
+static int hi6210_i2s_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
+{
+	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
+
+	i2s->format = fmt;
+	i2s->master = (i2s->format & SND_SOC_DAIFMT_MASTER_MASK) ==
+		      SND_SOC_DAIFMT_CBS_CFS;
+
+	return 0;
+}
+
+static int hi6210_i2s_hw_params(struct snd_pcm_substream *substream,
+			    struct snd_pcm_hw_params *params,
+			    struct snd_soc_dai *cpu_dai)
+{
+	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
+	u32 u, signed_data = 0;
+	struct snd_dmaengine_dai_dma_data *dma_data;
+
+	dma_data = snd_soc_dai_get_dma_data(cpu_dai, substream);
+
+	_hi6210_i2s_set_fmt(i2s, substream);
+
+	dma_data->maxburst = 2;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		dma_data->addr = i2s->base_phys + HII2S_ST_DL_CHANNEL;
+	else
+		dma_data->addr = i2s->base_phys + HII2S_STEREO_UPLINK_CHANNEL;
+
+	i2s->channels = params_channels(params);
+	if (i2s->channels == 1)
+		hi6210_bits(i2s, HII2S_I2S_CFG,
+				0, HII2S_I2S_CFG__S2_FRAME_MODE);
+	else
+		hi6210_bits(i2s, HII2S_I2S_CFG,
+				HII2S_I2S_CFG__S2_FRAME_MODE, 0);
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_U16_LE:
+		signed_data = HII2S_I2S_CFG__S2_CODEC_DATA_FORMAT;
+		/* fallthru */
+	case SNDRV_PCM_FORMAT_S16_LE:
+		i2s->bits = 16;
+		dma_data->addr_width = 2;
+		u = HII2S_BITS_16;
+		break;
+
+	case SNDRV_PCM_FORMAT_U24_LE:
+		signed_data = HII2S_I2S_CFG__S2_CODEC_DATA_FORMAT;
+		/* fallthru */
+	case SNDRV_PCM_FORMAT_S24_LE:
+		i2s->bits = 32;
+		u = HII2S_BITS_24;
+		dma_data->addr_width = 3;
+		break;
+	default:
+		dev_err(cpu_dai->dev, "Bad format\n");
+		return -EINVAL;
+	}
+
+	/* clear loopback, set signed type and word length */
+	hi6210_bits(i2s, HII2S_I2S_CFG,
+		    HII2S_I2S_CFG__S2_CODEC_DATA_FORMAT |
+		    (HII2S_I2S_CFG__S2_CODEC_IO_WORDLENGTH_MASK <<
+		     HII2S_I2S_CFG__S2_CODEC_IO_WORDLENGTH_SHIFT) |
+		    (HII2S_I2S_CFG__S2_DIRECT_LOOP_MASK <<
+		     HII2S_I2S_CFG__S2_DIRECT_LOOP_SHIFT),
+		    signed_data |
+		    (u << HII2S_I2S_CFG__S2_CODEC_IO_WORDLENGTH_SHIFT));
+
+	i2s->channel_length = i2s->channels * i2s->bits;
+	i2s->rate = params_rate(params);
+
+	switch (i2s->rate) {
+	case 8000:
+		u = HII2S_FS_RATE_8KHZ;
+		break;
+	case 16000:
+		u = HII2S_FS_RATE_16KHZ;
+		break;
+	case 32000:
+		u = HII2S_FS_RATE_32KHZ;
+		break;
+	case 48000:
+		u = HII2S_FS_RATE_48KHZ;
+		break;
+	case 96000:
+		u = HII2S_FS_RATE_96KHZ;
+		break;
+	case 192000:
+		u = HII2S_FS_RATE_192KHZ;
+		break;
+	};
+
+	if (!i2s->rate || !i2s->channel_length) {
+		dev_err(cpu_dai->dev, "channels/rate/bits on i2s bad\n");
+		return -EINVAL;
+	}
+
+	if (!i2s->master)
+		return 0;
+
+	/* set DAC and related units to correct rate */
+	hi6210_bits(i2s, HII2S_FS_CFG,
+		    (HII2S_FS_CFG__FS_S2_MASK <<
+		     HII2S_FS_CFG__FS_S2_SHIFT) |
+		    (HII2S_FS_CFG__FS_DACLR_MASK <<
+		     HII2S_FS_CFG__FS_DACLR_SHIFT) |
+		    (HII2S_FS_CFG__FS_ST_DL_R_MASK <<
+		     HII2S_FS_CFG__FS_ST_DL_R_SHIFT) |
+		    (HII2S_FS_CFG__FS_ST_DL_L_MASK <<
+		     HII2S_FS_CFG__FS_ST_DL_L_SHIFT),
+		    (u << HII2S_FS_CFG__FS_S2_SHIFT) |
+		    (u << HII2S_FS_CFG__FS_DACLR_SHIFT) |
+		    (u << HII2S_FS_CFG__FS_ST_DL_R_SHIFT) |
+		    (u << HII2S_FS_CFG__FS_ST_DL_L_SHIFT)
+	);
+
+	return 0;
+}
+
+static int hi6210_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
+			  struct snd_soc_dai *cpu_dai)
+{
+	pr_debug("%s\n", __func__);
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+			hi6210_i2s_rxctrl(cpu_dai, 1);
+		else
+			hi6210_i2s_txctrl(cpu_dai, 1);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+			hi6210_i2s_rxctrl(cpu_dai, 0);
+		else
+			hi6210_i2s_txctrl(cpu_dai, 0);
+		break;
+	default:
+		dev_err(cpu_dai->dev, "uknown cmd\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int hi6210_i2s_dai_probe(struct snd_soc_dai *dai)
+{
+	struct hi6210_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+
+	snd_soc_dai_init_dma_data(dai,
+				  &i2s->dma_data[SNDRV_PCM_STREAM_PLAYBACK],
+				  &i2s->dma_data[SNDRV_PCM_STREAM_CAPTURE]);
+
+	return 0;
+}
+
+
+static struct snd_soc_dai_ops hi6210_i2s_dai_ops = {
+	.trigger	= hi6210_i2s_trigger,
+	.hw_params	= hi6210_i2s_hw_params,
+	.set_fmt	= hi6210_i2s_set_fmt,
+	.set_sysclk	= hi6210_i2s_set_sysclk,
+	.startup	= hi6210_i2s_startup,
+	.shutdown	= hi6210_i2s_shutdown,
+};
+
+struct snd_soc_dai_driver hi6210_i2s_dai_init = {
+	.name = "hi6210_i2s",
+	.probe		= hi6210_i2s_dai_probe,
+	.playback = {
+		.channels_min = 2,
+		.channels_max = 2,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE |
+			   SNDRV_PCM_FMTBIT_U16_LE,
+		.rates = SNDRV_PCM_RATE_48000,
+	},
+	.capture = {
+		.channels_min = 2,
+		.channels_max = 2,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE |
+			   SNDRV_PCM_FMTBIT_U16_LE,
+		.rates = SNDRV_PCM_RATE_48000,
+	},
+	.ops = &hi6210_i2s_dai_ops,
+};
+
+static const struct snd_soc_component_driver hi6210_i2s_i2s_comp = {
+	.name = "hi6210_i2s-i2s",
+};
+
+#include <sound/dmaengine_pcm.h>
+
+static const struct snd_pcm_hardware snd_hi6210_hardware = {
+	.info                   = SNDRV_PCM_INFO_MMAP |
+				  SNDRV_PCM_INFO_MMAP_VALID |
+				  SNDRV_PCM_INFO_PAUSE |
+				  SNDRV_PCM_INFO_RESUME |
+				  SNDRV_PCM_INFO_INTERLEAVED |
+				  SNDRV_PCM_INFO_HALF_DUPLEX,
+	.period_bytes_min       = 4096,
+	.period_bytes_max       = 4096,
+	.periods_min            = 4,
+	.periods_max            = UINT_MAX,
+	.buffer_bytes_max       = SIZE_MAX,
+};
+
+static const struct snd_dmaengine_pcm_config hi6210_dmaengine_pcm_config = {
+	.pcm_hardware = &snd_hi6210_hardware,
+	.prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
+	.prealloc_buffer_size = 64 * 1024,
+};
+
+static int hi6210_i2s_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hi6210_i2s *i2s;
+	struct resource *res;
+	int ret;
+
+	i2s = kzalloc(sizeof(*i2s), GFP_KERNEL);
+	if (!i2s)
+		return -ENOMEM;
+
+	i2s->dev = dev;
+	spin_lock_init(&i2s->lock);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENODEV;
+		goto err2;
+	}
+	i2s->base_phys = (phys_addr_t)res->start;
+
+	i2s->dai = hi6210_i2s_dai_init;
+	dev_set_drvdata(&pdev->dev, i2s);
+
+	i2s->base = devm_ioremap_resource(dev, res);
+	if (i2s->base == NULL) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		ret = -ENOMEM;
+		goto err2;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		ret = -ENODEV;
+		goto err2;
+	}
+	i2s->base_syscon = devm_ioremap_resource(dev, res);
+	if (i2s->base_syscon == NULL) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		ret = -ENOMEM;
+		goto err2;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res) {
+		ret = -ENODEV;
+		goto err2;
+	}
+	i2s->base_pmctrl = devm_ioremap_resource(dev, res);
+	if (i2s->base_pmctrl == NULL) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		ret = -ENOMEM;
+		goto err2;
+	}
+
+	do {
+		i2s->clk[i2s->clocks] = of_clk_get(pdev->dev.of_node,
+						   i2s->clocks);
+		if (IS_ERR_OR_NULL(i2s->clk[i2s->clocks]))
+			break;
+		i2s->clocks++;
+	} while (i2s->clocks < ARRAY_SIZE(i2s->clk));
+	if (!i2s->clocks) {
+		ret = PTR_ERR(i2s->clk[0]);
+		dev_err(&pdev->dev, "Failed to get clock\n");
+		goto err2;
+	}
+
+	ret = devm_snd_dmaengine_pcm_register(&pdev->dev,
+					      &hi6210_dmaengine_pcm_config,
+					      0);
+	if (ret)
+		goto err3;
+
+	ret = snd_soc_register_component(&pdev->dev, &hi6210_i2s_i2s_comp,
+					 &i2s->dai, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register dai\n");
+		goto err3;
+	}
+
+	dev_info(&pdev->dev, "Registered as %s\n", i2s->dai.name);
+
+	return 0;
+
+err3:
+	while (--i2s->clocks)
+		clk_put(i2s->clk[i2s->clocks]);
+
+err2:
+	kfree(i2s);
+
+	return ret;
+}
+
+static int hi6210_i2s_remove(struct platform_device *pdev)
+{
+	struct hi6210_i2s *i2s = dev_get_drvdata(&pdev->dev);
+
+	snd_soc_unregister_component(&pdev->dev);
+	dev_set_drvdata(&pdev->dev, NULL);
+	iounmap(i2s->base);
+
+	while (--i2s->clocks)
+		clk_put(i2s->clk[i2s->clocks]);
+
+	kfree(i2s);
+
+	return 0;
+}
+
+static const struct of_device_id hi6210_i2s_dt_ids[] = {
+	{ .compatible = "hisilicon,hi6210-i2s" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, hi6210_i2s_dt_ids);
+
+static struct platform_driver hi6210_i2s_driver = {
+	.probe = hi6210_i2s_probe,
+	.remove = hi6210_i2s_remove,
+	.driver = {
+		.name = "hi6210_i2s",
+		.owner = THIS_MODULE,
+		.of_match_table = hi6210_i2s_dt_ids,
+	},
+};
+
+module_platform_driver(hi6210_i2s_driver);
+
+MODULE_DESCRIPTION("Hisilicon HI6210 I2S driver");
+MODULE_AUTHOR("Andy Green <andy.green@linaro.org>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/hisilicon/hi6210-i2s.h b/sound/soc/hisilicon/hi6210-i2s.h
new file mode 100644
index 0000000..85cecc4
--- /dev/null
+++ b/sound/soc/hisilicon/hi6210-i2s.h
@@ -0,0 +1,276 @@
+/*
+ * linux/sound/soc/hisilicon/hi6210-i2s.h
+ *
+ * Copyright (C) 2015 Linaro, Ltd
+ * Author: Andy Green <andy.green@linaro.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Note at least on 6220, S2 == BT, S1 == Digital FM Radio IF
+ */
+
+#ifndef _HI6210_I2S_H
+#define _HI6210_I2S_H
+
+#define HII2S_SW_RST_N				0
+
+#define HII2S_SW_RST_N__STEREO_UPLINK_WORDLEN_SHIFT			28
+#define HII2S_SW_RST_N__STEREO_UPLINK_WORDLEN_MASK			3
+#define HII2S_SW_RST_N__THIRDMD_UPLINK_WORDLEN_SHIFT			26
+#define HII2S_SW_RST_N__THIRDMD_UPLINK_WORDLEN_MASK			3
+#define HII2S_SW_RST_N__VOICE_UPLINK_WORDLEN_SHIFT			24
+#define HII2S_SW_RST_N__VOICE_UPLINK_WORDLEN_MASK			3
+#define HII2S_SW_RST_N__ST_DL_WORDLEN_SHIFT				20
+#define HII2S_SW_RST_N__ST_DL_WORDLEN_MASK				3
+#define HII2S_SW_RST_N__THIRDMD_DLINK_WORDLEN_SHIFT			18
+#define HII2S_SW_RST_N__THIRDMD_DLINK_WORDLEN_MASK			3
+#define HII2S_SW_RST_N__VOICE_DLINK_WORDLEN_SHIFT			16
+#define HII2S_SW_RST_N__VOICE_DLINK_WORDLEN_MASK			3
+
+#define HII2S_SW_RST_N__SW_RST_N					BIT(0)
+
+enum hi6210_bits {
+	HII2S_BITS_16,
+	HII2S_BITS_18,
+	HII2S_BITS_20,
+	HII2S_BITS_24,
+};
+
+
+#define HII2S_IF_CLK_EN_CFG			4
+
+#define HII2S_IF_CLK_EN_CFG__THIRDMD_UPLINK_EN				BIT(25)
+#define HII2S_IF_CLK_EN_CFG__THIRDMD_DLINK_EN				BIT(24)
+#define HII2S_IF_CLK_EN_CFG__S3_IF_CLK_EN				BIT(20)
+#define HII2S_IF_CLK_EN_CFG__S2_IF_CLK_EN				BIT(16)
+#define HII2S_IF_CLK_EN_CFG__S2_OL_MIXER_EN				BIT(15)
+#define HII2S_IF_CLK_EN_CFG__S2_OL_SRC_EN				BIT(14)
+#define HII2S_IF_CLK_EN_CFG__S2_IR_PGA_EN				BIT(13)
+#define HII2S_IF_CLK_EN_CFG__S2_IL_PGA_EN				BIT(12)
+#define HII2S_IF_CLK_EN_CFG__S1_IR_PGA_EN				BIT(10)
+#define HII2S_IF_CLK_EN_CFG__S1_IL_PGA_EN				BIT(9)
+#define HII2S_IF_CLK_EN_CFG__S1_IF_CLK_EN				BIT(8)
+#define HII2S_IF_CLK_EN_CFG__VOICE_DLINK_SRC_EN				BIT(7)
+#define HII2S_IF_CLK_EN_CFG__VOICE_DLINK_EN				BIT(6)
+#define HII2S_IF_CLK_EN_CFG__ST_DL_R_EN					BIT(5)
+#define HII2S_IF_CLK_EN_CFG__ST_DL_L_EN					BIT(4)
+#define HII2S_IF_CLK_EN_CFG__VOICE_UPLINK_R_EN				BIT(3)
+#define HII2S_IF_CLK_EN_CFG__VOICE_UPLINK_L_EN				BIT(2)
+#define HII2S_IF_CLK_EN_CFG__STEREO_UPLINK_R_EN				BIT(1)
+#define HII2S_IF_CLK_EN_CFG__STEREO_UPLINK_L_EN				BIT(0)
+
+#define HII2S_DIG_FILTER_CLK_EN_CFG		8
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACR_SDM_EN			BIT(30)
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACR_HBF2I_EN			BIT(28)
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACR_MIXER_EN			BIT(25)
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACR_AGC_EN			BIT(24)
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACL_SDM_EN			BIT(22)
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACL_HBF2I_EN			BIT(20)
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACL_MIXER_EN			BIT(17)
+#define HII2S_DIG_FILTER_CLK_EN_CFG__DACL_AGC_EN			BIT(16)
+
+#define HII2S_FS_CFG				0xc
+
+#define HII2S_FS_CFG__FS_S2_SHIFT					28
+#define HII2S_FS_CFG__FS_S2_MASK					7
+#define HII2S_FS_CFG__FS_S1_SHIFT					24
+#define HII2S_FS_CFG__FS_S1_MASK					7
+#define HII2S_FS_CFG__FS_ADCLR_SHIFT					20
+#define HII2S_FS_CFG__FS_ADCLR_MASK					7
+#define HII2S_FS_CFG__FS_DACLR_SHIFT					16
+#define HII2S_FS_CFG__FS_DACLR_MASK					7
+#define HII2S_FS_CFG__FS_ST_DL_R_SHIFT					8
+#define HII2S_FS_CFG__FS_ST_DL_R_MASK					7
+#define HII2S_FS_CFG__FS_ST_DL_L_SHIFT					4
+#define HII2S_FS_CFG__FS_ST_DL_L_MASK					7
+#define HII2S_FS_CFG__FS_VOICE_DLINK_SHIFT				0
+#define HII2S_FS_CFG__FS_VOICE_DLINK_MASK				7
+
+enum hi6210_i2s_rates {
+	HII2S_FS_RATE_8KHZ = 0,
+	HII2S_FS_RATE_16KHZ = 1,
+	HII2S_FS_RATE_32KHZ = 2,
+	HII2S_FS_RATE_48KHZ = 4,
+	HII2S_FS_RATE_96KHZ = 5,
+	HII2S_FS_RATE_192KHZ = 6,
+};
+
+#define HII2S_I2S_CFG				0x10
+
+#define HII2S_I2S_CFG__S2_IF_TX_EN					BIT(31)
+#define HII2S_I2S_CFG__S2_IF_RX_EN					BIT(30)
+#define HII2S_I2S_CFG__S2_FRAME_MODE					BIT(29)
+#define HII2S_I2S_CFG__S2_MST_SLV					BIT(28)
+#define HII2S_I2S_CFG__S2_LRCK_MODE					BIT(27)
+#define HII2S_I2S_CFG__S2_CHNNL_MODE					BIT(26)
+#define HII2S_I2S_CFG__S2_CODEC_IO_WORDLENGTH_SHIFT			24
+#define HII2S_I2S_CFG__S2_CODEC_IO_WORDLENGTH_MASK			3
+#define HII2S_I2S_CFG__S2_DIRECT_LOOP_SHIFT				22
+#define HII2S_I2S_CFG__S2_DIRECT_LOOP_MASK				3
+#define HII2S_I2S_CFG__S2_TX_CLK_SEL					BIT(21)
+#define HII2S_I2S_CFG__S2_RX_CLK_SEL					BIT(20)
+#define HII2S_I2S_CFG__S2_CODEC_DATA_FORMAT				BIT(19)
+#define HII2S_I2S_CFG__S2_FUNC_MODE_SHIFT				16
+#define HII2S_I2S_CFG__S2_FUNC_MODE_MASK				7
+#define HII2S_I2S_CFG__S1_IF_TX_EN					BIT(15)
+#define HII2S_I2S_CFG__S1_IF_RX_EN					BIT(14)
+#define HII2S_I2S_CFG__S1_FRAME_MODE					BIT(13)
+#define HII2S_I2S_CFG__S1_MST_SLV					BIT(12)
+#define HII2S_I2S_CFG__S1_LRCK_MODE					BIT(11)
+#define HII2S_I2S_CFG__S1_CHNNL_MODE					BIT(10)
+#define HII2S_I2S_CFG__S1_CODEC_IO_WORDLENGTH_SHIFT			8
+#define HII2S_I2S_CFG__S1_CODEC_IO_WORDLENGTH_MASK			3
+#define HII2S_I2S_CFG__S1_DIRECT_LOOP_SHIFT				6
+#define HII2S_I2S_CFG__S1_DIRECT_LOOP_MASK				3
+#define HII2S_I2S_CFG__S1_TX_CLK_SEL					BIT(5)
+#define HII2S_I2S_CFG__S1_RX_CLK_SEL					BIT(4)
+#define HII2S_I2S_CFG__S1_CODEC_DATA_FORMAT				BIT(3)
+#define HII2S_I2S_CFG__S1_FUNC_MODE_SHIFT				0
+#define HII2S_I2S_CFG__S1_FUNC_MODE_MASK				7
+
+enum hi6210_i2s_formats {
+	HII2S_FORMAT_I2S,
+	HII2S_FORMAT_PCM_STD,
+	HII2S_FORMAT_PCM_USER,
+	HII2S_FORMAT_LEFT_JUST,
+	HII2S_FORMAT_RIGHT_JUST,
+};
+
+#define HII2S_DIG_FILTER_MODULE_CFG		0x14
+
+#define HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_GAIN_SHIFT		28
+#define HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_GAIN_MASK		3
+#define HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_IN4_MUTE		BIT(27)
+#define HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_IN3_MUTE		BIT(26)
+#define HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_IN2_MUTE		BIT(25)
+#define HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_IN1_MUTE		BIT(24)
+#define HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_GAIN_SHIFT		20
+#define HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_GAIN_MASK		3
+#define HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_IN4_MUTE		BIT(19)
+#define HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_IN3_MUTE		BIT(18)
+#define HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_IN2_MUTE		BIT(17)
+#define HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_IN1_MUTE		BIT(16)
+#define HII2S_DIG_FILTER_MODULE_CFG__SW_DACR_SDM_DITHER			BIT(9)
+#define HII2S_DIG_FILTER_MODULE_CFG__SW_DACL_SDM_DITHER			BIT(8)
+#define HII2S_DIG_FILTER_MODULE_CFG__LM_CODEC_DAC2ADC_SHIFT		4
+#define HII2S_DIG_FILTER_MODULE_CFG__LM_CODEC_DAC2ADC_MASK		7
+#define HII2S_DIG_FILTER_MODULE_CFG__RM_CODEC_DAC2ADC_SHIFT		0
+#define HII2S_DIG_FILTER_MODULE_CFG__RM_CODEC_DAC2ADC_MASK		7
+
+enum hi6210_gains {
+	HII2S_GAIN_100PC,
+	HII2S_GAIN_50PC,
+	HII2S_GAIN_25PC,
+};
+
+#define HII2S_MUX_TOP_MODULE_CFG		0x18
+
+#define HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_MIXER_GAIN_SHIFT		14
+#define HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_MIXER_GAIN_MASK		3
+#define HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_MIXER_IN2_MUTE		BIT(13)
+#define HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_MIXER_IN1_MUTE		BIT(12)
+#define HII2S_MUX_TOP_MODULE_CFG__S2_OL_MIXER_GAIN_SHIFT		10
+#define HII2S_MUX_TOP_MODULE_CFG__S2_OL_MIXER_GAIN_MASK			3
+#define HII2S_MUX_TOP_MODULE_CFG__S2_OL_MIXER_IN2_MUTE			BIT(9)
+#define HII2S_MUX_TOP_MODULE_CFG__S2_OL_MIXER_IN1_MUTE			BIT(8)
+#define HII2S_MUX_TOP_MODULE_CFG__S2_OL_SRC_RDY				BIT(6)
+#define HII2S_MUX_TOP_MODULE_CFG__S2_OL_SRC_MODE_SHIFT			4
+#define HII2S_MUX_TOP_MODULE_CFG__S2_OL_SRC_MODE_MASK			3
+#define HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_SRC_RDY			BIT(3)
+#define HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_SRC_MODE_SHIFT		0
+#define HII2S_MUX_TOP_MODULE_CFG__VOICE_DLINK_SRC_MODE_MASK		7
+
+enum hi6210_s2_src_mode {
+	HII2S_S2_SRC_MODE_3,
+	HII2S_S2_SRC_MODE_12,
+	HII2S_S2_SRC_MODE_6,
+	HII2S_S2_SRC_MODE_2,
+};
+
+enum hi6210_voice_dlink_src_mode {
+	HII2S_VOICE_DL_SRC_MODE_12 = 1,
+	HII2S_VOICE_DL_SRC_MODE_6,
+	HII2S_VOICE_DL_SRC_MODE_2,
+	HII2S_VOICE_DL_SRC_MODE_3,
+};
+
+#define HII2S_ADC_PGA_CFG			0x1c
+#define HII2S_S1_INPUT_PGA_CFG			0x20
+#define HII2S_S2_INPUT_PGA_CFG			0x24
+#define HII2S_ST_DL_PGA_CFG			0x28
+#define HII2S_VOICE_SIDETONE_DLINK_PGA_CFG	0x2c
+#define HII2S_APB_AFIFO_CFG_1			0x30
+#define HII2S_APB_AFIFO_CFG_2			0x34
+#define HII2S_ST_DL_FIFO_TH_CFG			0x38
+
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AEMPTY_SHIFT			24
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AEMPTY_MASK			0x1f
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AFULL_SHIFT			16
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_R_AFULL_MASK			0x1f
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AEMPTY_SHIFT			8
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AEMPTY_MASK			0x1f
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AFULL_SHIFT			0
+#define HII2S_ST_DL_FIFO_TH_CFG__ST_DL_L_AFULL_MASK			0x1f
+
+#define HII2S_STEREO_UPLINK_FIFO_TH_CFG		0x3c
+#define HII2S_VOICE_UPLINK_FIFO_TH_CFG		0x40
+#define HII2S_CODEC_IRQ_MASK			0x44
+#define HII2S_CODEC_IRQ				0x48
+#define HII2S_DACL_AGC_CFG_1			0x4c
+#define HII2S_DACL_AGC_CFG_2			0x50
+#define HII2S_DACR_AGC_CFG_1			0x54
+#define HII2S_DACR_AGC_CFG_2			0x58
+#define HII2S_DMIC_SIF_CFG			0x5c
+#define HII2S_MISC_CFG				0x60
+
+#define HII2S_MISC_CFG__THIRDMD_DLINK_TEST_SEL				BIT(17)
+#define HII2S_MISC_CFG__THIRDMD_DLINK_DIN_SEL				BIT(16)
+#define HII2S_MISC_CFG__S3_DOUT_RIGHT_SEL				BIT(14)
+#define HII2S_MISC_CFG__S3_DOUT_LEFT_SEL				BIT(13)
+#define HII2S_MISC_CFG__S3_DIN_TEST_SEL					BIT(12)
+#define HII2S_MISC_CFG__VOICE_DLINK_SRC_UP_DOUT_VLD_SEL			BIT(8)
+#define HII2S_MISC_CFG__VOICE_DLINK_TEST_SEL				BIT(7)
+#define HII2S_MISC_CFG__VOICE_DLINK_DIN_SEL				BIT(6)
+#define HII2S_MISC_CFG__ST_DL_TEST_SEL					BIT(4)
+#define HII2S_MISC_CFG__S2_DOUT_RIGHT_SEL				BIT(3)
+#define HII2S_MISC_CFG__S2_DOUT_TEST_SEL				BIT(2)
+#define HII2S_MISC_CFG__S1_DOUT_TEST_SEL				BIT(1)
+#define HII2S_MISC_CFG__S2_DOUT_LEFT_SEL				BIT(0)
+
+#define HII2S_S2_SRC_CFG			0x64
+#define HII2S_MEM_CFG				0x68
+#define HII2S_THIRDMD_PCM_PGA_CFG		0x6c
+#define HII2S_THIRD_MODEM_FIFO_TH		0x70
+#define HII2S_S3_ANTI_FREQ_JITTER_TX_INC_CNT	0x74
+#define HII2S_S3_ANTI_FREQ_JITTER_TX_DEC_CNT	0x78
+#define HII2S_S3_ANTI_FREQ_JITTER_RX_INC_CNT	0x7c
+#define HII2S_S3_ANTI_FREQ_JITTER_RX_DEC_CNT	0x80
+#define HII2S_ANTI_FREQ_JITTER_EN		0x84
+#define HII2S_CLK_SEL				0x88
+
+/* 0 = BT owns the i2s */
+#define HII2S_CLK_SEL__I2S_BT_FM_SEL					BIT(0)
+/* 0 = internal source, 1 = ext */
+#define HII2S_CLK_SEL__EXT_12_288MHZ_SEL				BIT(1)
+
+
+#define HII2S_THIRDMD_DLINK_CHANNEL		0xe8
+#define HII2S_THIRDMD_ULINK_CHANNEL		0xec
+#define HII2S_VOICE_DLINK_CHANNEL		0xf0
+
+/* shovel data in here for playback */
+#define HII2S_ST_DL_CHANNEL			0xf4
+#define HII2S_STEREO_UPLINK_CHANNEL		0xf8
+#define HII2S_VOICE_UPLINK_CHANNEL		0xfc
+
+#endif/* _HI6210_I2S_H */
-- 
1.9.1

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

* [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
                   ` (5 preceding siblings ...)
  2016-07-16  2:13 ` [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio John Stultz
@ 2016-07-16  2:13 ` John Stultz
  2016-07-16 11:48   ` Mark Brown
  2016-07-16  3:15 ` [RFC][PATCH 0/7] Add HDMI audio support for HiKey Andy Green
  7 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  2:13 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Andy Green, Dave Long, Guodong Xu

Add entry for k3-dma driver and i2s/hdmi audio devices.

This enables HDMI audio output.

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 36 +++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 189d215..ba34962 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -228,6 +228,8 @@
 		compatible = "simple-bus";
 		#address-cells = <2>;
 		#size-cells = <2>;
+		#sound-dai-cells = <0>;
+		interrupt-parent = <&gic>;
 		ranges;
 
 		sram: sram@fff80000 {
@@ -325,6 +327,19 @@
 			status = "disabled";
 		};
 
+		dma0: dma@f7370000 {
+			compatible = "hisilicon,k3-dma-1.0";
+			reg = <0x0 0xf7370000 0x0 0x1000>;
+			#dma-cells = <1>;
+			dma-channels = <15>;
+			dma-requests = <32>;
+			interrupts = <0 84 4>;
+			clocks = <&sys_ctrl HI6220_EDMAC_ACLK>;
+			dma-no-cci;
+			dma-type = "hi6220_dma";
+			status = "ok";
+		};
+
 		dual_timer0: timer@f8008000 {
 			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x0 0xf8008000 0x0 0x1000>;
@@ -800,6 +815,27 @@
 			#thermal-sensor-cells = <1>;
 		};
 
+		i2s0: hi6210_i2s {
+			compatible = "hisilicon,hi6210-i2s";
+			reg = <0x0 0xf7118000 0x0 0x8000>, /* i2s unit */
+			      <0x0 0xf7030000 0x0 0x400>,  /* syscon */
+			      <0x0 0xf7032000 0x0 0x400>;  /* pmctrl */
+			interrupts = <0 123 0x4>; /* 155 "DigACodec_intr"-32 */
+			pinctrl-names = "default";
+			pinctrl-0 = <&bt_pmx_func &bt_cfg_func>;
+			clocks = <&sys_ctrl HI6220_DACODEC_PCLK>,
+				 <&sys_ctrl HI6220_BBPPLL0_DIV>;
+			clock-names = "dacodec", "i2s-base";
+			dmas = <&dma0 15 &dma0 14>;
+			dma-names = "rx", "tx";
+		};
+
+		hi6210_hdmi_card: hi6210_hdmi_card {
+			compatible = "hisilicon,hi6210-hdmi-audio-card";
+			reg = <0 0 0 0>;
+			sound-dai = <&i2s0>;
+		};
+
 		thermal-zones {
 
 			cls0: cls0 {
-- 
1.9.1

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

* Re: [RFC][PATCH 0/7] Add HDMI audio support for HiKey
  2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
                   ` (6 preceding siblings ...)
  2016-07-16  2:13 ` [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support John Stultz
@ 2016-07-16  3:15 ` Andy Green
  2016-07-16  3:38   ` John Stultz
  7 siblings, 1 reply; 21+ messages in thread
From: Andy Green @ 2016-07-16  3:15 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski, Maxime Ripard,
	Vinod Koul, Dan Williams, Liam Girdwood, Mark Brown,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Dave Long,
	Guodong Xu

On Fri, 2016-07-15 at 19:13 -0700, John Stultz wrote:
> This patch set is required for HDMI audio support on HiKey.
> 
> This patchset hasn't yet seen the light of lkml, so I suspect
> there will be a few revisions, but I wanted to send it out for
> an initial review.
> 
> The work is mostly that of Andy Green's, but I've taking a swing
> at forward porting and cleaning it up where I saw fit. So credit
> to Andy and blame to me. Apologies in advance, as I'm not super
> familiar with either DMA or ASoC driver.
> 
> The one bit missing to have audio fully working is changes to the
> adv7511 driver, but most of those changes are still out of tree, so
> I'll submit those changes once they land.
> 
> Feedback would be very much appreicated!

Thanks John, it's good to know that work didn't go to waste.

The linaro.org email in the patches is dead, since I resigned from
Linaro a few months ago.  If the goal of adding it to the kernel is to
make it possible to contact the author, it should change to
<andy@warmcat.com>.
There are (were) a couple of limitations with it that should be
commented somewhere:

 1) The cyclic DMA, at least going into the I2S FIFO, had what appeared
to be hw bugs when I left it, I had asked hisilicon about it but got no
useful reply.  The DMA worked well generally, but there were audible
clicks and pops at intervals even though the DMA really is cyclic.  I
dunno whether they got around to looking at it or not: if not, there
should probably be a comment in the driver about it.  There were notes
in the I2S FIFO docs (it seemed the likely culprit) about needing to
take care about FIFO trigger levels but didn't seem to change anything.

 2) The driver only exposes 48kHz / 2ch.

Otherwise it worked well.

Thanks again for upstreaming it.

-Andy

> thanks
> -john
> 
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> 
> Andy Green (5):
>   k3dma: Fix hisi burst clipping
>   k3dma: Fix dma err offsets
>   k3dma: Fix "nobody cared" message seen on any error
>   k3dma: Add cyclic mode for audio
>   ASoC: hisilicon:  Add hi6210 i2s audio driver for hdmi audio
> 
> John Stultz (2):
>   Kconfig: Allow k3dma driver to be selected for more then HISI3xx
>     platforms
>   dts: hi6220: Add k3-dma and i2s/hdmi audio support
> 
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  36 ++
>  drivers/dma/Kconfig                       |   1 -
>  drivers/dma/k3dma.c                       | 149 ++++++-
>  sound/soc/Kconfig                         |   1 +
>  sound/soc/Makefile                        |   1 +
>  sound/soc/hisilicon/Kconfig               |   5 +
>  sound/soc/hisilicon/Makefile              |   2 +
>  sound/soc/hisilicon/hi6210-hdmi-card.c    | 131 ++++++
>  sound/soc/hisilicon/hi6210-i2s.c          | 641
> ++++++++++++++++++++++++++++++
>  sound/soc/hisilicon/hi6210-i2s.h          | 276 +++++++++++++
>  10 files changed, 1222 insertions(+), 21 deletions(-)
>  create mode 100644 sound/soc/hisilicon/Kconfig
>  create mode 100644 sound/soc/hisilicon/Makefile
>  create mode 100644 sound/soc/hisilicon/hi6210-hdmi-card.c
>  create mode 100644 sound/soc/hisilicon/hi6210-i2s.c
>  create mode 100644 sound/soc/hisilicon/hi6210-i2s.h
> 

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

* Re: [RFC][PATCH 0/7] Add HDMI audio support for HiKey
  2016-07-16  3:15 ` [RFC][PATCH 0/7] Add HDMI audio support for HiKey Andy Green
@ 2016-07-16  3:38   ` John Stultz
  2016-07-16 11:12     ` Mark Brown
  0 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-16  3:38 UTC (permalink / raw)
  To: Andy Green
  Cc: lkml, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring,
	Dave Long, Guodong Xu

On Fri, Jul 15, 2016 at 8:15 PM, Andy Green <andy@warmcat.com> wrote:
> On Fri, 2016-07-15 at 19:13 -0700, John Stultz wrote:
>> This patch set is required for HDMI audio support on HiKey.
>>
>> This patchset hasn't yet seen the light of lkml, so I suspect
>> there will be a few revisions, but I wanted to send it out for
>> an initial review.
>>
>> The work is mostly that of Andy Green's, but I've taking a swing
>> at forward porting and cleaning it up where I saw fit. So credit
>> to Andy and blame to me. Apologies in advance, as I'm not super
>> familiar with either DMA or ASoC driver.
>>
>> The one bit missing to have audio fully working is changes to the
>> adv7511 driver, but most of those changes are still out of tree, so
>> I'll submit those changes once they land.
>>
>> Feedback would be very much appreicated!
>
> Thanks John, it's good to know that work didn't go to waste.
>
> The linaro.org email in the patches is dead, since I resigned from
> Linaro a few months ago.  If the goal of adding it to the kernel is to
> make it possible to contact the author, it should change to
> <andy@warmcat.com>.

Yea. I'm not sure what the communities policy on Author/SoB lines in
the face of email address changes.
For the moment I'll leave the credit lines as is (since that's how I
got them, and changing SoB's is usually a big no no). But if others
have advice on how to best handle this I'd appreciate it. I'll be sure
to leave your new email in the Cc: entries.
(Though I need to figure out how to get git send-email to not cc the
author line to avoid the reply-all noise)

> There are (were) a couple of limitations with it that should be
> commented somewhere:
>
>  1) The cyclic DMA, at least going into the I2S FIFO, had what appeared
> to be hw bugs when I left it, I had asked hisilicon about it but got no
> useful reply.  The DMA worked well generally, but there were audible
> clicks and pops at intervals even though the DMA really is cyclic.  I
> dunno whether they got around to looking at it or not: if not, there
> should probably be a comment in the driver about it.  There were notes
> in the I2S FIFO docs (it seemed the likely culprit) about needing to
> take care about FIFO trigger levels but didn't seem to change anything.

So against the 4.4 and later kernels, I've no longer had trouble with
the pops and noise. There is an outstanding issue of a occasional DMA
error from the hardware on the first transfer after opening the audio
device, but some of the HiSi folks are looking into that.

>  2) The driver only exposes 48kHz / 2ch.

Yea. I've limited the i2s/hdmi-card driver to only 48k to match.

thanks
-john

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

* Re: [RFC][PATCH 0/7] Add HDMI audio support for HiKey
  2016-07-16  3:38   ` John Stultz
@ 2016-07-16 11:12     ` Mark Brown
  0 siblings, 0 replies; 21+ messages in thread
From: Mark Brown @ 2016-07-16 11:12 UTC (permalink / raw)
  To: John Stultz
  Cc: Andy Green, lkml, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Dave Long,
	Guodong Xu

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

On Fri, Jul 15, 2016 at 08:38:47PM -0700, John Stultz wrote:

> Yea. I'm not sure what the communities policy on Author/SoB lines in
> the face of email address changes.

Given that the signoff is all about DCO and hence licensing it should
probably stay with Linaro.

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

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

* Re: [RFC][PATCH 5/7] Kconfig: Allow k3dma driver to be selected for more then HISI3xx platforms
  2016-07-16  2:13 ` [RFC][PATCH 5/7] Kconfig: Allow k3dma driver to be selected for more then HISI3xx platforms John Stultz
@ 2016-07-16 11:18   ` Mark Brown
  0 siblings, 0 replies; 21+ messages in thread
From: Mark Brown @ 2016-07-16 11:18 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu

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

On Fri, Jul 15, 2016 at 07:13:25PM -0700, John Stultz wrote:

> This allows the k3dma driver to be selected on HiKey

>  config K3_DMA
>  	tristate "Hisilicon K3 DMA support"
> -	depends on ARCH_HI3xxx
>  	select DMA_ENGINE
>  	select DMA_VIRTUAL_CHANNELS
>  	help

And *every* other platform ever.  You probably want 

	ARCH_HI3xxx || ARCH_HISI || COMPILE_TEST

here.

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

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

* Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio
  2016-07-16  2:13 ` [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio John Stultz
@ 2016-07-16 11:44   ` Mark Brown
  2016-07-19 21:59     ` John Stultz
  0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2016-07-16 11:44 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu

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

On Fri, Jul 15, 2016 at 07:13:26PM -0700, John Stultz wrote:

>  sound/soc/Kconfig                      |   1 +
>  sound/soc/Makefile                     |   1 +
>  sound/soc/hisilicon/Kconfig            |   5 +
>  sound/soc/hisilicon/Makefile           |   2 +
>  sound/soc/hisilicon/hi6210-hdmi-card.c | 131 +++++++
>  sound/soc/hisilicon/hi6210-i2s.c       | 641 +++++++++++++++++++++++++++++++++
>  sound/soc/hisilicon/hi6210-i2s.h       | 276 ++++++++++++++
>  7 files changed, 1057 insertions(+)

This is adding several different drivers so should be at least one
patch per driver.  Combining many different drivers into one patch makes
it more difficult to review as the patch gets larger and more fatiguing
and causes review problems in one driver to block others.  Look at how
existing drivers are added to the same subsystem.

> +static int hdmi_hw_params(struct snd_pcm_substream *substream,
> +			  struct snd_pcm_hw_params *params)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> +	int ret;
> +
> +	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S |
> +			SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS);
> +	if (ret)
> +		return ret;
> +
> +	/* set i2s system clock */
> +	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, 24576000, SND_SOC_CLOCK_IN);
> +	if (ret)
> +		return ret;

Neither of these depends at all on the parameters so they should be
being set on init, not at hw_params time.  In the case of the DAI format
this should be done with dai_fmt in the dai_link.

> +	.cpu_dai_name = "f7118000.hi6210_i2s",

Why is this not connected up in DT - as things stand the card has zero
reuse potential.  Though given that this has 

> +	.codec_name = "0.hi6210_hdmi_card",

If you've got a CODEC with hdmi_card in the name that suggests there's a
serious abstraction problem going on somewhere.  

> +static void hi6210_bits(struct hi6210_i2s *i2s, u32 ofs, u32 reset, u32 set)
> +{
> +	u32 val = readl(i2s->base + ofs) & ~reset;
> +
> +	writel(val | set, i2s->base + ofs);
> +}

This isn't the usual pattern for read/modify/write operations in the
kernel and doesn't seem like it's going to be great for legibility - the
reader has to remember which order clear and set are and the name
doesn't give many clues.

> +	hi6210_bits(i2s, HII2S_DIG_FILTER_MODULE_CFG,
> +		    HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_IN2_MUTE |
> +		    HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_IN2_MUTE,
> +		    0
> +	);

Coding style, the final ); is just weird.  These register names make
this look like there's some routing control going on here which should
be being exposed to users.

> +	for (n = 0; n < i2s->clocks; n++) {
> +		ret = clk_prepare_enable(i2s->clk[n]);
> +		if (ret)
> +			return ret;
> +	}

This won't unwind things on error.

> +	ret = clk_set_rate(i2s->clk[1], 49152000);
> +	if (ret) {
> +		dev_err(i2s->dev, "%s: setting 49.152MHz base rate failed %d\n",
> +			__func__, ret);
> +		return ret;
> +	}

Magic array index there isn't great.

> +static void hi6210_i2s_txctrl(struct snd_soc_dai *cpu_dai, int on)
> +{
> +	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
> +
> +	spin_lock(&i2s->lock);
> +
> +	if (on) {
> +		/* enable S2 TX */
> +		hi6210_bits(i2s, HII2S_I2S_CFG, 0, HII2S_I2S_CFG__S2_IF_TX_EN);
> +	} else
> +		/* disable S2 TX */
> +		hi6210_bits(i2s, HII2S_I2S_CFG, HII2S_I2S_CFG__S2_IF_TX_EN, 0);

Coding style, { } on both side or neither :(

> +static int hi6210_i2s_set_sysclk(struct snd_soc_dai *cpu_dai,
> +			     int clk_id, unsigned int freq, int dir)
> +{
> +	return 0;
> +}

Remove empty operations, if the operation can reasonably be ignored the
frameworks will handle that well.

> +static int hi6210_i2s_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
> +{
> +	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
> +
> +	i2s->format = fmt;
> +	i2s->master = (i2s->format & SND_SOC_DAIFMT_MASTER_MASK) ==
> +		      SND_SOC_DAIFMT_CBS_CFS;

We're validating this elsewhere rather than when the user sets it.

> +	switch (i2s->rate) {
> +	case 8000:
> +		u = HII2S_FS_RATE_8KHZ;
> +		break;
> +	case 16000:
> +		u = HII2S_FS_RATE_16KHZ;
> +		break;
> +	case 32000:
> +		u = HII2S_FS_RATE_32KHZ;
> +		break;
> +	case 48000:
> +		u = HII2S_FS_RATE_48KHZ;
> +		break;
> +	case 96000:
> +		u = HII2S_FS_RATE_96KHZ;
> +		break;
> +	case 192000:
> +		u = HII2S_FS_RATE_192KHZ;
> +		break;
> +	};

Should have validation on the default case.  I'm not super convinced
that u is a helpful variable name for the temporaries in this driver,
it's not a convention I've ever seen before so I had to go and check
what was going on.

> +};
> +
> +#include <sound/dmaengine_pcm.h>
> +
> +static const struct snd_pcm_hardware snd_hi6210_hardware = {

Why are we including files in the middle of the driver?

> +	.info                   = SNDRV_PCM_INFO_MMAP |
> +				  SNDRV_PCM_INFO_MMAP_VALID |
> +				  SNDRV_PCM_INFO_PAUSE |
> +				  SNDRV_PCM_INFO_RESUME |
> +				  SNDRV_PCM_INFO_INTERLEAVED |
> +				  SNDRV_PCM_INFO_HALF_DUPLEX,
> +	.period_bytes_min       = 4096,
> +	.period_bytes_max       = 4096,
> +	.periods_min            = 4,
> +	.periods_max            = UINT_MAX,
> +	.buffer_bytes_max       = SIZE_MAX,
> +};

Why are we open coding dmaengine configuration here?  The core code
should be able to discover this by querying the dmaengine driver.

> +static int hi6210_i2s_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct hi6210_i2s *i2s;
> +	struct resource *res;
> +	int ret;
> +
> +	i2s = kzalloc(sizeof(*i2s), GFP_KERNEL);
> +	if (!i2s)
> +		return -ENOMEM;

devm_kzalloc().

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {

No need to check, devm_ioremap_resource() will do this for you.

> +	i2s->base = devm_ioremap_resource(dev, res);
> +	if (i2s->base == NULL) {
> +		dev_err(&pdev->dev, "ioremap failed\n");

devm_ioremap_resource() is quite noisy when it fails, probably no need
for another error especially one that's not saying what we failed to
map.

> +	do {
> +		i2s->clk[i2s->clocks] = of_clk_get(pdev->dev.of_node,
> +						   i2s->clocks);
> +		if (IS_ERR_OR_NULL(i2s->clk[i2s->clocks]))
> +			break;
> +		i2s->clocks++;
> +	} while (i2s->clocks < ARRAY_SIZE(i2s->clk));
> +	if (!i2s->clocks) {
> +		ret = PTR_ERR(i2s->clk[0]);
> +		dev_err(&pdev->dev, "Failed to get clock\n");
> +		goto err2;
> +	}

So we grab some random clocks until we hit an error, hope that the
second one we find is whatever one we were explicitly setting rates on
and ignore errors other than possibly a failure to find any clocks at
all.  This isn't great at all, it's not going to be very robust.  We
should know what clocks we're looking for, we should use the regular
clock API to get them (so we're not tied to DT and so we get a devm
version) and we should care about errors on all the clocks.

> +	dev_info(&pdev->dev, "Registered as %s\n", i2s->dai.name);

This is just noise, remove it.

> +static const struct of_device_id hi6210_i2s_dt_ids[] = {
> +	{ .compatible = "hisilicon,hi6210-i2s" },
> +	{ /* sentinel */ }
> +};

The code makes this look like it's not just an I2S controller.

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

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

* Re: [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support
  2016-07-16  2:13 ` [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support John Stultz
@ 2016-07-16 11:48   ` Mark Brown
  2016-07-18 17:20     ` John Stultz
  0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2016-07-16 11:48 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu

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

On Fri, Jul 15, 2016 at 07:13:27PM -0700, John Stultz wrote:

> Add entry for k3-dma driver and i2s/hdmi audio devices.

> This enables HDMI audio output.

These bindings appear to be undocumented.  All new bindings require
documentation.

> +		i2s0: hi6210_i2s {
> +			compatible = "hisilicon,hi6210-i2s";
> +			reg = <0x0 0xf7118000 0x0 0x8000>, /* i2s unit */
> +			      <0x0 0xf7030000 0x0 0x400>,  /* syscon */
> +			      <0x0 0xf7032000 0x0 0x400>;  /* pmctrl */

Some of this looks like we should be using a system controller binding
rather than listing the system controller directly here.  The magic
number indexing for the resources is also not good, we should be using
reg-names to get name based lookups (I had been going to query that on
the binding document when I failed to find it...).

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

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

* Re: [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping
  2016-07-16  2:13 ` [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping John Stultz
@ 2016-07-18  6:37   ` zhangfei
  0 siblings, 0 replies; 21+ messages in thread
From: zhangfei @ 2016-07-18  6:37 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Andy Green, Jingoo Han, Krzysztof Kozlowski, Maxime Ripard,
	Vinod Koul, Dan Williams, Liam Girdwood, Mark Brown,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu



On 07/16/2016 10:13 AM, John Stultz wrote:
> From: Andy Green <andy.green@linaro.org>
>
> Max burst len is a 4-bit field, but at the moment it's clipped with
> a 5-bit constant... reduce it to that which can be expressed
>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> [jstultz: Forward ported to mainline]
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Acked-by: Zhangfei Gao <zhangfei.gao@linaro.org>

> ---
>   drivers/dma/k3dma.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> index 1ba2fd7..d01a11d 100644
> --- a/drivers/dma/k3dma.c
> +++ b/drivers/dma/k3dma.c
> @@ -552,7 +552,7 @@ static int k3_dma_config(struct dma_chan *chan,
>   	c->ccfg |= (val << 12) | (val << 16);
>
>   	if ((maxburst == 0) || (maxburst > 16))
> -		val = 16;
> +		val = 15;
>   	else
>   		val = maxburst - 1;
>   	c->ccfg |= (val << 20) | (val << 24);
>

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

* Re: [RFC][PATCH 2/7] k3dma: Fix dma err offsets
  2016-07-16  2:13 ` [RFC][PATCH 2/7] k3dma: Fix dma err offsets John Stultz
@ 2016-07-18  6:39   ` zhangfei
  0 siblings, 0 replies; 21+ messages in thread
From: zhangfei @ 2016-07-18  6:39 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Andy Green, Jingoo Han, Krzysztof Kozlowski, Maxime Ripard,
	Vinod Koul, Dan Williams, Liam Girdwood, Mark Brown,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu



On 07/16/2016 10:13 AM, John Stultz wrote:
> From: Andy Green <andy.green@linaro.org>
>
> The offsets for ERR1 and ERR2 are wrong actually.
> That's why you can never clear an error.
>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> [jstultz: Forward ported to mainline]
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Acked-by: Zhangfei Gao <zhangfei.gao@linaro.org>

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

* Re: [RFC][PATCH 3/7] k3dma: Fix "nobody cared" message seen on any error
  2016-07-16  2:13 ` [RFC][PATCH 3/7] k3dma: Fix "nobody cared" message seen on any error John Stultz
@ 2016-07-18  6:40   ` zhangfei
  0 siblings, 0 replies; 21+ messages in thread
From: zhangfei @ 2016-07-18  6:40 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Andy Green, Jingoo Han, Krzysztof Kozlowski, Maxime Ripard,
	Vinod Koul, Dan Williams, Liam Girdwood, Mark Brown,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu



On 07/16/2016 10:13 AM, John Stultz wrote:
> From: Andy Green <andy.green@linaro.org>
>
> As it was before, as soon as the DMAC IP felt there was an error
> he would return IRQ_NONE since no actual transfer had completed.
>
> After spinning on that for 100K interrupts, Linux yanks the IRQ with
> a "nobody cared" error.
>
> This patch lets it handle the interrupt and keep the IRQ alive.
>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> [jstultz: Forward ported to mainline]
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Acked-by: Zhangfei Gao <zhangfei.gao@linaro.org>

> ---
>   drivers/dma/k3dma.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> index 8dd050c..c2906a82 100644
> --- a/drivers/dma/k3dma.c
> +++ b/drivers/dma/k3dma.c
> @@ -220,11 +220,13 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id)
>   	writel_relaxed(err1, d->base + INT_ERR1_RAW);
>   	writel_relaxed(err2, d->base + INT_ERR2_RAW);
>
> -	if (irq_chan) {
> +	if (irq_chan)
>   		tasklet_schedule(&d->task);
> +
> +	if (irq_chan || err1 || err2)
>   		return IRQ_HANDLED;
> -	} else
> -		return IRQ_NONE;
> +
> +	return IRQ_NONE;
>   }
>
>   static int k3_dma_start_txd(struct k3_dma_chan *c)
>

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

* Re: [RFC][PATCH 4/7] k3dma: Add cyclic mode for audio
  2016-07-16  2:13 ` [RFC][PATCH 4/7] k3dma: Add cyclic mode for audio John Stultz
@ 2016-07-18  6:43   ` zhangfei
  0 siblings, 0 replies; 21+ messages in thread
From: zhangfei @ 2016-07-18  6:43 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Andy Green, Jingoo Han, Krzysztof Kozlowski, Maxime Ripard,
	Vinod Koul, Dan Williams, Liam Girdwood, Mark Brown,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu



On 07/16/2016 10:13 AM, John Stultz wrote:
> From: Andy Green <andy.green@linaro.org>
>
> Currently the k3dma driver doesn't offer the cyclic mode
> necessary for handling audio.
>
> This patch adds it.
>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> [jstultz: Forward ported to mainline, removed a few
>   bits of logic that didn't seem to have much effect]
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Could you change pr_xxx to dev_xxx, and remove one print.

Otherwise
Acked-by: Zhangfei Gao <zhangfei.gao@linaro.org>

> ---
>   drivers/dma/k3dma.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 121 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> index c2906a82..eff7483 100644
> --- a/drivers/dma/k3dma.c
> +++ b/drivers/dma/k3dma.c
> @@ -1,5 +1,5 @@
>   /*
> - * Copyright (c) 2013 Linaro Ltd.
> + * Copyright (c) 2013 - 2015 Linaro Ltd.
>    * Copyright (c) 2013 Hisilicon Limited.
>    *
>    * This program is free software; you can redistribute it and/or modify
> @@ -25,22 +25,27 @@
>
>   #define DRIVER_NAME		"k3-dma"
>   #define DMA_MAX_SIZE		0x1ffc
> +#define DMA_CYCLIC_MAX_PERIOD	0x1000
>
>   #define INT_STAT		0x00
>   #define INT_TC1			0x04
> +#define INT_TC2			0x08
>   #define INT_ERR1		0x0c
>   #define INT_ERR2		0x10
>   #define INT_TC1_MASK		0x18
> +#define INT_TC2_MASK		0x1c
>   #define INT_ERR1_MASK		0x20
>   #define INT_ERR2_MASK		0x24
>   #define INT_TC1_RAW		0x600
> +#define INT_TC2_RAW		0x608
>   #define INT_ERR1_RAW		0x610
>   #define INT_ERR2_RAW		0x618
>   #define CH_PRI			0x688
>   #define CH_STAT			0x690
>   #define CX_CUR_CNT		0x704
>   #define CX_LLI			0x800
> -#define CX_CNT			0x810
> +#define CX_CNT1			0x80c
> +#define CX_CNT0			0x810
>   #define CX_SRC			0x814
>   #define CX_DST			0x818
>   #define CX_CFG			0x81c
> @@ -49,6 +54,7 @@
>
>   #define CX_LLI_CHAIN_EN		0x2
>   #define CX_CFG_EN		0x1
> +#define CX_CFG_NODEIRQ		BIT(1)
>   #define CX_CFG_MEM2PER		(0x1 << 2)
>   #define CX_CFG_PER2MEM		(0x2 << 2)
>   #define CX_CFG_SRCINCR		(0x1 << 31)
> @@ -81,6 +87,7 @@ struct k3_dma_chan {
>   	enum dma_transfer_direction dir;
>   	dma_addr_t		dev_addr;
>   	enum dma_status		status;
> +	bool			cyclic;
>   };
>
>   struct k3_dma_phy {
> @@ -134,6 +141,7 @@ static void k3_dma_terminate_chan(struct k3_dma_phy *phy, struct k3_dma_dev *d)
>
>   	val = 0x1 << phy->idx;
>   	writel_relaxed(val, d->base + INT_TC1_RAW);
> +	writel_relaxed(val, d->base + INT_TC2_RAW);
>   	writel_relaxed(val, d->base + INT_ERR1_RAW);
>   	writel_relaxed(val, d->base + INT_ERR2_RAW);
>   }
> @@ -141,11 +149,15 @@ static void k3_dma_terminate_chan(struct k3_dma_phy *phy, struct k3_dma_dev *d)
>   static void k3_dma_set_desc(struct k3_dma_phy *phy, struct k3_desc_hw *hw)
>   {
>   	writel_relaxed(hw->lli, phy->base + CX_LLI);
> -	writel_relaxed(hw->count, phy->base + CX_CNT);
> +	writel_relaxed(hw->count, phy->base + CX_CNT0);
>   	writel_relaxed(hw->saddr, phy->base + CX_SRC);
>   	writel_relaxed(hw->daddr, phy->base + CX_DST);
>   	writel_relaxed(AXI_CFG_DEFAULT, phy->base + AXI_CFG);
>   	writel_relaxed(hw->config, phy->base + CX_CFG);
> +	pr_debug("%s: desc %p: ch idx = %d, lli: 0x%x, count: 0x%x,"
> +		" saddr: 0x%x, daddr 0x%x, cfg: 0x%x\n", __func__,
> +		(void *) hw, phy->idx, hw->lli,	hw->count, hw->saddr,
> +		hw->daddr, hw->config);
>   }
>
>   static u32 k3_dma_get_curr_cnt(struct k3_dma_dev *d, struct k3_dma_phy *phy)
> @@ -175,11 +187,13 @@ static void k3_dma_enable_dma(struct k3_dma_dev *d, bool on)
>
>   		/* unmask irq */
>   		writel_relaxed(0xffff, d->base + INT_TC1_MASK);
> +		writel_relaxed(0xffff, d->base + INT_TC2_MASK);
>   		writel_relaxed(0xffff, d->base + INT_ERR1_MASK);
>   		writel_relaxed(0xffff, d->base + INT_ERR2_MASK);
>   	} else {
>   		/* mask irq */
>   		writel_relaxed(0x0, d->base + INT_TC1_MASK);
> +		writel_relaxed(0x0, d->base + INT_TC2_MASK);
>   		writel_relaxed(0x0, d->base + INT_ERR1_MASK);
>   		writel_relaxed(0x0, d->base + INT_ERR2_MASK);
>   	}
> @@ -192,24 +206,31 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id)
>   	struct k3_dma_chan *c;
>   	u32 stat = readl_relaxed(d->base + INT_STAT);
>   	u32 tc1  = readl_relaxed(d->base + INT_TC1);
> +	u32 tc2  = readl_relaxed(d->base + INT_TC2);
>   	u32 err1 = readl_relaxed(d->base + INT_ERR1);
>   	u32 err2 = readl_relaxed(d->base + INT_ERR2);
>   	u32 i, irq_chan = 0;
>
>   	while (stat) {
>   		i = __ffs(stat);
> -		stat &= (stat - 1);
> -		if (likely(tc1 & BIT(i))) {
> +		stat &= ~BIT(i);
> +		if (likely(tc1 & BIT(i)) || (tc2 & BIT(i))) {
> +			unsigned long flags;
> +
>   			p = &d->phy[i];
>   			c = p->vchan;
> -			if (c) {
> -				unsigned long flags;
> -
> +			if (c && (tc1 & BIT(i))) {
>   				spin_lock_irqsave(&c->vc.lock, flags);
>   				vchan_cookie_complete(&p->ds_run->vd);
>   				p->ds_done = p->ds_run;
>   				spin_unlock_irqrestore(&c->vc.lock, flags);
>   			}
> +			if (c && (tc2 & BIT(i))) {
> +				spin_lock_irqsave(&c->vc.lock, flags);
> +				if (p->ds_run != NULL)
> +					vchan_cyclic_callback(&p->ds_run->vd);
> +				spin_unlock_irqrestore(&c->vc.lock, flags);
> +			}
>   			irq_chan |= BIT(i);
>   		}
>   		if (unlikely((err1 & BIT(i)) || (err2 & BIT(i))))
> @@ -217,6 +238,7 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id)
>   	}
>
>   	writel_relaxed(irq_chan, d->base + INT_TC1_RAW);
> +	writel_relaxed(irq_chan, d->base + INT_TC2_RAW);
>   	writel_relaxed(err1, d->base + INT_ERR1_RAW);
>   	writel_relaxed(err2, d->base + INT_ERR2_RAW);
>
> @@ -352,7 +374,7 @@ static enum dma_status k3_dma_tx_status(struct dma_chan *chan,
>   	 * its total size.
>   	 */
>   	vd = vchan_find_desc(&c->vc, cookie);
> -	if (vd) {
> +	if (vd && !c->cyclic) {
>   		bytes = container_of(vd, struct k3_dma_desc_sw, vd)->size;
>   	} else if ((!p) || (!p->ds_run)) {
>   		bytes = 0;
> @@ -362,7 +384,8 @@ static enum dma_status k3_dma_tx_status(struct dma_chan *chan,
>
>   		bytes = k3_dma_get_curr_cnt(d, p);
>   		clli = k3_dma_get_curr_lli(p);
> -		index = (clli - ds->desc_hw_lli) / sizeof(struct k3_desc_hw);
> +		index = ((clli - ds->desc_hw_lli) /
> +				sizeof(struct k3_desc_hw)) + 1;
>   		for (; index < ds->desc_num; index++) {
>   			bytes += ds->desc_hw[index].count;
>   			/* end of lli */
> @@ -403,14 +426,22 @@ static void k3_dma_issue_pending(struct dma_chan *chan)
>   static void k3_dma_fill_desc(struct k3_dma_desc_sw *ds, dma_addr_t dst,
>   			dma_addr_t src, size_t len, u32 num, u32 ccfg)
>   {
> -	if ((num + 1) < ds->desc_num)
> +	if (num != ds->desc_num - 1)
>   		ds->desc_hw[num].lli = ds->desc_hw_lli + (num + 1) *
>   			sizeof(struct k3_desc_hw);
> +
>   	ds->desc_hw[num].lli |= CX_LLI_CHAIN_EN;
>   	ds->desc_hw[num].count = len;
>   	ds->desc_hw[num].saddr = src;
>   	ds->desc_hw[num].daddr = dst;
>   	ds->desc_hw[num].config = ccfg;
> +
> +	pr_debug("%s: k3_dma_desc_sw = %p, desc_hw = %p (num = %d) lli: 0x%x,"
> +		" count: 0x%x, saddr: 0x%x, daddr 0x%x, cfg: 0x%x\n", __func__,
> +		(void *)ds, &ds->desc_hw[num], num,
> +		ds->desc_hw[num].lli, ds->desc_hw[num].count,
> +		ds->desc_hw[num].saddr, ds->desc_hw[num].daddr,
> +		ds->desc_hw[num].config);
>   }
>
>   static struct dma_async_tx_descriptor *k3_dma_prep_memcpy(
> @@ -431,6 +462,7 @@ static struct dma_async_tx_descriptor *k3_dma_prep_memcpy(
>   		dev_dbg(chan->device->dev, "vchan %p: kzalloc fail\n", &c->vc);
>   		return NULL;
>   	}
> +	c->cyclic = 0;
>   	ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]);
>   	ds->size = len;
>   	ds->desc_num = num;
> @@ -476,17 +508,19 @@ static struct dma_async_tx_descriptor *k3_dma_prep_slave_sg(
>   	if (sgl == NULL)
>   		return NULL;
>
> +	c->cyclic = 0;
> +
>   	for_each_sg(sgl, sg, sglen, i) {
>   		avail = sg_dma_len(sg);
> +		pr_err("   avail=0x%x\n", (int)avail);

Do we need this print?

>   		if (avail > DMA_MAX_SIZE)
>   			num += DIV_ROUND_UP(avail, DMA_MAX_SIZE) - 1;
>   	}
>
>   	ds = kzalloc(sizeof(*ds) + num * sizeof(ds->desc_hw[0]), GFP_ATOMIC);
> -	if (!ds) {
> -		dev_dbg(chan->device->dev, "vchan %p: kzalloc fail\n", &c->vc);
> +	if (!ds)
>   		return NULL;
> -	}
> +
>   	ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]);
>   	ds->desc_num = num;
>   	num = 0;
> @@ -519,6 +553,77 @@ static struct dma_async_tx_descriptor *k3_dma_prep_slave_sg(
>   	return vchan_tx_prep(&c->vc, &ds->vd, flags);
>   }
>
> +static struct dma_async_tx_descriptor *
> +k3_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
> +		       size_t buf_len, size_t period_len,
> +		       enum dma_transfer_direction dir,
> +		       unsigned long flags)
> +{
> +	struct k3_dma_chan *c = to_k3_chan(chan);
> +	struct k3_dma_desc_sw *ds;
> +	size_t len, avail, total = 0;
> +	dma_addr_t addr, src = 0, dst = 0;
> +	int num = 1, since = 0;
> +	size_t modulo = DMA_CYCLIC_MAX_PERIOD;
> +	u32 en_tc2 = 0;
> +
> +	pr_debug("%s: buf %p, dst %p, buf len %d, period_len = %d, dir %d\n",
> +	       __func__, (void *)buf_addr, (void *)to_k3_chan(chan)->dev_addr,
> +	       (int)buf_len, (int)period_len, (int)dir);
> +
> +	avail = buf_len;
> +	if (avail > modulo)
> +		num += DIV_ROUND_UP(avail, modulo) - 1;
> +
> +	ds = kzalloc(sizeof(*ds) + num * sizeof(ds->desc_hw[0]), GFP_ATOMIC);
> +	if (!ds)
> +		return NULL;
> +
> +	ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]);
> +	ds->desc_num = num;
> +
> +	c->cyclic = 1;
> +	addr = buf_addr;
> +	avail = buf_len;
> +	total = avail;
> +	num = 0;
> +
> +	if (period_len < modulo)
> +		modulo = period_len;
> +
> +	do {
> +		len = min_t(size_t, avail, modulo);
> +
> +		if (dir == DMA_MEM_TO_DEV) {
> +			src = addr;
> +			dst = c->dev_addr;
> +		} else if (dir == DMA_DEV_TO_MEM) {
> +			src = c->dev_addr;
> +			dst = addr;
> +		}
> +		since += len;
> +		if (since >= period_len) {
> +			/* descriptor asks for TC2 interrupt on completion */
> +			en_tc2 = CX_CFG_NODEIRQ;
> +			since -= period_len;
> +		} else
> +			en_tc2 = 0;
> +
> +		k3_dma_fill_desc(ds, dst, src, len, num++, c->ccfg | en_tc2);
> +
> +		addr += len;
> +		avail -= len;
> +	} while (avail);
> +
> +	/* "Cyclic" == end of link points back to start of link */
> +	ds->desc_hw[num - 1].lli |= ds->desc_hw_lli;
> +
> +	ds->size = total;
> +
> +	return vchan_tx_prep(&c->vc, &ds->vd, flags);
> +}
> +
> +
>   static int k3_dma_config(struct dma_chan *chan,
>   			 struct dma_slave_config *cfg)
>   {
> @@ -723,11 +828,13 @@ static int k3_dma_probe(struct platform_device *op)
>   	INIT_LIST_HEAD(&d->slave.channels);
>   	dma_cap_set(DMA_SLAVE, d->slave.cap_mask);
>   	dma_cap_set(DMA_MEMCPY, d->slave.cap_mask);
> +	dma_cap_set(DMA_CYCLIC, d->slave.cap_mask);
>   	d->slave.dev = &op->dev;
>   	d->slave.device_free_chan_resources = k3_dma_free_chan_resources;
>   	d->slave.device_tx_status = k3_dma_tx_status;
>   	d->slave.device_prep_dma_memcpy = k3_dma_prep_memcpy;
>   	d->slave.device_prep_slave_sg = k3_dma_prep_slave_sg;
> +	d->slave.device_prep_dma_cyclic = k3_dma_prep_dma_cyclic;
>   	d->slave.device_issue_pending = k3_dma_issue_pending;
>   	d->slave.device_config = k3_dma_config;
>   	d->slave.device_pause = k3_dma_transfer_pause;
>

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

* Re: [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support
  2016-07-16 11:48   ` Mark Brown
@ 2016-07-18 17:20     ` John Stultz
  0 siblings, 0 replies; 21+ messages in thread
From: John Stultz @ 2016-07-18 17:20 UTC (permalink / raw)
  To: Mark Brown
  Cc: lkml, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu

On Sat, Jul 16, 2016 at 4:48 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 15, 2016 at 07:13:27PM -0700, John Stultz wrote:
>
>> Add entry for k3-dma driver and i2s/hdmi audio devices.
>
>> This enables HDMI audio output.
>
> These bindings appear to be undocumented.  All new bindings require
> documentation.

Yea. I figured there would be potentially substantial enough changes
required that doing a initial draft (which given my limited knowledge
of the hardware, would be mostly boilerplate) might not be worth it
for the first RFC.

>> +             i2s0: hi6210_i2s {
>> +                     compatible = "hisilicon,hi6210-i2s";
>> +                     reg = <0x0 0xf7118000 0x0 0x8000>, /* i2s unit */
>> +                           <0x0 0xf7030000 0x0 0x400>,  /* syscon */
>> +                           <0x0 0xf7032000 0x0 0x400>;  /* pmctrl */
>
> Some of this looks like we should be using a system controller binding
> rather than listing the system controller directly here.  The magic
> number indexing for the resources is also not good, we should be using
> reg-names to get name based lookups (I had been going to query that on
> the binding document when I failed to find it...).

Ok. I'll need to dig a bit to understand it better, but will try to
address this.

thanks
-john

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

* Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio
  2016-07-16 11:44   ` Mark Brown
@ 2016-07-19 21:59     ` John Stultz
  2016-07-20  0:21       ` Mark Brown
  0 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2016-07-19 21:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: lkml, Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu

On Sat, Jul 16, 2016 at 4:44 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 15, 2016 at 07:13:26PM -0700, John Stultz wrote:
>>
>> +     .cpu_dai_name = "f7118000.hi6210_i2s",
>
> Why is this not connected up in DT - as things stand the card has zero
> reuse potential.  Though given that this has
>
>> +     .codec_name = "0.hi6210_hdmi_card",
>
> If you've got a CODEC with hdmi_card in the name that suggests there's a
> serious abstraction problem going on somewhere.

Yea. So I've been splitting up the patch and looking at how this might
be wired up via devicetree instead.

I've basically stripped the hdmi-card driver down and it really just
seems to be:

static struct snd_soc_dai_driver hi6210_hdmi_dai = {
        .name = "hi6210_hdmi_dai",
        .playback = {
                .channels_min = 2,
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_48000,
                .formats = SNDRV_PCM_FMTBIT_S16_LE,
        },
};

Then the probe/remove logic to register the codec, and connected it up
with the simple card driver.

Though it seems kind of silly to have a whole driver (and devicetree
entry for the probe hook) just to fill and install the above
structure. Is there a simpler way to specify that via a DT node
instead?


>> +static const struct of_device_id hi6210_i2s_dt_ids[] = {
>> +     { .compatible = "hisilicon,hi6210-i2s" },
>> +     { /* sentinel */ }
>> +};
>
> The code makes this look like it's not just an I2S controller.

I'm not sure I follow this? The dt ids make it seem like this driver
is not an i2s controller?

I think I've addressed the other issue you've pointed out in this
patch, so I'll likely be sending out another revision later today.

thanks
-john

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

* Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio
  2016-07-19 21:59     ` John Stultz
@ 2016-07-20  0:21       ` Mark Brown
  0 siblings, 0 replies; 21+ messages in thread
From: Mark Brown @ 2016-07-20  0:21 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Andy Green, Zhangfei Gao, Jingoo Han, Krzysztof Kozlowski,
	Maxime Ripard, Vinod Koul, Dan Williams, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, Wei Xu, Rob Herring, Andy Green,
	Dave Long, Guodong Xu

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

On Tue, Jul 19, 2016 at 02:59:31PM -0700, John Stultz wrote:

> Then the probe/remove logic to register the codec, and connected it up
> with the simple card driver.

> Though it seems kind of silly to have a whole driver (and devicetree
> entry for the probe hook) just to fill and install the above
> structure. Is there a simpler way to specify that via a DT node
> instead?

Well, if you can use simple-card why not just use simple-card?

> >> +static const struct of_device_id hi6210_i2s_dt_ids[] = {
> >> +     { .compatible = "hisilicon,hi6210-i2s" },
> >> +     { /* sentinel */ }
> >> +};

> > The code makes this look like it's not just an I2S controller.

> I'm not sure I follow this? The dt ids make it seem like this driver
> is not an i2s controller?

No.  The *code* makes this look like it's not an I2S controller.  It
looks like it's a driver for a DSP/mixing block which may have an
integrated I2S controller, I pointed out things like some of the routing
control.

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

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

end of thread, other threads:[~2016-07-20  0:21 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-16  2:13 [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz
2016-07-16  2:13 ` [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping John Stultz
2016-07-18  6:37   ` zhangfei
2016-07-16  2:13 ` [RFC][PATCH 2/7] k3dma: Fix dma err offsets John Stultz
2016-07-18  6:39   ` zhangfei
2016-07-16  2:13 ` [RFC][PATCH 3/7] k3dma: Fix "nobody cared" message seen on any error John Stultz
2016-07-18  6:40   ` zhangfei
2016-07-16  2:13 ` [RFC][PATCH 4/7] k3dma: Add cyclic mode for audio John Stultz
2016-07-18  6:43   ` zhangfei
2016-07-16  2:13 ` [RFC][PATCH 5/7] Kconfig: Allow k3dma driver to be selected for more then HISI3xx platforms John Stultz
2016-07-16 11:18   ` Mark Brown
2016-07-16  2:13 ` [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio John Stultz
2016-07-16 11:44   ` Mark Brown
2016-07-19 21:59     ` John Stultz
2016-07-20  0:21       ` Mark Brown
2016-07-16  2:13 ` [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support John Stultz
2016-07-16 11:48   ` Mark Brown
2016-07-18 17:20     ` John Stultz
2016-07-16  3:15 ` [RFC][PATCH 0/7] Add HDMI audio support for HiKey Andy Green
2016-07-16  3:38   ` John Stultz
2016-07-16 11:12     ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.