[PATCH v2 0/2] Sluggish AT91 I2C driver causes SMBus timeouts

[PATCH v2 0/2] Sluggish AT91 I2C driver causes SMBus timeouts

[Peter Rosin]

Hi!

[I was waiting for a comment from Rob for the initial submission,
 but that never came and I nearly forgot. Instead of pinging again,
 I'm resubmitting with the review comment from Guenter fixed, hoping
 that Rob will react this time.]


This is a workaround for a problem in the AT91 I2C adapter driver
(or perhaps the hardware?) when it drives the TWI peripheral on an
Atmel sama5d3 chip as I2C.

Apparently, that driver can delay in excess of 100 ms just after
the transfer of the 7th bit of the last byte. When it does this
the I2C bus, when viewed from SMBUS client devices, appears
stuck with SCL low. Some SMBUS devices times out under these
conditions, in particular temperature sensors. The I2C adapter
driver does however not notice the timeout, and thinks the transfer
completed successfully when it finally desides to finish the
transaction. When this happens, the 8th bit of the last byte is
always set, and thus quite possibly corrupted.

The chip this was observed with (an nxp SE97) has a means to disable
the SMBUS timeout detector, which "fixes" things. Do that.

This should probably go to stable?

Previous discussion: https://lkml.org/lkml/2017/10/12/227

Changes since v1:    https://lkml.org/lkml/2017/10/13/184
- Added #include of bitops.h
- Rebased to v4.15-rc1

Cheers,
Peter

Peter Rosin (2):
  hwmon: (jc42) optionally try to disable the SMBUS timeout
  ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

 Documentation/devicetree/bindings/hwmon/jc42.txt |  4 ++++
 arch/arm/boot/dts/at91-tse850-3.dts              |  1 +
 drivers/hwmon/jc42.c                             | 21 +++++++++++++++++++++
 3 files changed, 26 insertions(+)

-- 
2.11.0

[PATCH v2 1/2] hwmon: (jc42) optionally try to disable the SMBUS timeout

[Peter Rosin]

With a nxp,se97 chip on an atmel sama5d31 board, the I2C adapter driver
is not always capable of avoiding the 25-35 ms timeout as specified by
the SMBUS protocol. This may cause silent corruption of the last bit of
any transfer, e.g. a one is read instead of a zero if the sensor chip
times out. This also affects the eeprom half of the nxp-se97 chip, where
this silent corruption was originally noticed. Other I2C adapters probably
suffer similar issues, e.g. bit-banging comes to mind as risky...

The SMBUS register in the nxp chip is not a standard Jedec register, but
it is not special to the nxp chips either, at least the atmel chips
have the same mechanism. Therefore, do not special case this on the
manufacturer, it is opt-in via the device property anyway.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/hwmon/jc42.txt |  4 ++++
 drivers/hwmon/jc42.c                             | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/jc42.txt b/Documentation/devicetree/bindings/hwmon/jc42.txt
index 07a250498fbb..f569db58f64a 100644
--- a/Documentation/devicetree/bindings/hwmon/jc42.txt
+++ b/Documentation/devicetree/bindings/hwmon/jc42.txt
@@ -34,6 +34,10 @@ Required properties:
 
 - reg: I2C address
 
+Optional properties:
+- smbus-timeout-disable: When set, the smbus timeout function will be disabled.
+			 This is not supported on all chips.
+
 Example:
 
 temp-sensor@1a {
diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c
index 5f11dc014ed6..e5234f953a6d 100644
--- a/drivers/hwmon/jc42.c
+++ b/drivers/hwmon/jc42.c
@@ -22,6 +22,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#include <linux/bitops.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -45,6 +46,7 @@ static const unsigned short normal_i2c[] = {
 #define JC42_REG_TEMP		0x05
 #define JC42_REG_MANID		0x06
 #define JC42_REG_DEVICEID	0x07
+#define JC42_REG_SMBUS		0x22 /* NXP and Atmel, possibly others? */
 
 /* Status bits in temperature register */
 #define JC42_ALARM_CRIT_BIT	15
@@ -75,6 +77,9 @@ static const unsigned short normal_i2c[] = {
 #define GT_MANID		0x1c68	/* Giantec */
 #define GT_MANID2		0x132d	/* Giantec, 2nd mfg ID */
 
+/* SMBUS register */
+#define SMBUS_STMOUT		BIT(7)  /* SMBus time-out, active low */
+
 /* Supported chips */
 
 /* Analog Devices */
@@ -495,6 +500,22 @@ static int jc42_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	data->extended = !!(cap & JC42_CAP_RANGE);
 
+	if (device_property_read_bool(dev, "smbus-timeout-disable")) {
+		int smbus;
+
+		/*
+		 * Not all chips support this register, but from a
+		 * quick read of various datasheets no chip appears
+		 * incompatible with the below attempt to disable
+		 * the timeout. And the whole thing is opt-in...
+		 */
+		smbus = i2c_smbus_read_word_swapped(client, JC42_REG_SMBUS);
+		if (smbus < 0)
+			return smbus;
+		i2c_smbus_write_word_swapped(client, JC42_REG_SMBUS,
+					     smbus | SMBUS_STMOUT);
+	}
+
 	config = i2c_smbus_read_word_swapped(client, JC42_REG_CONFIG);
 	if (config < 0)
 		return config;
-- 
2.11.0

[PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Peter Rosin]

The I2C adapter driver is sometimes slow, causing the SCL line to
be stuck low for more than the stipulated SMBUS timeout of 25-35 ms.
This causes the client device to give up which in turn causes silent
corruption of data. So, disable the SMBUS timeout in the client device.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 arch/arm/boot/dts/at91-tse850-3.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/at91-tse850-3.dts b/arch/arm/boot/dts/at91-tse850-3.dts
index 5f29010cdbd8..9b82cc8843e1 100644
--- a/arch/arm/boot/dts/at91-tse850-3.dts
+++ b/arch/arm/boot/dts/at91-tse850-3.dts
@@ -221,6 +221,7 @@
 	jc42@18 {
 		compatible = "nxp,se97b", "jedec,jc-42.4-temp";
 		reg = <0x18>;
+		smbus-timeout-disable;
 	};
 
 	dpot: mcp4651-104@28 {
-- 
2.11.0

Re: [PATCH v2 1/2] hwmon: (jc42) optionally try to disable the SMBUS timeout

[Guenter Roeck]

On Mon, Nov 27, 2017 at 05:31:00PM +0100, Peter Rosin wrote:
> With a nxp,se97 chip on an atmel sama5d31 board, the I2C adapter driver
> is not always capable of avoiding the 25-35 ms timeout as specified by
> the SMBUS protocol. This may cause silent corruption of the last bit of
> any transfer, e.g. a one is read instead of a zero if the sensor chip
> times out. This also affects the eeprom half of the nxp-se97 chip, where
> this silent corruption was originally noticed. Other I2C adapters probably
> suffer similar issues, e.g. bit-banging comes to mind as risky...
> 
> The SMBUS register in the nxp chip is not a standard Jedec register, but
> it is not special to the nxp chips either, at least the atmel chips
> have the same mechanism. Therefore, do not special case this on the
> manufacturer, it is opt-in via the device property anyway.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Looks ok to me. I'll apply after Rob's approval.

Guenter

> ---
>  Documentation/devicetree/bindings/hwmon/jc42.txt |  4 ++++
>  drivers/hwmon/jc42.c                             | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/jc42.txt b/Documentation/devicetree/bindings/hwmon/jc42.txt
> index 07a250498fbb..f569db58f64a 100644
> --- a/Documentation/devicetree/bindings/hwmon/jc42.txt
> +++ b/Documentation/devicetree/bindings/hwmon/jc42.txt
> @@ -34,6 +34,10 @@ Required properties:
>  
>  - reg: I2C address
>  
> +Optional properties:
> +- smbus-timeout-disable: When set, the smbus timeout function will be disabled.
> +			 This is not supported on all chips.
> +
>  Example:
>  
>  temp-sensor@1a {
> diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c
> index 5f11dc014ed6..e5234f953a6d 100644
> --- a/drivers/hwmon/jc42.c
> +++ b/drivers/hwmon/jc42.c
> @@ -22,6 +22,7 @@
>   * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>   */
>  
> +#include <linux/bitops.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> @@ -45,6 +46,7 @@ static const unsigned short normal_i2c[] = {
>  #define JC42_REG_TEMP		0x05
>  #define JC42_REG_MANID		0x06
>  #define JC42_REG_DEVICEID	0x07
> +#define JC42_REG_SMBUS		0x22 /* NXP and Atmel, possibly others? */
>  
>  /* Status bits in temperature register */
>  #define JC42_ALARM_CRIT_BIT	15
> @@ -75,6 +77,9 @@ static const unsigned short normal_i2c[] = {
>  #define GT_MANID		0x1c68	/* Giantec */
>  #define GT_MANID2		0x132d	/* Giantec, 2nd mfg ID */
>  
> +/* SMBUS register */
> +#define SMBUS_STMOUT		BIT(7)  /* SMBus time-out, active low */
> +
>  /* Supported chips */
>  
>  /* Analog Devices */
> @@ -495,6 +500,22 @@ static int jc42_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  
>  	data->extended = !!(cap & JC42_CAP_RANGE);
>  
> +	if (device_property_read_bool(dev, "smbus-timeout-disable")) {
> +		int smbus;
> +
> +		/*
> +		 * Not all chips support this register, but from a
> +		 * quick read of various datasheets no chip appears
> +		 * incompatible with the below attempt to disable
> +		 * the timeout. And the whole thing is opt-in...
> +		 */
> +		smbus = i2c_smbus_read_word_swapped(client, JC42_REG_SMBUS);
> +		if (smbus < 0)
> +			return smbus;
> +		i2c_smbus_write_word_swapped(client, JC42_REG_SMBUS,
> +					     smbus | SMBUS_STMOUT);
> +	}
> +
>  	config = i2c_smbus_read_word_swapped(client, JC42_REG_CONFIG);
>  	if (config < 0)
>  		return config;
> -- 
> 2.11.0
> 

Re: [PATCH v2 1/2] hwmon: (jc42) optionally try to disable the SMBUS timeout

[Rob Herring]

On Mon, Nov 27, 2017 at 05:31:00PM +0100, Peter Rosin wrote:
> With a nxp,se97 chip on an atmel sama5d31 board, the I2C adapter driver
> is not always capable of avoiding the 25-35 ms timeout as specified by
> the SMBUS protocol. This may cause silent corruption of the last bit of
> any transfer, e.g. a one is read instead of a zero if the sensor chip
> times out. This also affects the eeprom half of the nxp-se97 chip, where
> this silent corruption was originally noticed. Other I2C adapters probably
> suffer similar issues, e.g. bit-banging comes to mind as risky...
> 
> The SMBUS register in the nxp chip is not a standard Jedec register, but
> it is not special to the nxp chips either, at least the atmel chips
> have the same mechanism. Therefore, do not special case this on the
> manufacturer, it is opt-in via the device property anyway.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  Documentation/devicetree/bindings/hwmon/jc42.txt |  4 ++++

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/hwmon/jc42.c                             | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)

Re: [v2,1/2] hwmon: (jc42) optionally try to disable the SMBUS timeout

[Guenter Roeck]

On Mon, Nov 27, 2017 at 05:31:00PM +0100, Peter Rosin wrote:
> With a nxp,se97 chip on an atmel sama5d31 board, the I2C adapter driver
> is not always capable of avoiding the 25-35 ms timeout as specified by
> the SMBUS protocol. This may cause silent corruption of the last bit of
> any transfer, e.g. a one is read instead of a zero if the sensor chip
> times out. This also affects the eeprom half of the nxp-se97 chip, where
> this silent corruption was originally noticed. Other I2C adapters probably
> suffer similar issues, e.g. bit-banging comes to mind as risky...
> 
> The SMBUS register in the nxp chip is not a standard Jedec register, but
> it is not special to the nxp chips either, at least the atmel chips
> have the same mechanism. Therefore, do not special case this on the
> manufacturer, it is opt-in via the device property anyway.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> Acked-by: Rob Herring <robh@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  Documentation/devicetree/bindings/hwmon/jc42.txt |  4 ++++
>  drivers/hwmon/jc42.c                             | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/jc42.txt b/Documentation/devicetree/bindings/hwmon/jc42.txt
> index 07a250498fbb..f569db58f64a 100644
> --- a/Documentation/devicetree/bindings/hwmon/jc42.txt
> +++ b/Documentation/devicetree/bindings/hwmon/jc42.txt
> @@ -34,6 +34,10 @@ Required properties:
>  
>  - reg: I2C address
>  
> +Optional properties:
> +- smbus-timeout-disable: When set, the smbus timeout function will be disabled.
> +			 This is not supported on all chips.
> +
>  Example:
>  
>  temp-sensor@1a {
> diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c
> index 5f11dc014ed6..e5234f953a6d 100644
> --- a/drivers/hwmon/jc42.c
> +++ b/drivers/hwmon/jc42.c
> @@ -22,6 +22,7 @@
>   * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>   */
>  
> +#include <linux/bitops.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> @@ -45,6 +46,7 @@ static const unsigned short normal_i2c[] = {
>  #define JC42_REG_TEMP		0x05
>  #define JC42_REG_MANID		0x06
>  #define JC42_REG_DEVICEID	0x07
> +#define JC42_REG_SMBUS		0x22 /* NXP and Atmel, possibly others? */
>  
>  /* Status bits in temperature register */
>  #define JC42_ALARM_CRIT_BIT	15
> @@ -75,6 +77,9 @@ static const unsigned short normal_i2c[] = {
>  #define GT_MANID		0x1c68	/* Giantec */
>  #define GT_MANID2		0x132d	/* Giantec, 2nd mfg ID */
>  
> +/* SMBUS register */
> +#define SMBUS_STMOUT		BIT(7)  /* SMBus time-out, active low */
> +
>  /* Supported chips */
>  
>  /* Analog Devices */
> @@ -495,6 +500,22 @@ static int jc42_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  
>  	data->extended = !!(cap & JC42_CAP_RANGE);
>  
> +	if (device_property_read_bool(dev, "smbus-timeout-disable")) {
> +		int smbus;
> +
> +		/*
> +		 * Not all chips support this register, but from a
> +		 * quick read of various datasheets no chip appears
> +		 * incompatible with the below attempt to disable
> +		 * the timeout. And the whole thing is opt-in...
> +		 */
> +		smbus = i2c_smbus_read_word_swapped(client, JC42_REG_SMBUS);
> +		if (smbus < 0)
> +			return smbus;
> +		i2c_smbus_write_word_swapped(client, JC42_REG_SMBUS,
> +					     smbus | SMBUS_STMOUT);
> +	}
> +
>  	config = i2c_smbus_read_word_swapped(client, JC42_REG_CONFIG);
>  	if (config < 0)
>  		return config;

Re: [PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Guenter Roeck]

On Mon, Nov 27, 2017 at 05:31:01PM +0100, Peter Rosin wrote:
> The I2C adapter driver is sometimes slow, causing the SCL line to
> be stuck low for more than the stipulated SMBUS timeout of 25-35 ms.
> This causes the client device to give up which in turn causes silent
> corruption of data. So, disable the SMBUS timeout in the client device.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Acked-by: Guenter Roeck <linux@roeck-us.net>

I assume this will be sent upstream through an arm tree.

Thanks,
Guenter

> ---
>  arch/arm/boot/dts/at91-tse850-3.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/at91-tse850-3.dts b/arch/arm/boot/dts/at91-tse850-3.dts
> index 5f29010cdbd8..9b82cc8843e1 100644
> --- a/arch/arm/boot/dts/at91-tse850-3.dts
> +++ b/arch/arm/boot/dts/at91-tse850-3.dts
> @@ -221,6 +221,7 @@
>  	jc42@18 {
>  		compatible = "nxp,se97b", "jedec,jc-42.4-temp";
>  		reg = <0x18>;
> +		smbus-timeout-disable;
>  	};
>  
>  	dpot: mcp4651-104@28 {
> -- 
> 2.11.0
> 

Re: [PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Alexandre Belloni]

On 29/11/2017 at 12:53:11 -0800, Guenter Roeck wrote:
> On Mon, Nov 27, 2017 at 05:31:01PM +0100, Peter Rosin wrote:
> > The I2C adapter driver is sometimes slow, causing the SCL line to
> > be stuck low for more than the stipulated SMBUS timeout of 25-35 ms.
> > This causes the client device to give up which in turn causes silent
> > corruption of data. So, disable the SMBUS timeout in the client device.
> > 
> > Signed-off-by: Peter Rosin <peda@axentia.se>
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> 
> I assume this will be sent upstream through an arm tree.
> 

Yes, I'm applying it right now.

> Thanks,
> Guenter
> 
> > ---
> >  arch/arm/boot/dts/at91-tse850-3.dts | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/boot/dts/at91-tse850-3.dts b/arch/arm/boot/dts/at91-tse850-3.dts
> > index 5f29010cdbd8..9b82cc8843e1 100644
> > --- a/arch/arm/boot/dts/at91-tse850-3.dts
> > +++ b/arch/arm/boot/dts/at91-tse850-3.dts
> > @@ -221,6 +221,7 @@
> >  	jc42@18 {
> >  		compatible = "nxp,se97b", "jedec,jc-42.4-temp";
> >  		reg = <0x18>;
> > +		smbus-timeout-disable;
> >  	};
> >  
> >  	dpot: mcp4651-104@28 {
> > -- 
> > 2.11.0
> > 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Guenter Roeck]

On Wed, Nov 29, 2017 at 09:56:29PM +0100, Alexandre Belloni wrote:
> On 29/11/2017 at 12:53:11 -0800, Guenter Roeck wrote:
> > On Mon, Nov 27, 2017 at 05:31:01PM +0100, Peter Rosin wrote:
> > > The I2C adapter driver is sometimes slow, causing the SCL line to
> > > be stuck low for more than the stipulated SMBUS timeout of 25-35 ms.
> > > This causes the client device to give up which in turn causes silent
> > > corruption of data. So, disable the SMBUS timeout in the client device.
> > > 
> > > Signed-off-by: Peter Rosin <peda@axentia.se>
> > 
> > Acked-by: Guenter Roeck <linux@roeck-us.net>
> > 
> > I assume this will be sent upstream through an arm tree.
> > 
> 
> Yes, I'm applying it right now.
> 
Are you going to apply the patch for 4.15, or queue it up for 4.16 ?
I have been arguing with myself if this is a feature or a bug fix.
So far I queued the driver change up for 4.16, but I am open to
applying it to 4.15. Any thoughts ?

Guenter

> > Thanks,
> > Guenter
> > 
> > > ---
> > >  arch/arm/boot/dts/at91-tse850-3.dts | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/at91-tse850-3.dts b/arch/arm/boot/dts/at91-tse850-3.dts
> > > index 5f29010cdbd8..9b82cc8843e1 100644
> > > --- a/arch/arm/boot/dts/at91-tse850-3.dts
> > > +++ b/arch/arm/boot/dts/at91-tse850-3.dts
> > > @@ -221,6 +221,7 @@
> > >  	jc42@18 {
> > >  		compatible = "nxp,se97b", "jedec,jc-42.4-temp";
> > >  		reg = <0x18>;
> > > +		smbus-timeout-disable;
> > >  	};
> > >  
> > >  	dpot: mcp4651-104@28 {
> > > -- 
> > > 2.11.0
> > > 
> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Alexandre Belloni]

On 30/11/2017 at 09:16:38 -0800, Guenter Roeck wrote:
> On Wed, Nov 29, 2017 at 09:56:29PM +0100, Alexandre Belloni wrote:
> > On 29/11/2017 at 12:53:11 -0800, Guenter Roeck wrote:
> > > On Mon, Nov 27, 2017 at 05:31:01PM +0100, Peter Rosin wrote:
> > > > The I2C adapter driver is sometimes slow, causing the SCL line to
> > > > be stuck low for more than the stipulated SMBUS timeout of 25-35 ms.
> > > > This causes the client device to give up which in turn causes silent
> > > > corruption of data. So, disable the SMBUS timeout in the client device.
> > > > 
> > > > Signed-off-by: Peter Rosin <peda@axentia.se>
> > > 
> > > Acked-by: Guenter Roeck <linux@roeck-us.net>
> > > 
> > > I assume this will be sent upstream through an arm tree.
> > > 
> > 
> > Yes, I'm applying it right now.
> > 
> Are you going to apply the patch for 4.15, or queue it up for 4.16 ?
> I have been arguing with myself if this is a feature or a bug fix.
> So far I queued the driver change up for 4.16, but I am open to
> applying it to 4.15. Any thoughts ?
> 

I was wondering that myself. I'm open to have it as a fix in 4.15. Or
maybe Peter can send the series to stable if he needs it in 4.14.

Peter, what do you think/want?


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Peter Rosin]

On 2017-11-30 18:26, Alexandre Belloni wrote:
> On 30/11/2017 at 09:16:38 -0800, Guenter Roeck wrote:
>> On Wed, Nov 29, 2017 at 09:56:29PM +0100, Alexandre Belloni wrote:
>>> On 29/11/2017 at 12:53:11 -0800, Guenter Roeck wrote:
>>>> On Mon, Nov 27, 2017 at 05:31:01PM +0100, Peter Rosin wrote:
>>>>> The I2C adapter driver is sometimes slow, causing the SCL line to
>>>>> be stuck low for more than the stipulated SMBUS timeout of 25-35 ms.
>>>>> This causes the client device to give up which in turn causes silent
>>>>> corruption of data. So, disable the SMBUS timeout in the client device.
>>>>>
>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>
>>>> Acked-by: Guenter Roeck <linux@roeck-us.net>
>>>>
>>>> I assume this will be sent upstream through an arm tree.
>>>>
>>>
>>> Yes, I'm applying it right now.
>>>
>> Are you going to apply the patch for 4.15, or queue it up for 4.16 ?
>> I have been arguing with myself if this is a feature or a bug fix.
>> So far I queued the driver change up for 4.16, but I am open to
>> applying it to 4.15. Any thoughts ?
>>
> 
> I was wondering that myself. I'm open to have it as a fix in 4.15. Or
> maybe Peter can send the series to stable if he needs it in 4.14.
> 
> Peter, what do you think/want?

TL;DR Either way is fine.

I think it's a bugfix; it fixes real problems where the application
misbehave due to faulty content when reading from an eeprom. I'm
expecting to make a new release for the hw in question RSN and these
are the only local patches. So, it would be nice if they made it to
4.14.x before my release happens. However, it's not like it's difficult
to rebase the patches should that backport not happen or take too long.

The badness started to happen much more frequently due to some timing
difference affecting the i2c bus driver, but in theory it's a problem
that has been there from the start. I have just not noticed it before...

Cheers,
Peter

Re: [PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Guenter Roeck]

On Thu, Nov 30, 2017 at 07:46:09PM +0100, Peter Rosin wrote:
> On 2017-11-30 18:26, Alexandre Belloni wrote:
> > On 30/11/2017 at 09:16:38 -0800, Guenter Roeck wrote:
> >> On Wed, Nov 29, 2017 at 09:56:29PM +0100, Alexandre Belloni wrote:
> >>> On 29/11/2017 at 12:53:11 -0800, Guenter Roeck wrote:
> >>>> On Mon, Nov 27, 2017 at 05:31:01PM +0100, Peter Rosin wrote:
> >>>>> The I2C adapter driver is sometimes slow, causing the SCL line to
> >>>>> be stuck low for more than the stipulated SMBUS timeout of 25-35 ms.
> >>>>> This causes the client device to give up which in turn causes silent
> >>>>> corruption of data. So, disable the SMBUS timeout in the client device.
> >>>>>
> >>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> >>>>
> >>>> Acked-by: Guenter Roeck <linux@roeck-us.net>
> >>>>
> >>>> I assume this will be sent upstream through an arm tree.
> >>>>
> >>>
> >>> Yes, I'm applying it right now.
> >>>
> >> Are you going to apply the patch for 4.15, or queue it up for 4.16 ?
> >> I have been arguing with myself if this is a feature or a bug fix.
> >> So far I queued the driver change up for 4.16, but I am open to
> >> applying it to 4.15. Any thoughts ?
> >>
> > 
> > I was wondering that myself. I'm open to have it as a fix in 4.15. Or
> > maybe Peter can send the series to stable if he needs it in 4.14.
> > 
> > Peter, what do you think/want?
> 
> TL;DR Either way is fine.
> 
> I think it's a bugfix; it fixes real problems where the application
> misbehave due to faulty content when reading from an eeprom. I'm
> expecting to make a new release for the hw in question RSN and these
> are the only local patches. So, it would be nice if they made it to
> 4.14.x before my release happens. However, it's not like it's difficult
> to rebase the patches should that backport not happen or take too long.
> 
Good enough for me. I'll send it as a fix for v4.15, with Cc: stable.

Guenter

> The badness started to happen much more frequently due to some timing
> difference affecting the i2c bus driver, but in theory it's a problem
> that has been there from the start. I have just not noticed it before...
> 
> Cheers,
> Peter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850

[Alexandre Belloni]

On 30/11/2017 at 13:17:40 -0800, Guenter Roeck wrote:
> > I think it's a bugfix; it fixes real problems where the application
> > misbehave due to faulty content when reading from an eeprom. I'm
> > expecting to make a new release for the hw in question RSN and these
> > are the only local patches. So, it would be nice if they made it to
> > 4.14.x before my release happens. However, it's not like it's difficult
> > to rebase the patches should that backport not happen or take too long.
> > 
> Good enough for me. I'll send it as a fix for v4.15, with Cc: stable.
> 

I have it in my fixes branch too, I'll send it to arm-soc soon.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com