uli526x: add support for netpoll
Subsystems (2)
PATCH 1/1 uli526x: add support for netpoll
This patch adds netpoll support for the uli526x ethernet driver -- simply call the interrupt handler for polling.
To do this without disable_irq()/enable_irq() pair we should fully protect the handler. Luckily, it's already using irqsave spinlock, the only unprotected place is interrupts re-enabling write. It was safe to re-enable interrupts without holding the spinlock, but with netpoll possibility now it doesn't seem so.
Patch was tested using netconsole and KGDBoE.
--- drivers/net/tulip/uli526x.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-)
drivers/net/tulip/uli526x.c +13 -1
@@ -225,6 +225,7 @@ static void uli526x_set_filter_mode(struct net_device *);static const struct ethtool_ops netdev_ethtool_ops;static u16 read_srom_word (long ,int );static irqreturn_t uli526x_interrupt (int ,void *); +static void uli526x_poll (struct net_device *dev);static void uli526x_descriptor_init (struct uli526x_board_info *,unsigned long );static void allocate_rx_buffer (struct uli526x_board_info *);static void update_cr6 (u32 ,unsigned long );
@@ -339,6 +340,9 @@ static int __devinit uli526x_init_one (struct pci_dev *pdev, dev->get_stats = &uli526x_get_stats; dev->set_multicast_list = &uli526x_set_filter_mode; dev->ethtool_ops = &netdev_ethtool_ops; +#ifdef CONFIG_NET_POLL_CONTROLLER + dev->poll_controller = &uli526x_poll; +#endif spin_lock_init (&db->lock);
@@ -681,8 +685,9 @@ static irqreturn_t uli526x_interrupt(int irq, void *dev_id) db->cr5_data =inl (ioaddr +DCR5 );outl (db->cr5_data, ioaddr +DCR5 );if ( !(db->cr5_data &0x180c1 ) ) { -spin_unlock_irqrestore (&db->lock, flags); +/* Restore CR7 to enable interrupt mask */ outl (db->cr7_data, ioaddr +DCR7 ); +spin_unlock_irqrestore (&db->lock, flags);return IRQ_HANDLED ; }
@@ -715,6 +720,13 @@ static irqreturn_t uli526x_interrupt(int irq, void *dev_id)return IRQ_HANDLED ; } +#ifdef CONFIG_NET_POLL_CONTROLLER +static void uli526x_poll (struct net_device *dev) +{ +/* ISR grabs the irqsave lock, so this should be safe */ +uli526x_interrupt (dev->irq, dev); +} +#endif /* * Free TX resource after TX complete
Signature · 1 line
1.5.5.1
Hello Anton,
Le Thursday 15 May 2008 17:12:55 Anton Vorontsov, vous avez écrit :
drivers/net/tulip/uli526x.c static irqreturn_t uli526x_interrupt(int irq, void *dev_id) · line 729
Quoted 7 lines
drivers/net/tulip/uli526x.c +7
-0+#ifdef CONFIG_NET_POLL_CONTROLLER +static void uli526x_poll (struct net_device *dev) +{ +/* ISR grabs the irqsave lock, so this should be safe */ +uli526x_interrupt (dev->irq, dev); +} +#endif
You do not need to wrap this function into the ifdef, since setting the poll callback is between ifdef. Otherwise patch looks good.
Signature · 2 lines
Cordialement, Florian Fainelli ------------------------------
Quoted 13 lines · Florian Fainelli
Hello Anton,
Le Thursday 15 May 2008 17:12:55 Anton Vorontsov, vous avez écrit :
drivers/net/tulip/uli526x.c static irqreturn_t uli526x_interrupt(int irq, void *dev_id) · line 729
Quoted 7 lines
drivers/net/tulip/uli526x.c +7
-0+#ifdef CONFIG_NET_POLL_CONTROLLER +static void uli526x_poll (struct net_device *dev) +{ +/* ISR grabs the irqsave lock, so this should be safe */ +uli526x_interrupt (dev->irq, dev); +} +#endif You do not need to wrap this function into the ifdef, since setting the poll callback is between ifdef. Otherwise patch looks good.
Nope, I need this #ifdef, otherwise gcc will warn about unused function if netpoll is not selected.
Of course, I could do __maybe_unused, but #ifdef is what most (all?) drivers are doing.
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
PATCH v2 1/1 uli526x: add support for netpoll
This patch adds netpoll support for the uli526x ethernet driver -- simply call the interrupt handler for polling.
To do this without disable_irq()/enable_irq() pair we should fully protect the handler. Luckily, it's already using irqsave spinlock, the only unprotected place is interrupts re-enabling write. It was safe to re-enable interrupts without holding the spinlock, but with netpoll possibility now it doesn't seem so.
Patch was tested using netconsole and KGDBoE.
---
Quoted 15 lines · Anton Vorontsov
Quoted 13 lines · Florian Fainelli
Hello Anton,
Le Thursday 15 May 2008 17:12:55 Anton Vorontsov, vous avez écrit :
Review
Quoted 7 lines
(quoted diff) +7
-0+#ifdef CONFIG_NET_POLL_CONTROLLER +static void uli526x_poll(struct net_device *dev) +{ + /* ISR grabs the irqsave lock, so this should be safe */ + uli526x_interrupt(dev->irq, dev); +} +#endifYou do not need to wrap this function into the ifdef, since setting the poll callback is between ifdef. Otherwise patch looks good.
Quoted 2 lines
Nope, I need this #ifdef, otherwise gcc will warn about unused function if netpoll is not selected.
Even more, I need #ifdef the declaration, because gcc will warn too. :-) Oops. Here is v2.
drivers/net/tulip/uli526x.c | 16 +++++++++++++++- 1 files changed, 15 insertions(+), 1 deletions(-)
drivers/net/tulip/uli526x.c +15 -1
@@ -225,6 +225,9 @@ static void uli526x_set_filter_mode(struct net_device *);static const struct ethtool_ops netdev_ethtool_ops;static u16 read_srom_word (long ,int );static irqreturn_t uli526x_interrupt (int ,void *); +#ifdef CONFIG_NET_POLL_CONTROLLER +static void uli526x_poll (struct net_device *dev); +#endif static void uli526x_descriptor_init (struct uli526x_board_info *,unsigned long );static void allocate_rx_buffer (struct uli526x_board_info *);static void update_cr6 (u32 ,unsigned long );
@@ -339,6 +342,9 @@ static int __devinit uli526x_init_one (struct pci_dev *pdev, dev->get_stats = &uli526x_get_stats; dev->set_multicast_list = &uli526x_set_filter_mode; dev->ethtool_ops = &netdev_ethtool_ops; +#ifdef CONFIG_NET_POLL_CONTROLLER + dev->poll_controller = &uli526x_poll; +#endif spin_lock_init (&db->lock);
@@ -681,8 +687,9 @@ static irqreturn_t uli526x_interrupt(int irq, void *dev_id) db->cr5_data =inl (ioaddr +DCR5 );outl (db->cr5_data, ioaddr +DCR5 );if ( !(db->cr5_data &0x180c1 ) ) { -spin_unlock_irqrestore (&db->lock, flags); +/* Restore CR7 to enable interrupt mask */ outl (db->cr7_data, ioaddr +DCR7 ); +spin_unlock_irqrestore (&db->lock, flags);return IRQ_HANDLED ; }
@@ -715,6 +722,13 @@ static irqreturn_t uli526x_interrupt(int irq, void *dev_id)return IRQ_HANDLED ; } +#ifdef CONFIG_NET_POLL_CONTROLLER +static void uli526x_poll (struct net_device *dev) +{ +/* ISR grabs the irqsave lock, so this should be safe */ +uli526x_interrupt (dev->irq, dev); +} +#endif /* * Free TX resource after TX complete
Signature · 1 line
1.5.5.1
Jeff Garzik
AppliedQuoted 9 lines · Anton Vorontsov
This patch adds netpoll support for the uli526x ethernet driver -- simply call the interrupt handler for polling.
To do this without disable_irq()/enable_irq() pair we should fully protect the handler. Luckily, it's already using irqsave spinlock, the only unprotected place is interrupts re-enabling write. It was safe to re-enable interrupts without holding the spinlock, but with netpoll possibility now it doesn't seem so.
Quoted 3 lines
Patch was tested using netconsole and KGDBoE.
Signed-off-by Anton Vorontsov <[email protected]>
applied