Skip to contentSkip to conversation

mach-pxa/viper: Fix timeout usage for I2C

PATCH Superseded Wolfram Sang 7 participants 14 messages Apr 4, 2010 – Apr 20, 2010

Subsystems (4)
Arm (32-bit) Arm sub-architectures I2C subsystem host drivers PXA2xx/PXA3xx support
lore
Share & tools
lore

Wolfram Sang

author
#0 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume '100' means 100ms here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>

---

Janitorial fix, not tested due to no hardware.

 arch/arm/mach-pxa/viper.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
arch/arm/mach-pxa/viper.c +3 -2
index 1dd1334..c25921f 100644
@@ -33,6 +33,7 @@
 #include <linux/pm.h>
 #include <linux/sched.h>
 #include <linux/gpio.h>
+#include <linux/jiffies.h>
 #include <linux/i2c-gpio.h>
 #include <linux/serial_8250.h>
 #include <linux/smc91x.h>

@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
 	.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
 	.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
 	.udelay  = 10,
-	.timeout = 100,
+	.timeout = HZ / 10,
 };
 
 static struct platform_device i2c_bus_device = {

@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
 		.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
 		.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
 		.udelay  = 10,
-		.timeout = 100,
+		.timeout = HZ / 10,
 	};
 	char *errstr;
Signature · 1 line
1.7.0
#1 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 38 lines · Wolfram Sang

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume '100' means 100ms here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>

---

Janitorial fix, not tested due to no hardware.

?arch/arm/mach-pxa/viper.c | ? ?5 +++--
?1 files changed, 3 insertions(+), 2 deletions(-)

arch/arm/mach-pxa/viper.c +0 -0
index 1dd1334..c25921f 100644
@@ -33,6 +33,7 @@

?#include <linux/pm.h>
?#include <linux/sched.h>
?#include <linux/gpio.h>
+#include <linux/jiffies.h>
?#include <linux/i2c-gpio.h>
?#include <linux/serial_8250.h>
?#include <linux/smc91x.h>

arch/arm/mach-pxa/viper.c +1 -1
@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
 ? ? ? ?.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
 ? ? ? ?.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
 ? ? ? ?.udelay ?= 10,
- ? ? ? .timeout = 100,
+ ? ? ? .timeout = HZ / 10,
 ?};
 
 ?static struct platform_device i2c_bus_device = {

arch/arm/mach-pxa/viper.c static void __init viper_tpm_init(void) · line 785

Quoted 8 lines
arch/arm/mach-pxa/viper.c +1 -1
@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
 ? ? ? ? ? ? ? ?.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
 ? ? ? ? ? ? ? ?.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
 ? ? ? ? ? ? ? ?.udelay ?= 10,
- ? ? ? ? ? ? ? .timeout = 100,
+ ? ? ? ? ? ? ? .timeout = HZ / 10,
 ? ? ? ?};
 ? ? ? ?char *errstr;

One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time.

#2 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 37 lines · Eric Miao
Quoted 36 lines · Wolfram Sang

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume '100' means 100ms here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>

---

Janitorial fix, not tested due to no hardware.

?arch/arm/mach-pxa/viper.c | ? ?5 +++--
?1 files changed, 3 insertions(+), 2 deletions(-)

arch/arm/mach-pxa/viper.c +0 -0
index 1dd1334..c25921f 100644
@@ -33,6 +33,7 @@

?#include <linux/pm.h>
?#include <linux/sched.h>
?#include <linux/gpio.h>
+#include <linux/jiffies.h>
?#include <linux/i2c-gpio.h>
?#include <linux/serial_8250.h>
?#include <linux/smc91x.h>

arch/arm/mach-pxa/viper.c +1 -1
@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
 ? ? ? ?.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
 ? ? ? ?.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
 ? ? ? ?.udelay ?= 10,
- ? ? ? .timeout = 100,
+ ? ? ? .timeout = HZ / 10,
 ?};
Quoted 14 lines

arch/arm/mach-pxa/viper.c static void __init viper_tpm_init(void) · line 785

Quoted 10 lines
arch/arm/mach-pxa/viper.c +1 -1
 ?static struct platform_device i2c_bus_device = {

@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
 ? ? ? ? ? ? ? ?.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
 ? ? ? ? ? ? ? ?.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
 ? ? ? ? ? ? ? ?.udelay ?= 10,
- ? ? ? ? ? ? ? .timeout = 100,
+ ? ? ? ? ? ? ? .timeout = HZ / 10,
 ? ? ? ?};
 ? ? ? ?char *errstr;

One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time.

With what benefit? Expressing time values in units of HZ is very frequent in the kernel code and shouldn't actually surprise anyone.

Signature · 1 line
Jean Delvare
#3 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 53 lines · Jean Delvare
Quoted 37 lines · Eric Miao
Quoted 36 lines · Wolfram Sang

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume '100' means 100ms here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>

---

Janitorial fix, not tested due to no hardware.

?arch/arm/mach-pxa/viper.c | ? ?5 +++--
?1 files changed, 3 insertions(+), 2 deletions(-)

arch/arm/mach-pxa/viper.c +0 -0
index 1dd1334..c25921f 100644
@@ -33,6 +33,7 @@

?#include <linux/pm.h>
?#include <linux/sched.h>
?#include <linux/gpio.h>
+#include <linux/jiffies.h>
?#include <linux/i2c-gpio.h>
?#include <linux/serial_8250.h>
?#include <linux/smc91x.h>

arch/arm/mach-pxa/viper.c +1 -1
@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
 ? ? ? ?.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
 ? ? ? ?.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
 ? ? ? ?.udelay ?= 10,
- ? ? ? .timeout = 100,
+ ? ? ? .timeout = HZ / 10,
 ?};
Quoted 14 lines

arch/arm/mach-pxa/viper.c static void __init viper_tpm_init(void) · line 785

Quoted 10 lines
arch/arm/mach-pxa/viper.c +1 -1
 ?static struct platform_device i2c_bus_device = {

@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
 ? ? ? ? ? ? ? ?.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
 ? ? ? ? ? ? ? ?.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
 ? ? ? ? ? ? ? ?.udelay ?= 10,
- ? ? ? ? ? ? ? .timeout = 100,
+ ? ? ? ? ? ? ? .timeout = HZ / 10,
 ? ? ? ?};
 ? ? ? ?char *errstr;

One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time.

Quoted 2 lines

With what benefit? Expressing time values in units of HZ is very frequent in the kernel code and shouldn't actually surprise anyone.

Actually, this patch shows there is confusion.

"Assume '100' means 100ms here and adapt accordingly."

Since this patch is for ARM, where HZ=100, the above patch is not a simple "convert how we derive this constant" patch - it's a functional change, reducing the timeouts by a factor of 10.

Could that be because the patch author misinterpreted the HZ-based values?

I suspect I'm not the only one who thinks that the latter of "HZ / 10" "100ms" is easier to read and comprehend without mistake.

#4 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 59 lines · Russell King - ARM Linux
Quoted 53 lines · Jean Delvare
Quoted 37 lines · Eric Miao
Quoted 36 lines · Wolfram Sang

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume '100' means 100ms here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>

---

Janitorial fix, not tested due to no hardware.

?arch/arm/mach-pxa/viper.c | ? ?5 +++--
?1 files changed, 3 insertions(+), 2 deletions(-)

arch/arm/mach-pxa/viper.c +0 -0
index 1dd1334..c25921f 100644
@@ -33,6 +33,7 @@

?#include <linux/pm.h>
?#include <linux/sched.h>
?#include <linux/gpio.h>
+#include <linux/jiffies.h>
?#include <linux/i2c-gpio.h>
?#include <linux/serial_8250.h>
?#include <linux/smc91x.h>

arch/arm/mach-pxa/viper.c +1 -1
@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
 ? ? ? ?.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
 ? ? ? ?.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
 ? ? ? ?.udelay ?= 10,
- ? ? ? .timeout = 100,
+ ? ? ? .timeout = HZ / 10,
 ?};
Quoted 14 lines

arch/arm/mach-pxa/viper.c static void __init viper_tpm_init(void) · line 785

Quoted 10 lines
arch/arm/mach-pxa/viper.c +1 -1
 ?static struct platform_device i2c_bus_device = {

@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
 ? ? ? ? ? ? ? ?.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
 ? ? ? ? ? ? ? ?.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
 ? ? ? ? ? ? ? ?.udelay ?= 10,
- ? ? ? ? ? ? ? .timeout = 100,
+ ? ? ? ? ? ? ? .timeout = HZ / 10,
 ? ? ? ?};
 ? ? ? ?char *errstr;

One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time.

Quoted 2 lines

With what benefit? Expressing time values in units of HZ is very frequent in the kernel code and shouldn't actually surprise anyone.

Actually, this patch shows there is confusion.

Quoted 8 lines

"Assume '100' means 100ms here and adapt accordingly."

Since this patch is for ARM, where HZ=100, the above patch is not a simple "convert how we derive this constant" patch - it's a functional change, reducing the timeouts by a factor of 10.

Could that be because the patch author misinterpreted the HZ-based values?

I admit I would have assumed 100 -> HZ, as hard-coded HZ-dependent value typically assume HZ=100.

Quoted 2 lines

I suspect I'm not the only one who thinks that the latter of "HZ / 10" "100ms" is easier to read and comprehend without mistake.

OTOH, converting from ms to jiffies each time you need the value has a cost.

Signature · 1 line
Jean Delvare
#5 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 21 lines · Jean Delvare
Quoted 11 lines · Russell King - ARM Linux
Quoted 7 lines · Jean Delvare
Quoted 3 lines · Eric Miao

One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time.

With what benefit? Expressing time values in units of HZ is very frequent in the kernel code and shouldn't actually surprise anyone.

Actually, this patch shows there is confusion.

Quoted 8 lines

"Assume '100' means 100ms here and adapt accordingly."

Since this patch is for ARM, where HZ=100, the above patch is not a simple "convert how we derive this constant" patch - it's a functional change, reducing the timeouts by a factor of 10.

Could that be because the patch author misinterpreted the HZ-based values?

Quoted 8 lines

I admit I would have assumed 100 -> HZ, as hard-coded HZ-dependent value typically assume HZ=100.

Quoted 2 lines

I suspect I'm not the only one who thinks that the latter of "HZ / 10" "100ms" is easier to read and comprehend without mistake.

OTOH, converting from ms to jiffies each time you need the value has a cost.

True; what I did for MMC stuff is converted it from ms to jiffies at initialization time when copying it in from platform data in the driver's probe function.

I'm not saying that I care either way, I'm merely showing that dealing with HZ-based values can be (maybe unexpectedly) more error prone.

#6 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 31 lines · Russell King - ARM Linux
Quoted 21 lines · Jean Delvare
Quoted 11 lines · Russell King - ARM Linux
Quoted 7 lines · Jean Delvare
Quoted 3 lines · Eric Miao

One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time.

With what benefit? Expressing time values in units of HZ is very frequent in the kernel code and shouldn't actually surprise anyone.

Actually, this patch shows there is confusion.

Quoted 8 lines

"Assume '100' means 100ms here and adapt accordingly."

Since this patch is for ARM, where HZ=100, the above patch is not a simple "convert how we derive this constant" patch - it's a functional change, reducing the timeouts by a factor of 10.

Could that be because the patch author misinterpreted the HZ-based values?

Quoted 8 lines

I admit I would have assumed 100 -> HZ, as hard-coded HZ-dependent value typically assume HZ=100.

Quoted 2 lines

I suspect I'm not the only one who thinks that the latter of "HZ / 10" "100ms" is easier to read and comprehend without mistake.

OTOH, converting from ms to jiffies each time you need the value has a cost.

Quoted 6 lines

True; what I did for MMC stuff is converted it from ms to jiffies at initialization time when copying it in from platform data in the driver's probe function.

I'm not saying that I care either way, I'm merely showing that dealing with HZ-based values can be (maybe unexpectedly) more error prone.

HZ is used a lot in kernel timeouts, so even though it's confusing, it is something everyone ought to get used to.

But I agree it is too confusing. An obvious remedy is:

#define milliseconds(ms) (((ms) * HZ + 999) / 1000)
#define seconds(s) ((s) * HZ)

-- Jamie

#7 lore raw

PATCH 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 36 lines · Jamie Lokier
Quoted 35 lines · Russell King - ARM Linux
Quoted 21 lines · Jean Delvare
Quoted 11 lines · Russell King - ARM Linux
Quoted 7 lines · Jean Delvare
Quoted 3 lines · Eric Miao

One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time.

With what benefit? Expressing time values in units of HZ is very frequent in the kernel code and shouldn't actually surprise anyone.

Actually, this patch shows there is confusion.

Quoted 8 lines

"Assume '100' means 100ms here and adapt accordingly."

Since this patch is for ARM, where HZ=100, the above patch is not a simple "convert how we derive this constant" patch - it's a functional change, reducing the timeouts by a factor of 10.

Could that be because the patch author misinterpreted the HZ-based values?

Quoted 8 lines

I admit I would have assumed 100 -> HZ, as hard-coded HZ-dependent value typically assume HZ=100.

Quoted 2 lines

I suspect I'm not the only one who thinks that the latter of "HZ / 10" "100ms" is easier to read and comprehend without mistake.

OTOH, converting from ms to jiffies each time you need the value has a cost.

True; what I did for MMC stuff is converted it from ms to jiffies at initialization time when copying it in from platform data in the driver's probe function.

Quoted 5 lines
Quoted 2 lines

I'm not saying that I care either way, I'm merely showing that dealing with HZ-based values can be (maybe unexpectedly) more error prone.

HZ is used a lot in kernel timeouts, so even though it's confusing, it is something everyone ought to get used to.

I don't understand why people has to live with confusions where there is apparently a cleaner way to go. HZ is everywhere, and as long as they live within their own driver code, that's OK.

Quoted 6 lines

But I agree it is too confusing. ?An obvious remedy is:

#define milliseconds(ms) (((ms) * HZ + 999) / 1000) #define seconds(s) ? ? ? ((s) * HZ)

This is to reinvent the wheel as there are already msecs_to_jiffies() and usecs_to_jiffies(), and vice versa. The only benefit of using macros instead of a function is for constants. This, however, can be worked around as Russell suggested. The additional run-time cost by this function, compared with a unit of jiffies, is insignificant.

Quoted 2 lines

-- Jamie

author
#8 lore raw

PATCH v2 1/1 mach-pxa/viper: Fix timeout usage for I2C

In reply to Jean Delvare

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume with HZ=100 '100' means 1s here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Jean Delvare <[email protected]>

---

Changes since V1:

* Don't assume 100 means 100ms

Thanks for the comments!

Admitted, the first try was a really bad guess. Maybe I got distracted from the previous patch for another arch where the value was 10000.

While this may still not be the favoured solution for Eric, I think it is at least better than before, so it might be worth applying after all?

Janitorial fix, not tested due to no hardware.

 arch/arm/mach-pxa/viper.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
arch/arm/mach-pxa/viper.c +3 -2
index 1dd1334..12cf38c 100644
@@ -33,6 +33,7 @@
 #include <linux/pm.h>
 #include <linux/sched.h>
 #include <linux/gpio.h>
+#include <linux/jiffies.h>
 #include <linux/i2c-gpio.h>
 #include <linux/serial_8250.h>
 #include <linux/smc91x.h>

@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
 	.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
 	.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
 	.udelay  = 10,
-	.timeout = 100,
+	.timeout = HZ,
 };
 
 static struct platform_device i2c_bus_device = {

@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
 		.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
 		.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
 		.udelay  = 10,
-		.timeout = 100,
+		.timeout = HZ,
 	};
 	char *errstr;
Signature · 1 line
1.7.0
#9 lore raw

PATCH v2 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 16 lines · Wolfram Sang

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume with HZ=100 '100' means 1s here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Jean Delvare <[email protected]>

---

Changes since V1:

* Don't assume 100 means 100ms

Quoted 7 lines

Thanks for the comments!

Admitted, the first try was a really bad guess. Maybe I got distracted from the previous patch for another arch where the value was 10000.

While this may still not be the favoured solution for Eric, I think it is at least better than before, so it might be worth applying after all?

Ack. The clearest fix would involve more code to be updated, which can be postponed. This fix, at least, makes the time quantity clearer.

Quoted 40 lines

Janitorial fix, not tested due to no hardware.

?arch/arm/mach-pxa/viper.c | ? ?5 +++--
?1 files changed, 3 insertions(+), 2 deletions(-)

arch/arm/mach-pxa/viper.c +0 -0
index 1dd1334..12cf38c 100644
@@ -33,6 +33,7 @@

?#include <linux/pm.h>
?#include <linux/sched.h>
?#include <linux/gpio.h>
+#include <linux/jiffies.h>
?#include <linux/i2c-gpio.h>
?#include <linux/serial_8250.h>
?#include <linux/smc91x.h>

arch/arm/mach-pxa/viper.c +2 -3
@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
 ? ? ? ?.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
 ? ? ? ?.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
 ? ? ? ?.udelay ?= 10,
- ? ? ? .timeout = 100,
+ ? ? ? .timeout = HZ,
 ?};
 
 ?static struct platform_device i2c_bus_device = {

@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
 ? ? ? ? ? ? ? ?.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
 ? ? ? ? ? ? ? ?.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
 ? ? ? ? ? ? ? ?.udelay ?= 10,
- ? ? ? ? ? ? ? .timeout = 100,
+ ? ? ? ? ? ? ? .timeout = HZ,
 ? ? ? ?};
 ? ? ? ?char *errstr;
 
--

1.7.0

author
#10 lore raw

PATCH v2 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 19 lines · Eric Miao
Quoted 18 lines · Wolfram Sang

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume with HZ=100 '100' means 1s here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Jean Delvare <[email protected]>

---

Changes since V1:

* Don't assume 100 means 100ms

Thanks for the comments!

Quoted 8 lines
Quoted 5 lines

Admitted, the first try was a really bad guess. Maybe I got distracted from the previous patch for another arch where the value was 10000.

While this may still not be the favoured solution for Eric, I think it is at least better than before, so it might be worth applying after all?

Ack. The clearest fix would involve more code to be updated, which can be postponed. This fix, at least, makes the time quantity clearer.

Thanks. Can you pick it up? Because it is a change to a board-file.

Regards,

Wolfram

Signature · 9 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100420/8019b103/attachment.sig>
#11 lore raw

PATCH v2 1/1 mach-pxa/viper: Fix timeout usage for I2C

In reply to Wolfram Sang
Quoted 10 lines · Wolfram Sang

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume with HZ=100 '100' means 1s here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Jean Delvare <[email protected]>
Acked-by Marc Zyngier <[email protected]>

Eric, can you merge this directly via your -devel branch?

Thanks,

        M.
Signature · 1 line
Who you jivin' with that Cosmik Debris?
#12 lore raw

PATCH v2 1/1 mach-pxa/viper: Fix timeout usage for I2C

Dne ?t 20. dubna 2010 10:27:00 Marc Zyngier napsal(a):

Quoted 14 lines

On Sun, 18 Apr 2010 13:48:29 +0200, Wolfram Sang <[email protected]>

Quoted 10 lines · wrote:

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume with HZ=100 '100' means 1s here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Jean Delvare <[email protected]>
Quoted 7 lines
Acked-by Marc Zyngier <[email protected]>

Eric, can you merge this directly via your -devel branch?

Thanks,

        M.

And possibly updating your git tree? There are a few patches in -next that will need it's arm counterpart.

Cheers

#13 lore raw

PATCH v2 1/1 mach-pxa/viper: Fix timeout usage for I2C

Quoted 17 lines · Marek Vasut

Dne ?t 20. dubna 2010 10:27:00 Marc Zyngier napsal(a):

Quoted 16 lines

On Sun, 18 Apr 2010 13:48:29 +0200, Wolfram Sang <[email protected]>

Quoted 10 lines · wrote:

The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume with HZ=100 '100' means 1s here and adapt accordingly.

Signed-off-by Wolfram Sang <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Paul Shen <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Jean Delvare <[email protected]>
Acked-by Marc Zyngier <[email protected]>
Quoted 9 lines
Quoted 5 lines

Eric, can you merge this directly via your -devel branch?

Thanks,

? ? ? ? M.

And possibly updating your git tree? There are a few patches in -next that will need it's arm counterpart.

Merged and updated.