[PATCH 0/4] ASoC: Make set_tdm_slot() semantics consistent

[PATCH 0/4] ASoC: Make set_tdm_slot() semantics consistent

[Lars-Peter Clausen]

This is something we shortly discussed during the ALSA mini conference in
Duesseldorf last year. We want to do more useful stuff with the
snd_soc_set_tdm_slot() API and build generic infrastructure on top of it.
But there are currently two different incompatible semantics of how the
tx_mask and rx_mask parameters of snd_soc_dai_set_tdm_slot() are used. If we
want to be able to use the API in a generic way we need to fix this
inconsistency. Both interpretations agree that the slot masks should be
represented as bitmasks which bit 0 representing the first slot, bit 1 the
second slot and so on. But where they differ is in the meaning of value of
the bit. Some implementations assume that if a bit is set the channel is
enabled and if the bit is cleared the channel is disabled, others have
inverse semantics, they assume that if a bit is cleared the channel is
enabled and if the bit is set the channel is disabled.

The former is used by most drivers and later is only used by some the
Freescale DAI drivers and the MC13783 CODEC driver. This appears to be a
result of Freescale DAI drivers directly passing the masks through to the
registers which expect the inverted semantics. And the MC13783 appears to
have picked up the same meaning since it is used on a board which has a
Freescale DAI on the CPU side of the DAI link.

This series updates the MC13783 and Freescale DAI drivers to use the same
semantics as other drivers and also updates the documentation of the
snd_soc_dai_set_tdm_slot() function to be more specific on the intended
semantics.

- Lars

Lars-Peter Clausen (4):
  ASoC: mc13783: Update set_tdm_slot() semantics
  ASoC: fsl: Update set_tdm_slot() semantics
  ASoC: fsl: Remove fsl_asoc_xlate_tdm_slot_mask()
  ASoC: Update snd_soc_dai_set_tdm_slot() documentation

 sound/soc/codecs/mc13783.c    | 10 +++++-----
 sound/soc/fsl/eukrea-tlv320.c |  2 +-
 sound/soc/fsl/fsl_esai.c      |  3 +++
 sound/soc/fsl/fsl_ssi.c       |  4 ++--
 sound/soc/fsl/fsl_utils.c     | 27 ---------------------------
 sound/soc/fsl/fsl_utils.h     |  3 ---
 sound/soc/fsl/imx-mc13783.c   |  5 ++---
 sound/soc/fsl/imx-ssi.c       |  5 ++---
 sound/soc/fsl/wm1133-ev1.c    |  4 ++--
 sound/soc/soc-core.c          | 20 ++++++++++++++++----
 10 files changed, 33 insertions(+), 50 deletions(-)

-- 
1.8.0

[PATCH 1/4] ASoC: mc13783: Update set_tdm_slot() semantics

[Lars-Peter Clausen]

The mc13783 driver uses inverted semantics for the tx_mask and rx_mask
parameter of the set_tdm_slot() callback compared to rest of ASoC. This
patch updates the driver's semantics to be consistent with the rest of ASoC,
i.e. a set bit means a active slot and a cleared bit means a inactive slot.
This will allow us to use the set_tdm_slot() API in a more generic way.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/codecs/mc13783.c  | 10 +++++-----
 sound/soc/fsl/imx-mc13783.c |  3 +--
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
index c1e441c..2ffb9a0 100644
--- a/sound/soc/codecs/mc13783.c
+++ b/sound/soc/codecs/mc13783.c
@@ -328,16 +328,16 @@ static int mc13783_set_tdm_slot_dac(struct snd_soc_dai *dai,
 	}
 
 	switch (rx_mask) {
-	case 0xfffffffc:
+	case 0x03:
 		val |= SSI_NETWORK_DAC_RXSLOT_0_1;
 		break;
-	case 0xfffffff3:
+	case 0x0c:
 		val |= SSI_NETWORK_DAC_RXSLOT_2_3;
 		break;
-	case 0xffffffcf:
+	case 0x30:
 		val |= SSI_NETWORK_DAC_RXSLOT_4_5;
 		break;
-	case 0xffffff3f:
+	case 0xc0:
 		val |= SSI_NETWORK_DAC_RXSLOT_6_7;
 		break;
 	default:
@@ -360,7 +360,7 @@ static int mc13783_set_tdm_slot_codec(struct snd_soc_dai *dai,
 	if (slots != 4)
 		return -EINVAL;
 
-	if (tx_mask != 0xfffffffc)
+	if (tx_mask != 0x3)
 		return -EINVAL;
 
 	val |= (0x00 << 2);	/* primary timeslot RX/TX(?) is 0 */
diff --git a/sound/soc/fsl/imx-mc13783.c b/sound/soc/fsl/imx-mc13783.c
index 6bf5bce..9589452 100644
--- a/sound/soc/fsl/imx-mc13783.c
+++ b/sound/soc/fsl/imx-mc13783.c
@@ -37,8 +37,7 @@ static int imx_mc13783_hifi_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int ret;
 
-	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xfffffffc, 0xfffffffc,
-					4, 16);
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x3, 0x3, 4, 16);
 	if (ret)
 		return ret;
 
-- 
1.8.0

[PATCH 2/4] ASoC: fsl: Update set_tdm_slot() semantics

[Lars-Peter Clausen]

The fsl-esai, fsl-ssi and imx-ssi drivers use inverted semantics for the
tx_mask and rx_mask parameter of the set_tdm_slot() callback compared to
rest of ASoC. This patch updates the driver's semantics to be consistent
with the rest of ASoC, i.e. a set bit means a active slot and a cleared bit
means a inactive slot.  This will allow us to use the set_tdm_slot() API in
a more generic way.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/fsl/eukrea-tlv320.c | 2 +-
 sound/soc/fsl/fsl_esai.c      | 3 +++
 sound/soc/fsl/fsl_ssi.c       | 4 ++--
 sound/soc/fsl/fsl_utils.c     | 6 +++---
 sound/soc/fsl/imx-mc13783.c   | 2 +-
 sound/soc/fsl/imx-ssi.c       | 4 ++--
 sound/soc/fsl/wm1133-ev1.c    | 4 ++--
 7 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/sound/soc/fsl/eukrea-tlv320.c b/sound/soc/fsl/eukrea-tlv320.c
index 8c9e900..e1aa3834 100644
--- a/sound/soc/fsl/eukrea-tlv320.c
+++ b/sound/soc/fsl/eukrea-tlv320.c
@@ -50,7 +50,7 @@ static int eukrea_tlv320_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0);
+	snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 0);
 
 	ret = snd_soc_dai_set_sysclk(cpu_dai, IMX_SSP_SYS_CLK, 0,
 				SND_SOC_CLOCK_IN);
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 5c75971..12ed857 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -347,6 +347,9 @@ static int fsl_esai_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask,
 {
 	struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai);
 
+	tx_mask = ~tx_mask;
+	rx_mask = ~rx_mask;
+
 	regmap_update_bits(esai_priv->regmap, REG_ESAI_TCCR,
 			   ESAI_xCCR_xDC_MASK, ESAI_xCCR_xDC(slots));
 
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 65400be..791cdb3 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -992,8 +992,8 @@ static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, u32 tx_mask,
 	regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_SSIEN,
 			CCSR_SSI_SCR_SSIEN);
 
-	regmap_write(regs, CCSR_SSI_STMSK, tx_mask);
-	regmap_write(regs, CCSR_SSI_SRMSK, rx_mask);
+	regmap_write(regs, CCSR_SSI_STMSK, ~tx_mask);
+	regmap_write(regs, CCSR_SSI_SRMSK, ~rx_mask);
 
 	regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_SSIEN, val);
 
diff --git a/sound/soc/fsl/fsl_utils.c b/sound/soc/fsl/fsl_utils.c
index 2ac7755..5fd4463 100644
--- a/sound/soc/fsl/fsl_utils.c
+++ b/sound/soc/fsl/fsl_utils.c
@@ -94,7 +94,7 @@ EXPORT_SYMBOL(fsl_asoc_get_dma_channel);
  * @rx_mask: bitmask representing active RX slots.
  *
  * This function used to generate the TDM slot TX/RX mask. And the TX/RX
- * mask will use a 0 bit for an active slot as default, and the default
+ * mask will use a 1 bit for an active slot as default, and the default
  * active bits are at the LSB of the mask value.
  */
 int fsl_asoc_xlate_tdm_slot_mask(unsigned int slots,
@@ -105,9 +105,9 @@ int fsl_asoc_xlate_tdm_slot_mask(unsigned int slots,
 		return -EINVAL;
 
 	if (tx_mask)
-		*tx_mask = ~((1 << slots) - 1);
+		*tx_mask = ((1 << slots) - 1);
 	if (rx_mask)
-		*rx_mask = ~((1 << slots) - 1);
+		*rx_mask = ((1 << slots) - 1);
 
 	return 0;
 }
diff --git a/sound/soc/fsl/imx-mc13783.c b/sound/soc/fsl/imx-mc13783.c
index 9589452..9e6493d 100644
--- a/sound/soc/fsl/imx-mc13783.c
+++ b/sound/soc/fsl/imx-mc13783.c
@@ -45,7 +45,7 @@ static int imx_mc13783_hifi_hw_params(struct snd_pcm_substream *substream,
 	if (ret)
 		return ret;
 
-	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x0, 0xfffffffc, 2, 16);
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 16);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
index fa801e1..6aeaac3 100644
--- a/sound/soc/fsl/imx-ssi.c
+++ b/sound/soc/fsl/imx-ssi.c
@@ -74,8 +74,8 @@ static int imx_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai,
 	sccr |= SSI_STCCR_DC(slots - 1);
 	writel(sccr, ssi->base + SSI_SRCCR);
 
-	writel(tx_mask, ssi->base + SSI_STMSK);
-	writel(rx_mask, ssi->base + SSI_SRMSK);
+	writel(~tx_mask, ssi->base + SSI_STMSK);
+	writel(~rx_mask, ssi->base + SSI_SRMSK);
 
 	return 0;
 }
diff --git a/sound/soc/fsl/wm1133-ev1.c b/sound/soc/fsl/wm1133-ev1.c
index d072bd1..a958937 100644
--- a/sound/soc/fsl/wm1133-ev1.c
+++ b/sound/soc/fsl/wm1133-ev1.c
@@ -106,10 +106,10 @@ static int wm1133_ev1_hw_params(struct snd_pcm_substream *substream,
 	/* TODO: The SSI driver should figure this out for us */
 	switch (channels) {
 	case 2:
-		snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0);
+		snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 0);
 		break;
 	case 1:
-		snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffe, 0xffffffe, 1, 0);
+		snd_soc_dai_set_tdm_slot(cpu_dai, 0x1, 0x1, 1, 0);
 		break;
 	default:
 		return -EINVAL;
-- 
1.8.0

[PATCH 3/4] ASoC: fsl: Remove fsl_asoc_xlate_tdm_slot_mask()

[Lars-Peter Clausen]

Now that the fsl DAI drivers uses the same semantics as the rest of a ASoC
the custom fsl_asoc_xlate_tdm_slot_mask() callback can be removed as it is
identical to the generic one.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/fsl/fsl_utils.c | 27 ---------------------------
 sound/soc/fsl/fsl_utils.h |  3 ---
 sound/soc/fsl/imx-ssi.c   |  1 -
 3 files changed, 31 deletions(-)

diff --git a/sound/soc/fsl/fsl_utils.c b/sound/soc/fsl/fsl_utils.c
index 5fd4463..b9e42b5 100644
--- a/sound/soc/fsl/fsl_utils.c
+++ b/sound/soc/fsl/fsl_utils.c
@@ -86,33 +86,6 @@ int fsl_asoc_get_dma_channel(struct device_node *ssi_np,
 }
 EXPORT_SYMBOL(fsl_asoc_get_dma_channel);
 
-/**
- * fsl_asoc_xlate_tdm_slot_mask - generate TDM slot TX/RX mask.
- *
- * @slots: Number of slots in use.
- * @tx_mask: bitmask representing active TX slots.
- * @rx_mask: bitmask representing active RX slots.
- *
- * This function used to generate the TDM slot TX/RX mask. And the TX/RX
- * mask will use a 1 bit for an active slot as default, and the default
- * active bits are at the LSB of the mask value.
- */
-int fsl_asoc_xlate_tdm_slot_mask(unsigned int slots,
-				    unsigned int *tx_mask,
-				    unsigned int *rx_mask)
-{
-	if (!slots)
-		return -EINVAL;
-
-	if (tx_mask)
-		*tx_mask = ((1 << slots) - 1);
-	if (rx_mask)
-		*rx_mask = ((1 << slots) - 1);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(fsl_asoc_xlate_tdm_slot_mask);
-
 MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
 MODULE_DESCRIPTION("Freescale ASoC utility code");
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/fsl/fsl_utils.h b/sound/soc/fsl/fsl_utils.h
index df535db..1687b66 100644
--- a/sound/soc/fsl/fsl_utils.h
+++ b/sound/soc/fsl/fsl_utils.h
@@ -22,7 +22,4 @@ int fsl_asoc_get_dma_channel(struct device_node *ssi_np, const char *name,
 			     struct snd_soc_dai_link *dai,
 			     unsigned int *dma_channel_id,
 			     unsigned int *dma_id);
-int fsl_asoc_xlate_tdm_slot_mask(unsigned int slots,
-				    unsigned int *tx_mask,
-				    unsigned int *rx_mask);
 #endif /* _FSL_UTILS_H */
diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
index 6aeaac3..461ce27 100644
--- a/sound/soc/fsl/imx-ssi.c
+++ b/sound/soc/fsl/imx-ssi.c
@@ -340,7 +340,6 @@ static const struct snd_soc_dai_ops imx_ssi_pcm_dai_ops = {
 	.set_fmt	= imx_ssi_set_dai_fmt,
 	.set_clkdiv	= imx_ssi_set_dai_clkdiv,
 	.set_sysclk	= imx_ssi_set_dai_sysclk,
-	.xlate_tdm_slot_mask = fsl_asoc_xlate_tdm_slot_mask,
 	.set_tdm_slot	= imx_ssi_set_dai_tdm_slot,
 	.trigger	= imx_ssi_trigger,
 };
-- 
1.8.0

[PATCH 4/4] ASoC: Update snd_soc_dai_set_tdm_slot() documentation

[Lars-Peter Clausen]

There have been some conflicting interpretations of how
snd_soc_dai_set_tdm_slot() is supposed to work. This patch updates the
documentation to be more specific on the exact semantics to avoid such
problems in the future.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-core.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 2c02e9c..ededb97 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2130,15 +2130,27 @@ static int snd_soc_xlate_tdm_slot_mask(unsigned int slots,
 }
 
 /**
- * snd_soc_dai_set_tdm_slot - configure DAI TDM.
- * @dai: DAI
+ * snd_soc_dai_set_tdm_slot() - Configures a DAI for TDM operation
+ * @dai: The DAI to configure
  * @tx_mask: bitmask representing active TX slots.
  * @rx_mask: bitmask representing active RX slots.
  * @slots: Number of slots in use.
  * @slot_width: Width in bits for each slot.
  *
- * Configures a DAI for TDM operation. Both mask and slots are codec and DAI
- * specific.
+ * This function configures the specified DAI for TDM operation. @slot contains
+ * the total number of slots of the TDM stream and @slot_with the width of each
+ * slot in bit clock cycles. @tx_mask and @rx_mask are bitmasks specifying the
+ * active slots of the TDM stream for the specified DAI, i.e. which slots the
+ * DAI should write to or read from. If a bit is set the corresponding slot is
+ * active, if a bit is cleared the corresponding slot is inactive. Bit 0 maps to
+ * the first slot, bit 1 to the second slot and so on. The first active slot
+ * maps to the first channel of the DAI, the second active slot to the second
+ * channel and so on.
+ *
+ * TDM mode can be disabled by passing 0 for @slots. In this case @tx_mask,
+ * @rx_mask and @slot_width will be ignored.
+ *
+ * Returns 0 on success, a negative error code otherwise.
  */
 int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
 	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
-- 
1.8.0

Re: [PATCH 2/4] ASoC: fsl: Update set_tdm_slot() semantics

[Nicolin Chen]

Hi Lars,

On Sat, Jan 10, 2015 at 11:52:35AM +0100, Lars-Peter Clausen wrote:
> The fsl-esai, fsl-ssi and imx-ssi drivers use inverted semantics for the
> tx_mask and rx_mask parameter of the set_tdm_slot() callback compared to
> rest of ASoC. This patch updates the driver's semantics to be consistent
> with the rest of ASoC, i.e. a set bit means a active slot and a cleared bit
> means a inactive slot.  This will allow us to use the set_tdm_slot() API in
> a more generic way.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  sound/soc/fsl/eukrea-tlv320.c | 2 +-
>  sound/soc/fsl/fsl_esai.c      | 3 +++
>  sound/soc/fsl/fsl_ssi.c       | 4 ++--
>  sound/soc/fsl/fsl_utils.c     | 6 +++---
>  sound/soc/fsl/imx-mc13783.c   | 2 +-
>  sound/soc/fsl/imx-ssi.c       | 4 ++--
>  sound/soc/fsl/wm1133-ev1.c    | 4 ++--

The SSI part and these machine drivers (all of them using SSI) being
patched should be okay because the SSI uses the inverted semantics.

However, the mask for ESAI does follow the common semantics, that is,
a set bit means a active slot and a cleared bit.

After hardware or software reset, the ESAI_TSMA register is preset to
0x0000FFFF, which means that all 16 possible slots are enabled for data
transmission.				-- From i.MX6 Reference Manual

So I think ESAI doesn't need this change at all.

Thank you
Nicolin

>  7 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/soc/fsl/eukrea-tlv320.c b/sound/soc/fsl/eukrea-tlv320.c
> index 8c9e900..e1aa3834 100644
> --- a/sound/soc/fsl/eukrea-tlv320.c
> +++ b/sound/soc/fsl/eukrea-tlv320.c
> @@ -50,7 +50,7 @@ static int eukrea_tlv320_hw_params(struct snd_pcm_substream *substream,
>  		return ret;
>  	}
>  
> -	snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0);
> +	snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 0);
>  
>  	ret = snd_soc_dai_set_sysclk(cpu_dai, IMX_SSP_SYS_CLK, 0,
>  				SND_SOC_CLOCK_IN);
> diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
> index 5c75971..12ed857 100644
> --- a/sound/soc/fsl/fsl_esai.c
> +++ b/sound/soc/fsl/fsl_esai.c
> @@ -347,6 +347,9 @@ static int fsl_esai_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask,
>  {
>  	struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai);
>  
> +	tx_mask = ~tx_mask;
> +	rx_mask = ~rx_mask;
> +
>  	regmap_update_bits(esai_priv->regmap, REG_ESAI_TCCR,
>  			   ESAI_xCCR_xDC_MASK, ESAI_xCCR_xDC(slots));
>  
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 65400be..791cdb3 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -992,8 +992,8 @@ static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, u32 tx_mask,
>  	regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_SSIEN,
>  			CCSR_SSI_SCR_SSIEN);
>  
> -	regmap_write(regs, CCSR_SSI_STMSK, tx_mask);
> -	regmap_write(regs, CCSR_SSI_SRMSK, rx_mask);
> +	regmap_write(regs, CCSR_SSI_STMSK, ~tx_mask);
> +	regmap_write(regs, CCSR_SSI_SRMSK, ~rx_mask);
>  
>  	regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_SSIEN, val);
>  
> diff --git a/sound/soc/fsl/fsl_utils.c b/sound/soc/fsl/fsl_utils.c
> index 2ac7755..5fd4463 100644
> --- a/sound/soc/fsl/fsl_utils.c
> +++ b/sound/soc/fsl/fsl_utils.c
> @@ -94,7 +94,7 @@ EXPORT_SYMBOL(fsl_asoc_get_dma_channel);
>   * @rx_mask: bitmask representing active RX slots.
>   *
>   * This function used to generate the TDM slot TX/RX mask. And the TX/RX
> - * mask will use a 0 bit for an active slot as default, and the default
> + * mask will use a 1 bit for an active slot as default, and the default
>   * active bits are at the LSB of the mask value.
>   */
>  int fsl_asoc_xlate_tdm_slot_mask(unsigned int slots,
> @@ -105,9 +105,9 @@ int fsl_asoc_xlate_tdm_slot_mask(unsigned int slots,
>  		return -EINVAL;
>  
>  	if (tx_mask)
> -		*tx_mask = ~((1 << slots) - 1);
> +		*tx_mask = ((1 << slots) - 1);
>  	if (rx_mask)
> -		*rx_mask = ~((1 << slots) - 1);
> +		*rx_mask = ((1 << slots) - 1);
>  
>  	return 0;
>  }
> diff --git a/sound/soc/fsl/imx-mc13783.c b/sound/soc/fsl/imx-mc13783.c
> index 9589452..9e6493d 100644
> --- a/sound/soc/fsl/imx-mc13783.c
> +++ b/sound/soc/fsl/imx-mc13783.c
> @@ -45,7 +45,7 @@ static int imx_mc13783_hifi_hw_params(struct snd_pcm_substream *substream,
>  	if (ret)
>  		return ret;
>  
> -	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x0, 0xfffffffc, 2, 16);
> +	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 16);
>  	if (ret)
>  		return ret;
>  
> diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
> index fa801e1..6aeaac3 100644
> --- a/sound/soc/fsl/imx-ssi.c
> +++ b/sound/soc/fsl/imx-ssi.c
> @@ -74,8 +74,8 @@ static int imx_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai,
>  	sccr |= SSI_STCCR_DC(slots - 1);
>  	writel(sccr, ssi->base + SSI_SRCCR);
>  
> -	writel(tx_mask, ssi->base + SSI_STMSK);
> -	writel(rx_mask, ssi->base + SSI_SRMSK);
> +	writel(~tx_mask, ssi->base + SSI_STMSK);
> +	writel(~rx_mask, ssi->base + SSI_SRMSK);
>  
>  	return 0;
>  }
> diff --git a/sound/soc/fsl/wm1133-ev1.c b/sound/soc/fsl/wm1133-ev1.c
> index d072bd1..a958937 100644
> --- a/sound/soc/fsl/wm1133-ev1.c
> +++ b/sound/soc/fsl/wm1133-ev1.c
> @@ -106,10 +106,10 @@ static int wm1133_ev1_hw_params(struct snd_pcm_substream *substream,
>  	/* TODO: The SSI driver should figure this out for us */
>  	switch (channels) {
>  	case 2:
> -		snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0);
> +		snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 0);
>  		break;
>  	case 1:
> -		snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffe, 0xffffffe, 1, 0);
> +		snd_soc_dai_set_tdm_slot(cpu_dai, 0x1, 0x1, 1, 0);
>  		break;
>  	default:
>  		return -EINVAL;
> -- 
> 1.8.0
> 

Re: [PATCH 2/4] ASoC: fsl: Update set_tdm_slot() semantics

[Lars-Peter Clausen]

On 01/10/2015 09:00 PM, Nicolin Chen wrote:
> Hi Lars,
>
> On Sat, Jan 10, 2015 at 11:52:35AM +0100, Lars-Peter Clausen wrote:
>> The fsl-esai, fsl-ssi and imx-ssi drivers use inverted semantics for the
>> tx_mask and rx_mask parameter of the set_tdm_slot() callback compared to
>> rest of ASoC. This patch updates the driver's semantics to be consistent
>> with the rest of ASoC, i.e. a set bit means a active slot and a cleared bit
>> means a inactive slot.  This will allow us to use the set_tdm_slot() API in
>> a more generic way.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>   sound/soc/fsl/eukrea-tlv320.c | 2 +-
>>   sound/soc/fsl/fsl_esai.c      | 3 +++
>>   sound/soc/fsl/fsl_ssi.c       | 4 ++--
>>   sound/soc/fsl/fsl_utils.c     | 6 +++---
>>   sound/soc/fsl/imx-mc13783.c   | 2 +-
>>   sound/soc/fsl/imx-ssi.c       | 4 ++--
>>   sound/soc/fsl/wm1133-ev1.c    | 4 ++--
>
> The SSI part and these machine drivers (all of them using SSI) being
> patched should be okay because the SSI uses the inverted semantics.
>
> However, the mask for ESAI does follow the common semantics, that is,
> a set bit means a active slot and a cleared bit.
>
> After hardware or software reset, the ESAI_TSMA register is preset to
> 0x0000FFFF, which means that all 16 possible slots are enabled for data
> transmission.				-- From i.MX6 Reference Manual
>
> So I think ESAI doesn't need this change at all.

Thanks, will fix it and send a v2.

- Lars

Re: [PATCH 0/4] ASoC: Make set_tdm_slot() semantics consistent

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 466 bytes --]

On Mon, Jan 12, 2015 at 10:27:16AM +0100, Lars-Peter Clausen wrote:
> Changes since v1:
> 	* Don't modify the fsl-esai driver as it already uses the non-inverted
> 	  semantics
> 
> Original cover letter below:
> 
> This is something we shortly discussed during the ALSA mini conference in
> Duesseldorf last year. We want to do more useful stuff with the
> snd_soc_set_tdm_slot() API and build generic infrastructure on top of it.

Applied all, thanks.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 0/4] ASoC: Make set_tdm_slot() semantics consistent

[Lars-Peter Clausen]

Changes since v1:
	* Don't modify the fsl-esai driver as it already uses the non-inverted
	  semantics

Original cover letter below:

This is something we shortly discussed during the ALSA mini conference in
Duesseldorf last year. We want to do more useful stuff with the
snd_soc_set_tdm_slot() API and build generic infrastructure on top of it.
But there are currently two different incompatible semantics of how the
tx_mask and rx_mask parameters of snd_soc_dai_set_tdm_slot() are used. If we
want to be able to use the API in a generic way we need to fix this
inconsistency. Both interpretations agree that the slot masks should be
represented as bitmasks which bit 0 representing the first slot, bit 1 the
second slot and so on. But where they differ is in the meaning of value of
the bit. Some implementations assume that if a bit is set the channel is
enabled and if the bit is cleared the channel is disabled, others have
inverse semantics, they assume that if a bit is cleared the channel is
enabled and if the bit is set the channel is disabled.

The former is used by most drivers and later is only used by some the
Freescale DAI drivers and the MC13783 CODEC driver. This appears to be a
result of Freescale DAI drivers directly passing the masks through to the
registers which expect the inverted semantics. And the MC13783 appears to
have picked up the same meaning since it is used on a board which has a
Freescale DAI on the CPU side of the DAI link.

This series updates the MC13783 and Freescale DAI drivers to use the same
semantics as other drivers and also updates the documentation of the
snd_soc_dai_set_tdm_slot() function to be more specific on the intended
semantics.

- Lars

Lars-Peter Clausen (4):
  ASoC: mc13783: Update set_tdm_slot() semantics
  ASoC: fsl: Update set_tdm_slot() semantics
  ASoC: fsl: Remove fsl_asoc_xlate_tdm_slot_mask()
  ASoC: Update snd_soc_dai_set_tdm_slot() documentation

 sound/soc/codecs/mc13783.c    | 10 +++++-----
 sound/soc/fsl/eukrea-tlv320.c |  2 +-
 sound/soc/fsl/fsl_ssi.c       |  4 ++--
 sound/soc/fsl/fsl_utils.c     | 27 ---------------------------
 sound/soc/fsl/fsl_utils.h     |  3 ---
 sound/soc/fsl/imx-mc13783.c   |  5 ++---
 sound/soc/fsl/imx-ssi.c       |  5 ++---
 sound/soc/fsl/wm1133-ev1.c    |  4 ++--
 sound/soc/soc-core.c          | 20 ++++++++++++++++----
 9 files changed, 30 insertions(+), 50 deletions(-)

-- 
1.8.0