[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 09/15] eepro100: convert to new pci interface

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 09/15] eepro100: convert to new pci interface
Date: Wed, 10 Feb 2010 08:13:23 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20091209 Fedora/3.0-4.fc12 Lightning/1.0pre Thunderbird/3.0

On 02/10/2010 12:32 AM, Stefan Weil wrote:
See my inline comments.

Anthony Liguori schrieb:
  - Removed some dead defines for TARGET_I386 so that we could build once

Signed-off-by: Anthony Liguori<address@hidden>
  hw/eepro100.c |  238 ++++++++++++++-------------------------------------------
  1 files changed, 57 insertions(+), 181 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index b33dbb8..16230c9 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -33,10 +33,6 @@
   * Open Source Software Developer Manual

-#if defined(TARGET_I386)
-# warning "PXE boot still not working!"

You did not fix PXE boot here, did you?
So the warning or a comment should stay there.

A comment is fine, but the TARGET_I386 makes this file unnecessarily dependent on TARGET. With this change, we only need to build eepro100.o once.
  /* PCI EEPRO100 definitions */

-static void pci_map(PCIDevice * pci_dev, int region_num,
-                    pcibus_t addr, pcibus_t size, int type)
+static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
+                         uint32_t value)
-    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
-    TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
-          "size=0x%08"FMT_PCIBUS", type=%d\n",
-          region_num, addr, size, type));
+    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);

Please don't change the name of the PCIDevice pointer argument
from pci_dev to dev.

This dev, dev in DO_UPCAST is ugly and misleading.

It's very common and I changed it for consistency. I honestly don't care though either way.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]