Re: [PATCH 3/4] Make Intel e1000 driver legacy I/O port free

Jeff Garzik
2006-06-08T12:32:10Z

Kenji Kaneshige wrote:
> static struct pci_device_id e1000_pci_tbl[] = {
> - INTEL_E1000_ETHERNET_DEVICE(0x1000),
> - INTEL_E1000_ETHERNET_DEVICE(0x1001),
> - INTEL_E1000_ETHERNET_DEVICE(0x1004),
> - INTEL_E1000_ETHERNET_DEVICE(0x1008),
> - INTEL_E1000_ETHERNET_DEVICE(0x1009),
> - INTEL_E1000_ETHERNET_DEVICE(0x100C),
> - INTEL_E1000_ETHERNET_DEVICE(0x100D),
> - INTEL_E1000_ETHERNET_DEVICE(0x100E),
> - INTEL_E1000_ETHERNET_DEVICE(0x100F),
> - INTEL_E1000_ETHERNET_DEVICE(0x1010),
> - INTEL_E1000_ETHERNET_DEVICE(0x1011),
> - INTEL_E1000_ETHERNET_DEVICE(0x1012),
> - INTEL_E1000_ETHERNET_DEVICE(0x1013),
> - INTEL_E1000_ETHERNET_DEVICE(0x1014),
> - INTEL_E1000_ETHERNET_DEVICE(0x1015),
> - INTEL_E1000_ETHERNET_DEVICE(0x1016),
> - INTEL_E1000_ETHERNET_DEVICE(0x1017),
> - INTEL_E1000_ETHERNET_DEVICE(0x1018),
> - INTEL_E1000_ETHERNET_DEVICE(0x1019),
> - INTEL_E1000_ETHERNET_DEVICE(0x101A),
> - INTEL_E1000_ETHERNET_DEVICE(0x101D),
> - INTEL_E1000_ETHERNET_DEVICE(0x101E),
> - INTEL_E1000_ETHERNET_DEVICE(0x1026),
> - INTEL_E1000_ETHERNET_DEVICE(0x1027),
> - INTEL_E1000_ETHERNET_DEVICE(0x1028),
> - INTEL_E1000_ETHERNET_DEVICE(0x105E),
> - INTEL_E1000_ETHERNET_DEVICE(0x105F),
> - INTEL_E1000_ETHERNET_DEVICE(0x1060),
> - INTEL_E1000_ETHERNET_DEVICE(0x1075),
> - INTEL_E1000_ETHERNET_DEVICE(0x1076),
> - INTEL_E1000_ETHERNET_DEVICE(0x1077),
> - INTEL_E1000_ETHERNET_DEVICE(0x1078),
> - INTEL_E1000_ETHERNET_DEVICE(0x1079),
> - INTEL_E1000_ETHERNET_DEVICE(0x107A),
> - INTEL_E1000_ETHERNET_DEVICE(0x107B),
> - INTEL_E1000_ETHERNET_DEVICE(0x107C),
> - INTEL_E1000_ETHERNET_DEVICE(0x107D),
> - INTEL_E1000_ETHERNET_DEVICE(0x107E),
> - INTEL_E1000_ETHERNET_DEVICE(0x107F),
> - INTEL_E1000_ETHERNET_DEVICE(0x108A),
> - INTEL_E1000_ETHERNET_DEVICE(0x108B),
> - INTEL_E1000_ETHERNET_DEVICE(0x108C),
> - INTEL_E1000_ETHERNET_DEVICE(0x1096),
> - INTEL_E1000_ETHERNET_DEVICE(0x1098),
> - INTEL_E1000_ETHERNET_DEVICE(0x1099),
> - INTEL_E1000_ETHERNET_DEVICE(0x109A),
> - INTEL_E1000_ETHERNET_DEVICE(0x10B5),
> - INTEL_E1000_ETHERNET_DEVICE(0x10B9),
> + INTEL_E1000_ETHERNET_DEVICE(0x1000, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1001, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1004, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1008, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1009, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x100C, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x100D, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x100E, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x100F, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1010, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1011, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1012, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1013, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1014, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1015, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1016, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1017, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1018, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1019, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x101A, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x101D, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x101E, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1026, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1027, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1028, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x105E, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x105F, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1060, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1075, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1076, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1077, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1078, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1079, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x107A, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x107B, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x107C, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x107D, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x107E, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x107F, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x108A, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x108B, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x108C, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x1096, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1098, 0),
> + INTEL_E1000_ETHERNET_DEVICE(0x1099, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x109A, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x10B5, E1000_NO_IOPORT),
> + INTEL_E1000_ETHERNET_DEVICE(0x10B9, 0),
> /* required last entry */
> {0,}
> };

Why change all the entries? I would just change the ones with flags...

> @@ -621,7 +621,14 @@
> int i, err, pci_using_dac;
> uint16_t eeprom_data;
> uint16_t eeprom_apme_mask = E1000_EEPROM_APME;
> - if ((err = pci_enable_device(pdev)))
> + int bars;
> +
> + if (ent->driver_data & E1000_NO_IOPORT)
> + bars = pci_select_bars(pdev, IORESOURCE_MEM);
> + else
> + bars = pci_select_bars(pdev, IORESOURCE_MEM | IORESOURCE_IO);
> +
> + if ((err = pci_enable_device_bars(pdev, bars)))
> return err;

NAK, this is an obvious regression.

pci_enable_device() also powers up the device, and enables irq delivery
(on e.g. cardbus), and is allowed to do other platform-specific device
bring-up tasks.

Jeff