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

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Thu, Jul 23, 2026 at 12:42:14AM +0100, Srinivas Kandagatla wrote:
> Add pde_pre_pmu and pde_post_pmu callbacks to struct sdca_class_hw_ops
> so codec drivers can hook into DAPM PDE power-domain transitions.
> 
> pde_pre_pmu is called before REQUESTED_PS=PS0 is written, allowing
> the driver to stage state (e.g. IT_USAGE, MIC_BIAS) that must be valid
> before the PDE sequencer runs.
> 
> pde_post_pmu is called after REQUESTED_PS=PS0 and before ACTUAL_PS
> polling begins, allowing the driver to commit pending writes (e.g.
> FU mute/volume via SCP_COMMIT) that the sequencer needs to see.
> 
> Both callbacks receive function_id and entity_id so the driver can act
> selectively per power domain.
> 
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
> ---
>  sound/soc/sdca/sdca_asoc.c  | 33 ++++++++++++++++++++++++++++++++-
>  sound/soc/sdca/sdca_class.h | 11 +++++++++++
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
> index ac688cd62970..795498b9a314 100644
> --- a/sound/soc/sdca/sdca_asoc.c
> +++ b/sound/soc/sdca/sdca_asoc.c
> @@ -32,6 +32,7 @@
>  #include <sound/soc-dai.h>
>  #include <sound/soc-dapm.h>
>  #include <sound/tlv.h>
> +#include "sdca_class.h"

I really don't like accessing the class code from here, bit of a
layering violation.

> @@ -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);

This is probably not a good way to do this. Ideally we don't want
to assume the class driver structure above this code, we have
users and are likely to for a while, that are doing more custom
stuff. I suspect we need to change what is stored in the
widget->priv to give us access to the bits we need.

> +		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;
>  	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);
> +		}
> +	}

Can't we just include this in the switch?

> +
>  	ret = sdca_asoc_pde_poll_actual_ps(component->dev, component->regmap,
>  					   SDW_SDCA_CTL_FUNC(widget->reg),
>  					   SDW_SDCA_CTL_ENT(widget->reg),
> @@ -502,7 +532,8 @@ static int entity_parse_pde(struct device *dev,
>  	(*widget)->mask = GENMASK(control->nbits - 1, 0);
>  	(*widget)->on_val = SDCA_PDE_PS0;
>  	(*widget)->off_val = SDCA_PDE_PS3;
> -	(*widget)->event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD;
> +	(*widget)->event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
> +				 SND_SOC_DAPM_POST_PMD;
>  	(*widget)->event = entity_pde_event;
>  	(*widget)->priv = entity;
>  	(*widget)++;
> diff --git a/sound/soc/sdca/sdca_class.h b/sound/soc/sdca/sdca_class.h
> index 74db9d4ce4a2..1278159ea61b 100644
> --- a/sound/soc/sdca/sdca_class.h
> +++ b/sound/soc/sdca/sdca_class.h
> @@ -29,6 +29,13 @@ struct sdca_function_data;
>   *             an array of struct sdca_function_data and stores the number
>   *             of entries in @num (must be > 0 and <= SDCA_MAX_FUNCTION_COUNT).
>   *             May be NULL.
> + * @pde_pre_pmu: called before DAPM writes REQUESTED_PS=PS0; use to
> + *             prepare device state that must be valid before the PDE
> + *             sequencer runs (e.g. IT_USAGE); may be NULL
> + * @pde_post_pmu: called after DAPM writes REQUESTED_PS=PS0 and before
> + *             ACTUAL_PS polling begins; use to commit pending register
> + *             writes (e.g. FUNCTION_ACTION) on devices that require an
> + *             explicit commit trigger for PDE power-up; may be NULL
>   *
>   * Codec-specific SoundWire drivers pass a pointer to this struct to
>   * sdca_class_probe() from their sdw_driver.probe.  Codec-specific
> @@ -39,6 +46,10 @@ struct sdca_function_data;
>  struct sdca_class_hw_ops {
>  	int  (*hw_init)(struct sdw_slave *slave);
>  	struct sdca_function_data *(*get_function_data)(unsigned int *num);
> +	int  (*pde_pre_pmu)(struct sdw_slave *slave, struct regmap *regmap,
> +			    unsigned int function_id, unsigned int entity_id);
> +	int  (*pde_post_pmu)(struct sdw_slave *slave, struct regmap *regmap,
> +			     unsigned int function_id, unsigned int entity_id);

Yeah this needs a little thought, the top two are directly class
driver things, but the bottom two are used by the lower layers.
We may need to do something like split these ops into two sets.

Thanks,
Charles