| Message ID | 20171213195852.30439-12-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > zero-initialize ADMADescr 'dscr' in sdhci_do_adma() to avoid: > > hw/sd/sdhci.c: In function ‘sdhci_do_adma’: > hw/sd/sdhci.c:714:29: error: ‘dscr.addr’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > trace_sdhci_adma("link", s->admasysaddr); > ^ > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Acked-by: Alistair Francis <alistair.francis@xilinx.com> Alistair > --- > hw/sd/sdhci.c | 89 ++++++++++++++++++------------------------------------ > hw/sd/trace-events | 14 +++++++++ > 2 files changed, 44 insertions(+), 59 deletions(-) > > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index 8fcd48f849..aa2d2fa3d3 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -33,30 +33,7 @@ > #include "sd-internal.h" > #include "qapi/error.h" > #include "qemu/log.h" > - > -/* host controller debug messages */ > -#ifndef SDHC_DEBUG > -#define SDHC_DEBUG 0 > -#endif > - > -#define DPRINT_L1(fmt, args...) \ > - do { \ > - if (SDHC_DEBUG) { \ > - fprintf(stderr, "QEMU SDHC: " fmt, ## args); \ > - } \ > - } while (0) > -#define DPRINT_L2(fmt, args...) \ > - do { \ > - if (SDHC_DEBUG > 1) { \ > - fprintf(stderr, "QEMU SDHC: " fmt, ## args); \ > - } \ > - } while (0) > -#define ERRPRINT(fmt, args...) \ > - do { \ > - if (SDHC_DEBUG) { \ > - fprintf(stderr, "QEMU SDHC ERROR: " fmt, ## args); \ > - } \ > - } while (0) > +#include "trace.h" > > #define TYPE_SDHCI_BUS "sdhci-bus" > #define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS) > @@ -154,8 +131,8 @@ static void sdhci_raise_insertion_irq(void *opaque) > static void sdhci_set_inserted(DeviceState *dev, bool level) > { > SDHCIState *s = (SDHCIState *)dev; > - DPRINT_L1("Card state changed: %s!\n", level ? "insert" : "eject"); > > + trace_sdhci_set_inserted(level ? "insert" : "eject"); > if ((s->norintsts & SDHC_NIS_REMOVE) && level) { > /* Give target some time to notice card ejection */ > timer_mod(s->insert_timer, > @@ -237,7 +214,8 @@ static void sdhci_send_command(SDHCIState *s) > s->acmd12errsts = 0; > request.cmd = s->cmdreg >> 8; > request.arg = s->argument; > - DPRINT_L1("sending CMD%u ARG[0x%08x]\n", request.cmd, request.arg); > + > + trace_sdhci_send_command(request.cmd, request.arg); > rlen = sdbus_do_command(&s->sdbus, &request, response); > > if (s->cmdreg & SDHC_CMD_RESPONSE) { > @@ -245,7 +223,7 @@ static void sdhci_send_command(SDHCIState *s) > s->rspreg[0] = (response[0] << 24) | (response[1] << 16) | > (response[2] << 8) | response[3]; > s->rspreg[1] = s->rspreg[2] = s->rspreg[3] = 0; > - DPRINT_L1("Response: RSPREG[31..0]=0x%08x\n", s->rspreg[0]); > + trace_sdhci_response4(s->rspreg[0]); > } else if (rlen == 16) { > s->rspreg[0] = (response[11] << 24) | (response[12] << 16) | > (response[13] << 8) | response[14]; > @@ -255,11 +233,10 @@ static void sdhci_send_command(SDHCIState *s) > (response[5] << 8) | response[6]; > s->rspreg[3] = (response[0] << 16) | (response[1] << 8) | > response[2]; > - DPRINT_L1("Response received:\n RSPREG[127..96]=0x%08x, RSPREG[95.." > - "64]=0x%08x,\n RSPREG[63..32]=0x%08x, RSPREG[31..0]=0x%08x\n", > - s->rspreg[3], s->rspreg[2], s->rspreg[1], s->rspreg[0]); > + trace_sdhci_response16(s->rspreg[3], s->rspreg[2], > + s->rspreg[1], s->rspreg[0]); > } else { > - ERRPRINT("Timeout waiting for command response\n"); > + trace_sdhci_error("timeout waiting for command response"); > if (s->errintstsen & SDHC_EISEN_CMDTIMEOUT) { > s->errintsts |= SDHC_EIS_CMDTIMEOUT; > s->norintsts |= SDHC_NIS_ERR; > @@ -293,7 +270,7 @@ static void sdhci_end_transfer(SDHCIState *s) > > request.cmd = 0x0C; > request.arg = 0; > - DPRINT_L1("Automatically issue CMD%d %08x\n", request.cmd, request.arg); > + trace_sdhci_end_transfer(request.cmd, request.arg); > sdbus_do_command(&s->sdbus, &request, response); > /* Auto CMD12 response goes to the upper Response register */ > s->rspreg[3] = (response[0] << 24) | (response[1] << 16) | > @@ -362,7 +339,7 @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size) > > /* first check that a valid data exists in host controller input buffer */ > if ((s->prnsts & SDHC_DATA_AVAILABLE) == 0) { > - ERRPRINT("Trying to read from empty buffer\n"); > + trace_sdhci_error("read from empty buffer"); > return 0; > } > > @@ -371,8 +348,7 @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size) > s->data_count++; > /* check if we've read all valid data (blksize bytes) from buffer */ > if ((s->data_count) >= (s->blksize & 0x0fff)) { > - DPRINT_L2("All %u bytes of data have been read from input buffer\n", > - s->data_count); > + trace_sdhci_read_dataport(s->data_count); > s->prnsts &= ~SDHC_DATA_AVAILABLE; /* no more data in a buffer */ > s->data_count = 0; /* next buff read must start at position [0] */ > > @@ -455,7 +431,7 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size) > > /* Check that there is free space left in a buffer */ > if (!(s->prnsts & SDHC_SPACE_AVAILABLE)) { > - ERRPRINT("Can't write to data buffer: buffer full\n"); > + trace_sdhci_error("Can't write to data buffer: buffer full"); > return; > } > > @@ -464,8 +440,7 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size) > s->data_count++; > value >>= 8; > if (s->data_count >= (s->blksize & 0x0fff)) { > - DPRINT_L2("write buffer filled with %u bytes of data\n", > - s->data_count); > + trace_sdhci_write_dataport(s->data_count); > s->data_count = 0; > s->prnsts &= ~SDHC_SPACE_AVAILABLE; > if (s->prnsts & SDHC_DOING_WRITE) { > @@ -644,15 +619,14 @@ static void sdhci_do_adma(SDHCIState *s) > { > unsigned int n, begin, length; > const uint16_t block_size = s->blksize & 0x0fff; > - ADMADescr dscr; > + ADMADescr dscr = {}; > int i; > > for (i = 0; i < SDHC_ADMA_DESCS_PER_DELAY; ++i) { > s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH; > > get_adma_description(s, &dscr); > - DPRINT_L2("ADMA loop: addr=" TARGET_FMT_plx ", len=%d, attr=%x\n", > - dscr.addr, dscr.length, dscr.attr); > + trace_sdhci_adma_loop(dscr.addr, dscr.length, dscr.attr); > > if ((dscr.attr & SDHC_ADMA_ATTR_VALID) == 0) { > /* Indicate that error occurred in ST_FDS state */ > @@ -735,8 +709,7 @@ static void sdhci_do_adma(SDHCIState *s) > break; > case SDHC_ADMA_ATTR_ACT_LINK: /* link to next descriptor table */ > s->admasysaddr = dscr.addr; > - DPRINT_L1("ADMA link: admasysaddr=0x%" PRIx64 "\n", > - s->admasysaddr); > + trace_sdhci_adma("link", s->admasysaddr); > break; > default: > s->admasysaddr += dscr.incr; > @@ -744,8 +717,7 @@ static void sdhci_do_adma(SDHCIState *s) > } > > if (dscr.attr & SDHC_ADMA_ATTR_INT) { > - DPRINT_L1("ADMA interrupt: admasysaddr=0x%" PRIx64 "\n", > - s->admasysaddr); > + trace_sdhci_adma("interrupt", s->admasysaddr); > if (s->norintstsen & SDHC_NISEN_DMA) { > s->norintsts |= SDHC_NIS_DMA; > } > @@ -756,15 +728,15 @@ static void sdhci_do_adma(SDHCIState *s) > /* ADMA transfer terminates if blkcnt == 0 or by END attribute */ > if (((s->trnmod & SDHC_TRNS_BLK_CNT_EN) && > (s->blkcnt == 0)) || (dscr.attr & SDHC_ADMA_ATTR_END)) { > - DPRINT_L2("ADMA transfer completed\n"); > + trace_sdhci_adma_transfer_completed(); > if (length || ((dscr.attr & SDHC_ADMA_ATTR_END) && > (s->trnmod & SDHC_TRNS_BLK_CNT_EN) && > s->blkcnt != 0)) { > - ERRPRINT("SD/MMC host ADMA length mismatch\n"); > + trace_sdhci_error("SD/MMC host ADMA length mismatch"); > s->admaerr |= SDHC_ADMAERR_LENGTH_MISMATCH | > SDHC_ADMAERR_STATE_ST_TFR; > if (s->errintstsen & SDHC_EISEN_ADMAERR) { > - ERRPRINT("Set ADMA error flag\n"); > + trace_sdhci_error("Set ADMA error flag"); > s->errintsts |= SDHC_EIS_ADMAERR; > s->norintsts |= SDHC_NIS_ERR; > } > @@ -800,7 +772,7 @@ static void sdhci_data_transfer(void *opaque) > break; > case SDHC_CTRL_ADMA1_32: > if (!(s->capareg & SDHC_CAN_DO_ADMA1)) { > - ERRPRINT("ADMA1 not supported\n"); > + trace_sdhci_error("ADMA1 not supported"); > break; > } > > @@ -808,7 +780,7 @@ static void sdhci_data_transfer(void *opaque) > break; > case SDHC_CTRL_ADMA2_32: > if (!(s->capareg & SDHC_CAN_DO_ADMA2)) { > - ERRPRINT("ADMA2 not supported\n"); > + trace_sdhci_error("ADMA2 not supported"); > break; > } > > @@ -817,14 +789,14 @@ static void sdhci_data_transfer(void *opaque) > case SDHC_CTRL_ADMA2_64: > if (!(s->capareg & SDHC_CAN_DO_ADMA2) || > !(s->capareg & SDHC_64_BIT_BUS_SUPPORT)) { > - ERRPRINT("64 bit ADMA not supported\n"); > + trace_sdhci_error("64 bit ADMA not supported"); > break; > } > > sdhci_do_adma(s); > break; > default: > - ERRPRINT("Unsupported DMA type\n"); > + trace_sdhci_error("Unsupported DMA type"); > break; > } > } else { > @@ -859,8 +831,8 @@ static inline bool > sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num) > { > if ((s->data_count & 0x3) != byte_num) { > - ERRPRINT("Non-sequential access to Buffer Data Port register" > - "is prohibited\n"); > + trace_sdhci_error("Non-sequential access to Buffer Data Port register" > + "is prohibited\n"); > return false; > } > return true; > @@ -890,8 +862,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size) > case SDHC_BDATA: > if (sdhci_buff_access_is_sequential(s, offset - SDHC_BDATA)) { > ret = sdhci_read_dataport(s, size); > - DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, (int)offset, > - ret, ret); > + trace_sdhci_access("read", size, offset, "->", ret, ret); > return ret; > } > break; > @@ -943,7 +914,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size) > > ret >>= (offset & 0x3) * 8; > ret &= (1ULL << (size * 8)) - 1; > - DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, (int)offset, ret, ret); > + trace_sdhci_access("read", size, offset, "->", ret, ret); > return ret; > } > > @@ -1145,8 +1116,8 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size) > "not implemented\n", size, offset, value >> shift); > break; > } > - DPRINT_L2("write %ub: addr[0x%04x] <- %u(0x%x)\n", > - size, (int)offset, value >> shift, value >> shift); > + trace_sdhci_access("write", size, offset, "<-", > + value >> shift, value >> shift); > } > > static const MemoryRegionOps sdhci_mmio_ops = { > diff --git a/hw/sd/trace-events b/hw/sd/trace-events > index 1fc0bcf44b..f7a85be53d 100644 > --- a/hw/sd/trace-events > +++ b/hw/sd/trace-events > @@ -1,5 +1,19 @@ > # See docs/devel/tracing.txt for syntax documentation. > > +# hw/sd/sdhci.c > +sdhci_set_inserted(const char *level) "card state changed: %s" > +sdhci_send_command(uint8_t cmd, uint32_t arg) "sending CMD%02u ARG[0x%08x]" > +sdhci_error(const char *msg) "%s" > +sdhci_response4(uint32_t r0) "Response: RSPREG[31..0]=0x%08x" > +sdhci_response16(uint32_t r3, uint32_t r2, uint32_t r1, uint32_t r0) "Response received: RSPREG[127..96]=0x%08x, RSPREG[95..64]=0x%08x, RSPREG[63..32]=0x%08x, RSPREG[31..0]=0x%08x" > +sdhci_end_transfer(uint8_t cmd, uint32_t arg) "Automatically issue CMD%02u 0x%08x" > +sdhci_adma(const char *desc, uint32_t sysad) "ADMA %s: admasysaddr=0x%" PRIx32 > +sdhci_adma_loop(uint64_t addr, uint16_t length, uint8_t attr) "ADMA loop: addr=0x%08" HWADDR_PRIx ", len=%d, attr=0x%x" > +sdhci_adma_transfer_completed(void) "ADMA transfer completed" > +sdhci_access(const char *access, unsigned int size, uint64_t offset, const char *dir, uint64_t val, uint64_t val2) "%s %ub: addr[0x%04" HWADDR_PRIx "] %s %" PRIu64 "(0x%" PRIx64 ")" > +sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read from input buffer" > +sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data" > + > # hw/sd/milkymist-memcard.c > milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x" > milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x" > -- > 2.15.1 > >
On Thu, Dec 14, 2017 at 2:54 PM, Alistair Francis <alistair.francis@xilinx.com> wrote: > On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé > <f4bug@amsat.org> wrote: >> zero-initialize ADMADescr 'dscr' in sdhci_do_adma() to avoid: >> >> hw/sd/sdhci.c: In function ‘sdhci_do_adma’: >> hw/sd/sdhci.c:714:29: error: ‘dscr.addr’ may be used uninitialized in this function [-Werror=maybe-uninitialized] >> trace_sdhci_adma("link", s->admasysaddr); >> ^ >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Acked-by: Alistair Francis <alistair.francis@xilinx.com> Thanks, note this patch has two incorrect uint64_t string format, I'll add your Acked-by in the respin with "%x" replaced by PRIx64. >> +# hw/sd/sdhci.c >> +sdhci_set_inserted(const char *level) "card state changed: %s" >> +sdhci_send_command(uint8_t cmd, uint32_t arg) "sending CMD%02u ARG[0x%08x]" >> +sdhci_error(const char *msg) "%s" >> +sdhci_response4(uint32_t r0) "Response: RSPREG[31..0]=0x%08x" >> +sdhci_response16(uint32_t r3, uint32_t r2, uint32_t r1, uint32_t r0) "Response received: RSPREG[127..96]=0x%08x, RSPREG[95..64]=0x%08x, RSPREG[63..32]=0x%08x, RSPREG[31..0]=0x%08x" >> +sdhci_end_transfer(uint8_t cmd, uint32_t arg) "Automatically issue CMD%02u 0x%08x" >> +sdhci_adma(const char *desc, uint32_t sysad) "ADMA %s: admasysaddr=0x%" PRIx32 >> +sdhci_adma_loop(uint64_t addr, uint16_t length, uint8_t attr) "ADMA loop: addr=0x%08" HWADDR_PRIx ", len=%d, attr=0x%x" ^ incorrect uint64_t format >> +sdhci_adma_transfer_completed(void) "ADMA transfer completed" >> +sdhci_access(const char *access, unsigned int size, uint64_t offset, const char *dir, uint64_t val, uint64_t val2) "%s %ub: addr[0x%04" HWADDR_PRIx "] %s %" PRIu64 "(0x%" PRIx64 ")" >> +sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read from input buffer" >> +sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data"
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 8fcd48f849..aa2d2fa3d3 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -33,30 +33,7 @@ #include "sd-internal.h" #include "qapi/error.h" #include "qemu/log.h" - -/* host controller debug messages */ -#ifndef SDHC_DEBUG -#define SDHC_DEBUG 0 -#endif - -#define DPRINT_L1(fmt, args...) \ - do { \ - if (SDHC_DEBUG) { \ - fprintf(stderr, "QEMU SDHC: " fmt, ## args); \ - } \ - } while (0) -#define DPRINT_L2(fmt, args...) \ - do { \ - if (SDHC_DEBUG > 1) { \ - fprintf(stderr, "QEMU SDHC: " fmt, ## args); \ - } \ - } while (0) -#define ERRPRINT(fmt, args...) \ - do { \ - if (SDHC_DEBUG) { \ - fprintf(stderr, "QEMU SDHC ERROR: " fmt, ## args); \ - } \ - } while (0) +#include "trace.h" #define TYPE_SDHCI_BUS "sdhci-bus" #define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS) @@ -154,8 +131,8 @@ static void sdhci_raise_insertion_irq(void *opaque) static void sdhci_set_inserted(DeviceState *dev, bool level) { SDHCIState *s = (SDHCIState *)dev; - DPRINT_L1("Card state changed: %s!\n", level ? "insert" : "eject"); + trace_sdhci_set_inserted(level ? "insert" : "eject"); if ((s->norintsts & SDHC_NIS_REMOVE) && level) { /* Give target some time to notice card ejection */ timer_mod(s->insert_timer, @@ -237,7 +214,8 @@ static void sdhci_send_command(SDHCIState *s) s->acmd12errsts = 0; request.cmd = s->cmdreg >> 8; request.arg = s->argument; - DPRINT_L1("sending CMD%u ARG[0x%08x]\n", request.cmd, request.arg); + + trace_sdhci_send_command(request.cmd, request.arg); rlen = sdbus_do_command(&s->sdbus, &request, response); if (s->cmdreg & SDHC_CMD_RESPONSE) { @@ -245,7 +223,7 @@ static void sdhci_send_command(SDHCIState *s) s->rspreg[0] = (response[0] << 24) | (response[1] << 16) | (response[2] << 8) | response[3]; s->rspreg[1] = s->rspreg[2] = s->rspreg[3] = 0; - DPRINT_L1("Response: RSPREG[31..0]=0x%08x\n", s->rspreg[0]); + trace_sdhci_response4(s->rspreg[0]); } else if (rlen == 16) { s->rspreg[0] = (response[11] << 24) | (response[12] << 16) | (response[13] << 8) | response[14]; @@ -255,11 +233,10 @@ static void sdhci_send_command(SDHCIState *s) (response[5] << 8) | response[6]; s->rspreg[3] = (response[0] << 16) | (response[1] << 8) | response[2]; - DPRINT_L1("Response received:\n RSPREG[127..96]=0x%08x, RSPREG[95.." - "64]=0x%08x,\n RSPREG[63..32]=0x%08x, RSPREG[31..0]=0x%08x\n", - s->rspreg[3], s->rspreg[2], s->rspreg[1], s->rspreg[0]); + trace_sdhci_response16(s->rspreg[3], s->rspreg[2], + s->rspreg[1], s->rspreg[0]); } else { - ERRPRINT("Timeout waiting for command response\n"); + trace_sdhci_error("timeout waiting for command response"); if (s->errintstsen & SDHC_EISEN_CMDTIMEOUT) { s->errintsts |= SDHC_EIS_CMDTIMEOUT; s->norintsts |= SDHC_NIS_ERR; @@ -293,7 +270,7 @@ static void sdhci_end_transfer(SDHCIState *s) request.cmd = 0x0C; request.arg = 0; - DPRINT_L1("Automatically issue CMD%d %08x\n", request.cmd, request.arg); + trace_sdhci_end_transfer(request.cmd, request.arg); sdbus_do_command(&s->sdbus, &request, response); /* Auto CMD12 response goes to the upper Response register */ s->rspreg[3] = (response[0] << 24) | (response[1] << 16) | @@ -362,7 +339,7 @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size) /* first check that a valid data exists in host controller input buffer */ if ((s->prnsts & SDHC_DATA_AVAILABLE) == 0) { - ERRPRINT("Trying to read from empty buffer\n"); + trace_sdhci_error("read from empty buffer"); return 0; } @@ -371,8 +348,7 @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size) s->data_count++; /* check if we've read all valid data (blksize bytes) from buffer */ if ((s->data_count) >= (s->blksize & 0x0fff)) { - DPRINT_L2("All %u bytes of data have been read from input buffer\n", - s->data_count); + trace_sdhci_read_dataport(s->data_count); s->prnsts &= ~SDHC_DATA_AVAILABLE; /* no more data in a buffer */ s->data_count = 0; /* next buff read must start at position [0] */ @@ -455,7 +431,7 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size) /* Check that there is free space left in a buffer */ if (!(s->prnsts & SDHC_SPACE_AVAILABLE)) { - ERRPRINT("Can't write to data buffer: buffer full\n"); + trace_sdhci_error("Can't write to data buffer: buffer full"); return; } @@ -464,8 +440,7 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size) s->data_count++; value >>= 8; if (s->data_count >= (s->blksize & 0x0fff)) { - DPRINT_L2("write buffer filled with %u bytes of data\n", - s->data_count); + trace_sdhci_write_dataport(s->data_count); s->data_count = 0; s->prnsts &= ~SDHC_SPACE_AVAILABLE; if (s->prnsts & SDHC_DOING_WRITE) { @@ -644,15 +619,14 @@ static void sdhci_do_adma(SDHCIState *s) { unsigned int n, begin, length; const uint16_t block_size = s->blksize & 0x0fff; - ADMADescr dscr; + ADMADescr dscr = {}; int i; for (i = 0; i < SDHC_ADMA_DESCS_PER_DELAY; ++i) { s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH; get_adma_description(s, &dscr); - DPRINT_L2("ADMA loop: addr=" TARGET_FMT_plx ", len=%d, attr=%x\n", - dscr.addr, dscr.length, dscr.attr); + trace_sdhci_adma_loop(dscr.addr, dscr.length, dscr.attr); if ((dscr.attr & SDHC_ADMA_ATTR_VALID) == 0) { /* Indicate that error occurred in ST_FDS state */ @@ -735,8 +709,7 @@ static void sdhci_do_adma(SDHCIState *s) break; case SDHC_ADMA_ATTR_ACT_LINK: /* link to next descriptor table */ s->admasysaddr = dscr.addr; - DPRINT_L1("ADMA link: admasysaddr=0x%" PRIx64 "\n", - s->admasysaddr); + trace_sdhci_adma("link", s->admasysaddr); break; default: s->admasysaddr += dscr.incr; @@ -744,8 +717,7 @@ static void sdhci_do_adma(SDHCIState *s) } if (dscr.attr & SDHC_ADMA_ATTR_INT) { - DPRINT_L1("ADMA interrupt: admasysaddr=0x%" PRIx64 "\n", - s->admasysaddr); + trace_sdhci_adma("interrupt", s->admasysaddr); if (s->norintstsen & SDHC_NISEN_DMA) { s->norintsts |= SDHC_NIS_DMA; } @@ -756,15 +728,15 @@ static void sdhci_do_adma(SDHCIState *s) /* ADMA transfer terminates if blkcnt == 0 or by END attribute */ if (((s->trnmod & SDHC_TRNS_BLK_CNT_EN) && (s->blkcnt == 0)) || (dscr.attr & SDHC_ADMA_ATTR_END)) { - DPRINT_L2("ADMA transfer completed\n"); + trace_sdhci_adma_transfer_completed(); if (length || ((dscr.attr & SDHC_ADMA_ATTR_END) && (s->trnmod & SDHC_TRNS_BLK_CNT_EN) && s->blkcnt != 0)) { - ERRPRINT("SD/MMC host ADMA length mismatch\n"); + trace_sdhci_error("SD/MMC host ADMA length mismatch"); s->admaerr |= SDHC_ADMAERR_LENGTH_MISMATCH | SDHC_ADMAERR_STATE_ST_TFR; if (s->errintstsen & SDHC_EISEN_ADMAERR) { - ERRPRINT("Set ADMA error flag\n"); + trace_sdhci_error("Set ADMA error flag"); s->errintsts |= SDHC_EIS_ADMAERR; s->norintsts |= SDHC_NIS_ERR; } @@ -800,7 +772,7 @@ static void sdhci_data_transfer(void *opaque) break; case SDHC_CTRL_ADMA1_32: if (!(s->capareg & SDHC_CAN_DO_ADMA1)) { - ERRPRINT("ADMA1 not supported\n"); + trace_sdhci_error("ADMA1 not supported"); break; } @@ -808,7 +780,7 @@ static void sdhci_data_transfer(void *opaque) break; case SDHC_CTRL_ADMA2_32: if (!(s->capareg & SDHC_CAN_DO_ADMA2)) { - ERRPRINT("ADMA2 not supported\n"); + trace_sdhci_error("ADMA2 not supported"); break; } @@ -817,14 +789,14 @@ static void sdhci_data_transfer(void *opaque) case SDHC_CTRL_ADMA2_64: if (!(s->capareg & SDHC_CAN_DO_ADMA2) || !(s->capareg & SDHC_64_BIT_BUS_SUPPORT)) { - ERRPRINT("64 bit ADMA not supported\n"); + trace_sdhci_error("64 bit ADMA not supported"); break; } sdhci_do_adma(s); break; default: - ERRPRINT("Unsupported DMA type\n"); + trace_sdhci_error("Unsupported DMA type"); break; } } else { @@ -859,8 +831,8 @@ static inline bool sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num) { if ((s->data_count & 0x3) != byte_num) { - ERRPRINT("Non-sequential access to Buffer Data Port register" - "is prohibited\n"); + trace_sdhci_error("Non-sequential access to Buffer Data Port register" + "is prohibited\n"); return false; } return true; @@ -890,8 +862,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size) case SDHC_BDATA: if (sdhci_buff_access_is_sequential(s, offset - SDHC_BDATA)) { ret = sdhci_read_dataport(s, size); - DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, (int)offset, - ret, ret); + trace_sdhci_access("read", size, offset, "->", ret, ret); return ret; } break; @@ -943,7 +914,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size) ret >>= (offset & 0x3) * 8; ret &= (1ULL << (size * 8)) - 1; - DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, (int)offset, ret, ret); + trace_sdhci_access("read", size, offset, "->", ret, ret); return ret; } @@ -1145,8 +1116,8 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size) "not implemented\n", size, offset, value >> shift); break; } - DPRINT_L2("write %ub: addr[0x%04x] <- %u(0x%x)\n", - size, (int)offset, value >> shift, value >> shift); + trace_sdhci_access("write", size, offset, "<-", + value >> shift, value >> shift); } static const MemoryRegionOps sdhci_mmio_ops = { diff --git a/hw/sd/trace-events b/hw/sd/trace-events index 1fc0bcf44b..f7a85be53d 100644 --- a/hw/sd/trace-events +++ b/hw/sd/trace-events @@ -1,5 +1,19 @@ # See docs/devel/tracing.txt for syntax documentation. +# hw/sd/sdhci.c +sdhci_set_inserted(const char *level) "card state changed: %s" +sdhci_send_command(uint8_t cmd, uint32_t arg) "sending CMD%02u ARG[0x%08x]" +sdhci_error(const char *msg) "%s" +sdhci_response4(uint32_t r0) "Response: RSPREG[31..0]=0x%08x" +sdhci_response16(uint32_t r3, uint32_t r2, uint32_t r1, uint32_t r0) "Response received: RSPREG[127..96]=0x%08x, RSPREG[95..64]=0x%08x, RSPREG[63..32]=0x%08x, RSPREG[31..0]=0x%08x" +sdhci_end_transfer(uint8_t cmd, uint32_t arg) "Automatically issue CMD%02u 0x%08x" +sdhci_adma(const char *desc, uint32_t sysad) "ADMA %s: admasysaddr=0x%" PRIx32 +sdhci_adma_loop(uint64_t addr, uint16_t length, uint8_t attr) "ADMA loop: addr=0x%08" HWADDR_PRIx ", len=%d, attr=0x%x" +sdhci_adma_transfer_completed(void) "ADMA transfer completed" +sdhci_access(const char *access, unsigned int size, uint64_t offset, const char *dir, uint64_t val, uint64_t val2) "%s %ub: addr[0x%04" HWADDR_PRIx "] %s %" PRIu64 "(0x%" PRIx64 ")" +sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read from input buffer" +sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data" + # hw/sd/milkymist-memcard.c milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x" milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
zero-initialize ADMADescr 'dscr' in sdhci_do_adma() to avoid: hw/sd/sdhci.c: In function ‘sdhci_do_adma’: hw/sd/sdhci.c:714:29: error: ‘dscr.addr’ may be used uninitialized in this function [-Werror=maybe-uninitialized] trace_sdhci_adma("link", s->admasysaddr); ^ Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/sd/sdhci.c | 89 ++++++++++++++++++------------------------------------ hw/sd/trace-events | 14 +++++++++ 2 files changed, 44 insertions(+), 59 deletions(-)