[PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[Nicolin Chen]

As commit 6c8ca30eec7b ("ASoC: fsl_ssi: Don't try to round-up for PM
divisor calculation") mentioned that there's no more need to use a
round up work around to get a better divisor since the clk-divider
driver has been refined a lot.

So this patch applies the same modification to fsl_spdif driver.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Zidan Wang <zidan.wang@freescale.com>
---

Fabio and Zidan,
Theoretically, it should have the same problem as fsl_ssi driver had.
But I don't have an S/PDIF test environment. So I need your helps to
confirm it. Thank you.

 sound/soc/fsl/fsl_spdif.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 91eb3ae..8e93221 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -417,11 +417,9 @@ static int spdif_set_sample_rate(struct snd_pcm_substream *substream,
 	if (clk != STC_TXCLK_SPDIF_ROOT)
 		goto clk_set_bypass;
 
-	/*
-	 * The S/PDIF block needs a clock of 64 * fs * txclk_df.
-	 * So request 64 * fs * (txclk_df + 1) to get rounded.
-	 */
-	ret = clk_set_rate(spdif_priv->txclk[rate], 64 * sample_rate * (txclk_df + 1));
+	/* The S/PDIF block needs a clock of 64 * fs * txclk_df */
+	ret = clk_set_rate(spdif_priv->txclk[rate],
+			   64 * sample_rate * txclk_df);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to set tx clock rate\n");
 		return ret;
@@ -1060,7 +1058,7 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
 
 	for (sysclk_df = sysclk_dfmin; sysclk_df <= sysclk_dfmax; sysclk_df++) {
 		for (txclk_df = 1; txclk_df <= 128; txclk_df++) {
-			rate_ideal = rate[index] * (txclk_df + 1) * 64;
+			rate_ideal = rate[index] * txclk_df * 64;
 			if (round)
 				rate_actual = clk_round_rate(clk, rate_ideal);
 			else
-- 
1.9.1

Re: [PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[Mark Brown]

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

On Sun, May 24, 2015 at 01:12:41AM -0700, Nicolin Chen wrote:
> As commit 6c8ca30eec7b ("ASoC: fsl_ssi: Don't try to round-up for PM
> divisor calculation") mentioned that there's no more need to use a
> round up work around to get a better divisor since the clk-divider
> driver has been refined a lot.

Applied, thanks.

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

Re: [PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[Nicolin Chen]

On Mon, May 25, 2015 at 12:58:12PM +0100, Mark Brown wrote:
> On Sun, May 24, 2015 at 01:12:41AM -0700, Nicolin Chen wrote:
> > As commit 6c8ca30eec7b ("ASoC: fsl_ssi: Don't try to round-up for PM
> > divisor calculation") mentioned that there's no more need to use a
> > round up work around to get a better divisor since the clk-divider
> > driver has been refined a lot.
> 
> Applied, thanks.

Hi Mark,

Is that possible for you to provisionally revert this patch?
I wanted to wait for the test result from Fabio or Zidan in
the Cc list because I don't have a test environment for SPDIF
even though this change seems to make sense.

Thank you
Nicolin

Re: [alsa-devel] [PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[Fabio Estevam]

Hi Nicolin,

On Mon, May 25, 2015 at 12:11 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:

> Hi Mark,
>
> Is that possible for you to provisionally revert this patch?
> I wanted to wait for the test result from Fabio or Zidan in
> the Cc list because I don't have a test environment for SPDIF
> even though this change seems to make sense.

I currently don't have access to a SPDIF receiver to test it.

Regards,

Fabio Estevam

Re: [alsa-devel] [PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[Nicolin Chen]

On Mon, May 25, 2015 at 12:13:45PM -0300, Fabio Estevam wrote:
> Hi Nicolin,
> 
> On Mon, May 25, 2015 at 12:11 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> 
> > Hi Mark,
> >
> > Is that possible for you to provisionally revert this patch?
> > I wanted to wait for the test result from Fabio or Zidan in
> > the Cc list because I don't have a test environment for SPDIF
> > even though this change seems to make sense.
> 
> I currently don't have access to a SPDIF receiver to test it.

Okay, let's wait for Zidan then. We only need to test the
playback route of supporting sample rates.

Thanks
Nicolin

Re: [alsa-devel] [PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[Zidan Wang]

On Mon, May 25, 2015 at 08:24:25AM -0700, Nicolin Chen wrote:
> On Mon, May 25, 2015 at 12:13:45PM -0300, Fabio Estevam wrote:
> > Hi Nicolin,
> > 
> > On Mon, May 25, 2015 at 12:11 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > 
> > > Hi Mark,
> > >
> > > Is that possible for you to provisionally revert this patch?
> > > I wanted to wait for the test result from Fabio or Zidan in
> > > the Cc list because I don't have a test environment for SPDIF
> > > even though this change seems to make sense.
> > 
> > I currently don't have access to a SPDIF receiver to test it.
> 
> Okay, let's wait for Zidan then. We only need to test the
> playback route of supporting sample rates.
> 
> Thanks
> Nicolin
I don't have the board which supported by community to test spdif out. So i used the imx7 board
and test it with internal branch.
I found that (txclk_df + 1) is better than txclk_df.
I suspect the patch for clk_round_rate() is not in our branch. Could you please tell me which
patch is for clk_round_rate? I want to cherry-pick it to our branch and test it.

Best Regards,
Zidan Wang

Re: [alsa-devel] [PATCH] ASoC: fsl_spdif: Don't try to round-up for clock divisor calculation

[Nicolin Chen]

On Tue, May 26, 2015 at 07:02:48PM +0800, Zidan Wang wrote:
> On Mon, May 25, 2015 at 08:24:25AM -0700, Nicolin Chen wrote:
> > On Mon, May 25, 2015 at 12:13:45PM -0300, Fabio Estevam wrote:
> > > Hi Nicolin,
> > > 
> > > On Mon, May 25, 2015 at 12:11 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > 
> > > > Hi Mark,
> > > >
> > > > Is that possible for you to provisionally revert this patch?
> > > > I wanted to wait for the test result from Fabio or Zidan in
> > > > the Cc list because I don't have a test environment for SPDIF
> > > > even though this change seems to make sense.
> > > 
> > > I currently don't have access to a SPDIF receiver to test it.
> > 
> > Okay, let's wait for Zidan then. We only need to test the
> > playback route of supporting sample rates.

> I don't have the board which supported by community to test spdif out. So i used the imx7 board

i.MX6 ARM2, the one with the square green base board, should work,
although you may add some device nodes in dts to make it work.

> and test it with internal branch.
> I found that (txclk_df + 1) is better than txclk_df.
> I suspect the patch for clk_round_rate() is not in our branch. Could you please tell me which
> patch is for clk_round_rate? I want to cherry-pick it to our branch and test it.

https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/drivers/clk/clk-divider.c

Here is commit logs for the clock driver. You can check git-log of
your local one and cherry-pick those incremental patches. Another
clue is that after updating the clock driver to the latest version,
SSI master mode in your local branch would become abnormal since
it's still using pm + 2 to round the calculation.

Thanks a lot
Nicolin