lkml.org 
[lkml]   [2006]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [i2c] [patch 2.6.18-rc3] build fixes: tps65010
    Date
    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
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-08-01 02:01    [W:0.026 / U:1.628 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site