[PATCH 0/4] sound:soc: jz4740: DT, dynamic sampling, enable clocks

[PATCH 0/4] sound:soc: jz4740: DT, dynamic sampling, enable clocks

[Zubair Lutfullah Kakakhel]

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

-- 
1.9.1

[PATCH 1/4] sound: soc: jz4740: Add dynamic sampling rate support to jz4740-i2s

[Zubair Lutfullah Kakakhel]

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;
 }
-- 
1.9.1

Re: [PATCH 1/4] sound: soc: jz4740: Add dynamic sampling rate support to jz4740-i2s

[Lars-Peter Clausen]

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.

[PATCH 2/4] dt: sound: jz4740: Add binding documentation for jz4740-i2s

[Zubair Lutfullah Kakakhel]

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";
+
+};
-- 
1.9.1

Re: [PATCH 2/4] dt: sound: jz4740: Add binding documentation for jz4740-i2s

[Lars-Peter Clausen]

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.

Re: [PATCH 2/4] dt: sound: jz4740: Add binding documentation for jz4740-i2s

[Zubair Lutfullah Kakakhel]

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

Re: [alsa-devel] [PATCH 2/4] dt: sound: jz4740: Add binding documentation for jz4740-i2s

[Lars-Peter Clausen]

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

Re: [PATCH 2/4] dt: sound: jz4740: Add binding documentation for jz4740-i2s

[Arnd Bergmann]

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

[PATCH 3/4] sound: soc: jz4740: Add DT support to jz4740-i2s driver

[Zubair Lutfullah Kakakhel]

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
 	},
 };
 
-- 
1.9.1

[PATCH 4/4] sound: jz4740: Enable codec clock during dai_probe

[Zubair Lutfullah Kakakhel]

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);
 
-- 
1.9.1

Re: [PATCH 4/4] sound: jz4740: Enable codec clock during dai_probe

[Lars-Peter Clausen]

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);
>
>

Re: [PATCH 4/4] sound: jz4740: Enable codec clock during dai_probe

[Zubair Lutfullah Kakakhel]

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);
>>
>>
> 

Re: [alsa-devel] [PATCH 4/4] sound: jz4740: Enable codec clock during dai_probe

[Lars-Peter Clausen]

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