2001-04-15 08:59:31

by Jeff Garzik

[permalink] [raw]
Subject: PATCH 2.4.4.3: pci_enable/disable_device stuff

diff -urN linux.vanilla/drivers/pci/pci.c linux/drivers/pci/pci.c
--- linux.vanilla/drivers/pci/pci.c Wed Mar 7 01:44:15 2001
+++ linux/drivers/pci/pci.c Sun Apr 15 04:32:41 2001
@@ -286,12 +286,33 @@
{
int err;

+ pci_set_power_state(dev, 0);
if ((err = pcibios_enable_device(dev)) < 0)
return err;
- pci_set_power_state(dev, 0);
return 0;
}

+/**
+ * pci_disable_device - Disable PCI device after use
+ * @dev: PCI device to be disabled
+ *
+ * Signal to the system that the PCI device is not in use by the system
+ * anymore. Currently this only involves disabling PCI busmastering,
+ * if active.
+ */
+
+void
+pci_disable_device(struct pci_dev *dev)
+{
+ u16 pci_command;
+
+ pci_read_config_word(dev, PCI_COMMAND, &pci_command);
+ if (pci_command & PCI_COMMAND_MASTER) {
+ pci_command &= ~PCI_COMMAND_MASTER;
+ pci_write_config_word(dev, PCI_COMMAND, pci_command);
+ }
+}
+
int
pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge)
{
@@ -1381,6 +1402,7 @@
EXPORT_SYMBOL(pci_devices);
EXPORT_SYMBOL(pci_root_buses);
EXPORT_SYMBOL(pci_enable_device);
+EXPORT_SYMBOL(pci_disable_device);
EXPORT_SYMBOL(pci_find_capability);
EXPORT_SYMBOL(pci_release_regions);
EXPORT_SYMBOL(pci_request_regions);
diff -urN linux.vanilla/include/linux/pci.h linux/include/linux/pci.h
--- linux.vanilla/include/linux/pci.h Mon Mar 26 18:48:46 2001
+++ linux/include/linux/pci.h Sun Apr 15 04:27:28 2001
@@ -525,7 +525,9 @@
int pci_write_config_word(struct pci_dev *dev, int where, u16 val);
int pci_write_config_dword(struct pci_dev *dev, int where, u32 val);

+#define HAVE_PCI_DISABLE_DEVICE
int pci_enable_device(struct pci_dev *dev);
+void pci_disable_device(struct pci_dev *dev);
void pci_set_master(struct pci_dev *dev);
int pci_set_dma_mask(struct pci_dev *dev, dma_addr_t mask);
int pci_set_power_state(struct pci_dev *dev, int state);
@@ -594,6 +596,7 @@

static inline void pci_set_master(struct pci_dev *dev) { }
static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; }
+static inline void pci_disable_device(struct pci_dev *dev) { }
static inline int pci_module_init(struct pci_driver *drv) { return -ENODEV; }
static inline int pci_assign_resource(struct pci_dev *dev, int i) { return -EBUSY;}
static inline int pci_register_driver(struct pci_driver *drv) { return 0;}


Attachments:
pci-disable-2.4.4.3.patch (2.28 kB)

2001-04-16 22:36:39

by Martin Mares

[permalink] [raw]
Subject: Re: PATCH 2.4.4.3: pci_enable/disable_device stuff

Hi!

> The attached patch does two things:
>
> 1) Take PCI devices to D0 state before enabling them. We both think
> this is the right thing to do, but there is always the crazy chance this
> change will break something. So, think twice before applying, but IMHO
> apply :)

I'm not able to cite the PCI PM specs by heart :) ... but looks OK to me.

> 2) Adds pci_disable_device. Right now is just disables busmastering.
> When suspending devices, the last task that should occur is to disable
> busmastering, before ceding control to ACPI. Also its a good idea in
> general to disable busmastering when its not in use; it's friendlier to
> the bus.

OK.

> When unloading drivers too, we should be more "green" about
> disabling devices.

Yes, but not before we're sure we can wake them up correctly. Probably
also needs to handle wakeup of PCI-to-PCI bridges.

> I wonder if we should disable IO and MEM decoding too, and I also like
> to ack PCI_STATUS. I didn't add those things because I'm not yet sure
> we want to do that unconditionally.

I'd rather prefer to avoid this. It brings nothing except for possible
problems.

Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
Compatible: Gracefully accepts erroneous data from any source.