Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Linus Walleij <linusw@kernel.org>
To: Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	 Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	 Philipp Zabel <p.zabel@pengutronix.de>
Cc: linux-sound@vger.kernel.org, Linus Walleij <linusw@kernel.org>
Subject: [PATCH v2 3/9] ASoC: ux500: Correct MSP frame and bit clock setup
Date: Wed, 02 Sep 2026 09:55:53 +0200	[thread overview]
Message-ID: <20260902-ux500-msp-fixes-v2-3-4b60b002d55a@kernel.org> (raw)
In-Reply-To: <20260902-ux500-msp-fixes-v2-0-4b60b002d55a@kernel.org>

FRPER plus one is the number of bit clocks in a frame. It must follow
the configured slot count and width. The legacy rate-dependent
constants produce malformed frames; notably, a 16-slot, 16-bit frame
is programmed as 278 rather than 256 clocks.

Derive the frame period from the TDM geometry and use the real
functional clock rate. Validate that the requested bit clock has an
exact, representable divider, program SCKDIV as divider minus one, and
report the resulting bit clock using that same divisor.

Fixes: 3592b7f69a54 ("ASoC: Ux500: Add MSP I2S-driver")
Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 sound/soc/ux500/ux500_msp_dai.c | 75 +++++++++--------------------------------
 sound/soc/ux500/ux500_msp_dai.h | 11 ------
 sound/soc/ux500/ux500_msp_i2s.c | 54 ++++++++++++++++-------------
 sound/soc/ux500/ux500_msp_i2s.h |  2 --
 4 files changed, 46 insertions(+), 96 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 29d25a3f6f40..56d5591e2269 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -59,72 +59,21 @@ static int setup_pcm_multichan(struct snd_soc_dai *dai,
 	return 0;
 }
 
-static int setup_frameper(struct snd_soc_dai *dai, unsigned int rate,
-			struct msp_protdesc *prot_desc)
+static void setup_frameper(struct snd_soc_dai *dai,
+			   struct msp_protdesc *prot_desc)
 {
 	struct ux500_msp_i2s_drvdata *drvdata = dev_get_drvdata(dai->dev);
 
-	switch (drvdata->slots) {
-	case 1:
-		switch (rate) {
-		case 8000:
-			prot_desc->frame_period =
-				FRAME_PER_SINGLE_SLOT_8_KHZ;
-			break;
-
-		case 16000:
-			prot_desc->frame_period =
-				FRAME_PER_SINGLE_SLOT_16_KHZ;
-			break;
-
-		case 44100:
-			prot_desc->frame_period =
-				FRAME_PER_SINGLE_SLOT_44_1_KHZ;
-			break;
-
-		case 48000:
-			prot_desc->frame_period =
-				FRAME_PER_SINGLE_SLOT_48_KHZ;
-			break;
-
-		default:
-			dev_err(dai->dev,
-				"%s: Error: Unsupported sample-rate (freq = %d)!\n",
-				__func__, rate);
-			return -EINVAL;
-		}
-		break;
-
-	case 2:
-		prot_desc->frame_period = FRAME_PER_2_SLOTS;
-		break;
-
-	case 8:
-		prot_desc->frame_period = FRAME_PER_8_SLOTS;
-		break;
-
-	case 16:
-		prot_desc->frame_period = FRAME_PER_16_SLOTS;
-		break;
-	default:
-		dev_err(dai->dev,
-			"%s: Error: Unsupported slot-count (slots = %d)!\n",
-			__func__, drvdata->slots);
-		return -EINVAL;
-	}
-
-	prot_desc->clocks_per_frame =
-			prot_desc->frame_period+1;
+	prot_desc->clocks_per_frame = drvdata->slots * drvdata->slot_width;
+	prot_desc->frame_period = prot_desc->clocks_per_frame - 1;
 
 	dev_dbg(dai->dev, "%s: Clocks per frame: %u\n",
 		__func__,
 		prot_desc->clocks_per_frame);
-
-	return 0;
 }
 
-static int setup_pcm_framing(struct snd_soc_dai *dai, unsigned int rate,
-			struct msp_protdesc *prot_desc)
+static int setup_pcm_framing(struct snd_soc_dai *dai,
+			     struct msp_protdesc *prot_desc)
 {
 	struct ux500_msp_i2s_drvdata *drvdata = dev_get_drvdata(dai->dev);
 
@@ -165,7 +114,9 @@ static int setup_pcm_framing(struct snd_soc_dai *dai, unsigned int rate,
 	prot_desc->tx_elem_len_2 = MSP_ELEM_LEN_16;
 	prot_desc->rx_elem_len_2 = MSP_ELEM_LEN_16;
 
-	return setup_frameper(dai, rate, prot_desc);
+	setup_frameper(dai, prot_desc);
+
+	return 0;
 }
 
 static int setup_clocking(struct snd_soc_dai *dai,
@@ -366,7 +317,7 @@ static int setup_msp_config(struct snd_pcm_substream *substream,
 		if (ret < 0)
 			return ret;
 
-		ret = setup_pcm_framing(dai, runtime->rate, prot_desc);
+		ret = setup_pcm_framing(dai, prot_desc);
 		if (ret < 0)
 			return ret;
 
@@ -735,7 +686,6 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
 	drvdata->tx_mask = 0x01;
 	drvdata->rx_mask = 0x01;
 	drvdata->slot_width = 16;
-	drvdata->master_clk = MSP_INPUT_FREQ_APB;
 
 	drvdata->reg_vape = devm_regulator_get(&pdev->dev, "v-ape");
 	if (IS_ERR(drvdata->reg_vape)) {
@@ -764,6 +714,11 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
 			__func__, ret);
 		return ret;
 	}
+	drvdata->master_clk = clk_get_rate(drvdata->clk);
+	if (!drvdata->master_clk) {
+		dev_err(&pdev->dev, "MSP clock has no rate\n");
+		return -EINVAL;
+	}
 
 	ret = ux500_msp_i2s_init_msp(pdev, &drvdata->msp);
 	if (ret) {
diff --git a/sound/soc/ux500/ux500_msp_dai.h b/sound/soc/ux500/ux500_msp_dai.h
index 30bf70838196..19058c238420 100644
--- a/sound/soc/ux500/ux500_msp_dai.h
+++ b/sound/soc/ux500/ux500_msp_dai.h
@@ -22,17 +22,6 @@
 
 #define UX500_I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE)
 
-#define FRAME_PER_SINGLE_SLOT_8_KHZ		31
-#define FRAME_PER_SINGLE_SLOT_16_KHZ	124
-#define FRAME_PER_SINGLE_SLOT_44_1_KHZ	63
-#define FRAME_PER_SINGLE_SLOT_48_KHZ	49
-#define FRAME_PER_2_SLOTS				31
-#define FRAME_PER_8_SLOTS				138
-#define FRAME_PER_16_SLOTS				277
-
-#define UX500_MSP_INTERNAL_CLOCK_FREQ  40000000
-#define UX500_MSP1_INTERNAL_CLOCK_FREQ UX500_MSP_INTERNAL_CLOCK_FREQ
-
 #define UX500_MSP_MIN_CHANNELS		1
 #define UX500_MSP_MAX_CHANNELS		8
 
diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index ec6f0874294a..ef41de92d8e7 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -212,35 +212,20 @@ static int configure_protocol(struct ux500_msp *msp,
 
 static int setup_bitclk(struct ux500_msp *msp, struct ux500_msp_config *config)
 {
+	struct msp_protdesc *protdesc;
+	u64 desired_bitclk;
+	unsigned int bitclk;
 	u32 reg_val_GCR;
-	u32 frame_per = 0;
-	u32 sck_div = 0;
-	u32 frame_width = 0;
-	u32 temp_reg = 0;
-	struct msp_protdesc *protdesc = NULL;
+	u32 sck_div;
+	u32 temp_reg;
 
 	reg_val_GCR = readl(msp->registers + MSP_GCR);
 	writel(reg_val_GCR & ~SRG_ENABLE, msp->registers + MSP_GCR);
 
-	if (config->default_protdesc)
-		protdesc =
-			(struct msp_protdesc *)&prot_descs[config->protocol];
-	else
-		protdesc = (struct msp_protdesc *)&config->protdesc;
-
 	switch (config->protocol) {
 	case MSP_PCM_PROTOCOL:
 	case MSP_PCM_COMPAND_PROTOCOL:
-		frame_width = protdesc->frame_width;
-		sck_div = config->f_inputclk / (config->frame_freq *
-			(protdesc->clocks_per_frame));
-		frame_per = protdesc->frame_period;
-		break;
 	case MSP_I2S_PROTOCOL:
-		frame_width = protdesc->frame_width;
-		sck_div = config->f_inputclk / (config->frame_freq *
-			(protdesc->clocks_per_frame));
-		frame_per = protdesc->frame_period;
 		break;
 	default:
 		dev_err(msp->dev, "%s: ERROR: Unknown protocol (%d)!\n",
@@ -249,12 +234,35 @@ static int setup_bitclk(struct ux500_msp *msp, struct ux500_msp_config *config)
 		return -EINVAL;
 	}
 
+	if (config->default_protdesc)
+		protdesc = (struct msp_protdesc *)&prot_descs[config->protocol];
+	else
+		protdesc = &config->protdesc;
+
+	if (!config->frame_freq || !protdesc->clocks_per_frame)
+		return -EINVAL;
+
+	desired_bitclk = (u64)config->frame_freq * protdesc->clocks_per_frame;
+	if (desired_bitclk > config->f_inputclk)
+		return -EINVAL;
+	bitclk = desired_bitclk;
+	if (config->f_inputclk % bitclk) {
+		dev_err(msp->dev,
+			"Input clock %u cannot generate bit clock %u\n",
+			config->f_inputclk, bitclk);
+		return -EINVAL;
+	}
+
+	sck_div = config->f_inputclk / bitclk;
+	if (!sck_div || sck_div > SCK_DIV_MASK + 1)
+		return -EINVAL;
+
 	temp_reg = (sck_div - 1) & SCK_DIV_MASK;
-	temp_reg |= FRAME_WIDTH_BITS(frame_width);
-	temp_reg |= FRAME_PERIOD_BITS(frame_per);
+	temp_reg |= FRAME_WIDTH_BITS(protdesc->frame_width);
+	temp_reg |= FRAME_PERIOD_BITS(protdesc->frame_period);
 	writel(temp_reg, msp->registers + MSP_SRG);
 
-	msp->f_bitclk = (config->f_inputclk)/(sck_div + 1);
+	msp->f_bitclk = config->f_inputclk / sck_div;
 
 	/* Enable bit-clock */
 	udelay(100);
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index d75a0974369a..80085dde5079 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -12,8 +12,6 @@
 
 #include <linux/platform_device.h>
 
-#define MSP_INPUT_FREQ_APB 48000000
-
 /*** Stereo mode. Used for APB data accesses as 16 bits accesses (mono),
  *   32 bits accesses (stereo).
  ***/

-- 
2.55.0


  parent reply	other threads:[~2026-09-02  7:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02  7:55 [PATCH v2 0/9] ASoC: ux500: Fix MSP lifecycle, clocking and resources Linus Walleij
2026-09-02  7:55 ` [PATCH v2 1/9] ASoC: ux500: Fix MSP stream lifecycle handling Linus Walleij
2026-09-02  7:55 ` [PATCH v2 2/9] ASoC: ux500: Propagate MSP setup errors Linus Walleij
2026-09-02  7:55 ` Linus Walleij [this message]
2026-09-02  7:55 ` [PATCH v2 4/9] ASoC: ux500: Validate MSP DAI configuration Linus Walleij
2026-09-02  7:55 ` [PATCH v2 5/9] ASoC: ux500: Deassert the MSP reset during probe Linus Walleij
2026-09-02  7:55 ` [PATCH v2 6/9] ASoC: ux500: Request the MSP MMIO resource Linus Walleij
2026-09-02  7:55 ` [PATCH v2 7/9] ASoC: ux500: Remove obsolete PRCMU QoS calls Linus Walleij
2026-09-02  7:55 ` [PATCH v2 8/9] ASoC: ux500: Allow repeated MSP prepare calls Linus Walleij
2026-09-02  7:55 ` [PATCH v2 9/9] ASoC: ux500: Program the MSP FIFO watermarks Linus Walleij
2026-09-02 11:58 ` [PATCH v2 0/9] ASoC: ux500: Fix MSP lifecycle, clocking and resources Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260902-ux500-msp-fixes-v2-3-4b60b002d55a@kernel.org \
    --to=linusw@kernel.org \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-sound@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox