Re: [PATCH v2 5/9] ASoC: ux500: Remove the MOP500 machine driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] ALSA: devicetree ABI breakage due to removal of stericsson,snd-soc-mop500
- [High] ALSA: ux500: mono streams fail with -EINVAL due to rigid static TDM slot masks
- [Medium] ALSA: silent loss of audio on kernel upgrade due to missing SND_AUDIO_GRAPH_CARD2 selection
- [Medium] ALSA: ux500: loss of cross-DAI synchronization allows conflicting hardware parameters
--

commit 310750ed2beb07b175f87e9079b77587ed249ddd
Author: Linus Walleij <linusw@kernel.org>

ASoC: ux500: Remove the MOP500 machine driver

This patch removes the MOP500 machine driver and its obsolete binding, replacing
it with audio-graph-card2. It updates the ux500 ASoC Kconfig to directly select
the codec, MSP, and DMA platform components previously selected by the machine
driver.

> diff --git a/sound/soc/ux500/Kconfig b/sound/soc/ux500/Kconfig
> index 1130580603500..f3c7c216267c0 100644
> --- a/sound/soc/ux500/Kconfig
> +++ b/sound/soc/ux500/Kconfig
> @@ -6,28 +6,19 @@ menuconfig SND_SOC_UX500
>  	tristate "Ux500"
>  	depends on SND_SOC
>  	depends on MFD_DB8500_PRCMU
> +	depends on AB8500_CORE && AB8500_GPADC
> +	select SND_SOC_AB8500_CODEC
> +	select SND_SOC_UX500_PLAT_MSP_I2S
> +	select SND_SOC_UX500_PLAT_DMA

[Severity: Medium]
Will this lead to a silent loss of audio on kernel upgrade?

When upgrading a kernel config where CONFIG_SND_SOC_UX500_MACH_MOP500 was set,
the old config is dropped. Since the required replacement
CONFIG_SND_AUDIO_GRAPH_CARD2 is neither selected nor implied by SND_SOC_UX500,
users upgrading their kernel using make oldconfig might lose audio support.

[ ... ]

> diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c
> deleted file mode 100644
> index 6d196b4b88027..0000000000000
> --- a/sound/soc/ux500/mop500.c
> +++ /dev/null
[ ... ]
> -static const struct of_device_id snd_soc_mop500_match[] = {
> -	{ .compatible = "stericsson,snd-soc-mop500", },
> -	{},
> -};

[Severity: High]
Does removing this compatible string break devicetree backward compatibility?

Without a backward-compatibility binding provided in the generic driver, audio
will fail to probe on systems booting with existing or older devicetrees that
contain the stericsson,snd-soc-mop500 compatible string.

[ ... ]

> diff --git a/sound/soc/ux500/mop500_ab8500.c b/sound/soc/ux500/mop500_ab8500.c
> deleted file mode 100644
> index feb683c55d113..0000000000000
> --- a/sound/soc/ux500/mop500_ab8500.c
> +++ /dev/null
[ ... ]
> -static int mop500_ab8500_hw_params(struct snd_pcm_substream *substream,
> -			struct snd_pcm_hw_params *params)
> -{
[ ... ]
> -	/* Ensure configuration consistency between DAIs */
> -	scoped_guard(mutex, &mop500_ab8500_params_lock) {
> -		if (mop500_ab8500_usage) {
> -			if (mop500_ab8500_rate != params_rate(params) ||
> -			    mop500_ab8500_channels != params_channels(params)) {
> -				return -EBUSY;
> -			}
> -		} else {
> -			mop500_ab8500_rate = params_rate(params);
> -			mop500_ab8500_channels = params_channels(params);
> -		}
> -		__set_bit(cpu_dai->id, &mop500_ab8500_usage);
> -	}

[Severity: Medium]
Will the removal of these consistency checks allow conflicting hardware
parameters?

When two applications open concurrent streams on the separate DAIs exposed by
the AB8500 codec, requesting different sample rates or channel counts, the
hardware might receive conflicting configuration requests. Since this lock is
removed and neither the generic audio-graph-card2 nor the codec driver
enforces cross-DAI consistency, can this cause synchronization regressions?

[ ... ]
> -	is_playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
> -	switch (channels) {
> -	case 1:
> -		slots = 16;
> -		tx_slots = (is_playback) ? TX_SLOT_MONO : 0;
> -		rx_slots = (is_playback) ? 0 : RX_SLOT_MONO;
> -		break;
> -	case 2:

[Severity: High]
Does removing this dynamic TDM slot configuration cause mono streams to fail?

Without this dynamic update, the CPU DAI falls back to enforcing the static
devicetree mask. The ALSA core will call ux500_msp_dai_hw_params(), which
validates the channel count against the static TDM mask. 

Because the devicetree statically sets a stereo TDM mask (slots_active = 2),
a mono playback request evaluates params_channels() != slots_active as true
and returns -EINVAL. Can this prevent users or applications from requesting
mono playback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-ux500-simple-sound-v2-0-88d29416e3c3@kernel.org?part=5