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

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.