[Qemu-devel] [PATCH] usb-uhci: Use defines for base address registers, instead of magic numbers

[Brad Hards]

Signed-off-by: Brad Hards <address@hidden>
---
 hw/usb-uhci.c |   58 ++++++++++++++++++++++++++++++++++----------------------
 hw/usb.h      |    6 +++++
 2 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 346db3e..210b918 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -75,6 +75,18 @@
 
 #define NB_PORTS 2
 
+/* address offsets for PCI config registers - UHCI 1.1d spec Table 2 */
+#define UHCI_REG_USBCMD     0x00
+#define UHCI_REG_USBSTS     0x02
+#define UHCI_REG_USBINTR    0x04
+#define UHCI_REG_FRNUM      0x06
+#define UHCI_REG_FLBASEADD  0x08
+#define UHCI_REG_SOF_MOD    0x0c
+#define UHCI_REG_PORTSC     0x10
+
+#define UHCI_REG_PORTSC_TOP 0x1F /* last potential Port address */
+#define UHCI_BAR_SIZE       0x20 /* the size of the base address registers */
+
 #ifdef DEBUG
 #define DPRINTF printf
 
@@ -407,7 +419,7 @@ static void uhci_ioport_writeb(void *opaque, uint32_t addr, 
uint32_t val)
 
     addr &= 0x1f;
     switch(addr) {
-    case 0x0c:
+    case UHCI_REG_SOF_MOD:
         s->sof_timing = val;
         break;
     }
@@ -420,7 +432,7 @@ static uint32_t uhci_ioport_readb(void *opaque, uint32_t 
addr)
 
     addr &= 0x1f;
     switch(addr) {
-    case 0x0c:
+    case UHCI_REG_SOF_MOD:
         val = s->sof_timing;
         break;
     default:
@@ -438,7 +450,7 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, 
uint32_t val)
     DPRINTF("uhci: writew port=0x%04x val=0x%04x\n", addr, val);
 
     switch(addr) {
-    case 0x00:
+    case UHCI_REG_USBCMD:
         if ((val & UHCI_CMD_RS) && !(s->cmd & UHCI_CMD_RS)) {
             /* start frame processing */
             qemu_mod_timer(s->frame_timer, qemu_get_clock_ns(vm_clock));
@@ -468,7 +480,7 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, 
uint32_t val)
         }
         s->cmd = val;
         break;
-    case 0x02:
+    case UHCI_REG_USBSTS:
         s->status &= ~val;
         /* XXX: the chip spec is not coherent, so we add a hidden
            register to distinguish between IOC and SPD */
@@ -476,15 +488,15 @@ static void uhci_ioport_writew(void *opaque, uint32_t 
addr, uint32_t val)
             s->status2 = 0;
         uhci_update_irq(s);
         break;
-    case 0x04:
+    case UHCI_REG_USBINTR:
         s->intr = val;
         uhci_update_irq(s);
         break;
-    case 0x06:
+    case UHCI_REG_FRNUM:
         if (s->status & UHCI_STS_HCHALTED)
             s->frnum = val & 0x7ff;
         break;
-    case 0x10 ... 0x1f:
+    case UHCI_REG_PORTSC ... UHCI_REG_PORTSC_TOP:
         {
             UHCIPort *port;
             USBDevice *dev;
@@ -518,19 +530,19 @@ static uint32_t uhci_ioport_readw(void *opaque, uint32_t 
addr)
 
     addr &= 0x1f;
     switch(addr) {
-    case 0x00:
+    case UHCI_REG_USBCMD:
         val = s->cmd;
         break;
-    case 0x02:
+    case UHCI_REG_USBSTS:
         val = s->status;
         break;
-    case 0x04:
+    case UHCI_REG_USBINTR:
         val = s->intr;
         break;
-    case 0x06:
+    case UHCI_REG_FRNUM:
         val = s->frnum;
         break;
-    case 0x10 ... 0x1f:
+    case UHCI_REG_PORTSC ... UHCI_REG_PORTSC_TOP:
         {
             UHCIPort *port;
             int n;
@@ -560,7 +572,7 @@ static void uhci_ioport_writel(void *opaque, uint32_t addr, 
uint32_t val)
     DPRINTF("uhci: writel port=0x%04x val=0x%08x\n", addr, val);
 
     switch(addr) {
-    case 0x08:
+    case UHCI_REG_FLBASEADD:
         s->fl_base_addr = val & ~0xfff;
         break;
     }
@@ -573,7 +585,7 @@ static uint32_t uhci_ioport_readl(void *opaque, uint32_t 
addr)
 
     addr &= 0x1f;
     switch(addr) {
-    case 0x08:
+    case UHCI_REG_FLBASEADD:
         val = s->fl_base_addr;
         break;
     default:
@@ -1101,12 +1113,12 @@ static void uhci_map(PCIDevice *pci_dev, int region_num,
 {
     UHCIState *s = (UHCIState *)pci_dev;
 
-    register_ioport_write(addr, 32, 2, uhci_ioport_writew, s);
-    register_ioport_read(addr, 32, 2, uhci_ioport_readw, s);
-    register_ioport_write(addr, 32, 4, uhci_ioport_writel, s);
-    register_ioport_read(addr, 32, 4, uhci_ioport_readl, s);
-    register_ioport_write(addr, 32, 1, uhci_ioport_writeb, s);
-    register_ioport_read(addr, 32, 1, uhci_ioport_readb, s);
+    register_ioport_write(addr, UHCI_BAR_SIZE, 2, uhci_ioport_writew, s);
+    register_ioport_read(addr, UHCI_BAR_SIZE, 2, uhci_ioport_readw, s);
+    register_ioport_write(addr, UHCI_BAR_SIZE, 4, uhci_ioport_writel, s);
+    register_ioport_read(addr, UHCI_BAR_SIZE, 4, uhci_ioport_readl, s);
+    register_ioport_write(addr, UHCI_BAR_SIZE, 1, uhci_ioport_writeb, s);
+    register_ioport_read(addr, UHCI_BAR_SIZE, 1, uhci_ioport_readb, s);
 }
 
 static USBPortOps uhci_port_ops = {
@@ -1125,7 +1137,7 @@ static int usb_uhci_common_initfn(UHCIState *s)
     pci_config_set_class(pci_conf, PCI_CLASS_SERIAL_USB);
     /* TODO: reset value should be 0. */
     pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
-    pci_conf[0x60] = 0x10; // release number
+    pci_conf[USB_SBRN] = USB_RELEASE_1;
 
     usb_bus_new(&s->bus, &s->dev.qdev);
     for(i = 0; i < NB_PORTS; i++) {
@@ -1142,8 +1154,8 @@ static int usb_uhci_common_initfn(UHCIState *s)
 
     /* Use region 4 for consistency with real hardware.  BSD guests seem
        to rely on this.  */
-    pci_register_bar(&s->dev, 4, 0x20,
-                           PCI_BASE_ADDRESS_SPACE_IO, uhci_map);
+    pci_register_bar(&s->dev, 4, UHCI_BAR_SIZE, PCI_BASE_ADDRESS_SPACE_IO,
+                     uhci_map);
 
     return 0;
 }
diff --git a/hw/usb.h b/hw/usb.h
index d3d755d..f71f421 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -130,6 +130,12 @@
 #define USB_ENDPOINT_XFER_BULK         2
 #define USB_ENDPOINT_XFER_INT          3
 
+/* Constants related to the USB / PCI interaction */
+#define USB_SBRN    0x60 /* Seruial Bus Release Number Register */
+#define USB_RELEASE_1  0x10 /* USB 1.0 */
+#define USB_RELEASE_2  0x20 /* USB 2.0 */
+#define USB_RELEASE_3  0x30 /* USB 3.0 */
+
 typedef struct USBBus USBBus;
 typedef struct USBPort USBPort;
 typedef struct USBDevice USBDevice;
-- 
1.7.1