[patch 2.6.18-rc3] build fixes: tps65010

[patch 2.6.18-rc3] build fixes: tps65010

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: tps65010.patch --]
[-- Type: text/x-diff, Size: 1722 bytes --]

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.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

Index: o26/drivers/i2c/chips/tps65010.c
===================================================================
--- o26.orig/drivers/i2c/chips/tps65010.c	2006-07-30 22:09:03.000000000 -0700
+++ o26/drivers/i2c/chips/tps65010.c	2006-07-31 04:56:47.000000000 -0700
@@ -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) {

Re: [i2c] [patch 2.6.18-rc3] build fixes: tps65010

[Jean Delvare]

Hi David,

> 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".

> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> 
> Index: o26/drivers/i2c/chips/tps65010.c
> ===================================================================
> --- o26.orig/drivers/i2c/chips/tps65010.c	2006-07-30 22:09:03.000000000 -0700
> +++ o26/drivers/i2c/chips/tps65010.c	2006-07-31 04:56:47.000000000 -0700
> @@ -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;
>  

Yup, obviously correct.

> @@ -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) {

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?

-- 
Jean Delvare

Re: [i2c] [patch 2.6.18-rc3] build fixes: tps65010

[David Brownell]

On Monday 31 July 2006 11:53 am, Jean Delvare wrote:
> Hi David,
> 
> > 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.  :)


 > @@ -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) {
> 
> 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.)


> 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!


> 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