Re: [PATCH v4 2/2] ASoC: cs35l45: Adds support for SYNC bus

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Spurious ALSA event generation in `cs35l45_sync_en_put` due to unnormalized boolean comparison.
--

--- Patch [2]: [PATCH v4 2/2] ASoC: cs35l45: Adds support for SYNC bus ---
commit 7447f2ac68a757cce556917899f132a9bfc50231
Author: Ricardo Rivera-Matos <[email protected]>

ASoC: cs35l45: Adds support for SYNC bus

Adds support for the multidevice synchronization (SYNC) bus. Initial support
only includes software (SW) data packets.

> diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c
> index a032bb23b4ac7..2d6360bc555b3 100644
> --- a/sound/soc/codecs/cs35l45.c
> +++ b/sound/soc/codecs/cs35l45.c
[ ... ]
> @@ -195,6 +196,46 @@ static int cs35l45_activate_ctl(struct snd_soc_component *component,
>  	return 0;
>  }
[ ... ]
> +static int cs35l45_sync_en_put(struct snd_kcontrol *kcontrol,
> +				      struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct cs35l45_private *cs35l45 =
> +			snd_soc_component_get_drvdata(component);
> +	struct snd_soc_dapm_context *dapm =
> +			snd_soc_component_to_dapm(component);
> +
> +	snd_soc_dapm_mutex_lock(dapm);
> +
> +	if (ucontrol->value.integer.value[0] == cs35l45->sync_en) {

[Severity: Medium]
Will this comparison cause spurious ALSA events if userspace writes a non-zero,
non-one value? 

Since cs35l45->sync_en is a bool, if userspace writes a value like 2, the
comparison (2 == 1) evaluates to false. This bypasses the short-circuit check.

> +		snd_soc_dapm_mutex_unlock(dapm);
> +		return 0;
> +	}
> +
> +	if ((bool)ucontrol->value.integer.value[0])
> +		regmap_set_bits(cs35l45->regmap, CS35L45_BLOCK_ENABLES2, CS35L45_SYNC_EN_MASK);
> +	else
> +		regmap_clear_bits(cs35l45->regmap, CS35L45_BLOCK_ENABLES2, CS35L45_SYNC_EN_MASK);
> +
> +	cs35l45->sync_en = ucontrol->value.integer.value[0];

When the unnormalized value is later assigned to the bool, it normalizes to 1. 
The function then returns 1, incorrectly generating a state change notification
even though the logical state didn't change.

Should the input value be normalized to a boolean before comparing and
assigning?

> +
> +	snd_soc_dapm_mutex_unlock(dapm);
> +
> +	return 1;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-sound-for-next-cs35l45-sync-v4-v4-0-20d3a07c3b8d@opensource.cirrus.com?part=2