[PATCH v2 0/9] ASoC: ux500: Fix MSP lifecycle, clocking and resources

[PATCH v2 0/9] ASoC: ux500: Fix MSP lifecycle, clocking and resources

[Linus Walleij]

Repair independent correctness problems in the DB8500 multichannel
serial port driver. The series fixes duplex and repeated-prepare stream
ownership, error propagation, frame and divider programming, FIFO
watermarks, DAI validation, reset handling, and MMIO resource ownership.
It also removes obsolete PRCMU QoS calls which are unconditional stubs.

The register programming follows the DB8500 v2 reference manual. These
changes are independent of the later sound-card devicetree conversion.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Changes in v2:
- Rebased on v7.3-rc1
- Link to v1: https://lore.kernel.org/r/20260902-ux500-msp-fixes-v1-0-da1447fd9f7b@kernel.org

---
Linus Walleij (9):
      ASoC: ux500: Fix MSP stream lifecycle handling
      ASoC: ux500: Propagate MSP setup errors
      ASoC: ux500: Correct MSP frame and bit clock setup
      ASoC: ux500: Validate MSP DAI configuration
      ASoC: ux500: Deassert the MSP reset during probe
      ASoC: ux500: Request the MSP MMIO resource
      ASoC: ux500: Remove obsolete PRCMU QoS calls
      ASoC: ux500: Allow repeated MSP prepare calls
      ASoC: ux500: Program the MSP FIFO watermarks

 sound/soc/ux500/ux500_msp_dai.c | 189 ++++++++++++----------------
 sound/soc/ux500/ux500_msp_dai.h |  14 +--
 sound/soc/ux500/ux500_msp_i2s.c | 264 +++++++++++++++++++++++++++-------------
 sound/soc/ux500/ux500_msp_i2s.h |  12 +-
 4 files changed, 271 insertions(+), 208 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260825-ux500-msp-fixes-58d434a9bac3

Best regards,
-- 
Linus Walleij <linusw@kernel.org>

[PATCH v2 1/9] ASoC: ux500: Fix MSP stream lifecycle handling

[Linus Walleij]

The trigger stop path drops the direction busy flag even though ALSA
still owns the stream until shutdown. A later trigger cannot reliably
restart it, shutdown may leave the block configured, and a second
stream may overwrite shared duplex configuration.

Keep configured and running directions as separate state. Program
shared settings only for the first direction, require a compatible
configuration for the other half of a duplex stream, and enable the
frame generator only while a provider stream is running. Also fix the
RX-disable direction test and preserve the other direction multichannel
setup.

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 |   8 +-
 sound/soc/ux500/ux500_msp_i2s.c | 183 ++++++++++++++++++++++++++++++----------
 sound/soc/ux500/ux500_msp_i2s.h |   4 +
 3 files changed, 149 insertions(+), 46 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 499e826d7120..994422e72512 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -34,8 +34,10 @@ static int setup_pcm_multichan(struct snd_soc_dai *dai,
 	if (drvdata->slots > 1) {
 		msp_config->multichannel_configured = 1;
 
-		multi->tx_multichannel_enable = true;
-		multi->rx_multichannel_enable = true;
+		multi->tx_multichannel_enable =
+			msp_config->direction & MSP_DIR_TX;
+		multi->rx_multichannel_enable =
+			msp_config->direction & MSP_DIR_RX;
 		multi->rx_comparison_enable_mode = MSP_COMPARISON_DISABLED;
 
 		multi->tx_channel_0_enable = drvdata->tx_mask;
@@ -192,6 +194,7 @@ static int setup_clocking(struct snd_soc_dai *dai,
 	case SND_SOC_DAIFMT_BC_FC:
 		dev_dbg(dai->dev, "%s: Codec is master.\n", __func__);
 
+		msp_config->clock_provider = false;
 		msp_config->iodelay = 0x20;
 		msp_config->rx_fsync_sel = 0;
 		msp_config->tx_fsync_sel = 1 << TFSSEL_SHIFT;
@@ -204,6 +207,7 @@ static int setup_clocking(struct snd_soc_dai *dai,
 	case SND_SOC_DAIFMT_BP_FP:
 		dev_dbg(dai->dev, "%s: Codec is slave.\n", __func__);
 
+		msp_config->clock_provider = true;
 		msp_config->tx_clk_sel = TX_CLK_SEL_SRG;
 		msp_config->tx_fsync_sel = TX_SYNC_SRG_PROG;
 		msp_config->rx_clk_sel = RX_CLK_SEL_SRG;
diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index fbfeefa418ca..ec6f0874294a 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -344,20 +344,27 @@ static int configure_multichannel(struct ux500_msp *msp,
 	return 0;
 }
 
-static int enable_msp(struct ux500_msp *msp, struct ux500_msp_config *config)
+static int enable_msp(struct ux500_msp *msp, struct ux500_msp_config *config,
+		      bool first)
 {
-	int status = 0;
-	u32 reg_val_DMACR, reg_val_GCR;
+	int status;
+	u32 reg_val_DMACR;
 
 	/* Configure msp with protocol dependent settings */
-	configure_protocol(msp, config);
-	setup_bitclk(msp, config);
+	status = configure_protocol(msp, config);
+	if (status)
+		return status;
+
+	if (first && config->clock_provider) {
+		status = setup_bitclk(msp, config);
+		if (status)
+			return status;
+	}
+
 	if (config->multichannel_configured == 1) {
 		status = configure_multichannel(msp, config);
 		if (status)
-			dev_warn(msp->dev,
-				"%s: WARN: configure_multichannel failed (%d)!\n",
-				__func__, status);
+			return status;
 	}
 
 	reg_val_DMACR = readl(msp->registers + MSP_DMACR);
@@ -369,11 +376,7 @@ static int enable_msp(struct ux500_msp *msp, struct ux500_msp_config *config)
 
 	writel(config->iodelay, msp->registers + MSP_IODLY);
 
-	/* Enable frame generation logic */
-	reg_val_GCR = readl(msp->registers + MSP_GCR);
-	writel(reg_val_GCR | FRAME_GEN_ENABLE, msp->registers + MSP_GCR);
-
-	return status;
+	return 0;
 }
 
 static void flush_fifo_rx(struct ux500_msp *msp)
@@ -411,12 +414,36 @@ static void flush_fifo_tx(struct ux500_msp *msp)
 	writel(reg_val_GCR, msp->registers + MSP_GCR);
 }
 
+static bool ux500_msp_config_compatible(struct ux500_msp *msp,
+					struct ux500_msp_config *config)
+{
+	struct ux500_msp_config *active = &msp->config;
+
+	return active->f_inputclk == config->f_inputclk &&
+	       active->tx_clk_sel == config->tx_clk_sel &&
+	       active->rx_clk_sel == config->rx_clk_sel &&
+	       active->srg_clk_sel == config->srg_clk_sel &&
+	       active->rx_fsync_pol == config->rx_fsync_pol &&
+	       active->tx_fsync_pol == config->tx_fsync_pol &&
+	       active->rx_fsync_sel == config->rx_fsync_sel &&
+	       active->tx_fsync_sel == config->tx_fsync_sel &&
+	       active->default_protdesc == config->default_protdesc &&
+	       active->protocol == config->protocol &&
+	       active->frame_freq == config->frame_freq &&
+	       active->data_size == config->data_size &&
+	       active->def_elem_len == config->def_elem_len &&
+	       active->clock_provider == config->clock_provider &&
+	       !memcmp(&active->protdesc, &config->protdesc,
+		       sizeof(active->protdesc));
+}
+
 int ux500_msp_i2s_open(struct ux500_msp *msp,
 		struct ux500_msp_config *config)
 {
 	u32 old_reg, new_reg, mask;
 	int res;
 	unsigned int tx_sel, rx_sel, tx_busy, rx_busy;
+	bool first;
 
 	if (in_interrupt()) {
 		dev_err(msp->dev,
@@ -444,40 +471,66 @@ int ux500_msp_i2s_open(struct ux500_msp *msp,
 		return -EBUSY;
 	}
 
-	msp->dir_busy |= (tx_sel ? MSP_DIR_TX : 0) | (rx_sel ? MSP_DIR_RX : 0);
-
-	/* First do the global config register */
-	mask = RX_CLK_SEL_MASK | TX_CLK_SEL_MASK | RX_FSYNC_MASK |
-	    TX_FSYNC_MASK | RX_SYNC_SEL_MASK | TX_SYNC_SEL_MASK |
-	    RX_FIFO_ENABLE_MASK | TX_FIFO_ENABLE_MASK | SRG_CLK_SEL_MASK |
-	    LOOPBACK_MASK | TX_EXTRA_DELAY_MASK;
-
-	new_reg = (config->tx_clk_sel | config->rx_clk_sel |
-		config->rx_fsync_pol | config->tx_fsync_pol |
-		config->rx_fsync_sel | config->tx_fsync_sel |
-		config->rx_fifo_config | config->tx_fifo_config |
-		config->srg_clk_sel | config->loopback_enable |
-		config->tx_data_enable);
+	first = !msp->dir_busy;
+	if (!first && !ux500_msp_config_compatible(msp, config)) {
+		dev_err(msp->dev, "%s: Incompatible duplex configuration\n",
+			__func__);
+		return -EBUSY;
+	}
 
-	old_reg = readl(msp->registers + MSP_GCR);
-	old_reg &= ~mask;
-	new_reg |= old_reg;
-	writel(new_reg, msp->registers + MSP_GCR);
+	if (first) {
+		/* First do the global config register */
+		mask = RX_CLK_SEL_MASK | TX_CLK_SEL_MASK | RX_FSYNC_MASK |
+		       TX_FSYNC_MASK | RX_SYNC_SEL_MASK | TX_SYNC_SEL_MASK |
+		       RX_FIFO_ENABLE_MASK | TX_FIFO_ENABLE_MASK |
+		       SRG_CLK_SEL_MASK | LOOPBACK_MASK | TX_EXTRA_DELAY_MASK;
+
+		new_reg = config->tx_clk_sel | config->rx_clk_sel |
+			  config->rx_fsync_pol | config->tx_fsync_pol |
+			  config->rx_fsync_sel | config->tx_fsync_sel |
+			  config->rx_fifo_config | config->tx_fifo_config |
+			  config->srg_clk_sel | config->loopback_enable |
+			  config->tx_data_enable;
+
+		old_reg = readl(msp->registers + MSP_GCR);
+		old_reg &= ~mask;
+		new_reg |= old_reg;
+		writel(new_reg, msp->registers + MSP_GCR);
+	}
 
-	res = enable_msp(msp, config);
+	res = enable_msp(msp, config, first);
 	if (res < 0) {
 		dev_err(msp->dev, "%s: ERROR: enable_msp failed (%d)!\n",
 			__func__, res);
-		return -EBUSY;
+		if (tx_sel)
+			writel(0, msp->registers + MSP_TCF);
+		if (rx_sel)
+			writel(0, msp->registers + MSP_RCF);
+		if (first) {
+			writel(0, msp->registers + MSP_GCR);
+			writel(0, msp->registers + MSP_DMACR);
+			writel(0, msp->registers + MSP_SRG);
+			writel(0, msp->registers + MSP_MCR);
+		}
+		return res;
+	}
+
+	msp->dir_busy |= config->direction;
+	if (first) {
+		msp->config = *config;
+		msp->clock_provider = config->clock_provider;
 	}
 	if (config->loopback_enable & 0x80)
 		msp->loopback_enable = 1;
 
 	/* Flush FIFOs */
-	flush_fifo_tx(msp);
-	flush_fifo_rx(msp);
+	if (tx_sel)
+		flush_fifo_tx(msp);
+	if (rx_sel)
+		flush_fifo_rx(msp);
 
-	msp->msp_state = MSP_STATE_CONFIGURED;
+	if (!msp->dir_running)
+		msp->msp_state = MSP_STATE_CONFIGURED;
 	return 0;
 }
 
@@ -494,7 +547,6 @@ static void disable_msp_rx(struct ux500_msp *msp)
 			~(RX_SERVICE_INT | RX_OVERRUN_ERROR_INT),
 			msp->registers + MSP_IMSC);
 
-	msp->dir_busy &= ~MSP_DIR_RX;
 }
 
 static void disable_msp_tx(struct ux500_msp *msp)
@@ -510,7 +562,6 @@ static void disable_msp_tx(struct ux500_msp *msp)
 			~(TX_SERVICE_INT | TX_UNDERRUN_ERR_INT),
 			msp->registers + MSP_IMSC);
 
-	msp->dir_busy &= ~MSP_DIR_TX;
 }
 
 static int disable_msp(struct ux500_msp *msp, unsigned int dir)
@@ -520,7 +571,7 @@ static int disable_msp(struct ux500_msp *msp, unsigned int dir)
 
 	reg_val_GCR = readl(msp->registers + MSP_GCR);
 	disable_tx = dir & MSP_DIR_TX;
-	disable_rx = dir & MSP_DIR_TX;
+	disable_rx = dir & MSP_DIR_RX;
 	if (disable_tx && disable_rx) {
 		reg_val_GCR = readl(msp->registers + MSP_GCR);
 		writel(reg_val_GCR | LOOPBACK_MASK,
@@ -553,7 +604,15 @@ static int disable_msp(struct ux500_msp *msp, unsigned int dir)
 
 int ux500_msp_i2s_trigger(struct ux500_msp *msp, int cmd, int direction)
 {
-	u32 reg_val_GCR, enable_bit;
+	u32 reg_val_DMACR, reg_val_GCR, dma_enable_bit, enable_bit;
+	unsigned int dir;
+
+	if (direction == SNDRV_PCM_STREAM_PLAYBACK)
+		dir = MSP_DIR_TX;
+	else if (direction == SNDRV_PCM_STREAM_CAPTURE)
+		dir = MSP_DIR_RX;
+	else
+		return -EINVAL;
 
 	if (msp->msp_state == MSP_STATE_IDLE) {
 		dev_err(msp->dev, "%s: ERROR: MSP is not configured!\n",
@@ -565,21 +624,44 @@ int ux500_msp_i2s_trigger(struct ux500_msp *msp, int cmd, int direction)
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (direction == SNDRV_PCM_STREAM_PLAYBACK)
+		if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
 			enable_bit = TX_ENABLE;
-		else
+			dma_enable_bit = TX_DMA_ENABLE;
+		} else {
 			enable_bit = RX_ENABLE;
+			dma_enable_bit = RX_DMA_ENABLE;
+		}
+		if (!(msp->dir_busy & dir))
+			return -EINVAL;
+		reg_val_DMACR = readl(msp->registers + MSP_DMACR);
+		writel(reg_val_DMACR | dma_enable_bit,
+		       msp->registers + MSP_DMACR);
 		reg_val_GCR = readl(msp->registers + MSP_GCR);
+		if (msp->clock_provider)
+			enable_bit |= FRAME_GEN_ENABLE;
 		writel(reg_val_GCR | enable_bit, msp->registers + MSP_GCR);
+		msp->dir_running |= dir;
+		msp->msp_state = MSP_STATE_RUNNING;
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		if (direction == SNDRV_PCM_STREAM_PLAYBACK)
+		if (!(msp->dir_busy & dir))
+			return -EINVAL;
+		if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
 			disable_msp_tx(msp);
-		else
+			msp->dir_running &= ~MSP_DIR_TX;
+		} else {
 			disable_msp_rx(msp);
+			msp->dir_running &= ~MSP_DIR_RX;
+		}
+		if (!msp->dir_running) {
+			reg_val_GCR = readl(msp->registers + MSP_GCR);
+			writel(reg_val_GCR & ~FRAME_GEN_ENABLE,
+			       msp->registers + MSP_GCR);
+			msp->msp_state = MSP_STATE_CONFIGURED;
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -594,7 +676,18 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
 
 	dev_dbg(msp->dev, "%s: Enter (dir = 0x%01x).\n", __func__, dir);
 
+	if (!dir || dir & ~(MSP_DIR_TX | MSP_DIR_RX) ||
+	    (msp->dir_busy & dir) != dir)
+		return -EINVAL;
+
 	status = disable_msp(msp, dir);
+	msp->dir_busy &= ~dir;
+	msp->dir_running &= ~dir;
+	if (msp->dir_busy && !msp->dir_running) {
+		writel(readl(msp->registers + MSP_GCR) & ~FRAME_GEN_ENABLE,
+		       msp->registers + MSP_GCR);
+		msp->msp_state = MSP_STATE_CONFIGURED;
+	}
 	if (msp->dir_busy == 0) {
 		/* disable sample rate and frame generators */
 		msp->msp_state = MSP_STATE_IDLE;
@@ -618,6 +711,8 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
 		writel(0, msp->registers + MSP_RCE1);
 		writel(0, msp->registers + MSP_RCE2);
 		writel(0, msp->registers + MSP_RCE3);
+		memset(&msp->config, 0, sizeof(msp->config));
+		msp->clock_provider = false;
 	}
 
 	return status;
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index 69d4ebc409fc..d75a0974369a 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -460,6 +460,7 @@ struct ux500_msp_config {
 	enum msp_data_size data_size;
 	unsigned int def_elem_len;
 	unsigned int iodelay;
+	bool clock_provider;
 };
 
 struct ux500_msp {
@@ -470,8 +471,11 @@ struct ux500_msp {
 	enum msp_state msp_state;
 	int def_elem_len;
 	unsigned int dir_busy;
+	unsigned int dir_running;
 	int loopback_enable;
 	unsigned int f_bitclk;
+	bool clock_provider;
+	struct ux500_msp_config config;
 };
 
 int ux500_msp_i2s_init_msp(struct platform_device *pdev,

-- 
2.55.0

[PATCH v2 2/9] ASoC: ux500: Propagate MSP setup errors

[Linus Walleij]

The prepare callback continues with a partly initialized configuration
when format setup fails. Probe likewise tests the allocated pointer
instead of the return value, so an MMIO resource or mapping failure can
be ignored after allocation succeeds.

Return configuration failures from prepare and test the MSP
initialization result directly.

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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 994422e72512..29d25a3f6f40 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -468,7 +468,9 @@ static int ux500_msp_dai_prepare(struct snd_pcm_substream *substream,
 	dev_dbg(dai->dev, "%s: MSP %d (%s): Enter (rate = %d).\n", __func__,
 		dai->id, snd_pcm_stream_str(substream), runtime->rate);
 
-	setup_msp_config(substream, dai, &msp_config);
+	ret = setup_msp_config(substream, dai, &msp_config);
+	if (ret)
+		return ret;
 
 	ret = ux500_msp_i2s_open(drvdata->msp, &msp_config);
 	if (ret < 0) {
@@ -764,7 +766,7 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
 	}
 
 	ret = ux500_msp_i2s_init_msp(pdev, &drvdata->msp);
-	if (!drvdata->msp) {
+	if (ret) {
 		dev_err(&pdev->dev,
 			"%s: ERROR: Failed to init MSP-struct (%d)!",
 			__func__, ret);

-- 
2.55.0

[PATCH v2 3/9] ASoC: ux500: Correct MSP frame and bit clock setup

[Linus Walleij]

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

[PATCH v2 4/9] ASoC: ux500: Validate MSP DAI configuration

[Linus Walleij]

Installing channel constraints from hw_params is too late to affect the
parameters being committed. The driver consequently accepts channel
counts which disagree with the I2S or TDM setup. It also silently
truncates out-of-range slot masks and accepts inverted bit clock formats
which prepare then rejects.

Validate the selected channel count directly, reject invalid masks
before changing cached TDM state, and implement all four standard clock
and frame inversion combinations. Use the requested format in
validation diagnostics.

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 | 41 +++++++++++++++++++++++++++--------------
 sound/soc/ux500/ux500_msp_i2s.c |  7 +++++--
 sound/soc/ux500/ux500_msp_i2s.h |  1 +
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 56d5591e2269..78278927cc53 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -130,7 +130,16 @@ static int setup_clocking(struct snd_soc_dai *dai,
 	case SND_SOC_DAIFMT_NB_IF:
 		msp_config->tx_fsync_pol ^= 1 << TFSPOL_SHIFT;
 		msp_config->rx_fsync_pol ^= 1 << RFSPOL_SHIFT;
+		break;
+
+	case SND_SOC_DAIFMT_IB_NF:
+		msp_config->bclk_inverted = true;
+		break;
 
+	case SND_SOC_DAIFMT_IB_IF:
+		msp_config->bclk_inverted = true;
+		msp_config->tx_fsync_pol ^= 1 << TFSPOL_SHIFT;
+		msp_config->rx_fsync_pol ^= 1 << RFSPOL_SHIFT;
 		break;
 
 	default:
@@ -453,7 +462,6 @@ static int ux500_msp_dai_hw_params(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai)
 {
 	unsigned int mask, slots_active;
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct ux500_msp_i2s_drvdata *drvdata = dev_get_drvdata(dai->dev);
 
 	dev_dbg(dai->dev, "%s: MSP %d (%s): Enter.\n",
@@ -461,9 +469,8 @@ static int ux500_msp_dai_hw_params(struct snd_pcm_substream *substream,
 
 	switch (drvdata->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
-		snd_pcm_hw_constraint_minmax(runtime,
-				SNDRV_PCM_HW_PARAM_CHANNELS,
-				1, 2);
+		if (params_channels(params) < 1 || params_channels(params) > 2)
+			return -EINVAL;
 		break;
 
 	case SND_SOC_DAIFMT_DSP_B:
@@ -475,9 +482,8 @@ static int ux500_msp_dai_hw_params(struct snd_pcm_substream *substream,
 		slots_active = hweight32(mask);
 		dev_dbg(dai->dev, "TDM-slots active: %d", slots_active);
 
-		snd_pcm_hw_constraint_single(runtime,
-				SNDRV_PCM_HW_PARAM_CHANNELS,
-				slots_active);
+		if (!slots_active || params_channels(params) != slots_active)
+			return -EINVAL;
 		break;
 
 	default:
@@ -510,20 +516,21 @@ static int ux500_msp_dai_set_dai_fmt(struct snd_soc_dai *dai,
 	default:
 		dev_err(dai->dev,
 			"%s: Error: Unsupported protocol/master (fmt = 0x%x)!\n",
-			__func__, drvdata->fmt);
+			__func__, fmt);
 		return -EINVAL;
 	}
 
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_NB_NF:
 	case SND_SOC_DAIFMT_NB_IF:
+	case SND_SOC_DAIFMT_IB_NF:
 	case SND_SOC_DAIFMT_IB_IF:
 		break;
 
 	default:
 		dev_err(dai->dev,
 			"%s: Error: Unsupported inversion (fmt = 0x%x)!\n",
-			__func__, drvdata->fmt);
+			__func__, fmt);
 		return -EINVAL;
 	}
 
@@ -557,17 +564,23 @@ static int ux500_msp_dai_set_tdm_slot(struct snd_soc_dai *dai,
 			__func__, slots);
 		return -EINVAL;
 	}
-	drvdata->slots = slots;
 
-	if (!(slot_width == 16)) {
+	if (slot_width != 16) {
 		dev_err(dai->dev, "%s: Error: Unsupported slot-width (%d)!\n",
 			__func__, slot_width);
 		return -EINVAL;
 	}
-	drvdata->slot_width = slot_width;
 
-	drvdata->tx_mask = tx_mask & cap;
-	drvdata->rx_mask = rx_mask & cap;
+	if ((tx_mask | rx_mask) & ~cap) {
+		dev_err(dai->dev, "%s: Slot mask exceeds %d slots\n",
+			__func__, slots);
+		return -EINVAL;
+	}
+
+	drvdata->slots = slots;
+	drvdata->slot_width = slot_width;
+	drvdata->tx_mask = tx_mask;
+	drvdata->rx_mask = rx_mask;
 
 	return 0;
 }
diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index ef41de92d8e7..bc77174e0070 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -201,10 +201,12 @@ static int configure_protocol(struct ux500_msp *msp,
 
 	/* The code below should not be separated. */
 	temp_reg = readl(msp->registers + MSP_GCR) & ~TX_CLK_POL_RISING;
-	temp_reg |= MSP_TX_CLKPOL_BIT(~protdesc->tx_clk_pol);
+	temp_reg |= MSP_TX_CLKPOL_BIT(!protdesc->tx_clk_pol ^
+					  config->bclk_inverted);
 	writel(temp_reg, msp->registers + MSP_GCR);
 	temp_reg = readl(msp->registers + MSP_GCR) & ~RX_CLK_POL_RISING;
-	temp_reg |= MSP_RX_CLKPOL_BIT(protdesc->rx_clk_pol);
+	temp_reg |= MSP_RX_CLKPOL_BIT(protdesc->rx_clk_pol ^
+					  config->bclk_inverted);
 	writel(temp_reg, msp->registers + MSP_GCR);
 
 	return 0;
@@ -441,6 +443,7 @@ static bool ux500_msp_config_compatible(struct ux500_msp *msp,
 	       active->data_size == config->data_size &&
 	       active->def_elem_len == config->def_elem_len &&
 	       active->clock_provider == config->clock_provider &&
+	       active->bclk_inverted == config->bclk_inverted &&
 	       !memcmp(&active->protdesc, &config->protdesc,
 		       sizeof(active->protdesc));
 }
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index 80085dde5079..17b5c37a7e5d 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -459,6 +459,7 @@ struct ux500_msp_config {
 	unsigned int def_elem_len;
 	unsigned int iodelay;
 	bool clock_provider;
+	bool bclk_inverted;
 };
 
 struct ux500_msp {

-- 
2.55.0

[PATCH v2 5/9] ASoC: ux500: Deassert the MSP reset during probe

[Linus Walleij]

The devicetree has described each MSP reset line since the PRCC reset
controller was added, but the driver never acquires or deasserts it. The
block can consequently remain inaccessible when firmware has left it in
reset.

Acquire the reset exclusively and keep it deasserted for the lifetime of
the bound device.

Fixes: 95f04048325c ("ARM: dts: ux500: Add reset lines to IP blocks")
Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 sound/soc/ux500/ux500_msp_dai.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 78278927cc53..b3de115d829a 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -14,6 +14,7 @@
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include <linux/mfd/db8500-prcmu.h>
 
 #include <sound/soc.h>
@@ -686,6 +687,7 @@ static const struct snd_soc_component_driver ux500_msp_component = {
 static int ux500_msp_drv_probe(struct platform_device *pdev)
 {
 	struct ux500_msp_i2s_drvdata *drvdata;
+	struct reset_control *reset;
 	int ret = 0;
 
 	drvdata = devm_kzalloc(&pdev->dev,
@@ -733,6 +735,11 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	reset = devm_reset_control_get_exclusive_deasserted(&pdev->dev, NULL);
+	if (IS_ERR(reset))
+		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
+				     "Failed to deassert MSP reset\n");
+
 	ret = ux500_msp_i2s_init_msp(pdev, &drvdata->msp);
 	if (ret) {
 		dev_err(&pdev->dev,

-- 
2.55.0

[PATCH v2 6/9] ASoC: ux500: Request the MSP MMIO resource

[Linus Walleij]

A bare devm_ioremap() neither reserves the register range nor preserves
the platform resource error. This permits another driver to claim the
same range and reports every mapping failure as an allocation failure.

Use the managed platform resource helper, retaining the resolved
resource only to derive the DMA register address.

Fixes: 3592b7f69a54 ("ASoC: Ux500: Add MSP I2S-driver")
Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 sound/soc/ux500/ux500_msp_i2s.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index bc77174e0070..43dc9b3aa4ef 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -733,7 +733,7 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
 int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 			struct ux500_msp **msp_p)
 {
-	struct resource *res = NULL;
+	struct resource *res;
 	struct ux500_msp *msp;
 
 	*msp_p = devm_kzalloc(&pdev->dev, sizeof(struct ux500_msp), GFP_KERNEL);
@@ -743,20 +743,10 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 
 	msp->dev = &pdev->dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (res == NULL) {
-		dev_err(&pdev->dev, "%s: ERROR: Unable to get resource!\n",
-			__func__);
-		return -ENOMEM;
-	}
-
+	msp->registers = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(msp->registers))
+		return PTR_ERR(msp->registers);
 	msp->tx_rx_addr = res->start + MSP_DR;
-	msp->registers = devm_ioremap(&pdev->dev, res->start,
-				      resource_size(res));
-	if (msp->registers == NULL) {
-		dev_err(&pdev->dev, "%s: ERROR: ioremap failed!\n", __func__);
-		return -ENOMEM;
-	}
 
 	msp->msp_state = MSP_STATE_IDLE;
 	msp->loopback_enable = 0;

-- 
2.55.0

[PATCH v2 7/9] ASoC: ux500: Remove obsolete PRCMU QoS calls

[Linus Walleij]

The DB8500 PRCMU QoS interface consists of unconditional inline stubs,
so the MSP calls and cached constraint state have no effect. Device
power and clocks are already represented by the regulator, power-domain
and common-clock frameworks.

Remove the dead calls and their private state instead of pretending to
change the APE operating point.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 sound/soc/ux500/ux500_msp_dai.c | 26 --------------------------
 sound/soc/ux500/ux500_msp_dai.h |  2 --
 2 files changed, 28 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index b3de115d829a..5b4b3126637e 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -15,7 +15,6 @@
 #include <linux/of.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
-#include <linux/mfd/db8500-prcmu.h>
 
 #include <sound/soc.h>
 #include <sound/soc-dai.h>
@@ -393,12 +392,6 @@ static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream,
 	dev_dbg(dai->dev, "%s: MSP %d (%s): Enter.\n", __func__, dai->id,
 		snd_pcm_stream_str(substream));
 
-	if (drvdata->vape_opp_constraint == 1) {
-		prcmu_qos_update_requirement(PRCMU_QOS_APE_OPP,
-					"ux500_msp_i2s", 50);
-		drvdata->vape_opp_constraint = 0;
-	}
-
 	if (ux500_msp_i2s_close(drvdata->msp,
 				is_playback ? MSP_DIR_TX : MSP_DIR_RX)) {
 		dev_err(dai->dev,
@@ -440,21 +433,6 @@ static int ux500_msp_dai_prepare(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	/* Set OPP-level */
-	if ((drvdata->fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) &&
-		(drvdata->msp->f_bitclk > 19200000)) {
-		/* If the bit-clock is higher than 19.2MHz, Vape should be
-		 * run in 100% OPP. Only when bit-clock is used (MSP master)
-		 */
-		prcmu_qos_update_requirement(PRCMU_QOS_APE_OPP,
-					"ux500-msp-i2s", 100);
-		drvdata->vape_opp_constraint = 1;
-	} else {
-		prcmu_qos_update_requirement(PRCMU_QOS_APE_OPP,
-					"ux500-msp-i2s", 50);
-		drvdata->vape_opp_constraint = 0;
-	}
-
 	return ret;
 }
 
@@ -710,8 +688,6 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
 			__func__, ret);
 		return ret;
 	}
-	prcmu_qos_add_requirement(PRCMU_QOS_APE_OPP, (char *)pdev->name, 50);
-
 	drvdata->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
 	if (IS_ERR(drvdata->pclk)) {
 		ret = PTR_ERR(drvdata->pclk);
@@ -780,8 +756,6 @@ static void ux500_msp_drv_remove(struct platform_device *pdev)
 
 	snd_soc_unregister_component(&pdev->dev);
 
-	prcmu_qos_remove_requirement(PRCMU_QOS_APE_OPP, "ux500_msp_i2s");
-
 	ux500_msp_i2s_cleanup_msp(pdev, drvdata->msp);
 }
 
diff --git a/sound/soc/ux500/ux500_msp_dai.h b/sound/soc/ux500/ux500_msp_dai.h
index 19058c238420..ad4ce69bfbf5 100644
--- a/sound/soc/ux500/ux500_msp_dai.h
+++ b/sound/soc/ux500/ux500_msp_dai.h
@@ -46,8 +46,6 @@ struct ux500_msp_i2s_drvdata {
 	struct clk *clk;
 	struct clk *pclk;
 
-	/* Regulators */
-	int vape_opp_constraint;
 };
 
 int ux500_msp_dai_set_data_delay(struct snd_soc_dai *dai, int delay);

-- 
2.55.0

[PATCH v2 8/9] ASoC: ux500: Allow repeated MSP prepare calls

[Linus Walleij]

ALSA can call the DAI prepare callback again after an XRUN without
first shutting down the stream. The MSP open helper rejects the second
call with -EBUSY because the direction remains configured.

Track successful playback and capture configurations at the DAI layer.
Make repeated prepare calls no-ops and only close directions which were
successfully prepared.

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 | 28 +++++++++++++++++++++-------
 sound/soc/ux500/ux500_msp_dai.h |  1 +
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 5b4b3126637e..37c48cc70394 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -388,15 +388,21 @@ static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream,
 	int ret;
 	struct ux500_msp_i2s_drvdata *drvdata = dev_get_drvdata(dai->dev);
 	bool is_playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
+	unsigned int configured = is_playback ? PLAYBACK_CONFIGURED :
+		CAPTURE_CONFIGURED;
+	unsigned int dir = is_playback ? MSP_DIR_TX : MSP_DIR_RX;
 
 	dev_dbg(dai->dev, "%s: MSP %d (%s): Enter.\n", __func__, dai->id,
 		snd_pcm_stream_str(substream));
 
-	if (ux500_msp_i2s_close(drvdata->msp,
-				is_playback ? MSP_DIR_TX : MSP_DIR_RX)) {
-		dev_err(dai->dev,
-			"%s: Error: MSP %d (%s): Unable to close i2s.\n",
-			__func__, dai->id, snd_pcm_stream_str(substream));
+	if (drvdata->configured & configured) {
+		if (ux500_msp_i2s_close(drvdata->msp, dir)) {
+			dev_err(dai->dev,
+				"%s: Error: MSP %d (%s): Unable to close i2s.\n",
+				__func__, dai->id,
+				snd_pcm_stream_str(substream));
+		}
+		drvdata->configured &= ~configured;
 	}
 
 	/* Disable and unprepare clocks */
@@ -414,14 +420,20 @@ static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream,
 static int ux500_msp_dai_prepare(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai)
 {
-	int ret = 0;
 	struct ux500_msp_i2s_drvdata *drvdata = dev_get_drvdata(dai->dev);
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct ux500_msp_config msp_config;
+	bool is_playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+	unsigned int configured = is_playback ? PLAYBACK_CONFIGURED :
+		CAPTURE_CONFIGURED;
+	int ret;
 
 	dev_dbg(dai->dev, "%s: MSP %d (%s): Enter (rate = %d).\n", __func__,
 		dai->id, snd_pcm_stream_str(substream), runtime->rate);
 
+	if (drvdata->configured & configured)
+		return 0;
+
 	ret = setup_msp_config(substream, dai, &msp_config);
 	if (ret)
 		return ret;
@@ -433,7 +445,9 @@ static int ux500_msp_dai_prepare(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	return ret;
+	drvdata->configured |= configured;
+
+	return 0;
 }
 
 static int ux500_msp_dai_hw_params(struct snd_pcm_substream *substream,
diff --git a/sound/soc/ux500/ux500_msp_dai.h b/sound/soc/ux500/ux500_msp_dai.h
index ad4ce69bfbf5..aae582030d95 100644
--- a/sound/soc/ux500/ux500_msp_dai.h
+++ b/sound/soc/ux500/ux500_msp_dai.h
@@ -36,6 +36,7 @@ struct ux500_msp_i2s_drvdata {
 	struct ux500_msp *msp;
 	struct regulator *reg_vape;
 	unsigned int fmt;
+	unsigned int configured;
 	unsigned int tx_mask;
 	unsigned int rx_mask;
 	int slots;

-- 
2.55.0

[PATCH v2 9/9] ASoC: ux500: Program the MSP FIFO watermarks

[Linus Walleij]

The DMA engine is configured for four-element bursts, but the MSP
driver never programs the FIFO watermark register and instead depends
on its previous or reset value. The DB8500 DMA request protocol requires
the peripheral watermark to match the DMA packet size.

Program four-element receive and transmit watermarks when configuring
the first direction, before enabling MSP DMA requests.

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

diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index 43dc9b3aa4ef..683b485fb570 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -507,6 +507,8 @@ int ux500_msp_i2s_open(struct ux500_msp *msp,
 		old_reg &= ~mask;
 		new_reg |= old_reg;
 		writel(new_reg, msp->registers + MSP_GCR);
+		writel(MSP_WMRK_TX_4_ELEMENTS | MSP_WMRK_RX_4_ELEMENTS,
+		       msp->registers + MSP_WMRK);
 	}
 
 	res = enable_msp(msp, config, first);
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index 17b5c37a7e5d..2bf2699bdc49 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -62,6 +62,7 @@ enum msp_direction {
 #define MSP_SRG		0x10
 #define MSP_FLR		0x14
 #define MSP_DMACR	0x18
+#define MSP_WMRK	0x1c
 
 #define MSP_IMSC	0x20
 #define MSP_RIS		0x24
@@ -228,6 +229,10 @@ enum msp_direction {
 #define RDMAE_SHIFT		0
 #define TDMAE_SHIFT		1
 
+/* FIFO watermark register */
+#define MSP_WMRK_RX_4_ELEMENTS	BIT(0)
+#define MSP_WMRK_TX_4_ELEMENTS	BIT(3)
+
 /* Interrupt Register */
 #define RX_SERVICE_INT		BIT(0)
 #define RX_OVERRUN_ERROR_INT	BIT(1)

-- 
2.55.0

Re: [PATCH v2 0/9] ASoC: ux500: Fix MSP lifecycle, clocking and resources

[Mark Brown]

On Wed, 02 Sep 2026 09:55:50 +0200, Linus Walleij wrote:
> ASoC: ux500: Fix MSP lifecycle, clocking and resources
> 
> Repair independent correctness problems in the DB8500 multichannel
> serial port driver. The series fixes duplex and repeated-prepare stream
> ownership, error propagation, frame and divider programming, FIFO
> watermarks, DAI validation, reset handling, and MMIO resource ownership.
> It also removes obsolete PRCMU QoS calls which are unconditional stubs.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.3

Thanks!

[1/9] ASoC: ux500: Fix MSP stream lifecycle handling
      https://git.kernel.org/broonie/sound/c/c37ba8fe00f2
[2/9] ASoC: ux500: Propagate MSP setup errors
      https://git.kernel.org/broonie/sound/c/3415421a2b0b
[3/9] ASoC: ux500: Correct MSP frame and bit clock setup
      https://git.kernel.org/broonie/sound/c/94c18cea657c
[4/9] ASoC: ux500: Validate MSP DAI configuration
      https://git.kernel.org/broonie/sound/c/9ccbacf5a012
[5/9] ASoC: ux500: Deassert the MSP reset during probe
      https://git.kernel.org/broonie/sound/c/66ec63e7a90b
[6/9] ASoC: ux500: Request the MSP MMIO resource
      https://git.kernel.org/broonie/sound/c/4fb67925f33a
[7/9] ASoC: ux500: Remove obsolete PRCMU QoS calls
      https://git.kernel.org/broonie/sound/c/7b819677b503
[8/9] ASoC: ux500: Allow repeated MSP prepare calls
      https://git.kernel.org/broonie/sound/c/dc1a1b1e2206
[9/9] ASoC: ux500: Program the MSP FIFO watermarks
      https://git.kernel.org/broonie/sound/c/2519439b4b5f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark