Re: [PATCH v10 1/4] ASoC: SDCA: Add PDE state transition helper

[Charles Keepax]

On Tue, Apr 21, 2026 at 06:21:09PM +0200, Pierre-Louis Bossart wrote:
> On 4/21/26 17:57, Charles Keepax wrote:
> > On Tue, Apr 21, 2026 at 09:18:01PM +0530, Niranjan H Y wrote:
> >> Per SDCA specification, after writing REQUESTED_PS, drivers must
> >> poll ACTUAL_PS until the target power state is reached.
> >> Implement sdca_asoc_set_pde_state_sync() helper function for
> >> changing the power state of the PDE widget and wait for the power
> >> transition to happen or timeout.
> >>
> >> Changes include:
> >> - Add sdca_asoc_set_pde_poll_sync() to handle write
> >>   REQUESTED_PS register and poll ACTUAL_PS register until
> >>   the target state is reached or timed out.
> >> - Export function via sdca_asoc.h for use by SDCA-compliant drivers
> >> - Refactor entity_pde_event() in sdca_asoc.c to use the helper
> >>
> >> Signed-off-by: Niranjan H Y <niranjan.hy@xxxxxx>
> >> ---
> >>  static int entity_parse_pde(struct device *dev,
> >> @@ -449,8 +492,8 @@ static int entity_parse_pde(struct device *dev,
> >>  	}
> >>  
> >>  	(*widget)->id = snd_soc_dapm_supply;
> >> -	(*widget)->reg = SDW_SDCA_CTL(function->desc->adr, entity->id, control->sel, 0);
> >> -	(*widget)->mask = GENMASK(control->nbits - 1, 0);
> >> +	(*widget)->reg = SND_SOC_NOPM;
> >> +	(*widget)->mask = 0;
> > 
> > Hmm... yeah I am really sorry I totally overlooked this. Yeah we
> > should leave the write outside the helper it is much nicer to
> > have DAPM do it.
> 
> Not following that comment, there are quite a few codecs that
> trap a DAPM event, do a regmap_write then wait with a loop,
> see e.g. rt722_sdca_pde47_event

If you have a good reason to do the write manually you can
(typically because it needs sequencing with other writes), but
generally if the DAPM widget can do the write its better to do it
that way. Just cleaner.

Apologies totally my bad on this I should have realised the
implications of this change the first time.

> I haven't seen DAPM deal directly with the write?
> what am I missing?

That is what is happening in the above code setting the
reg/mask/on_val/off_val, DAPM uses that information to do the
register write.

Thanks,
Charles