Re: [RFC PATCH 4/8] ASoC: SDCA: add PDE pre/post-pmu hooks to hw_ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Errors returned from the new `pde_pre_pmu` and `pde_post_pmu` hooks are incorrectly swallowed or shadowed, allowing DAPM transitions to proceed with invalid hardware state.
--

--- Patch [4]: [RFC PATCH 4/8] ASoC: SDCA: add PDE pre/post-pmu hooks to hw_ops ---
commit 9ebbdf1f4165266d78edf14f3e10a54fa79025c5
Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>

ASoC: SDCA: add PDE pre/post-pmu hooks to hw_ops

This patch adds pde_pre_pmu and pde_post_pmu callbacks to struct
sdca_class_hw_ops. These hooks allow codec drivers to stage state and
commit pending writes before and after DAPM PDE power-domain transitions.

> diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
> index ac688cd629706..795498b9a314b 100644
> --- a/sound/soc/sdca/sdca_asoc.c
> +++ b/sound/soc/sdca/sdca_asoc.c

[ ... ]

> @@ -432,6 +433,8 @@ static int entity_pde_event(struct snd_soc_dapm_widget *widget,
>  {
>  	struct snd_soc_component *component = snd_soc_dapm_to_component(widget->dapm);
>  	struct sdca_entity *entity = widget->priv;
> +	struct sdca_class_drv *core;
> +	unsigned int fn;
>  	int from, to;
>  	int ret;
>  
> @@ -439,6 +442,19 @@ static int entity_pde_event(struct snd_soc_dapm_widget *widget,
>  		return -EIO;
>  
>  	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		core = dev_get_drvdata(component->dev->parent);
> +		if (core && core->hw_ops && core->hw_ops->pde_pre_pmu) {
> +			fn = SDW_SDCA_CTL_FUNC(widget->reg);
> +			ret = core->hw_ops->pde_pre_pmu(core->sdw,
> +							component->regmap, fn,
> +							SDW_SDCA_CTL_ENT(widget->reg));
> +			if (ret)
> +				dev_warn(component->dev,
> +					 "%s: pde_pre_pmu failed: %d\n",
> +					 entity->label, ret);
> +		}
> +		return 0;

[Severity: Medium]
Should this return ret instead of 0 when pde_pre_pmu() fails? 

Returning 0 allows the DAPM power sequence to proceed even though the 
prerequisite state might be invalid.

>  	case SND_SOC_DAPM_POST_PMD:
>  		from = widget->on_val;
>  		to = widget->off_val;

[ ... ]

> @@ -451,6 +467,20 @@ static int entity_pde_event(struct snd_soc_dapm_widget *widget,
>  		return 0;
>  	}
>  
> +	if (event == SND_SOC_DAPM_POST_PMU) {
> +		core = dev_get_drvdata(component->dev->parent);
> +		if (core && core->hw_ops && core->hw_ops->pde_post_pmu) {
> +			fn = SDW_SDCA_CTL_FUNC(widget->reg);
> +			ret = core->hw_ops->pde_post_pmu(core->sdw,
> +							 component->regmap, fn,
> +							 SDW_SDCA_CTL_ENT(widget->reg));
> +			if (ret)
> +				dev_warn(component->dev,
> +					 "%s: pde_post_pmu failed: %d\n",
> +					 entity->label, ret);
> +		}
> +	}
> +
>  	ret = sdca_asoc_pde_poll_actual_ps(component->dev, component->regmap,
>  					   SDW_SDCA_CTL_FUNC(widget->reg),
>  					   SDW_SDCA_CTL_ENT(widget->reg),

[Severity: Medium]
Does this overwrite the error from pde_post_pmu()? 

If pde_post_pmu() fails, ret is logged in the warning above, but then 
immediately overwritten by the return value of sdca_asoc_pde_poll_actual_ps(), 
losing the original failure context.

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