[alsa-devel] [PATCH] ASoC: fsl_esai: Clear the xPM bit when using xFP
When setting xFP directly, set the xPM predivider to 1, otherwise it could remain set to previously set incorrect value and interfere with the correct clocking.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Gustavo A. R. Silva garsilva@embeddedor.com Cc: Mark Brown broonie@kernel.org --- sound/soc/fsl/fsl_esai.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c index 40a700493f4c..9f69823b50d7 100644 --- a/sound/soc/fsl/fsl_esai.c +++ b/sound/soc/fsl/fsl_esai.c @@ -128,8 +128,11 @@ static int fsl_esai_divisor_cal(struct snd_soc_dai *dai, bool tx, u32 ratio,
maxfp = usefp ? 16 : 1;
- if (usefp && fp) + if (usefp && fp) { + regmap_update_bits(esai_priv->regmap, REG_ESAI_xCCR(tx), + ESAI_xCCR_xPM_MASK, 0); goto out_fp; + }
if (ratio > 2 * 8 * 256 * maxfp || ratio < 2) { dev_err(dai->dev, "the ratio is out of range (2 ~ %d)\n",
On Sat, Apr 7, 2018 at 10:02 AM, Marek Vasut marex@denx.de wrote:
When setting xFP directly, set the xPM predivider to 1, otherwise it could remain set to previously set incorrect value and interfere with the correct clocking.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Gustavo A. R. Silva garsilva@embeddedor.com Cc: Mark Brown broonie@kernel.org
Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
On Sat, Apr 07, 2018 at 03:02:21PM +0200, Marek Vasut wrote:
When setting xFP directly, set the xPM predivider to 1, otherwise it could remain set to previously set incorrect value and interfere with the correct clocking.
This doesn't sound right to me. Could you please provide a failed instance? It's been a while since I wrote the code. But I can tell that PM is supposed to be called by set_sysclk() only, while FP is used for bclk. If you clear PM when setting FP, the output of HCK could be messed.
Thanks Nicolin
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Gustavo A. R. Silva garsilva@embeddedor.com Cc: Mark Brown broonie@kernel.org
sound/soc/fsl/fsl_esai.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c index 40a700493f4c..9f69823b50d7 100644 --- a/sound/soc/fsl/fsl_esai.c +++ b/sound/soc/fsl/fsl_esai.c @@ -128,8 +128,11 @@ static int fsl_esai_divisor_cal(struct snd_soc_dai *dai, bool tx, u32 ratio,
maxfp = usefp ? 16 : 1;
- if (usefp && fp)
if (usefp && fp) {
regmap_update_bits(esai_priv->regmap, REG_ESAI_xCCR(tx),ESAI_xCCR_xPM_MASK, 0);goto out_fp;
}
if (ratio > 2 * 8 * 256 * maxfp || ratio < 2) { dev_err(dai->dev, "the ratio is out of range (2 ~ %d)\n",
-- 2.16.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 04/08/2018 03:14 AM, Nicolin Chen wrote:
On Sat, Apr 07, 2018 at 03:02:21PM +0200, Marek Vasut wrote:
When setting xFP directly, set the xPM predivider to 1, otherwise it could remain set to previously set incorrect value and interfere with the correct clocking.
This doesn't sound right to me.
OK
Could you please provide a failed instance? It's been a while since I wrote the code. But I can tell that PM is supposed to be called by set_sysclk() only, while FP is used for bclk. If you clear PM when setting FP, the output of HCK could be messed.
Try feeding it the following values, The codec I use is PCM1808.
fsl_esai_set_dai_sysclk[227] clk_id=0 freq=24576000 dir=1 fsl_esai_divisor_cal[131] tx=0 ratio=2 usefp=0 fp=0 fsl_esai_set_bclk[322] tx=0 freq=3072000 fsl_esai_divisor_cal[131] tx=0 ratio=8 usefp=1 fp=8
Also, I think there is another bug in the fsl_esai_divisor_cal() now that I look at it. If usefp = 0, then maxfp = 1 , then savesub = 0 and then in the loop sub is never < savesub , although the loop will terminate on savesub == 0 immediately with pm = 999 and the driver will fail.
Thanks Nicolin
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Gustavo A. R. Silva garsilva@embeddedor.com Cc: Mark Brown broonie@kernel.org
sound/soc/fsl/fsl_esai.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c index 40a700493f4c..9f69823b50d7 100644 --- a/sound/soc/fsl/fsl_esai.c +++ b/sound/soc/fsl/fsl_esai.c @@ -128,8 +128,11 @@ static int fsl_esai_divisor_cal(struct snd_soc_dai *dai, bool tx, u32 ratio,
maxfp = usefp ? 16 : 1;
- if (usefp && fp)
if (usefp && fp) {
regmap_update_bits(esai_priv->regmap, REG_ESAI_xCCR(tx),ESAI_xCCR_xPM_MASK, 0);goto out_fp;
}
if (ratio > 2 * 8 * 256 * maxfp || ratio < 2) { dev_err(dai->dev, "the ratio is out of range (2 ~ %d)\n",
-- 2.16.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Sun, Apr 08, 2018 at 04:16:39AM +0200, Marek Vasut wrote:
Could you please provide a failed instance? It's been a while since I wrote the code. But I can tell that PM is supposed to be called by set_sysclk() only, while FP is used for bclk. If you clear PM when setting FP, the output of HCK could be messed.
Try feeding it the following values, The codec I use is PCM1808.
[1] fsl_esai_set_dai_sysclk[227] clk_id=0 freq=24576000 dir=1 [2] fsl_esai_divisor_cal[131] tx=0 ratio=2 usefp=0 fp=0 [3] fsl_esai_set_bclk[322] tx=0 freq=3072000 [4] fsl_esai_divisor_cal[131] tx=0 ratio=8 usefp=1 fp=8
So [1] tries to output HCKT (using FSYS) and to set its freq at 24MHz. If [2] is correct, you'd have HCKT outputting 24MHz.
Let's put this in the graph: FSYS (48MHz) --> PSR & PM (input_rate / 2) --> HCKT (24MHz)
[3] tries to output SCKT (bit clock) at 3MHz. So you got fp=8 and usefp=1. The combination of usefp=1 and fp=8 should bypass PSR/PM settings because both should be correctly configured:
HCKT (24MHz) --> FP (input_rate / 8) --> SCKT (3MHz)
If you set PM=0 at [4], the HCKT at [2] would be changed. So your change shouldn't be a good fix. Meanwhile, I don't see any problem of the driver at the FP/PM part. I think you can help me figure it out as it might because of something else. You may dump the PSR, PM here to check if HCKT is 24MHz.
Also, I think there is another bug in the fsl_esai_divisor_cal() now that I look at it.
If usefp = 0, then maxfp = 1 , then savesub = 0 and
I see. This actually would happen when PSR=0. And the ratio in this case is <= 256 (which could be satisfied by PM only). Btw, is this the reason why you got a "dirty" PM?
Anyway, this part is a bug. Would you like to fix it?
Thanks Nicolin
On 04/08/2018 06:01 AM, Nicolin Chen wrote:
On Sun, Apr 08, 2018 at 04:16:39AM +0200, Marek Vasut wrote:
Could you please provide a failed instance? It's been a while since I wrote the code. But I can tell that PM is supposed to be called by set_sysclk() only, while FP is used for bclk. If you clear PM when setting FP, the output of HCK could be messed.
Try feeding it the following values, The codec I use is PCM1808.
[1] fsl_esai_set_dai_sysclk[227] clk_id=0 freq=24576000 dir=1 [2] fsl_esai_divisor_cal[131] tx=0 ratio=2 usefp=0 fp=0 [3] fsl_esai_set_bclk[322] tx=0 freq=3072000 [4] fsl_esai_divisor_cal[131] tx=0 ratio=8 usefp=1 fp=8
Sorry for confusing you, clk_id in [1] should be 3 . [...]
Also, I think there is another bug in the fsl_esai_divisor_cal() now that I look at it.
If usefp = 0, then maxfp = 1 , then savesub = 0 and
I see. This actually would happen when PSR=0. And the ratio in this case is <= 256 (which could be satisfied by PM only). Btw, is this the reason why you got a "dirty" PM?
Anyway, this part is a bug. Would you like to fix it?
Clearly I'm getting a bit lost on this convoluted clock calculation. What is the code trying so elaborately to come up with, ideal configuration for each of the clock ?
On Sun, Apr 08, 2018 at 01:00:07PM +0200, Marek Vasut wrote:
On 04/08/2018 06:01 AM, Nicolin Chen wrote:
On Sun, Apr 08, 2018 at 04:16:39AM +0200, Marek Vasut wrote:
Could you please provide a failed instance? It's been a while since I wrote the code. But I can tell that PM is supposed to be called by set_sysclk() only, while FP is used for bclk. If you clear PM when setting FP, the output of HCK could be messed.
Try feeding it the following values, The codec I use is PCM1808.
[1] fsl_esai_set_dai_sysclk[227] clk_id=0 freq=24576000 dir=1 [2] fsl_esai_divisor_cal[131] tx=0 ratio=2 usefp=0 fp=0 [3] fsl_esai_set_bclk[322] tx=0 freq=3072000 [4] fsl_esai_divisor_cal[131] tx=0 ratio=8 usefp=1 fp=8
Sorry for confusing you, clk_id in [1] should be 3 .
No worries, it doesn't change the program flow. But it makes sense now as the input is 48MHz.
[...]
Also, I think there is another bug in the fsl_esai_divisor_cal() now that I look at it.
If usefp = 0, then maxfp = 1 , then savesub = 0 and
I see. This actually would happen when PSR=0. And the ratio in this case is <= 256 (which could be satisfied by PM only). Btw, is this the reason why you got a "dirty" PM?
Anyway, this part is a bug. Would you like to fix it?
Clearly I'm getting a bit lost on this convoluted clock calculation. What is the code trying so elaborately to come up with, ideal configuration for each of the clock ?
It just tires to get the closet ratio to set three divisors correspondingly. Your case should be simpler but the program didn't cover. It's a design flaw. I will figure out the best fix and send it soon -- will put you in Reported-by and Cc.
Thanks
On 04/08/2018 09:27 PM, Nicolin Chen wrote:
On Sun, Apr 08, 2018 at 01:00:07PM +0200, Marek Vasut wrote:
On 04/08/2018 06:01 AM, Nicolin Chen wrote:
On Sun, Apr 08, 2018 at 04:16:39AM +0200, Marek Vasut wrote:
Could you please provide a failed instance? It's been a while since I wrote the code. But I can tell that PM is supposed to be called by set_sysclk() only, while FP is used for bclk. If you clear PM when setting FP, the output of HCK could be messed.
Try feeding it the following values, The codec I use is PCM1808.
[1] fsl_esai_set_dai_sysclk[227] clk_id=0 freq=24576000 dir=1 [2] fsl_esai_divisor_cal[131] tx=0 ratio=2 usefp=0 fp=0 [3] fsl_esai_set_bclk[322] tx=0 freq=3072000 [4] fsl_esai_divisor_cal[131] tx=0 ratio=8 usefp=1 fp=8
Sorry for confusing you, clk_id in [1] should be 3 .
No worries, it doesn't change the program flow. But it makes sense now as the input is 48MHz.
Except with this configuration , the audio doesn't work without this patch. I'm happy to receive any feedback on why or what is the problem.
[...]
Also, I think there is another bug in the fsl_esai_divisor_cal() now that I look at it.
If usefp = 0, then maxfp = 1 , then savesub = 0 and
I see. This actually would happen when PSR=0. And the ratio in this case is <= 256 (which could be satisfied by PM only). Btw, is this the reason why you got a "dirty" PM?
Anyway, this part is a bug. Would you like to fix it?
Clearly I'm getting a bit lost on this convoluted clock calculation. What is the code trying so elaborately to come up with, ideal configuration for each of the clock ?
It just tires to get the closet ratio to set three divisors correspondingly. Your case should be simpler but the program didn't cover. It's a design flaw. I will figure out the best fix and send it soon -- will put you in Reported-by and Cc.
OK. I wonder though, don't we have code to calculate dividers in the clk framework already? Why is it reinvented in here ?
Thanks
On Sun, Apr 08, 2018 at 09:33:52PM +0200, Marek Vasut wrote:
Try feeding it the following values, The codec I use is PCM1808.
[1] fsl_esai_set_dai_sysclk[227] clk_id=0 freq=24576000 dir=1 [2] fsl_esai_divisor_cal[131] tx=0 ratio=2 usefp=0 fp=0 [3] fsl_esai_set_bclk[322] tx=0 freq=3072000 [4] fsl_esai_divisor_cal[131] tx=0 ratio=8 usefp=1 fp=8
Sorry for confusing you, clk_id in [1] should be 3 .
No worries, it doesn't change the program flow. But it makes sense now as the input is 48MHz.
Except with this configuration , the audio doesn't work without this patch. I'm happy to receive any feedback on why or what is the problem.
I sent a patch to you and in the maillist for review. Would you please test it and sent a Tested-by to the patch? Thanks
[...]
Also, I think there is another bug in the fsl_esai_divisor_cal() now that I look at it.
If usefp = 0, then maxfp = 1 , then savesub = 0 and
I see. This actually would happen when PSR=0. And the ratio in this case is <= 256 (which could be satisfied by PM only). Btw, is this the reason why you got a "dirty" PM?
Anyway, this part is a bug. Would you like to fix it?
Clearly I'm getting a bit lost on this convoluted clock calculation. What is the code trying so elaborately to come up with, ideal configuration for each of the clock ?
It just tires to get the closet ratio to set three divisors correspondingly. Your case should be simpler but the program didn't cover. It's a design flaw. I will figure out the best fix and send it soon -- will put you in Reported-by and Cc.
OK. I wonder though, don't we have code to calculate dividers in the clk framework already? Why is it reinvented in here ?
Well, I wrote the code four years ago, based on a much older downstream kernel; the code also got upstream that time too. Even NXP's latest code doesn't seemly have some diff at this part. I guess no one had reported this bug to them or to me until you did yesterday...
On 04/09/2018 02:07 AM, Nicolin Chen wrote:
On Sun, Apr 08, 2018 at 09:33:52PM +0200, Marek Vasut wrote:
Try feeding it the following values, The codec I use is PCM1808.
[1] fsl_esai_set_dai_sysclk[227] clk_id=0 freq=24576000 dir=1 [2] fsl_esai_divisor_cal[131] tx=0 ratio=2 usefp=0 fp=0 [3] fsl_esai_set_bclk[322] tx=0 freq=3072000 [4] fsl_esai_divisor_cal[131] tx=0 ratio=8 usefp=1 fp=8
Sorry for confusing you, clk_id in [1] should be 3 .
No worries, it doesn't change the program flow. But it makes sense now as the input is 48MHz.
Except with this configuration , the audio doesn't work without this patch. I'm happy to receive any feedback on why or what is the problem.
I sent a patch to you and in the maillist for review. Would you please test it and sent a Tested-by to the patch? Thanks
Done, thanks for your help! :)
participants (3)
-
Fabio Estevam -
Marek Vasut -
Nicolin Chen