[alsa-devel] [PATCH 0/4] sound:soc: jz4740: DT, dynamic sampling, enable clocks
Hi,
Here are a few simple patches for the jz4740.
First adds dynamic sampling support to jz4740-i2s. Then two to add a simple binding and DT support. Then a patch to enable the codec clock.
These are in preparation for jz4780 and ci20 later on.
These are based on 3.19-rc6.
If you would like to have them rebased to a different tree, please tell.
Thank-you
Zubair Lutfullah Kakakhel (4): sound: soc: jz4740: Add dynamic sampling rate support to jz4740-i2s dt: sound: jz4740: Add binding documentation for jz4740-i2s sound: soc: jz4740: Add DT support to jz4740-i2s driver sound: jz4740: Enable codec clock during dai_probe
.../bindings/sound/ingenic,jz4740-i2s.txt | 18 ++++++++++++++++ sound/soc/jz4740/jz4740-i2s.c | 24 +++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/sound/ingenic,jz4740-i2s.txt
The div clock register is not modified during jz4740_i2s_hw_params. Hence, default sampling rates are actually used regardless of sampling rates input from userspace.
This patch adds support to calculate the value of the divider from the parameters passed from userspace and update the relevant div registers
Signed-off-by: Zubair Lutfullah Kakakhel Zubair.Kakakhel@imgtec.com --- sound/soc/jz4740/jz4740-i2s.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c index d3d45c6..98c77a63 100644 --- a/sound/soc/jz4740/jz4740-i2s.c +++ b/sound/soc/jz4740/jz4740-i2s.c @@ -77,12 +77,15 @@ #define JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_OFFSET 16
#define JZ_AIC_I2S_FMT_DISABLE_BIT_CLK BIT(12) +#define JZ_AIC_I2S_FMT_DISABLE_BIT_ICLK BIT(13) #define JZ_AIC_I2S_FMT_ENABLE_SYS_CLK BIT(4) #define JZ_AIC_I2S_FMT_MSB BIT(0)
#define JZ_AIC_I2S_STATUS_BUSY BIT(2)
#define JZ_AIC_CLK_DIV_MASK 0xf +#define I2SDIV_DV_SHIFT 8 +#define I2SDIV_DV_MASK (0xf << I2SDIV_DV_SHIFT)
struct jz4740_i2s { struct resource *mem; @@ -237,10 +240,14 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream *substream, { struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); unsigned int sample_size; - uint32_t ctrl; + uint32_t ctrl, div_reg; + int div;
ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL);
+ div_reg = jz4740_i2s_read(i2s, JZ_REG_AIC_CLK_DIV); + div = clk_get_rate(i2s->clk_i2s) / (64 * params_rate(params)); + switch (params_format(params)) { case SNDRV_PCM_FORMAT_S8: sample_size = 0; @@ -264,7 +271,10 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream *substream, ctrl |= sample_size << JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_OFFSET; }
+ div_reg &= ~I2SDIV_DV_MASK; + div_reg |= (div - 1) << I2SDIV_DV_SHIFT; jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); + jz4740_i2s_write(i2s, JZ_REG_AIC_CLK_DIV, div_reg);
return 0; }
On 01/26/2015 11:18 AM, Zubair Lutfullah Kakakhel wrote: [...]
#define JZ_AIC_I2S_FMT_DISABLE_BIT_CLK BIT(12) +#define JZ_AIC_I2S_FMT_DISABLE_BIT_ICLK BIT(13)
This looks like it slipped in by accident. Otherwise the patch looks good.
This patch adds binding for the jz4740-i2s driver.
Signed-off-by: Zubair Lutfullah Kakakhel Zubair.Kakakhel@imgtec.com
--- The jz4740 is platform only at the moment.
But DT support is being added
See http://patchwork.linux-mips.org/bundle/paulburton/ci20-v3.20/ --- .../devicetree/bindings/sound/ingenic,jz4740-i2s.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/ingenic,jz4740-i2s.txt
diff --git a/Documentation/devicetree/bindings/sound/ingenic,jz4740-i2s.txt b/Documentation/devicetree/bindings/sound/ingenic,jz4740-i2s.txt new file mode 100644 index 0000000..0e40517 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/ingenic,jz4740-i2s.txt @@ -0,0 +1,18 @@ +Ingenic JZ4740 I2S controller + +Required properties: +- compatible : "ingenic,jz4740-i2s" +- reg : I2S registers location and length +- clocks : AIC and I2S PLL clock specifiers. +- clock-names: "aic" and "i2s" + +Example: + +i2s: i2s@10020000 { + compatible = "ingenic,jz4740-i2s"; + reg = <0x10020000 0x94>; + + clocks = <&cgu JZ4740_CLK_AIC>, <&cgu JZ4740_CLK_I2SPLL>; + clock-names = "aic", "i2s"; + +};
On 01/26/2015 11:18 AM, Zubair Lutfullah Kakakhel wrote: [...]
+Required properties: +- compatible : "ingenic,jz4740-i2s" +- reg : I2S registers location and length +- clocks : AIC and I2S PLL clock specifiers. +- clock-names: "aic" and "i2s"
We also need a handle to the DMA channels. Currently the request ids are hardcoded in the driver, but that needs to go away once the driver starts to support multiple SoCs with different request lines.
On 26/01/15 10:37, Lars-Peter Clausen wrote:
On 01/26/2015 11:18 AM, Zubair Lutfullah Kakakhel wrote: [...]
+Required properties: +- compatible : "ingenic,jz4740-i2s" +- reg : I2S registers location and length +- clocks : AIC and I2S PLL clock specifiers. +- clock-names: "aic" and "i2s"
We also need a handle to the DMA channels. Currently the request ids are hardcoded in the driver, but that needs to go away once the driver starts to support multiple SoCs with different request lines.
I know. I've left DMA until we push the jz4780-dma driver upstream and see how things fit..
ZubairLK
On 01/26/2015 12:32 PM, Zubair Lutfullah Kakakhel wrote:
On 26/01/15 10:37, Lars-Peter Clausen wrote:
On 01/26/2015 11:18 AM, Zubair Lutfullah Kakakhel wrote: [...]
+Required properties: +- compatible : "ingenic,jz4740-i2s" +- reg : I2S registers location and length +- clocks : AIC and I2S PLL clock specifiers. +- clock-names: "aic" and "i2s"
We also need a handle to the DMA channels. Currently the request ids are hardcoded in the driver, but that needs to go away once the driver starts to support multiple SoCs with different request lines.
I know. I've left DMA until we push the jz4780-dma driver upstream and see how things fit..
It doesn't hurt to put it in the spec. It will be two DMA channels one for rx, one for tx. See for example Documentation/devicetree/bindings/sound/bcm2835-i2s.txt
- Lars
On Monday 26 January 2015 10:18:29 Zubair Lutfullah Kakakhel wrote:
index 0000000..0e40517 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/ingenic,jz4740-i2s.txt @@ -0,0 +1,18 @@ +Ingenic JZ4740 I2S controller
+Required properties: +- compatible : "ingenic,jz4740-i2s" +- reg : I2S registers location and length +- clocks : AIC and I2S PLL clock specifiers. +- clock-names: "aic" and "i2s"
+Example:
+i2s: i2s@10020000 {
compatible = "ingenic,jz4740-i2s";
reg = <0x10020000 0x94>;
clocks = <&cgu JZ4740_CLK_AIC>, <&cgu JZ4740_CLK_I2SPLL>;
clock-names = "aic", "i2s";
+};
I would argue that the binding should mandate "dmas" property already. The code currently does not use it, but it will need to get changed in order to allow the dmaengine driver for this platform to be converted. You should require only one argument for the channel ID, so something like
dmas = <&dma 24>, <&dma 25>; dma-names = "tx", "rx";
would be enough for the binding. Unfortunately that also requires having a device node for the dma engine, but you can cheat there and not call of_dma_controller_register() in the first step.
Arnd
This patch adds device tree support for the jz4740 driver.
Signed-off-by: Zubair Lutfullah Kakakhel Zubair.Kakakhel@imgtec.com --- sound/soc/jz4740/jz4740-i2s.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c index 98c77a63..3c309fc 100644 --- a/sound/soc/jz4740/jz4740-i2s.c +++ b/sound/soc/jz4740/jz4740-i2s.c @@ -14,6 +14,8 @@
#include <linux/init.h> #include <linux/io.h> +#include <linux/of.h> +#include <linux/of_device.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/platform_device.h> @@ -425,6 +427,11 @@ static const struct snd_soc_component_driver jz4740_i2s_component = { .name = "jz4740-i2s", };
+static const struct of_device_id jz4740_of_matches[] = { + { .compatible = "ingenic,jz4740-i2s" }, + { /* sentinel */ } +}; + static int jz4740_i2s_dev_probe(struct platform_device *pdev) { struct jz4740_i2s *i2s; @@ -465,6 +472,7 @@ static struct platform_driver jz4740_i2s_driver = { .probe = jz4740_i2s_dev_probe, .driver = { .name = "jz4740-i2s", + .of_match_table = jz4740_of_matches }, };
As we are moving away from platform to DT, we cant rely on the board file to do this now. So enable it here.
Signed-off-by: Zubair Lutfullah Kakakhel Zubair.Kakakhel@imgtec.com --- sound/soc/jz4740/jz4740-i2s.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c index 3c309fc..a7e4a7b 100644 --- a/sound/soc/jz4740/jz4740-i2s.c +++ b/sound/soc/jz4740/jz4740-i2s.c @@ -376,6 +376,10 @@ static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai) JZ_AIC_CONF_I2S | JZ_AIC_CONF_INTERNAL_CODEC;
+ /* enable codec sysclk */ + clk_set_rate(i2s->clk_i2s, 12000000); + clk_prepare_enable(i2s->clk_i2s); + jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf);
On 01/26/2015 11:18 AM, Zubair Lutfullah Kakakhel wrote:
As we are moving away from platform to DT, we cant rely on the board file to do this now. So enable it here.
I don't understand this changelog. The board file never did this. The driver enables the clock in the startup() callback.
Signed-off-by: Zubair Lutfullah Kakakhel Zubair.Kakakhel@imgtec.com
sound/soc/jz4740/jz4740-i2s.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c index 3c309fc..a7e4a7b 100644 --- a/sound/soc/jz4740/jz4740-i2s.c +++ b/sound/soc/jz4740/jz4740-i2s.c @@ -376,6 +376,10 @@ static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai) JZ_AIC_CONF_I2S | JZ_AIC_CONF_INTERNAL_CODEC;
- /* enable codec sysclk */
- clk_set_rate(i2s->clk_i2s, 12000000);
- clk_prepare_enable(i2s->clk_i2s);
- jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf);
On 26/01/15 10:40, Lars-Peter Clausen wrote:
On 01/26/2015 11:18 AM, Zubair Lutfullah Kakakhel wrote:
As we are moving away from platform to DT, we cant rely on the board file to do this now. So enable it here.
I don't understand this changelog. The board file never did this. The driver enables the clock in the startup() callback.
My bad.
I couldn't get the ci20 audio to work without this change.
I double checked. The clock is indeed enabled.
But the rate needs to be set for the ci20.
clk_set_rate(i2s->clk_i2s, 12000000);
Where should I put it? I couldn’t trace how the rate is set for the jz4740..
ZubairLK
Signed-off-by: Zubair Lutfullah Kakakhel Zubair.Kakakhel@imgtec.com
sound/soc/jz4740/jz4740-i2s.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c index 3c309fc..a7e4a7b 100644 --- a/sound/soc/jz4740/jz4740-i2s.c +++ b/sound/soc/jz4740/jz4740-i2s.c @@ -376,6 +376,10 @@ static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai) JZ_AIC_CONF_I2S | JZ_AIC_CONF_INTERNAL_CODEC;
- /* enable codec sysclk */
- clk_set_rate(i2s->clk_i2s, 12000000);
- clk_prepare_enable(i2s->clk_i2s);
jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf);
On 01/26/2015 12:30 PM, Zubair Lutfullah Kakakhel wrote:
On 26/01/15 10:40, Lars-Peter Clausen wrote:
On 01/26/2015 11:18 AM, Zubair Lutfullah Kakakhel wrote:
As we are moving away from platform to DT, we cant rely on the board file to do this now. So enable it here.
I don't understand this changelog. The board file never did this. The driver enables the clock in the startup() callback.
My bad.
I couldn't get the ci20 audio to work without this change.
I double checked. The clock is indeed enabled.
But the rate needs to be set for the ci20.
clk_set_rate(i2s->clk_i2s, 12000000);
Where should I put it? I couldn’t trace how the rate is set for the jz4740..
There is no support for specifying clock rate defaults in the devicetree itself. See commit 86be408bfbd8 ("clk: Support for clock parents and rates assigned from device tree"). Since the preferred or correct clock rate will be board specific this is probably where it should go.
- Lars
participants (3)
-
Arnd Bergmann
-
Lars-Peter Clausen
-
Zubair Lutfullah Kakakhel