> On Tue, 2004-09-28 at 13:12, Linux Kernel Mailing List wrote:
> > * IO-mapping with virt_to_bus(), gdth_readb(), gdth_writeb(), ...
> > - * register_reboot_notifier() to get a notify on shutdown used
> > + * register_reboot_notifier() to get a notify on shutown used
>
> why this change ?
>
OK, my fault.
> > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> > +static irqreturn_t gdth_interrupt(int irq, void *dev_id, struct
pt_regs *regs);
> > #else
> > -static void gdth_interrupt(int irq,struct pt_regs *regs);
> > +static void gdth_interrupt(int irq, void *dev_id, struct pt_regs
*regs);
> > #endif
>
> this really is the wrong way to do such irq prototype compatibility in
> drivers. *really*
>
So please tell me what the right way should be. It works without any
problem.
> > +static struct file_operations gdth_fops = {
> > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> > + .ioctl = gdth_ioctl,
> > + .open = gdth_open,
> > + .release = gdth_close,
> > +#else
> > + ioctl:gdth_ioctl,
> > + open:gdth_open,
> > + release:gdth_close,
> > +#endif
>
> C99 initializers work in all kernel versions since it's a property of
> the C compiler not of the kernel. I wonder why you are putting this
> ifdef here....
>
Agree. If the initializers works also fine with compiler versions in
older distributions with the 2.4.x and 2.2.x kernels, the ifdef is
really useless.
> the rest of your ifdefs are generally quite fishy too
unfortionately...
>
Could you please explain it exactly? I really want to learn what the
problems are to correct it in the next version.
On Wed, Sep 29, 2004 at 02:15:57PM +0200, Leubner, Achim wrote:
> > C99 initializers work in all kernel versions since it's a property of
> > the C compiler not of the kernel. I wonder why you are putting this
> > ifdef here....
> >
> Agree. If the initializers works also fine with compiler versions in
> older distributions with the 2.4.x and 2.2.x kernels, the ifdef is
> really useless.
C99 initializes (.foo) are supported at least down to gcc 2.7
> > the rest of your ifdefs are generally quite fishy too
> unfortionately...
> >
> Could you please explain it exactly? I really want to learn what the
> problems are to correct it in the next version.
e.g. you have
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,11)
MODULE_LICENSE("GPL");
#endif
much better would be to put a
#ifndef MODULE_LICENSE
#define MODULE_LICENSE(name)
#endif
into some header and use it unconditionally later on.
or you have
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
b = virt_ctr ? NUMDATA(scp->device->host)->busnum : scp->device->channel;
t = scp->device->id;
#else
b = virt_ctr ? NUMDATA(scp->host)->busnum : scp->channel;
t = scp->target;
#endif
where the 2.6 branch just works for 2.4 and 2.2 kernels aswell, so you
could get rid of the old branch completely.
In genereal always try to write to the current API and emulate it on
older releases.
On Wed, 29 September 2004 14:15:57 +0200, Leubner, Achim wrote:
>
> > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> > > +static irqreturn_t gdth_interrupt(int irq, void *dev_id, struct
> pt_regs *regs);
> > > #else
> > > -static void gdth_interrupt(int irq,struct pt_regs *regs);
> > > +static void gdth_interrupt(int irq, void *dev_id, struct pt_regs
> *regs);
> > > #endif
> >
> > this really is the wrong way to do such irq prototype compatibility in
> > drivers. *really*
> >
> So please tell me what the right way should be. It works without any
> problem.
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
#define irqreturn_t void
#define IRQ_NONE
#define IRQ_HANDLED
#endif
static irqreturn_t gdth_interrupt(int irq, void *_dev, struct pt_regs *regs)
{
if (/*not for me*/)
return IRQ_NONE;
/* some work */
return IRQ_HANDLED;
}
Magically get's converted to old driver code by the macros above.
Point is that all ugly parts are confined to some header and don't
pollute the driver proper.
J?rn
--
Do not stop an army on its way home.
-- Sun Tzu
On Wed, Sep 29, 2004 at 03:43:01PM +0200, J?rn Engel wrote:
> On Wed, 29 September 2004 14:15:57 +0200, Leubner, Achim wrote:
> >
> > > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> > > > +static irqreturn_t gdth_interrupt(int irq, void *dev_id, struct
> > pt_regs *regs);
> > > > #else
> > > > -static void gdth_interrupt(int irq,struct pt_regs *regs);
> > > > +static void gdth_interrupt(int irq, void *dev_id, struct pt_regs
> > *regs);
> > > > #endif
> > >
> > > this really is the wrong way to do such irq prototype compatibility in
> > > drivers. *really*
> > >
> > So please tell me what the right way should be. It works without any
> > problem.
>
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
> #define irqreturn_t void
> #define IRQ_NONE
> #define IRQ_HANDLED
> #endif
Actually all these are in recent 2.4.x release. So better check for
#ifndef IRQ_HANDLED.