| Message ID | 20081212124913.GC10372@solarflare.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Ben Hutchings <bhutchings@solarflare.com> Date: Fri, 12 Dec 2008 12:49:14 +0000 > pci_disable_device() disables many features, like MSI-X, which we > never reenable in efx_reset(). Further, calls to pci_enable_device() > and pci_disable_device() must be matched since the nesting count was > introduced. > > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> Since the PCI layer provides interfaces which do manage this setting, there is no way you should be doing this behind it's back. You know why? Because someone is going to change some of the internals of this stuff in the PIC layer and it will break your driver. Please instead submit an interface to the PCI layer maintainers that does what you want, then use it. Patch not applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller wrote: > From: Ben Hutchings <bhutchings@solarflare.com> > Date: Fri, 12 Dec 2008 12:49:14 +0000 > > > pci_disable_device() disables many features, like MSI-X, which we > > never reenable in efx_reset(). Further, calls to pci_enable_device() > > and pci_disable_device() must be matched since the nesting count was > > introduced. > > > > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> > > Since the PCI layer provides interfaces which do manage > this setting, there is no way you should be doing this > behind it's back. > > You know why? > > Because someone is going to change some of the internals > of this stuff in the PIC layer and it will break your > driver. > > Please instead submit an interface to the PCI layer > maintainers that does what you want, then use it. I did ask on #linux-pci why this didn't exist; apparently it wasn't considered to be worth providing. So obviously I can submit a patch to the PCI layer but I'm not sure it would be accepted. Ben. > Patch not applied.
diff --git a/drivers/net/sfc/falcon.c b/drivers/net/sfc/falcon.c index 271cbf8..ac0b671 100644 --- a/drivers/net/sfc/falcon.c +++ b/drivers/net/sfc/falcon.c @@ -1344,6 +1344,17 @@ static inline void falcon_irq_ack_a1(struct efx_nic *efx) falcon_readl(efx, ®, WORK_AROUND_BROKEN_PCI_READS_REG_KER_A1); } +static void efx_pci_clear_master(struct pci_dev *dev) +{ + u16 command; + + if (dev->is_busmaster && + !pci_read_config_word(dev, PCI_COMMAND, &command) && + !pci_write_config_word(dev, PCI_COMMAND, + command & ~PCI_COMMAND_MASTER)) + dev->is_busmaster = false; +} + /* Process a fatal interrupt * Disable bus mastering ASAP and schedule a reset */ @@ -1375,9 +1386,9 @@ static irqreturn_t falcon_fatal_interrupt(struct efx_nic *efx) } /* Disable both devices */ - pci_disable_device(efx->pci_dev); + efx_pci_clear_master(efx->pci_dev); if (FALCON_IS_DUAL_FUNC(efx)) - pci_disable_device(nic_data->pci_dev2); + efx_pci_clear_master(nic_data->pci_dev2); falcon_disable_interrupts(efx); if (++n_int_errors < FALCON_MAX_INT_ERRORS) {
pci_disable_device() disables many features, like MSI-X, which we never reenable in efx_reset(). Further, calls to pci_enable_device() and pci_disable_device() must be matched since the nesting count was introduced. Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> --- drivers/net/sfc/falcon.c | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-)