All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe()
@ 2013-05-10  0:15 Fabio Estevam
  2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Fabio Estevam @ 2013-05-10  0:15 UTC (permalink / raw)
  To: broonie; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, troy.kisky

From: Fabio Estevam <fabio.estevam@freescale.com>

The usual place for reading chip ID is inside i2c_probe, so move it there and 
also convert it to regmap.

sgtl5000_enable_regulators() needs to read the chip revision, so keep the
revision check there.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v4:
- Just introduced in this version.

 sound/soc/codecs/sgtl5000.c |   39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 327b443..af0c8aa 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -1275,7 +1275,7 @@ static int sgtl5000_replace_vddd_with_ldo(struct snd_soc_codec *codec)
 
 static int sgtl5000_enable_regulators(struct snd_soc_codec *codec)
 {
-	u16 reg;
+	int reg;
 	int ret;
 	int rev;
 	int i;
@@ -1303,23 +1303,17 @@ static int sgtl5000_enable_regulators(struct snd_soc_codec *codec)
 	/* wait for all power rails bring up */
 	udelay(10);
 
-	/* read chip information */
-	reg = snd_soc_read(codec, SGTL5000_CHIP_ID);
-	if (((reg & SGTL5000_PARTID_MASK) >> SGTL5000_PARTID_SHIFT) !=
-	    SGTL5000_PARTID_PART_ID) {
-		dev_err(codec->dev,
-			"Device with ID register %x is not a sgtl5000\n", reg);
-		ret = -ENODEV;
-		goto err_regulator_disable;
-	}
-
-	rev = (reg & SGTL5000_REVID_MASK) >> SGTL5000_REVID_SHIFT;
-	dev_info(codec->dev, "sgtl5000 revision 0x%x\n", rev);
-
 	/*
 	 * workaround for revision 0x11 and later,
 	 * roll back to use internal LDO
 	 */
+
+	ret = regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, &reg);
+	if (ret)
+		goto err_regulator_disable;
+
+	rev = (reg & SGTL5000_REVID_MASK) >> SGTL5000_REVID_SHIFT;
+
 	if (external_vddd && rev >= 0x11) {
 		/* disable all regulator first */
 		regulator_bulk_disable(ARRAY_SIZE(sgtl5000->supplies),
@@ -1478,7 +1472,7 @@ static int sgtl5000_i2c_probe(struct i2c_client *client,
 			      const struct i2c_device_id *id)
 {
 	struct sgtl5000_priv *sgtl5000;
-	int ret;
+	int ret, reg, rev;
 
 	sgtl5000 = devm_kzalloc(&client->dev, sizeof(struct sgtl5000_priv),
 								GFP_KERNEL);
@@ -1492,6 +1486,21 @@ static int sgtl5000_i2c_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	/* read chip information */
+	ret = regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, &reg);
+	if (ret)
+		return ret;
+
+	if (((reg & SGTL5000_PARTID_MASK) >> SGTL5000_PARTID_SHIFT) !=
+	    SGTL5000_PARTID_PART_ID) {
+		dev_err(&client->dev,
+			"Device with ID register %x is not a sgtl5000\n", reg);
+		return -ENODEV;
+	}
+
+	rev = (reg & SGTL5000_REVID_MASK) >> SGTL5000_REVID_SHIFT;
+	dev_info(&client->dev, "sgtl5000 revision 0x%x\n", rev);
+
 	i2c_set_clientdata(client, sgtl5000);
 
 	ret = snd_soc_register_codec(&client->dev,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10  0:15 [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe() Fabio Estevam
@ 2013-05-10  0:15 ` Fabio Estevam
  2013-05-10  1:17   ` Troy Kisky
                     ` (3 more replies)
  2013-05-10  9:15 ` [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe() Mark Brown
  2013-05-10 16:37 ` Eric Nelson
  2 siblings, 4 replies; 18+ messages in thread
From: Fabio Estevam @ 2013-05-10  0:15 UTC (permalink / raw)
  To: broonie; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, troy.kisky

From: Fabio Estevam <fabio.estevam@freescale.com>

After a 'reboot' command in Linux or after pressing the system's reset button
the sgtl5000 driver fails to probe:

sgtl5000 0-000a: Device with ID register ffff is not a sgtl5000
sgtl5000 0-000a: ASoC: failed to probe CODEC -19
imx-sgtl5000 sound.12: ASoC: failed to instantiate card -19
imx-sgtl5000 sound.12: snd_soc_register_card failed (-19)

sgtl5000 codec does not have a reset line, nor a reset command in software, so
after a system reset the codec does not contain the default register values
from sgtl5000_reg_defaults[] anymore, as these are only valid after a 
power-on-reset cycle.

Fix this issue by explicitly reading all the reset register values from
sgtl5000_reg_defaults[] and writing them back into sgtl5000 to ensure a sane
state.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v4:
- Only call sgtl5000_fill_defaults if we know we have a sgtl5000
Changes since v3:
- Read sgtl5000_reg_defaults and write these values into sgtl5000
Changes since v2:
- Do not use reg_defaults_raw as it is not the correct purpose
- Manually build sgtl5000_reg_default
- Improve commitlog
Changes since v1:
- Remove sgtl5000_reg_defaults array
- Do not use num_reg_defaults_raw
 sound/soc/codecs/sgtl5000.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index af0c8aa..ec97636 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -1468,6 +1468,31 @@ static const struct regmap_config sgtl5000_regmap = {
 	.num_reg_defaults = ARRAY_SIZE(sgtl5000_reg_defaults),
 };
 
+/*
+ * Write all the default values from sgtl5000_reg_defaults[] array into the
+ * sgtl5000 registers, to make sure we always start with the sane registers
+ * values as stated in the datasheet.
+ *
+ * Since sgtl5000 does not have a reset line, nor a reset command in software,
+ * we follow this approach to guarantee we always start from the default values
+ * and avoid problems like, not being able to probe after an audio playback
+ * followed by a system reset or a 'reboot' command in Linux
+ */
+static int sgtl5000_fill_defaults(struct sgtl5000_priv *sgtl5000)
+{
+	int i, ret, val, index;
+
+	for (i = 0; i < ARRAY_SIZE(sgtl5000_reg_defaults); i++) {
+		val = sgtl5000_reg_defaults[i].def;
+		index = sgtl5000_reg_defaults[i].reg;
+		ret = regmap_write(sgtl5000->regmap, index, val);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int sgtl5000_i2c_probe(struct i2c_client *client,
 			      const struct i2c_device_id *id)
 {
@@ -1503,6 +1528,11 @@ static int sgtl5000_i2c_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, sgtl5000);
 
+	/* Ensure sgtl5000 will start with sane register values */
+	ret = sgtl5000_fill_defaults(sgtl5000);
+	if (ret)
+		return ret;
+
 	ret = snd_soc_register_codec(&client->dev,
 			&sgtl5000_driver, &sgtl5000_dai, 1);
 	return ret;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
@ 2013-05-10  1:17   ` Troy Kisky
  2013-05-10  2:12     ` Fabio Estevam
  2013-05-10  9:16   ` Mark Brown
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Troy Kisky @ 2013-05-10  1:17 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On 5/9/2013 5:15 PM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> After a 'reboot' command in Linux or after pressing the system's reset button
> the sgtl5000 driver fails to probe:
>
> sgtl5000 0-000a: Device with ID register ffff is not a sgtl5000
> sgtl5000 0-000a: ASoC: failed to probe CODEC -19
> imx-sgtl5000 sound.12: ASoC: failed to instantiate card -19
> imx-sgtl5000 sound.12: snd_soc_register_card failed (-19)
>
> sgtl5000 codec does not have a reset line, nor a reset command in software, so
> after a system reset the codec does not contain the default register values
> from sgtl5000_reg_defaults[] anymore, as these are only valid after a
> power-on-reset cycle.
>
> Fix this issue by explicitly reading all the reset register values from
> sgtl5000_reg_defaults[] and writing them back into sgtl5000 to ensure a sane
> state.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v4:
> - Only call sgtl5000_fill_defaults if we know we have a sgtl5000
> Changes since v3:
> - Read sgtl5000_reg_defaults and write these values into sgtl5000
> Changes since v2:
> - Do not use reg_defaults_raw as it is not the correct purpose
> - Manually build sgtl5000_reg_default
> - Improve commitlog
> Changes since v1:
> - Remove sgtl5000_reg_defaults array
> - Do not use num_reg_defaults_raw
>   sound/soc/codecs/sgtl5000.c |   30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
>
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
> index af0c8aa..ec97636 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -1468,6 +1468,31 @@ static const struct regmap_config sgtl5000_regmap = {
>   	.num_reg_defaults = ARRAY_SIZE(sgtl5000_reg_defaults),
>   };
>   
> +/*
> + * Write all the default values from sgtl5000_reg_defaults[] array into the
> + * sgtl5000 registers, to make sure we always start with the sane registers
> + * values as stated in the datasheet.
> + *
> + * Since sgtl5000 does not have a reset line, nor a reset command in software,
> + * we follow this approach to guarantee we always start from the default values
> + * and avoid problems like, not being able to probe after an audio playback
> + * followed by a system reset or a 'reboot' command in Linux
> + */
> +static int sgtl5000_fill_defaults(struct sgtl5000_priv *sgtl5000)
> +{
> +	int i, ret, val, index;
> +
> +	for (i = 0; i < ARRAY_SIZE(sgtl5000_reg_defaults); i++) {
> +		val = sgtl5000_reg_defaults[i].def;
> +		index = sgtl5000_reg_defaults[i].reg;
> +		ret = regmap_write(sgtl5000->regmap, index, val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static int sgtl5000_i2c_probe(struct i2c_client *client,
>   			      const struct i2c_device_id *id)
>   {
> @@ -1503,6 +1528,11 @@ static int sgtl5000_i2c_probe(struct i2c_client *client,
>   
>   	i2c_set_clientdata(client, sgtl5000);
>   
> +	/* Ensure sgtl5000 will start with sane register values */
> +	ret = sgtl5000_fill_defaults(sgtl5000);
> +	if (ret)
> +		return ret;
> +
>   	ret = snd_soc_register_codec(&client->dev,
>   			&sgtl5000_driver, &sgtl5000_dai, 1);
>   	return ret;

Did you test a reset/reboot ?  Since the fill_defaults now happens after 
the read of the device id,
how does this fix the mentioned problem ?

Troy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10  1:17   ` Troy Kisky
@ 2013-05-10  2:12     ` Fabio Estevam
       [not found]       ` <CAKGA1bnhE3dZe8P+aJm+cps+Rp4abSGOMmPckGw_0fCnh9e6+A@mail.gmail.com>
  2013-05-10 19:11       ` Troy Kisky
  0 siblings, 2 replies; 18+ messages in thread
From: Fabio Estevam @ 2013-05-10  2:12 UTC (permalink / raw)
  To: Troy Kisky; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On Thu, May 9, 2013 at 10:17 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:

> Did you test a reset/reboot ?  Since the fill_defaults now happens after the
> read of the device id,
> how does this fix the mentioned problem ?

Yes, tested several times :-)

The chip ID has to be read from the register and it always read
correctly. In patch 1/2, I  just moved the reading to a more standard
location (in i2c_probe function), just like many other codecs do.

Patch 2/2 fixes the issue by ensuring that we start from sane values
from power-on reset.

Let's look at the original error:

sgtl5000 0-000a: Device with ID register ffff is not a sgtl5000
sgtl5000 0-000a: ASoC: failed to probe CODEC -19
imx-sgtl5000 sound.12: ASoC: failed to instantiate card -19
imx-sgtl5000 sound.12: snd_soc_register_card failed (-19)

If I move the chip ID reading to i2c_probe (ie, only apply patch 1/2),
the device ID would be read correctly, but the fail would happen at a
later point:

sgtl5000 0-000a: sgtl5000 revision 0x11
sgtl5000 0-000a: Failed to get supply 'VDDD': -517
 mmcblk0: p1
0-000a: 1200 mV normal
sgtl5000 0-000a: Using internal LDO instead of VDDD
usb 2-1: new high-speed USB device number 2 using ci_hdrc
hub 2-1:1.0: USB hub found
hub 2-1:1.0: 3 ports detected
sgtl5000 0-000a: ASoC: failed to probe CODEC -110
imx-sgtl5000 sound.12: ASoC: failed to instantiate card -110
imx-sgtl5000 sound.12: snd_soc_register_card failed (-110)
imx-sgtl5000: probe of sound.12 failed with error -110

So the original issue was not about reading the chip ID correctly.

The power related registers change from POR to reset (among others)

If the chip is not properly powered, then we are not able to read its
ID and we get that original error.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe()
  2013-05-10  0:15 [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe() Fabio Estevam
  2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
@ 2013-05-10  9:15 ` Mark Brown
  2013-05-10 16:37 ` Eric Nelson
  2 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2013-05-10  9:15 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, troy.kisky


[-- Attachment #1.1: Type: text/plain, Size: 253 bytes --]

On Thu, May 09, 2013 at 09:15:46PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> The usual place for reading chip ID is inside i2c_probe, so move it there and 
> also convert it to regmap.

Applied, thanks.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
  2013-05-10  1:17   ` Troy Kisky
@ 2013-05-10  9:16   ` Mark Brown
  2013-05-10 16:38   ` Eric Nelson
  2013-05-10 20:59   ` Mark Brown
  3 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2013-05-10  9:16 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, troy.kisky


[-- Attachment #1.1: Type: text/plain, Size: 318 bytes --]

On Thu, May 09, 2013 at 09:15:47PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> After a 'reboot' command in Linux or after pressing the system's reset button
> the sgtl5000 driver fails to probe:

Looks good, can everyone confirm that this fixes the reset issues please?

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe()
  2013-05-10  0:15 [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe() Fabio Estevam
  2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
  2013-05-10  9:15 ` [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe() Mark Brown
@ 2013-05-10 16:37 ` Eric Nelson
  2 siblings, 0 replies; 18+ messages in thread
From: Eric Nelson @ 2013-05-10 16:37 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Fabio Estevam, alsa-devel, lars, matt, troy.kisky, broonie

On 05/09/2013 05:15 PM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> The usual place for reading chip ID is inside i2c_probe, so move it there and
> also convert it to regmap.
>
> sgtl5000_enable_regulators() needs to read the chip revision, so keep the
> revision check there.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v4:
> - Just introduced in this version.
>
>   sound/soc/codecs/sgtl5000.c |   39 ++++++++++++++++++++++++---------------
>   1 file changed, 24 insertions(+), 15 deletions(-)
>

Tested-by: Eric Nelson <eric.nelson@boundarydevices.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
  2013-05-10  1:17   ` Troy Kisky
  2013-05-10  9:16   ` Mark Brown
@ 2013-05-10 16:38   ` Eric Nelson
  2013-05-10 20:59   ` Mark Brown
  3 siblings, 0 replies; 18+ messages in thread
From: Eric Nelson @ 2013-05-10 16:38 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Fabio Estevam, alsa-devel, lars, matt, troy.kisky, broonie

On 05/09/2013 05:15 PM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> After a 'reboot' command in Linux or after pressing the system's reset button
> the sgtl5000 driver fails to probe:
>
> sgtl5000 0-000a: Device with ID register ffff is not a sgtl5000
> sgtl5000 0-000a: ASoC: failed to probe CODEC -19
> imx-sgtl5000 sound.12: ASoC: failed to instantiate card -19
> imx-sgtl5000 sound.12: snd_soc_register_card failed (-19)
>
> sgtl5000 codec does not have a reset line, nor a reset command in software, so
> after a system reset the codec does not contain the default register values
> from sgtl5000_reg_defaults[] anymore, as these are only valid after a
> power-on-reset cycle.
>
> Fix this issue by explicitly reading all the reset register values from
> sgtl5000_reg_defaults[] and writing them back into sgtl5000 to ensure a sane
> state.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v4:
> - Only call sgtl5000_fill_defaults if we know we have a sgtl5000
> Changes since v3:
> - Read sgtl5000_reg_defaults and write these values into sgtl5000
> Changes since v2:
> - Do not use reg_defaults_raw as it is not the correct purpose
> - Manually build sgtl5000_reg_default
> - Improve commitlog
> Changes since v1:
> - Remove sgtl5000_reg_defaults array
> - Do not use num_reg_defaults_raw
>   sound/soc/codecs/sgtl5000.c |   30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
>

Tested-by: Eric Nelson <eric.nelson@boundarydevices.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
       [not found]       ` <CAKGA1bnhE3dZe8P+aJm+cps+Rp4abSGOMmPckGw_0fCnh9e6+A@mail.gmail.com>
@ 2013-05-10 17:31         ` Fabio Estevam
  0 siblings, 0 replies; 18+ messages in thread
From: Fabio Estevam @ 2013-05-10 17:31 UTC (permalink / raw)
  To: Matt Sealey
  Cc: Fabio Estevam, alsa-devel, Lars-Peter Clausen, Eric Nelson,
	Troy Kisky, Mark Brown

Hi Matt,

On Fri, May 10, 2013 at 2:14 PM, Matt Sealey <matt@genesi-usa.com> wrote:

> Hi Fabio!
>
> Is there any way to make that failure a little less of a "failure" or
> even a warning, since the message below;
>
>> sgtl5000 0-000a: Using internal LDO instead of VDDD
>
> Pretty much covers that VDDD was not supplied. I would consider that
> you would want to say "VDDD wasn't supplied" in the sense that it is
> optional and not give in pdata or dt, and then remark that the LDO was
> actually configured.. I don't like it when my kernels boot up and spit
> out high numbered errno's for the sake of a completely optional entry
> in the design vs. missing and still optional entry in the device tree.

Yes, I plan to clean the sgtl5000 log messages. I agree that it is
currently too noisy.

Will do it after Mark applies this series.

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10  2:12     ` Fabio Estevam
       [not found]       ` <CAKGA1bnhE3dZe8P+aJm+cps+Rp4abSGOMmPckGw_0fCnh9e6+A@mail.gmail.com>
@ 2013-05-10 19:11       ` Troy Kisky
  2013-05-10 19:16         ` Troy Kisky
  2013-05-10 19:23         ` Fabio Estevam
  1 sibling, 2 replies; 18+ messages in thread
From: Troy Kisky @ 2013-05-10 19:11 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On 5/9/2013 7:12 PM, Fabio Estevam wrote:
> On Thu, May 9, 2013 at 10:17 PM, Troy Kisky
> <troy.kisky@boundarydevices.com> wrote:
>
>> Did you test a reset/reboot ?  Since the fill_defaults now happens after the
>> read of the device id,
>> how does this fix the mentioned problem ?
> Yes, tested several times :-)
>
> The chip ID has to be read from the register and it always read
> correctly. In patch 1/2, I  just moved the reading to a more standard
> location (in i2c_probe function), just like many other codecs do.
>
> Patch 2/2 fixes the issue by ensuring that we start from sane values
> from power-on reset.
>
> Let's look at the original error:
>
> sgtl5000 0-000a: Device with ID register ffff is not a sgtl5000
> sgtl5000 0-000a: ASoC: failed to probe CODEC -19
> imx-sgtl5000 sound.12: ASoC: failed to instantiate card -19
> imx-sgtl5000 sound.12: snd_soc_register_card failed (-19)
>
> If I move the chip ID reading to i2c_probe (ie, only apply patch 1/2),
> the device ID would be read correctly, but the fail would happen at a
> later point:
>
> sgtl5000 0-000a: sgtl5000 revision 0x11
> sgtl5000 0-000a: Failed to get supply 'VDDD': -517
>   mmcblk0: p1
> 0-000a: 1200 mV normal
> sgtl5000 0-000a: Using internal LDO instead of VDDD
> usb 2-1: new high-speed USB device number 2 using ci_hdrc
> hub 2-1:1.0: USB hub found
> hub 2-1:1.0: 3 ports detected
> sgtl5000 0-000a: ASoC: failed to probe CODEC -110
> imx-sgtl5000 sound.12: ASoC: failed to instantiate card -110
> imx-sgtl5000 sound.12: snd_soc_register_card failed (-110)
> imx-sgtl5000: probe of sound.12 failed with error -110

Shouldn't you update your commit log with this then ?
>
> So the original issue was not about reading the chip ID correctly.
>
> The power related registers change from POR to reset (among others)
>
> If the chip is not properly powered, then we are not able to read its
> ID and we get that original error.
>

Thanks for fixing, I guess I don't need to understand how the fix works.

Troy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10 19:11       ` Troy Kisky
@ 2013-05-10 19:16         ` Troy Kisky
  2013-05-10 19:18           ` Fabio Estevam
  2013-05-10 19:23         ` Fabio Estevam
  1 sibling, 1 reply; 18+ messages in thread
From: Troy Kisky @ 2013-05-10 19:16 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On 5/10/2013 12:11 PM, Troy Kisky wrote:
> On 5/9/2013 7:12 PM, Fabio Estevam wrote:
>> On Thu, May 9, 2013 at 10:17 PM, Troy Kisky
>> <troy.kisky@boundarydevices.com> wrote:
>>
>>> Did you test a reset/reboot ?  Since the fill_defaults now happens 
>>> after the
>>> read of the device id,
>>> how does this fix the mentioned problem ?
>> Yes, tested several times :-)

Since Sabrelite uses "dummy" regulators (always on), have you tested 
with a boards that
actually uses regulators?

I would think that some regulators would need to be on before you could 
even read
the ID register.


>>
>> The chip ID has to be read from the register and it always read
>> correctly. In patch 1/2, I  just moved the reading to a more standard
>> location (in i2c_probe function), just like many other codecs do.
>>
>> Patch 2/2 fixes the issue by ensuring that we start from sane values
>> from power-on reset.
>>
>> Let's look at the original error:
>>
>> sgtl5000 0-000a: Device with ID register ffff is not a sgtl5000
>> sgtl5000 0-000a: ASoC: failed to probe CODEC -19
>> imx-sgtl5000 sound.12: ASoC: failed to instantiate card -19
>> imx-sgtl5000 sound.12: snd_soc_register_card failed (-19)
>>
>> If I move the chip ID reading to i2c_probe (ie, only apply patch 1/2),
>> the device ID would be read correctly, but the fail would happen at a
>> later point:
>>
>> sgtl5000 0-000a: sgtl5000 revision 0x11
>> sgtl5000 0-000a: Failed to get supply 'VDDD': -517
>>   mmcblk0: p1
>> 0-000a: 1200 mV normal
>> sgtl5000 0-000a: Using internal LDO instead of VDDD
>> usb 2-1: new high-speed USB device number 2 using ci_hdrc
>> hub 2-1:1.0: USB hub found
>> hub 2-1:1.0: 3 ports detected
>> sgtl5000 0-000a: ASoC: failed to probe CODEC -110
>> imx-sgtl5000 sound.12: ASoC: failed to instantiate card -110
>> imx-sgtl5000 sound.12: snd_soc_register_card failed (-110)
>> imx-sgtl5000: probe of sound.12 failed with error -110
>
> Shouldn't you update your commit log with this then ?
>>
>> So the original issue was not about reading the chip ID correctly.
>>
>> The power related registers change from POR to reset (among others)
>>
>> If the chip is not properly powered, then we are not able to read its
>> ID and we get that original error.
>>
>
> Thanks for fixing, I guess I don't need to understand how the fix works.
>
> Troy
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10 19:16         ` Troy Kisky
@ 2013-05-10 19:18           ` Fabio Estevam
  2013-05-10 19:39             ` Fabio Estevam
  2013-05-10 20:13             ` Troy Kisky
  0 siblings, 2 replies; 18+ messages in thread
From: Fabio Estevam @ 2013-05-10 19:18 UTC (permalink / raw)
  To: Troy Kisky; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On Fri, May 10, 2013 at 4:16 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:

> Since Sabrelite uses "dummy" regulators (always on), have you tested with a
> boards that
> actually uses regulators?

I don't have any board that drives the sgtl5000 power supplies from a
PMIC, for example.

>
> I would think that some regulators would need to be on before you could even
> read
> the ID register.

Sure. That's why I placed the ID reading after turning on the regulators :-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10 19:11       ` Troy Kisky
  2013-05-10 19:16         ` Troy Kisky
@ 2013-05-10 19:23         ` Fabio Estevam
  1 sibling, 0 replies; 18+ messages in thread
From: Fabio Estevam @ 2013-05-10 19:23 UTC (permalink / raw)
  To: Troy Kisky; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On Fri, May 10, 2013 at 4:11 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:

> Shouldn't you update your commit log with this then ?

 I provided the explanations in the commit log and in the source code in v5.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10 19:18           ` Fabio Estevam
@ 2013-05-10 19:39             ` Fabio Estevam
  2013-05-10 20:56               ` Mark Brown
  2013-05-10 20:13             ` Troy Kisky
  1 sibling, 1 reply; 18+ messages in thread
From: Fabio Estevam @ 2013-05-10 19:39 UTC (permalink / raw)
  To: Troy Kisky; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On Fri, May 10, 2013 at 4:18 PM, Fabio Estevam <festevam@gmail.com> wrote:

>> I would think that some regulators would need to be on before you could even
>> read
>> the ID register.
>
> Sure. That's why I placed the ID reading after turning on the regulators :-)

Ops, actually I did not. Will fix it.

Thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10 19:18           ` Fabio Estevam
  2013-05-10 19:39             ` Fabio Estevam
@ 2013-05-10 20:13             ` Troy Kisky
  1 sibling, 0 replies; 18+ messages in thread
From: Troy Kisky @ 2013-05-10 20:13 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, broonie

On 5/10/2013 12:18 PM, Fabio Estevam wrote:
> On Fri, May 10, 2013 at 4:16 PM, Troy Kisky
> <troy.kisky@boundarydevices.com> wrote:
>
>> Since Sabrelite uses "dummy" regulators (always on), have you tested with a
>> boards that
>> actually uses regulators?
> I don't have any board that drives the sgtl5000 power supplies from a
> PMIC, for example.
>
>> I would think that some regulators would need to be on before you could even
>> read
>> the ID register.
> Sure. That's why I placed the ID reading after turning on the regulators :-)
>
Hmm. Maybe I need some remedial classes. This is what the flow looks 
like to me.

sgtl5000_i2c_probe
	regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, &reg);
         snd_soc_register_codec(&client->dev,&sgtl5000_driver, &sgtl5000_dai, 1);

	sgtl5000_probe
		sgtl5000_enable_regulators

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10 19:39             ` Fabio Estevam
@ 2013-05-10 20:56               ` Mark Brown
  2013-05-10 20:58                 ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2013-05-10 20:56 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, Troy Kisky


[-- Attachment #1.1: Type: text/plain, Size: 293 bytes --]

On Fri, May 10, 2013 at 04:39:32PM -0300, Fabio Estevam wrote:
> On Fri, May 10, 2013 at 4:18 PM, Fabio Estevam <festevam@gmail.com> wrote:

> > Sure. That's why I placed the ID reading after turning on the regulators :-)

> Ops, actually I did not. Will fix it.

I'll wait for a new version.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10 20:56               ` Mark Brown
@ 2013-05-10 20:58                 ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2013-05-10 20:58 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, Troy Kisky


[-- Attachment #1.1: Type: text/plain, Size: 271 bytes --]

On Fri, May 10, 2013 at 09:56:16PM +0100, Mark Brown wrote:
> On Fri, May 10, 2013 at 04:39:32PM -0300, Fabio Estevam wrote:

> > Ops, actually I did not. Will fix it.

> I'll wait for a new version.

Ah, actually that really only affects the already applied patch so...

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset
  2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
                     ` (2 preceding siblings ...)
  2013-05-10 16:38   ` Eric Nelson
@ 2013-05-10 20:59   ` Mark Brown
  3 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2013-05-10 20:59 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Fabio Estevam, alsa-devel, lars, matt, eric.nelson, troy.kisky


[-- Attachment #1.1: Type: text/plain, Size: 261 bytes --]

On Thu, May 09, 2013 at 09:15:47PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> After a 'reboot' command in Linux or after pressing the system's reset button
> the sgtl5000 driver fails to probe:

Applied, thanks.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2013-05-10 20:59 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-10  0:15 [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe() Fabio Estevam
2013-05-10  0:15 ` [PATCH v5 2/2] ASoC: sgtl5000: Fix driver probe after reset Fabio Estevam
2013-05-10  1:17   ` Troy Kisky
2013-05-10  2:12     ` Fabio Estevam
     [not found]       ` <CAKGA1bnhE3dZe8P+aJm+cps+Rp4abSGOMmPckGw_0fCnh9e6+A@mail.gmail.com>
2013-05-10 17:31         ` Fabio Estevam
2013-05-10 19:11       ` Troy Kisky
2013-05-10 19:16         ` Troy Kisky
2013-05-10 19:18           ` Fabio Estevam
2013-05-10 19:39             ` Fabio Estevam
2013-05-10 20:56               ` Mark Brown
2013-05-10 20:58                 ` Mark Brown
2013-05-10 20:13             ` Troy Kisky
2013-05-10 19:23         ` Fabio Estevam
2013-05-10  9:16   ` Mark Brown
2013-05-10 16:38   ` Eric Nelson
2013-05-10 20:59   ` Mark Brown
2013-05-10  9:15 ` [PATCH v5 1/2] ASoC: sgtl5000: Read SGTL5000_CHIP_ID in i2c_probe() Mark Brown
2013-05-10 16:37 ` Eric Nelson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.