RE: [EXTERNAL] Re: [PATCH v8 1/3] ASoC: tac5xx2-sdw: add soundwire based codec driver

[Holalu Yogendra, Niranjan]

> On 17:18-20260414, Pierre-Louis Bossart wrote:
> Subject: Re: [PATCH v8 1/3] ASoC: tac5xx2-sdw: add soundwire based codec driver
> 
> > +static int tac_sdw_hw_params(struct snd_pcm_substream *substream,
> > +			     struct snd_pcm_hw_params *params,
> > +			     struct snd_soc_dai *dai)

> > +
> > +	mutex_lock(&tac_dev->pde_lock);
> > +	regmap_write(tac_dev->regmap, SDW_SDCA_CTL(function_id,
> pde_entity,
> > +						   TAC_SDCA_REQUESTED_PS,
> 0),
> > +		     0x03);
> > +	mutex_unlock(&tac_dev->pde_lock);
> 
> I think I provided the feedback several times that the "SDCA way" is to write
> REQUESTED_PS but then wait for ACTUAL_PS to reflect the intended power
> state.
> If this is not required for these devices, you absolutely want to comment on
> this non-standard device-specific sequences.
> 
> Edit: you have a loop to wait on ACTUAL_PS below when you write
> REQUESTED_PS a second time.
> This does require more comments or a code simplification...

This is to ensure D3, so that other writes will happen with device powered down ...

> > +
> > +	guard(mutex)(&tac_dev->pde_lock);
> > +	ret = regmap_write(tac_dev->regmap,
> > +			   SDW_SDCA_CTL(function_id, pde_entity,
> > +					TAC_SDCA_REQUESTED_PS, 0), 0);
> > +	if (ret) {
> > +		dev_err(tac_dev->dev, "failed to set PS to 0: %d\n", ret);
> > +		return ret;
> > +	}
> 
> humm, why do you need to write REQUESTED_PS again?

... and this is D3 -> D0 transition. I will see if I can improve this logic in the next patch.  Thanks 

Regards
Niranjan