Re: [PATCH] ASoC: davinci-mcasp: Add clocking option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do CC the maintainers when sending patches.

On 07/26/16 15:05, Stefan Müller-Klieser wrote:
> The AHCLKX clock output of the McASP can be used as audio MCLK, even
> if the McASP runs in slave mode. Add a DT binding to make this option
> available in a simple-card setup.

In other words: McASP is clock master, but we want to have the HCLK to be
coming from internal AUXCLK and not from the AHCLKX pin, right?
There might be also cases when McASP is slave _and_ we want to configure the
HCLK from the AHCLKX ping. Adding new flag for this is not a good idea IMHO.

This is simply a workaround for the current simple card and might break if
something is changing in there..

I'm aware that this is a problematic issue. We can not have clock id support
in the simple-card and I don't think it is a good idea to hack around this in
the McASP driver.
I don't see any other way right now then moving the McASP driver to CCF, or at
first probably add CCF support to configure the clock routes while still
keeping the ASoC way of configuring the clocks.

> Signed-off-by: Stefan Müller-Klieser <s.mueller-klieser@xxxxxxxxx>
> ---
>  Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt | 4 ++++
>  include/linux/platform_data/davinci_asp.h                       | 5 +++++
>  sound/soc/davinci/davinci-mcasp.c                               | 8 ++++++++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt
> index 46bc982..96d6244 100644
> --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt
> +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt
> @@ -39,6 +39,10 @@ Optional properties:
>    		 please refer to pinctrl-bindings.txt
>  - fck_parent : Should contain a valid clock name which will be used as parent
>  	       for the McASP fck
> +- ahclkx-en: In case the MCASP runs in slave mode, e.g. i2s SCK and WS slave,
> +             we can still provide the MCLK to a codec PLL using the AHCLKX
> +             transmit clock output. This setup can be activated by using this
> +             property. Clock master mode remains unaffected.

_if_ we hack this around in the driver I would rather have something like:
hclk-source: Source of HCLK internal clock when McASP is clock master.
With a new DT binding header file with defines:

/* Source of High-frequency transmit/receive clock */
#define MCASP_CLK_HCLK_AHCLK		0 /* AHCLKX/R */
#define MCASP_CLK_HCLK_AUXCLK		1 /* Internal functional clock */

Basically my old patches for McASP clock selection but w/o the improving the
simple card.
The only change would be that we ignore the set_sysclk's clk_id parameter and
use the private clk_id we got via DT when the dir is SND_SOC_CLOCK_IN.
Oh, and set the PDIR also when AUXCLK is selected for HCLK.
In the DT you would have for your setup:

hclk-source = MCASP_CLK_HCLK_AUXCLK;

Or something like that.

But I think in this way we can work around the simple-card posed limitation a
bit better, but it is still a questionable workaround.

I have attached my local patches for reference.

>  
>  Example:
>  
> diff --git a/include/linux/platform_data/davinci_asp.h b/include/linux/platform_data/davinci_asp.h
> index 85ad68f..c878836 100644
> --- a/include/linux/platform_data/davinci_asp.h
> +++ b/include/linux/platform_data/davinci_asp.h
> @@ -44,6 +44,11 @@ struct davinci_mcasp_pdata {
>  	int clk_input_pin;
>  
>  	/*
> +	 * Activate AHCLKX clock output in slave mode.
> +	 */
> +	bool ahclkx_en;
> +
> +	/*
>  	 * This flag works when both clock and FS are outputs for the cpu
>  	 * and makes clock more accurate (FS is not symmetrical and the
>  	 * clock is very fast.
> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
> index 237dc67..aba888e 100644
> --- a/sound/soc/davinci/davinci-mcasp.c
> +++ b/sound/soc/davinci/davinci-mcasp.c
> @@ -94,6 +94,7 @@ struct davinci_mcasp {
>  
>  	int	sysclk_freq;
>  	bool	bclk_master;
> +	bool    ahclkx_en;
>  
>  	/* McASP FIFO related */
>  	u8	txnumevt;
> @@ -605,6 +606,10 @@ static int davinci_mcasp_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>  		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 if (mcasp->ahclkx_en) {
> +		mcasp_clr_bits(mcasp, DAVINCI_MCASP_AHCLKRCTL_REG, AHCLKRE);

It is safe to set this bit as well.

> +		mcasp_set_bits(mcasp, DAVINCI_MCASP_AHCLKXCTL_REG, AHCLKXE);
> +		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);
> @@ -1610,6 +1615,8 @@ static struct davinci_mcasp_pdata *davinci_mcasp_set_pdata_from_of(
>  	if (ret >= 0)
>  		pdata->op_mode = val;
>  
> +	pdata->ahclkx_en = of_property_read_bool(np, "ahclkx-en");
> +
>  	ret = of_property_read_u32(np, "tdm-slots", &val);
>  	if (ret >= 0) {
>  		if (val < 2 || val > 32) {
> @@ -1851,6 +1858,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
>  	mcasp->version = pdata->version;
>  	mcasp->txnumevt = pdata->txnumevt;
>  	mcasp->rxnumevt = pdata->rxnumevt;
> +	mcasp->ahclkx_en = pdata->ahclkx_en;
>  
>  	mcasp->dev = &pdev->dev;
>  
> 


-- 
Péter
From 407288736709e922e5ade6803e255d0e52a57e66 Mon Sep 17 00:00:00 2001
From: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
Date: Wed, 4 May 2016 16:26:42 +0300
Subject: [PATCH 1/2] ASoC: davinci-mcasp: Move the clkdiv IDs to DT binding
 header

Move the clkdiv defines to a new binding header file so they ca be used in
DT files as well.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
---
 include/dt-bindings/sound/ti-mcasp.h | 9 +++++++++
 sound/soc/davinci/davinci-mcasp.c    | 1 +
 sound/soc/davinci/davinci-mcasp.h    | 5 -----
 3 files changed, 10 insertions(+), 5 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 05c2d33aa74d..8753f36755cf 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"
diff --git a/sound/soc/davinci/davinci-mcasp.h b/sound/soc/davinci/davinci-mcasp.h
index afddc8010c54..10b52d6bdeca 100644
--- a/sound/soc/davinci/davinci-mcasp.h
+++ b/sound/soc/davinci/davinci-mcasp.h
@@ -306,9 +306,4 @@
 #define NUMEVT(x)	(((x) & 0xFF) << 8)
 #define NUMDMA_MASK	(0xFF)
 
-/* 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	/* DAVINCI_MCASP_H */
-- 
2.9.2

From c7ef1ef97266a06591706c82c3e298884202c221 Mon Sep 17 00:00:00 2001
From: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
Date: Mon, 18 Jan 2016 15:00:07 +0200
Subject: [PATCH 2/2] ASoC: davinci-mcasp: Improve the sysclk selection

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@xxxxxx>
---
 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 8753f36755cf..53d71c5dbe37 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -602,18 +602,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.9.2

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux