[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 22/22] adb: add ADB bus trace events
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 22/22] adb: add ADB bus trace events |
Date: |
Sun, 14 Jun 2020 19:16:28 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
Hi Mark,
On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/input/adb.c | 23 ++++++++++++++++++++++-
> hw/input/trace-events | 7 +++++++
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index fe0f6c7ef3..4976f52c36 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -29,10 +29,18 @@
> #include "qemu/module.h"
> #include "qemu/timer.h"
> #include "adb-internal.h"
> +#include "trace.h"
>
> /* error codes */
> #define ADB_RET_NOTPRESENT (-2)
>
> +static const char *adb_commands[] = {
> + "RESET", "FLUSH", "(Reserved 0x2)", "(Reserved 0x3)",
> + "Reserved (0x4)", "(Reserved 0x5)", "(Reserved 0x6)", "(Reserved 0x7)",
> + "LISTEN r0", "LISTEN r1", "LISTEN r2", "LISTEN r3",
> + "TALK r0", "TALK r1", "TALK r2", "TALK r3",
> +};
> +
> static void adb_device_reset(ADBDevice *d)
> {
> qdev_reset_all(DEVICE(d));
> @@ -86,9 +94,16 @@ static int do_adb_request(ADBBusState *s, uint8_t *obuf,
> const uint8_t *buf,
>
> int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
> {
> + int ret;
> +
> + trace_adb_bus_request(buf[0] >> 4, adb_commands[buf[0] & 0xf], len);
> +
> assert(s->autopoll_blocked);
>
> - return do_adb_request(s, obuf, buf, len);
> + ret = do_adb_request(s, obuf, buf, len);
> +
> + trace_adb_bus_request_done(buf[0] >> 4, adb_commands[buf[0] & 0xf], ret);
> + return ret;
> }
>
> int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
> @@ -160,6 +175,8 @@ void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask)
>
> void adb_autopoll_block(ADBBusState *s)
> {
> + trace_adb_bus_autopoll_block("autopoll BLOCKED");
Regarding how trace backends work, in this case it is better
to use a boolean value and let the backend do the formatting:
trace_adb_bus_autopoll_block(true);
The rationale is it is easier for backends to filter on a
bool (register) arg rather than fetching memory for strcmp.
So format can be:
adb_bus_autopoll_block(bool state) "autopoll is_blocked:%u"
Anyway if you want to keep as it, it is cleaner to change the
format as "autopoll %s".
> +
> s->autopoll_blocked = true;
This can also be:
trace_adb_bus_autopoll_block(s->autopoll_blocked);
>
> if (s->autopoll_enabled) {
> @@ -169,6 +186,8 @@ void adb_autopoll_block(ADBBusState *s)
>
> void adb_autopoll_unblock(ADBBusState *s)
> {
> + trace_adb_bus_autopoll_block("autopoll UNBLOCKED");
> +
> s->autopoll_blocked = false;
Ditto:
trace_adb_bus_autopoll_block(s->autopoll_blocked);
>
> if (s->autopoll_enabled) {
> @@ -183,7 +202,9 @@ static void adb_autopoll(void *opaque)
> ADBBusState *s = opaque;
>
> if (!s->autopoll_blocked) {
> + trace_adb_bus_autopoll_cb(s->autopoll_mask);
> s->autopoll_cb(s->autopoll_cb_opaque);
> + trace_adb_bus_autopoll_cb_done(s->autopoll_mask);
> }
>
> timer_mod(s->autopoll_timer,
> diff --git a/hw/input/trace-events b/hw/input/trace-events
> index 6f0d78241c..119d1ce2bd 100644
> --- a/hw/input/trace-events
> +++ b/hw/input/trace-events
> @@ -14,6 +14,13 @@ adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t
> val1) "reg %d obuf[0] 0x
> adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
> adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler)
> "change addr and handler to 0x%x, 0x%x"
>
> +# adb.c
> +adb_bus_request(uint8_t addr, const char *cmd, int size) "device 0x%x %s
> cmdsize=%d"
> +adb_bus_request_done(uint8_t addr, const char *cmd, int size) "device 0x%x
> %s replysize=%d"
> +adb_bus_autopoll_block(const char *s) "%s"
> +adb_bus_autopoll_cb(uint16_t mask) "executing autopoll_cb with autopoll mask
> 0x%x"
> +adb_bus_autopoll_cb_done(uint16_t mask) "done executing autopoll_cb with
> autopoll mask 0x%x"
> +
> # pckbd.c
> pckbd_kbd_read_data(uint32_t val) "0x%02x"
> pckbd_kbd_read_status(int status) "0x%02x"
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
- [PATCH 16/22] cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions, (continued)
- [PATCH 16/22] cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions, Mark Cave-Ayland, 2020/06/14
- [PATCH 17/22] pmu: add adb_autopoll_block() and adb_autopoll_unblock() functions, Mark Cave-Ayland, 2020/06/14
- [PATCH 18/22] mac_via: move VIA1 portB write logic into mos6522_q800_via1_write(), Mark Cave-Ayland, 2020/06/14
- [PATCH 20/22] adb: only call autopoll callbacks when autopoll is not blocked, Mark Cave-Ayland, 2020/06/14
- [PATCH 19/22] mac_via: rework ADB state machine to be compatible with both MacOS and Linux, Mark Cave-Ayland, 2020/06/14
- [PATCH 21/22] adb: use adb_device prefix for ADB device trace events, Mark Cave-Ayland, 2020/06/14
- [PATCH 22/22] adb: add ADB bus trace events, Mark Cave-Ayland, 2020/06/14
- Re: [PATCH 22/22] adb: add ADB bus trace events,
Philippe Mathieu-Daudé <=
- Re: [PATCH 00/22] ADB: fix autopoll issues and rework mac_via state machine, Finn Thain, 2020/06/16