From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang, Yanmin" Subject: Re: [PATCH] PCI Error Recovery: e100 network device driver Date: Sat, 29 Apr 2006 10:48:23 +0800 Message-ID: <1146278903.4314.145.camel@ymzhang-perf.sh.intel.com> References: <20060406222359.GA30037@austin.ibm.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Jeff Garzik , netdev@vger.kernel.org, linux-pci@atrey.karlin.mff.cuni.cz, LKML , linuxppc-dev@ozlabs.org, john.ronciak@intel.com, jesse.brandeburg@intel.com, jeffrey.t.kirsher@intel.com Return-path: To: Linas Vepstas In-Reply-To: <20060406222359.GA30037@austin.ibm.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Fri, 2006-04-07 at 06:24, Linas Vepstas wrote: > Please apply and forward upstream. > > --linas > > [PATCH] PCI Error Recovery: e100 network device driver > > Various PCI bus errors can be signaled by newer PCI controllers. This > patch adds the PCI error recovery callbacks to the intel ethernet e100 > device driver. The patch has been tested, and appears to work well. > > Signed-off-by: Linas Vepstas > Acked-by: Jesse Brandeburg I am enabling PCI-Express AER (Advanced Error Reporting) in kernel and glad to see many drivers to support pci error handling. > > ---- > > drivers/net/e100.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 65 insertions(+) > > Index: linux-2.6.17-rc1/drivers/net/e100.c > =================================================================== > --- linux-2.6.17-rc1.orig/drivers/net/e100.c 2006-04-05 09:56:06.000000000 -0500 > +++ linux-2.6.17-rc1/drivers/net/e100.c 2006-04-06 15:17:29.000000000 -0500 > @@ -2781,6 +2781,70 @@ static void e100_shutdown(struct pci_dev > } > > > +/* ------------------ PCI Error Recovery infrastructure -------------- */ > +/** e100_io_error_detected() is called when PCI error is detected */ > +static pci_ers_result_t e100_io_error_detected(struct pci_dev *pdev, pci_channel_state_t state) > +{ > + struct net_device *netdev = pci_get_drvdata(pdev); > + > + /* Same as calling e100_down(netdev_priv(netdev)), but generic */ > + netdev->stop(netdev); e100 stop method e100_close calls e100_down which would do IO. Does it violate the rule defined in Documentation/pci-error-recovery.txt that error_detected shouldn't do any IO? Suggest to create a new function, such like e100_close_noreset. > + > + /* Detach; put netif into state similar to hotplug unplug */ > + netif_poll_enable(netdev); > + netif_device_detach(netdev); > + > + /* Request a slot reset. */ > + return PCI_ERS_RESULT_NEED_RESET; > +} > + > +/** e100_io_slot_reset is called after the pci bus has been reset. > + * Restart the card from scratch. */ > +static pci_ers_result_t e100_io_slot_reset(struct pci_dev *pdev) > +{ > + struct net_device *netdev = pci_get_drvdata(pdev); > + struct nic *nic = netdev_priv(netdev); > + > + if(pci_enable_device(pdev)) { > + printk(KERN_ERR "e100: Cannot re-enable PCI device after reset.\n"); > + return PCI_ERS_RESULT_DISCONNECT; > + } > + pci_set_master(pdev); > + > + /* Only one device per card can do a reset */ > + if (0 != PCI_FUNC (pdev->devfn)) > + return PCI_ERS_RESULT_RECOVERED; > + e100_hw_reset(nic); > + e100_phy_init(nic); Should pci_set_master be called after e100_hw_reset like in function e100_probe? > + > + return PCI_ERS_RESULT_RECOVERED; > +} > + > +/** e100_io_resume is called when the error recovery driver > + * tells us that its OK to resume normal operation. > + */ > +static void e100_io_resume(struct pci_dev *pdev) > +{ > + struct net_device *netdev = pci_get_drvdata(pdev); > + struct nic *nic = netdev_priv(netdev); > + > + /* ack any pending wake events, disable PME */ > + pci_enable_wake(pdev, 0, 0); > + > + netif_device_attach(netdev); > + if(netif_running(netdev)) { > + e100_open (netdev); > + mod_timer(&nic->watchdog, jiffies); e100_open calls e100_up which already sets watchdog timer. Why to set it again? > + } > +} > +