Re: [PATCH] ASoC: codec: pcm1681: Acquire sck when bound to a card

[Shawn Guo <shengchao.guo@oss.qualcomm.com>]

On Mon, Nov 09, 2026 at 04:24:37PM +0800, Shawn Guo wrote:
> + Srini
> 
> On Thu, Nov 05, 2026 at 05:42:03PM +0000, Mark Brown wrote:
> > > The sck clock is taken once at i2c probe and held for the lifetime of
> > > the device. On Qualcomm boards it is an LPASS clock exported by the
> > > q6prm clock controller, which lives on the DSP and is unregistered and
> > > registered anew whenever the ADSP is stopped and restarted. fw_devlink
> > > cannot tie pcm1681 to that provider, since the provider node only gets
> > > a device once the DSP is up, so it falls back to the remoteproc device,
> > > which stays bound across a DSP restart. pcm1681 is therefore never
> > > unbound and keeps a clk handle to the dead provider.
> > 
> > This seems like something that ought to be dealt with on the Qualcomm
> > or fw_devlink side TBH, registering the clocks and returning -EBUSY on
> > attempts to use them would be less disruptive to other components in the
> > system.  I don't understand what fw_devlink would be doing with
> > reparenting clocks...
> 
> Thanks for the review, Mark! I agree this belongs in the Qualcomm clock
> provider, not in pcm1681, so I'll drop this patch.
> 
> Sorry, my wording about fw_devlink was misleading. No clock gets
> reparented. My point was only that pcm1681 is never unbound when the DSP
> restarts. The q6prm clock-controller device only exists while the DSP is
> up. So fw_devlink links pcm1681 to the remoteproc device instead, which
> stays bound across the restart.
> 
> Today the clocks can't stay registered across a DSP restart. When the DSP
> stops, the GPR bus unregisters the q6prm device and its clock-controller
> child, and that unregisters the clocks. The clk core then switches them
> to clk_nodrv_ops, which return -ENXIO. That's where the "Failed to enable
> sck: -6" in my report comes from. The consumer's struct clk handle can't
> be reconnected after that.
> 
> So I'd like to change q6prm along the lines you suggested:
> 
> - Keep the clocks registered, on a device that is not removed when
>   the DSP stops.
> - While the DSP is down, make prepare/enable return -EBUSY.
> - When the DSP comes back, re-send the requests for clocks that are still
>   enabled, at their last set rate. This also fixes the rate being reset
>   to the 19.2 MHz default.
> 
> The part I'm unsure about is which device should own the clocks:
> 
> a) A platform device that q6prm creates on its first probe and keeps
>    until the module is unloaded. This needs no DT change.
> 
> b) The ADSP remoteproc device, which already survives restarts.
>    It's cleaner, but probably needs a binding change, since the clock
>    controller node is a child of qcom,q6prm today.
> 
> Srini, Mark, do you prefer a), b), or something else?

I'm happy to report that we can go for b) without DT binding changes.

Shawn