Four new i2c drivers and __init/__exit cleanup to i2c

Four new i2c drivers and __init/__exit cleanup to i2c

Post by Albert Cranfor » Tue, 17 Sep 2002 07:50:08



Hello Linus,
New I2C drivers that have been adjusted after Russell King comments of August.
o i2c-algo-8xx.c
o i2c-pport.c
o i2c-adap-ibm_ocp.c
o i2c-pcf-epp.c
o Add new drivers to Config.in and Makefile.
o Add new drivers to i2c-core for initialization.
o Remove EXPORT_NO_SYMBOLS statement from i2c-dev, i2c-elektor and i2c-frodo.
o Cleanup init_module and cleanup_module adding __init and __exit to most drivers.
o Adjust i2c-elektor with cli/sti replacement.
--

  47-i2c-8-patch
< 1K Download
 
 
 

Four new i2c drivers and __init/__exit cleanup to i2c

Post by Jeff Garzi » Tue, 17 Sep 2002 08:30:06



> --- linux/drivers/i2c/i2c-elektor.c.orig   2002-09-14 22:10:45.000000000 -0400
> +++ linux-2.5.34/drivers/i2c/i2c-elektor.c 2002-09-15 01:18:55.000000000 -0400

>    int timeout = 2;

>    if (irq > 0) {
> -          cli();
> +          local_irq_disable();
>            if (pcf_pending == 0) {
>                    interruptible_sleep_on_timeout(&pcf_wait, timeout*HZ );
>            } else
>                    pcf_pending = 0;
> -          sti();
> +          local_irq_enable();
>    } else {
>            udelay(100);
>    }

this is _not_ the way to fix...   use a proper spinlock

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

Four new i2c drivers and __init/__exit cleanup to i2c

Post by Russell Kin » Tue, 17 Sep 2002 08:30:13




> > --- linux/drivers/i2c/i2c-elektor.c.orig      2002-09-14 22:10:45.000000000 -0400
> > +++ linux-2.5.34/drivers/i2c/i2c-elektor.c    2002-09-15 01:18:55.000000000 -0400

> >       int timeout = 2;

> >       if (irq > 0) {
> > -             cli();
> > +             local_irq_disable();
> >               if (pcf_pending == 0) {
> >                       interruptible_sleep_on_timeout(&pcf_wait, timeout*HZ );
> >               } else
> >                       pcf_pending = 0;
> > -             sti();
> > +             local_irq_enable();
> >       } else {
> >               udelay(100);
> >       }

> this is _not_ the way to fix...   use a proper spinlock

You can't hold a spinlock and sleep though, was one of my points back
in August.  (Albert submitted a patch with all cli()/sti() converted
to spin_lock_irqsave()/spin_unlock_irqrestore().)

--

             http://www.arm.linux.org.uk/personal/aboutme.html

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

Four new i2c drivers and __init/__exit cleanup to i2c

Post by Jeff Garzi » Tue, 17 Sep 2002 08:40:04





>>>--- linux/drivers/i2c/i2c-elektor.c.orig    2002-09-14 22:10:45.000000000 -0400
>>>+++ linux-2.5.34/drivers/i2c/i2c-elektor.c  2002-09-15 01:18:55.000000000 -0400

>>>    int timeout = 2;

>>>    if (irq > 0) {
>>>-           cli();
>>>+           local_irq_disable();
>>>            if (pcf_pending == 0) {
>>>                    interruptible_sleep_on_timeout(&pcf_wait, timeout*HZ );
>>>            } else
>>>                    pcf_pending = 0;
>>>-           sti();
>>>+           local_irq_enable();
>>>    } else {
>>>            udelay(100);
>>>    }

>>this is _not_ the way to fix...   use a proper spinlock

> You can't hold a spinlock and sleep though, was one of my points back
> in August.  (Albert submitted a patch with all cli()/sti() converted
> to spin_lock_irqsave()/spin_unlock_irqrestore().)

whoops, you're right.

That follows along with my suggestion in another email, then :)  use a
semaphore.  The timeout can be handled with a kernel timer.  The timeout
is clearly multiple seconds, so there's no fine grain involved.

AND, since the timeout is multiple seconds, the code should not be
disable interrupts for that long anyway.

        Jeff

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/