From 94c18cea657c48680e4ee20b635b6c01f3eb352e Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Wed, 2 Sep 2026 09:55:53 +0200 Subject: [PATCH] ASoC: ux500: Correct MSP frame and bit clock setup 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 Link: https://patch.msgid.link/20260902-ux500-msp-fixes-v2-3-4b60b002d55a@kernel.org Signed-off-by: Mark Brown --- 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 -#define MSP_INPUT_FREQ_APB 48000000 - /*** Stereo mode. Used for APB data accesses as 16 bits accesses (mono), * 32 bits accesses (stereo). ***/