public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: Make sure i2c_algo_bit_data.timeout is HZ-independent
@ 2009-02-22 11:50 Jean Delvare
       [not found] ` <20090222125005.75b2d661-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Jean Delvare @ 2009-02-22 11:50 UTC (permalink / raw)
  To: Linux I2C; +Cc: Russell King, Lennert Buytenhek, Len Sorensen

i2c_algo_bit_data.timeout is supposed to be in jiffies, so drivers
should use set this value in terms of HZ.

Ultimately I think this field should be discarded in favor of
i2c_adapter.timeout, but that's left for a future patch.

Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Lennert Buytenhek <kernel-OLH4Qvv75CYX/NnBR394Jw@public.gmane.org>
Cc: Len Sorensen <lsorense-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
---
 drivers/i2c/busses/i2c-acorn.c   |    2 +-
 drivers/i2c/busses/i2c-ixp2000.c |    2 +-
 drivers/i2c/busses/scx200_i2c.c  |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

--- linux-2.6.29-rc5.orig/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:33.000000000 +0100
+++ linux-2.6.29-rc5/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:45.000000000 +0100
@@ -76,7 +76,7 @@ static struct i2c_algo_bit_data scx200_i
 	.getsda		= scx200_i2c_getsda,
 	.getscl		= scx200_i2c_getscl,
 	.udelay		= 10,
-	.timeout	= 100,
+	.timeout	= HZ,
 };
 
 static struct i2c_adapter scx200_i2c_ops = {
--- linux-2.6.29-rc5.orig/drivers/i2c/busses/i2c-acorn.c	2009-02-22 12:32:33.000000000 +0100
+++ linux-2.6.29-rc5/drivers/i2c/busses/i2c-acorn.c	2009-02-22 12:32:45.000000000 +0100
@@ -79,7 +79,7 @@ static struct i2c_algo_bit_data ioc_data
 	.getsda		= ioc_getsda,
 	.getscl		= ioc_getscl,
 	.udelay		= 80,
-	.timeout	= 100
+	.timeout	= HZ,
 };
 
 static struct i2c_adapter ioc_ops = {
--- linux-2.6.29-rc5.orig/drivers/i2c/busses/i2c-ixp2000.c	2009-02-22 12:32:33.000000000 +0100
+++ linux-2.6.29-rc5/drivers/i2c/busses/i2c-ixp2000.c	2009-02-22 12:32:45.000000000 +0100
@@ -114,7 +114,7 @@ static int ixp2000_i2c_probe(struct plat
 	drv_data->algo_data.getsda = ixp2000_bit_getsda;
 	drv_data->algo_data.getscl = ixp2000_bit_getscl;
 	drv_data->algo_data.udelay = 6;
-	drv_data->algo_data.timeout = 100;
+	drv_data->algo_data.timeout = HZ;
 
 	strlcpy(drv_data->adapter.name, plat_dev->dev.driver->name,
 		sizeof(drv_data->adapter.name));


-- 
Jean Delvare

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

* Re: [PATCH] i2c: Make sure i2c_algo_bit_data.timeout is HZ-independent
       [not found] ` <20090222125005.75b2d661-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
@ 2009-02-22 11:52   ` Lennert Buytenhek
  2009-02-22 12:03   ` Russell King
  2009-02-23 15:00   ` Lennart Sorensen
  2 siblings, 0 replies; 5+ messages in thread
From: Lennert Buytenhek @ 2009-02-22 11:52 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Linux I2C, Russell King, Len Sorensen

On Sun, Feb 22, 2009 at 12:50:05PM +0100, Jean Delvare wrote:

> i2c_algo_bit_data.timeout is supposed to be in jiffies, so drivers
> should use set this value in terms of HZ.
> 
> Ultimately I think this field should be discarded in favor of
> i2c_adapter.timeout, but that's left for a future patch.
> 
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Lennert Buytenhek <kernel-OLH4Qvv75CYX/NnBR394Jw@public.gmane.org>
> Cc: Len Sorensen <lsorense-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-acorn.c   |    2 +-
>  drivers/i2c/busses/i2c-ixp2000.c |    2 +-
>  drivers/i2c/busses/scx200_i2c.c  |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

For the drivers/i2c/busses/i2c-ixp2000.c part:

Acked-by: Lennert Buytenhek <kernel-OLH4Qvv75CYX/NnBR394Jw@public.gmane.org>

Thanks.

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

* Re: [PATCH] i2c: Make sure i2c_algo_bit_data.timeout is HZ-independent
       [not found] ` <20090222125005.75b2d661-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
  2009-02-22 11:52   ` Lennert Buytenhek
@ 2009-02-22 12:03   ` Russell King
  2009-02-23 15:00   ` Lennart Sorensen
  2 siblings, 0 replies; 5+ messages in thread
From: Russell King @ 2009-02-22 12:03 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Linux I2C, Lennert Buytenhek, Len Sorensen

On Sun, Feb 22, 2009 at 12:50:05PM +0100, Jean Delvare wrote:
> i2c_algo_bit_data.timeout is supposed to be in jiffies, so drivers
> should use set this value in terms of HZ.
> 
> Ultimately I think this field should be discarded in favor of
> i2c_adapter.timeout, but that's left for a future patch.
> 
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>

Thanks.

Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>

> Cc: Lennert Buytenhek <kernel-OLH4Qvv75CYX/NnBR394Jw@public.gmane.org>
> Cc: Len Sorensen <lsorense-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-acorn.c   |    2 +-
>  drivers/i2c/busses/i2c-ixp2000.c |    2 +-
>  drivers/i2c/busses/scx200_i2c.c  |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> --- linux-2.6.29-rc5.orig/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:33.000000000 +0100
> +++ linux-2.6.29-rc5/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:45.000000000 +0100
> @@ -76,7 +76,7 @@ static struct i2c_algo_bit_data scx200_i
>  	.getsda		= scx200_i2c_getsda,
>  	.getscl		= scx200_i2c_getscl,
>  	.udelay		= 10,
> -	.timeout	= 100,
> +	.timeout	= HZ,
>  };
>  
>  static struct i2c_adapter scx200_i2c_ops = {
> --- linux-2.6.29-rc5.orig/drivers/i2c/busses/i2c-acorn.c	2009-02-22 12:32:33.000000000 +0100
> +++ linux-2.6.29-rc5/drivers/i2c/busses/i2c-acorn.c	2009-02-22 12:32:45.000000000 +0100
> @@ -79,7 +79,7 @@ static struct i2c_algo_bit_data ioc_data
>  	.getsda		= ioc_getsda,
>  	.getscl		= ioc_getscl,
>  	.udelay		= 80,
> -	.timeout	= 100
> +	.timeout	= HZ,
>  };
>  
>  static struct i2c_adapter ioc_ops = {
> --- linux-2.6.29-rc5.orig/drivers/i2c/busses/i2c-ixp2000.c	2009-02-22 12:32:33.000000000 +0100
> +++ linux-2.6.29-rc5/drivers/i2c/busses/i2c-ixp2000.c	2009-02-22 12:32:45.000000000 +0100
> @@ -114,7 +114,7 @@ static int ixp2000_i2c_probe(struct plat
>  	drv_data->algo_data.getsda = ixp2000_bit_getsda;
>  	drv_data->algo_data.getscl = ixp2000_bit_getscl;
>  	drv_data->algo_data.udelay = 6;
> -	drv_data->algo_data.timeout = 100;
> +	drv_data->algo_data.timeout = HZ;
>  
>  	strlcpy(drv_data->adapter.name, plat_dev->dev.driver->name,
>  		sizeof(drv_data->adapter.name));
> 
> 
> -- 
> Jean Delvare

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

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

* Re: [PATCH] i2c: Make sure i2c_algo_bit_data.timeout is HZ-independent
       [not found] ` <20090222125005.75b2d661-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
  2009-02-22 11:52   ` Lennert Buytenhek
  2009-02-22 12:03   ` Russell King
@ 2009-02-23 15:00   ` Lennart Sorensen
       [not found]     ` <20090223150012.GD23244-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
  2 siblings, 1 reply; 5+ messages in thread
From: Lennart Sorensen @ 2009-02-23 15:00 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Linux I2C, Russell King, Lennert Buytenhek

On Sun, Feb 22, 2009 at 12:50:05PM +0100, Jean Delvare wrote:
> i2c_algo_bit_data.timeout is supposed to be in jiffies, so drivers
> should use set this value in terms of HZ.
> 
> Ultimately I think this field should be discarded in favor of
> i2c_adapter.timeout, but that's left for a future patch.
> 
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Lennert Buytenhek <kernel-OLH4Qvv75CYX/NnBR394Jw@public.gmane.org>
> Cc: Len Sorensen <lsorense-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-acorn.c   |    2 +-
>  drivers/i2c/busses/i2c-ixp2000.c |    2 +-
>  drivers/i2c/busses/scx200_i2c.c  |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> --- linux-2.6.29-rc5.orig/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:33.000000000 +0100
> +++ linux-2.6.29-rc5/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:45.000000000 +0100
> @@ -76,7 +76,7 @@ static struct i2c_algo_bit_data scx200_i
>  	.getsda		= scx200_i2c_getsda,
>  	.getscl		= scx200_i2c_getscl,
>  	.udelay		= 10,
> -	.timeout	= 100,
> +	.timeout	= HZ,
>  };

OK.  Well I know the driver has worked for me with both HZ=100 and now
HZ=1000, so making it consistent sounds good to me.  I think a lot of
people thought it was in microseconds or miliseconds.

-- 
Len Sorensen

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

* Re: [PATCH] i2c: Make sure i2c_algo_bit_data.timeout is HZ-independent
       [not found]     ` <20090223150012.GD23244-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
@ 2009-02-23 15:20       ` Jean Delvare
  0 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2009-02-23 15:20 UTC (permalink / raw)
  To: Lennart Sorensen; +Cc: Linux I2C, Russell King, Lennert Buytenhek

On Mon, 23 Feb 2009 10:00:12 -0500, Lennart Sorensen wrote:
> On Sun, Feb 22, 2009 at 12:50:05PM +0100, Jean Delvare wrote:
> > i2c_algo_bit_data.timeout is supposed to be in jiffies, so drivers
> > should use set this value in terms of HZ.
> > 
> > Ultimately I think this field should be discarded in favor of
> > i2c_adapter.timeout, but that's left for a future patch.
> > 
> > Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> > Cc: Lennert Buytenhek <kernel-OLH4Qvv75CYX/NnBR394Jw@public.gmane.org>
> > Cc: Len Sorensen <lsorense-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-acorn.c   |    2 +-
> >  drivers/i2c/busses/i2c-ixp2000.c |    2 +-
> >  drivers/i2c/busses/scx200_i2c.c  |    2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > --- linux-2.6.29-rc5.orig/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:33.000000000 +0100
> > +++ linux-2.6.29-rc5/drivers/i2c/busses/scx200_i2c.c	2009-02-22 12:32:45.000000000 +0100
> > @@ -76,7 +76,7 @@ static struct i2c_algo_bit_data scx200_i
> >  	.getsda		= scx200_i2c_getsda,
> >  	.getscl		= scx200_i2c_getscl,
> >  	.udelay		= 10,
> > -	.timeout	= 100,
> > +	.timeout	= HZ,
> >  };
> 
> OK.  Well I know the driver has worked for me with both HZ=100 and now
> HZ=1000, so making it consistent sounds good to me.  I think a lot of
> people thought it was in microseconds or miliseconds.

It was clarified in i2c-algo-bit.h in kernel version 2.5.54, that's
quite a while ago. The problem is that i2c-algo-bit.c itself had it
wrong.

At HZ=1000, 100 jiffies is 100 ms, that's already a reasonably long
timeout. And not that many slaves actually stretch the I2C clock.

-- 
Jean Delvare

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

end of thread, other threads:[~2009-02-23 15:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-22 11:50 [PATCH] i2c: Make sure i2c_algo_bit_data.timeout is HZ-independent Jean Delvare
     [not found] ` <20090222125005.75b2d661-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2009-02-22 11:52   ` Lennert Buytenhek
2009-02-22 12:03   ` Russell King
2009-02-23 15:00   ` Lennart Sorensen
     [not found]     ` <20090223150012.GD23244-1wCw9BSqJbv44Nm34jS7GywD8/FfD2ys@public.gmane.org>
2009-02-23 15:20       ` Jean Delvare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox