[PATCH RFC v2] ASoC: cs43130: Take the frame layout from set_tdm_slot()

[PATCH RFC v2] ASoC: cs43130: Take the frame layout from set_tdm_slot()

[Olivier Louvignes]

hw_params() derives the slot width from the frequency given to the DAI's
set_sysclk(), which it takes for SCLK. Generic cards pass MCLK there: the
simple-audio-card of a HiBy R1 (Ingenic X1600 as I2S master, 64 bit
clocks per frame, mclk-fs = 512) hands over 22.5792 MHz, so the driver
sets up 256-bit slots for a 44.1 kHz stream and the CS43131 plays white
noise. The card cannot simply leave mclk-fs out: simple-audio-card then
sets no clock on the I2S controller either, and the codec, as clock
slave, fails hw_params() with "SCLK freq is not set".

Add a set_tdm_slot() callback so the card can give the layout instead.
The two channels go to the first and second slots set in tx_mask, within
frames of 16 to 64 bits, the sizes the serial port clock tables cover.
When set, the layout takes precedence over the one derived from
set_sysclk(); zero slots go back to deriving it.

Tested on a HiBy R1, on v7.3, with dai-tdm-slot-num = <2> and
dai-tdm-slot-width = <32> on the codec node.

Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
---
Changes in v2, after Charles Keepax and the review bot:
 - Full TDM: the two channels go to the first and second slots set in
   tx_mask, with the frame length from the slot count, instead of only
   accepting two slots with mask 0x3. Limited to frames of 16 to 64 bits,
   the sizes cs43130_set_sp_fmt() has clock tables for (beyond that it
   returns -EINVAL after the frame registers are written, and hw_params()
   ignores the error).
 - The callback stays: on the R1, without mclk-fs simple-audio-card sets
   no clock on the I2S controller, and the codec as clock slave fails with
   "SCLK freq is not set" (tried, see my reply to v1).

Still an RFC on the approach: set_tdm_slot() is what simple-audio-card can
express from the device tree (dai-tdm-slot-num and dai-tdm-slot-width on
the codec node).

Tested on v7.3 on a HiBy R1 with two 32-bit slots and mask 0x3, the only
layout this board can run: the DAC gets the same frame registers as with
v1 (64-bit frame, channel 2 at bit 32) in stereo at 44.1 kHz and mono at
48 kHz. Other layouts are build-tested only, with W=1 on 734bef548a96
(v7.3-7363).

 sound/soc/codecs/cs43130.c | 76 +++++++++++++++++++++++++++++++-------
 sound/soc/codecs/cs43130.h |  3 ++
 2 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/sound/soc/codecs/cs43130.c b/sound/soc/codecs/cs43130.c
index 48d88dad4ceb..e3571ea08252 100644
--- a/sound/soc/codecs/cs43130.c
+++ b/sound/soc/codecs/cs43130.c
@@ -651,9 +651,16 @@ static int cs43130_set_sp_fmt(int dai_id, unsigned int bitwidth_sclk,
 		return -EINVAL;
 	}
 
-	frm_size = bitwidth_sclk * params_channels(params);
-	loc_ch1 = 0;
-	loc_ch2 = bitwidth_sclk * (params_channels(params) - 1);
+	if (cs43130->dais[dai_id].slots) {
+		/* The layout set with set_tdm_slot() */
+		frm_size = bitwidth_sclk * cs43130->dais[dai_id].slots;
+		loc_ch1 = cs43130->dais[dai_id].slot_loc[0];
+		loc_ch2 = cs43130->dais[dai_id].slot_loc[1];
+	} else {
+		frm_size = bitwidth_sclk * params_channels(params);
+		loc_ch1 = 0;
+		loc_ch2 = bitwidth_sclk * (params_channels(params) - 1);
+	}
 
 	frm_data = frm_delay & CS43130_SP_FSD_MASK;
 	frm_data |= (frm_phase << CS43130_SP_STP_SHIFT) & CS43130_SP_STP_MASK;
@@ -952,18 +959,24 @@ static int cs43130_hw_params(struct snd_pcm_substream *substream,
 		break;
 	}
 
-	if (!sclk && cs43130->dais[dai->id].dai_mode == SND_SOC_DAIFMT_CBP_CFP)
-		/* Calculate SCLK in master mode if unassigned */
-		sclk = params_rate(params) * bitwidth_dai *
-		       params_channels(params);
+	if (cs43130->dais[dai->id].slot_width) {
+		/* The slot width set with set_tdm_slot() */
+		bitwidth_sclk = cs43130->dais[dai->id].slot_width;
+	} else {
+		if (!sclk && cs43130->dais[dai->id].dai_mode == SND_SOC_DAIFMT_CBP_CFP)
+			/* Calculate SCLK in master mode if unassigned */
+			sclk = params_rate(params) * bitwidth_dai *
+			       params_channels(params);
+
+		if (!sclk) {
+			/* at this point, SCLK must be set */
+			dev_err(cs43130->dev, "SCLK freq is not set\n");
+			return -EINVAL;
+		}
 
-	if (!sclk) {
-		/* at this point, SCLK must be set */
-		dev_err(cs43130->dev, "SCLK freq is not set\n");
-		return -EINVAL;
+		bitwidth_sclk = (sclk / params_rate(params)) /
+				params_channels(params);
 	}
-
-	bitwidth_sclk = (sclk / params_rate(params)) / params_channels(params);
 	if (bitwidth_sclk < bitwidth_dai) {
 		dev_err(cs43130->dev, "Format not supported: SCLK freq is too low\n");
 		return -EINVAL;
@@ -1609,12 +1622,49 @@ static int cs43130_set_sysclk(struct snd_soc_dai *codec_dai,
 	return 0;
 }
 
+/*
+ * Sets the frame layout directly, which then takes precedence over the one
+ * derived from the set_sysclk() frequency: generic sound cards pass MCLK to
+ * set_sysclk(), not SCLK. The two channels go to the first and second slots
+ * set in tx_mask (both to the same slot for one), within frames of 16 to 64
+ * bits, the sizes the serial port clock tables cover. The PCM DAI only
+ * plays, so rx_mask is not used. Zero slots go back to the derived layout.
+ */
+static int cs43130_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
+				unsigned int rx_mask, int slots, int slot_width)
+{
+	struct snd_soc_component *component = dai->component;
+	struct cs43130_private *cs43130 = snd_soc_component_get_drvdata(component);
+	struct cs43130_dai *cs_dai = &cs43130->dais[dai->id];
+	unsigned int frm_size = slots * slot_width;
+
+	if (!slots) {
+		cs_dai->slots = 0;
+		cs_dai->slot_width = 0;
+		return 0;
+	}
+	if (slots < 0 || slot_width < 16 || slot_width > 32 ||
+	    frm_size > 64 || frm_size % 16)
+		return -EINVAL;
+	if (!tx_mask || hweight32(tx_mask) > 2 || tx_mask >= BIT(slots))
+		return -EINVAL;
+
+	cs_dai->slot_loc[0] = __ffs(tx_mask) * slot_width;
+	tx_mask &= tx_mask - 1;
+	cs_dai->slot_loc[1] = tx_mask ? __ffs(tx_mask) * slot_width :
+					cs_dai->slot_loc[0];
+	cs_dai->slots = slots;
+	cs_dai->slot_width = slot_width;
+	return 0;
+}
+
 static const struct snd_soc_dai_ops cs43130_pcm_ops = {
 	.startup	= cs43130_pcm_startup,
 	.hw_params	= cs43130_hw_params,
 	.hw_free	= cs43130_hw_free,
 	.set_sysclk	= cs43130_set_sysclk,
 	.set_fmt	= cs43130_pcm_set_fmt,
+	.set_tdm_slot	= cs43130_set_tdm_slot,
 };
 
 static const struct snd_soc_dai_ops cs43130_dop_ops = {
diff --git a/sound/soc/codecs/cs43130.h b/sound/soc/codecs/cs43130.h
index dbdb5b262f1b..f0a28ab0a3f9 100644
--- a/sound/soc/codecs/cs43130.h
+++ b/sound/soc/codecs/cs43130.h
@@ -495,6 +495,9 @@ static const char *const cs43130_supply_names[CS43130_NUM_SUPPLIES] = {
 
 struct cs43130_dai {
 	unsigned int			sclk;
+	unsigned int			slots;
+	unsigned int			slot_width;
+	u8				slot_loc[2];
 	unsigned int			dai_format;
 	unsigned int			dai_mode;
 	unsigned int			dai_invert;

base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
-- 
2.39.5

Re: [PATCH RFC v2] ASoC: cs43130: Take the frame layout from set_tdm_slot()

[Charles Keepax]

On Fri, Oct 23, 2026 at 06:32:06PM +0200, Olivier Louvignes wrote:
> hw_params() derives the slot width from the frequency given to the DAI's
> set_sysclk(), which it takes for SCLK. Generic cards pass MCLK there: the
> simple-audio-card of a HiBy R1 (Ingenic X1600 as I2S master, 64 bit
> clocks per frame, mclk-fs = 512) hands over 22.5792 MHz, so the driver
> sets up 256-bit slots for a 44.1 kHz stream and the CS43131 plays white
> noise. The card cannot simply leave mclk-fs out: simple-audio-card then
> sets no clock on the I2S controller either, and the codec, as clock
> slave, fails hw_params() with "SCLK freq is not set".
> 
> Add a set_tdm_slot() callback so the card can give the layout instead.
> The two channels go to the first and second slots set in tx_mask, within
> frames of 16 to 64 bits, the sizes the serial port clock tables cover.
> When set, the layout takes precedence over the one derived from
> set_sysclk(); zero slots go back to deriving it.
> 
> Tested on a HiBy R1, on v7.3, with dai-tdm-slot-num = <2> and
> dai-tdm-slot-width = <32> on the codec node.
> 
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
> ---
> +static int cs43130_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
> +				unsigned int rx_mask, int slots, int slot_width)
> +{
> +	struct snd_soc_component *component = dai->component;
> +	struct cs43130_private *cs43130 = snd_soc_component_get_drvdata(component);
> +	struct cs43130_dai *cs_dai = &cs43130->dais[dai->id];
> +	unsigned int frm_size = slots * slot_width;
> +
> +	if (!slots) {
> +		cs_dai->slots = 0;
> +		cs_dai->slot_width = 0;
> +		return 0;
> +	}
> +	if (slots < 0 || slot_width < 16 || slot_width > 32 ||
> +	    frm_size > 64 || frm_size % 16)
> +		return -EINVAL;
> +	if (!tx_mask || hweight32(tx_mask) > 2 || tx_mask >= BIT(slots))
> +		return -EINVAL;

Should we not be using rx_mask here? I think set_tdm is usually
centric to the device.

Otherwise, I think this looks good to me.

Thanks,
Charles