[PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[Alexander Shiyan]

The i.MX51 datasheet says:
Chapter 56.1.2.4 I2S Mode
...
When I2S modes are entered (I2S master (01) or I2S slave (10)),
the following settings are recommended:
...
- TX Frame Rate should be 2 i.e. (STCCR[12:8] = 1)
- RX Frame Rate should be 2 i.e. (SRCCR[12:8] = 1)

Chapter 56.3.3.12 SSI Transmit and Receive Clock Control Registers (STCCR & SRCCR)
...
Bits 12-8 DC4-DC0
Frame Rate Divider Control. These bits are used to control the divide ratio
for the programmable frame rate dividers. The divide ratio works on the word
clock. In Normal mode, this ratio determines the word transfer rate.
In Network mode, this ratio sets the number of words per frame. The divide
ratio ranges from 1 to 32 in Normal mode and from 2 to 32 in Network mode.
In Normal mode, a divide ratio of 1 (DC=00000) provides continuous periodic
data word transfer. A bit-length frame sync must be used in this case.

Function fsl_ssi_hw_params() setup Normal mode for MONO output,
so with DC=0, SSI enters to continuous periodic data word transfer.
To fix this, setup DC for any I2S mode.
Patch has tested on custom board based on Digi CCMX-51 module (i.MX51).

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 sound/soc/fsl/fsl_ssi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 632ecc0..bedec4a 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -952,16 +952,16 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 	ssi_private->i2s_mode = CCSR_SSI_SCR_NET;
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+		regmap_update_bits(regs, CCSR_SSI_STCCR,
+				   CCSR_SSI_SxCCR_DC_MASK,
+				   CCSR_SSI_SxCCR_DC(2));
+		regmap_update_bits(regs, CCSR_SSI_SRCCR,
+				   CCSR_SSI_SxCCR_DC_MASK,
+				   CCSR_SSI_SxCCR_DC(2));
 		switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 		case SND_SOC_DAIFMT_CBM_CFS:
 		case SND_SOC_DAIFMT_CBS_CFS:
 			ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_MASTER;
-			regmap_update_bits(regs, CCSR_SSI_STCCR,
-					CCSR_SSI_SxCCR_DC_MASK,
-					CCSR_SSI_SxCCR_DC(2));
-			regmap_update_bits(regs, CCSR_SSI_SRCCR,
-					CCSR_SSI_SxCCR_DC_MASK,
-					CCSR_SSI_SxCCR_DC(2));
 			break;
 		case SND_SOC_DAIFMT_CBM_CFM:
 			ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE;
-- 
2.4.9

Re: [PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[Nicolin Chen]

On Sat, Jun 25, 2016 at 07:59:22AM +0300, Alexander Shiyan wrote:
> The i.MX51 datasheet says:
> Chapter 56.1.2.4 I2S Mode
> ...
> When I2S modes are entered (I2S master (01) or I2S slave (10)),
> the following settings are recommended:
> ...
> - TX Frame Rate should be 2 i.e. (STCCR[12:8] = 1)
> - RX Frame Rate should be 2 i.e. (SRCCR[12:8] = 1)
> 
> Chapter 56.3.3.12 SSI Transmit and Receive Clock Control Registers (STCCR & SRCCR)
> ...
> Bits 12-8 DC4-DC0
> Frame Rate Divider Control. These bits are used to control the divide ratio
> for the programmable frame rate dividers. The divide ratio works on the word
> clock. In Normal mode, this ratio determines the word transfer rate.
> In Network mode, this ratio sets the number of words per frame. The divide
> ratio ranges from 1 to 32 in Normal mode and from 2 to 32 in Network mode.
> In Normal mode, a divide ratio of 1 (DC=00000) provides continuous periodic
> data word transfer. A bit-length frame sync must be used in this case.
> 
> Function fsl_ssi_hw_params() setup Normal mode for MONO output,
> so with DC=0, SSI enters to continuous periodic data word transfer.
> To fix this, setup DC for any I2S mode.

The change looks fine and didn't show any problem when I tested it.
But I am wondering what's the failed user case here so we need this
patch to fix? (The mono playback works without this patch as well.)

Thanks
Nicolin

Re: [PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[Nicolin Chen]

On Sat, Jun 25, 2016 at 07:59:22AM +0300, Alexander Shiyan wrote:
> The i.MX51 datasheet says:
> Chapter 56.1.2.4 I2S Mode
> ...
> When I2S modes are entered (I2S master (01) or I2S slave (10)),
> the following settings are recommended:
> ...
> - TX Frame Rate should be 2 i.e. (STCCR[12:8] = 1)
> - RX Frame Rate should be 2 i.e. (SRCCR[12:8] = 1)
> 
> Chapter 56.3.3.12 SSI Transmit and Receive Clock Control Registers (STCCR & SRCCR)
> ...
> Bits 12-8 DC4-DC0
> Frame Rate Divider Control. These bits are used to control the divide ratio
> for the programmable frame rate dividers. The divide ratio works on the word
> clock. In Normal mode, this ratio determines the word transfer rate.
> In Network mode, this ratio sets the number of words per frame. The divide
> ratio ranges from 1 to 32 in Normal mode and from 2 to 32 in Network mode.
> In Normal mode, a divide ratio of 1 (DC=00000) provides continuous periodic
> data word transfer. A bit-length frame sync must be used in this case.
> 
> Function fsl_ssi_hw_params() setup Normal mode for MONO output,
> so with DC=0, SSI enters to continuous periodic data word transfer.
> To fix this, setup DC for any I2S mode.
> Patch has tested on custom board based on Digi CCMX-51 module (i.MX51).
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>

Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>

Thank you

> ---
>  sound/soc/fsl/fsl_ssi.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 632ecc0..bedec4a 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -952,16 +952,16 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
>  	ssi_private->i2s_mode = CCSR_SSI_SCR_NET;
>  	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>  	case SND_SOC_DAIFMT_I2S:
> +		regmap_update_bits(regs, CCSR_SSI_STCCR,
> +				   CCSR_SSI_SxCCR_DC_MASK,
> +				   CCSR_SSI_SxCCR_DC(2));
> +		regmap_update_bits(regs, CCSR_SSI_SRCCR,
> +				   CCSR_SSI_SxCCR_DC_MASK,
> +				   CCSR_SSI_SxCCR_DC(2));
>  		switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
>  		case SND_SOC_DAIFMT_CBM_CFS:
>  		case SND_SOC_DAIFMT_CBS_CFS:
>  			ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_MASTER;
> -			regmap_update_bits(regs, CCSR_SSI_STCCR,
> -					CCSR_SSI_SxCCR_DC_MASK,
> -					CCSR_SSI_SxCCR_DC(2));
> -			regmap_update_bits(regs, CCSR_SSI_SRCCR,
> -					CCSR_SSI_SxCCR_DC_MASK,
> -					CCSR_SSI_SxCCR_DC(2));
>  			break;
>  		case SND_SOC_DAIFMT_CBM_CFM:
>  			ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE;
> -- 
> 2.4.9
> 

Re: [PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[Nicolin Chen]

On Tue, Jun 28, 2016 at 10:59:32PM +0300, Alexander Shiyan wrote:
> >Вторник, 28 июня 2016, 21:20 +03:00 от Nicolin Chen <nicoleotsuka@gmail.com>:
> >
> >On Sat, Jun 25, 2016 at 07:59:22AM +0300, Alexander Shiyan wrote:
> >> The i.MX51 datasheet says:
> >> Chapter 56.1.2.4 I2S Mode
> >> ...
> >> When I2S modes are entered (I2S master (01) or I2S slave (10)),
> >> the following settings are recommended:
> >> ...
> >> - TX Frame Rate should be 2 i.e. (STCCR[12:8] = 1)
> >> - RX Frame Rate should be 2 i.e. (SRCCR[12:8] = 1)
> >> 
> >> Chapter 56.3.3.12 SSI Transmit and Receive Clock Control Registers (STCCR & SRCCR)
> >> ...
> >> Bits 12-8 DC4-DC0
> >> Frame Rate Divider Control. These bits are used to control the divide ratio
> >> for the programmable frame rate dividers. The divide ratio works on the word
> >> clock. In Normal mode, this ratio determines the word transfer rate.
> >> In Network mode, this ratio sets the number of words per frame. The divide
> >> ratio ranges from 1 to 32 in Normal mode and from 2 to 32 in Network mode.
> >> In Normal mode, a divide ratio of 1 (DC=00000) provides continuous periodic
> >> data word transfer. A bit-length frame sync must be used in this case.
> >> 
> >> Function fsl_ssi_hw_params() setup Normal mode for MONO output,
> >> so with DC=0, SSI enters to continuous periodic data word transfer.
> >> To fix this, setup DC for any I2S mode.
> >
> >The change looks fine and didn't show any problem when I tested it.
> >But I am wondering what's the failed user case here so we need this
> >patch to fix? (The mono playback works without this patch as well.)
> 
> Probably this works fine without patch if FSclk == BITclk * size.

I acked this patch since it's more likely a correct configuration
according to the reference manual while still want to know what's
your failed case. Could you please tell me what were the FSclk,
BITclk and size of your case?

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

Re: [PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[Alexander Shiyan]

>Среда, 29 июня 2016, 1:06 +03:00 от Nicolin Chen <nicoleotsuka@gmail.com>:
>
>On Tue, Jun 28, 2016 at 10:59:32PM +0300, Alexander Shiyan wrote:
>> >Вторник, 28 июня 2016, 21:20 +03:00 от Nicolin Chen < nicoleotsuka@gmail.com >:
>> >
>> >On Sat, Jun 25, 2016 at 07:59:22AM +0300, Alexander Shiyan wrote:
>> >> The i.MX51 datasheet says:
>> >> Chapter 56.1.2.4 I2S Mode
>> >> ...
>> >> When I2S modes are entered (I2S master (01) or I2S slave (10)),
>> >> the following settings are recommended:
>> >> ...
>> >> - TX Frame Rate should be 2 i.e. (STCCR[12:8] = 1)
>> >> - RX Frame Rate should be 2 i.e. (SRCCR[12:8] = 1)
>> >> 
>> >> Chapter 56.3.3.12 SSI Transmit and Receive Clock Control Registers (STCCR & SRCCR)
>> >> ...
>> >> Bits 12-8 DC4-DC0
>> >> Frame Rate Divider Control. These bits are used to control the divide ratio
>> >> for the programmable frame rate dividers. The divide ratio works on the word
>> >> clock. In Normal mode, this ratio determines the word transfer rate.
>> >> In Network mode, this ratio sets the number of words per frame. The divide
>> >> ratio ranges from 1 to 32 in Normal mode and from 2 to 32 in Network mode.
>> >> In Normal mode, a divide ratio of 1 (DC=00000) provides continuous periodic
>> >> data word transfer. A bit-length frame sync must be used in this case.
>> >> 
>> >> Function fsl_ssi_hw_params() setup Normal mode for MONO output,
>> >> so with DC=0, SSI enters to continuous periodic data word transfer.
>> >> To fix this, setup DC for any I2S mode.
>> >
>> >The change looks fine and didn't show any problem when I tested it.
>> >But I am wondering what's the failed user case here so we need this
>> >patch to fix? (The mono playback works without this patch as well.)
>> 
>> Probably this works fine without patch if FSclk == BITclk * size.
>
>I acked this patch since it's more likely a correct configuration
>according to the reference manual while still want to know what's
>your failed case. Could you please tell me what were the FSclk,
>BITclk and size of your case?

Hello.

Here is my oscillograms with/without patch:
Bclk=12 MHz, sound is just a "cat /dev/urandom > /dev/dsp", i.e. FS=8 KHz, MONO.
Upper (yellow) line is TXDATA, Lower (blue) is FS.

Measurement without patch: https://cloud.mail.ru/public/Rcsv/xr88V5Sa1
Measurement with patch: https://cloud.mail.ru/public/NCwn/DQnzioQEL

Thanks.
---

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

Applied "ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode" to the asoc tree

[Mark Brown]

The patch

   ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 4f14f5c11db161ab89b02f7196496ca32ca5dbf8 Mon Sep 17 00:00:00 2001
From: Alexander Shiyan <shc_work@mail.ru>
Date: Sat, 25 Jun 2016 07:59:22 +0300
Subject: [PATCH] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave
 mode

The i.MX51 datasheet says:
Chapter 56.1.2.4 I2S Mode
...
When I2S modes are entered (I2S master (01) or I2S slave (10)),
the following settings are recommended:
...
- TX Frame Rate should be 2 i.e. (STCCR[12:8] = 1)
- RX Frame Rate should be 2 i.e. (SRCCR[12:8] = 1)

Chapter 56.3.3.12 SSI Transmit and Receive Clock Control Registers (STCCR & SRCCR)
...
Bits 12-8 DC4-DC0
Frame Rate Divider Control. These bits are used to control the divide ratio
for the programmable frame rate dividers. The divide ratio works on the word
clock. In Normal mode, this ratio determines the word transfer rate.
In Network mode, this ratio sets the number of words per frame. The divide
ratio ranges from 1 to 32 in Normal mode and from 2 to 32 in Network mode.
In Normal mode, a divide ratio of 1 (DC=00000) provides continuous periodic
data word transfer. A bit-length frame sync must be used in this case.

Function fsl_ssi_hw_params() setup Normal mode for MONO output,
so with DC=0, SSI enters to continuous periodic data word transfer.
To fix this, setup DC for any I2S mode.
Patch has tested on custom board based on Digi CCMX-51 module (i.MX51).

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/fsl/fsl_ssi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 632ecc0e3956..bedec4a32581 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -952,16 +952,16 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 	ssi_private->i2s_mode = CCSR_SSI_SCR_NET;
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+		regmap_update_bits(regs, CCSR_SSI_STCCR,
+				   CCSR_SSI_SxCCR_DC_MASK,
+				   CCSR_SSI_SxCCR_DC(2));
+		regmap_update_bits(regs, CCSR_SSI_SRCCR,
+				   CCSR_SSI_SxCCR_DC_MASK,
+				   CCSR_SSI_SxCCR_DC(2));
 		switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 		case SND_SOC_DAIFMT_CBM_CFS:
 		case SND_SOC_DAIFMT_CBS_CFS:
 			ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_MASTER;
-			regmap_update_bits(regs, CCSR_SSI_STCCR,
-					CCSR_SSI_SxCCR_DC_MASK,
-					CCSR_SSI_SxCCR_DC(2));
-			regmap_update_bits(regs, CCSR_SSI_SRCCR,
-					CCSR_SSI_SxCCR_DC_MASK,
-					CCSR_SSI_SxCCR_DC(2));
 			break;
 		case SND_SOC_DAIFMT_CBM_CFM:
 			ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE;
-- 
2.8.1

Re: [PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[Nicolin Chen]

On Wed, Jun 29, 2016 at 10:52:01AM +0300, Alexander Shiyan wrote:

> >your failed case. Could you please tell me what were the FSclk,
> >BITclk and size of your case?
 
> Here is my oscillograms with/without patch:
> Bclk=12 MHz, sound is just a "cat /dev/urandom > /dev/dsp", i.e. FS=8 KHz, MONO.
> Upper (yellow) line is TXDATA, Lower (blue) is FS.
> 
> Measurement without patch: https://cloud.mail.ru/public/Rcsv/xr88V5Sa1
> Measurement with patch: https://cloud.mail.ru/public/NCwn/DQnzioQEL

Thanks for the reply. So in the case without patch, it looks like
SSI just sent the data ignoring the slot size right?

Re: [PATCH 2/2] ASoC: fsl_ssi: Fix number of words per frame for I2S-slave mode

[Nicolin Chen]

On Wed, Jun 29, 2016 at 09:50:24PM +0300, Alexander Shiyan wrote:
> >Среда, 29 июня 2016, 21:26 +03:00 от Nicolin Chen <nicoleotsuka@gmail.com>:
> >
> >On Wed, Jun 29, 2016 at 10:52:01AM +0300, Alexander Shiyan wrote:
> >
> >> >your failed case. Could you please tell me what were the FSclk,
> >> >BITclk and size of your case?
> > 
> >> Here is my oscillograms with/without patch:
> >> Bclk=12 MHz, sound is just a "cat /dev/urandom > /dev/dsp", i.e. FS=8 KHz, MONO.
> >> Upper (yellow) line is TXDATA, Lower (blue) is FS.
> >> 
> >> Measurement without patch:  https://cloud.mail.ru/public/Rcsv/xr88V5Sa1
> >> Measurement with patch:  https://cloud.mail.ru/public/NCwn/DQnzioQEL
> >
> >Thanks for the reply. So in the case without patch, it looks like
> >SSI just sent the data ignoring the slot size right?
> 
> I could be wrong, but think that WL (word length) still point to number of valid
> data bits within 32bit chunk (as defined for I2S-Master mode),
> but since DC=0, this cause to send next word to the channel again and again.

I see. Thanks for the fix.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel