Re: [PATCH 11/28] ASoC: ops: Introduce 'snd_soc_deactivate_kctl'

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: ops: snd_soc_deactivate_kctl() API contract violation when no controls match
--

commit 5406f895c4457827c28646b6821be67f779d1939
Author: Martin Povišer <povik+lin@cutebit.org>

ASoC: ops: Introduce 'snd_soc_deactivate_kctl'

This commit introduces snd_soc_deactivate_kctl() which allows machine drivers
to activate or deactivate controls singly or by pattern. It is intended to be
useful for driver fixups.

> diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
> index 2f91ee7946bb1..d35e24df891b6 100644
> --- a/sound/soc/soc-ops.c
> +++ b/sound/soc/soc-ops.c

[ ... ]

> +/**
> + * snd_soc_deactivate_kctl - Activate/deactivate controls matching a pattern
> + *
> + * @card: where to look for the controls
> + * @name: name pattern
> + * @active: non-zero to activate, zero to deactivate
> + *
> + * Return number of matching controls on success, else error.
> + * No controls need to match.
> + */

[Severity: Medium]
The kernel-doc explicitly states that no controls need to match, which implies
calling this for an optional control shouldn't result in an error.

> +int snd_soc_deactivate_kctl(struct snd_soc_card *card,
> +	const char *name, int active)
> +{
> +	struct snd_kcontrol *kctl;
> +	int hits = 0;
> +	int ret;
> +
> +	/* Sanity check for name */
> +	if (unlikely(!name))
> +		return -EINVAL;
> +
> +	list_for_each_entry(kctl, &card->snd_card->controls, list) {
> +		if (!soc_control_matches(kctl, name))
> +			continue;
> +
> +		ret = snd_ctl_activate_id(card->snd_card, &kctl->id, active);
> +		if (ret < 0)
> +			return ret;
> +		hits++;
> +	}
> +
> +	if (!hits)
> +		return -EINVAL;

[Severity: Medium]
Does returning -EINVAL here violate the documentation above?

If a machine driver relies on the documentation to perform best-effort
deactivation of optional hardware-variant controls, its initialization
sequence could unexpectedly fail and abort the probe.

Should this return 0 instead when there are no matches?

> +
> +	return hits;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920-macaudio-v1-0-741cc20a74e5@gmail.com?part=11