[alsa-devel] [PATCH 0/4] ASoC: Make set_tdm_slot() semantics consistent
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(-)
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;
The 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 --- Changes since v1: * Don't change the esai driver as it does not use the inverted semantics. --- sound/soc/fsl/eukrea-tlv320.c | 2 +- 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 ++-- 6 files changed, 11 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_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;
On Mon, Jan 12, 2015 at 10:27:18AM +0100, Lars-Peter Clausen wrote:
The 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
Changes since v1:
- Don't change the esai driver as it does not use the inverted semantics.
sound/soc/fsl/eukrea-tlv320.c | 2 +- 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 ++-- 6 files changed, 11 insertions(+), 11 deletions(-)
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);
Hmm..I just notice the original configuration was 0x0 with 0xfffffffc. It doesn't look making sense to me by using 0x0 here but not sure if it was intentional.
However, I suppose it should be fine since the time slot number was 2.
Acked-by: Nicolin Chen nicoleotsuka@gmail.com
Thank you
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, };
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)
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.
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Nicolin Chen