Re: [PATCH v2] ASoC: SGTL5000: Fix kernel failed while trying to get optional VDDD supply.

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On Thu, Nov 28, 2013 at 02:46:59PM +0800, Xiubo Li wrote:

> +static int sgtl5000_external_vddd_used(struct snd_soc_codec *codec)
> +{
> +	struct regulator *consumer;
> +	struct sgtl5000_priv *sgtl5000 = snd_soc_codec_get_drvdata(codec);
> +
> +	consumer = regulator_get_optional(codec->dev,
> +			sgtl5000->supplies[VDDD].supply);
> +	if (IS_ERR(consumer))
> +		return 0;
> +
> +	regulator_put(consumer);
> +
> +	return 1;
> +}

This is broken with respect to deferred probe, deferred probe returns
an error when an external regulator is in use but not yet registered.
The driver needs to at least handle -EPROBE_DEFER specially here.

It's also not nice to get the regulator, release it and get it again
which you end up needing to do because...

> -	ret = regulator_bulk_get(codec->dev, ARRAY_SIZE(sgtl5000->supplies),
> +	if (sgtl5000_external_vddd_used(codec)) {
> +		ret = regulator_bulk_get(codec->dev,
> +				ARRAY_SIZE(sgtl5000->supplies),
>  				sgtl5000->supplies);

...I think my previous comments about this code being poorly structured
in the existing driver stand.  The bulk get should be used for the
supplies that are always needed and a separate optional get should be
used for this one.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]