[PATCH] tps65910: Work around silicon erratum SWCZ010

[PATCH] tps65910: Work around silicon erratum SWCZ010

[Arnout Vandecappelle (Essensium/Mind)]

>From http://www.ti.com/lit/pdf/SWCZ010 :

Glitch on SDA-SCL not managed correctly by the I2C IP

Impact:
The standard specifies that the I2C transfer should restart on a start
event in all cases. The current design does not support two consecutive
Start conditions. This can cause the first real access after such a
glitch to be corrupted.

Description:
An unexpected glitch on SDA and SCL can generate a wrong start event.
In the current design, the SCL line must toggle two times to detect a
new start event and completely restart the I2C access; hence the real
start event is not detected in the case of a single SCL toggle.

Workaround:
Repeat I2C access.

The first access to the tps65910 that we do is when loading the regmap
in the probe function. If there has been a glitch during boot and the
erratum is triggered, then the regmap loading will fail with -REMOTEIO
since the I2C transfer is not ACKed by the tps65910.

A simple retry will work around it, because a subsequent I2C start
condition is detected properly by the tps65910.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
This patch is based on v3.17-rc1.
Build-tested with omap2plus_defconfig.
Runtime tested based on v3.4.97 with a custom config.
---
 drivers/mfd/tps65910.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index f243e75..cc1ca33 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -487,8 +487,18 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
 	tps65910->id = chip_id;
 
 	tps65910->regmap = devm_regmap_init_i2c(i2c, &tps65910_regmap_config);
-	if (IS_ERR(tps65910->regmap)) {
+	if (IS_ERR(tps65910->regmap))
 		ret = PTR_ERR(tps65910->regmap);
+	if (ret == -EREMOTEIO) {
+		dev_warn(&i2c->dev, "regmap initialization failed, possibly due to erratum SWCZ010; trying again\n");
+		tps65910->regmap = regmap_init_i2c(i2c,
+			&tps65910_regmap_config);
+		if (IS_ERR(tps65910->regmap))
+			ret = PTR_ERR(tps65910->regmap);
+		else
+			ret = 0;
+	}
+	if (ret) {
 		dev_err(&i2c->dev, "regmap initialization failed: %d\n", ret);
 		return ret;
 	}
-- 
2.1.0

Re: [PATCH] tps65910: Work around silicon erratum SWCZ010

[Laxman Dewangan]

On Friday 22 August 2014 05:44 PM, Arnout Vandecappelle (Essensium/Mind) 
wrote:
>  From http://www.ti.com/lit/pdf/SWCZ010 :
>
> Glitch on SDA-SCL not managed correctly by the I2C IP
>
> Impact:
> The standard specifies that the I2C transfer should restart on a start
> event in all cases. The current design does not support two consecutive
> Start conditions. This can cause the first real access after such a
> glitch to be corrupted.
>
> Description:
> An unexpected glitch on SDA and SCL can generate a wrong start event.
> In the current design, the SCL line must toggle two times to detect a
> new start event and completely restart the I2C access; hence the real
> start event is not detected in the case of a single SCL toggle.
>
> Workaround:
> Repeat I2C access.
>
> The first access to the tps65910 that we do is when loading the regmap
> in the probe function. If there has been a glitch during boot and the
> erratum is triggered, then the regmap loading will fail with -REMOTEIO
> since the I2C transfer is not ACKed by the tps65910.
>
> A simple retry will work around it, because a subsequent I2C start
> condition is detected properly by the tps65910.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> This patch is based on v3.17-rc1.
> Build-tested with omap2plus_defconfig.
> Runtime tested based on v3.4.97 with a custom config.
> ---
>   drivers/mfd/tps65910.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
> index f243e75..cc1ca33 100644
> --- a/drivers/mfd/tps65910.c
> +++ b/drivers/mfd/tps65910.c
> @@ -487,8 +487,18 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
>   	tps65910->id = chip_id;
>   

I think one dummy read of any register unconditionally before regmap 
init will resolve this issue.
Similar issue we face with Palmas TPS65913 and TI recommended to use 
dummy read before any valid transfer.

Re: [PATCH] tps65910: Work around silicon erratum SWCZ010

[Arnout Vandecappelle]

On 08/22/14 15:46, Laxman Dewangan wrote:
> On Friday 22 August 2014 05:44 PM, Arnout Vandecappelle (Essensium/Mind) wrote:
> >  From http://www.ti.com/lit/pdf/SWCZ010 :
> >
> > Glitch on SDA-SCL not managed correctly by the I2C IP
> >
> > Impact:
> > The standard specifies that the I2C transfer should restart on a start
> > event in all cases. The current design does not support two consecutive
> > Start conditions. This can cause the first real access after such a
> > glitch to be corrupted.
> >
> > Description:
> > An unexpected glitch on SDA and SCL can generate a wrong start event.
> > In the current design, the SCL line must toggle two times to detect a
> > new start event and completely restart the I2C access; hence the real
> > start event is not detected in the case of a single SCL toggle.
> >
> > Workaround:
> > Repeat I2C access.
> >
> > The first access to the tps65910 that we do is when loading the regmap
> > in the probe function. If there has been a glitch during boot and the
> > erratum is triggered, then the regmap loading will fail with -REMOTEIO
> > since the I2C transfer is not ACKed by the tps65910.
> >
> > A simple retry will work around it, because a subsequent I2C start
> > condition is detected properly by the tps65910.
> >
> > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > ---
> > This patch is based on v3.17-rc1.
> > Build-tested with omap2plus_defconfig.
> > Runtime tested based on v3.4.97 with a custom config.
> > ---
> >   drivers/mfd/tps65910.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
> > index f243e75..cc1ca33 100644
> > --- a/drivers/mfd/tps65910.c
> > +++ b/drivers/mfd/tps65910.c
> > @@ -487,8 +487,18 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
> >       tps65910->id = chip_id;
> >   
>
> I think one dummy read of any register unconditionally before regmap init will
> resolve this issue.
> Similar issue we face with Palmas TPS65913 and TI recommended to use dummy read
> before any valid transfer.
>

 Yeah, good idea.

 So something like

i2c_master_send(i2c, "", 1);

just before the regmap_init.

 Is abusing the empty string like that OK or should I declare a dummy buffer?

 Regards,
 Arnout


-- 
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Arnout Vandecappelle (Essensium/Mind)]

>From http://www.ti.com/lit/pdf/SWCZ010 :

Glitch on SDA-SCL not managed correctly by the I2C IP

Impact:
The standard specifies that the I2C transfer should restart on a start
event in all cases. The current design does not support two consecutive
Start conditions. This can cause the first real access after such a
glitch to be corrupted.

Description:
An unexpected glitch on SDA and SCL can generate a wrong start event.
In the current design, the SCL line must toggle two times to detect a
new start event and completely restart the I2C access; hence the real
start event is not detected in the case of a single SCL toggle.

Workaround:
Repeat I2C access.

A simpler workaround is to make a dummy transfer just before the first
access to the tps65910 chip. This can be done unconditionally.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
This patch is based on v3.17-rc1.
Build-tested with omap2plus_defconfig.
Runtime tested based on v3.4.97 with a custom config.

v2: use dummy transfer instead of retrying (Laxman Dewangan)
---
 drivers/mfd/tps65910.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index f243e75..7612d89 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -486,6 +486,11 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
 	tps65910->i2c_client = i2c;
 	tps65910->id = chip_id;
 
+	/* Work around silicon erratum SWCZ010: the tps65910 may miss the
+	 * first I2C transfer. So issue a dummy transfer before the first
+	 * real transfer.
+	 */
+	i2c_master_send(i2c, "", 1);
 	tps65910->regmap = devm_regmap_init_i2c(i2c, &tps65910_regmap_config);
 	if (IS_ERR(tps65910->regmap)) {
 		ret = PTR_ERR(tps65910->regmap);
-- 
2.1.0

Re: [PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Mark Brown]

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

On Fri, Aug 22, 2014 at 05:30:56PM +0200, Arnout Vandecappelle (Essensium/Mind) wrote:
> From http://www.ti.com/lit/pdf/SWCZ010 :
> 
> Glitch on SDA-SCL not managed correctly by the I2C IP

Applied, thanks.

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

Re: [PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Lee Jones]

On Fri, 22 Aug 2014, Mark Brown wrote:

> On Fri, Aug 22, 2014 at 05:30:56PM +0200, Arnout Vandecappelle (Essensium/Mind) wrote:
> > From http://www.ti.com/lit/pdf/SWCZ010 :
> > 
> > Glitch on SDA-SCL not managed correctly by the I2C IP

 drivers/mfd/tps65910.c | 5 +++++
 1 file changed, 5 insertions(+)

> Applied, thanks.

Eh?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Mark Brown]

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

On Tue, Aug 26, 2014 at 10:25:21AM +0100, Lee Jones wrote:
> On Fri, 22 Aug 2014, Mark Brown wrote:
> 
> > On Fri, Aug 22, 2014 at 05:30:56PM +0200, Arnout Vandecappelle (Essensium/Mind) wrote:
> > > From http://www.ti.com/lit/pdf/SWCZ010 :
> > > 
> > > Glitch on SDA-SCL not managed correctly by the I2C IP

>  drivers/mfd/tps65910.c | 5 +++++
>  1 file changed, 5 insertions(+)

> > Applied, thanks.

> Eh?

So, this is an example of why it's important to both use subject lines
matching the pattern for the subsystem and try to select CC lists
sensibly.  I've dropped the patch.

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

Re: [PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Arnout Vandecappelle]

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

On 08/26/14 11:46, Mark Brown wrote:
> On Tue, Aug 26, 2014 at 10:25:21AM +0100, Lee Jones wrote:
>> On Fri, 22 Aug 2014, Mark Brown wrote:
>>
>>> On Fri, Aug 22, 2014 at 05:30:56PM +0200, Arnout Vandecappelle (Essensium/Mind) wrote:
>>>> From http://www.ti.com/lit/pdf/SWCZ010 :
>>>>
>>>> Glitch on SDA-SCL not managed correctly by the I2C IP
> 
>>  drivers/mfd/tps65910.c | 5 +++++
>>  1 file changed, 5 insertions(+)
> 
>>> Applied, thanks.
> 
>> Eh?
> 
> So, this is an example of why it's important to both use subject lines
> matching the pattern for the subsystem and try to select CC lists
> sensibly.  I've dropped the patch.

 So, what is the proper subsystem in this case? mfd?

 Should I repost with the subsystem in the subject?

 BTW, while I'm asking questions: is this something that should be sent to stable?

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Laxman Dewangan]

Because this patch is dropped from Mark's tree, here is opportunity to 
revisit patch.

On Friday 22 August 2014 09:00 PM, Arnout Vandecappelle (Essensium/Mind) 
wrote:
>  From http://www.ti.com/lit/pdf/SWCZ010 :
>
>
> Workaround:
> Repeat I2C access.

> A simpler workaround is to make a dummy transfer just before the first
> access to the tps65910 chip. This can be done unconditionally.
> >id = chip_id;
>   
> +	/* Work around silicon erratum SWCZ010: the tps65910 may miss the
> +	 * first I2C transfer. So issue a dummy transfer before the first
> +	 * real transfer.
> +	 */
> +	i2c_master_send(i2c, "", 1);

I think dummy read is more safe operation than dummy write.
Dummy write can create the write on any register which can damage the 
critical settings or it may be possible that it will be incomplete 
calls. Datasheet has not been explained this clearly.


>   	

Re: [PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Arnout Vandecappelle]

On 08/26/14 12:14, Laxman Dewangan wrote:
> Because this patch is dropped from Mark's tree, here is opportunity to revisit
> patch.
> 
> On Friday 22 August 2014 09:00 PM, Arnout Vandecappelle (Essensium/Mind) wrote:
>>  From http://www.ti.com/lit/pdf/SWCZ010 :
>>
>>
>> Workaround:
>> Repeat I2C access.
> 
>> A simpler workaround is to make a dummy transfer just before the first
>> access to the tps65910 chip. This can be done unconditionally.
>> >id = chip_id;
>>   +    /* Work around silicon erratum SWCZ010: the tps65910 may miss the
>> +     * first I2C transfer. So issue a dummy transfer before the first
>> +     * real transfer.
>> +     */
>> +    i2c_master_send(i2c, "", 1);
> 
> I think dummy read is more safe operation than dummy write.
> Dummy write can create the write on any register which can damage the critical
> settings or it may be possible that it will be incomplete calls. Datasheet has
> not been explained this clearly.

 We're just sending the register address 0 of the SMBus transfer, without an
actual read/write request. If we do an i2c_master_recv, it's not a valid SMBus
transfer so we're equally unsure about how the chip will react to that.

 But if you like, I can check how the chip reacts to it - most likely it'll just
ignore it, possibly it won't even ack it.


 Regards,
 Arnout
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

Re: [PATCH v2] tps65910: Work around silicon erratum SWCZ010

[Lee Jones]

On Fri, 22 Aug 2014, Arnout Vandecappelle (Essensium/Mind) wrote:

> From http://www.ti.com/lit/pdf/SWCZ010 :
> 
> Glitch on SDA-SCL not managed correctly by the I2C IP
> 
> Impact:
> The standard specifies that the I2C transfer should restart on a start
> event in all cases. The current design does not support two consecutive
> Start conditions. This can cause the first real access after such a
> glitch to be corrupted.
> 
> Description:
> An unexpected glitch on SDA and SCL can generate a wrong start event.
> In the current design, the SCL line must toggle two times to detect a
> new start event and completely restart the I2C access; hence the real
> start event is not detected in the case of a single SCL toggle.
> 
> Workaround:
> Repeat I2C access.
> 
> A simpler workaround is to make a dummy transfer just before the first
> access to the tps65910 chip. This can be done unconditionally.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> This patch is based on v3.17-rc1.
> Build-tested with omap2plus_defconfig.
> Runtime tested based on v3.4.97 with a custom config.
> 
> v2: use dummy transfer instead of retrying (Laxman Dewangan)
> ---
>  drivers/mfd/tps65910.c | 5 +++++
>  1 file changed, 5 insertions(+)

Applied with Mark's Ack.

> diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
> index f243e75..7612d89 100644
> --- a/drivers/mfd/tps65910.c
> +++ b/drivers/mfd/tps65910.c
> @@ -486,6 +486,11 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
>  	tps65910->i2c_client = i2c;
>  	tps65910->id = chip_id;
>  
> +	/* Work around silicon erratum SWCZ010: the tps65910 may miss the
> +	 * first I2C transfer. So issue a dummy transfer before the first
> +	 * real transfer.
> +	 */
> +	i2c_master_send(i2c, "", 1);
>  	tps65910->regmap = devm_regmap_init_i2c(i2c, &tps65910_regmap_config);
>  	if (IS_ERR(tps65910->regmap)) {
>  		ret = PTR_ERR(tps65910->regmap);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog