From 0c575ced95219f11d554bdf5c34008362fb38f7b Mon Sep 17 00:00:00 2001 From: Tomas Vanek Date: Sat, 8 Feb 2025 21:36:15 +0100 Subject: [PATCH] drivers/ch347: fix conditional sending of idle clock cycles ch347_swd_run_queue() fulfilled ADI requirement for idle cycles after the last transaction just if there was room in the buffer. A lazy programmer avoided this way the danger of recurrent call. Introduce ch347_swd_run_queue_inner() without sending idle cycles. They are useless when flushing the queue to get room for the next transaction. ch347_swd_run_queue() now can make room for idle cycles in the queue without recursion. While on it remove two useless debug logs showing ap_delay_clk value and prevent the overflow of ap_delay_clk forcing it to be <= 255. Change-Id: Ia7b7f0d373ff463e2f0742bdd068c3833c57f340 Signed-off-by: Tomas Vanek Reviewed-on: https://review.openocd.org/c/openocd/+/8744 Reviewed-by: ZhiYuanNJ <871238103@qq.com> Tested-by: jenkins --- src/jtag/drivers/ch347.c | 50 ++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/src/jtag/drivers/ch347.c b/src/jtag/drivers/ch347.c index ecec155f7..530ac5096 100644 --- a/src/jtag/drivers/ch347.c +++ b/src/jtag/drivers/ch347.c @@ -2076,13 +2076,13 @@ static bool ch347_chk_buf_size(uint8_t cmd, uint32_t ap_delay_clk) return flush; } -static int ch347_swd_run_queue(void); +static int ch347_swd_run_queue_inner(void); static int ch347_swd_send_idle(uint32_t ap_delay_clk) { struct ch347_swd_io *pswd_io = ch347_get_one_swd_io(); if (!pswd_io) { - int retval = ch347_swd_run_queue(); + int retval = ch347_swd_run_queue_inner(); if (retval != ERROR_OK) return retval; @@ -2099,7 +2099,7 @@ static int ch347_swd_send_idle(uint32_t ap_delay_clk) return ERROR_OK; } -static int ch347_swd_run_queue(void) +static int ch347_swd_run_queue_inner(void) { LOG_DEBUG_IO("Executing %u queued transactions", ch347_swd_context.sent_cmd_count); if (ch347_swd_context.queued_retval != ERROR_OK) { @@ -2107,20 +2107,7 @@ static int ch347_swd_run_queue(void) goto skip; } - /* A transaction must be followed by another transaction or at least 8 - idle cycles to ensure that data is clocked through the AP. */ - if ((ch347_swd_context.send_len + (1 + 2 + 1)) > CH347_MAX_SEND_BUF) - goto skip_idle; - - if ((ch347_swd_context.need_recv_len + 1) > CH347_MAX_RECV_BUF) - goto skip_idle; - - int retval = ch347_swd_send_idle((uint32_t)8); - if (retval != ERROR_OK) - return retval; - -skip_idle: - retval = ch347_swd_queue_flush(); + int retval = ch347_swd_queue_flush(); if (retval != ERROR_OK) return retval; @@ -2248,27 +2235,38 @@ skip: return retval; } +static int ch347_swd_run_queue(void) +{ + /* A transaction must be followed by another transaction or at least 8 + idle cycles to ensure that data is clocked through the AP. */ + int retval = ch347_swd_send_idle(8); + if (retval != ERROR_OK) + return retval; + + return ch347_swd_run_queue_inner(); +} + static int ch347_swd_queue_cmd(uint8_t cmd, uint32_t *dst, uint32_t data, uint32_t ap_delay_clk) { - if (ap_delay_clk > 255) - LOG_DEBUG("ap_delay_clk = %d", ap_delay_clk); - int retval = ERROR_OK; + if (ap_delay_clk > 255) + ap_delay_clk = 255; // limit imposed by spec. seq. size in one byte + if (ch347_swd_context.sent_cmd_count >= CH347_MAX_SEND_CMD) { - retval = ch347_swd_run_queue(); + retval = ch347_swd_run_queue_inner(); if (retval != ERROR_OK) return retval; } if (!ch347_chk_buf_size(cmd, ap_delay_clk)) { - retval = ch347_swd_run_queue(); + retval = ch347_swd_run_queue_inner(); if (retval != ERROR_OK) return retval; } struct ch347_swd_io *pswd_io = ch347_get_one_swd_io(); if (!pswd_io) { - retval = ch347_swd_run_queue(); + retval = ch347_swd_run_queue_inner(); if (retval != ERROR_OK) return retval; @@ -2294,12 +2292,10 @@ static int ch347_swd_queue_cmd(uint8_t cmd, uint32_t *dst, uint32_t data, uint32 ch347_swd_context.sent_cmd_count++; list_add_tail(&pswd_io->list_entry, &ch347_swd_context.send_cmd_head); + // Insert idle cycles after AP accesses to avoid WAIT - if (cmd & SWD_CMD_APNDP) { - if (ap_delay_clk == 0) - LOG_DEBUG("ap_delay_clk == 0"); + if (ap_delay_clk) retval = ch347_swd_send_idle(ap_delay_clk); - } return retval; }