diff mbox

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

Message ID 20171127163101.27859-3-peda@axentia.se (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Rosin Nov. 27, 2017, 4:31 p.m. UTC
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(+)

Comments

Guenter Roeck Nov. 29, 2017, 8:53 p.m. UTC | #1
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
>
Alexandre Belloni Nov. 29, 2017, 8:56 p.m. UTC | #2
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
> >
Guenter Roeck Nov. 30, 2017, 5:16 p.m. UTC | #3
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
Alexandre Belloni Nov. 30, 2017, 5:26 p.m. UTC | #4
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?
Peter Rosin Nov. 30, 2017, 6:46 p.m. UTC | #5
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
Guenter Roeck Nov. 30, 2017, 9:17 p.m. UTC | #6
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
Alexandre Belloni Dec. 4, 2017, 7:39 p.m. UTC | #7
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.
diff mbox

Patch

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 {