Skip to contentSkip to conversation

pcnet: address TODOs

PATCH In review Michael S. Tsirkin 2 participants 2 messages Jun 17, 2010

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pcnet: address TODOs

pcnet enables memory/io on init, which does not make sense as BAR values are wrong. Fix this, disabling BARs according to PCI spec. Address other minor TODOs.

---

The following untested patch brings pcnet in compliance with the spec.
Could someone who's interested in pcnet look at this patch please?

 hw/pcnet.c |   17 ++---------------
 1 files changed, 2 insertions(+), 15 deletions(-)
hw/pcnet.c +2 -15
index 5e63eb5..b52935a 100644
@@ -1981,26 +1981,14 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
 
     pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_AMD);
     pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_AMD_LANCE);
-    /* TODO: value should be 0 at RST# */
-    pci_set_word(pci_conf + PCI_COMMAND,
-                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
     pci_set_word(pci_conf + PCI_STATUS,
                  PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MEDIUM);
     pci_conf[PCI_REVISION_ID] = 0x10;
-    /* TODO: 0 is the default anyway, no need to set it. */
-    pci_conf[PCI_CLASS_PROG] = 0x00;
     pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
-    pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
-
-    /* TODO: not necessary, is set when BAR is registered. */
-    pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_SPACE_IO);
-    pci_set_long(pci_conf + PCI_BASE_ADDRESS_0 + 4,
-                 PCI_BASE_ADDRESS_SPACE_MEMORY);
 
     pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID, 0x0);
     pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, 0x0);
 
-    /* TODO: value must be 0 at RST# */
     pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0
     pci_conf[PCI_MIN_GNT] = 0x06;
     pci_conf[PCI_MAX_LAT] = 0xff;
@@ -2009,11 +1997,10 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     s->mmio_index =
       cpu_register_io_memory(pcnet_mmio_read, pcnet_mmio_write, &d->state);
 
-    /* TODO: use pci_dev, avoid cast below. */
-    pci_register_bar((PCIDevice *)d, 0, PCNET_IOPORT_SIZE,
+    pci_register_bar(pci_dev, 0, PCNET_IOPORT_SIZE,
                            PCI_BASE_ADDRESS_SPACE_IO, pcnet_ioport_map);
 
-    pci_register_bar((PCIDevice *)d, 1, PCNET_PNPMMIO_SIZE,
+    pci_register_bar(pci_dev, 1, PCNET_PNPMMIO_SIZE,
                            PCI_BASE_ADDRESS_SPACE_MEMORY, pcnet_mmio_map);
 
     s->irq = pci_dev->irq[0];
Signature · 1 line
1.7.1.12.g42b7f
Tested-by on v1 1/1
#1 lore raw
Quoted 5 lines · Michael S. Tsirkin

pcnet enables memory/io on init, which does not make sense as BAR values are wrong. Fix this, disabling BARs according to PCI spec. Address other minor TODOs.

Quoted 8 lines

---

The following untested patch brings pcnet in compliance with the spec.
Could someone who's interested in pcnet look at this patch please?

At least our special guest still works with your patch applied.

Jan

Quoted 49 lines
 hw/pcnet.c |   17 ++---------------
 1 files changed, 2 insertions(+), 15 deletions(-)
hw/pcnet.c +2 -15
index 5e63eb5..b52935a 100644
@@ -1981,26 +1981,14 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
 
     pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_AMD);
     pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_AMD_LANCE);
-    /* TODO: value should be 0 at RST# */
-    pci_set_word(pci_conf + PCI_COMMAND,
-                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
     pci_set_word(pci_conf + PCI_STATUS,
                  PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MEDIUM);
     pci_conf[PCI_REVISION_ID] = 0x10;
-    /* TODO: 0 is the default anyway, no need to set it. */
-    pci_conf[PCI_CLASS_PROG] = 0x00;
     pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
-    pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
-
-    /* TODO: not necessary, is set when BAR is registered. */
-    pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_SPACE_IO);
-    pci_set_long(pci_conf + PCI_BASE_ADDRESS_0 + 4,
-                 PCI_BASE_ADDRESS_SPACE_MEMORY);
 
     pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID, 0x0);
     pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, 0x0);
 
-    /* TODO: value must be 0 at RST# */
     pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0
     pci_conf[PCI_MIN_GNT] = 0x06;
     pci_conf[PCI_MAX_LAT] = 0xff;
@@ -2009,11 +1997,10 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     s->mmio_index =
       cpu_register_io_memory(pcnet_mmio_read, pcnet_mmio_write, &d->state);
 
-    /* TODO: use pci_dev, avoid cast below. */
-    pci_register_bar((PCIDevice *)d, 0, PCNET_IOPORT_SIZE,
+    pci_register_bar(pci_dev, 0, PCNET_IOPORT_SIZE,
                            PCI_BASE_ADDRESS_SPACE_IO, pcnet_ioport_map);
 
-    pci_register_bar((PCIDevice *)d, 1, PCNET_PNPMMIO_SIZE,
+    pci_register_bar(pci_dev, 1, PCNET_PNPMMIO_SIZE,
                            PCI_BASE_ADDRESS_SPACE_MEMORY, pcnet_mmio_map);
 
     s->irq = pci_dev->irq[0];
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux