[PATCH 0/2] Fix rxclk rate measurement

[PATCH 0/2] Fix rxclk rate measurement

[Nicolin Chen]

Add sysclk clock so as to calculate the rxclk rate correctly

Nicolin Chen (2):
  ASoC: fsl_spdif: Fix clock source for rxclk rate measurement
  ASoC: fsl_spdif: Drop hard code in clk_get() for rxclk

 sound/soc/fsl/fsl_spdif.c | 17 ++++++++++++++---
 sound/soc/fsl/fsl_spdif.h |  2 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

-- 
1.8.4

[PATCH 1/2] ASoC: fsl_spdif: Fix clock source for rxclk rate measurement

[Nicolin Chen]

The rxclk rate actually uses sysclk, ipg clock for example, as its
reference clock to calculate it. But the driver currently doesn't
pass a correct clock source. So fix it.

Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
---
 sound/soc/fsl/fsl_spdif.c | 12 +++++++++++-
 sound/soc/fsl/fsl_spdif.h |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index e2836b3..4ce4ffa 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -81,6 +81,7 @@ struct fsl_spdif_priv {
 	struct clk *txclk[SPDIF_TXRATE_MAX];
 	struct clk *rxclk;
 	struct clk *coreclk;
+	struct clk *sysclk;
 	struct snd_dmaengine_dai_dma_data dma_params_tx;
 	struct snd_dmaengine_dai_dma_data dma_params_rx;
 
@@ -767,7 +768,7 @@ static int spdif_get_rxclk_rate(struct fsl_spdif_priv *spdif_priv,
 	clksrc = (phaseconf >> SRPC_CLKSRC_SEL_OFFSET) & 0xf;
 	if (srpc_dpll_locked[clksrc] && (phaseconf & SRPC_DPLL_LOCKED)) {
 		/* Get bus clock from system */
-		busclk_freq = clk_get_rate(spdif_priv->rxclk);
+		busclk_freq = clk_get_rate(spdif_priv->sysclk);
 	}
 
 	/* FreqMeas_CLK = (BUS_CLK * FreqMeas) / 2 ^ 10 / GAINSEL / 128 */
@@ -1100,6 +1101,7 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *regs;
 	int irq, ret, i;
+	char tmp[16];
 
 	if (!np)
 		return -ENODEV;
@@ -1147,6 +1149,14 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* Get system clock for rx clock rate calculation */
+	sprintf(tmp, "rxtx%d", SPDIF_CLK_SRC_SYSCLK);
+	spdif_priv->sysclk = devm_clk_get(&pdev->dev, tmp);
+	if (IS_ERR(spdif_priv->sysclk)) {
+		dev_err(&pdev->dev, "no sys clock (%s) in devicetree\n", tmp);
+		return PTR_ERR(spdif_priv->sysclk);
+	}
+
 	/* Get core clock for data register access via DMA */
 	spdif_priv->coreclk = devm_clk_get(&pdev->dev, "core");
 	if (IS_ERR(spdif_priv->coreclk)) {
diff --git a/sound/soc/fsl/fsl_spdif.h b/sound/soc/fsl/fsl_spdif.h
index 605a10b..f9241e7 100644
--- a/sound/soc/fsl/fsl_spdif.h
+++ b/sound/soc/fsl/fsl_spdif.h
@@ -157,6 +157,8 @@ enum spdif_gainsel {
 #define STC_TXCLK_DIV(x)		((((x) - 1) << STC_TXCLK_DIV_OFFSET) & STC_TXCLK_DIV_MASK)
 #define STC_TXCLK_SRC_MAX		8
 
+#define SPDIF_CLK_SRC_SYSCLK		5
+
 /* SPDIF tx rate */
 enum spdif_txrate {
 	SPDIF_TXRATE_32000 = 0,
-- 
1.8.4

Re: [PATCH 1/2] ASoC: fsl_spdif: Fix clock source for rxclk rate measurement

[Mark Brown]

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

On Fri, Apr 25, 2014 at 07:58:19PM +0800, Nicolin Chen wrote:

> +	/* Get system clock for rx clock rate calculation */
> +	sprintf(tmp, "rxtx%d", SPDIF_CLK_SRC_SYSCLK);
> +	spdif_priv->sysclk = devm_clk_get(&pdev->dev, tmp);
> +	if (IS_ERR(spdif_priv->sysclk)) {
> +		dev_err(&pdev->dev, "no sys clock (%s) in devicetree\n", tmp);
> +		return PTR_ERR(spdif_priv->sysclk);
> +	}
> +

Why is this not just a fixed string - it seems like the clock name is a
constant anyway?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH 1/2] ASoC: fsl_spdif: Fix clock source for rxclk rate measurement

[Nicolin Chen]

On Fri, Apr 25, 2014 at 05:10:15PM +0100, Mark Brown wrote:
> On Fri, Apr 25, 2014 at 07:58:19PM +0800, Nicolin Chen wrote:
> 
> > +	/* Get system clock for rx clock rate calculation */
> > +	sprintf(tmp, "rxtx%d", SPDIF_CLK_SRC_SYSCLK);
> > +	spdif_priv->sysclk = devm_clk_get(&pdev->dev, tmp);
> > +	if (IS_ERR(spdif_priv->sysclk)) {
> > +		dev_err(&pdev->dev, "no sys clock (%s) in devicetree\n", tmp);
> > +		return PTR_ERR(spdif_priv->sysclk);
> > +	}
> > +
> 
> Why is this not just a fixed string - it seems like the clock name is a
> constant anyway?

For current version, yes, it always ties system clock to rxtx5. I was just
considering if further version changes the sysclk route to another source,
the driver will simply diversify this id here for different versions.

I'll later send a v2 for it to fix the name. And you can decide which one
would be better based on what I've just explained.

Thank you,
Nicolin

[PATCH 2/2] ASoC: fsl_spdif: Drop hard code in clk_get() for rxclk

[Nicolin Chen]

Then we will no longer need to change the name in dev_err() if one day
we are going to change the DEFAULT_RXCLK_SRC.

Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
---
 sound/soc/fsl/fsl_spdif.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 4ce4ffa..29b5da2 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1165,9 +1165,10 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 	}
 
 	/* Select clock source for rx/tx clock */
-	spdif_priv->rxclk = devm_clk_get(&pdev->dev, "rxtx1");
+	sprintf(tmp, "rxtx%d", DEFAULT_RXCLK_SRC);
+	spdif_priv->rxclk = devm_clk_get(&pdev->dev, tmp);
 	if (IS_ERR(spdif_priv->rxclk)) {
-		dev_err(&pdev->dev, "no rxtx1 clock in devicetree\n");
+		dev_err(&pdev->dev, "no %s clock in devicetree\n", tmp);
 		return PTR_ERR(spdif_priv->rxclk);
 	}
 	spdif_priv->rxclk_src = DEFAULT_RXCLK_SRC;
-- 
1.8.4