Skip to contentSkip to conversation

Fix rxclk rate measurement

PATCH Superseded Nicolin Chen 2 participants 5 messages Apr 25, 2014 – Apr 28, 2014

Subsystems (3)
Sound ASoC Freescale SoC sound drivers

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Nicolin Chen

author
#0 lore raw

cover Fix rxclk rate measurement

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(-)
Signature · 1 line
1.8.4
author
#1 lore raw

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

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 <[email protected]>
---
 sound/soc/fsl/fsl_spdif.c | 12 +++++++++++-
 sound/soc/fsl/fsl_spdif.h |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)
sound/soc/fsl/fsl_spdif.c +11 -1
index e2836b3..4ce4ffa 100644
@@ -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)) {
sound/soc/fsl/fsl_spdif.h +2 -0
index 605a10b..f9241e7 100644
@@ -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,
Signature · 1 line
1.8.4
maintainer
#2 lore raw

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

sound/soc/fsl/fsl_spdif.c static int fsl_spdif_probe(struct platform_device *pdev) · line 1159

Quoted 8 lines · Nicolin Chen
sound/soc/fsl/fsl_spdif.c +8 -0
+	/* 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?

author
#3 lore raw

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

Quoted 13 lines · Mark Brown

sound/soc/fsl/fsl_spdif.c static int fsl_spdif_probe(struct platform_device *pdev) · line 1159

Quoted 8 lines · Nicolin Chen
sound/soc/fsl/fsl_spdif.c +8 -0
+	/* 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

author
#4 lore raw

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

In reply to 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 <[email protected]>
---
 sound/soc/fsl/fsl_spdif.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
sound/soc/fsl/fsl_spdif.c +3 -2
index 4ce4ffa..29b5da2 100644
@@ -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;
Signature · 1 line
1.8.4