Patchwork [1/2] usb-ehci: replace PORTSC macros with variables

login
register
mail settings
Submitter Kuo-Jung Su
Date Jan. 29, 2013, 5:43 a.m.
Message ID <1359438234-5337-2-git-send-email-dantesu@gmail.com>
Download mbox | patch
Permalink /patch/216435/
State New
Headers show

Comments

Kuo-Jung Su - Jan. 29, 2013, 5:43 a.m.
From: Kuo-Jung Su <dantesu@faraday-tech.com>

Replace PORTSC macros with variables which could then be
configured in ehci_xxxx_class_init(...)

Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Andreas <afaerber@suse.de>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/usb/hcd-ehci-sysbus.c |    6 ++++++
 hw/usb/hcd-ehci.c        |   21 +++++++++++++--------
 hw/usb/hcd-ehci.h        |   12 ++++++------
 3 files changed, 25 insertions(+), 14 deletions(-)
Gerd Hoffmann - Jan. 29, 2013, 6:32 a.m.
Hi,

>  hw/usb/hcd-ehci-sysbus.c |    6 ++++++
>  hw/usb/hcd-ehci.c        |   21 +++++++++++++--------
>  hw/usb/hcd-ehci.h        |   12 ++++++------

> +    s->portscbase = sec->portscbase;
> +    s->portnr = sec->portnr;

These two must be initialized in usb_ehci_pci_initfn too.

Otherwise the patch looks fine.

cheers,
  Gerd
Kuo-Jung Su - Jan. 29, 2013, 6:42 a.m.
Hi Gerd:

Thanks for reminding me of the usb_ehci_pci_initfn.

But I have a stupid question.......

Which one do the upcoming path v2 looks like ?

1. It contains only the diff to usb_ehci_pci_initfn, for example:

    [Qemu-devel][PATCH v2 0/1] usb-ehci: add Faraday FUSBH200 support
    [Qemu-devel][PATCH v2 1/1] usb-ehci: update usb_ehci_pci_initfn to use
variable version of portsc

2. It contains the entired change logs, for example:

    [Qemu-devel][PATCH v2 0/3] usb-ehci: add Faraday FUSBH200 support
    [Qemu-devel][PATCH v2 1/3] usb-ehci: replace PORTSC macros with
variables ..
    [Qemu-devel][PATCH v2 2/3] usb-ehci: add Faraday FUSBH200 support
    [Qemu-devel][PATCH v2 3/3] usb-ehci: update usb_ehci_pci_initfn ...

I know this is stupid, but I really need the answer.
Thanks you!

Best Wishes
Dante Su



2013/1/29 Gerd Hoffmann <kraxel@redhat.com>

>   Hi,
>
> >  hw/usb/hcd-ehci-sysbus.c |    6 ++++++
> >  hw/usb/hcd-ehci.c        |   21 +++++++++++++--------
> >  hw/usb/hcd-ehci.h        |   12 ++++++------
>
> > +    s->portscbase = sec->portscbase;
> > +    s->portnr = sec->portnr;
>
> These two must be initialized in usb_ehci_pci_initfn too.
>
> Otherwise the patch looks fine.
>
> cheers,
>   Gerd
>
>
Andreas Färber - Jan. 29, 2013, 8:18 a.m.
Hi,

Am 29.01.2013 07:42, schrieb Kuo-Jung Su:
> Thanks for reminding me of the usb_ehci_pci_initfn.
> 
> But I have a stupid question.......
> 
> Which one do the upcoming path v2 looks like ?
> 
> 1. It contains only the diff to usb_ehci_pci_initfn, for example:
> 
>     [Qemu-devel][PATCH v2 0/1] usb-ehci: add Faraday FUSBH200 support
>     [Qemu-devel][PATCH v2 1/1] usb-ehci: update usb_ehci_pci_initfn to
> use variable version of portsc
> 
> 2. It contains the entired change logs, for example:
> 
>     [Qemu-devel][PATCH v2 0/3] usb-ehci: add Faraday FUSBH200 support
>     [Qemu-devel][PATCH v2 1/3] usb-ehci: replace PORTSC macros with
> variables ..
>     [Qemu-devel][PATCH v2 2/3] usb-ehci: add Faraday FUSBH200 support
>     [Qemu-devel][PATCH v2 3/3] usb-ehci: update usb_ehci_pci_initfn ...

Neither. You should send a v2 consisting of two patches, with 1/2
containing the requested fix, so that at no point in your patch series
anything breaks.

Regards,
Andreas
Gerd Hoffmann - Jan. 29, 2013, 8:21 a.m.
On 01/29/13 07:42, Kuo-Jung Su wrote:
> Hi Gerd:
> 
> Thanks for reminding me of the usb_ehci_pci_initfn.
> 
> But I have a stupid question.......
> 
> Which one do the upcoming path v2 looks like ?
> 
> 1. It contains only the diff to usb_ehci_pci_initfn, for example:
> 
>     [Qemu-devel][PATCH v2 0/1] usb-ehci: add Faraday FUSBH200 support
>     [Qemu-devel][PATCH v2 1/1] usb-ehci: update usb_ehci_pci_initfn to use
> variable version of portsc
> 
> 2. It contains the entired change logs, for example:
> 
>     [Qemu-devel][PATCH v2 0/3] usb-ehci: add Faraday FUSBH200 support
>     [Qemu-devel][PATCH v2 1/3] usb-ehci: replace PORTSC macros with
> variables ..
>     [Qemu-devel][PATCH v2 2/3] usb-ehci: add Faraday FUSBH200 support
>     [Qemu-devel][PATCH v2 3/3] usb-ehci: update usb_ehci_pci_initfn ...
> 
> I know this is stupid, but I really need the answer.
> Thanks you!

(3) - Update patch #1 to also change usb_ehci_pci_initfn. If you have
      the usb_ehci_pci_initfn update as separate commit now you can use
      'git rebase' to squash.
    - Waiting a bit for more feedback (Andreas might want comment on
      patch #2).
    - Resend everything.

cheers,
  Gerd

Patch

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index e504703..ae2db1a 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -41,6 +41,8 @@  static void usb_ehci_sysbus_realizefn(DeviceState *dev, Error **errp)
 
     s->capsbase = sec->capsbase;
     s->opregbase = sec->opregbase;
+    s->portscbase = sec->portscbase;
+    s->portnr = sec->portnr;
     s->dma = &dma_context_memory;
 
     usb_ehci_initfn(s, dev);
@@ -72,6 +74,8 @@  static void ehci_xlnx_class_init(ObjectClass *oc, void *data)
 
     sec->capsbase = 0x100;
     sec->opregbase = 0x140;
+    sec->portscbase = 0x44;
+    sec->portnr = NB_PORTS;
 }
 
 static const TypeInfo ehci_xlnx_type_info = {
@@ -86,6 +90,8 @@  static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
 
     sec->capsbase = 0x0;
     sec->opregbase = 0x10;
+    sec->portscbase = 0x44;
+    sec->portnr = NB_PORTS;
 }
 
 static const TypeInfo ehci_exynos4210_type_info = {
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 7040659..4c17990 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -988,7 +988,7 @@  static uint64_t ehci_port_read(void *ptr, hwaddr addr,
     uint32_t val;
 
     val = s->portsc[addr >> 2];
-    trace_usb_ehci_portsc_read(addr + PORTSC_BEGIN, addr >> 2, val);
+    trace_usb_ehci_portsc_read(addr + s->portscbase, addr >> 2, val);
     return val;
 }
 
@@ -1029,7 +1029,7 @@  static void ehci_port_write(void *ptr, hwaddr addr,
     uint32_t old = *portsc;
     USBDevice *dev = s->ports[port].dev;
 
-    trace_usb_ehci_portsc_write(addr + PORTSC_BEGIN, addr >> 2, val);
+    trace_usb_ehci_portsc_write(addr + s->portscbase, addr >> 2, val);
 
     /* Clear rwc bits */
     *portsc &= ~(val & PORTSC_RWC_MASK);
@@ -1062,7 +1062,7 @@  static void ehci_port_write(void *ptr, hwaddr addr,
 
     *portsc &= ~PORTSC_RO_MASK;
     *portsc |= val;
-    trace_usb_ehci_portsc_change(addr + PORTSC_BEGIN, addr >> 2, *portsc, old);
+    trace_usb_ehci_portsc_change(addr + s->portscbase, addr >> 2, *portsc, old);
 }
 
 static void ehci_opreg_write(void *ptr, hwaddr addr,
@@ -2510,7 +2510,7 @@  void usb_ehci_initfn(EHCIState *s, DeviceState *dev)
     s->caps[0x01] = 0x00;
     s->caps[0x02] = 0x00;
     s->caps[0x03] = 0x01;        /* HC version */
-    s->caps[0x04] = NB_PORTS;    /* Number of downstream ports */
+    s->caps[0x04] = s->portnr;   /* Number of downstream ports */
     s->caps[0x05] = 0x00;        /* No companion ports at present */
     s->caps[0x06] = 0x00;
     s->caps[0x07] = 0x00;
@@ -2518,8 +2518,13 @@  void usb_ehci_initfn(EHCIState *s, DeviceState *dev)
     s->caps[0x0a] = 0x00;
     s->caps[0x0b] = 0x00;
 
+    if (s->portnr > NB_PORTS) {
+        hw_error("hcd-ehci: too many ports! max. port number=%d\n", NB_PORTS);
+        exit(1);
+    }
+
     usb_bus_new(&s->bus, &ehci_bus_ops, dev);
-    for(i = 0; i < NB_PORTS; i++) {
+    for (i = 0; i < s->portnr; i++) {
         usb_register_port(&s->bus, &s->ports[i], s, i, &ehci_port_ops,
                           USB_SPEED_MASK_HIGH);
         s->ports[i].dev = 0;
@@ -2538,13 +2543,13 @@  void usb_ehci_initfn(EHCIState *s, DeviceState *dev)
     memory_region_init_io(&s->mem_caps, &ehci_mmio_caps_ops, s,
                           "capabilities", CAPA_SIZE);
     memory_region_init_io(&s->mem_opreg, &ehci_mmio_opreg_ops, s,
-                          "operational", PORTSC_BEGIN);
+                          "operational", s->portscbase);
     memory_region_init_io(&s->mem_ports, &ehci_mmio_port_ops, s,
-                          "ports", PORTSC_END - PORTSC_BEGIN);
+                          "ports", 4 * s->portnr);
 
     memory_region_add_subregion(&s->mem, s->capsbase, &s->mem_caps);
     memory_region_add_subregion(&s->mem, s->opregbase, &s->mem_opreg);
-    memory_region_add_subregion(&s->mem, s->opregbase + PORTSC_BEGIN,
+    memory_region_add_subregion(&s->mem, s->opregbase + s->portscbase,
                                 &s->mem_ports);
 }
 
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index e95bb7e..e587b67 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -40,11 +40,7 @@ 
 #define MMIO_SIZE        0x1000
 #define CAPA_SIZE        0x10
 
-#define PORTSC               0x0044
-#define PORTSC_BEGIN         PORTSC
-#define PORTSC_END           (PORTSC + 4 * NB_PORTS)
-
-#define NB_PORTS         6        /* Number of downstream ports */
+#define NB_PORTS         6        /* Max. Number of downstream ports */
 
 typedef struct EHCIPacket EHCIPacket;
 typedef struct EHCIQueue EHCIQueue;
@@ -268,6 +264,8 @@  struct EHCIState {
     int companion_count;
     uint16_t capsbase;
     uint16_t opregbase;
+    uint16_t portscbase;
+    uint16_t portnr;
 
     /* properties */
     uint32_t maxframes;
@@ -278,7 +276,7 @@  struct EHCIState {
      */
     uint8_t caps[CAPA_SIZE];
     union {
-        uint32_t opreg[PORTSC_BEGIN/sizeof(uint32_t)];
+        uint32_t opreg[0x44/sizeof(uint32_t)];
         struct {
             uint32_t usbcmd;
             uint32_t usbsts;
@@ -361,6 +359,8 @@  typedef struct SysBusEHCIClass {
 
     uint16_t capsbase;
     uint16_t opregbase;
+    uint16_t portscbase;
+    uint16_t portnr;
 } SysBusEHCIClass;
 
 #endif