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

[Jeff Garzik <jeff@garzik.org>]

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



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/