Re: [RFC PATCH 1/8] ASoC: SDCA: hw_params: program upstream Input Terminals for OT DAI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The recursive graph traversal in `set_input_terminal_sources()` lacks cycle detection, allowing malformed firmware topologies to cause infinite recursion and a kernel stack overflow.
- [High] Output Terminal format parameters are blindly propagated to all upstream Input Terminals, breaking topologies with intermediate processing units (e.g., Sample Rate Converters or Mixers).
- [High] Unconditional traversal of the static source graph configures inactive alternative paths and clobbers shared hardware nodes without mutual exclusion.
--

--- Patch [1]: [RFC PATCH 1/8] ASoC: SDCA: hw_params: program upstream Input Terminals for OT DAI ---
commit e1c19ca1fbb78240fd7df2f80b2ad3f08d85052a
Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>

ASoC: SDCA: hw_params: program upstream Input Terminals for OT DAI

For a capture stream the DAI is the Output Terminal so
sdca_asoc_hw_params() only programs the OT itself: cluster, clock
and Usage.  The upstream Input Terminals that actually source the
audio never get their Usage, ClusterIndex or clock programmed, so
the power domain comes up but no samples are produced.
    
Walk the source graph from the OT and program every Input Terminal
reachable through it.

> diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
> index b4dedba719dc6..ac688cd629706 100644
> --- a/sound/soc/sdca/sdca_asoc.c
> +++ b/sound/soc/sdca/sdca_asoc.c
[ ... ]
> @@ -1726,6 +1726,51 @@ static int set_usage(struct device *dev, struct regmap *regmap,
>  	return -EINVAL;
>  }
>  
> +/*
> + * For a capture stream the ASoC DAI is the Output Terminal, so hw_params
> + * only programs the OT itself.  The upstream Input Terminal(s) that
> + * actually source the audio (e.g. a mic input behind a power-domain entity)
> + * never get their Usage / ClusterIndex / clock programmed and stay inactive,
> + * so the power domain comes up but no samples are produced.  Walk the source
> + * graph from the OT and program every Input Terminal we reach.
> + */
> +static int set_input_terminal_sources(struct device *dev, struct regmap *regmap,
> +				      struct sdca_function_data *function,
> +				      struct sdca_entity *entity,
> +				      int channels, int rate, int width)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < entity->num_sources; i++) {
> +		struct sdca_entity *src = entity->sources[i];

[Severity: High]
Does this code inadvertently configure inactive alternative paths?

The loop iterates over all statically defined sources in the firmware graph,
which might ignore active ALSA/DAPM routing state like MUX selections.

Additionally, if an IT is actively shared between multiple OTs, could starting
a stream on the second OT dynamically overwrite the active IT's registers
without locking and corrupt the first stream?

> +
> +		if (src->type == SDCA_ENTITY_TYPE_IT) {
> +			ret = set_cluster(dev, regmap, function, src, channels);
> +			if (ret)
> +				return ret;
> +
> +			if (src->iot.clock) {
> +				ret = set_clock(dev, regmap, function,
> +						src->iot.clock, rate);
> +				if (ret)
> +					return ret;
> +			}
> +
> +			ret = set_usage(dev, regmap, function, src,
> +					SDCA_CTL_IT_USAGE, rate, width);

[Severity: High]
Can this break topologies that use intermediate processing units like a
Sample Rate Converter or channel mixer?

This propagates the Output Terminal's format parameters (channels, rate, width)
directly to all upstream Input Terminals. If the IT does not natively support
the exact same hardware parameters as the OT, won't set_usage() or
set_cluster() fail with -EINVAL?

> +			if (ret)
> +				return ret;
> +		}
> +
> +		ret = set_input_terminal_sources(dev, regmap, function, src,
> +						 channels, rate, width);

[Severity: High]
Is it possible for a malformed firmware topology to cause an infinite loop
here?

The recursive graph traversal in set_input_terminal_sources() doesn't appear
to have cycle detection. If entity->sources contains a cycle, could this
lead to a kernel stack overflow?

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722234221.884765-1-srinivas.kandagatla@oss.qualcomm.com?part=1