[PATCH 0/2] rockchip-i2s: add power setting for I2S controller and fix some critical bugs

[PATCH 0/2] rockchip-i2s: add power setting for I2S controller and fix some critical bugs

[Jianqun]

Add optional power setting for i2s controller found on rk3066, rk3168 and rk3288
processors from rockchip, should according to hardware design.

Default setting for I2S controller is powered by 3.3V, there needs this patch if
it's powered by 1.8V by hardware design.

Jianqun (2):
  rockchip-i2s: dt: add grf requested properties to set power of I2S controller
  rockchip-i2s: add power setting for I2S controller, also fix some bugs

 .../devicetree/bindings/sound/rockchip-i2s.txt     |  6 +-
 sound/soc/rockchip/rockchip_i2s.c                  | 93 +++++++++++++---------
 sound/soc/rockchip/rockchip_i2s.h                  | 13 +--
 3 files changed, 68 insertions(+), 44 deletions(-)

-- 
1.8.3.2

[PATCH 1/2] rockchip-i2s: dt: add grf requested properties to set power of I2S controller

[Jianqun]

Add "rockchip,grf" for driver to get physical address of GRF, and "rockchip,grf-io-vsel"
for driver to set voltage for I2S controller according to hardware request.

Requested by rk3xxx I2S controller and tested ok on rk3288-pinky board.

Change-Id: I2587d15c25e64c569857369326653d8a450bae19
Signed-off-by: Jianqun <jay.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
 Documentation/devicetree/bindings/sound/rockchip-i2s.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
index 6c55fcf..b995fe6 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
@@ -9,6 +9,8 @@ Required properties:
    - "rockchip,rk3066-i2s": for rk3066
    - "rockchip,rk3188-i2s", "rockchip,rk3066-i2s": for rk3188
    - "rockchip,rk3288-i2s", "rockchip,rk3066-i2s": for rk3288
+- rockchip,grf: physical base address of GRF.
+- rockchip,grf-io-vsel: select this if I2S controller powerd by 1.8v.
 - reg: physical base address of the controller and length of memory mapped
   region.
 - interrupts: should contain the I2S interrupt.
@@ -26,12 +28,14 @@ Example for rk3288 I2S controller:
 
 i2s@ff890000 {
 	compatible = "rockchip,rk3288-i2s", "rockchip,rk3066-i2s";
+	rockchip,grf = <&grf>;
+	rockchip,grf-io-vsel;
 	reg = <0xff890000 0x10000>;
 	interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
 	#address-cells = <1>;
 	#size-cells = <0>;
 	dmas = <&pdma1 0>, <&pdma1 1>;
-	dma-names = "rx", "tx";
+	dma-names = "tx", "rx";
 	clock-names = "i2s_hclk", "i2s_clk";
 	clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>;
 };
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] rockchip-i2s: add power setting for I2S controller, also fix some bugs

[Jianqun]

changes:
* add snd_soc_dai_init_dma_data
* fix duplicated argument to "I2S_DMACR_TDE_DISABLE"
* set 1.8v or 3.3v power for I2S controller by GRF interface
* enable "hclk" always
* dma maxburst change to 16

Requested on RK3XXX I2S controllers, and tested ok on rk3288-pinky board.

Change-Id: If17b8022a38c2974f32bfb2dd4b8d16644ec57ac
Signed-off-by: Jianqun <jay.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
 sound/soc/rockchip/rockchip_i2s.c | 93 +++++++++++++++++++++++----------------
 sound/soc/rockchip/rockchip_i2s.h | 13 +++---
 2 files changed, 63 insertions(+), 43 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 8d8e4b5..e07bdbd 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -10,14 +10,15 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/module.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
 #include <linux/of_gpio.h>
-#include <linux/clk.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
-#include <sound/pcm_params.h>
 #include <sound/dmaengine_pcm.h>
+#include <sound/pcm_params.h>
 
 #include "rockchip_i2s.h"
 
@@ -25,6 +26,7 @@
 
 struct rk_i2s_dev {
 	struct device *dev;
+	struct regmap *grf;
 
 	struct clk *hclk;
 	struct clk *mclk;
@@ -66,11 +68,6 @@ static int i2s_runtime_resume(struct device *dev)
 	return 0;
 }
 
-static inline struct rk_i2s_dev *to_info(struct snd_soc_dai *dai)
-{
-	return snd_soc_dai_get_drvdata(dai);
-}
-
 static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on)
 {
 	unsigned int val = 0;
@@ -79,7 +76,6 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on)
 	if (on) {
 		regmap_update_bits(i2s->regmap, I2S_DMACR,
 				   I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
-
 		regmap_update_bits(i2s->regmap, I2S_XFER,
 				   I2S_XFER_TXS_START | I2S_XFER_RXS_START,
 				   I2S_XFER_TXS_START | I2S_XFER_RXS_START);
@@ -159,19 +155,20 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
 	}
 }
 
-static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
+static int rockchip_i2s_set_fmt(struct snd_soc_dai *dai,
 				unsigned int fmt)
 {
-	struct rk_i2s_dev *i2s = to_info(cpu_dai);
+	struct rk_i2s_dev *i2s = snd_soc_dai_get_drvdata(dai);
 	unsigned int mask = 0, val = 0;
 
-	mask = I2S_CKR_MSS_SLAVE;
+	mask = I2S_CKR_MSS_MASK;
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
-		val = I2S_CKR_MSS_SLAVE;
+		/* Set default source clock in Master mode */
+		val = I2S_CKR_MSS_MASTER;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
-		val = I2S_CKR_MSS_MASTER;
+		val = I2S_CKR_MSS_SLAVE;
 		break;
 	default:
 		return -EINVAL;
@@ -220,7 +217,7 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 				  struct snd_pcm_hw_params *params,
 				  struct snd_soc_dai *dai)
 {
-	struct rk_i2s_dev *i2s = to_info(dai);
+	struct rk_i2s_dev *i2s = snd_soc_dai_get_drvdata(dai);
 	unsigned int val = 0;
 
 	switch (params_format(params)) {
@@ -243,24 +240,13 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 	regmap_update_bits(i2s->regmap, I2S_TXCR, I2S_TXCR_VDW_MASK, val);
 	regmap_update_bits(i2s->regmap, I2S_RXCR, I2S_RXCR_VDW_MASK, val);
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		dai->playback_dma_data = &i2s->playback_dma_data;
-		regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDL_MASK,
-				   I2S_DMACR_TDL(1) | I2S_DMACR_TDE_ENABLE);
-	} else {
-		dai->capture_dma_data = &i2s->capture_dma_data;
-		regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK,
-				   I2S_DMACR_RDL(1) | I2S_DMACR_RDE_ENABLE);
-	}
-
 	return 0;
 }
 
 static int rockchip_i2s_trigger(struct snd_pcm_substream *substream,
 				int cmd, struct snd_soc_dai *dai)
 {
-	struct rk_i2s_dev *i2s = to_info(dai);
-	int ret = 0;
+	struct rk_i2s_dev *i2s = snd_soc_dai_get_drvdata(dai);
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -280,17 +266,16 @@ static int rockchip_i2s_trigger(struct snd_pcm_substream *substream,
 			rockchip_snd_txctrl(i2s, 0);
 		break;
 	default:
-		ret = -EINVAL;
-		break;
+		return -EINVAL;
 	}
 
-	return ret;
+	return 0;
 }
 
-static int rockchip_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id,
+static int rockchip_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
 				   unsigned int freq, int dir)
 {
-	struct rk_i2s_dev *i2s = to_info(cpu_dai);
+	struct rk_i2s_dev *i2s = snd_soc_dai_get_drvdata(dai);
 	int ret;
 
 	ret = clk_set_rate(i2s->mclk, freq);
@@ -300,6 +285,16 @@ static int rockchip_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id,
 	return ret;
 }
 
+static int rockchip_i2s_dai_probe(struct snd_soc_dai *dai)
+{
+	struct rk_i2s_dev *i2s = snd_soc_dai_get_drvdata(dai);
+
+	dai->capture_dma_data = &i2s->capture_dma_data;
+	dai->playback_dma_data = &i2s->playback_dma_data;
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops rockchip_i2s_dai_ops = {
 	.hw_params = rockchip_i2s_hw_params,
 	.set_sysclk = rockchip_i2s_set_sysclk,
@@ -308,7 +303,9 @@ static const struct snd_soc_dai_ops rockchip_i2s_dai_ops = {
 };
 
 static struct snd_soc_dai_driver rockchip_i2s_dai = {
+	.probe = rockchip_i2s_dai_probe,
 	.playback = {
+		.stream_name = "Playback",
 		.channels_min = 2,
 		.channels_max = 8,
 		.rates = SNDRV_PCM_RATE_8000_192000,
@@ -318,6 +315,7 @@ static struct snd_soc_dai_driver rockchip_i2s_dai = {
 			    SNDRV_PCM_FMTBIT_S24_LE),
 	},
 	.capture = {
+		.stream_name = "Capture",
 		.channels_min = 2,
 		.channels_max = 2,
 		.rates = SNDRV_PCM_RATE_8000_192000,
@@ -361,6 +359,8 @@ static bool rockchip_i2s_rd_reg(struct device *dev, unsigned int reg)
 	case I2S_XFER:
 	case I2S_CLR:
 	case I2S_RXDR:
+	case I2S_FIFOLR:
+	case I2S_INTSR:
 		return true;
 	default:
 		return false;
@@ -370,8 +370,8 @@ static bool rockchip_i2s_rd_reg(struct device *dev, unsigned int reg)
 static bool rockchip_i2s_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
-	case I2S_FIFOLR:
 	case I2S_INTSR:
+	case I2S_CLR:
 		return true;
 	default:
 		return false;
@@ -381,8 +381,6 @@ static bool rockchip_i2s_volatile_reg(struct device *dev, unsigned int reg)
 static bool rockchip_i2s_precious_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
-	case I2S_FIFOLR:
-		return true;
 	default:
 		return false;
 	}
@@ -402,15 +400,20 @@ static const struct regmap_config rockchip_i2s_regmap_config = {
 
 static int rockchip_i2s_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct rk_i2s_dev *i2s;
 	struct resource *res;
 	void __iomem *regs;
 	int ret;
 
 	i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
-	if (!i2s) {
-		dev_err(&pdev->dev, "Can't allocate rk_i2s_dev\n");
+	if (!i2s)
 		return -ENOMEM;
+
+	i2s->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+	if (IS_ERR(i2s->grf)) {
+		dev_err(&pdev->dev, "Can't retrieve grf property\n");
+		return PTR_ERR(i2s->grf);
 	}
 
 	/* try to prepare related clocks */
@@ -419,6 +422,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Can't retrieve i2s bus clock\n");
 		return PTR_ERR(i2s->hclk);
 	}
+	ret = clk_prepare_enable(i2s->hclk);
+	if (ret) {
+		dev_err(i2s->dev, "hclock enable failed %d\n", ret);
+		return ret;
+	}
 
 	i2s->mclk = devm_clk_get(&pdev->dev, "i2s_clk");
 	if (IS_ERR(i2s->mclk)) {
@@ -450,6 +458,17 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
 	i2s->dev = &pdev->dev;
 	dev_set_drvdata(&pdev->dev, i2s);
 
+	if (of_property_read_bool(np, "rockchip,grf-io-vsel"))
+		ret = regmap_write(i2s->grf, GRF_IO_VSEL,
+				   BIT(6) << 16 | BIT(6));
+	else
+		ret = regmap_write(i2s->grf, GRF_IO_VSEL,
+				   BIT(6) << 16);
+	if (ret) {
+		dev_err(i2s->dev, "Could not write to GRF: %d\n", ret);
+		return ret;
+	}
+
 	pm_runtime_enable(&pdev->dev);
 	if (!pm_runtime_enabled(&pdev->dev)) {
 		ret = i2s_runtime_resume(&pdev->dev);
diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h
index 89a5d8b..8f9a589 100644
--- a/sound/soc/rockchip/rockchip_i2s.h
+++ b/sound/soc/rockchip/rockchip_i2s.h
@@ -201,12 +201,6 @@
 */
 #define I2S_RXDR_MASK	(0xff)
 
-/* Clock divider id */
-enum {
-	ROCKCHIP_DIV_MCLK = 0,
-	ROCKCHIP_DIV_BCLK,
-};
-
 /* I2S REGS */
 #define I2S_TXCR	(0x0000)
 #define I2S_RXCR	(0x0004)
@@ -220,4 +214,11 @@ enum {
 #define I2S_TXDR	(0x0024)
 #define I2S_RXDR	(0x0028)
 
+/*
+ * IO voltage select
+ * 1: I2S controller powerd by 1.8v
+ * 0: I2S controller powerd by 3.3v (default)
+*/
+#define GRF_IO_VSEL (0x380)
+
 #endif /* _ROCKCHIP_IIS_H */
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] rockchip-i2s: add power setting for I2S controller, also fix some bugs

[Mark Brown]

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

On Fri, Aug 29, 2014 at 03:09:56PM -0700, Jianqun wrote:
> changes:
> * add snd_soc_dai_init_dma_data
> * fix duplicated argument to "I2S_DMACR_TDE_DISABLE"
> * set 1.8v or 3.3v power for I2S controller by GRF interface
> * enable "hclk" always
> * dma maxburst change to 16
> 
> Requested on RK3XXX I2S controllers, and tested ok on rk3288-pinky board.

Please don't submit multiple changes in one patch unless there's a
strong, specific reason to do so.  It makes it very hard to review the
change if there's lots of different things going on (in this case
there's also some coding style updates that aren't mentioned in the
changelog which doesn't help either).  Split the changes out into
multiple patches instead.

> Change-Id: If17b8022a38c2974f32bfb2dd4b8d16644ec57ac

Don't include noise from your internal review system in upstream
commits.

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

Re: [PATCH 0/2] rockchip-i2s: add power setting for I2S controller and fix some critical bugs

[Doug Anderson]

Jianqun,

On Fri, Aug 29, 2014 at 3:07 PM, Jianqun <jay.xu@rock-chips.com> wrote:
> Add optional power setting for i2s controller found on rk3066, rk3168 and rk3288
> processors from rockchip, should according to hardware design.
>
> Default setting for I2S controller is powered by 3.3V, there needs this patch if
> it's powered by 1.8V by hardware design.
>
> Jianqun (2):
>   rockchip-i2s: dt: add grf requested properties to set power of I2S controller
>   rockchip-i2s: add power setting for I2S controller, also fix some bugs
>
>  .../devicetree/bindings/sound/rockchip-i2s.txt     |  6 +-
>  sound/soc/rockchip/rockchip_i2s.c                  | 93 +++++++++++++---------
>  sound/soc/rockchip/rockchip_i2s.h                  | 13 +--
>  3 files changed, 68 insertions(+), 44 deletions(-)

Did the general solution I posted at
<https://patchwork.kernel.org/patch/4807821/> not work for you?  ...or
did you not see that?

-Doug

Re: [PATCH 0/2] rockchip-i2s: add power setting for I2S controller and fix some critical bugs

[Heiko Stübner]

Am Freitag, 29. August 2014, 21:30:43 schrieb Doug Anderson:
> Jianqun,
> 
> On Fri, Aug 29, 2014 at 3:07 PM, Jianqun <jay.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> > Add optional power setting for i2s controller found on rk3066, rk3168 and
> > rk3288 processors from rockchip, should according to hardware design.
> > 
> > Default setting for I2S controller is powered by 3.3V, there needs this
> > patch if it's powered by 1.8V by hardware design.
> > 
> > Jianqun (2):
> >   rockchip-i2s: dt: add grf requested properties to set power of I2S
> >   controller rockchip-i2s: add power setting for I2S controller, also fix
> >   some bugs>  
> >  .../devicetree/bindings/sound/rockchip-i2s.txt     |  6 +-
> >  sound/soc/rockchip/rockchip_i2s.c                  | 93
> >  +++++++++++++--------- sound/soc/rockchip/rockchip_i2s.h                
> >   | 13 +--
> >  3 files changed, 68 insertions(+), 44 deletions(-)
> 
> Did the general solution I posted at
> <https://patchwork.kernel.org/patch/4807821/> not work for you?  ...or
> did you not see that?

Also, I don't think it's a good idea to set the io voltage bit simply from a 
dt setting without looking at the actual regulator voltage ... as a 1.8V 
setting when the regulator supplies 3.3V supposedly is able to damage the 
chip.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html