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

[Mark Brown]

> 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...

> +static int pcm1681_component_probe(struct snd_soc_component *component)
> +{
> +	struct pcm1681_private *priv = snd_soc_component_get_drvdata(component);
> +	struct device *dev = component->dev;
> +	int ret;
> +
> +	priv->sck = clk_get_optional(dev, "sck");
> +	if (IS_ERR(priv->sck)) {
> +		ret = PTR_ERR(priv->sck);
> +		priv->sck = NULL;
> +		return dev_err_probe(dev, ret, "Failed to get sck\n");
> +	}

This is the oppositie of the standard advice for drivers, and we don't
deal well with probe deferral from component probes - in particular,
nothing will trigger a rebind of the card if we don't probe a new
component and that's not going to happen if the CODEC already bound and
isn't waiting for the clock.

Also dev_err_probe() will only DTRT from device model probe.

-- 
Mark Brown <broonie@xxxxxxxxxx>