Re: [PATCH] RIO

From: Christoph Hellwig
Date: Wed Dec 08 2004 - 09:00:16 EST


On Wed, Dec 08, 2004 at 02:29:51PM +0100, Patrick van de Lageweg wrote:
> Hi,
>
> This patch converts all save_flags/restore_flags to the new
> spin_lick_irqsave/spin_unlock_irqrestore calls, as well as some
> other 2.6.X cleanups. This allows the "rio" driver to become
> SMP safe.
>
>
> Signed-off-by: Patrick vd Lageweg <patrick@xxxxxxxxxxxx>
> Signed-off-by: Rogier Wolff <R.E.Wolff@xxxxxxxxxxxx>
>
> Patrick

> diff -u -r linux-2.6.10-rc3-clean/drivers/char/Kconfig linux-2.6.10-rc3-rio/drivers/char/Kconfig
> --- linux-2.6.10-rc3-clean/drivers/char/Kconfig Fri Dec 3 15:13:32 2004
> +++ linux-2.6.10-rc3-rio/drivers/char/Kconfig Fri Dec 3 15:28:48 2004
> @@ -299,7 +299,7 @@
>
> config RIO
> tristate "Specialix RIO system support"
> - depends on SERIAL_NONSTANDARD && BROKEN_ON_SMP
> + depends on SERIAL_NONSTANDARD
> help
> This is a driver for the Specialix RIO, a smart serial card which
> drives an outboard box that can support up to 128 ports. Product
> diff -u -r linux-2.6.10-rc3-clean/drivers/char/rio/linux_compat.h linux-2.6.10-rc3-rio/drivers/char/rio/linux_compat.h
> --- linux-2.6.10-rc3-clean/drivers/char/rio/linux_compat.h Fri Dec 3 15:11:52 2004
> +++ linux-2.6.10-rc3-rio/drivers/char/rio/linux_compat.h Fri Dec 3 15:28:48 2004
> @@ -19,8 +19,8 @@
> #include <linux/interrupt.h>
>
>
> -#define disable(oldspl) save_flags (oldspl)
> -#define restore(oldspl) restore_flags (oldspl)
> +#define disable(oldspl) local_irq_save(oldspl);
> +#define restore(oldspl) local_irq_restore(oldspl) ;

This looks broken. local_irq_* really isn't for driver use except for
exception case. Also please kill such silly wrappers..

>
> #define sysbrk(x) kmalloc ((x),in_interrupt()? GFP_ATOMIC : GFP_KERNEL)
> #define sysfree(p,size) kfree ((p))

dito here. Also the in_interrupt() check is wrong.


Looking at the driver with it's deep mess and K&R prototypes you might
be better off with a start from scratch.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/