Re: [PATCH 3/4] ASoC: SDCA: Add basic SDCA class driver

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

On Tue, Nov 04, 2025 at 05:13:39PM +0100, Pierre-Louis Bossart wrote:
> On 10/30/25 16:36, Richard Fitzgerald wrote:
> > On 30/10/2025 3:29 pm, Charles Keepax wrote:
> >> On Mon, Oct 27, 2025 at 04:02:54PM +0100, Pierre-Louis Bossart wrote:
> > 
> > SNIP
> > 
> >>>> +    pm_runtime_set_autosuspend_delay(dev, 250);
> >>>> +    pm_runtime_use_autosuspend(dev);
> >>>> +    pm_runtime_set_active(dev);
> >>>> +    pm_runtime_get_noresume(dev);
> >>>
> >>> This seems odd, I don't recall that we do something similar in existing codec drivers.
> >>> It'd be worth explaining the whole pm_runtime flow.
> >>
> >> I suspect that means those codec drivers are broken :-) at least
> >> if they actively use pm_runtime. The first two enable
> >> autosuspend, that is suspending the device after a short period
> >> of inactivity. set_active tells the PM runtime the device is
> >> already powered up, as it is. Finally, get_noresume takes a
> >> reference, this will stop it powering down immediately once the
> >> pm_runtime is enabled, it will waiting until we put the reference
> >> we are holding.
> >>
> > Other drivers definitely do that.
> > For example:
> > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/codecs/rt722-sdca.c#n1544
> > 
> > (the other rt drivers are similar)
> 
> This rt722-sdca driver seems wrong, IIRC we aligned all
> drivers to do most of the inits in the driver probe, and then
> the only thing left to do when the device attaches is the
> set_active/get_noresume/put_autosuspend.
> 
> See the rt711 and rt1308 for reference, they have comments to
> explain why we used this code pattern.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/codecs/rt1308-sdw.c#n709

That is some scary stuff there, that is basically working around
the fact that with those drivers the soundcard is created before
the hardware is actually ready. But its one aspect of that and
there are likely many knock on effects/races hiding in there. At
some point we should probably revisit the whole enumeration
thing in soundwire it does cause a lot of scary code.

That said... the class driver doesn't have the same problem
however, because of the two layer nature of the auxiliary driver
stuff. The soundwire driver binds to the device and completes
probe, but it is the auxiliary drivers that are used in the
soundcard and those are only probed once the device itself has
enumerated in class_boot_work(). This means the sound card is
only created after the device has enumerated, so the same problem
isn't present and we can have a more normal PM runtime startup.

Thanks,
Charles