Use 1 container holding 2 regions:
- I/O registers
- FIFOs
Remove all the static base addresses.
Name address space.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Sometime a patch is cleaner/quicker than explanations.
Suggestion to be squashed on patch:
'dwc-hsotg (dwc2) USB host controller emulation'
Further simplificatio would be to move some exploded read/write
functions directly into dwc2_hsotg_read/write.
Based-on: <20200421014551.10426-1-pauldzim@gmail.com>
---
hw/usb/hcd-dwc2.h | 17 +----
hw/usb/hcd-dwc2.c | 181 ++++++++++++++++++++++------------------------
2 files changed, 88 insertions(+), 110 deletions(-)
diff --git a/hw/usb/hcd-dwc2.h b/hw/usb/hcd-dwc2.h
index 403afd1747..7d7ea604a4 100644
--- a/hw/usb/hcd-dwc2.h
+++ b/hw/usb/hcd-dwc2.h
@@ -61,20 +61,9 @@ struct DWC2State {
qemu_irq irq;
MemoryRegion *dma_mr;
AddressSpace dma_as;
- MemoryRegion mem;
- MemoryRegion mem_glbreg;
- MemoryRegion mem_fszreg;
- MemoryRegion mem_hreg0;
- MemoryRegion mem_hreg1;
- MemoryRegion mem_pcgreg;
- MemoryRegion mem_hreg2;
-
- uint16_t glbregbase;
- uint16_t fszregbase;
- uint16_t hreg0base;
- uint16_t hreg1base;
- uint16_t pcgregbase;
- uint16_t hfifobase;
+ MemoryRegion container;
+ MemoryRegion hsotg;
+ MemoryRegion fifos;
union {
#define DWC2_GLBREG_SIZE 0x70
diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c
index 31de82bcd3..5c0e6a66b8 100644
--- a/hw/usb/hcd-dwc2.c
+++ b/hw/usb/hcd-dwc2.c
@@ -28,6 +28,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "qapi/error.h"
#include "hw/usb/dwc2-regs.h"
#include "hw/usb/hcd-dwc2.h"
@@ -655,7 +656,7 @@ static const char *glbregnm[] = {
static uint64_t dwc2_glbreg_read(void *ptr, hwaddr addr, unsigned size)
{
DWC2State *s = ptr;
- uint32_t reg = s->glbregbase + addr;
+ uint32_t reg = addr;
uint32_t val;
assert(reg <= GINTSTS2);
@@ -681,7 +682,7 @@ static void dwc2_glbreg_write(void *ptr, hwaddr addr, uint64_t val,
{
DWC2State *s = ptr;
uint64_t orig = val;
- uint32_t reg = s->glbregbase + addr;
+ uint32_t reg = addr;
uint32_t *mmio;
uint32_t old;
int iflg = 0;
@@ -762,10 +763,9 @@ static void dwc2_glbreg_write(void *ptr, hwaddr addr, uint64_t val,
static uint64_t dwc2_fszreg_read(void *ptr, hwaddr addr, unsigned size)
{
DWC2State *s = ptr;
- uint32_t reg = s->fszregbase + addr;
uint32_t val;
- assert(reg <= HPTXFSIZ);
+ assert(addr <= HPTXFSIZ);
val = s->fszreg[addr >> 2];
trace_usb_dwc2_fszreg_read(addr, val);
@@ -777,11 +777,10 @@ static void dwc2_fszreg_write(void *ptr, hwaddr addr, uint64_t val,
{
DWC2State *s = ptr;
uint64_t orig = val;
- uint32_t reg = s->fszregbase + addr;
uint32_t *mmio;
uint32_t old;
- assert(reg <= HPTXFSIZ);
+ assert(addr <= HPTXFSIZ);
mmio = &s->fszreg[addr >> 2];
old = *mmio;
@@ -800,13 +799,12 @@ static const char *hreg0nm[] = {
static uint64_t dwc2_hreg0_read(void *ptr, hwaddr addr, unsigned size)
{
DWC2State *s = ptr;
- uint32_t reg = s->hreg0base + addr;
uint32_t val;
- assert(reg <= HPRT0);
+ assert(addr <= HPRT0);
val = s->hreg0[addr >> 2];
- switch (reg) {
+ switch (addr) {
case HFNUM:
val = (dwc2_get_frame_remaining(s) << HFNUM_FRREM_SHIFT) |
(s->hfnum << HFNUM_FRNUM_SHIFT);
@@ -825,17 +823,16 @@ static void dwc2_hreg0_write(void *ptr, hwaddr addr, uint64_t val,
DWC2State *s = ptr;
USBDevice *dev = s->uport.dev;
uint64_t orig = val;
- uint32_t reg = s->hreg0base + addr;
uint32_t *mmio;
uint32_t tval, told, old;
int prst = 0;
int iflg = 0;
- assert(reg <= HPRT0);
+ assert(addr <= HPRT0);
mmio = &s->hreg0[addr >> 2];
old = *mmio;
- switch (reg) {
+ switch (addr) {
case HFIR:
break;
case HFNUM:
@@ -915,14 +912,13 @@ static const char *hreg1nm[] = {
static uint64_t dwc2_hreg1_read(void *ptr, hwaddr addr, unsigned size)
{
DWC2State *s = ptr;
- uint32_t reg = s->hreg1base + addr;
uint32_t val;
- assert(reg <= HCDMAB(DWC2_NB_CHAN - 1));
+ assert(addr <= HCDMAB(DWC2_NB_CHAN - 1));
val = s->hreg1[addr >> 2];
trace_usb_dwc2_hreg1_read(addr, hreg1nm[(addr >> 2) & 7], addr >> 5, val);
- assert(s->hreg1base + (addr & 0x1c) <= HCDMAB(DWC2_NB_CHAN));
+ assert((addr & 0x1c) <= HCDMAB(DWC2_NB_CHAN));
return val;
}
@@ -931,18 +927,17 @@ static void dwc2_hreg1_write(void *ptr, hwaddr addr, uint64_t val,
{
DWC2State *s = ptr;
uint64_t orig = val;
- uint32_t reg = s->hreg1base + addr;
uint32_t *mmio;
uint32_t old;
int iflg = 0;
int enflg = 0;
int disflg = 0;
- assert(reg <= HCDMAB(DWC2_NB_CHAN - 1));
+ assert(addr <= HCDMAB(DWC2_NB_CHAN - 1));
mmio = &s->hreg1[addr >> 2];
old = *mmio;
- switch (s->hreg1base + (addr & 0x1c)) {
+ switch (addr & 0x1c) {
case HCCHAR(0):
if ((val & HCCHAR_CHDIS) && !(old & HCCHAR_CHDIS)) {
val &= ~(HCCHAR_CHENA | HCCHAR_CHDIS);
@@ -1002,10 +997,9 @@ static const char *pcgregnm[] = {
static uint64_t dwc2_pcgreg_read(void *ptr, hwaddr addr, unsigned size)
{
DWC2State *s = ptr;
- uint32_t reg = s->pcgregbase + addr;
uint32_t val;
- assert(reg <= PCGCCTL1);
+ assert(addr <= PCGCCTL1);
val = s->pcgreg[addr >> 2];
trace_usb_dwc2_pcgreg_read(addr, pcgregnm[addr >> 2], val);
@@ -1017,11 +1011,10 @@ static void dwc2_pcgreg_write(void *ptr, hwaddr addr, uint64_t val,
{
DWC2State *s = ptr;
uint64_t orig = val;
- uint32_t reg = s->pcgregbase + addr;
uint32_t *mmio;
uint32_t old;
- assert(reg <= PCGCCTL1);
+ assert(addr <= PCGCCTL1);
mmio = &s->pcgreg[addr >> 2];
old = *mmio;
@@ -1030,6 +1023,65 @@ static void dwc2_pcgreg_write(void *ptr, hwaddr addr, uint64_t val,
*mmio = val;
}
+static uint64_t dwc2_hsotg_read(void *ptr, hwaddr addr, unsigned size)
+{
+ uint64_t val;
+
+ switch (addr) {
+ case HSOTG_REG(0x000) ... HSOTG_REG(0x0fc):
+ val = dwc2_glbreg_read(ptr, addr - HSOTG_REG(0x000), size);
+ break;
+ case HSOTG_REG(0x100) ... HSOTG_REG(0x3fc):
+ val = dwc2_fszreg_read(ptr, addr - HSOTG_REG(0x100), size);
+ break;
+ case HSOTG_REG(0x400) ... HSOTG_REG(0x4fc):
+ val = dwc2_hreg0_read(ptr, addr - HSOTG_REG(0x400), size);
+ break;
+ case HSOTG_REG(0x500) ... HSOTG_REG(0xdfc):
+ val = dwc2_hreg1_read(ptr, addr - HSOTG_REG(0x500), size);
+ break;
+ case HSOTG_REG(0xe00) ... HSOTG_REG(0xffc):
+ val = dwc2_pcgreg_read(ptr, addr - HSOTG_REG(0xe00), size);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ return val;
+}
+
+static void dwc2_hsotg_write(void *ptr, hwaddr addr, uint64_t val,
+ unsigned size)
+{
+ switch (addr) {
+ case HSOTG_REG(0x000) ... HSOTG_REG(0x0fc):
+ dwc2_glbreg_write(ptr, addr - HSOTG_REG(0x000), val, size);
+ break;
+ case HSOTG_REG(0x100) ... HSOTG_REG(0x3fc):
+ dwc2_fszreg_write(ptr, addr - HSOTG_REG(0x100), val, size);
+ break;
+ case HSOTG_REG(0x400) ... HSOTG_REG(0x4fc):
+ dwc2_hreg0_write(ptr, addr - HSOTG_REG(0x400), val, size);
+ break;
+ case HSOTG_REG(0x500) ... HSOTG_REG(0xdfc):
+ dwc2_hreg1_write(ptr, addr - HSOTG_REG(0x500), val, size);
+ break;
+ case HSOTG_REG(0xe00) ... HSOTG_REG(0xffc):
+ dwc2_pcgreg_write(ptr, addr - HSOTG_REG(0xe00), val, size);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+}
+
+static const MemoryRegionOps dwc2_mmio_hsotg_ops = {
+ .read = dwc2_hsotg_read,
+ .write = dwc2_hsotg_write,
+ .valid.min_access_size = 4,
+ .valid.max_access_size = 4,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
static uint64_t dwc2_hreg2_read(void *ptr, hwaddr addr, unsigned size)
{
/* TODO - implement FIFOs to support slave mode */
@@ -1047,46 +1099,6 @@ static void dwc2_hreg2_write(void *ptr, hwaddr addr, uint64_t val,
trace_usb_dwc2_hreg2_write(addr, addr >> 12, orig, 0, val);
}
-static const MemoryRegionOps dwc2_mmio_glbreg_ops = {
- .read = dwc2_glbreg_read,
- .write = dwc2_glbreg_write,
- .valid.min_access_size = 4,
- .valid.max_access_size = 4,
- .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static const MemoryRegionOps dwc2_mmio_fszreg_ops = {
- .read = dwc2_fszreg_read,
- .write = dwc2_fszreg_write,
- .valid.min_access_size = 4,
- .valid.max_access_size = 4,
- .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static const MemoryRegionOps dwc2_mmio_hreg0_ops = {
- .read = dwc2_hreg0_read,
- .write = dwc2_hreg0_write,
- .valid.min_access_size = 4,
- .valid.max_access_size = 4,
- .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static const MemoryRegionOps dwc2_mmio_hreg1_ops = {
- .read = dwc2_hreg1_read,
- .write = dwc2_hreg1_write,
- .valid.min_access_size = 4,
- .valid.max_access_size = 4,
- .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static const MemoryRegionOps dwc2_mmio_pcgreg_ops = {
- .read = dwc2_pcgreg_read,
- .write = dwc2_pcgreg_write,
- .valid.min_access_size = 4,
- .valid.max_access_size = 4,
- .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
static const MemoryRegionOps dwc2_mmio_hreg2_ops = {
.read = dwc2_hreg2_read,
.write = dwc2_hreg2_write,
@@ -1219,7 +1231,7 @@ static void dwc2_realize(DeviceState *dev, Error **errp)
}
s->dma_mr = MEMORY_REGION(obj);
- address_space_init(&s->dma_as, s->dma_mr, NULL);
+ address_space_init(&s->dma_as, s->dma_mr, "dwc2");
usb_bus_new(&s->bus, sizeof(s->bus), &dwc2_bus_ops, dev);
usb_register_port(&s->bus, &s->uport, s, 0, &dwc2_port_ops,
@@ -1247,35 +1259,17 @@ static void dwc2_init(Object *obj)
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
DWC2State *s = DWC2_USB(obj);
- s->glbregbase = 0;
- s->fszregbase = 0x0100;
- s->hreg0base = 0x0400;
- s->hreg1base = 0x0500;
- s->pcgregbase = 0x0e00;
- s->hfifobase = 0x1000;
+ memory_region_init(&s->container, obj, "dwc2", DWC2_MMIO_SIZE);
+ sysbus_init_mmio(sbd, &s->container);
- memory_region_init(&s->mem, obj, "dwc2", DWC2_MMIO_SIZE);
- memory_region_init_io(&s->mem_glbreg, obj, &dwc2_mmio_glbreg_ops, s,
- "global", DWC2_GLBREG_SIZE);
- memory_region_init_io(&s->mem_fszreg, obj, &dwc2_mmio_fszreg_ops, s,
- "hptxfsiz", DWC2_FSZREG_SIZE);
- memory_region_init_io(&s->mem_hreg0, obj, &dwc2_mmio_hreg0_ops, s,
- "host", DWC2_HREG0_SIZE);
- memory_region_init_io(&s->mem_hreg1, obj, &dwc2_mmio_hreg1_ops, s,
- "host channels", DWC2_HREG1_SIZE);
- memory_region_init_io(&s->mem_pcgreg, obj, &dwc2_mmio_pcgreg_ops, s,
- "power/clock", DWC2_PCGREG_SIZE);
- memory_region_init_io(&s->mem_hreg2, obj, &dwc2_mmio_hreg2_ops, s,
- "host fifos", DWC2_HFIFO_SIZE);
+ memory_region_init_io(&s->hsotg, obj, &dwc2_mmio_hsotg_ops, s,
+ "dwc2-io", 4 * KiB);
+ memory_region_add_subregion(&s->container, 0x0000, &s->hsotg);
- memory_region_add_subregion(&s->mem, s->glbregbase, &s->mem_glbreg);
- memory_region_add_subregion(&s->mem, s->fszregbase, &s->mem_fszreg);
- memory_region_add_subregion(&s->mem, s->hreg0base, &s->mem_hreg0);
- memory_region_add_subregion(&s->mem, s->hreg1base, &s->mem_hreg1);
- memory_region_add_subregion(&s->mem, s->pcgregbase, &s->mem_pcgreg);
- memory_region_add_subregion(&s->mem, s->hfifobase, &s->mem_hreg2);
+ memory_region_init_io(&s->fifos, obj, &dwc2_mmio_hreg2_ops, s,
+ "dwc2-fifo", 64 * KiB);
+ memory_region_add_subregion(&s->container, 0x1000, &s->fifos);
- sysbus_init_mmio(sbd, &s->mem);
}
static const VMStateDescription vmstate_dwc2_state_packet = {
@@ -1303,13 +1297,6 @@ const VMStateDescription vmstate_dwc2_state = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
- VMSTATE_UINT16(glbregbase, DWC2State),
- VMSTATE_UINT16(fszregbase, DWC2State),
- VMSTATE_UINT16(hreg0base, DWC2State),
- VMSTATE_UINT16(hreg1base, DWC2State),
- VMSTATE_UINT16(pcgregbase, DWC2State),
- VMSTATE_UINT16(hfifobase, DWC2State),
-
VMSTATE_UINT32_ARRAY(glbreg, DWC2State,
DWC2_GLBREG_SIZE / sizeof(uint32_t)),
VMSTATE_UINT32_ARRAY(fszreg, DWC2State,
@@ -1343,6 +1330,8 @@ const VMStateDescription vmstate_dwc2_state = {
static Property dwc2_usb_properties[] = {
DEFINE_PROP_UINT32("usb_version", DWC2State, usb_version, 2),
+ /* FIXME isn't 'usb_version=2' const? */
+ DEFINE_PROP_END_OF_LIST()
};
static void dwc2_class_init(ObjectClass *klass, void *data)
--
2.21.1
On 4/21/20 12:13 AM, Philippe Mathieu-Daudé wrote:
> Use 1 container holding 2 regions:
> - I/O registers
> - FIFOs
>
> Remove all the static base addresses.
>
> Name address space.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Sometime a patch is cleaner/quicker than explanations.
> Suggestion to be squashed on patch:
> 'dwc-hsotg (dwc2) USB host controller emulation'
>
> Further simplificatio would be to move some exploded read/write
> functions directly into dwc2_hsotg_read/write.
>
> Based-on: <20200421014551.10426-1-pauldzim@gmail.com>
> ---
> hw/usb/hcd-dwc2.h | 17 +----
> hw/usb/hcd-dwc2.c | 181 ++++++++++++++++++++++------------------------
> 2 files changed, 88 insertions(+), 110 deletions(-)
<snip>
Ooh, yes, that looks much cleaner and simpler, thanks! I will test this
and fold it into the next version of the patch.
> static Property dwc2_usb_properties[] = {
> DEFINE_PROP_UINT32("usb_version", DWC2State, usb_version, 2),
> + /* FIXME isn't 'usb_version=2' const? */
This allows to switch between full-speed and high-speed at boot time,
which I've found very useful for testing purposes.
> + DEFINE_PROP_END_OF_LIST()
> };
>
> static void dwc2_class_init(ObjectClass *klass, void *data)
Thanks,
Paul
© 2016 - 2024 Red Hat, Inc.