Re: [PATCH v3 2/7] ASoC: codec: pcm1681: Add optional SCK clock and runtime PM support

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

On Fri, Oct 09, 2026 at 06:42:48PM +0530, Mohammad Rafi Shaik wrote:
> The PCM1681 requires its SCK system clock to be running before register
> access. On platforms where SCK is provided by a gateable clock, register
> access may fail when the clock is disabled.
>  
> Add support for an optional "sck" clock and enable it before accessing the
> device. After enabling SCK, wait for the required 65536 system clock cycles
> to allow the device to complete its internal reset sequence.
>  
> Use runtime PM to manage SCK instead of keeping it enabled for the lifetime
> of the device. Disable the clock during runtime suspend and restore it on
> runtime resume. Use autosuspend to avoid unnecessary clock toggling between
> closely spaced accesses.
>  
> Since the device register state may be lost when SCK is disabled, enable
> the regmap cache. Switch regmap to cache-only mode and mark the cache
> dirty on runtime suspend, then synchronize the cached register state after
> SCK is restored on runtime resume. Mark the zero-detect status register
> volatile since it is updated by hardware.
>  
> Drop idle_bias_on so that the component can reach SND_SOC_BIAS_OFF and
> runtime suspend can gate SCK.
> 
> Assisted-by: LLM
> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

[...]

> @@ -304,34 +335,128 @@ MODULE_DEVICE_TABLE(i2c, pcm1681_i2c_id);
>  
>  static int pcm1681_i2c_probe(struct i2c_client *client)
>  {
> +	struct device *dev = &client->dev;
>  	int ret;
>  	struct pcm1681_private *priv;
>  
> -	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	priv->sck = devm_clk_get_optional(dev, "sck");
> +	if (IS_ERR(priv->sck))
> +		return dev_err_probe(dev, PTR_ERR(priv->sck),
> +				     "Failed to get sck\n");

Holding sck from i2c probe for the lifetime of the device breaks audio
after an ADSP restart on Nord.

There sck is LPASS_CLK_ID_MCLK_1 from q6prmcc, a clock controller that
lives under the ADSP GPR service. Stopping the ADSP removes q6prm and
unregisters its clocks; starting it again registers new ones. pcm1681
is not unbound in between: fw_devlink can't link it to q6prmcc, since
that node has no device until the DSP is up, so the supplier link goes
to the remoteproc device, which stays bound across the restart.

pcm1681 is left with a handle to the orphaned clk, and the first runtime
resume after the restart fails:

  pcm1681 0-004c: Failed to enable sck: -6

Reproduce with:

  echo stop > /sys/class/remoteproc/remoteproc1/state
  echo start > /sys/class/remoteproc/remoteproc1/state
  speaker-test -D plughw:0,0 -t sine -l 1

Even with a fresh handle the DAC stays silent, because the
assigned-clock-rates (12.288 MHz) are applied by the i2c core only once,
before probe, and the re-registered provider comes back at its default
19.2 MHz.

The sound card, on the other hand, is torn down and re-created on an
ADSP restart. So what works for me is to move the clock handling into
the component .probe/.remove callbacks: clk_get_optional(), then
of_clk_set_defaults() to re-apply the assigned rate, enable, settle,
regcache_sync() and enable runtime PM in .probe; and the reverse plus
clk_put() in .remove. The i2c probe then only creates the regmap, in
cache-only mode until sck is running, and registers the component.

Shawn