[PATCH 0/4] ASoC: simple-card/davinci-mcasp: System clock configuration support

[PATCH 0/4] ASoC: simple-card/davinci-mcasp: System clock configuration support

[Peter Ujfalusi]

Hi,

Currently davinci-mcasp is hardwired for two configuration regarding to reference
clock:
McASP is master: AHCLKX is selected as source of the reference clock used to
		 generate the FS and BCLK
McASP is slave: AHCLKX is selected as output and the default AUXCLK is routed
		there.

This works in most of the boards, but not all configuration can be supported
with this limitation.
McASP uses it's internal AUXCLK as a reference clock. AUXCLK can be sourced from
the external AHCLKX pin or from internal functional clock. If the board uses
McASP as master and AHCLKX is not receiving clock, we need to be able to select
the internal clock source to be able to generate the FS and BCLK.

Since we try to move all of our boards to simple-card instead of the davinci-evm
custom machine driver, I have included Jyri's two patch to add the needed support
for it to be able to select clocks and directions.

I remember the discussion regarding to clock IDs, and to try to move to common
clock framework. I have taken a look at that path and I'm not convinced that by
trying to add ASoC level common clock handling will simplify or makes things
cleaner/simpler.

Regards,
Peter
---
Jyri Sarha (2):
  ASoC: simple-card: Add system-clock-direction DT parameter to dai
    nodes
  ASoC: simple-card: Support for selecting system clocks by ID

Peter Ujfalusi (2):
  ASoC: davinci-mcasp: Use defines for clkdiv IDs via DT binding header
  ASoC: davinci-mcasp: Improve the sysclk selection

 .../devicetree/bindings/sound/simple-card.txt      |  3 ++
 include/dt-bindings/sound/ti-mcasp.h               | 13 +++++++
 include/sound/simple_card.h                        |  2 ++
 sound/soc/davinci/davinci-mcasp.c                  | 42 ++++++++++++++++------
 sound/soc/generic/simple-card.c                    | 17 ++++++++-
 5 files changed, 66 insertions(+), 11 deletions(-)
 create mode 100644 include/dt-bindings/sound/ti-mcasp.h

-- 
2.7.1

[PATCH 1/4] ASoC: davinci-mcasp: Use defines for clkdiv IDs via DT binding header

[Peter Ujfalusi]

Instead of hardwired IDs add defines to select which divider to configure.
The defines are placed in the dt binding header so legacy and DT files can
both use them.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 include/dt-bindings/sound/ti-mcasp.h | 9 +++++++++
 sound/soc/davinci/davinci-mcasp.c    | 8 +++++---
 2 files changed, 14 insertions(+), 3 deletions(-)
 create mode 100644 include/dt-bindings/sound/ti-mcasp.h

diff --git a/include/dt-bindings/sound/ti-mcasp.h b/include/dt-bindings/sound/ti-mcasp.h
new file mode 100644
index 000000000000..03519ab67b44
--- /dev/null
+++ b/include/dt-bindings/sound/ti-mcasp.h
@@ -0,0 +1,9 @@
+#ifndef _DT_BINDINGS_TI_MCASP_H
+#define _DT_BINDINGS_TI_MCASP_H
+
+/* clock divider IDs */
+#define MCASP_CLKDIV_AUXCLK		0 /* HCLK divider from AUXCLK */
+#define MCASP_CLKDIV_BCLK		1 /* BCLK divider from HCLK */
+#define MCASP_CLKDIV_BCLK_FS_RATIO	2 /* to set BCLK FS ration */
+
+#endif /* _DT_BINDINGS_TI_MCASP_H */
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 3cf46afb353f..a4e40606742a 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -37,6 +37,7 @@
 #include <sound/soc.h>
 #include <sound/dmaengine_pcm.h>
 #include <sound/omap-pcm.h>
+#include <dt-bindings/sound/ti-mcasp.h>
 
 #include "edma-pcm.h"
 #include "davinci-mcasp.h"
@@ -541,14 +542,14 @@ static int __davinci_mcasp_set_clkdiv(struct snd_soc_dai *dai, int div_id,
 
 	pm_runtime_get_sync(mcasp->dev);
 	switch (div_id) {
-	case 0:		/* MCLK divider */
+	case MCASP_CLKDIV_AUXCLK:			/* MCLK divider */
 		mcasp_mod_bits(mcasp, DAVINCI_MCASP_AHCLKXCTL_REG,
 			       AHCLKXDIV(div - 1), AHCLKXDIV_MASK);
 		mcasp_mod_bits(mcasp, DAVINCI_MCASP_AHCLKRCTL_REG,
 			       AHCLKRDIV(div - 1), AHCLKRDIV_MASK);
 		break;
 
-	case 1:		/* BCLK divider */
+	case MCASP_CLKDIV_BCLK:			/* BCLK divider */
 		mcasp_mod_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG,
 			       ACLKXDIV(div - 1), ACLKXDIV_MASK);
 		mcasp_mod_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG,
@@ -557,7 +558,8 @@ static int __davinci_mcasp_set_clkdiv(struct snd_soc_dai *dai, int div_id,
 			mcasp->bclk_div = div;
 		break;
 
-	case 2:	/*
+	case MCASP_CLKDIV_BCLK_FS_RATIO:
+		/*
 		 * BCLK/LRCLK ratio descries how many bit-clock cycles
 		 * fit into one frame. The clock ratio is given for a
 		 * full period of data (for I2S format both left and
-- 
2.7.1

[PATCH 2/4] ASoC: davinci-mcasp: Improve the sysclk selection

[Peter Ujfalusi]

When McASP is master the bclk can be generated from two main source:
AUXCLK: functional clock for McASP or
AHCLK: from external source or internal mux in dra7x family

With this patch it is possible to select between the two source. The patch
is not breaking existing machine drivers since historically the clk_id was
ignored and left as 0 in all cases.

When output clock is configured - which can be only the AHCLK, we select
the AUXCLK as source for the internal HCLK. In this case the HCLK rate is
the same as the output clock.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 include/dt-bindings/sound/ti-mcasp.h |  4 ++++
 sound/soc/davinci/davinci-mcasp.c    | 34 +++++++++++++++++++++++++++-------
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/include/dt-bindings/sound/ti-mcasp.h b/include/dt-bindings/sound/ti-mcasp.h
index 03519ab67b44..86ee4d0d79b5 100644
--- a/include/dt-bindings/sound/ti-mcasp.h
+++ b/include/dt-bindings/sound/ti-mcasp.h
@@ -1,6 +1,10 @@
 #ifndef _DT_BINDINGS_TI_MCASP_H
 #define _DT_BINDINGS_TI_MCASP_H
 
+/* Source of High-frequency transmit/receive clock */
+#define MCASP_CLK_HCLK_AHCLK		0 /* AHCLKX/R */
+#define MCASP_CLK_HCLK_AUXCLK		1 /* Internal functional clock */
+
 /* clock divider IDs */
 #define MCASP_CLKDIV_AUXCLK		0 /* HCLK divider from AUXCLK */
 #define MCASP_CLKDIV_BCLK		1 /* BCLK divider from HCLK */
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index a4e40606742a..1bd86c8e7bc8 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -596,18 +596,38 @@ static int davinci_mcasp_set_sysclk(struct snd_soc_dai *dai, int clk_id,
 	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
 
 	pm_runtime_get_sync(mcasp->dev);
-	if (dir == SND_SOC_CLOCK_OUT) {
+
+	if (dir == SND_SOC_CLOCK_IN) {
+		switch (clk_id) {
+		case MCASP_CLK_HCLK_AHCLK:
+			mcasp_clr_bits(mcasp, DAVINCI_MCASP_AHCLKXCTL_REG,
+				       AHCLKXE);
+			mcasp_clr_bits(mcasp, DAVINCI_MCASP_AHCLKRCTL_REG,
+				       AHCLKRE);
+			mcasp_clr_bits(mcasp, DAVINCI_MCASP_PDIR_REG, AHCLKX);
+			break;
+		case MCASP_CLK_HCLK_AUXCLK:
+			mcasp_set_bits(mcasp, DAVINCI_MCASP_AHCLKXCTL_REG,
+				       AHCLKXE);
+			mcasp_set_bits(mcasp, DAVINCI_MCASP_AHCLKRCTL_REG,
+				       AHCLKRE);
+			break;
+		default:
+			dev_err(mcasp->dev, "Invalid clk id: %d\n", clk_id);
+			goto out;
+		}
+	} else {
+		/* Select AUXCLK as HCLK */
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_AHCLKXCTL_REG, AHCLKXE);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_AHCLKRCTL_REG, AHCLKRE);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_PDIR_REG, AHCLKX);
-	} else {
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_AHCLKXCTL_REG, AHCLKXE);
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_AHCLKRCTL_REG, AHCLKRE);
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_PDIR_REG, AHCLKX);
 	}
-
+	/*
+	 * When AHCLK X/R is selected to be output it means that the HCLK is
+	 * the same clock - coming via AUXCLK.
+	 */
 	mcasp->sysclk_freq = freq;
-
+out:
 	pm_runtime_put(mcasp->dev);
 	return 0;
 }
-- 
2.7.1

[PATCH 3/4] ASoC: simple-card: Add system-clock-direction DT parameter to dai nodes

[Peter Ujfalusi]

From: Jyri Sarha <jsarha@ti.com>

Select dir parameters for set_sysclk calls in the card init phase.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 Documentation/devicetree/bindings/sound/simple-card.txt |  1 +
 include/sound/simple_card.h                             |  1 +
 sound/soc/generic/simple-card.c                         | 14 +++++++++++++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index cf3979eb3578..1f2cf76d701a 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -84,6 +84,7 @@ Optional CPU/CODEC subnodes properties:
 					  in dai startup() and disabled with
 					  clk_disable_unprepare() in dai
 					  shutdown().
+- system-clock-direction		: "in" or "out", default "in"
 
 Example 1 - single DAI link:
 
diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index 0399352f3a62..783bc5499794 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -17,6 +17,7 @@
 struct asoc_simple_dai {
 	const char *name;
 	unsigned int sysclk;
+	int sysclk_dir;
 	int slots;
 	int slot_width;
 	unsigned int tx_slot_mask;
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 2389ab47e25f..f0d89e6f28a6 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -142,7 +142,8 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 	int ret;
 
 	if (set->sysclk) {
-		ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
+		ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk,
+					     set->sysclk_dir);
 		if (ret && ret != -ENOTSUPP) {
 			dev_err(dai->dev, "simple-card: set_sysclk error\n");
 			goto err;
@@ -220,6 +221,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 {
 	struct of_phandle_args args;
 	struct clk *clk;
+	const char *str;
 	u32 val;
 	int ret;
 
@@ -249,6 +251,16 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 	if (ret)
 		return ret;
 
+	ret = of_property_read_string(np, "system-clock-direction", &str);
+	if (ret == 0) {
+		if (!strcmp(str, "out"))
+			dai->sysclk_dir = SND_SOC_CLOCK_OUT;
+		else if (!strcmp(str, "in"))
+			dai->sysclk_dir = SND_SOC_CLOCK_IN;
+		else
+			return -EINVAL;
+	}
+
 	/*
 	 * Parse dai->sysclk come from "clocks = <&xxx>"
 	 * (if system has common clock)
-- 
2.7.1

[PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

From: Jyri Sarha <jsarha@ti.com>

Most CPU and codec drivers can select their system clock from different
sources. They rely on snd_soc_dai_set_sysclk(x, sysclk_id, x, x) to do so.
Add support for selecting which clock need to be selected along with the
rate.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 Documentation/devicetree/bindings/sound/simple-card.txt | 2 ++
 include/sound/simple_card.h                             | 1 +
 sound/soc/generic/simple-card.c                         | 5 ++++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index 1f2cf76d701a..bc8f9e4a17cc 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -85,6 +85,8 @@ Optional CPU/CODEC subnodes properties:
 					  clk_disable_unprepare() in dai
 					  shutdown().
 - system-clock-direction		: "in" or "out", default "in"
+- system-clock-id			: Numberic ID of the system clock to
+					  select within the dai, default is 0.
 
 Example 1 - single DAI link:
 
diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index 783bc5499794..f930ad3763ff 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -18,6 +18,7 @@ struct asoc_simple_dai {
 	const char *name;
 	unsigned int sysclk;
 	int sysclk_dir;
+	int sysclk_id;
 	int slots;
 	int slot_width;
 	unsigned int tx_slot_mask;
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index f0d89e6f28a6..c6111e7a6d93 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -142,7 +142,7 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 	int ret;
 
 	if (set->sysclk) {
-		ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk,
+		ret = snd_soc_dai_set_sysclk(dai, set->sysclk_id, set->sysclk,
 					     set->sysclk_dir);
 		if (ret && ret != -ENOTSUPP) {
 			dev_err(dai->dev, "simple-card: set_sysclk error\n");
@@ -284,6 +284,9 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 			dai->sysclk = clk_get_rate(clk);
 	}
 
+	if (!of_property_read_u32(np, "system-clock-id", &val))
+		dai->sysclk_id = val;
+
 	return 0;
 }
 
-- 
2.7.1

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 582 bytes --]

On Mon, Feb 15, 2016 at 04:11:35PM +0200, Peter Ujfalusi wrote:

> Most CPU and codec drivers can select their system clock from different
> sources. They rely on snd_soc_dai_set_sysclk(x, sysclk_id, x, x) to do so.
> Add support for selecting which clock need to be selected along with the
> rate.

Rather than adding new bindings for the ASoC specific clocking APIs it
seems better to use the standard clock API bindings and transition
drivers to use that.  In the past the clock API didn't have DT bindings
so wehad to do our own thing but I think now that's no longer the case?

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 02/15/2016 05:26 PM, Mark Brown wrote:
> On Mon, Feb 15, 2016 at 04:11:35PM +0200, Peter Ujfalusi wrote:
> 
>> Most CPU and codec drivers can select their system clock from different
>> sources. They rely on snd_soc_dai_set_sysclk(x, sysclk_id, x, x) to do so.
>> Add support for selecting which clock need to be selected along with the
>> rate.
> 
> Rather than adding new bindings for the ASoC specific clocking APIs it
> seems better to use the standard clock API bindings and transition
> drivers to use that.  In the past the clock API didn't have DT bindings
> so wehad to do our own thing but I think now that's no longer the case?

McASP is relatively simple regarding to clocking and it is only problematic in
case when it is master. When it is slave it is going to use the BCLK and FS
coming from outside for clocking the data.

The driver currently only supports symmetric clocking, meaning we only use the
TX clock path (RX is using the TX clocks).
To generate the bit clock we have something like this:

[mcaspA_auxclkx_div]->
		    [mcaspA_hclkx_mux] -> [mcaspA_hclkx_div] -> [mcaspA_bclkx]
[mcaspA_ahclkx_in]  ->

If [mcaspA_auxclkx_div] is selected in [mcaspA_hclkx_mux], then:
[mcaspA_hclkx_mux] -> [mcaspA_ahclkx_out]
the HCLKX clock can be sent to mcaspA_ahclkx_out, as output from the SoC.

mcaspA is mcasp0...number of MCASP in the SoC.

We would need three (or two if we decide to not represent the mcaspA_bclkx)
simple clock node, two dividers and one mux.

We could have binding for this regarding to clocks:

/* McASP clk IDs */
#define MCASP_AUXCLKX_DIV	0
#define MCASP_AHCLKX_IN		1
#define MCASP_HCLKX_MUX		2
#define MCASP_HCLKX_DIV		3
#define MCASP_AHCLKX_OUT	4
#define MCASP_BCLK		5

/* dtsi file */
mcasp3: mcasp@48468000 {
	compatible = "ti,dra7-mcasp-audio";
	#clock-cells = <1>;
	clocks = <&mcasp3_aux_gfclk_mux>, <&mcasp3_ahclkx_mux>;
	clock-names = "fck", "ahclkx";
	assigned-clocks = <&mcasp3 MCASP_AHCLKX_IN>,
			  <&mcasp3 MCASP_AUXCLKX_DIV>;
	assigned-clock-parents = <&mcasp3_ahclkx_mux>,
				 <&mcasp3_aux_gfclk_mux>;
};

/* board dts */
&mcasp3 {
	assigned-clocks = <&mcasp3 MCASP_AHCLKX_IN>,
			  <&mcasp3 MCASP_AUXCLKX_DIV>,
			  <&mcasp3_ahclkx_mux>,
			  <&mcasp3 MCASP_HCLKX_MUX>;
	assigned-clock-parents = <&mcasp3_ahclkx_mux>,
				 <&mcasp3_aux_gfclk_mux>,
				 <&atl_clkin2_ck>,
				 <&mcasp3 MCASP_AHCLKX_IN>;
};

With this one we can set up the clock tree via clk bindings. Not sure how this
will work if audio including McASP is built as module?

There are couple of important questions regarding to this: if the
MCASP_HCLKX_DIV is not explicitly set the McASP driver will configure it based
on the need for the starting stream (rate, bits, etc), but in some case we
need to have different calculation since we want to have more bclk then it is
needed for the audio data (bclk-fck ratio).

The clock tree within the McASP need to be built up inside of the code, can
not be done via DT.

As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
can receive it's reference clock via:
MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
frequency it can use it directly or it needs to use the internal PLL to
generate the cocks.
It can output generated clock via GPIO1

I don't think it will bring any clarity or features we miss right now if we
try to move CPU and codec drivers to clk API. IMHO.

-- 
Péter

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 02/15/2016 05:26 PM, Mark Brown wrote:
> On Mon, Feb 15, 2016 at 04:11:35PM +0200, Peter Ujfalusi wrote:
> 
>> Most CPU and codec drivers can select their system clock from different
>> sources. They rely on snd_soc_dai_set_sysclk(x, sysclk_id, x, x) to do so.
>> Add support for selecting which clock need to be selected along with the
>> rate.
> 
> Rather than adding new bindings for the ASoC specific clocking APIs it
> seems better to use the standard clock API bindings and transition
> drivers to use that.  In the past the clock API didn't have DT bindings
> so wehad to do our own thing but I think now that's no longer the case?

McASP is relatively simple regarding to clocking and it is only problematic in
case when it is master. When it is slave it is going to use the BCLK and FS
coming from outside for clocking the data.

The driver currently only supports symmetric clocking, meaning we only use the
TX clock path (RX is using the TX clocks).
To generate the bit clock we have something like this:

[mcaspA_auxclkx_div]->
		    [mcaspA_hclkx_mux] -> [mcaspA_hclkx_div] -> [mcaspA_bclkx]
[mcaspA_ahclkx_in]  ->

If [mcaspA_auxclkx_div] is selected in [mcaspA_hclkx_mux], then:
[mcaspA_hclkx_mux] -> [mcaspA_ahclkx_out]
the HCLKX clock can be sent to mcaspA_ahclkx_out, as output from the SoC.

mcaspA is mcasp0...number of MCASP in the SoC.

We would need three (or two if we decide to not represent the mcaspA_bclkx)
simple clock node, two dividers and one mux.

We could have binding for this regarding to clocks:

/* McASP clk IDs */
#define MCASP_AUXCLKX_DIV	0
#define MCASP_AHCLKX_IN		1
#define MCASP_HCLKX_MUX		2
#define MCASP_HCLKX_DIV		3
#define MCASP_AHCLKX_OUT	4
#define MCASP_BCLK		5

/* dtsi file */
mcasp3: mcasp@48468000 {
	compatible = "ti,dra7-mcasp-audio";
	#clock-cells = <1>;
	clocks = <&mcasp3_aux_gfclk_mux>, <&mcasp3_ahclkx_mux>;
	clock-names = "fck", "ahclkx";
	assigned-clocks = <&mcasp3 MCASP_AHCLKX_IN>,
			  <&mcasp3 MCASP_AUXCLKX_DIV>;
	assigned-clock-parents = <&mcasp3_ahclkx_mux>,
				 <&mcasp3_aux_gfclk_mux>;
};

/* board dts */
&mcasp3 {
	assigned-clocks = <&mcasp3 MCASP_AHCLKX_IN>,
			  <&mcasp3 MCASP_AUXCLKX_DIV>,
			  <&mcasp3_ahclkx_mux>,
			  <&mcasp3 MCASP_HCLKX_MUX>;
	assigned-clock-parents = <&mcasp3_ahclkx_mux>,
				 <&mcasp3_aux_gfclk_mux>,
				 <&atl_clkin2_ck>,
				 <&mcasp3 MCASP_AHCLKX_IN>;
};

With this one we can set up the clock tree via clk bindings. Not sure how this
will work if audio including McASP is built as module?

There are couple of important questions regarding to this: if the
MCASP_HCLKX_DIV is not explicitly set the McASP driver will configure it based
on the need for the starting stream (rate, bits, etc), but in some case we
need to have different calculation since we want to have more bclk then it is
needed for the audio data (bclk-fck ratio).

The clock tree within the McASP need to be built up inside of the code, can
not be done via DT.

As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
can receive it's reference clock via:
MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
frequency it can use it directly or it needs to use the internal PLL to
generate the cocks.
It can output generated clock via GPIO1

I don't think it will bring any clarity or features we miss right now if we
try to move CPU and codec drivers to clk API. IMHO.

-- 
Péter

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Andreas Irestål]

On Mon, Feb 15, 2016 at 03:26:35PM +0000, Mark Brown wrote:
> On Mon, Feb 15, 2016 at 04:11:35PM +0200, Peter Ujfalusi wrote:
> 
> > Most CPU and codec drivers can select their system clock from different
> > sources. They rely on snd_soc_dai_set_sysclk(x, sysclk_id, x, x) to do so.
> > Add support for selecting which clock need to be selected along with the
> > rate.
> 
> Rather than adding new bindings for the ASoC specific clocking APIs it
> seems better to use the standard clock API bindings and transition
> drivers to use that.  In the past the clock API didn't have DT bindings
> so wehad to do our own thing but I think now that's no longer the case?

I'm interested in this as well since I'm working on a set of similair 
patches implementing clocking and PLL related issues in the simple-card
driver. 

There would be some problems as the typical use-cases involving
setting a codecs clocks / PLLs to a multiple of the sample rate, data
width, etc. The clocking API should support this as well if that's the
way forward. However, since many of the clock settings are related to
codec internals, the gain from moving codec PLLs, clocks, etc to the
standard clock API would be rather limited. There is already support for
getting sysclk from this API, which makes sense. Shouldn't clock id be
seen more as a pin selection from the dai's point of view? 

I really like the idea of the simple-card driver. It seems most board
drivers really doesn't do much except setting up some clocks, which
should be done in DT anyway, given there are enough available bindings
to do so...

/Andreas

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Mark Brown]

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

On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:

> As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
> can receive it's reference clock via:
> MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
> frequency it can use it directly or it needs to use the internal PLL to
> generate the cocks.
> It can output generated clock via GPIO1

That already sounds like there is room for configuration and hooking
into a wider clock tree - we've got three different source options and
an output plus a PLL that can presumably take in non-audio rates.

> I don't think it will bring any clarity or features we miss right now if we
> try to move CPU and codec drivers to clk API. IMHO.

You happen to be looking at a particularly simple system but things do
scale up and there's not a clear cutoff point which would allow us to
make a clear distinction between things that might get used in a simple
system and things that might need something more complex.  This seems
particularly important when we're adding things to simple-card, we want
it to be usable with as many different devices as possible.

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

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Michael Turquette]

Quoting Mark Brown (2016-02-16 05:42:33)
> On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:
> 
> > As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
> > can receive it's reference clock via:
> > MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
> > frequency it can use it directly or it needs to use the internal PLL to
> > generate the cocks.
> > It can output generated clock via GPIO1
> 
> That already sounds like there is room for configuration and hooking
> into a wider clock tree - we've got three different source options and
> an output plus a PLL that can presumably take in non-audio rates.

+1

It is quite easy for existing drivers to become clock providers. Please
see struct isp_xclk in:

drivers/media/platform/omap3isp/isp.h
drivers/media/platform/omap3isp/isp.c

CCF is intentionally designed as a library, meaning that you don't need
to create a new struct device to register clocks. Feel free to BYOD
(bring your own device).

Then your IP block clocks (McASP in this case) can hook into the
system-wide clock tree (e.g. where some of the parent clocks come from).

> 
> > I don't think it will bring any clarity or features we miss right now if we
> > try to move CPU and codec drivers to clk API. IMHO.

CPU drivers? Peter, you wrote the CCF clock provider driver for DRA7
ATL, so I'm not sure what you mean here.

> 
> You happen to be looking at a particularly simple system but things do
> scale up and there's not a clear cutoff point which would allow us to
> make a clear distinction between things that might get used in a simple
> system and things that might need something more complex.  This seems
> particularly important when we're adding things to simple-card, we want
> it to be usable with as many different devices as possible.

The original patches didn't hit my inbox, only the last two replies. Can
someone fill me in on the DT side of this discussion? Why are DT
bindings needed here? Are other devices besides McASP consuming the
clocks provided by McASP?

DT can also be a good candidate for doing per-board (or per-use case)
clock configuration via the assigned-clocks, assigned-clock-rates, and
assigned-clock-parents properties.

Regards,
Mike

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Michael Turquette]

Quoting Mark Brown (2016-02-16 05:42:33)
> On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:
> =

> > As for codecs, tlv320aic3106 is also pretty simple device from the outs=
ide, it
> > can receive it's reference clock via:
> > MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the i=
ncoming
> > frequency it can use it directly or it needs to use the internal PLL to
> > generate the cocks.
> > It can output generated clock via GPIO1
> =

> That already sounds like there is room for configuration and hooking
> into a wider clock tree - we've got three different source options and
> an output plus a PLL that can presumably take in non-audio rates.

+1

It is quite easy for existing drivers to become clock providers. Please
see struct isp_xclk in:

drivers/media/platform/omap3isp/isp.h
drivers/media/platform/omap3isp/isp.c

CCF is intentionally designed as a library, meaning that you don't need
to create a new struct device to register clocks. Feel free to BYOD
(bring your own device).

Then your IP block clocks (McASP in this case) can hook into the
system-wide clock tree (e.g. where some of the parent clocks come from).

> =

> > I don't think it will bring any clarity or features we miss right now i=
f we
> > try to move CPU and codec drivers to clk API. IMHO.

CPU drivers? Peter, you wrote the CCF clock provider driver for DRA7
ATL, so I'm not sure what you mean here.

> =

> You happen to be looking at a particularly simple system but things do
> scale up and there's not a clear cutoff point which would allow us to
> make a clear distinction between things that might get used in a simple
> system and things that might need something more complex.  This seems
> particularly important when we're adding things to simple-card, we want
> it to be usable with as many different devices as possible.

The original patches didn't hit my inbox, only the last two replies. Can
someone fill me in on the DT side of this discussion? Why are DT
bindings needed here? Are other devices besides McASP consuming the
clocks provided by McASP?

DT can also be a good candidate for doing per-board (or per-use case)
clock configuration via the assigned-clocks, assigned-clock-rates, and
assigned-clock-parents properties.

Regards,
Mike

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

Mike,

On 02/16/2016 09:13 PM, Michael Turquette wrote:
> Quoting Mark Brown (2016-02-16 05:42:33)
>> On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:
>>
>>> As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
>>> can receive it's reference clock via:
>>> MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
>>> frequency it can use it directly or it needs to use the internal PLL to
>>> generate the cocks.
>>> It can output generated clock via GPIO1
>>
>> That already sounds like there is room for configuration and hooking
>> into a wider clock tree - we've got three different source options and
>> an output plus a PLL that can presumably take in non-audio rates.
> 
> +1
> 
> It is quite easy for existing drivers to become clock providers. Please
> see struct isp_xclk in:
> 
> drivers/media/platform/omap3isp/isp.h
> drivers/media/platform/omap3isp/isp.c
> 
> CCF is intentionally designed as a library, meaning that you don't need
> to create a new struct device to register clocks. Feel free to BYOD
> (bring your own device).

I have already started to look around what I would need for at least McASP and
tlv320aic3x drivers (the most common combination).
I did not know that omap3isp has CCF provider implementation.

> Then your IP block clocks (McASP in this case) can hook into the
> system-wide clock tree (e.g. where some of the parent clocks come from).
>>
>>> I don't think it will bring any clarity or features we miss right now if we
>>> try to move CPU and codec drivers to clk API. IMHO.
> 
> CPU drivers? Peter, you wrote the CCF clock provider driver for DRA7
> ATL, so I'm not sure what you mean here.

also the clk-twl6040, which is not in use upstream yet.
The ATL is providing the clocks to be used as master clock in McASP and
external components (audio codec).
In ASoC the CPU driver means the CPU DAI (Digital Audio Interface) driver,
like the McASP, McBSP, DMIC, McPDM in TI parts.
In order them to generate the needed clocks on the bus they have internal
clock selection, divider(s). From the SoC point of view these are not really
important, but McASP can also output high speed reference clock to outside so
the codec for example can use the same reference, reducing jitter.

>> You happen to be looking at a particularly simple system but things do
>> scale up and there's not a clear cutoff point which would allow us to
>> make a clear distinction between things that might get used in a simple
>> system and things that might need something more complex.  This seems
>> particularly important when we're adding things to simple-card, we want
>> it to be usable with as many different devices as possible.
> 
> The original patches didn't hit my inbox, only the last two replies. Can
> someone fill me in on the DT side of this discussion?

the original patch:
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-February/104320.html

> Why are DT bindings needed here?

ASoC have an API to configure the DAI system clock:
snd_soc_dai_set_sysclk(dai, sysclk_id, sysclk, sysclk_dir);

The sysclk_id is to select among the possible clock sources, sysclk is the
rate of the clock and sysclk_dir is to tell the DAI driver if the given clock
should be input or output.
McASP (and McBSP also) can select between two master clock source. aic3106 can
select between 3 reference clocks.
Unfortunately the simple-card was hardwired to only handle DAIs which have no
master clock selection. We have added additional properties to simple-card so
it will be possible to select clocks and directions.
With this change we don't need to write custom machine drivers for setup not
using sysclk_id == 0.
I do think this is reasonable change by itself.

> Are other devices besides McASP consuming the clocks provided by McASP?

Yes, it can output the same reference clock it is using to external pin, but
it can do that only if it is set to generate the audio clocks on the bus.

> DT can also be a good candidate for doing per-board (or per-use case)
> clock configuration via the assigned-clocks, assigned-clock-rates, and
> assigned-clock-parents properties.

Yes, if the DAI driver implements it's clock tree with CCF internally and with
the assigned-clock* properties we will not need snd_soc_dai_set_sysclk() and
we don't need simple-card to know anything about the clocking of the
components. All of this can be done in the board's DTS file with standard CCF
bindings.

Mark: after thinking over how this should work I can see that it can bring
benefits for the DAI/codec drivers. They need to do things differently, but it
is just implementation question.
However I do think that the current simple-card is flawed regarding to clock
selection and the change Jyri and me are proposing is reasonable.
But if we have the CCF providers popping up in DAI/codec drivers the use of
clock configuration via simple-card not going to be needed. But right know it
is needed.

-- 
Péter

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

Mike,

On 02/16/2016 09:13 PM, Michael Turquette wrote:
> Quoting Mark Brown (2016-02-16 05:42:33)
>> On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:
>>
>>> As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
>>> can receive it's reference clock via:
>>> MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
>>> frequency it can use it directly or it needs to use the internal PLL to
>>> generate the cocks.
>>> It can output generated clock via GPIO1
>>
>> That already sounds like there is room for configuration and hooking
>> into a wider clock tree - we've got three different source options and
>> an output plus a PLL that can presumably take in non-audio rates.
> 
> +1
> 
> It is quite easy for existing drivers to become clock providers. Please
> see struct isp_xclk in:
> 
> drivers/media/platform/omap3isp/isp.h
> drivers/media/platform/omap3isp/isp.c
> 
> CCF is intentionally designed as a library, meaning that you don't need
> to create a new struct device to register clocks. Feel free to BYOD
> (bring your own device).

I have already started to look around what I would need for at least McASP and
tlv320aic3x drivers (the most common combination).
I did not know that omap3isp has CCF provider implementation.

> Then your IP block clocks (McASP in this case) can hook into the
> system-wide clock tree (e.g. where some of the parent clocks come from).
>>
>>> I don't think it will bring any clarity or features we miss right now if we
>>> try to move CPU and codec drivers to clk API. IMHO.
> 
> CPU drivers? Peter, you wrote the CCF clock provider driver for DRA7
> ATL, so I'm not sure what you mean here.

also the clk-twl6040, which is not in use upstream yet.
The ATL is providing the clocks to be used as master clock in McASP and
external components (audio codec).
In ASoC the CPU driver means the CPU DAI (Digital Audio Interface) driver,
like the McASP, McBSP, DMIC, McPDM in TI parts.
In order them to generate the needed clocks on the bus they have internal
clock selection, divider(s). From the SoC point of view these are not really
important, but McASP can also output high speed reference clock to outside so
the codec for example can use the same reference, reducing jitter.

>> You happen to be looking at a particularly simple system but things do
>> scale up and there's not a clear cutoff point which would allow us to
>> make a clear distinction between things that might get used in a simple
>> system and things that might need something more complex.  This seems
>> particularly important when we're adding things to simple-card, we want
>> it to be usable with as many different devices as possible.
> 
> The original patches didn't hit my inbox, only the last two replies. Can
> someone fill me in on the DT side of this discussion?

the original patch:
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-February/104320.html

> Why are DT bindings needed here?

ASoC have an API to configure the DAI system clock:
snd_soc_dai_set_sysclk(dai, sysclk_id, sysclk, sysclk_dir);

The sysclk_id is to select among the possible clock sources, sysclk is the
rate of the clock and sysclk_dir is to tell the DAI driver if the given clock
should be input or output.
McASP (and McBSP also) can select between two master clock source. aic3106 can
select between 3 reference clocks.
Unfortunately the simple-card was hardwired to only handle DAIs which have no
master clock selection. We have added additional properties to simple-card so
it will be possible to select clocks and directions.
With this change we don't need to write custom machine drivers for setup not
using sysclk_id == 0.
I do think this is reasonable change by itself.

> Are other devices besides McASP consuming the clocks provided by McASP?

Yes, it can output the same reference clock it is using to external pin, but
it can do that only if it is set to generate the audio clocks on the bus.

> DT can also be a good candidate for doing per-board (or per-use case)
> clock configuration via the assigned-clocks, assigned-clock-rates, and
> assigned-clock-parents properties.

Yes, if the DAI driver implements it's clock tree with CCF internally and with
the assigned-clock* properties we will not need snd_soc_dai_set_sysclk() and
we don't need simple-card to know anything about the clocking of the
components. All of this can be done in the board's DTS file with standard CCF
bindings.

Mark: after thinking over how this should work I can see that it can bring
benefits for the DAI/codec drivers. They need to do things differently, but it
is just implementation question.
However I do think that the current simple-card is flawed regarding to clock
selection and the change Jyri and me are proposing is reasonable.
But if we have the CCF providers popping up in DAI/codec drivers the use of
clock configuration via simple-card not going to be needed. But right know it
is needed.

-- 
Péter

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Mark Brown]

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

On Tue, Feb 16, 2016 at 11:13:46AM -0800, Michael Turquette wrote:
> Quoting Mark Brown (2016-02-16 05:42:33)
> > On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:

> > > I don't think it will bring any clarity or features we miss right now if we
> > > try to move CPU and codec drivers to clk API. IMHO.

> CPU drivers? Peter, you wrote the CCF clock provider driver for DRA7
> ATL, so I'm not sure what you mean here.

In this case he's referring to the drivers for the audio IP blocks
inside the SoC.

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

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Mark Brown]

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

On Wed, Feb 17, 2016 at 10:13:35AM +0200, Peter Ujfalusi wrote:

> With this change we don't need to write custom machine drivers for setup not
> using sysclk_id == 0.
> I do think this is reasonable change by itself.

> However I do think that the current simple-card is flawed regarding to clock
> selection and the change Jyri and me are proposing is reasonable.

But you define a new ABI to specify it in the process, I'd rather fix
the flaws by using the common clock ABI than extend any device stuff.
If it didn't define a new ABI I'd probably not worry about it but one of
the issues we have with DT is that we do end up making ABIs every time
we put something in DT.

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

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Ricard Wanderlof]

Quoting Mark Brown (2016-02-16 05:42:33)
> On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:
> 
> > As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
> > can receive it's reference clock via:
> > MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
> > frequency it can use it directly or it needs to use the internal PLL to
> > generate the cocks.
> > It can output generated clock via GPIO1
> 
> That already sounds like there is room for configuration and hooking
> into a wider clock tree - we've got three different source options and
> an output plus a PLL that can presumably take in non-audio rates.

It makes sense to use an already existing clock framework, but I'm 
wondering, how do we model the clock select function? Using a clock mux? 
Somewhere, somehow, something needs to look at the setup and decide which 
pin on the codec should be the clock source (e.g. MCLK, GPIO2, or BCLK in 
the case of the tlv320aic3106).

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Ricard Wanderlof]

Quoting Mark Brown (2016-02-16 05:42:33)
> On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:
> 
> > As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
> > can receive it's reference clock via:
> > MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
> > frequency it can use it directly or it needs to use the internal PLL to
> > generate the cocks.
> > It can output generated clock via GPIO1
> 
> That already sounds like there is room for configuration and hooking
> into a wider clock tree - we've got three different source options and
> an output plus a PLL that can presumably take in non-audio rates.

It makes sense to use an already existing clock framework, but I'm 
wondering, how do we model the clock select function? Using a clock mux? 
Somewhere, somehow, something needs to look at the setup and decide which 
pin on the codec should be the clock source (e.g. MCLK, GPIO2, or BCLK in 
the case of the tlv320aic3106).

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Ricard Wanderlof]

Quoting Mark Brown (2016-02-16 05:42:33)
> On Tue, Feb 16, 2016 at 11:46:52AM +0200, Peter Ujfalusi wrote:
> 
> > As for codecs, tlv320aic3106 is also pretty simple device from the outside, it
> > can receive it's reference clock via:
> > MCLK pin, GPIO2 pin or it can use the BCLK from the bus. Based on the incoming
> > frequency it can use it directly or it needs to use the internal PLL to
> > generate the cocks.
> > It can output generated clock via GPIO1
> 
> That already sounds like there is room for configuration and hooking
> into a wider clock tree - we've got three different source options and
> an output plus a PLL that can presumably take in non-audio rates.

It makes sense to use an already existing clock framework, but I'm 
wondering, how do we model the clock select function? Using a clock mux? 
Somewhere, somehow, something needs to look at the setup and decide which 
pin on the codec should be the clock source (e.g. MCLK, GPIO2, or BCLK in 
the case of the tlv320aic3106).

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 02/17/2016 02:07 PM, Mark Brown wrote:
> On Wed, Feb 17, 2016 at 10:13:35AM +0200, Peter Ujfalusi wrote:
> 
>> With this change we don't need to write custom machine drivers for setup not
>> using sysclk_id == 0.
>> I do think this is reasonable change by itself.
> 
>> However I do think that the current simple-card is flawed regarding to clock
>> selection and the change Jyri and me are proposing is reasonable.
> 
> But you define a new ABI to specify it in the process, I'd rather fix
> the flaws by using the common clock ABI than extend any device stuff.
> If it didn't define a new ABI I'd probably not worry about it but one of
> the issues we have with DT is that we do end up making ABIs every time
> we put something in DT.

I understand. This should have been supported by simple-card since the
beginning. When we tried to move all boards to use simple-card, we hit the
wall by not being able to select and configure other sysclk_id than 0.
I don't want to create yet another simple card which handles only sysclk_id 1 ;)
On the other hand this ABI is backwards compatible since if it is missing it
will default to the configuration we right now have regarding to sysclk_dir
and sysclk_id.

I will look at the CCF implementation for McASP first then for aic3x.

-- 
Péter

Re: [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 02/17/2016 02:07 PM, Mark Brown wrote:
> On Wed, Feb 17, 2016 at 10:13:35AM +0200, Peter Ujfalusi wrote:
> 
>> With this change we don't need to write custom machine drivers for setup not
>> using sysclk_id == 0.
>> I do think this is reasonable change by itself.
> 
>> However I do think that the current simple-card is flawed regarding to clock
>> selection and the change Jyri and me are proposing is reasonable.
> 
> But you define a new ABI to specify it in the process, I'd rather fix
> the flaws by using the common clock ABI than extend any device stuff.
> If it didn't define a new ABI I'd probably not worry about it but one of
> the issues we have with DT is that we do end up making ABIs every time
> we put something in DT.

I understand. This should have been supported by simple-card since the
beginning. When we tried to move all boards to use simple-card, we hit the
wall by not being able to select and configure other sysclk_id than 0.
I don't want to create yet another simple card which handles only sysclk_id 1 ;)
On the other hand this ABI is backwards compatible since if it is missing it
will default to the configuration we right now have regarding to sysclk_dir
and sysclk_id.

I will look at the CCF implementation for McASP first then for aic3x.

-- 
Péter

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Mark Brown]

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

On Wed, Feb 17, 2016 at 03:18:57PM +0100, Ricard Wanderlof wrote:

> It makes sense to use an already existing clock framework, but I'm 
> wondering, how do we model the clock select function? Using a clock mux? 

Yes, that looks like a mux to me.

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

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 02/17/2016 09:52 PM, Peter Ujfalusi wrote:
> On 02/17/2016 02:07 PM, Mark Brown wrote:
>> On Wed, Feb 17, 2016 at 10:13:35AM +0200, Peter Ujfalusi wrote:
>>
>>> With this change we don't need to write custom machine drivers for setup not
>>> using sysclk_id == 0.
>>> I do think this is reasonable change by itself.
>>
>>> However I do think that the current simple-card is flawed regarding to clock
>>> selection and the change Jyri and me are proposing is reasonable.
>>
>> But you define a new ABI to specify it in the process, I'd rather fix
>> the flaws by using the common clock ABI than extend any device stuff.
>> If it didn't define a new ABI I'd probably not worry about it but one of
>> the issues we have with DT is that we do end up making ABIs every time
>> we put something in DT.
> 
> I understand. This should have been supported by simple-card since the
> beginning. When we tried to move all boards to use simple-card, we hit the
> wall by not being able to select and configure other sysclk_id than 0.
> I don't want to create yet another simple card which handles only sysclk_id 1 ;)
> On the other hand this ABI is backwards compatible since if it is missing it
> will default to the configuration we right now have regarding to sysclk_dir
> and sysclk_id.
> 
> I will look at the CCF implementation for McASP first then for aic3x.

The first issue with converting the McASP to use CCF internally for clock
selection, muxing and rate configuration is that the daVinci platform does not
use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
need to have non CCF way supported in ASoC...

-- 
Péter

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 02/17/2016 09:52 PM, Peter Ujfalusi wrote:
> On 02/17/2016 02:07 PM, Mark Brown wrote:
>> On Wed, Feb 17, 2016 at 10:13:35AM +0200, Peter Ujfalusi wrote:
>>
>>> With this change we don't need to write custom machine drivers for setup not
>>> using sysclk_id == 0.
>>> I do think this is reasonable change by itself.
>>
>>> However I do think that the current simple-card is flawed regarding to clock
>>> selection and the change Jyri and me are proposing is reasonable.
>>
>> But you define a new ABI to specify it in the process, I'd rather fix
>> the flaws by using the common clock ABI than extend any device stuff.
>> If it didn't define a new ABI I'd probably not worry about it but one of
>> the issues we have with DT is that we do end up making ABIs every time
>> we put something in DT.
> 
> I understand. This should have been supported by simple-card since the
> beginning. When we tried to move all boards to use simple-card, we hit the
> wall by not being able to select and configure other sysclk_id than 0.
> I don't want to create yet another simple card which handles only sysclk_id 1 ;)
> On the other hand this ABI is backwards compatible since if it is missing it
> will default to the configuration we right now have regarding to sysclk_dir
> and sysclk_id.
> 
> I will look at the CCF implementation for McASP first then for aic3x.

The first issue with converting the McASP to use CCF internally for clock
selection, muxing and rate configuration is that the daVinci platform does not
use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
need to have non CCF way supported in ASoC...

-- 
Péter

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Mark Brown]

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

On Mon, Apr 18, 2016 at 06:50:52PM +0300, Peter Ujfalusi wrote:
> On 02/17/2016 09:52 PM, Peter Ujfalusi wrote:

> > On the other hand this ABI is backwards compatible since if it is missing it
> > will default to the configuration we right now have regarding to sysclk_dir
> > and sysclk_id.

> > I will look at the CCF implementation for McASP first then for aic3x.

> The first issue with converting the McASP to use CCF internally for clock
> selection, muxing and rate configuration is that the daVinci platform does not
> use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
> need to have non CCF way supported in ASoC...

Well, at least long term we do need daVinci converting to CCF - this is
going to continue to cause problems, devices not part of the SoC can and
do contain clocks and are going to end up being supported via the clock
API.

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

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Stephen Boyd]

On 04/18, Mark Brown wrote:
> On Mon, Apr 18, 2016 at 06:50:52PM +0300, Peter Ujfalusi wrote:
> > On 02/17/2016 09:52 PM, Peter Ujfalusi wrote:
> 
> > > On the other hand this ABI is backwards compatible since if it is missing it
> > > will default to the configuration we right now have regarding to sysclk_dir
> > > and sysclk_id.
> 
> > > I will look at the CCF implementation for McASP first then for aic3x.
> 
> > The first issue with converting the McASP to use CCF internally for clock
> > selection, muxing and rate configuration is that the daVinci platform does not
> > use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
> > need to have non CCF way supported in ASoC...
> 
> Well, at least long term we do need daVinci converting to CCF - this is
> going to continue to cause problems, devices not part of the SoC can and
> do contain clocks and are going to end up being supported via the clock
> API.

Does anyone here know what's involved in converting daVinci to
CCF? It doesn't look too far off from what is in the CCF today,
so I'm not sure what's blocking the transition.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 04/22/16 01:29, Stephen Boyd wrote:
>>> The first issue with converting the McASP to use CCF internally for clock
>>> selection, muxing and rate configuration is that the daVinci platform does not
>>> use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
>>> need to have non CCF way supported in ASoC...
>>
>> Well, at least long term we do need daVinci converting to CCF - this is
>> going to continue to cause problems, devices not part of the SoC can and
>> do contain clocks and are going to end up being supported via the clock
>> API.
> 
> Does anyone here know what's involved in converting daVinci to
> CCF? It doesn't look too far off from what is in the CCF today,
> so I'm not sure what's blocking the transition.

Not entirely sure, but most likely new clk driver(s) for daVinci under
drivers/clk/ti/ new set of structures to describe the clocks if the ti_clk* is
not applicable I guess for starter. Support for DT, non DT boots as most of
daVinci is not booting with DT and most likely never will.
It might help to have different daVinci boards for testing the transition. I
only have OMAP-L138-evm. I don't think it is enough for testing an entire
architecture for this big change...

Tero might have better estimates on what is involved when switching an
architecture to CCF from custom, but at least synchronized API - so we don't
need to convert drivers at least.

-- 
Péter

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Peter Ujfalusi]

On 04/22/16 01:29, Stephen Boyd wrote:
>>> The first issue with converting the McASP to use CCF internally for clock
>>> selection, muxing and rate configuration is that the daVinci platform does not
>>> use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
>>> need to have non CCF way supported in ASoC...
>>
>> Well, at least long term we do need daVinci converting to CCF - this is
>> going to continue to cause problems, devices not part of the SoC can and
>> do contain clocks and are going to end up being supported via the clock
>> API.
> 
> Does anyone here know what's involved in converting daVinci to
> CCF? It doesn't look too far off from what is in the CCF today,
> so I'm not sure what's blocking the transition.

Not entirely sure, but most likely new clk driver(s) for daVinci under
drivers/clk/ti/ new set of structures to describe the clocks if the ti_clk* is
not applicable I guess for starter. Support for DT, non DT boots as most of
daVinci is not booting with DT and most likely never will.
It might help to have different daVinci boards for testing the transition. I
only have OMAP-L138-evm. I don't think it is enough for testing an entire
architecture for this big change...

Tero might have better estimates on what is involved when switching an
architecture to CCF from custom, but at least synchronized API - so we don't
need to convert drivers at least.

-- 
Péter

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Tero Kristo]

On 22/04/16 14:52, Peter Ujfalusi wrote:
> On 04/22/16 01:29, Stephen Boyd wrote:
>>>> The first issue with converting the McASP to use CCF internally for clock
>>>> selection, muxing and rate configuration is that the daVinci platform does not
>>>> use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
>>>> need to have non CCF way supported in ASoC...
>>>
>>> Well, at least long term we do need daVinci converting to CCF - this is
>>> going to continue to cause problems, devices not part of the SoC can and
>>> do contain clocks and are going to end up being supported via the clock
>>> API.
>>
>> Does anyone here know what's involved in converting daVinci to
>> CCF? It doesn't look too far off from what is in the CCF today,
>> so I'm not sure what's blocking the transition.
>
> Not entirely sure, but most likely new clk driver(s) for daVinci under
> drivers/clk/ti/ new set of structures to describe the clocks if the ti_clk* is
> not applicable I guess for starter. Support for DT, non DT boots as most of
> daVinci is not booting with DT and most likely never will.
> It might help to have different daVinci boards for testing the transition. I
> only have OMAP-L138-evm. I don't think it is enough for testing an entire
> architecture for this big change...
>
> Tero might have better estimates on what is involved when switching an
> architecture to CCF from custom, but at least synchronized API - so we don't
> need to convert drivers at least.
>

Davinci is currently a mutant architecture, it is overriding the common 
clk APIs and using its own. Converting these to CCF may open a can of 
worms in many ways.

All the clock data should be converted to support CCF, (from 
arch/arm/mach-davinci/), along with whatever Peter said.

This also in a situation where many/most upstream people don't even have 
davinci devices... Personally I have a grand total of zero davinci 
boards on my desk so at least I am unable to work on this right now.

-Tero

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Tero Kristo]

On 22/04/16 14:52, Peter Ujfalusi wrote:
> On 04/22/16 01:29, Stephen Boyd wrote:
>>>> The first issue with converting the McASP to use CCF internally for clock
>>>> selection, muxing and rate configuration is that the daVinci platform does not
>>>> use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
>>>> need to have non CCF way supported in ASoC...
>>>
>>> Well, at least long term we do need daVinci converting to CCF - this is
>>> going to continue to cause problems, devices not part of the SoC can and
>>> do contain clocks and are going to end up being supported via the clock
>>> API.
>>
>> Does anyone here know what's involved in converting daVinci to
>> CCF? It doesn't look too far off from what is in the CCF today,
>> so I'm not sure what's blocking the transition.
>
> Not entirely sure, but most likely new clk driver(s) for daVinci under
> drivers/clk/ti/ new set of structures to describe the clocks if the ti_clk* is
> not applicable I guess for starter. Support for DT, non DT boots as most of
> daVinci is not booting with DT and most likely never will.
> It might help to have different daVinci boards for testing the transition. I
> only have OMAP-L138-evm. I don't think it is enough for testing an entire
> architecture for this big change...
>
> Tero might have better estimates on what is involved when switching an
> architecture to CCF from custom, but at least synchronized API - so we don't
> need to convert drivers at least.
>

Davinci is currently a mutant architecture, it is overriding the common 
clk APIs and using its own. Converting these to CCF may open a can of 
worms in many ways.

All the clock data should be converted to support CCF, (from 
arch/arm/mach-davinci/), along with whatever Peter said.

This also in a situation where many/most upstream people don't even have 
davinci devices... Personally I have a grand total of zero davinci 
boards on my desk so at least I am unable to work on this right now.

-Tero