Skip to contentSkip to conversation

build fixes: tps65010

PATCH In review David Brownell 2 participants 3 messages Jul 31, 2006

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 build fixes: tps65010

The tps65010.c driver in the main tree never got updated with build fixes since the last batch of I2C driver changes; and the genirq trigger flags were updated wierdly too.

drivers/i2c/chips/tps65010.c +4 -6
Index: o26/drivers/i2c/chips/tps65010.c
===================================================================
@@ -43,13 +43,12 @@
 /*-------------------------------------------------------------------------*/
 
 #define	DRIVER_VERSION	"2 May 2005"
-#define	DRIVER_NAME	(tps65010_driver.name)
+#define	DRIVER_NAME	(tps65010_driver.driver.name)
 
 MODULE_DESCRIPTION("TPS6501x Power Management Driver");
 MODULE_LICENSE("GPL");
 
 static unsigned short normal_i2c[] = { 0x48, /* 0x49, */ I2C_CLIENT_END };
-static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
 
 I2C_CLIENT_INSMOD;
 
@@ -520,15 +519,16 @@ tps65010_probe(struct i2c_adapter *bus, 
 		goto fail1;
 	}
 
+	/* IRQ is active low, but some gpio lines can't support that */
+	irqflags = IRQF_SAMPLE_RANDOM;
 #ifdef	CONFIG_ARM
-	irqflags = IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_LOW;
 	if (machine_is_omap_h2()) {
 		tps->model = TPS65010;
 		omap_cfg_reg(W4_GPIO58);
 		tps->irq = OMAP_GPIO_IRQ(58);
 		omap_request_gpio(58);
 		omap_set_gpio_direction(58, 1);
-		irqflags |= IRQF_TRIGGER_FALLING;
+		irqflags |= IRQF_TRIGGER_LOW;
 	}
 	if (machine_is_omap_osk()) {
 		tps->model = TPS65010;
@@ -543,8 +543,6 @@ tps65010_probe(struct i2c_adapter *bus, 
 
 		// FIXME set up this board's IRQ ...
 	}
-#else
-	irqflags = IRQF_SAMPLE_RANDOM;
 #endif
 
 	if (tps->irq > 0) {
#1 lore raw

Hi David,

Quoted 3 lines

The tps65010.c driver in the main tree never got updated with build fixes since the last batch of I2C driver changes; and the genirq trigger flags were updated wierdly too.

Typo, I guess you mean "weirdly".

Quoted 12 lines

Index: o26/drivers/i2c/chips/tps65010.c
===================================================================

drivers/i2c/chips/tps65010.c +0 -1
@@ -43,13 +43,12 @@
 /*-------------------------------------------------------------------------*/
 
 #define	DRIVER_VERSION	"2 May 2005"
-#define	DRIVER_NAME	(tps65010_driver.name)

drivers/i2c/chips/tps65010.c line 54

Quoted 10 lines
drivers/i2c/chips/tps65010.c +1 -1
+#define	DRIVER_NAME	(tps65010_driver.driver.name)
 
 MODULE_DESCRIPTION("TPS6501x Power Management Driver");
 MODULE_LICENSE("GPL");
 
 static unsigned short normal_i2c[] = { 0x48, /* 0x49, */ I2C_CLIENT_END };
-static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
 
 I2C_CLIENT_INSMOD;
 

Yup, obviously correct.

Quoted 20 lines
drivers/i2c/chips/tps65010.c +3 -2
@@ -520,15 +519,16 @@ tps65010_probe(struct i2c_adapter *bus, 
 		goto fail1;
 	}
 
+	/* IRQ is active low, but some gpio lines can't support that */
+	irqflags = IRQF_SAMPLE_RANDOM;
 #ifdef	CONFIG_ARM
-	irqflags = IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_LOW;
 	if (machine_is_omap_h2()) {
 		tps->model = TPS65010;
 		omap_cfg_reg(W4_GPIO58);
 		tps->irq = OMAP_GPIO_IRQ(58);
 		omap_request_gpio(58);
 		omap_set_gpio_direction(58, 1);
-		irqflags |= IRQF_TRIGGER_FALLING;
+		irqflags |= IRQF_TRIGGER_LOW;
 	}
 	if (machine_is_omap_osk()) {
 		tps->model = TPS65010;

drivers/i2c/chips/tps65010.c tps65010_probe(struct i2c_adapter *bus, · line 548

Quoted 8 lines
drivers/i2c/chips/tps65010.c +0 -2
@@ -543,8 +543,6 @@ tps65010_probe(struct i2c_adapter *bus, 
 
 		// FIXME set up this board's IRQ ...
 	}
-#else
-	irqflags = IRQF_SAMPLE_RANDOM;
 #endif
 
 	if (tps->irq > 0) {

This is more surprising. How did the interrupt type suddenly change from "falling" to "low"? (Note that I am not an interrupt expert.)

Anyway, thanks for fixing this. This is one of the i2c drivers that I can't compile on the architectures I work on, so I can't spot the breakage.

I guess you want this fix in 2.6.18?

Signature · 1 line
Jean Delvare
#2 lore raw
Quoted 7 lines · Jean Delvare

Hi David,

Quoted 3 lines

The tps65010.c driver in the main tree never got updated with build fixes since the last batch of I2C driver changes; and the genirq trigger flags were updated wierdly too.

Typo, I guess you mean "weirdly".

OK, feel free to correct. :)

Quoted 27 lines
(quoted diff) +0 -0
@@ -520,15 +519,16 @@ tps65010_probe(struct i2c_adapter *bus, 
 >  		goto fail1;
 >  	}
Quoted 24 lines
drivers/i2c/chips/tps65010.c +3 -4
+	/* IRQ is active low, but some gpio lines can't support that */
+	irqflags = IRQF_SAMPLE_RANDOM;
 #ifdef	CONFIG_ARM
-	irqflags = IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_LOW;
 	if (machine_is_omap_h2()) {
 		tps->model = TPS65010;
 		omap_cfg_reg(W4_GPIO58);
 		tps->irq = OMAP_GPIO_IRQ(58);
 		omap_request_gpio(58);
 		omap_set_gpio_direction(58, 1);
-		irqflags |= IRQF_TRIGGER_FALLING;
+		irqflags |= IRQF_TRIGGER_LOW;
 	}
 	if (machine_is_omap_osk()) {
 		tps->model = TPS65010;
@@ -543,8 +543,6 @@ tps65010_probe(struct i2c_adapter *bus, 
 
 		// FIXME set up this board's IRQ ...
 	}
-#else
-	irqflags = IRQF_SAMPLE_RANDOM;
 #endif
 
Quoted 4 lines

drivers/i2c/chips/tps65010.c tps65010_probe(struct i2c_adapter *bus, · line 548

Quoted 1 line
drivers/i2c/chips/tps65010.c +0 -0
 	if (tps->irq > 0) {

This is more surprising. How did the interrupt type suddenly change from "falling" to "low"? (Note that I am not an interrupt expert.)

The IRQ is always active-low ... but sometimes that signal line will get hooked up to a type of GPIO pin that doesn't support that type of trigger. In that case the driver workaround is to trigger on the falling edge ... "falling" always precedes "low".

However, I double checked and in this case my patch goofed. It's incorrect to set TRIGGER_LOW and then add TRIGGER_FALLING later, so of course the previous code was buggy, but in this case the GPIO should have been left at TRIGGER_FALLING. So I'll resend this patch.

(The problem was that I misremembered the difference between MPUIO and GPIO. It's not that GPIO supports level triggering; it's that GPIO also allows "both edges" triggers.)

Quoted 3 lines

Anyway, thanks for fixing this. This is one of the i2c drivers that I can't compile on the architectures I work on, so I can't spot the breakage.

At some point it should be made to compile on non-OMAP systems, if for no other reason than to address that problem!

Quoted 1 line

I guess you want this fix in 2.6.18?

Yes, please. Fixes for build breakage and other "brown paper bag" errors should have a high merge priority.

- Dave