* [PATCH] ASoC: sgtl5000: add delay before first I2C access @ 2015-01-30 20:52 Eric Nelson 2015-01-30 21:07 ` [PATCH V2] " Eric Nelson 0 siblings, 1 reply; 8+ messages in thread From: Eric Nelson @ 2015-01-30 20:52 UTC (permalink / raw) To: alsa-devel, tiwai, perex; +Cc: fabio.estevam, broonie, Eric Nelson To quote from section 1.3.1 of the data sheet: The SGTL5000 has an internal reset that is deasserted 8 SYS_MCLK cycles after all power rails have been brought up. After this time, communication can start ... 1.0μs represents 8 SYS_MCLK cycles at the minimum 8.0 MHz SYS_MCLK. Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> --- sound/soc/codecs/sgtl5000.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index 29cf7ce..7d65a5b 100644 --- a/sound/soc/codecs/sgtl5000.c +++ b/sound/soc/codecs/sgtl5000.c @@ -1462,6 +1462,9 @@ static int sgtl5000_i2c_probe(struct i2c_client *client, if (ret) return ret; + /* Need 8 clocks before I2C accesses */ + usleep_range(1, 2); + /* read chip information */ ret = regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, ®); if (ret) -- 1.9.1 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH V2] ASoC: sgtl5000: add delay before first I2C access 2015-01-30 20:52 [PATCH] ASoC: sgtl5000: add delay before first I2C access Eric Nelson @ 2015-01-30 21:07 ` Eric Nelson 2015-01-30 21:31 ` Fabio Estevam 2015-02-03 13:09 ` Mark Brown 0 siblings, 2 replies; 8+ messages in thread From: Eric Nelson @ 2015-01-30 21:07 UTC (permalink / raw) To: alsa-devel, tiwai, perex; +Cc: fabio.estevam, broonie, Eric Nelson To quote from section 1.3.1 of the data sheet: The SGTL5000 has an internal reset that is deasserted 8 SYS_MCLK cycles after all power rails have been brought up. After this time, communication can start ... 1.0uS represents 8 SYS_MCLK cycles at the minimum 8.0 MHz SYS_MCLK. Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> --- V2 replaces usleep_range with udelay and fixes microseconds in comment. sound/soc/codecs/sgtl5000.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index 29cf7ce..7d65a5b 100644 --- a/sound/soc/codecs/sgtl5000.c +++ b/sound/soc/codecs/sgtl5000.c @@ -1462,6 +1462,9 @@ static int sgtl5000_i2c_probe(struct i2c_client *client, if (ret) return ret; + /* Need 8 clocks before I2C accesses */ + udelay(1); + /* read chip information */ ret = regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, ®); if (ret) -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] ASoC: sgtl5000: add delay before first I2C access 2015-01-30 21:07 ` [PATCH V2] " Eric Nelson @ 2015-01-30 21:31 ` Fabio Estevam 2015-01-30 21:50 ` Nikolay Dimitrov 2015-02-03 13:09 ` Mark Brown 1 sibling, 1 reply; 8+ messages in thread From: Fabio Estevam @ 2015-01-30 21:31 UTC (permalink / raw) To: Eric Nelson Cc: Takashi Iwai, Fabio Estevam, alsa-devel@alsa-project.org, Mark Brown Hi Eric, On Fri, Jan 30, 2015 at 7:07 PM, Eric Nelson <eric.nelson@boundarydevices.com> wrote: > To quote from section 1.3.1 of the data sheet: > The SGTL5000 has an internal reset that is deasserted > 8 SYS_MCLK cycles after all power rails have been brought > up. After this time, communication can start > > ... > 1.0uS represents 8 SYS_MCLK cycles at the minimum 8.0 MHz SYS_MCLK. Small detail: Should be us instead of uS. Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] ASoC: sgtl5000: add delay before first I2C access 2015-01-30 21:31 ` Fabio Estevam @ 2015-01-30 21:50 ` Nikolay Dimitrov 2015-01-30 22:02 ` Eric Nelson 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Dimitrov @ 2015-01-30 21:50 UTC (permalink / raw) To: Eric Nelson Cc: Takashi Iwai, Fabio Estevam, alsa-devel@alsa-project.org, Mark Brown, Fabio Estevam Hi Eric, On 01/30/2015 11:31 PM, Fabio Estevam wrote: > Hi Eric, > > On Fri, Jan 30, 2015 at 7:07 PM, Eric Nelson > <eric.nelson@boundarydevices.com> wrote: >> To quote from section 1.3.1 of the data sheet: >> The SGTL5000 has an internal reset that is deasserted >> 8 SYS_MCLK cycles after all power rails have been brought >> up. After this time, communication can start >> >> ... >> 1.0uS represents 8 SYS_MCLK cycles at the minimum 8.0 MHz SYS_MCLK. > > Small detail: Should be us instead of uS. FYI - If you're observing issues with communicating with SGTL5000 I2C, please make sure also that the chip has a valid clock signal on SYS_MCLK, otherwise it won't respond on I2C transactions (I2C will work with any SYS_MCLK in the range 8-27MHz). Kind regards, Nikolay ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] ASoC: sgtl5000: add delay before first I2C access 2015-01-30 21:50 ` Nikolay Dimitrov @ 2015-01-30 22:02 ` Eric Nelson 2015-01-30 22:24 ` Nikolay Dimitrov 0 siblings, 1 reply; 8+ messages in thread From: Eric Nelson @ 2015-01-30 22:02 UTC (permalink / raw) To: Nikolay Dimitrov Cc: Takashi Iwai, Fabio Estevam, alsa-devel@alsa-project.org, Mark Brown, Fabio Estevam Hi Nikolay, On 01/30/2015 02:50 PM, Nikolay Dimitrov wrote: > Hi Eric, > > On 01/30/2015 11:31 PM, Fabio Estevam wrote: >> Hi Eric, >> >> On Fri, Jan 30, 2015 at 7:07 PM, Eric Nelson >> <eric.nelson@boundarydevices.com> wrote: >>> To quote from section 1.3.1 of the data sheet: >>> The SGTL5000 has an internal reset that is deasserted >>> 8 SYS_MCLK cycles after all power rails have been brought >>> up. After this time, communication can start >>> >>> ... >>> 1.0uS represents 8 SYS_MCLK cycles at the minimum 8.0 MHz >>> SYS_MCLK. >> >> Small detail: Should be us instead of uS. > > FYI - If you're observing issues with communicating with SGTL5000 I2C, > please make sure also that the chip has a valid clock signal on > SYS_MCLK, otherwise it won't respond on I2C transactions (I2C will work > with any SYS_MCLK in the range 8-27MHz). > Thanks for that, but the issue we're seeing and that made me spot this was a very intermittent problem (1 in many 1000's of boots) reported by a customer on our 3.10.17 code base. We haven't been able to repeat the issue, but the failure is in the initial read of the CHIP_ID register (i.e. the first I2C access) and the docs clearly state that a 1us delay is needed. I haven't found the commit that removed it, but earlier versions of sgtl5000.c had a udelay(10) before the first I2C access. Regards, Eric ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] ASoC: sgtl5000: add delay before first I2C access 2015-01-30 22:02 ` Eric Nelson @ 2015-01-30 22:24 ` Nikolay Dimitrov 2015-01-30 22:34 ` Eric Nelson 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Dimitrov @ 2015-01-30 22:24 UTC (permalink / raw) To: Eric Nelson Cc: Takashi Iwai, Fabio Estevam, alsa-devel@alsa-project.org, Mark Brown, Fabio Estevam Hi Eric, On 01/31/2015 12:02 AM, Eric Nelson wrote: > Hi Nikolay, > > On 01/30/2015 02:50 PM, Nikolay Dimitrov wrote: >> Hi Eric, >> >> On 01/30/2015 11:31 PM, Fabio Estevam wrote: >>> Hi Eric, >>> >>> On Fri, Jan 30, 2015 at 7:07 PM, Eric Nelson >>> <eric.nelson@boundarydevices.com> wrote: >>>> To quote from section 1.3.1 of the data sheet: >>>> The SGTL5000 has an internal reset that is deasserted >>>> 8 SYS_MCLK cycles after all power rails have been brought >>>> up. After this time, communication can start >>>> >>>> ... >>>> 1.0uS represents 8 SYS_MCLK cycles at the minimum 8.0 MHz >>>> SYS_MCLK. >>> >>> Small detail: Should be us instead of uS. >> >> FYI - If you're observing issues with communicating with SGTL5000 I2C, >> please make sure also that the chip has a valid clock signal on >> SYS_MCLK, otherwise it won't respond on I2C transactions (I2C will work >> with any SYS_MCLK in the range 8-27MHz). >> > > Thanks for that, but the issue we're seeing and that made me spot this > was a very intermittent problem (1 in many 1000's of boots) reported by > a customer on our 3.10.17 code base. > > We haven't been able to repeat the issue, but the failure is in the > initial read of the CHIP_ID register (i.e. the first I2C access) and > the docs clearly state that a 1us delay is needed. > > I haven't found the commit that removed it, but earlier versions > of sgtl5000.c had a udelay(10) before the first I2C access. I see. Had similar issue with 3.10.17 (fsl), when sometimes on hot-reset the (imx6) I2C controller was somehow locked-up on boot and prevented the driver from reading CHIP_ID, and thus no sound at all. But on 3.17x/3.18x mainline kernels + riotboard, this initial SGTL5000 configuration is much much better, haven't seen cases when it doesn't detect the codec. If you can reproduce the defect, you can try to exercise the I2C channel and see whether it still works at all. I'm mentioning this because it's a different root-cause that has quite similar observable effect. Otherwise I truly agree that reset timings as per datasheet are very good to be followed. Kind regards, Nikolay ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] ASoC: sgtl5000: add delay before first I2C access 2015-01-30 22:24 ` Nikolay Dimitrov @ 2015-01-30 22:34 ` Eric Nelson 0 siblings, 0 replies; 8+ messages in thread From: Eric Nelson @ 2015-01-30 22:34 UTC (permalink / raw) To: Nikolay Dimitrov Cc: Takashi Iwai, Fabio Estevam, alsa-devel@alsa-project.org, Mark Brown, Fabio Estevam Thanks Nikolay, On 01/30/2015 03:24 PM, Nikolay Dimitrov wrote: > Hi Eric, > > On 01/31/2015 12:02 AM, Eric Nelson wrote: >> Hi Nikolay, >> >> On 01/30/2015 02:50 PM, Nikolay Dimitrov wrote: >>> Hi Eric, >>> >>> On 01/30/2015 11:31 PM, Fabio Estevam wrote: >>>> Hi Eric, >>>> >>>> On Fri, Jan 30, 2015 at 7:07 PM, Eric Nelson >>>> <eric.nelson@boundarydevices.com> wrote: >>>>> To quote from section 1.3.1 of the data sheet: >>>>> The SGTL5000 has an internal reset that is deasserted >>>>> 8 SYS_MCLK cycles after all power rails have been brought >>>>> up. After this time, communication can start >>>>> >>>>> ... >>>>> 1.0uS represents 8 SYS_MCLK cycles at the minimum 8.0 MHz >>>>> SYS_MCLK. >>>> >>>> Small detail: Should be us instead of uS. >>> >>> FYI - If you're observing issues with communicating with SGTL5000 I2C, >>> please make sure also that the chip has a valid clock signal on >>> SYS_MCLK, otherwise it won't respond on I2C transactions (I2C will work >>> with any SYS_MCLK in the range 8-27MHz). >>> >> >> Thanks for that, but the issue we're seeing and that made me spot this >> was a very intermittent problem (1 in many 1000's of boots) reported by >> a customer on our 3.10.17 code base. >> >> We haven't been able to repeat the issue, but the failure is in the >> initial read of the CHIP_ID register (i.e. the first I2C access) and >> the docs clearly state that a 1us delay is needed. >> >> I haven't found the commit that removed it, but earlier versions >> of sgtl5000.c had a udelay(10) before the first I2C access. > > I see. Had similar issue with 3.10.17 (fsl), when sometimes on > hot-reset the (imx6) I2C controller was somehow locked-up on boot and > prevented the driver from reading CHIP_ID, and thus no sound at all. > But on 3.17x/3.18x mainline kernels + riotboard, this initial SGTL5000 > configuration is much much better, haven't seen cases when it doesn't > detect the codec. > > If you can reproduce the defect, you can try to exercise the I2C > channel and see whether it still works at all. > Have you been reading my e-mails with the customer ;) ? We're doing a bunch of testing, trying to force I2C accesses to it and the other I2C device on that channel of Nitrogen6X (the ISL1208 RTC). > I'm mentioning this because it's a different root-cause that has quite > similar observable effect. Otherwise I truly agree that reset timings > as per datasheet are very good to be followed. > Yep. and so the patch... Regards, Eric ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] ASoC: sgtl5000: add delay before first I2C access 2015-01-30 21:07 ` [PATCH V2] " Eric Nelson 2015-01-30 21:31 ` Fabio Estevam @ 2015-02-03 13:09 ` Mark Brown 1 sibling, 0 replies; 8+ messages in thread From: Mark Brown @ 2015-02-03 13:09 UTC (permalink / raw) To: Eric Nelson; +Cc: tiwai, fabio.estevam, alsa-devel [-- Attachment #1.1: Type: text/plain, Size: 458 bytes --] On Fri, Jan 30, 2015 at 02:07:55PM -0700, Eric Nelson wrote: > To quote from section 1.3.1 of the data sheet: > The SGTL5000 has an internal reset that is deasserted > 8 SYS_MCLK cycles after all power rails have been brought > up. After this time, communication can start Applied, though given the normal speeds of I2C I'm always a bit suspicous that something like this that's messing around with microsecond delays is actually masking something else. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-02-03 13:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-30 20:52 [PATCH] ASoC: sgtl5000: add delay before first I2C access Eric Nelson 2015-01-30 21:07 ` [PATCH V2] " Eric Nelson 2015-01-30 21:31 ` Fabio Estevam 2015-01-30 21:50 ` Nikolay Dimitrov 2015-01-30 22:02 ` Eric Nelson 2015-01-30 22:24 ` Nikolay Dimitrov 2015-01-30 22:34 ` Eric Nelson 2015-02-03 13:09 ` Mark Brown
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.