Add some tracepoints to the bcm2835_sdhost driver, to assist
debugging.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/sd/bcm2835_sdhost.c | 10 ++++++++++
hw/sd/trace-events | 6 ++++++
2 files changed, 16 insertions(+)
diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
index f7f4e656df..79f3c5ceeb 100644
--- a/hw/sd/bcm2835_sdhost.c
+++ b/hw/sd/bcm2835_sdhost.c
@@ -15,6 +15,7 @@
#include "qemu/log.h"
#include "sysemu/blockdev.h"
#include "hw/sd/bcm2835_sdhost.h"
+#include "trace.h"
#define TYPE_BCM2835_SDHOST_BUS "bcm2835-sdhost-bus"
#define BCM2835_SDHOST_BUS(obj) \
@@ -99,6 +100,7 @@ static void bcm2835_sdhost_update_irq(BCM2835SDHostState *s)
{
uint32_t irq = s->status &
(SDHSTS_BUSY_IRPT | SDHSTS_BLOCK_IRPT | SDHSTS_SDIO_IRPT);
+ trace_bcm2835_sdhost_update_irq(irq);
qemu_set_irq(s->irq, !!irq);
}
@@ -211,6 +213,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
s->edm &= ~0xf;
s->edm |= SDEDM_FSM_DATAMODE;
+ trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm);
if (s->config & SDHCFG_DATA_IRPT_EN) {
s->status |= SDHSTS_SDIO_IRPT;
@@ -229,6 +232,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
s->edm &= ~(0x1f << 4);
s->edm |= ((s->fifo_len & 0x1f) << 4);
+ trace_bcm2835_sdhost_edm_change("fifo run", s->edm);
}
static uint64_t bcm2835_sdhost_read(void *opaque, hwaddr offset,
@@ -280,6 +284,8 @@ static uint64_t bcm2835_sdhost_read(void *opaque, hwaddr offset,
break;
}
+ trace_bcm2835_sdhost_read(offset, res, size);
+
return res;
}
@@ -288,6 +294,8 @@ static void bcm2835_sdhost_write(void *opaque, hwaddr offset,
{
BCM2835SDHostState *s = (BCM2835SDHostState *)opaque;
+ trace_bcm2835_sdhost_write(offset, value, size);
+
switch (offset) {
case SDCMD:
s->cmd = value;
@@ -314,6 +322,7 @@ static void bcm2835_sdhost_write(void *opaque, hwaddr offset,
value &= ~0xf;
}
s->edm = value;
+ trace_bcm2835_sdhost_edm_change("guest register write", s->edm);
break;
case SDHCFG:
s->config = value;
@@ -390,6 +399,7 @@ static void bcm2835_sdhost_reset(DeviceState *dev)
s->cmd = 0;
s->cmdarg = 0;
s->edm = 0x0000c60f;
+ trace_bcm2835_sdhost_edm_change("device reset", s->edm);
s->config = 0;
s->hbct = 0;
s->hblc = 0;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 2059ace61f..bfd1d62efc 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -1,5 +1,11 @@
# See docs/devel/tracing.txt for syntax documentation.
+# hw/sd/bcm2835_sdhost.c
+bcm2835_sdhost_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
+bcm2835_sdhost_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
+bcm2835_sdhost_edm_change(const char *why, uint32_t edm) "(%s) EDM now 0x%x"
+bcm2835_sdhost_update_irq(uint32_t irq) "IRQ bits 0x%x\n"
+
# hw/sd/core.c
sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg, uint8_t crc) "@%s CMD%02d arg 0x%08x crc 0x%02x"
sdbus_read(const char *bus_name, uint8_t value) "@%s value 0x%02x"
--
2.16.2
Hi Peter,
On 03/19/2018 01:15 PM, Peter Maydell wrote:
> Add some tracepoints to the bcm2835_sdhost driver, to assist
> debugging.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/sd/bcm2835_sdhost.c | 10 ++++++++++
> hw/sd/trace-events | 6 ++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
> index f7f4e656df..79f3c5ceeb 100644
> --- a/hw/sd/bcm2835_sdhost.c
> +++ b/hw/sd/bcm2835_sdhost.c
> @@ -15,6 +15,7 @@
> #include "qemu/log.h"
> #include "sysemu/blockdev.h"
> #include "hw/sd/bcm2835_sdhost.h"
> +#include "trace.h"
>
> #define TYPE_BCM2835_SDHOST_BUS "bcm2835-sdhost-bus"
> #define BCM2835_SDHOST_BUS(obj) \
> @@ -99,6 +100,7 @@ static void bcm2835_sdhost_update_irq(BCM2835SDHostState *s)
> {
> uint32_t irq = s->status &
> (SDHSTS_BUSY_IRPT | SDHSTS_BLOCK_IRPT | SDHSTS_SDIO_IRPT);
> + trace_bcm2835_sdhost_update_irq(irq);
> qemu_set_irq(s->irq, !!irq);
> }
>
> @@ -211,6 +213,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
>
> s->edm &= ~0xf;
> s->edm |= SDEDM_FSM_DATAMODE;
> + trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm);
>
> if (s->config & SDHCFG_DATA_IRPT_EN) {
> s->status |= SDHSTS_SDIO_IRPT;
> @@ -229,6 +232,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
>
> s->edm &= ~(0x1f << 4);
> s->edm |= ((s->fifo_len & 0x1f) << 4);
> + trace_bcm2835_sdhost_edm_change("fifo run", s->edm);
> }
>
> static uint64_t bcm2835_sdhost_read(void *opaque, hwaddr offset,
> @@ -280,6 +284,8 @@ static uint64_t bcm2835_sdhost_read(void *opaque, hwaddr offset,
> break;
> }
>
> + trace_bcm2835_sdhost_read(offset, res, size);
> +
> return res;
> }
>
> @@ -288,6 +294,8 @@ static void bcm2835_sdhost_write(void *opaque, hwaddr offset,
> {
> BCM2835SDHostState *s = (BCM2835SDHostState *)opaque;
>
> + trace_bcm2835_sdhost_write(offset, value, size);
> +
> switch (offset) {
> case SDCMD:
> s->cmd = value;
> @@ -314,6 +322,7 @@ static void bcm2835_sdhost_write(void *opaque, hwaddr offset,
> value &= ~0xf;
> }
> s->edm = value;
> + trace_bcm2835_sdhost_edm_change("guest register write", s->edm);
> break;
> case SDHCFG:
> s->config = value;
> @@ -390,6 +399,7 @@ static void bcm2835_sdhost_reset(DeviceState *dev)
> s->cmd = 0;
> s->cmdarg = 0;
> s->edm = 0x0000c60f;
> + trace_bcm2835_sdhost_edm_change("device reset", s->edm);
> s->config = 0;
> s->hbct = 0;
> s->hblc = 0;
> diff --git a/hw/sd/trace-events b/hw/sd/trace-events
> index 2059ace61f..bfd1d62efc 100644
> --- a/hw/sd/trace-events
> +++ b/hw/sd/trace-events
> @@ -1,5 +1,11 @@
> # See docs/devel/tracing.txt for syntax documentation.
>
> +# hw/sd/bcm2835_sdhost.c
> +bcm2835_sdhost_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
> +bcm2835_sdhost_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
Can you use the more explicit "size_t" and "%zu" please?
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> +bcm2835_sdhost_edm_change(const char *why, uint32_t edm) "(%s) EDM now 0x%x"
> +bcm2835_sdhost_update_irq(uint32_t irq) "IRQ bits 0x%x\n"
> +
> # hw/sd/core.c
> sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg, uint8_t crc) "@%s CMD%02d arg 0x%08x crc 0x%02x"
> sdbus_read(const char *bus_name, uint8_t value) "@%s value 0x%02x"
>
On 4 April 2018 at 12:42, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Peter, > > On 03/19/2018 01:15 PM, Peter Maydell wrote: >> Add some tracepoints to the bcm2835_sdhost driver, to assist >> debugging. >> +# hw/sd/bcm2835_sdhost.c >> +bcm2835_sdhost_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" >> +bcm2835_sdhost_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" > > Can you use the more explicit "size_t" and "%zu" please? The argument to the read/write functions which we're tracing here isn't a size_t, though (and since it's only ever 1/2/4/8 it makes sense that it isn't a size_t). What would be the point in casting it to an size_t here? A quick grep through hw/*/trace-events suggests we don't use size_t for tracing of mmio read/write functions in other devices. thanks -- PMM
On 04/04/2018 08:54 AM, Peter Maydell wrote: > On 4 April 2018 at 12:42, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> Hi Peter, >> >> On 03/19/2018 01:15 PM, Peter Maydell wrote: >>> Add some tracepoints to the bcm2835_sdhost driver, to assist >>> debugging. > >>> +# hw/sd/bcm2835_sdhost.c >>> +bcm2835_sdhost_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" >>> +bcm2835_sdhost_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" >> >> Can you use the more explicit "size_t" and "%zu" please? > > The argument to the read/write functions which we're tracing > here isn't a size_t, though (and since it's only ever 1/2/4/8 > it makes sense that it isn't a size_t). What would be the > point in casting it to an size_t here?> > A quick grep through hw/*/trace-events suggests we don't > use size_t for tracing of mmio read/write functions > in other devices. Oh... I find using 'unsigned' confusing, but that's fine this way. Regards, Phil.
© 2016 - 2025 Red Hat, Inc.