* [PATCH 0/5] ASoC: rockchip-i2s: patches for rockchip i2s driver
@ 2014-09-13 0:38 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:38 UTC (permalink / raw)
To: heiko, lgirdwood, broonie, perex, tiwai, linux-arm-kernel,
linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf, Jianqun
These patches to fix rockchip i2s driver bugs, also make driver codes
reasonable.
Tested on RK3288 board.
Jianqun (5):
ASoC: rockchip-i2s: fix rockchip i2s defination more reasonable
ASoC: rockchip-i2s: fix master mode set bit error
ASoC: rockchip-i2s: add dma data to snd_soc_dai
ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
sound/soc/rockchip/Kconfig | 3 +--
sound/soc/rockchip/Makefile | 2 +-
sound/soc/rockchip/rockchip_i2s.c | 41 ++++++++++++++++++++++++---------------
3 files changed, 27 insertions(+), 19 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 0/5] ASoC: rockchip-i2s: patches for rockchip i2s driver
@ 2014-09-13 0:38 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:38 UTC (permalink / raw)
To: linux-arm-kernel
These patches to fix rockchip i2s driver bugs, also make driver codes
reasonable.
Tested on RK3288 board.
Jianqun (5):
ASoC: rockchip-i2s: fix rockchip i2s defination more reasonable
ASoC: rockchip-i2s: fix master mode set bit error
ASoC: rockchip-i2s: add dma data to snd_soc_dai
ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
sound/soc/rockchip/Kconfig | 3 +--
sound/soc/rockchip/Makefile | 2 +-
sound/soc/rockchip/rockchip_i2s.c | 41 ++++++++++++++++++++++++---------------
3 files changed, 27 insertions(+), 19 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/5] ASoC: rockchip-i2s: fix rockchip i2s defination more reasonable
2014-09-13 0:38 ` Jianqun
@ 2014-09-13 0:40 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:40 UTC (permalink / raw)
To: heiko, lgirdwood, broonie, perex, tiwai, linux-arm-kernel,
linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf, Jianqun
Fix SND_ROCKCHIP_I2S to be more reasonable - SND_SOC_ROCKCHIP_I2S,
SND_SOC_ROCKCHIP_I2S should select by audio driver, instead of
SND_SOC_ROCKCHIP.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/Kconfig | 3 +--
sound/soc/rockchip/Makefile | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index c196a46..78fc159 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -2,11 +2,10 @@ config SND_SOC_ROCKCHIP
tristate "ASoC support for Rockchip"
depends on COMPILE_TEST || ARCH_ROCKCHIP
select SND_SOC_GENERIC_DMAENGINE_PCM
- select SND_ROCKCHIP_I2S
help
Say Y or M if you want to add support for codecs attached to
the Rockchip SoCs' Audio interfaces. You will also need to
select the audio interfaces to support below.
-config SND_ROCKCHIP_I2S
+config SND_SOC_ROCKCHIP_I2S
tristate
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index 1006418..b921909 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -1,4 +1,4 @@
# ROCKCHIP Platform Support
snd-soc-i2s-objs := rockchip_i2s.o
-obj-$(CONFIG_SND_ROCKCHIP_I2S) += snd-soc-i2s.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-i2s.o
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 1/5] ASoC: rockchip-i2s: fix rockchip i2s defination more reasonable
@ 2014-09-13 0:40 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:40 UTC (permalink / raw)
To: linux-arm-kernel
Fix SND_ROCKCHIP_I2S to be more reasonable - SND_SOC_ROCKCHIP_I2S,
SND_SOC_ROCKCHIP_I2S should select by audio driver, instead of
SND_SOC_ROCKCHIP.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/Kconfig | 3 +--
sound/soc/rockchip/Makefile | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index c196a46..78fc159 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -2,11 +2,10 @@ config SND_SOC_ROCKCHIP
tristate "ASoC support for Rockchip"
depends on COMPILE_TEST || ARCH_ROCKCHIP
select SND_SOC_GENERIC_DMAENGINE_PCM
- select SND_ROCKCHIP_I2S
help
Say Y or M if you want to add support for codecs attached to
the Rockchip SoCs' Audio interfaces. You will also need to
select the audio interfaces to support below.
-config SND_ROCKCHIP_I2S
+config SND_SOC_ROCKCHIP_I2S
tristate
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index 1006418..b921909 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -1,4 +1,4 @@
# ROCKCHIP Platform Support
snd-soc-i2s-objs := rockchip_i2s.o
-obj-$(CONFIG_SND_ROCKCHIP_I2S) += snd-soc-i2s.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-i2s.o
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
2014-09-13 0:38 ` Jianqun
@ 2014-09-13 0:41 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:41 UTC (permalink / raw)
To: heiko, lgirdwood, broonie, perex, tiwai, linux-arm-kernel,
linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf, Jianqun
Fix error format set to I2S master or slave mode.
Test on RK3288 board with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 8d8e4b5..870a664 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -165,13 +165,14 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
struct rk_i2s_dev *i2s = to_info(cpu_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 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;
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
@ 2014-09-13 0:41 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:41 UTC (permalink / raw)
To: linux-arm-kernel
Fix error format set to I2S master or slave mode.
Test on RK3288 board with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 8d8e4b5..870a664 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -165,13 +165,14 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
struct rk_i2s_dev *i2s = to_info(cpu_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 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;
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 3/5] ASoC: rockchip-i2s: add dma data to snd_soc_dai
2014-09-13 0:38 ` Jianqun
@ 2014-09-13 0:41 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:41 UTC (permalink / raw)
To: heiko, lgirdwood, broonie, perex, tiwai, linux-arm-kernel,
linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf, Jianqun
Add playback/capture dma data to snd_soc_dai.
Test on RK3288 with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 870a664..1b9b404 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -244,16 +244,6 @@ 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;
}
@@ -301,6 +291,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,
@@ -309,7 +309,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,
@@ -319,6 +321,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,
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 3/5] ASoC: rockchip-i2s: add dma data to snd_soc_dai
@ 2014-09-13 0:41 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:41 UTC (permalink / raw)
To: linux-arm-kernel
Add playback/capture dma data to snd_soc_dai.
Test on RK3288 with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 870a664..1b9b404 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -244,16 +244,6 @@ 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;
}
@@ -301,6 +291,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,
@@ -309,7 +309,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,
@@ -319,6 +321,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,
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
2014-09-13 0:38 ` Jianqun
@ 2014-09-13 0:42 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:42 UTC (permalink / raw)
To: heiko, lgirdwood, broonie, perex, tiwai, linux-arm-kernel,
linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf, Jianqun
Reference rockchip I2S controller TRM, modify some registers' property
I2S_FIFOLR: read / write, but not volatile, not precious
I2S_INTSR: read / write
I2S_CLR: volatile, register value will be cleared by read
Test on RK3288 with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 1b9b404..6595383 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -365,6 +365,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;
@@ -374,8 +376,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;
@@ -385,8 +387,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;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
@ 2014-09-13 0:42 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:42 UTC (permalink / raw)
To: linux-arm-kernel
Reference rockchip I2S controller TRM, modify some registers' property
I2S_FIFOLR: read / write, but not volatile, not precious
I2S_INTSR: read / write
I2S_CLR: volatile, register value will be cleared by read
Test on RK3288 with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 1b9b404..6595383 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -365,6 +365,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;
@@ -374,8 +376,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;
@@ -385,8 +387,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;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
2014-09-13 0:38 ` Jianqun
@ 2014-09-13 0:43 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:43 UTC (permalink / raw)
To: heiko, lgirdwood, broonie, perex, tiwai, linux-arm-kernel,
linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf, Jianqun
As "hclk" is used for rockchip I2S controller, driver must to enable
it in probe.
Tested on RK3288 with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 6595383..033487c 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -423,6 +423,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)) {
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
@ 2014-09-13 0:43 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-13 0:43 UTC (permalink / raw)
To: linux-arm-kernel
As "hclk" is used for rockchip I2S controller, driver must to enable
it in probe.
Tested on RK3288 with max98090.
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
sound/soc/rockchip/rockchip_i2s.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 6595383..033487c 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -423,6 +423,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)) {
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
2014-09-13 0:41 ` Jianqun
(?)
@ 2014-09-13 16:35 ` Mark Brown
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:35 UTC (permalink / raw)
To: Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
[-- Attachment #1: Type: text/plain, Size: 322 bytes --]
On Sat, Sep 13, 2014 at 08:41:03AM +0800, Jianqun wrote:
> Fix error format set to I2S master or slave mode.
> Test on RK3288 board with max98090.
Applied. Since this is a bug fix it should be one of the first patches
in the series so that it can be sent to Linus as a fix, bug fixes should
go before any other patches.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
@ 2014-09-13 16:35 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:35 UTC (permalink / raw)
To: Jianqun
Cc: huangtao, alsa-devel, heiko, tiwai, linux-kernel, lgirdwood,
linux-rockchip, cf, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 322 bytes --]
On Sat, Sep 13, 2014 at 08:41:03AM +0800, Jianqun wrote:
> Fix error format set to I2S master or slave mode.
> Test on RK3288 board with max98090.
Applied. Since this is a bug fix it should be one of the first patches
in the series so that it can be sent to Linus as a fix, bug fixes should
go before any other patches.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
@ 2014-09-13 16:35 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:35 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 08:41:03AM +0800, Jianqun wrote:
> Fix error format set to I2S master or slave mode.
> Test on RK3288 board with max98090.
Applied. Since this is a bug fix it should be one of the first patches
in the series so that it can be sent to Linus as a fix, bug fixes should
go before any other patches.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140913/00a92d24/attachment.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
2014-09-13 0:42 ` Jianqun
@ 2014-09-13 16:36 ` Mark Brown
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:36 UTC (permalink / raw)
To: Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
[-- Attachment #1: Type: text/plain, Size: 403 bytes --]
On Sat, Sep 13, 2014 at 08:42:12AM +0800, Jianqun wrote:
> Reference rockchip I2S controller TRM, modify some registers' property
> I2S_FIFOLR: read / write, but not volatile, not precious
> I2S_INTSR: read / write
> I2S_CLR: volatile, register value will be cleared by read
Applied, again this is a bug fix (volatile and precious being wrong seem
like bugs) so should have been earlier in the series.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
@ 2014-09-13 16:36 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:36 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 08:42:12AM +0800, Jianqun wrote:
> Reference rockchip I2S controller TRM, modify some registers' property
> I2S_FIFOLR: read / write, but not volatile, not precious
> I2S_INTSR: read / write
> I2S_CLR: volatile, register value will be cleared by read
Applied, again this is a bug fix (volatile and precious being wrong seem
like bugs) so should have been earlier in the series.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140913/cc829331/attachment-0001.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
2014-09-13 0:43 ` Jianqun
@ 2014-09-13 16:36 ` Mark Brown
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:36 UTC (permalink / raw)
To: Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
[-- Attachment #1: Type: text/plain, Size: 223 bytes --]
On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
> As "hclk" is used for rockchip I2S controller, driver must to enable
> it in probe.
Applied, again this is a bug fix. How did the original submission get
tested?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
@ 2014-09-13 16:36 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:36 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
> As "hclk" is used for rockchip I2S controller, driver must to enable
> it in probe.
Applied, again this is a bug fix. How did the original submission get
tested?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140913/1b8bb26c/attachment.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
2014-09-13 0:43 ` Jianqun
@ 2014-09-13 16:37 ` Mark Brown
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:37 UTC (permalink / raw)
To: Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
[-- Attachment #1: Type: text/plain, Size: 531 bytes --]
On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -423,6 +423,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;
> + }
BTW: you're also missing a clk_disable_unprepare() in the remove path
here, please send a followup fixing that.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
@ 2014-09-13 16:37 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:37 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -423,6 +423,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;
> + }
BTW: you're also missing a clk_disable_unprepare() in the remove path
here, please send a followup fixing that.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140913/2476a185/attachment.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/5] ASoC: rockchip-i2s: fix rockchip i2s defination more reasonable
2014-09-13 0:40 ` Jianqun
@ 2014-09-13 16:38 ` Mark Brown
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:38 UTC (permalink / raw)
To: Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
[-- Attachment #1: Type: text/plain, Size: 229 bytes --]
On Sat, Sep 13, 2014 at 08:40:19AM +0800, Jianqun wrote:
> Fix SND_ROCKCHIP_I2S to be more reasonable - SND_SOC_ROCKCHIP_I2S,
> SND_SOC_ROCKCHIP_I2S should select by audio driver, instead of
> SND_SOC_ROCKCHIP.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/5] ASoC: rockchip-i2s: fix rockchip i2s defination more reasonable
@ 2014-09-13 16:38 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:38 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 08:40:19AM +0800, Jianqun wrote:
> Fix SND_ROCKCHIP_I2S to be more reasonable - SND_SOC_ROCKCHIP_I2S,
> SND_SOC_ROCKCHIP_I2S should select by audio driver, instead of
> SND_SOC_ROCKCHIP.
Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140913/66e7aad2/attachment.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 3/5] ASoC: rockchip-i2s: add dma data to snd_soc_dai
2014-09-13 0:41 ` Jianqun
@ 2014-09-13 16:38 ` Mark Brown
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:38 UTC (permalink / raw)
To: Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
[-- Attachment #1: Type: text/plain, Size: 155 bytes --]
On Sat, Sep 13, 2014 at 08:41:38AM +0800, Jianqun wrote:
> Add playback/capture dma data to snd_soc_dai.
> Test on RK3288 with max98090.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 3/5] ASoC: rockchip-i2s: add dma data to snd_soc_dai
@ 2014-09-13 16:38 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-13 16:38 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 08:41:38AM +0800, Jianqun wrote:
> Add playback/capture dma data to snd_soc_dai.
> Test on RK3288 with max98090.
Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140913/1a44f92c/attachment.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
2014-09-13 0:42 ` Jianqun
@ 2014-09-13 20:57 ` Sergei Shtylyov
-1 siblings, 0 replies; 42+ messages in thread
From: Sergei Shtylyov @ 2014-09-13 20:57 UTC (permalink / raw)
To: Jianqun, heiko, lgirdwood, broonie, perex, tiwai,
linux-arm-kernel, linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf
Hello.
On 9/13/2014 3:42 AM, Jianqun wrote:
> Reference rockchip I2S controller TRM, modify some registers' property
> I2S_FIFOLR: read / write, but not volatile, not precious
> I2S_INTSR: read / write
> I2S_CLR: volatile, register value will be cleared by read
> Test on RK3288 with max98090.
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
> sound/soc/rockchip/rockchip_i2s.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 1b9b404..6595383 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
[...]
> @@ -385,8 +387,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;
> }
Shouldn't this be folded into simple *return* false now?
WBR, Sergei
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
@ 2014-09-13 20:57 ` Sergei Shtylyov
0 siblings, 0 replies; 42+ messages in thread
From: Sergei Shtylyov @ 2014-09-13 20:57 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 9/13/2014 3:42 AM, Jianqun wrote:
> Reference rockchip I2S controller TRM, modify some registers' property
> I2S_FIFOLR: read / write, but not volatile, not precious
> I2S_INTSR: read / write
> I2S_CLR: volatile, register value will be cleared by read
> Test on RK3288 with max98090.
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
> sound/soc/rockchip/rockchip_i2s.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 1b9b404..6595383 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
[...]
> @@ -385,8 +387,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;
> }
Shouldn't this be folded into simple *return* false now?
WBR, Sergei
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
2014-09-13 16:35 ` Mark Brown
(?)
@ 2014-09-14 2:06 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:06 UTC (permalink / raw)
To: Mark Brown, Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
在 09/14/2014 12:35 AM, Mark Brown 写道:
> On Sat, Sep 13, 2014 at 08:41:03AM +0800, Jianqun wrote:
>> Fix error format set to I2S master or slave mode.
>> Test on RK3288 board with max98090.
>
> Applied. Since this is a bug fix it should be one of the first patches
> in the series so that it can be sent to Linus as a fix, bug fixes should
> go before any other patches.
>
got it, I'll do it at new version patch, thanks
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
@ 2014-09-14 2:06 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:06 UTC (permalink / raw)
To: Mark Brown, Jianqun
Cc: huangtao, alsa-devel, heiko, tiwai, linux-kernel, lgirdwood,
linux-rockchip, cf, linux-arm-kernel
在 09/14/2014 12:35 AM, Mark Brown 写道:
> On Sat, Sep 13, 2014 at 08:41:03AM +0800, Jianqun wrote:
>> Fix error format set to I2S master or slave mode.
>> Test on RK3288 board with max98090.
>
> Applied. Since this is a bug fix it should be one of the first patches
> in the series so that it can be sent to Linus as a fix, bug fixes should
> go before any other patches.
>
got it, I'll do it at new version patch, thanks
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error
@ 2014-09-14 2:06 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:06 UTC (permalink / raw)
To: linux-arm-kernel
? 09/14/2014 12:35 AM, Mark Brown ??:
> On Sat, Sep 13, 2014 at 08:41:03AM +0800, Jianqun wrote:
>> Fix error format set to I2S master or slave mode.
>> Test on RK3288 board with max98090.
>
> Applied. Since this is a bug fix it should be one of the first patches
> in the series so that it can be sent to Linus as a fix, bug fixes should
> go before any other patches.
>
got it, I'll do it at new version patch, thanks
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
2014-09-13 16:36 ` Mark Brown
@ 2014-09-14 2:20 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:20 UTC (permalink / raw)
To: Mark Brown, Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
在 09/14/2014 12:36 AM, Mark Brown 写道:
> On Sat, Sep 13, 2014 at 08:42:12AM +0800, Jianqun wrote:
>> Reference rockchip I2S controller TRM, modify some registers' property
>> I2S_FIFOLR: read / write, but not volatile, not precious
>> I2S_INTSR: read / write
>> I2S_CLR: volatile, register value will be cleared by read
>
> Applied, again this is a bug fix (volatile and precious being wrong seem
> like bugs) so should have been earlier in the series.
>
ok, I'll re-order the patches
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
@ 2014-09-14 2:20 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:20 UTC (permalink / raw)
To: linux-arm-kernel
? 09/14/2014 12:36 AM, Mark Brown ??:
> On Sat, Sep 13, 2014 at 08:42:12AM +0800, Jianqun wrote:
>> Reference rockchip I2S controller TRM, modify some registers' property
>> I2S_FIFOLR: read / write, but not volatile, not precious
>> I2S_INTSR: read / write
>> I2S_CLR: volatile, register value will be cleared by read
>
> Applied, again this is a bug fix (volatile and precious being wrong seem
> like bugs) so should have been earlier in the series.
>
ok, I'll re-order the patches
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
2014-09-13 16:36 ` Mark Brown
@ 2014-09-14 2:24 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:24 UTC (permalink / raw)
To: Mark Brown, Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
在 09/14/2014 12:36 AM, Mark Brown 写道:
> On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
>> As "hclk" is used for rockchip I2S controller, driver must to enable
>> it in probe.
>
> Applied, again this is a bug fix. How did the original submission get
> tested?
>
The original submission is verified on rk3288 with kernel 3.10, but I had to make patches based on
kernel 3.14 +, also our sdk kernel has intalized the kernel in other ways, so I missed the enable but
the driver worked well.
The new patches is verified on kernel 3.14, so it will easy to test.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
@ 2014-09-14 2:24 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:24 UTC (permalink / raw)
To: linux-arm-kernel
? 09/14/2014 12:36 AM, Mark Brown ??:
> On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
>> As "hclk" is used for rockchip I2S controller, driver must to enable
>> it in probe.
>
> Applied, again this is a bug fix. How did the original submission get
> tested?
>
The original submission is verified on rk3288 with kernel 3.10, but I had to make patches based on
kernel 3.14 +, also our sdk kernel has intalized the kernel in other ways, so I missed the enable but
the driver worked well.
The new patches is verified on kernel 3.14, so it will easy to test.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
2014-09-13 16:37 ` Mark Brown
@ 2014-09-14 2:27 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:27 UTC (permalink / raw)
To: Mark Brown, Jianqun
Cc: heiko, lgirdwood, perex, tiwai, linux-arm-kernel, linux-rockchip,
linux-kernel, alsa-devel, huangtao, cf
在 09/14/2014 12:37 AM, Mark Brown 写道:
> On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
>
>> +++ b/sound/soc/rockchip/rockchip_i2s.c
>> @@ -423,6 +423,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;
>> + }
>
> BTW: you're also missing a clk_disable_unprepare() in the remove path
> here, please send a followup fixing that.
remove function has done the clk_disable_unprepare for "hclk".
One more thing, since "i2s_clk" is enabled at runtime_resume, and is disabled in runtime_suspend,
hasn't enable in probe, so do the "i2s_clk" to be disabled in remove ?
The current driver has disable in remove.
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
@ 2014-09-14 2:27 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:27 UTC (permalink / raw)
To: linux-arm-kernel
? 09/14/2014 12:37 AM, Mark Brown ??:
> On Sat, Sep 13, 2014 at 08:43:13AM +0800, Jianqun wrote:
>
>> +++ b/sound/soc/rockchip/rockchip_i2s.c
>> @@ -423,6 +423,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;
>> + }
>
> BTW: you're also missing a clk_disable_unprepare() in the remove path
> here, please send a followup fixing that.
remove function has done the clk_disable_unprepare for "hclk".
One more thing, since "i2s_clk" is enabled at runtime_resume, and is disabled in runtime_suspend,
hasn't enable in probe, so do the "i2s_clk" to be disabled in remove ?
The current driver has disable in remove.
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
2014-09-13 20:57 ` Sergei Shtylyov
@ 2014-09-14 2:29 ` Jianqun
-1 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:29 UTC (permalink / raw)
To: Sergei Shtylyov, Jianqun, heiko, lgirdwood, broonie, perex,
tiwai, linux-arm-kernel, linux-rockchip, linux-kernel,
alsa-devel
Cc: huangtao, cf
在 09/14/2014 04:57 AM, Sergei Shtylyov 写道:
> Hello.
>
> On 9/13/2014 3:42 AM, Jianqun wrote:
>
>> Reference rockchip I2S controller TRM, modify some registers' property
>> I2S_FIFOLR: read / write, but not volatile, not precious
>> I2S_INTSR: read / write
>> I2S_CLR: volatile, register value will be cleared by read
>
>> Test on RK3288 with max98090.
>
>> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
>> ---
>> sound/soc/rockchip/rockchip_i2s.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
>> index 1b9b404..6595383 100644
>> --- a/sound/soc/rockchip/rockchip_i2s.c
>> +++ b/sound/soc/rockchip/rockchip_i2s.c
> [...]
>> @@ -385,8 +387,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;
>> }
>
> Shouldn't this be folded into simple *return* false now?
That is more reasonable, thank you.
>
> WBR, Sergei
>
>
>
>
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
@ 2014-09-14 2:29 ` Jianqun
0 siblings, 0 replies; 42+ messages in thread
From: Jianqun @ 2014-09-14 2:29 UTC (permalink / raw)
To: linux-arm-kernel
? 09/14/2014 04:57 AM, Sergei Shtylyov ??:
> Hello.
>
> On 9/13/2014 3:42 AM, Jianqun wrote:
>
>> Reference rockchip I2S controller TRM, modify some registers' property
>> I2S_FIFOLR: read / write, but not volatile, not precious
>> I2S_INTSR: read / write
>> I2S_CLR: volatile, register value will be cleared by read
>
>> Test on RK3288 with max98090.
>
>> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
>> ---
>> sound/soc/rockchip/rockchip_i2s.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
>> index 1b9b404..6595383 100644
>> --- a/sound/soc/rockchip/rockchip_i2s.c
>> +++ b/sound/soc/rockchip/rockchip_i2s.c
> [...]
>> @@ -385,8 +387,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;
>> }
>
> Shouldn't this be folded into simple *return* false now?
That is more reasonable, thank you.
>
> WBR, Sergei
>
>
>
>
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
2014-09-14 2:29 ` Jianqun
@ 2014-09-14 18:53 ` Sergei Shtylyov
-1 siblings, 0 replies; 42+ messages in thread
From: Sergei Shtylyov @ 2014-09-14 18:53 UTC (permalink / raw)
To: Jianqun, Jianqun, heiko, lgirdwood, broonie, perex, tiwai,
linux-arm-kernel, linux-rockchip, linux-kernel, alsa-devel
Cc: huangtao, cf
Hello.
On 09/14/2014 06:29 AM, Jianqun wrote:
>>> Reference rockchip I2S controller TRM, modify some registers' property
>>> I2S_FIFOLR: read / write, but not volatile, not precious
>>> I2S_INTSR: read / write
>>> I2S_CLR: volatile, register value will be cleared by read
>>> Test on RK3288 with max98090.
>>> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
>>> ---
>>> sound/soc/rockchip/rockchip_i2s.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
>>> index 1b9b404..6595383 100644
>>> --- a/sound/soc/rockchip/rockchip_i2s.c
>>> +++ b/sound/soc/rockchip/rockchip_i2s.c
>> [...]
>>> @@ -385,8 +387,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;
>>> }
>> Shouldn't this be folded into simple *return* false now?
> That is more reasonable, thank you.
Moreover, this function may be completely eliminated.
WBR, Sergei
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller
@ 2014-09-14 18:53 ` Sergei Shtylyov
0 siblings, 0 replies; 42+ messages in thread
From: Sergei Shtylyov @ 2014-09-14 18:53 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 09/14/2014 06:29 AM, Jianqun wrote:
>>> Reference rockchip I2S controller TRM, modify some registers' property
>>> I2S_FIFOLR: read / write, but not volatile, not precious
>>> I2S_INTSR: read / write
>>> I2S_CLR: volatile, register value will be cleared by read
>>> Test on RK3288 with max98090.
>>> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
>>> ---
>>> sound/soc/rockchip/rockchip_i2s.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
>>> index 1b9b404..6595383 100644
>>> --- a/sound/soc/rockchip/rockchip_i2s.c
>>> +++ b/sound/soc/rockchip/rockchip_i2s.c
>> [...]
>>> @@ -385,8 +387,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;
>>> }
>> Shouldn't this be folded into simple *return* false now?
> That is more reasonable, thank you.
Moreover, this function may be completely eliminated.
WBR, Sergei
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
2014-09-14 2:27 ` Jianqun
@ 2014-09-15 16:54 ` Mark Brown
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-15 16:54 UTC (permalink / raw)
To: Jianqun
Cc: Jianqun, heiko, lgirdwood, perex, tiwai, linux-arm-kernel,
linux-rockchip, linux-kernel, alsa-devel, huangtao, cf
[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]
On Sun, Sep 14, 2014 at 10:27:43AM +0800, Jianqun wrote:
> 在 09/14/2014 12:37 AM, Mark Brown 写道:
> >> + ret = clk_prepare_enable(i2s->hclk);
> >> + if (ret) {
> >> + dev_err(i2s->dev, "hclock enable failed %d\n", ret);
> >> + return ret;
> >> + }
> > BTW: you're also missing a clk_disable_unprepare() in the remove path
> > here, please send a followup fixing that.
> remove function has done the clk_disable_unprepare for "hclk".
> One more thing, since "i2s_clk" is enabled at runtime_resume, and is disabled in runtime_suspend,
> hasn't enable in probe, so do the "i2s_clk" to be disabled in remove ?
> The current driver has disable in remove.
Again, please try to write clear changelogs which describe what the goal
of the patch is and cover obvious questions like this which a reviewer
might ask.
This is all extremely unclear - you're adding an enable here with no
matching disable. It seems that what you saying that there was
previously a bug where the clock was disabled without being enabled?
I had to look at the code to work that out...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller
@ 2014-09-15 16:54 ` Mark Brown
0 siblings, 0 replies; 42+ messages in thread
From: Mark Brown @ 2014-09-15 16:54 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Sep 14, 2014 at 10:27:43AM +0800, Jianqun wrote:
> ? 09/14/2014 12:37 AM, Mark Brown ??:
> >> + ret = clk_prepare_enable(i2s->hclk);
> >> + if (ret) {
> >> + dev_err(i2s->dev, "hclock enable failed %d\n", ret);
> >> + return ret;
> >> + }
> > BTW: you're also missing a clk_disable_unprepare() in the remove path
> > here, please send a followup fixing that.
> remove function has done the clk_disable_unprepare for "hclk".
> One more thing, since "i2s_clk" is enabled at runtime_resume, and is disabled in runtime_suspend,
> hasn't enable in probe, so do the "i2s_clk" to be disabled in remove ?
> The current driver has disable in remove.
Again, please try to write clear changelogs which describe what the goal
of the patch is and cover obvious questions like this which a reviewer
might ask.
This is all extremely unclear - you're adding an enable here with no
matching disable. It seems that what you saying that there was
previously a bug where the clock was disabled without being enabled?
I had to look at the code to work that out...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140915/4f25d2fa/attachment-0001.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2014-09-15 17:02 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-13 0:38 [PATCH 0/5] ASoC: rockchip-i2s: patches for rockchip i2s driver Jianqun
2014-09-13 0:38 ` Jianqun
2014-09-13 0:40 ` [PATCH 1/5] ASoC: rockchip-i2s: fix rockchip i2s defination more reasonable Jianqun
2014-09-13 0:40 ` Jianqun
2014-09-13 16:38 ` Mark Brown
2014-09-13 16:38 ` Mark Brown
2014-09-13 0:41 ` [PATCH 2/5] ASoC: rockchip-i2s: fix master mode set bit error Jianqun
2014-09-13 0:41 ` Jianqun
2014-09-13 16:35 ` Mark Brown
2014-09-13 16:35 ` Mark Brown
2014-09-13 16:35 ` Mark Brown
2014-09-14 2:06 ` Jianqun
2014-09-14 2:06 ` Jianqun
2014-09-14 2:06 ` Jianqun
2014-09-13 0:41 ` [PATCH 3/5] ASoC: rockchip-i2s: add dma data to snd_soc_dai Jianqun
2014-09-13 0:41 ` Jianqun
2014-09-13 16:38 ` Mark Brown
2014-09-13 16:38 ` Mark Brown
2014-09-13 0:42 ` [PATCH 4/5] ASoC: rockchip-i2s: fix registers' property of rockchip i2s controller Jianqun
2014-09-13 0:42 ` Jianqun
2014-09-13 16:36 ` Mark Brown
2014-09-13 16:36 ` Mark Brown
2014-09-14 2:20 ` Jianqun
2014-09-14 2:20 ` Jianqun
2014-09-13 20:57 ` Sergei Shtylyov
2014-09-13 20:57 ` Sergei Shtylyov
2014-09-14 2:29 ` Jianqun
2014-09-14 2:29 ` Jianqun
2014-09-14 18:53 ` Sergei Shtylyov
2014-09-14 18:53 ` Sergei Shtylyov
2014-09-13 0:43 ` [PATCH 5/5] ASoC: rockchip-i2s: enable "hclk" for rockchip I2S controller Jianqun
2014-09-13 0:43 ` Jianqun
2014-09-13 16:36 ` Mark Brown
2014-09-13 16:36 ` Mark Brown
2014-09-14 2:24 ` Jianqun
2014-09-14 2:24 ` Jianqun
2014-09-13 16:37 ` Mark Brown
2014-09-13 16:37 ` Mark Brown
2014-09-14 2:27 ` Jianqun
2014-09-14 2:27 ` Jianqun
2014-09-15 16:54 ` Mark Brown
2014-09-15 16:54 ` 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.