Re: [PATCH v2 4/6] ASoC: qcom: lpass-rx-macro: Add shikra compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data race on `rx_mclk_users` and MCLK enablement due to lack of synchronization across CCF and DAPM contexts.
--

commit 3459bbdd1dba9dbf895d5d3272f789b95e526f28
Author: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

ASoC: qcom: lpass-rx-macro: Add shikra compatible

This commit adds shikra RX macro compatible data and hooks it into the
existing LPASS codec v4.1 flow. It reuses the v2.5 register access helpers
and bypasses the FS counter before enabling MCLK for codec versions 4.1
and later.

> diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
> index 2e74ec1a52f8a..83372123c9018 100644
> --- a/sound/soc/codecs/lpass-rx-macro.c
> +++ b/sound/soc/codecs/lpass-rx-macro.c
[ ... ]
> @@ -2045,6 +2047,16 @@ static int rx_macro_mclk_enable(struct rx_macro *rx, bool mclk_enable)
>  					   CDC_RX_CLK_MCLK2_ENABLE);
>  			regmap_update_bits(regmap, CDC_RX_CLK_RST_CTRL_FS_CNT_CONTROL,
>  					   CDC_RX_FS_MCLK_CNT_CLR_MASK, 0x00);
> +
> +			/*
> +			 * From LPASS codec v4.1 onwards the FS counter has to be
> +			 * bypassed before it is enabled, otherwise the fsgen
> +			 * output does not come up.
> +			 */
> +			if (rx->codec_version >= LPASS_CODEC_VERSION_4_1)
> +				regmap_set_bits(regmap, CDC_RX_CLK_RST_CTRL_FS_CNT_CONTROL,
> +						CDC_RX_FS_CNT_BYPASS);
> +

[Severity: High]
This is a pre-existing issue, but is there a risk of a data race on
rx_mclk_users and the MCLK enablement here?

It looks like this shared state is being read and mutated without a driver
lock across different contexts. For example, an unprivileged user can trigger
DAPM events (e.g., via amixer or PCM start) which calls into this function
through rx_macro_mclk_event(). Concurrently, the Soundwire runtime PM or
clock framework can call into this via swclk_gate_enable().

If both paths invoke rx_macro_mclk_enable() simultaneously, could a
read-modify-write race on rx->rx_mclk_users occur, leading to lost increments
or decrements?

If the increments are lost, can this result in the hardware clock being
prematurely disabled while a subsystem (like the Soundwire master) still
expects it to be running? Accessing unclocked registers on Qualcomm platforms
typically triggers an XPU violation, which might lead to an unrecoverable
kernel panic.

Would it make sense to introduce a lock to protect rx_mclk_users and the
corresponding register updates in this function?

>  			regmap_update_bits(regmap, CDC_RX_CLK_RST_CTRL_FS_CNT_CONTROL,
>  					   CDC_RX_FS_MCLK_CNT_EN_MASK,
>  					   CDC_RX_FS_MCLK_CNT_ENABLE);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-add_shikra_support-v2-0-fdd2129c696c@oss.qualcomm.com?part=4