From 102d7d1fba6d1121c86ef31c33b808a3104ab263 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:09 +0100 Subject: hw/gpio/pl061: Convert DPRINTF to tracepoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the use of the DPRINTF debug macro in the PL061 model to use tracepoints. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson --- hw/gpio/pl061.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index e72e77572a..a6ace88895 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -15,19 +15,7 @@ #include "qemu/log.h" #include "qemu/module.h" #include "qom/object.h" - -//#define DEBUG_PL061 1 - -#ifdef DEBUG_PL061 -#define DPRINTF(fmt, ...) \ -do { printf("pl061: " fmt , ## __VA_ARGS__); } while (0) -#define BADF(fmt, ...) \ -do { fprintf(stderr, "pl061: error: " fmt , ## __VA_ARGS__); exit(1);} while (0) -#else -#define DPRINTF(fmt, ...) do {} while(0) -#define BADF(fmt, ...) \ -do { fprintf(stderr, "pl061: error: " fmt , ## __VA_ARGS__);} while (0) -#endif +#include "trace.h" static const uint8_t pl061_id[12] = { 0x00, 0x00, 0x00, 0x00, 0x61, 0x10, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1 }; @@ -107,7 +95,7 @@ static void pl061_update(PL061State *s) uint8_t out; int i; - DPRINTF("dir = %d, data = %d\n", s->dir, s->data); + trace_pl061_update(DEVICE(s)->canonical_path, s->dir, s->data); /* Outputs float high. */ /* FIXME: This is board dependent. */ @@ -118,8 +106,9 @@ static void pl061_update(PL061State *s) for (i = 0; i < N_GPIOS; i++) { mask = 1 << i; if (changed & mask) { - DPRINTF("Set output %d = %d\n", i, (out & mask) != 0); - qemu_set_irq(s->out[i], (out & mask) != 0); + int level = (out & mask) != 0; + trace_pl061_set_output(DEVICE(s)->canonical_path, i, level); + qemu_set_irq(s->out[i], level); } } } @@ -131,7 +120,8 @@ static void pl061_update(PL061State *s) for (i = 0; i < N_GPIOS; i++) { mask = 1 << i; if (changed & mask) { - DPRINTF("Changed input %d = %d\n", i, (s->data & mask) != 0); + trace_pl061_input_change(DEVICE(s)->canonical_path, i, + (s->data & mask) != 0); if (!(s->isense & mask)) { /* Edge interrupt */ @@ -150,7 +140,8 @@ static void pl061_update(PL061State *s) /* Level interrupt */ s->istate |= ~(s->data ^ s->iev) & s->isense; - DPRINTF("istate = %02X\n", s->istate); + trace_pl061_update_istate(DEVICE(s)->canonical_path, + s->istate, s->im, (s->istate & s->im) != 0); qemu_set_irq(s->irq, (s->istate & s->im) != 0); } -- cgit 1.4.1 From e24a9f6a595cc4502b67046ea3860cae2be15b71 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:10 +0100 Subject: hw/gpio/pl061: Clean up read/write offset handling logic Currently the pl061_read() and pl061_write() functions handle offsets using a combination of three if() statements and a switch(). Clean this up to use just a switch, using case ranges. This requires that instead of catching accesses to the luminary-only registers on a stock PL061 via a check on s->rsvd_start we use an "is this luminary?" check in the cases for each luminary-only register. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/gpio/pl061.c | 104 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 79 insertions(+), 25 deletions(-) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index a6ace88895..b21b230402 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -55,7 +55,6 @@ struct PL061State { qemu_irq irq; qemu_irq out[N_GPIOS]; const unsigned char *id; - uint32_t rsvd_start; /* reserved area: [rsvd_start, 0xfcc] */ }; static const VMStateDescription vmstate_pl061 = { @@ -151,16 +150,9 @@ static uint64_t pl061_read(void *opaque, hwaddr offset, { PL061State *s = (PL061State *)opaque; - if (offset < 0x400) { - return s->data & (offset >> 2); - } - if (offset >= s->rsvd_start && offset <= 0xfcc) { - goto err_out; - } - if (offset >= 0xfd0 && offset < 0x1000) { - return s->id[(offset - 0xfd0) >> 2]; - } switch (offset) { + case 0x0 ... 0x3ff: /* Data */ + return s->data & (offset >> 2); case 0x400: /* Direction */ return s->dir; case 0x404: /* Interrupt sense */ @@ -178,33 +170,68 @@ static uint64_t pl061_read(void *opaque, hwaddr offset, case 0x420: /* Alternate function select */ return s->afsel; case 0x500: /* 2mA drive */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->dr2r; case 0x504: /* 4mA drive */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->dr4r; case 0x508: /* 8mA drive */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->dr8r; case 0x50c: /* Open drain */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->odr; case 0x510: /* Pull-up */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->pur; case 0x514: /* Pull-down */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->pdr; case 0x518: /* Slew rate control */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->slr; case 0x51c: /* Digital enable */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->den; case 0x520: /* Lock */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->locked; case 0x524: /* Commit */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->cr; case 0x528: /* Analog mode select */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } return s->amsel; + case 0xfd0 ... 0xfff: /* ID registers */ + return s->id[(offset - 0xfd0) >> 2]; default: + bad_offset: + qemu_log_mask(LOG_GUEST_ERROR, + "pl061_read: Bad offset %x\n", (int)offset); break; } -err_out: - qemu_log_mask(LOG_GUEST_ERROR, - "pl061_read: Bad offset %x\n", (int)offset); return 0; } @@ -214,16 +241,12 @@ static void pl061_write(void *opaque, hwaddr offset, PL061State *s = (PL061State *)opaque; uint8_t mask; - if (offset < 0x400) { + switch (offset) { + case 0 ... 0x3ff: mask = (offset >> 2) & s->dir; s->data = (s->data & ~mask) | (value & mask); pl061_update(s); return; - } - if (offset >= s->rsvd_start) { - goto err_out; - } - switch (offset) { case 0x400: /* Direction */ s->dir = value & 0xff; break; @@ -247,47 +270,80 @@ static void pl061_write(void *opaque, hwaddr offset, s->afsel = (s->afsel & ~mask) | (value & mask); break; case 0x500: /* 2mA drive */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->dr2r = value & 0xff; break; case 0x504: /* 4mA drive */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->dr4r = value & 0xff; break; case 0x508: /* 8mA drive */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->dr8r = value & 0xff; break; case 0x50c: /* Open drain */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->odr = value & 0xff; break; case 0x510: /* Pull-up */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->pur = value & 0xff; break; case 0x514: /* Pull-down */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->pdr = value & 0xff; break; case 0x518: /* Slew rate control */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->slr = value & 0xff; break; case 0x51c: /* Digital enable */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->den = value & 0xff; break; case 0x520: /* Lock */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->locked = (value != 0xacce551); break; case 0x524: /* Commit */ + if (s->id != pl061_id_luminary) { + goto bad_offset; + } if (!s->locked) s->cr = value & 0xff; break; case 0x528: + if (s->id != pl061_id_luminary) { + goto bad_offset; + } s->amsel = value & 0xff; break; default: - goto err_out; + bad_offset: + qemu_log_mask(LOG_GUEST_ERROR, + "pl061_write: Bad offset %x\n", (int)offset); + return; } pl061_update(s); return; -err_out: - qemu_log_mask(LOG_GUEST_ERROR, - "pl061_write: Bad offset %x\n", (int)offset); } static void pl061_reset(DeviceState *dev) @@ -343,7 +399,6 @@ static void pl061_luminary_init(Object *obj) PL061State *s = PL061(obj); s->id = pl061_id_luminary; - s->rsvd_start = 0x52c; } static void pl061_init(Object *obj) @@ -353,7 +408,6 @@ static void pl061_init(Object *obj) SysBusDevice *sbd = SYS_BUS_DEVICE(obj); s->id = pl061_id; - s->rsvd_start = 0x424; memory_region_init_io(&s->iomem, obj, &pl061_ops, s, "pl061", 0x1000); sysbus_init_mmio(sbd, &s->iomem); -- cgit 1.4.1 From 74d359b52db760f8818476f4fbaab0ffab76a8ef Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:11 +0100 Subject: hw/gpio/pl061: Add tracepoints for register read and write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tracepoints for reads and writes to the PL061 registers. This requires restructuring pl061_read() to only return after the tracepoint, rather than having lots of early-returns. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson --- hw/gpio/pl061.c | 70 +++++++++++++++++++++++++++++++++++----------------- hw/gpio/trace-events | 2 ++ 2 files changed, 50 insertions(+), 22 deletions(-) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index b21b230402..42f6e6c489 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -149,90 +149,114 @@ static uint64_t pl061_read(void *opaque, hwaddr offset, unsigned size) { PL061State *s = (PL061State *)opaque; + uint64_t r = 0; switch (offset) { case 0x0 ... 0x3ff: /* Data */ - return s->data & (offset >> 2); + r = s->data & (offset >> 2); + break; case 0x400: /* Direction */ - return s->dir; + r = s->dir; + break; case 0x404: /* Interrupt sense */ - return s->isense; + r = s->isense; + break; case 0x408: /* Interrupt both edges */ - return s->ibe; + r = s->ibe; + break; case 0x40c: /* Interrupt event */ - return s->iev; + r = s->iev; + break; case 0x410: /* Interrupt mask */ - return s->im; + r = s->im; + break; case 0x414: /* Raw interrupt status */ - return s->istate; + r = s->istate; + break; case 0x418: /* Masked interrupt status */ - return s->istate & s->im; + r = s->istate & s->im; + break; case 0x420: /* Alternate function select */ - return s->afsel; + r = s->afsel; + break; case 0x500: /* 2mA drive */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->dr2r; + r = s->dr2r; + break; case 0x504: /* 4mA drive */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->dr4r; + r = s->dr4r; + break; case 0x508: /* 8mA drive */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->dr8r; + r = s->dr8r; + break; case 0x50c: /* Open drain */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->odr; + r = s->odr; + break; case 0x510: /* Pull-up */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->pur; + r = s->pur; + break; case 0x514: /* Pull-down */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->pdr; + r = s->pdr; + break; case 0x518: /* Slew rate control */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->slr; + r = s->slr; + break; case 0x51c: /* Digital enable */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->den; + r = s->den; + break; case 0x520: /* Lock */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->locked; + r = s->locked; + break; case 0x524: /* Commit */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->cr; + r = s->cr; + break; case 0x528: /* Analog mode select */ if (s->id != pl061_id_luminary) { goto bad_offset; } - return s->amsel; + r = s->amsel; + break; case 0xfd0 ... 0xfff: /* ID registers */ - return s->id[(offset - 0xfd0) >> 2]; + r = s->id[(offset - 0xfd0) >> 2]; + break; default: bad_offset: qemu_log_mask(LOG_GUEST_ERROR, "pl061_read: Bad offset %x\n", (int)offset); break; } - return 0; + + trace_pl061_read(DEVICE(s)->canonical_path, offset, r); + return r; } static void pl061_write(void *opaque, hwaddr offset, @@ -241,6 +265,8 @@ static void pl061_write(void *opaque, hwaddr offset, PL061State *s = (PL061State *)opaque; uint8_t mask; + trace_pl061_write(DEVICE(s)->canonical_path, offset, value); + switch (offset) { case 0 ... 0x3ff: mask = (offset >> 2) & s->dir; diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events index 48ccbb183c..442be9406f 100644 --- a/hw/gpio/trace-events +++ b/hw/gpio/trace-events @@ -18,6 +18,8 @@ pl061_update(const char *id, uint32_t dir, uint32_t data) "%s GPIODIR 0x%x GPIOD pl061_set_output(const char *id, int gpio, int level) "%s setting output %d to %d" pl061_input_change(const char *id, int gpio, int level) "%s input %d changed to %d" pl061_update_istate(const char *id, uint32_t istate, uint32_t im, int level) "%s GPIORIS 0x%x GPIOIE 0x%x interrupt level %d" +pl061_read(const char *id, uint64_t offset, uint64_t r) "%s offset 0x%" PRIx64 " value 0x%" PRIx64 +pl061_write(const char *id, uint64_t offset, uint64_t value) "%s offset 0x%" PRIx64 " value 0x%" PRIx64 # sifive_gpio.c sifive_gpio_read(uint64_t offset, uint64_t r) "offset 0x%" PRIx64 " value 0x%" PRIx64 -- cgit 1.4.1 From 455736df2cfd3a980782986d597132776d630823 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:12 +0100 Subject: hw/gpio/pl061: Document the interface of this device Add a comment documenting the "QEMU interface" of this device: which MMIO regions, IRQ lines, GPIO lines, etc it exposes. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/gpio/pl061.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index 42f6e6c489..a3c1386221 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -6,6 +6,13 @@ * Written by Paul Brook * * This code is licensed under the GPL. + * + * QEMU interface: + * + sysbus MMIO region 0: the device registers + * + sysbus IRQ: the GPIOINTR interrupt line + * + unnamed GPIO inputs 0..7: inputs to connect to the emulated GPIO lines + * + unnamed GPIO outputs 0..7: the emulated GPIO lines, considered as + * outputs */ #include "qemu/osdep.h" -- cgit 1.4.1 From ad06d56fc7155c7893b18efecb9fe0f2e9124eaf Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:13 +0100 Subject: hw/gpio/pl061: Honour Luminary PL061 PUR and PDR registers The Luminary variant of the PL061 has registers GPIOPUR and GPIOPDR which lets the guest configure whether the GPIO lines are pull-up, pull-down, or truly floating. Instead of assuming all lines are pulled high, honour the PUR and PDR registers. For the plain PL061, continue to assume that lines have an external pull-up resistor, as we did before. The stellaris board actually relies on this behaviour -- the CD line of the ssd0323 display device is connected to GPIO output C7, and it is only because of a different bug which we're about to fix that we weren't incorrectly driving this line high on reset and putting the ssd0323 into data mode. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/gpio/pl061.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++----- hw/gpio/trace-events | 2 +- 2 files changed, 57 insertions(+), 7 deletions(-) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index a3c1386221..9360c143ee 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -94,18 +94,68 @@ static const VMStateDescription vmstate_pl061 = { } }; +static uint8_t pl061_floating(PL061State *s) +{ + /* + * Return mask of bits which correspond to pins configured as inputs + * and which are floating (neither pulled up to 1 nor down to 0). + */ + uint8_t floating; + + if (s->id == pl061_id_luminary) { + /* + * If both PUR and PDR bits are clear, there is neither a pullup + * nor a pulldown in place, and the output truly floats. + */ + floating = ~(s->pur | s->pdr); + } else { + /* Assume outputs are pulled high. FIXME: this is board dependent. */ + floating = 0; + } + return floating & ~s->dir; +} + +static uint8_t pl061_pullups(PL061State *s) +{ + /* + * Return mask of bits which correspond to pins configured as inputs + * and which are pulled up to 1. + */ + uint8_t pullups; + + if (s->id == pl061_id_luminary) { + /* + * The Luminary variant of the PL061 has an extra registers which + * the guest can use to configure whether lines should be pullup + * or pulldown. + */ + pullups = s->pur; + } else { + /* Assume outputs are pulled high. FIXME: this is board dependent. */ + pullups = 0xff; + } + return pullups & ~s->dir; +} + static void pl061_update(PL061State *s) { uint8_t changed; uint8_t mask; uint8_t out; int i; - - trace_pl061_update(DEVICE(s)->canonical_path, s->dir, s->data); - - /* Outputs float high. */ - /* FIXME: This is board dependent. */ - out = (s->data & s->dir) | ~s->dir; + uint8_t pullups = pl061_pullups(s); + uint8_t floating = pl061_floating(s); + + trace_pl061_update(DEVICE(s)->canonical_path, s->dir, s->data, + pullups, floating); + + /* + * Pins configured as output are driven from the data register; + * otherwise if they're pulled up they're 1, and if they're floating + * then we give them the same value they had previously, so we don't + * report any change to the other end. + */ + out = (s->data & s->dir) | pullups | (s->old_out_data & floating); changed = s->old_out_data ^ out; if (changed) { s->old_out_data = out; diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events index 442be9406f..eb5fb4701c 100644 --- a/hw/gpio/trace-events +++ b/hw/gpio/trace-events @@ -14,7 +14,7 @@ nrf51_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64 nrf51_gpio_update_output_irq(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64 # pl061.c -pl061_update(const char *id, uint32_t dir, uint32_t data) "%s GPIODIR 0x%x GPIODATA 0x%x" +pl061_update(const char *id, uint32_t dir, uint32_t data, uint32_t pullups, uint32_t floating) "%s GPIODIR 0x%x GPIODATA 0x%x pullups 0x%x floating 0x%x" pl061_set_output(const char *id, int gpio, int level) "%s setting output %d to %d" pl061_input_change(const char *id, int gpio, int level) "%s input %d changed to %d" pl061_update_istate(const char *id, uint32_t istate, uint32_t im, int level) "%s GPIORIS 0x%x GPIOIE 0x%x interrupt level %d" -- cgit 1.4.1 From c1e69e92aea696fa148c4d79aff6a2fdf46ef2b8 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:14 +0100 Subject: hw/gpio/pl061: Make pullup/pulldown of outputs configurable The PL061 GPIO does not itself include pullup or pulldown resistors to set the value of a GPIO line treated as an output when it is configured as an input (ie when the PL061 itself is not driving it). In real hardware it is up to the board to add suitable pullups or pulldowns. Currently our implementation hardwires this to "outputs pulled high", which is correct for some boards (eg the realview ones: see figure 3-29 in the "RealView Platform Baseboard for ARM926EJ-S User Guide" DUI0224I), but wrong for others. In particular, the wiring in the 'virt' board and the gpio-pwr device assumes that wires should be pulled low, because otherwise the pull-to-high will trigger a shutdown or reset action. (The only reason this doesn't happen immediately on startup is due to another bug in the PL061, where we don't assert the GPIOs to the correct value on reset, but will do so as soon as the guest touches a register and pl061_update() gets called.) Add properties to the pl061 so the board can configure whether it wants GPIO lines to have pullup, pulldown, or neither. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/gpio/pl061.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 4 deletions(-) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index 9360c143ee..5ba398fcd4 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -13,12 +13,28 @@ * + unnamed GPIO inputs 0..7: inputs to connect to the emulated GPIO lines * + unnamed GPIO outputs 0..7: the emulated GPIO lines, considered as * outputs + * + QOM property "pullups": an integer defining whether non-floating lines + * configured as inputs should be pulled up to logical 1 (ie whether in + * real hardware they have a pullup resistor on the line out of the PL061). + * This should be an 8-bit value, where bit 0 is 1 if GPIO line 0 should + * be pulled high, bit 1 configures line 1, and so on. The default is 0xff, + * indicating that all GPIO lines are pulled up to logical 1. + * + QOM property "pulldowns": an integer defining whether non-floating lines + * configured as inputs should be pulled down to logical 0 (ie whether in + * real hardware they have a pulldown resistor on the line out of the PL061). + * This should be an 8-bit value, where bit 0 is 1 if GPIO line 0 should + * be pulled low, bit 1 configures line 1, and so on. The default is 0x0. + * It is an error to set a bit in both "pullups" and "pulldowns". If a bit + * is 0 in both, then the line is considered to be floating, and it will + * not have qemu_set_irq() called on it when it is configured as an input. */ #include "qemu/osdep.h" #include "hw/irq.h" #include "hw/sysbus.h" +#include "hw/qdev-properties.h" #include "migration/vmstate.h" +#include "qapi/error.h" #include "qemu/log.h" #include "qemu/module.h" #include "qom/object.h" @@ -62,6 +78,9 @@ struct PL061State { qemu_irq irq; qemu_irq out[N_GPIOS]; const unsigned char *id; + /* Properties, for non-Luminary PL061 */ + uint32_t pullups; + uint32_t pulldowns; }; static const VMStateDescription vmstate_pl061 = { @@ -109,8 +128,7 @@ static uint8_t pl061_floating(PL061State *s) */ floating = ~(s->pur | s->pdr); } else { - /* Assume outputs are pulled high. FIXME: this is board dependent. */ - floating = 0; + floating = ~(s->pullups | s->pulldowns); } return floating & ~s->dir; } @@ -131,8 +149,7 @@ static uint8_t pl061_pullups(PL061State *s) */ pullups = s->pur; } else { - /* Assume outputs are pulled high. FIXME: this is board dependent. */ - pullups = 0xff; + pullups = s->pullups; } return pullups & ~s->dir; } @@ -499,12 +516,38 @@ static void pl061_init(Object *obj) qdev_init_gpio_out(dev, s->out, N_GPIOS); } +static void pl061_realize(DeviceState *dev, Error **errp) +{ + PL061State *s = PL061(dev); + + if (s->pullups > 0xff) { + error_setg(errp, "pullups property must be between 0 and 0xff"); + return; + } + if (s->pulldowns > 0xff) { + error_setg(errp, "pulldowns property must be between 0 and 0xff"); + return; + } + if (s->pullups & s->pulldowns) { + error_setg(errp, "no bit may be set both in pullups and pulldowns"); + return; + } +} + +static Property pl061_props[] = { + DEFINE_PROP_UINT32("pullups", PL061State, pullups, 0xff), + DEFINE_PROP_UINT32("pulldowns", PL061State, pulldowns, 0x0), + DEFINE_PROP_END_OF_LIST() +}; + static void pl061_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->vmsd = &vmstate_pl061; dc->reset = &pl061_reset; + dc->realize = pl061_realize; + device_class_set_props(dc, pl061_props); } static const TypeInfo pl061_info = { -- cgit 1.4.1 From ef4989b0a898ae20a974d261b14d4e5c1c097292 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:16 +0100 Subject: hw/gpio/pl061: Convert to 3-phase reset and assert GPIO lines correctly on reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PL061 comes out of reset with all its lines configured as input, which means they might need to be pulled to 0 or 1 depending on the 'pullups' and 'pulldowns' properties. Currently we do not assert these lines on reset; they will only be set whenever the guest first touches a register that triggers a call to pl061_update(). Convert the device to three-phase reset so we have a place where we can safely call qemu_set_irq() to set the floating lines to their correct values. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson --- hw/gpio/pl061.c | 29 +++++++++++++++++++++++++---- hw/gpio/trace-events | 1 + 2 files changed, 26 insertions(+), 4 deletions(-) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index 5ba398fcd4..4002ab5154 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -446,13 +446,14 @@ static void pl061_write(void *opaque, hwaddr offset, return; } -static void pl061_reset(DeviceState *dev) +static void pl061_enter_reset(Object *obj, ResetType type) { - PL061State *s = PL061(dev); + PL061State *s = PL061(obj); + + trace_pl061_reset(DEVICE(s)->canonical_path); /* reset values from PL061 TRM, Stellaris LM3S5P31 & LM3S8962 Data Sheet */ s->data = 0; - s->old_out_data = 0; s->old_in_data = 0; s->dir = 0; s->isense = 0; @@ -474,6 +475,24 @@ static void pl061_reset(DeviceState *dev) s->amsel = 0; } +static void pl061_hold_reset(Object *obj) +{ + PL061State *s = PL061(obj); + int i, level; + uint8_t floating = pl061_floating(s); + uint8_t pullups = pl061_pullups(s); + + for (i = 0; i < N_GPIOS; i++) { + if (extract32(floating, i, 1)) { + continue; + } + level = extract32(pullups, i, 1); + trace_pl061_set_output(DEVICE(s)->canonical_path, i, level); + qemu_set_irq(s->out[i], level); + } + s->old_out_data = pullups; +} + static void pl061_set_irq(void * opaque, int irq, int level) { PL061State *s = (PL061State *)opaque; @@ -543,11 +562,13 @@ static Property pl061_props[] = { static void pl061_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); + ResettableClass *rc = RESETTABLE_CLASS(klass); dc->vmsd = &vmstate_pl061; - dc->reset = &pl061_reset; dc->realize = pl061_realize; device_class_set_props(dc, pl061_props); + rc->phases.enter = pl061_enter_reset; + rc->phases.hold = pl061_hold_reset; } static const TypeInfo pl061_info = { diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events index eb5fb4701c..1dab99c560 100644 --- a/hw/gpio/trace-events +++ b/hw/gpio/trace-events @@ -20,6 +20,7 @@ pl061_input_change(const char *id, int gpio, int level) "%s input %d changed to pl061_update_istate(const char *id, uint32_t istate, uint32_t im, int level) "%s GPIORIS 0x%x GPIOIE 0x%x interrupt level %d" pl061_read(const char *id, uint64_t offset, uint64_t r) "%s offset 0x%" PRIx64 " value 0x%" PRIx64 pl061_write(const char *id, uint64_t offset, uint64_t value) "%s offset 0x%" PRIx64 " value 0x%" PRIx64 +pl061_reset(const char *id) "%s reset" # sifive_gpio.c sifive_gpio_read(uint64_t offset, uint64_t r) "offset 0x%" PRIx64 " value 0x%" PRIx64 -- cgit 1.4.1 From 0642e159d2351a8fd7d03f78b5d97010cd514561 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Jul 2021 11:40:17 +0100 Subject: hw/gpio/pl061: Document a shortcoming in our implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Luminary PL061s in the Stellaris LM3S9695 don't all have the same reset value for GPIOPUR. We can get away with not letting the board configure the PUR reset value because we don't actually wire anything up to the lines which should reset to pull-up. Add a comment noting this omission. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé --- hw/gpio/pl061.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'hw/gpio/pl061.c') diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index 4002ab5154..899be861cc 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -453,6 +453,15 @@ static void pl061_enter_reset(Object *obj, ResetType type) trace_pl061_reset(DEVICE(s)->canonical_path); /* reset values from PL061 TRM, Stellaris LM3S5P31 & LM3S8962 Data Sheet */ + + /* + * FIXME: For the LM3S6965, not all of the PL061 instances have the + * same reset values for GPIOPUR, GPIOAFSEL and GPIODEN, so in theory + * we should allow the board to configure these via properties. + * In practice, we don't wire anything up to the affected GPIO lines + * (PB7, PC0, PC1, PC2, PC3 -- they're used for JTAG), so we can + * get away with this inaccuracy. + */ s->data = 0; s->old_in_data = 0; s->dir = 0; -- cgit 1.4.1