Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-08 Thread Michael Buesch
On Wednesday 08 August 2007 01:08:08 David Miller wrote: > From: Christoph Hellwig <[EMAIL PROTECTED]> > Date: Wed, 8 Aug 2007 00:04:59 +0100 > > > Please take a look at kernel/irq/handle.c. The irq handler is > > always called with the right dev_id argument. Everything would be a > > complete

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-07 Thread David Miller
From: Christoph Hellwig <[EMAIL PROTECTED]> Date: Wed, 8 Aug 2007 00:04:59 +0100 > Please take a look at kernel/irq/handle.c. The irq handler is > always called with the right dev_id argument. Everything would be a complete > nightmare to handle because you usually need to access the device priv

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-07 Thread Christoph Hellwig
On Wed, Aug 08, 2007 at 12:20:35AM +0200, Michael Buesch wrote: > On Wednesday 08 August 2007 00:15:47 Jeff Garzik wrote: > > Michael Buesch wrote: > > > On Wednesday 01 August 2007 10:31:17 Michael Chan wrote: > > >> +static irqreturn_t bnx2x_msix_sp_int(int irq, void *dev_instance) > > >> +{ > >

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-07 Thread Roland Dreier
> > +static irqreturn_t bnx2x_msix_sp_int(int irq, void *dev_instance) > > +{ > > + struct net_device *dev = dev_instance; > > You need to check if dev==NULL and bail out. > Another driver sharing the IRQ with this might choose to pass the dev > pointer as NULL. I don't really understand

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-07 Thread Michael Buesch
On Wednesday 08 August 2007 00:15:47 Jeff Garzik wrote: > Michael Buesch wrote: > > On Wednesday 01 August 2007 10:31:17 Michael Chan wrote: > >> +static irqreturn_t bnx2x_msix_sp_int(int irq, void *dev_instance) > >> +{ > >> + struct net_device *dev = dev_instance; > > > > You need to check if d

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-07 Thread Jeff Garzik
Michael Buesch wrote: On Wednesday 01 August 2007 10:31:17 Michael Chan wrote: +static irqreturn_t bnx2x_msix_sp_int(int irq, void *dev_instance) +{ + struct net_device *dev = dev_instance; You need to check if dev==NULL and bail out. Another driver sharing the IRQ with this might choose

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-02 Thread Michael Chan
On Thu, 2007-08-02 at 00:06 +0200, Michael Buesch wrote: > +static inline u32 bnx2x_tx_avail(struct bnx2x_fastpath *fp) > > Too big for inlining. > > > +{ > > + u16 used; > > + u32 prod = fp->tx_bd_prod; > > + u32 cons = fp->tx_bd_cons; > > + > > + smp_mb(); > > This barrier need

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-02 Thread Eliezer Tamir
Jeff, Roland, Thanks for taking a look. Jeff Garzik wrote: Roland Dreier wrote: > > +{ PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_NX2_5710, > > +PCI_ANY_ID, PCI_ANY_ID, 0, 0, BCM5710 }, FWIW, this could be neater as { PCI_VDEVICE(BROADCOM, PCI_DEVICE_ID_NX2_5710), BCM5710 } OK

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-02 Thread Eliezer Tamir
Michal, Thanks for going over the code. My responses are inline. Eliezer Michael Buesch wrote: On Wednesday 01 August 2007 10:31:17 Michael Chan wrote: +typedef struct { + u8 reserved[64]; +} license_key_t; No typedef. What is a "license key" used for, anyway? This will be removed.

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-01 Thread Jeff Garzik
Roland Dreier wrote: > > +{ PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_NX2_5710, > > +PCI_ANY_ID, PCI_ANY_ID, 0, 0, BCM5710 }, FWIW, this could be neater as { PCI_VDEVICE(BROADCOM, PCI_DEVICE_ID_NX2_5710), BCM5710 } Yes. And additionally, I prefer (but not require) that

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-01 Thread Roland Dreier
> > + { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_NX2_5710, > > + PCI_ANY_ID, PCI_ANY_ID, 0, 0, BCM5710 }, FWIW, this could be neater as { PCI_VDEVICE(BROADCOM, PCI_DEVICE_ID_NX2_5710), BCM5710 } - R. - To unsubscribe from this list: send the line "unsubscribe netdev" in the bod

Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

2007-08-01 Thread Michael Buesch
On Wednesday 01 August 2007 10:31:17 Michael Chan wrote: > +typedef struct { > + u8 reserved[64]; > +} license_key_t; No typedef. What is a "license key" used for, anyway? > +#define RUN_AT(x)(jiffies + (x)) That macro does only obfuscate code, in my opinion. If you want jiffies