* [PATCH] [NET] uli526x: add support for netpoll @ 2008-05-15 15:12 Anton Vorontsov 2008-05-15 20:51 ` Florian Fainelli 0 siblings, 1 reply; 5+ messages in thread From: Anton Vorontsov @ 2008-05-15 15:12 UTC (permalink / raw) To: Jeff Garzik; +Cc: netdev 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. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- drivers/net/tulip/uli526x.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/drivers/net/tulip/uli526x.c b/drivers/net/tulip/uli526x.c index 2511ca7..6a04a95 100644 --- a/drivers/net/tulip/uli526x.c +++ b/drivers/net/tulip/uli526x.c @@ -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 -- 1.5.5.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] [NET] uli526x: add support for netpoll 2008-05-15 15:12 [PATCH] [NET] uli526x: add support for netpoll Anton Vorontsov @ 2008-05-15 20:51 ` Florian Fainelli 2008-05-16 11:13 ` Anton Vorontsov 0 siblings, 1 reply; 5+ messages in thread From: Florian Fainelli @ 2008-05-15 20:51 UTC (permalink / raw) To: Anton Vorontsov; +Cc: Jeff Garzik, netdev Hello Anton, Le Thursday 15 May 2008 17:12:55 Anton Vorontsov, vous avez écrit : > +#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. -- Cordialement, Florian Fainelli ------------------------------ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [NET] uli526x: add support for netpoll 2008-05-15 20:51 ` Florian Fainelli @ 2008-05-16 11:13 ` Anton Vorontsov 2008-05-16 19:04 ` [PATCH v2] " Anton Vorontsov 0 siblings, 1 reply; 5+ messages in thread From: Anton Vorontsov @ 2008-05-16 11:13 UTC (permalink / raw) To: Florian Fainelli; +Cc: Jeff Garzik, netdev On Thu, May 15, 2008 at 10:51:23PM +0200, Florian Fainelli wrote: > Hello Anton, > > Le Thursday 15 May 2008 17:12:55 Anton Vorontsov, vous avez écrit : > > +#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. -- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] [NET] uli526x: add support for netpoll 2008-05-16 11:13 ` Anton Vorontsov @ 2008-05-16 19:04 ` Anton Vorontsov 2008-05-22 10:21 ` Jeff Garzik 0 siblings, 1 reply; 5+ messages in thread From: Anton Vorontsov @ 2008-05-16 19:04 UTC (permalink / raw) To: Jeff Garzik; +Cc: netdev, Florian Fainelli 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. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- On Fri, May 16, 2008 at 03:13:49PM +0400, Anton Vorontsov wrote: > On Thu, May 15, 2008 at 10:51:23PM +0200, Florian Fainelli wrote: > > Hello Anton, > > > > Le Thursday 15 May 2008 17:12:55 Anton Vorontsov, vous avez écrit : > > > +#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. 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(-) diff --git a/drivers/net/tulip/uli526x.c b/drivers/net/tulip/uli526x.c index 2511ca7..e9e6286 100644 --- a/drivers/net/tulip/uli526x.c +++ b/drivers/net/tulip/uli526x.c @@ -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 -- 1.5.5.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] [NET] uli526x: add support for netpoll 2008-05-16 19:04 ` [PATCH v2] " Anton Vorontsov @ 2008-05-22 10:21 ` Jeff Garzik 0 siblings, 0 replies; 5+ messages in thread From: Jeff Garzik @ 2008-05-22 10:21 UTC (permalink / raw) To: avorontsov; +Cc: netdev, Florian Fainelli Anton Vorontsov wrote: > 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. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> applied ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-05-22 10:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-05-15 15:12 [PATCH] [NET] uli526x: add support for netpoll Anton Vorontsov 2008-05-15 20:51 ` Florian Fainelli 2008-05-16 11:13 ` Anton Vorontsov 2008-05-16 19:04 ` [PATCH v2] " Anton Vorontsov 2008-05-22 10:21 ` Jeff Garzik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).