diff mbox

ASoC: imx-ssi: Fix DAI hardware signal inversions

Message ID 1442504814-16965-1-git-send-email-benoit@wsystem.com (mailing list archive)
State Accepted
Commit b7b01d345b83602a42b6ff02cacb9d9ada5ecd0a
Headers show

Commit Message

Benoît Thébaudeau Sept. 17, 2015, 3:46 p.m. UTC
SND_SOC_DAIFMT_{IB|NB}_{IF|NF} are defined as inverting or not BCLK or
FRM relatively to what is standard for the specified DAI hardware audio
format. Consequently, the absolute polarities of these signals cannot be
derived only from these settings as this driver did. The format has to
be taken into account too.

This fixes inverted left/right channels in I²S mode.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 sound/soc/fsl/imx-ssi.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

Comments

Nicolin Chen Sept. 17, 2015, 5:59 p.m. UTC | #1
On Thu, Sep 17, 2015 at 05:46:54PM +0200, Benoît Thébaudeau wrote:
> SND_SOC_DAIFMT_{IB|NB}_{IF|NF} are defined as inverting or not BCLK or
> FRM relatively to what is standard for the specified DAI hardware audio
> format. Consequently, the absolute polarities of these signals cannot be
> derived only from these settings as this driver did. The format has to
> be taken into account too.
> 
> This fixes inverted left/right channels in I²S mode.
> 
> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>

Looks like the same configurations of fsl_ssi, so it should be fine:

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

> ---
>  sound/soc/fsl/imx-ssi.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
> index 48b2d24..b95132e 100644
> --- a/sound/soc/fsl/imx-ssi.c
> +++ b/sound/soc/fsl/imx-ssi.c
> @@ -95,7 +95,8 @@ static int imx_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
>  	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>  	case SND_SOC_DAIFMT_I2S:
>  		/* data on rising edge of bclk, frame low 1clk before data */
> -		strcr |= SSI_STCR_TFSI | SSI_STCR_TEFS | SSI_STCR_TXBIT0;
> +		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSI |
> +			SSI_STCR_TEFS;
>  		scr |= SSI_SCR_NET;
>  		if (ssi->flags & IMX_SSI_USE_I2S_SLAVE) {
>  			scr &= ~SSI_I2S_MODE_MASK;
> @@ -104,33 +105,31 @@ static int imx_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
>  		break;
>  	case SND_SOC_DAIFMT_LEFT_J:
>  		/* data on rising edge of bclk, frame high with data */
> -		strcr |= SSI_STCR_TXBIT0;
> +		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP;
>  		break;
>  	case SND_SOC_DAIFMT_DSP_B:
>  		/* data on rising edge of bclk, frame high with data */
> -		strcr |= SSI_STCR_TFSL | SSI_STCR_TXBIT0;
> +		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSL;
>  		break;
>  	case SND_SOC_DAIFMT_DSP_A:
>  		/* data on rising edge of bclk, frame high 1clk before data */
> -		strcr |= SSI_STCR_TFSL | SSI_STCR_TXBIT0 | SSI_STCR_TEFS;
> +		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSL |
> +			SSI_STCR_TEFS;
>  		break;
>  	}
>  
>  	/* DAI clock inversion */
>  	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>  	case SND_SOC_DAIFMT_IB_IF:
> -		strcr |= SSI_STCR_TFSI;
> -		strcr &= ~SSI_STCR_TSCKP;
> +		strcr ^= SSI_STCR_TSCKP | SSI_STCR_TFSI;
>  		break;
>  	case SND_SOC_DAIFMT_IB_NF:
> -		strcr &= ~(SSI_STCR_TSCKP | SSI_STCR_TFSI);
> +		strcr ^= SSI_STCR_TSCKP;
>  		break;
>  	case SND_SOC_DAIFMT_NB_IF:
> -		strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP;
> +		strcr ^= SSI_STCR_TFSI;
>  		break;
>  	case SND_SOC_DAIFMT_NB_NF:
> -		strcr &= ~SSI_STCR_TFSI;
> -		strcr |= SSI_STCR_TSCKP;
>  		break;
>  	}
>  
> -- 
> 2.1.4
>
undefined@alsa-project.org Sept. 19, 2015, 12:51 a.m. UTC | #2
Nicolin Chen wrote:
> Looks like the same configurations of fsl_ssi, so it should be fine:

I'm so glad I ignored the IMX driver when I wrote fsl_ssi.c. :-)
Benoît Thébaudeau Sept. 28, 2015, 7:36 p.m. UTC | #3
Hi all,

On Thu, Sep 17, 2015 at 7:59 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> On Thu, Sep 17, 2015 at 05:46:54PM +0200, Benoît Thébaudeau wrote:
>> SND_SOC_DAIFMT_{IB|NB}_{IF|NF} are defined as inverting or not BCLK or
>> FRM relatively to what is standard for the specified DAI hardware audio
>> format. Consequently, the absolute polarities of these signals cannot be
>> derived only from these settings as this driver did. The format has to
>> be taken into account too.
>>
>> This fixes inverted left/right channels in I²S mode.
>>
>> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
>
> Looks like the same configurations of fsl_ssi, so it should be fine:
>
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
>> ---
>>  sound/soc/fsl/imx-ssi.c | 19 +++++++++----------
>>  1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
>> index 48b2d24..b95132e 100644
>> --- a/sound/soc/fsl/imx-ssi.c
>> +++ b/sound/soc/fsl/imx-ssi.c
>> @@ -95,7 +95,8 @@ static int imx_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
>>       switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>       case SND_SOC_DAIFMT_I2S:
>>               /* data on rising edge of bclk, frame low 1clk before data */
>> -             strcr |= SSI_STCR_TFSI | SSI_STCR_TEFS | SSI_STCR_TXBIT0;
>> +             strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSI |
>> +                     SSI_STCR_TEFS;
>>               scr |= SSI_SCR_NET;
>>               if (ssi->flags & IMX_SSI_USE_I2S_SLAVE) {
>>                       scr &= ~SSI_I2S_MODE_MASK;
>> @@ -104,33 +105,31 @@ static int imx_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
>>               break;
>>       case SND_SOC_DAIFMT_LEFT_J:
>>               /* data on rising edge of bclk, frame high with data */
>> -             strcr |= SSI_STCR_TXBIT0;
>> +             strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP;
>>               break;
>>       case SND_SOC_DAIFMT_DSP_B:
>>               /* data on rising edge of bclk, frame high with data */
>> -             strcr |= SSI_STCR_TFSL | SSI_STCR_TXBIT0;
>> +             strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSL;
>>               break;
>>       case SND_SOC_DAIFMT_DSP_A:
>>               /* data on rising edge of bclk, frame high 1clk before data */
>> -             strcr |= SSI_STCR_TFSL | SSI_STCR_TXBIT0 | SSI_STCR_TEFS;
>> +             strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSL |
>> +                     SSI_STCR_TEFS;
>>               break;
>>       }
>>
>>       /* DAI clock inversion */
>>       switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>>       case SND_SOC_DAIFMT_IB_IF:
>> -             strcr |= SSI_STCR_TFSI;
>> -             strcr &= ~SSI_STCR_TSCKP;
>> +             strcr ^= SSI_STCR_TSCKP | SSI_STCR_TFSI;
>>               break;
>>       case SND_SOC_DAIFMT_IB_NF:
>> -             strcr &= ~(SSI_STCR_TSCKP | SSI_STCR_TFSI);
>> +             strcr ^= SSI_STCR_TSCKP;
>>               break;
>>       case SND_SOC_DAIFMT_NB_IF:
>> -             strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP;
>> +             strcr ^= SSI_STCR_TFSI;
>>               break;
>>       case SND_SOC_DAIFMT_NB_NF:
>> -             strcr &= ~SSI_STCR_TFSI;
>> -             strcr |= SSI_STCR_TSCKP;
>>               break;
>>       }
>>
>> --
>> 2.1.4
>>

Adding the ASoC maintainers to the Cc list.

Best regards,
Benoît
Mark Brown Sept. 29, 2015, 3:35 p.m. UTC | #4
On Mon, Sep 28, 2015 at 09:36:49PM +0200, Benoît Thébaudeau wrote:

> >>       case SND_SOC_DAIFMT_NB_NF:
> >> -             strcr &= ~SSI_STCR_TFSI;
> >> -             strcr |= SSI_STCR_TSCKP;
> >>               break;
> >>       }

> Adding the ASoC maintainers to the Cc list.

Please send patches directly to maintainers as covered in SubmittingPatches,
I can't do anything with a quoted reply to an e-mail.
diff mbox

Patch

diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
index 48b2d24..b95132e 100644
--- a/sound/soc/fsl/imx-ssi.c
+++ b/sound/soc/fsl/imx-ssi.c
@@ -95,7 +95,8 @@  static int imx_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
 		/* data on rising edge of bclk, frame low 1clk before data */
-		strcr |= SSI_STCR_TFSI | SSI_STCR_TEFS | SSI_STCR_TXBIT0;
+		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSI |
+			SSI_STCR_TEFS;
 		scr |= SSI_SCR_NET;
 		if (ssi->flags & IMX_SSI_USE_I2S_SLAVE) {
 			scr &= ~SSI_I2S_MODE_MASK;
@@ -104,33 +105,31 @@  static int imx_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
 		break;
 	case SND_SOC_DAIFMT_LEFT_J:
 		/* data on rising edge of bclk, frame high with data */
-		strcr |= SSI_STCR_TXBIT0;
+		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP;
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
 		/* data on rising edge of bclk, frame high with data */
-		strcr |= SSI_STCR_TFSL | SSI_STCR_TXBIT0;
+		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSL;
 		break;
 	case SND_SOC_DAIFMT_DSP_A:
 		/* data on rising edge of bclk, frame high 1clk before data */
-		strcr |= SSI_STCR_TFSL | SSI_STCR_TXBIT0 | SSI_STCR_TEFS;
+		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP | SSI_STCR_TFSL |
+			SSI_STCR_TEFS;
 		break;
 	}
 
 	/* DAI clock inversion */
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_IB_IF:
-		strcr |= SSI_STCR_TFSI;
-		strcr &= ~SSI_STCR_TSCKP;
+		strcr ^= SSI_STCR_TSCKP | SSI_STCR_TFSI;
 		break;
 	case SND_SOC_DAIFMT_IB_NF:
-		strcr &= ~(SSI_STCR_TSCKP | SSI_STCR_TFSI);
+		strcr ^= SSI_STCR_TSCKP;
 		break;
 	case SND_SOC_DAIFMT_NB_IF:
-		strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP;
+		strcr ^= SSI_STCR_TFSI;
 		break;
 	case SND_SOC_DAIFMT_NB_NF:
-		strcr &= ~SSI_STCR_TFSI;
-		strcr |= SSI_STCR_TSCKP;
 		break;
 	}