From d4a64e3f38a9ad7a27554547c2720f17ed3580a3 Mon Sep 17 00:00:00 2001 From: Jan Matyas Date: Thu, 10 Oct 2024 20:35:19 +0200 Subject: [PATCH 01/26] autoconf: Add support for code coverage Add support for code coverage collection. This helps developers to check if their test scenarios really exercised all the OpenOCD functionality that they intended to test. - Option --enable-gcov has been added to configure.ac which enables the coverage collection using Gcov. (Disabled by default.) - The steps to collect and inspect the coverage have been described in HACKING file. Change-Id: I259e401937a255e7ad7f155359a0b7787e4d0752 Signed-off-by: Jan Matyas Reviewed-on: https://review.openocd.org/c/openocd/+/8521 Tested-by: jenkins Reviewed-by: Evgeniy Naydanov Reviewed-by: Antonio Borneo --- .gitignore | 4 ++++ HACKING | 16 ++++++++++++++++ Makefile.am | 7 +++++++ configure.ac | 21 ++++++++++++++++++++- src/openocd.c | 7 +++++++ 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 5de33077e..1b4e08ba3 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,10 @@ src/jtag/minidriver_imp.h src/jtag/jtag_minidriver.h +# coverage files (gcov) +*.gcda +*.gcno + # OpenULINK driver files generated by SDCC src/jtag/drivers/OpenULINK/*.rel src/jtag/drivers/OpenULINK/*.asm diff --git a/HACKING b/HACKING index 9e8cd357f..8988b1617 100644 --- a/HACKING +++ b/HACKING @@ -92,6 +92,22 @@ patch: make @endcode +- Code coverage analysis + + By inspecting the code coverage, you can identify potential gaps in your testing + and use that information to improve your test scenarios. + + Example usage: + @code + mkdir build-gcov; cd build-gcov + ../configure --enable-gcov [...] + make + # ... Now execute your test scenarios to collect OpenOCD code coverage ... + lcov --capture --directory ./src --output-file openocd-coverage.info + genhtml openocd-coverage.info --output-directory coverage_report + # ... Open coverage_report/index.html in a web browser ... + @endcode + Please consider performing these additional checks where appropriate (especially Clang Static Analyzer for big portions of new code) and mention the results (e.g. "Valgrind-clean, no new Clang analyzer diff --git a/Makefile.am b/Makefile.am index 2230e628f..ab0a2373d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,6 +38,7 @@ endif # common flags used in openocd build AM_CFLAGS = $(GCC_WARNINGS) +AM_LDFLAGS = AM_CPPFLAGS = $(HOST_CPPFLAGS)\ -I$(top_srcdir)/src \ @@ -51,6 +52,12 @@ AM_CPPFLAGS += -I$(top_srcdir)/jimtcl \ else AM_CPPFLAGS += $(JIMTCL_CFLAGS) endif + +if USE_GCOV +AM_CFLAGS += --coverage +AM_LDFLAGS += --coverage +endif + EXTRA_DIST += \ BUGS \ HACKING \ diff --git a/configure.ac b/configure.ac index 9355ffb93..291e854a4 100644 --- a/configure.ac +++ b/configure.ac @@ -171,6 +171,9 @@ m4_define([DUMMY_ADAPTER], m4_define([OPTIONAL_LIBRARIES], [[[capstone], [Use Capstone disassembly framework], []]]) +m4_define([COVERAGE], + [[[gcov], [Collect coverage using gcov], []]]) + AC_ARG_ENABLE([doxygen-html], AS_HELP_STRING([--disable-doxygen-html], [Disable building Doxygen manual as HTML.]), @@ -199,6 +202,19 @@ AC_ARG_ENABLE([werror], AS_HELP_STRING([--disable-werror], [Do not treat warnings as errors]), [gcc_werror=$enableval], [gcc_werror=$gcc_warnings]) +AC_ARG_ENABLE([gcov], + AS_HELP_STRING([--enable-gcov], [Enable runtime coverage collection via gcov]), + [enable_gcov=$enableval], [enable_gcov=no]) + +AS_IF([test "x$enable_gcov" = "xyes"], [ + AC_DEFINE([USE_GCOV], [1], [1 to enable coverage collection using gcov.]) + dnl When collecting coverage, disable optimizations. + dnl This overrides the "-O2" that autoconf uses by default: + CFLAGS+=" -O0" +], [ + AC_DEFINE([USE_GCOV], [0], [0 to leave coverage collection disabled.]) +]) + # set default verbose options, overridden by following options debug_usb_io=no debug_usb_comms=no @@ -787,6 +803,8 @@ AM_CONDITIONAL([INTERNAL_JIMTCL], [test "x$use_internal_jimtcl" = "xyes"]) AM_CONDITIONAL([HAVE_JIMTCL_PKG_CONFIG], [test "x$have_jimtcl_pkg_config" = "xyes"]) AM_CONDITIONAL([INTERNAL_LIBJAYLINK], [test "x$use_internal_libjaylink" = "xyes"]) +AM_CONDITIONAL([USE_GCOV], [test "x$enable_gcov" = "xyes"]) + # Look for environ alternatives. Possibility #1: is environ in unistd.h or stdlib.h? AC_MSG_CHECKING([for environ in unistd.h and stdlib.h]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ @@ -862,7 +880,8 @@ m4_foreach([adapter], [USB1_ADAPTERS, LIBGPIOD_ADAPTERS, LIBJAYLINK_ADAPTERS, PCIE_ADAPTERS, SERIAL_PORT_ADAPTERS, DUMMY_ADAPTER, - OPTIONAL_LIBRARIES], + OPTIONAL_LIBRARIES, + COVERAGE], [s=m4_format(["%-40s"], ADAPTER_DESC([adapter])) AS_CASE([$ADAPTER_VAR([adapter])], [auto], [ diff --git a/src/openocd.c b/src/openocd.c index 7a5147050..9fd709e32 100644 --- a/src/openocd.c +++ b/src/openocd.c @@ -375,6 +375,13 @@ int openocd_main(int argc, char *argv[]) log_exit(); +#if USE_GCOV + /* Always explicitly dump coverage data before terminating. + * Otherwise coverage would not be dumped when exit_on_signal occurs. */ + void __gcov_dump(void); + __gcov_dump(); +#endif + if (ret == ERROR_FAIL) return EXIT_FAILURE; else if (ret != ERROR_OK) From 564b24e7f899f401ec6e5adda7906244f60c0135 Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Fri, 1 Nov 2024 20:21:56 +0100 Subject: [PATCH 02/26] Makefile.am: generate ChangeLog with git log instead of git2cl git log is faster than git2cl and the result has a better format. Change-Id: I465ca62e3e30fed230fe9661e82a987980c05459 Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8531 Tested-by: jenkins Reviewed-by: R. Diez Reviewed-by: Antonio Borneo --- .gitmodules | 3 --- Makefile.am | 11 +++++------ tools/git2cl | 1 - 3 files changed, 5 insertions(+), 10 deletions(-) delete mode 160000 tools/git2cl diff --git a/.gitmodules b/.gitmodules index f2da17ed7..abb773538 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,6 +1,3 @@ -[submodule "tools/git2cl"] - path = tools/git2cl - url = https://git.savannah.nongnu.org/git/git2cl.git [submodule "jimtcl"] path = jimtcl url = https://github.com/msteveb/jimtcl.git diff --git a/Makefile.am b/Makefile.am index ab0a2373d..155a2b3bb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -123,14 +123,13 @@ TCL_PATH = tcl TCL_FILES = find $(srcdir)/$(TCL_PATH) -name '*.cfg' -o -name '*.tcl' -o -name '*.txt' | \ sed -e 's,^$(srcdir)/$(TCL_PATH),,' -# Without the PERL_UNICODE="IO" workaround below when running git2cl, you get several -# "Wide character" warnings and you also risk an invalid character encoding in -# the generated ChangeLog file. For more information, see this bug report: -# Warning "Wide character in print" -# https://savannah.nongnu.org/bugs/?65689 +# The git log command below generates many empty text lines with only some space characters +# for indentation purposes, so use sed to trim all trailing whitespace. dist-hook: if test -d $(srcdir)/.git -a \( ! -e $(distdir)/ChangeLog -o -w $(distdir)/ChangeLog \) ; then \ - git --git-dir $(srcdir)/.git log | PERL_UNICODE="IO" $(srcdir)/tools/git2cl/git2cl > $(distdir)/ChangeLog ; \ + git --git-dir $(srcdir)/.git log --date=short --pretty="format:%ad %aN <%aE>%n%n%w(0,4,6)* %B" \ + | sed 's/[[:space:]]*$$//' > $(distdir)/ChangeLog.tmp && \ + mv $(distdir)/ChangeLog.tmp $(distdir)/ChangeLog; \ fi for i in $$($(TCL_FILES)); do \ j="$(distdir)/$(TCL_PATH)/$$i" && \ diff --git a/tools/git2cl b/tools/git2cl deleted file mode 160000 index 8373c9f74..000000000 --- a/tools/git2cl +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 8373c9f74993e218a08819cbcdbab3f3564bbeba From 6d60ac5c08e6343008f40ee3d8daa5e8e00eb148 Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Fri, 1 Nov 2024 09:46:05 +0100 Subject: [PATCH 03/26] Make bootstrap more robust Change-Id: I67cc22752b34dd49c277e247f0b648047927a02b Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8532 Reviewed-by: R. Diez Tested-by: jenkins Reviewed-by: zapb Reviewed-by: Antonio Borneo --- bootstrap | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/bootstrap b/bootstrap index 7d4ca37bd..9dfdc41ac 100755 --- a/bootstrap +++ b/bootstrap @@ -3,8 +3,8 @@ # Run the autotools bootstrap sequence to create the configure script -# Abort execution on error -set -e +set -e # Abort execution on error. +set -u # Abort if you reference an undefined variable. if which libtoolize > /dev/null; then libtoolize="libtoolize" @@ -15,13 +15,21 @@ else exit 1 fi -if [ "$1" = "nosubmodule" ]; then - SKIP_SUBMODULE=1 -elif [ -n "$1" ]; then - echo "$0: Illegal argument $1" - echo "USAGE: $0 [nosubmodule]" - exit 1 -fi +SKIP_SUBMODULE=0 + +case "$#" in + 0) ;; + 1) if [ "$1" = "nosubmodule" ]; then + SKIP_SUBMODULE=1 + else + echo "$0: Illegal argument $1" >&2 + echo "USAGE: $0 [nosubmodule]" >&2 + exit 1 + fi;; + *) echo "$0: Wrong number of command-line arguments." >&2 + echo "USAGE: $0 [nosubmodule]" >&2 + exit 1;; +esac # bootstrap the autotools ( @@ -34,7 +42,7 @@ autoheader --warnings=all automake --warnings=all --gnu --add-missing --copy ) -if [ -n "$SKIP_SUBMODULE" ]; then +if [ "$SKIP_SUBMODULE" -ne 0 ]; then echo "Skipping submodule setup" else echo "Setting up submodules" From 8a3723022689dd078c3e61268615616d5566fc94 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Mon, 10 Apr 2023 12:02:55 +0200 Subject: [PATCH 04/26] checkpatch: exclude gerrit's Change-Id line from commit description Checkpatch rejects patches that have empty commit description and logs them with: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one But if the patch has a gerrit's Change-Id line placed before the line Signed-off-by, then checkpatch considers the Change-Id line as a valid commit description text. Use the Change-Id tag as a marker of the end of the commit message, thus not counting its line as part of the commit description. This patch is not relevant for the Linux kernel development process as gerrit is not involved and the Change-Id tag is rejected. But other projects, like OpenOCD, base the development on gerrit and reuse kernel's checkpatch with flag '--ignore GERRIT_CHANGE_ID'. This patch has been refused [1] in Linux upstream because it has not been considered relevant for that project. Let's take it as another add-on in OpenOCD checkpatch. Change-Id: I3b55b8fffa07ce67177c108e7c9554ca46674246 Signed-off-by: Antonio Borneo Link: [1] https://lore.kernel.org/lkml/20230410100255.16755-1-borneo.antonio@gmail.com/ Reviewed-on: https://review.openocd.org/c/openocd/+/8539 Tested-by: jenkins --- tools/scripts/checkpatch.pl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/scripts/checkpatch.pl b/tools/scripts/checkpatch.pl index 01bd547ff..1011b3305 100755 --- a/tools/scripts/checkpatch.pl +++ b/tools/scripts/checkpatch.pl @@ -3251,6 +3251,9 @@ sub process { # Check for Gerrit Change-Ids not in any patch context if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) { + # OpenOCD specific: Begin: exclude gerrit's Change-Id line from commit description + $in_commit_log = 0; + # OpenOCD specific: End if (ERROR("GERRIT_CHANGE_ID", "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) && $fix) { From 989e9e8b5488be6dbf4ed2e9f5cbda208b841860 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Fri, 11 Oct 2024 10:10:12 +0200 Subject: [PATCH 05/26] gitignore: drop ignoring files not generated anymore With the drop of the code for the probe zy1000 [1] and then the drop of minidriver code [2], there are no more auto-generated source files. Remove them from the list of generated files to be ignored. Change-Id: Iee65e21528674ea4cc94018e52126f882da4f07c Signed-off-by: Antonio Borneo [1] b0fe92dba7c0 ("zy1000: drop the code, deprecated in v0.10.0") [2] 25218e893503 ("jtag: remove minidriver code and minidriver-dummy") Reviewed-on: https://review.openocd.org/c/openocd/+/8522 Tested-by: jenkins --- .gitignore | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.gitignore b/.gitignore index 1b4e08ba3..f0746c195 100644 --- a/.gitignore +++ b/.gitignore @@ -11,10 +11,6 @@ *.la *.in -# generated source files -src/jtag/minidriver_imp.h -src/jtag/jtag_minidriver.h - # coverage files (gcov) *.gcda *.gcno From 2465f1851549e16337e0eef84e581b62d541a187 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 18:49:11 +0100 Subject: [PATCH 06/26] adapter: make adapter_config_khz() static The function is not referenced outside the file. Make it static. Change-Id: I72e96624749ae4cc7f4566d737a88186e899616a Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8541 Tested-by: jenkins --- src/jtag/adapter.c | 5 ++++- src/jtag/adapter.h | 3 --- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/jtag/adapter.c b/src/jtag/adapter.c index 996a23f1f..04942f753 100644 --- a/src/jtag/adapter.c +++ b/src/jtag/adapter.c @@ -66,6 +66,8 @@ static const struct gpio_map { [ADAPTER_GPIO_IDX_LED] = { "led", ADAPTER_GPIO_DIRECTION_OUTPUT, true, true, }, }; +static int adapter_config_khz(unsigned int khz); + bool is_adapter_initialized(void) { return adapter_config.adapter_initialized; @@ -245,7 +247,8 @@ static int adapter_set_speed(int speed) return is_adapter_initialized() ? adapter_driver->speed(speed) : ERROR_OK; } -int adapter_config_khz(unsigned int khz) +/** Attempt to configure the adapter for the specified kHz. */ +static int adapter_config_khz(unsigned int khz) { LOG_DEBUG("handle adapter khz"); adapter_config.clock_mode = CLOCK_MODE_KHZ; diff --git a/src/jtag/adapter.h b/src/jtag/adapter.h index 23ffe2cc5..556952f8d 100644 --- a/src/jtag/adapter.h +++ b/src/jtag/adapter.h @@ -97,9 +97,6 @@ int adapter_get_speed(int *speed); */ int adapter_get_speed_readable(int *speed); -/** Attempt to configure the adapter for the specified kHz. */ -int adapter_config_khz(unsigned int khz); - /** * Attempt to enable RTCK/RCLK. If that fails, fallback to the * specified frequency. From f82664ff827ba76f773c51b74705c13b9c813758 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:29:43 +0100 Subject: [PATCH 07/26] jtag: core: make local functions static The functions: - jtag_error_clear(); - jtag_tap_count(); are not referenced outside the file. Make them static. Change-Id: I00fcf06b1838b9f6c955c19772f1d41d486459e9 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8542 Tested-by: jenkins --- src/jtag/core.c | 10 ++++++++-- src/jtag/jtag.h | 6 ------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/jtag/core.c b/src/jtag/core.c index 907883f09..769e07571 100644 --- a/src/jtag/core.c +++ b/src/jtag/core.c @@ -51,6 +51,8 @@ static void jtag_add_scan_check(struct jtag_tap *active, tap_state_t state), int in_num_fields, struct scan_field *in_fields, tap_state_t state); +static int jtag_error_clear(void); + /** * The jtag_error variable is set when an error occurs while executing * the queue. Application code may set this using jtag_set_error(), @@ -127,7 +129,11 @@ void jtag_set_error(int error) jtag_error = error; } -int jtag_error_clear(void) +/** + * Resets jtag_error to ERROR_OK, returning its previous value. + * @returns The previous value of @c jtag_error. + */ +static int jtag_error_clear(void) { int temp = jtag_error; jtag_error = ERROR_OK; @@ -186,7 +192,7 @@ struct jtag_tap *jtag_all_taps(void) return __jtag_all_taps; }; -unsigned int jtag_tap_count(void) +static unsigned int jtag_tap_count(void) { struct jtag_tap *t = jtag_all_taps(); unsigned int n = 0; diff --git a/src/jtag/jtag.h b/src/jtag/jtag.h index b9d37b32a..86526a09a 100644 --- a/src/jtag/jtag.h +++ b/src/jtag/jtag.h @@ -153,7 +153,6 @@ struct jtag_tap *jtag_tap_by_jim_obj(Jim_Interp *interp, Jim_Obj *obj); struct jtag_tap *jtag_tap_by_position(unsigned int abs_position); struct jtag_tap *jtag_tap_next_enabled(struct jtag_tap *p); unsigned int jtag_tap_count_enabled(void); -unsigned int jtag_tap_count(void); /* * - TRST_ASSERTED triggers two sets of callbacks, after operations to @@ -568,11 +567,6 @@ void jtag_sleep(uint32_t us); * called with a non-zero error code. */ void jtag_set_error(int error); -/** - * Resets jtag_error to ERROR_OK, returning its previous value. - * @returns The previous value of @c jtag_error. - */ -int jtag_error_clear(void); /** * Return true if it's safe for a background polling task to access the From f4ac0c7022933adcbc68140ca565e93ece726ab5 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:39:11 +0100 Subject: [PATCH 08/26] jtag: driver: make local functions static The functions: - interface_jtag_add_callback(); - interface_jtag_add_callback4(); are not referenced outside the file. Make them static. Change-Id: I84f738309d23c8d0b5329aa04436db750cf185e5 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8543 Tested-by: jenkins --- src/jtag/drivers/driver.c | 4 ++-- src/jtag/drivers/minidriver_imp.h | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/jtag/drivers/driver.c b/src/jtag/drivers/driver.c index d52a345a0..58e59a559 100644 --- a/src/jtag/drivers/driver.c +++ b/src/jtag/drivers/driver.c @@ -341,7 +341,7 @@ int interface_jtag_add_sleep(uint32_t us) } /* add callback to end of queue */ -void interface_jtag_add_callback4(jtag_callback_t callback, +static void interface_jtag_add_callback4(jtag_callback_t callback, jtag_callback_data_t data0, jtag_callback_data_t data1, jtag_callback_data_t data2, jtag_callback_data_t data3) { @@ -395,7 +395,7 @@ static int jtag_convert_to_callback4(jtag_callback_data_t data0, return ERROR_OK; } -void interface_jtag_add_callback(jtag_callback1_t callback, jtag_callback_data_t data0) +static void interface_jtag_add_callback(jtag_callback1_t callback, jtag_callback_data_t data0) { jtag_add_callback4(jtag_convert_to_callback4, data0, (jtag_callback_data_t)callback, 0, 0); } diff --git a/src/jtag/drivers/minidriver_imp.h b/src/jtag/drivers/minidriver_imp.h index b29b3c9cc..f3582f6e3 100644 --- a/src/jtag/drivers/minidriver_imp.h +++ b/src/jtag/drivers/minidriver_imp.h @@ -17,12 +17,6 @@ static inline void interface_jtag_add_scan_check_alloc(struct scan_field *field) field->in_value = cmd_queue_alloc(num_bytes); } -void interface_jtag_add_callback(jtag_callback1_t f, jtag_callback_data_t data0); - -void interface_jtag_add_callback4(jtag_callback_t f, jtag_callback_data_t data0, - jtag_callback_data_t data1, jtag_callback_data_t data2, - jtag_callback_data_t data3); - void jtag_add_callback4(jtag_callback_t f, jtag_callback_data_t data0, jtag_callback_data_t data1, jtag_callback_data_t data2, jtag_callback_data_t data3); From 644742b4b218eefd855dc636ccea25b769e80315 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:33:58 +0100 Subject: [PATCH 09/26] driver: mpsse: make local functions static The functions: - mpsse_divide_by_5_config(); - mpsse_purge(); - mpsse_rtck_config(); - mpsse_set_divisor(); are not referenced outside the file. Make them static. Change-Id: Id6930183a3ce26693b2113f622046168ba289df8 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8544 Tested-by: jenkins --- src/jtag/drivers/mpsse.c | 10 ++++++---- src/jtag/drivers/mpsse.h | 4 ---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/jtag/drivers/mpsse.c b/src/jtag/drivers/mpsse.c index 1ef9550a0..38393ad0a 100644 --- a/src/jtag/drivers/mpsse.c +++ b/src/jtag/drivers/mpsse.c @@ -75,6 +75,8 @@ struct mpsse_ctx { int retval; }; +static void mpsse_purge(struct mpsse_ctx *ctx); + /* Returns true if the string descriptor indexed by str_index in device matches string */ static bool string_descriptor_equal(struct libusb_device_handle *device, uint8_t str_index, const char *string) @@ -421,7 +423,7 @@ bool mpsse_is_high_speed(struct mpsse_ctx *ctx) return ctx->type != TYPE_FT2232C; } -void mpsse_purge(struct mpsse_ctx *ctx) +static void mpsse_purge(struct mpsse_ctx *ctx) { int err; LOG_DEBUG("-"); @@ -709,7 +711,7 @@ void mpsse_loopback_config(struct mpsse_ctx *ctx, bool enable) single_byte_boolean_helper(ctx, enable, 0x84, 0x85); } -void mpsse_set_divisor(struct mpsse_ctx *ctx, uint16_t divisor) +static void mpsse_set_divisor(struct mpsse_ctx *ctx, uint16_t divisor) { LOG_DEBUG("%d", divisor); @@ -726,7 +728,7 @@ void mpsse_set_divisor(struct mpsse_ctx *ctx, uint16_t divisor) buffer_write_byte(ctx, divisor >> 8); } -int mpsse_divide_by_5_config(struct mpsse_ctx *ctx, bool enable) +static int mpsse_divide_by_5_config(struct mpsse_ctx *ctx, bool enable) { if (!mpsse_is_high_speed(ctx)) return ERROR_FAIL; @@ -737,7 +739,7 @@ int mpsse_divide_by_5_config(struct mpsse_ctx *ctx, bool enable) return ERROR_OK; } -int mpsse_rtck_config(struct mpsse_ctx *ctx, bool enable) +static int mpsse_rtck_config(struct mpsse_ctx *ctx, bool enable) { if (!mpsse_is_high_speed(ctx)) return ERROR_FAIL; diff --git a/src/jtag/drivers/mpsse.h b/src/jtag/drivers/mpsse.h index 4a625d890..e95f842c9 100644 --- a/src/jtag/drivers/mpsse.h +++ b/src/jtag/drivers/mpsse.h @@ -59,9 +59,6 @@ void mpsse_set_data_bits_high_byte(struct mpsse_ctx *ctx, uint8_t data, uint8_t void mpsse_read_data_bits_low_byte(struct mpsse_ctx *ctx, uint8_t *data); void mpsse_read_data_bits_high_byte(struct mpsse_ctx *ctx, uint8_t *data); void mpsse_loopback_config(struct mpsse_ctx *ctx, bool enable); -void mpsse_set_divisor(struct mpsse_ctx *ctx, uint16_t divisor); -int mpsse_divide_by_5_config(struct mpsse_ctx *ctx, bool enable); -int mpsse_rtck_config(struct mpsse_ctx *ctx, bool enable); /* Helper to set frequency in Hertz. Returns actual realizable frequency or negative error. * Frequency 0 means RTCK. */ @@ -69,6 +66,5 @@ int mpsse_set_frequency(struct mpsse_ctx *ctx, int frequency); /* Queue handling */ int mpsse_flush(struct mpsse_ctx *ctx); -void mpsse_purge(struct mpsse_ctx *ctx); #endif /* OPENOCD_JTAG_DRIVERS_MPSSE_H */ From 4da8f6d27a076a4a64c0d334cb647fdde08938a1 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 18:51:52 +0100 Subject: [PATCH 10/26] rtt: drop unused function rtt_started() The function is not used. Drop it! Change-Id: I176c9d6ba077e36b762c14f9b877d5152992763c Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8545 Tested-by: jenkins Reviewed-by: zapb --- src/rtt/rtt.c | 5 ----- src/rtt/rtt.h | 7 ------- 2 files changed, 12 deletions(-) diff --git a/src/rtt/rtt.c b/src/rtt/rtt.c index e31e75410..42c3ee3ad 100644 --- a/src/rtt/rtt.c +++ b/src/rtt/rtt.c @@ -297,11 +297,6 @@ int rtt_write_channel(unsigned int channel_index, const uint8_t *buffer, length, NULL); } -bool rtt_started(void) -{ - return rtt.started; -} - bool rtt_configured(void) { return rtt.configured; diff --git a/src/rtt/rtt.h b/src/rtt/rtt.h index a5630a951..49409074c 100644 --- a/src/rtt/rtt.h +++ b/src/rtt/rtt.h @@ -194,13 +194,6 @@ int rtt_get_polling_interval(unsigned int *interval); */ int rtt_set_polling_interval(unsigned int interval); -/** - * Get whether RTT is started. - * - * @returns Whether RTT is started. - */ -bool rtt_started(void); - /** * Get whether RTT is configured. * From a34d4b8cb41d99a55ae4dd4ee9f13478a34337a7 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 18:54:14 +0100 Subject: [PATCH 11/26] pld: make get_pld_device_by_num() static The function is not referenced outside the file. Make it static. Change-Id: I5f2a2c70085b9158df8806432bb9ed09bb256ab5 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8546 Tested-by: jenkins --- src/pld/pld.c | 2 +- src/pld/pld.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pld/pld.c b/src/pld/pld.c index 81fb0c463..edd779613 100644 --- a/src/pld/pld.c +++ b/src/pld/pld.c @@ -28,7 +28,7 @@ static struct pld_driver *pld_drivers[] = { static struct pld_device *pld_devices; -struct pld_device *get_pld_device_by_num(int num) +static struct pld_device *get_pld_device_by_num(int num) { struct pld_device *p; int i = 0; diff --git a/src/pld/pld.h b/src/pld/pld.h index 5e2fcd20c..a6e2e0fc8 100644 --- a/src/pld/pld.h +++ b/src/pld/pld.h @@ -54,7 +54,6 @@ struct pld_device { int pld_register_commands(struct command_context *cmd_ctx); -struct pld_device *get_pld_device_by_num(int num); struct pld_device *get_pld_device_by_name(const char *name); struct pld_device *get_pld_device_by_name_or_numstr(const char *str); From f3aeb3d67619510a2601e2a1055479b84a47fbf3 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:06:29 +0100 Subject: [PATCH 12/26] target: dsp563xx: make dsp563xx_once_reg_read_ex() static The function is not referenced outside the file. Make it static. Change-Id: Ifeccc5e38f3da4b4111422860bc1c1447d00f7fe Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8547 Tested-by: jenkins --- src/target/dsp563xx_once.c | 4 +++- src/target/dsp563xx_once.h | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/target/dsp563xx_once.c b/src/target/dsp563xx_once.c index 866f33152..bb41ae8c7 100644 --- a/src/target/dsp563xx_once.c +++ b/src/target/dsp563xx_once.c @@ -34,6 +34,8 @@ #define JTAG_INSTR_DEBUG_REQUEST 0x07 #define JTAG_INSTR_BYPASS 0x0F +static int dsp563xx_once_reg_read_ex(struct jtag_tap *tap, int flush, uint8_t reg, uint8_t len, uint32_t *data); + /** */ static inline int dsp563xx_write_dr(struct jtag_tap *tap, uint8_t *dr_in, uint8_t *dr_out, int dr_len, int rti) { @@ -185,7 +187,7 @@ int dsp563xx_once_read_register(struct jtag_tap *tap, int flush, struct once_reg } /** once read register with register len */ -int dsp563xx_once_reg_read_ex(struct jtag_tap *tap, int flush, uint8_t reg, uint8_t len, uint32_t *data) +static int dsp563xx_once_reg_read_ex(struct jtag_tap *tap, int flush, uint8_t reg, uint8_t len, uint32_t *data) { int err; diff --git a/src/target/dsp563xx_once.h b/src/target/dsp563xx_once.h index 871548837..4f27e1db7 100644 --- a/src/target/dsp563xx_once.h +++ b/src/target/dsp563xx_once.h @@ -65,8 +65,6 @@ int dsp563xx_once_target_status(struct jtag_tap *tap); /** once read registers */ int dsp563xx_once_read_register(struct jtag_tap *tap, int flush, struct once_reg *regs, int len); /** once read register */ -int dsp563xx_once_reg_read_ex(struct jtag_tap *tap, int flush, uint8_t reg, uint8_t len, uint32_t *data); -/** once read register */ int dsp563xx_once_reg_read(struct jtag_tap *tap, int flush, uint8_t reg, uint32_t *data); /** once write register */ int dsp563xx_once_reg_write(struct jtag_tap *tap, int flush, uint8_t reg, uint32_t data); From c5babec7949d3fc23fd3fe43d5452897b7c94553 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:08:25 +0100 Subject: [PATCH 13/26] target: x86_32: make x86_32_common_read_io() static The function is not referenced outside the file. Make it static. Change-Id: Ic2552c040b6b46c0334851a4fc0fdaa400e11e4c Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8548 Tested-by: jenkins --- src/target/x86_32_common.c | 2 +- src/target/x86_32_common.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/target/x86_32_common.c b/src/target/x86_32_common.c index 2c60b9f7e..8cca9a5e9 100644 --- a/src/target/x86_32_common.c +++ b/src/target/x86_32_common.c @@ -677,7 +677,7 @@ int x86_32_common_write_memory(struct target *t, target_addr_t addr, return retval; } -int x86_32_common_read_io(struct target *t, uint32_t addr, +static int x86_32_common_read_io(struct target *t, uint32_t addr, uint32_t size, uint8_t *buf) { struct x86_32_common *x86_32 = target_to_x86_32(t); diff --git a/src/target/x86_32_common.h b/src/target/x86_32_common.h index 7392447a6..e23274769 100644 --- a/src/target/x86_32_common.h +++ b/src/target/x86_32_common.h @@ -309,8 +309,6 @@ int x86_32_common_read_memory(struct target *t, target_addr_t addr, uint32_t size, uint32_t count, uint8_t *buf); int x86_32_common_write_memory(struct target *t, target_addr_t addr, uint32_t size, uint32_t count, const uint8_t *buf); -int x86_32_common_read_io(struct target *t, uint32_t addr, - uint32_t size, uint8_t *buf); int x86_32_common_write_io(struct target *t, uint32_t addr, uint32_t size, const uint8_t *buf); int x86_32_common_add_breakpoint(struct target *t, struct breakpoint *bp); From df42faf51d20d879beefacbd88b11271bcee181a Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:12:46 +0100 Subject: [PATCH 14/26] target: aarch64: drop unused armv8_mmu_translate_va() The function is not used. Drop it! Change-Id: I1625e03714b5a842f668098191c39cce34f815e8 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8549 Tested-by: jenkins --- src/target/armv8.c | 6 ------ src/target/armv8.h | 1 - 2 files changed, 7 deletions(-) diff --git a/src/target/armv8.c b/src/target/armv8.c index 61d72741a..88534d962 100644 --- a/src/target/armv8.c +++ b/src/target/armv8.c @@ -1021,12 +1021,6 @@ static __attribute__((unused)) void armv8_show_fault_registers(struct target *ta armv8_show_fault_registers32(armv8); } -/* method adapted to cortex A : reused arm v4 v5 method*/ -int armv8_mmu_translate_va(struct target *target, target_addr_t va, target_addr_t *val) -{ - return ERROR_OK; -} - static void armv8_decode_cacheability(int attr) { if (attr == 0) { diff --git a/src/target/armv8.h b/src/target/armv8.h index 349c8f002..49ab3e5cb 100644 --- a/src/target/armv8.h +++ b/src/target/armv8.h @@ -298,7 +298,6 @@ int armv8_identify_cache(struct armv8_common *armv8); int armv8_init_arch_info(struct target *target, struct armv8_common *armv8); int armv8_mmu_translate_va_pa(struct target *target, target_addr_t va, target_addr_t *val, int meminfo); -int armv8_mmu_translate_va(struct target *target, target_addr_t va, target_addr_t *val); int armv8_handle_cache_info_command(struct command_invocation *cmd, struct armv8_cache_common *armv8_cache); From b04a58e3fcbfd9a701126473279fa17d37f23784 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:46:03 +0100 Subject: [PATCH 15/26] target: esirisc: make local functions static The function esirisc_jtag_get_eid() is not used outside the file. Make it static. The function esirisc_jtag_disable_debug() is never used. Make it static and mark it as unused. Change-Id: I5c99cbf77cc9c527b6e18a3f67caa24f8551d09c Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8550 Tested-by: jenkins --- src/target/esirisc_jtag.c | 6 ++++-- src/target/esirisc_jtag.h | 2 -- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/target/esirisc_jtag.c b/src/target/esirisc_jtag.c index 5960e26cb..ad2bef543 100644 --- a/src/target/esirisc_jtag.c +++ b/src/target/esirisc_jtag.c @@ -19,6 +19,8 @@ #include "esirisc_jtag.h" +static uint8_t esirisc_jtag_get_eid(struct esirisc_jtag *jtag_info); + static void esirisc_jtag_set_instr(struct esirisc_jtag *jtag_info, uint32_t new_instr) { struct jtag_tap *tap = jtag_info->tap; @@ -221,7 +223,7 @@ bool esirisc_jtag_is_stopped(struct esirisc_jtag *jtag_info) return !!(jtag_info->status & 1<<6); /* S */ } -uint8_t esirisc_jtag_get_eid(struct esirisc_jtag *jtag_info) +static uint8_t esirisc_jtag_get_eid(struct esirisc_jtag *jtag_info) { return jtag_info->status & 0x3f; /* EID */ } @@ -490,7 +492,7 @@ int esirisc_jtag_enable_debug(struct esirisc_jtag *jtag_info) return esirisc_jtag_send_ctrl(jtag_info, DEBUG_ENABLE_DEBUG); } -int esirisc_jtag_disable_debug(struct esirisc_jtag *jtag_info) +static __attribute__((unused)) int esirisc_jtag_disable_debug(struct esirisc_jtag *jtag_info) { return esirisc_jtag_send_ctrl(jtag_info, DEBUG_DISABLE_DEBUG); } diff --git a/src/target/esirisc_jtag.h b/src/target/esirisc_jtag.h index 98ec8af8d..d59b75fbe 100644 --- a/src/target/esirisc_jtag.h +++ b/src/target/esirisc_jtag.h @@ -54,7 +54,6 @@ struct esirisc_jtag { bool esirisc_jtag_is_debug_active(struct esirisc_jtag *jtag_info); bool esirisc_jtag_is_stopped(struct esirisc_jtag *jtag_info); -uint8_t esirisc_jtag_get_eid(struct esirisc_jtag *jtag_info); int esirisc_jtag_read_byte(struct esirisc_jtag *jtag_info, uint32_t address, uint8_t *data); @@ -81,7 +80,6 @@ int esirisc_jtag_write_csr(struct esirisc_jtag *jtag_info, uint8_t bank, uint8_t csr, uint32_t data); int esirisc_jtag_enable_debug(struct esirisc_jtag *jtag_info); -int esirisc_jtag_disable_debug(struct esirisc_jtag *jtag_info); int esirisc_jtag_assert_reset(struct esirisc_jtag *jtag_info); int esirisc_jtag_deassert_reset(struct esirisc_jtag *jtag_info); From 61fbcbeca83452f3a035f3f4982703096fb5d8ef Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 9 Nov 2024 19:21:30 +0100 Subject: [PATCH 16/26] semihosting: make local functions static The functions: - semihosting_opcode_to_str(); - semihosting_write_fields(); - semihosting_set_field(); are not referenced outside the file. Make them static. Change-Id: Ia8d35554673145fdfe0e501543eb18919863039f Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8551 Tested-by: jenkins --- src/target/semihosting_common.c | 16 ++++++++++------ src/target/semihosting_common.h | 12 ------------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/target/semihosting_common.c b/src/target/semihosting_common.c index 1eb195712..ffcd3aafd 100644 --- a/src/target/semihosting_common.c +++ b/src/target/semihosting_common.c @@ -92,6 +92,8 @@ static int semihosting_common_fileio_info(struct target *target, struct gdb_fileio_info *fileio_info); static int semihosting_common_fileio_end(struct target *target, int result, int fileio_errno, bool ctrl_c); +static void semihosting_set_field(struct target *target, uint64_t value, size_t index, uint8_t *fields); +static int semihosting_write_fields(struct target *target, size_t number, uint8_t *fields); /** * Initialize common semihosting support. @@ -298,7 +300,12 @@ static inline int semihosting_getchar(struct semihosting *semihosting, int fd) */ static char *semihosting_user_op_params; -const char *semihosting_opcode_to_str(const uint64_t opcode) +/** + * @brief Convert the syscall opcode to a human-readable string + * @param[in] opcode Syscall opcode + * @return String representation of syscall opcode + */ +static const char *semihosting_opcode_to_str(const uint64_t opcode) { switch (opcode) { case SEMIHOSTING_SYS_CLOSE: @@ -1729,8 +1736,7 @@ int semihosting_read_fields(struct target *target, size_t number, /** * Write all fields of a command from buffer to target. */ -int semihosting_write_fields(struct target *target, size_t number, - uint8_t *fields) +static int semihosting_write_fields(struct target *target, size_t number, uint8_t *fields) { struct semihosting *semihosting = target->semihosting; /* Use 4-byte multiples to trigger fast memory access. */ @@ -1754,9 +1760,7 @@ uint64_t semihosting_get_field(struct target *target, size_t index, /** * Store a field in the buffer, considering register size and endianness. */ -void semihosting_set_field(struct target *target, uint64_t value, - size_t index, - uint8_t *fields) +static void semihosting_set_field(struct target *target, uint64_t value, size_t index, uint8_t *fields) { struct semihosting *semihosting = target->semihosting; if (semihosting->word_size_bytes == 8) diff --git a/src/target/semihosting_common.h b/src/target/semihosting_common.h index a1848b488..1821ca4e2 100644 --- a/src/target/semihosting_common.h +++ b/src/target/semihosting_common.h @@ -188,13 +188,6 @@ struct semihosting { int (*post_result)(struct target *target); }; -/** - * @brief Convert the syscall opcode to a human-readable string - * @param[in] opcode Syscall opcode - * @return String representation of syscall opcode - */ -const char *semihosting_opcode_to_str(uint64_t opcode); - int semihosting_common_init(struct target *target, void *setup, void *post_result); int semihosting_common(struct target *target); @@ -202,13 +195,8 @@ int semihosting_common(struct target *target); /* utility functions which may also be used by semihosting extensions (custom vendor-defined syscalls) */ int semihosting_read_fields(struct target *target, size_t number, uint8_t *fields); -int semihosting_write_fields(struct target *target, size_t number, - uint8_t *fields); uint64_t semihosting_get_field(struct target *target, size_t index, uint8_t *fields); -void semihosting_set_field(struct target *target, uint64_t value, - size_t index, - uint8_t *fields); extern const struct command_registration semihosting_common_handlers[]; From 8c739a45a042cf7a9b5bba48f0e20cbc3168ee73 Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Mon, 21 Oct 2024 10:39:28 +0200 Subject: [PATCH 17/26] helper/jim-nvp.h: Rework 'isconfigure' variable Change the variable name to 'is_configure' to be compatible with the coding style and use 'bool' as data type. Change-Id: I8609f9807c8bd14eaf6c93acf63fd51b55c9bbbb Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8573 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/helper/jim-nvp.h | 3 ++- src/rtos/rtos.c | 2 +- src/target/aarch64.c | 2 +- src/target/arm_adi_v5.c | 6 +++--- src/target/arm_cti.c | 2 +- src/target/arm_tpiu_swo.c | 22 +++++++++++----------- src/target/target.c | 30 +++++++++++++++--------------- 7 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/helper/jim-nvp.h b/src/helper/jim-nvp.h index 11824cabf..45af6c867 100644 --- a/src/helper/jim-nvp.h +++ b/src/helper/jim-nvp.h @@ -19,6 +19,7 @@ #ifndef OPENOCD_HELPER_JIM_NVP_H #define OPENOCD_HELPER_JIM_NVP_H +#include #include /** Name Value Pairs, aka: NVP @@ -136,7 +137,7 @@ struct jim_getopt_info { Jim_Interp *interp; int argc; Jim_Obj *const *argv; - int isconfigure; /* non-zero if configure */ + bool is_configure; }; /** GetOpt - how to. diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c index 54e31e426..cc0fecebe 100644 --- a/src/rtos/rtos.c +++ b/src/rtos/rtos.c @@ -103,7 +103,7 @@ int rtos_create(struct jim_getopt_info *goi, struct target *target) Jim_Obj *res; int e; - if (!goi->isconfigure && goi->argc != 0) { + if (!goi->is_configure && goi->argc != 0) { Jim_WrongNumArgs(goi->interp, goi->argc, goi->argv, "NO PARAMS"); return JIM_ERR; } diff --git a/src/target/aarch64.c b/src/target/aarch64.c index f0d486f58..9f122070a 100644 --- a/src/target/aarch64.c +++ b/src/target/aarch64.c @@ -2940,7 +2940,7 @@ static int aarch64_jim_configure(struct target *target, struct jim_getopt_info * switch (n->value) { case CFG_CTI: { - if (goi->isconfigure) { + if (goi->is_configure) { Jim_Obj *o_cti; struct arm_cti *cti; e = jim_getopt_obj(goi, &o_cti); diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c index eaaa209b9..0c7633bea 100644 --- a/src/target/arm_adi_v5.c +++ b/src/target/arm_adi_v5.c @@ -2359,7 +2359,7 @@ static int adiv5_jim_spot_configure(struct jim_getopt_info *goi, switch (n->value) { case CFG_DAP: - if (goi->isconfigure) { + if (goi->is_configure) { Jim_Obj *o_t; struct adiv5_dap *dap; e = jim_getopt_obj(goi, &o_t); @@ -2388,7 +2388,7 @@ static int adiv5_jim_spot_configure(struct jim_getopt_info *goi, break; case CFG_AP_NUM: - if (goi->isconfigure) { + if (goi->is_configure) { /* jim_wide is a signed 64 bits int, ap_num is unsigned with max 52 bits */ jim_wide ap_num; e = jim_getopt_wide(goi, &ap_num); @@ -2415,7 +2415,7 @@ static int adiv5_jim_spot_configure(struct jim_getopt_info *goi, LOG_WARNING("DEPRECATED! use \'-baseaddr' not \'-ctibase\'"); /* fall through */ case CFG_BASEADDR: - if (goi->isconfigure) { + if (goi->is_configure) { jim_wide base; e = jim_getopt_wide(goi, &base); if (e != JIM_OK) diff --git a/src/target/arm_cti.c b/src/target/arm_cti.c index 97d1fb34b..88eec832e 100644 --- a/src/target/arm_cti.c +++ b/src/target/arm_cti.c @@ -468,7 +468,7 @@ static int cti_create(struct jim_getopt_info *goi) adiv5_mem_ap_spot_init(&cti->spot); /* Do the rest as "configure" options */ - goi->isconfigure = 1; + goi->is_configure = true; e = cti_configure(goi, cti); if (e != JIM_OK) { free(cti); diff --git a/src/target/arm_tpiu_swo.c b/src/target/arm_tpiu_swo.c index 55a977844..c14fd5fc8 100644 --- a/src/target/arm_tpiu_swo.c +++ b/src/target/arm_tpiu_swo.c @@ -358,7 +358,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s { assert(obj); - if (goi->isconfigure && obj->enabled) { + if (goi->is_configure && obj->enabled) { Jim_SetResultFormatted(goi->interp, "Cannot configure TPIU/SWO; %s is enabled!", obj->name); return JIM_ERR; } @@ -382,7 +382,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s switch (n->value) { case CFG_PORT_WIDTH: - if (goi->isconfigure) { + if (goi->is_configure) { jim_wide port_width; e = jim_getopt_wide(goi, &port_width); if (e != JIM_OK) @@ -399,7 +399,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s } break; case CFG_PROTOCOL: - if (goi->isconfigure) { + if (goi->is_configure) { struct jim_nvp *p; e = jim_getopt_nvp(goi, nvp_arm_tpiu_swo_protocol_opts, &p); if (e != JIM_OK) @@ -418,7 +418,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s } break; case CFG_FORMATTER: - if (goi->isconfigure) { + if (goi->is_configure) { struct jim_nvp *p; e = jim_getopt_nvp(goi, nvp_arm_tpiu_swo_bool_opts, &p); if (e != JIM_OK) @@ -437,7 +437,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s } break; case CFG_TRACECLKIN: - if (goi->isconfigure) { + if (goi->is_configure) { jim_wide clk; e = jim_getopt_wide(goi, &clk); if (e != JIM_OK) @@ -450,7 +450,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s } break; case CFG_BITRATE: - if (goi->isconfigure) { + if (goi->is_configure) { jim_wide clk; e = jim_getopt_wide(goi, &clk); if (e != JIM_OK) @@ -463,7 +463,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s } break; case CFG_OUTFILE: - if (goi->isconfigure) { + if (goi->is_configure) { const char *s; e = jim_getopt_string(goi, &s, NULL); if (e != JIM_OK) @@ -491,7 +491,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s } break; case CFG_EVENT: - if (goi->isconfigure) { + if (goi->is_configure) { if (goi->argc < 2) { Jim_WrongNumArgs(goi->interp, goi->argc, goi->argv, "-event ?event-name? ?EVENT-BODY?"); return JIM_ERR; @@ -521,7 +521,7 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s ea = ea->next; } - if (goi->isconfigure) { + if (goi->is_configure) { if (!ea) { ea = calloc(1, sizeof(*ea)); if (!ea) { @@ -560,7 +560,7 @@ static int jim_arm_tpiu_swo_configure(Jim_Interp *interp, int argc, Jim_Obj * co struct jim_getopt_info goi; jim_getopt_setup(&goi, interp, argc - 1, argv + 1); - goi.isconfigure = !strcmp(c->name, "configure"); + goi.is_configure = !strcmp(c->name, "configure"); if (goi.argc < 1) { Jim_WrongNumArgs(goi.interp, goi.argc, goi.argv, "missing: -option ..."); @@ -977,7 +977,7 @@ static int jim_arm_tpiu_swo_create(Jim_Interp *interp, int argc, Jim_Obj *const } /* Do the rest as "configure" options */ - goi.isconfigure = 1; + goi.is_configure = true; int e = arm_tpiu_swo_configure(&goi, obj); if (e != JIM_OK) goto err_exit; diff --git a/src/target/target.c b/src/target/target.c index 49611dfb4..6c474899a 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -4937,7 +4937,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) switch (n->value) { case TCFG_TYPE: /* not settable */ - if (goi->isconfigure) { + if (goi->is_configure) { Jim_SetResultFormatted(goi->interp, "not settable: %s", n->name); return JIM_ERR; @@ -4966,7 +4966,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) return e; } - if (goi->isconfigure) { + if (goi->is_configure) { if (goi->argc != 1) { Jim_WrongNumArgs(goi->interp, goi->argc, goi->argv, "-event ?event-name? ?EVENT-BODY?"); return JIM_ERR; @@ -4989,7 +4989,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) teap = teap->next; } - if (goi->isconfigure) { + if (goi->is_configure) { /* START_DEPRECATED_TPIU */ if (n->value == TARGET_EVENT_TRACE_CONFIG) LOG_INFO("DEPRECATED target event %s; use TPIU events {pre,post}-{enable,disable}", n->name); @@ -5037,7 +5037,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_WORK_AREA_VIRT: - if (goi->isconfigure) { + if (goi->is_configure) { target_free_all_working_areas(target); e = jim_getopt_wide(goi, &w); if (e != JIM_OK) @@ -5053,7 +5053,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_WORK_AREA_PHYS: - if (goi->isconfigure) { + if (goi->is_configure) { target_free_all_working_areas(target); e = jim_getopt_wide(goi, &w); if (e != JIM_OK) @@ -5069,7 +5069,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_WORK_AREA_SIZE: - if (goi->isconfigure) { + if (goi->is_configure) { target_free_all_working_areas(target); e = jim_getopt_wide(goi, &w); if (e != JIM_OK) @@ -5084,7 +5084,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_WORK_AREA_BACKUP: - if (goi->isconfigure) { + if (goi->is_configure) { target_free_all_working_areas(target); e = jim_getopt_wide(goi, &w); if (e != JIM_OK) @@ -5101,7 +5101,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) case TCFG_ENDIAN: - if (goi->isconfigure) { + if (goi->is_configure) { e = jim_getopt_nvp(goi, nvp_target_endian, &n); if (e != JIM_OK) { jim_getopt_nvp_unknown(goi, nvp_target_endian, 1); @@ -5122,7 +5122,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_COREID: - if (goi->isconfigure) { + if (goi->is_configure) { e = jim_getopt_wide(goi, &w); if (e != JIM_OK) return e; @@ -5136,7 +5136,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_CHAIN_POSITION: - if (goi->isconfigure) { + if (goi->is_configure) { Jim_Obj *o_t; struct jtag_tap *tap; @@ -5163,7 +5163,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) /* loop for more e*/ break; case TCFG_DBGBASE: - if (goi->isconfigure) { + if (goi->is_configure) { e = jim_getopt_wide(goi, &w); if (e != JIM_OK) return e; @@ -5193,7 +5193,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_GDB_PORT: - if (goi->isconfigure) { + if (goi->is_configure) { struct command_context *cmd_ctx = current_command_context(goi->interp); if (cmd_ctx->mode != COMMAND_CONFIG) { Jim_SetResultString(goi->interp, "-gdb-port must be configured before 'init'", -1); @@ -5215,7 +5215,7 @@ static int target_configure(struct jim_getopt_info *goi, struct target *target) break; case TCFG_GDB_MAX_CONNECTIONS: - if (goi->isconfigure) { + if (goi->is_configure) { struct command_context *cmd_ctx = current_command_context(goi->interp); if (cmd_ctx->mode != COMMAND_CONFIG) { Jim_SetResultString(goi->interp, "-gdb-max-connections must be configured before 'init'", -1); @@ -5246,7 +5246,7 @@ static int jim_target_configure(Jim_Interp *interp, int argc, Jim_Obj * const *a struct jim_getopt_info goi; jim_getopt_setup(&goi, interp, argc - 1, argv + 1); - goi.isconfigure = !strcmp(c->name, "configure"); + goi.is_configure = !strcmp(c->name, "configure"); if (goi.argc < 1) { Jim_WrongNumArgs(goi.interp, goi.argc, goi.argv, "missing: -option ..."); @@ -5823,7 +5823,7 @@ static int target_create(struct jim_getopt_info *goi) target->gdb_max_connections = 1; /* Do the rest as "configure" options */ - goi->isconfigure = 1; + goi->is_configure = true; e = target_configure(goi, target); if (e == JIM_OK) { From c582cfbf758fdf188b02d8e57e0f57ce9c36663e Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sun, 10 Nov 2024 14:30:25 +0100 Subject: [PATCH 18/26] driver: stlink: get adapter speed through adapter_get_speed_khz() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stlink driver, both in dapdirect and in HLA modes, pretends to store locally the value of the adapter speed in order to use it later-on during adapter initialization. It doesn't work in dapdirect mode since the code to store locally the value will not be executed until the adapter is already fully initialized. This cause an issue in dapdirect mode: - due to the local value, still kept at -1, the adapter will be initialized to the lowest clock speed (5 KHz on stlink v2 in SWD mode); - after the adapter initialization the framework will again set the speed with the value requested by the user. Some target, like nRF51822, only accepts JTAG/SWD speed in a defined range of frequencies. The initial speed of 5 KHz used by dapdirect can be out of range, making the target debug port not working. The adapter framework already stores the value of speed and makes it available through adapter_get_speed_khz(). Drop struct hl_interface_param::initial_interface_speed. Let the code to use adapter_get_speed_khz(). Change-Id: Ie11bf0234574f2a9180d3d3a16efb78e08dfcd86 Reported-by: Andrzej Sierżęga Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8574 Reviewed-by: Andrzej Sierżęga Tested-by: jenkins --- src/jtag/drivers/stlink_usb.c | 3 +-- src/jtag/hla/hla_interface.c | 6 +----- src/jtag/hla/hla_interface.h | 2 -- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/jtag/drivers/stlink_usb.c b/src/jtag/drivers/stlink_usb.c index 0385e4d85..d77f28b0f 100644 --- a/src/jtag/drivers/stlink_usb.c +++ b/src/jtag/drivers/stlink_usb.c @@ -3781,7 +3781,7 @@ static int stlink_open(struct hl_interface_param *param, enum stlink_mode mode, } /* initialize the debug hardware */ - err = stlink_usb_init_mode(h, param->connect_under_reset, param->initial_interface_speed); + err = stlink_usb_init_mode(h, param->connect_under_reset, adapter_get_speed_khz()); if (err != ERROR_OK) { LOG_ERROR("init mode failed (unable to connect to the target)"); @@ -5174,7 +5174,6 @@ static int stlink_dap_speed(int speed) return ERROR_JTAG_NOT_IMPLEMENTED; } - stlink_dap_param.initial_interface_speed = speed; stlink_speed(stlink_dap_handle, speed, false); return ERROR_OK; } diff --git a/src/jtag/hla/hla_interface.c b/src/jtag/hla/hla_interface.c index e826f7910..96862b0d0 100644 --- a/src/jtag/hla/hla_interface.c +++ b/src/jtag/hla/hla_interface.c @@ -30,7 +30,6 @@ static struct hl_interface hl_if = { .pid = { 0 }, .transport = HL_TRANSPORT_UNKNOWN, .connect_under_reset = false, - .initial_interface_speed = -1, .use_stlink_tcp = false, .stlink_tcp_port = 7184, }, @@ -165,11 +164,8 @@ static int hl_interface_speed(int speed) if (!hl_if.layout->api->speed) return ERROR_OK; - if (!hl_if.handle) { - /* pass speed as initial param as interface not open yet */ - hl_if.param.initial_interface_speed = speed; + if (!hl_if.handle) return ERROR_OK; - } hl_if.layout->api->speed(hl_if.handle, speed, false); diff --git a/src/jtag/hla/hla_interface.h b/src/jtag/hla/hla_interface.h index c95638b92..f1550d991 100644 --- a/src/jtag/hla/hla_interface.h +++ b/src/jtag/hla/hla_interface.h @@ -31,8 +31,6 @@ struct hl_interface_param { enum hl_transports transport; /** */ bool connect_under_reset; - /** Initial interface clock clock speed */ - int initial_interface_speed; /** */ bool use_stlink_tcp; /** */ From 9ff79fd61fb0bd763d09820672e9e7ce50df4c41 Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Sat, 2 Nov 2024 23:34:23 +0100 Subject: [PATCH 19/26] enable the Bus Pirate adapter by default on most systems Also convert the Bus Pirate to the common PROCESS_ADAPTERS logic. Change-Id: Ifa8ebcee380c16d7e308ba7a75dbffdb74208285 Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8533 Reviewed-by: Antonio Borneo Reviewed-by: R. Diez Tested-by: jenkins --- configure.ac | 19 ++++++++----------- src/jtag/drivers/Makefile.am | 2 +- src/jtag/interfaces.c | 2 +- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 291e854a4..687c5c974 100644 --- a/configure.ac +++ b/configure.ac @@ -414,6 +414,8 @@ AS_CASE(["${host_cpu}"], parport_use_ppdev=yes ]) +can_build_buspirate=yes + AS_CASE([$host], [*-cygwin*], [ is_win32=yes @@ -445,12 +447,12 @@ AS_CASE([$host], ]) parport_use_giveio=yes - AS_IF([test "x$enable_buspirate" = "xyes"], [ - AC_MSG_ERROR([buspirate currently not supported by MinGW32 hosts]) + AS_IF([test "x$ADAPTER_VAR([buspirate])" = "xyes"], [ + AC_MSG_ERROR([The Bus Pirate adapter is currently not supported by MinGW32 hosts.]) ]) # In case enable_buspirate=auto, make sure it will not be built. - enable_buspirate=no + can_build_buspirate=no AC_SUBST([HOST_CPPFLAGS], ["-D__USE_MINGW_ANSI_STDIO -DFD_SETSIZE=128"]) ], @@ -594,12 +596,6 @@ AS_IF([test "x$build_gw16012" = "xyes"], [ AC_DEFINE([BUILD_GW16012], [0], [0 if you don't want the Gateworks GW16012 driver.]) ]) -AS_IF([test "x$enable_buspirate" != "xno"], [ - AC_DEFINE([BUILD_BUSPIRATE], [1], [1 if you want the Buspirate JTAG driver.]) -], [ - AC_DEFINE([BUILD_BUSPIRATE], [0], [0 if you don't want the Buspirate JTAG driver.]) -]) - AS_IF([test "x$use_internal_jimtcl" = "xyes"], [ AS_IF([test -f "$srcdir/jimtcl/configure"], [ AS_IF([test "x$use_internal_jimtcl_maintainer" = "xyes"], [ @@ -712,7 +708,7 @@ m4_define([PROCESS_ADAPTERS], [ ]) ], [ AS_IF([test "x$ADAPTER_VAR([adapter])" = "xyes"], [ - AC_MSG_ERROR([$3 is required for [adapter] ADAPTER_DESC([adapter]).]) + AC_MSG_ERROR([$3 is required for [adapter] "ADAPTER_DESC([adapter])".]) ]) ADAPTER_VAR([adapter])=no AC_DEFINE([BUILD_]ADAPTER_SYM([adapter]), [0], [0 if you do not want the ]ADAPTER_DESC([adapter]).) @@ -729,6 +725,8 @@ PROCESS_ADAPTERS([LIBFTDI_USB1_ADAPTERS], ["x$use_libftdi" = "xyes" -a "x$use_li PROCESS_ADAPTERS([LIBGPIOD_ADAPTERS], ["x$use_libgpiod" = "xyes"], [libgpiod]) PROCESS_ADAPTERS([LIBJAYLINK_ADAPTERS], ["x$use_internal_libjaylink" = "xyes" -o "x$use_libjaylink" = "xyes"], [libjaylink-0.2]) PROCESS_ADAPTERS([PCIE_ADAPTERS], ["x$is_linux" = "xyes"], [Linux build]) +PROCESS_ADAPTERS([SERIAL_PORT_ADAPTERS], ["x$can_build_buspirate" = "xyes"], + [internal error: validation should happen beforehand]) PROCESS_ADAPTERS([DUMMY_ADAPTER], [true], [unused]) AS_IF([test "x$enable_linuxgpiod" != "xno"], [ @@ -783,7 +781,6 @@ AM_CONDITIONAL([USB_BLASTER_DRIVER], [test "x$enable_usb_blaster" != "xno" -o "x AM_CONDITIONAL([AMTJTAGACCEL], [test "x$build_amtjtagaccel" = "xyes"]) AM_CONDITIONAL([GW16012], [test "x$build_gw16012" = "xyes"]) AM_CONDITIONAL([REMOTE_BITBANG], [test "x$build_remote_bitbang" = "xyes"]) -AM_CONDITIONAL([BUSPIRATE], [test "x$enable_buspirate" != "xno"]) AM_CONDITIONAL([SYSFSGPIO], [test "x$build_sysfsgpio" = "xyes"]) AM_CONDITIONAL([USE_LIBUSB1], [test "x$use_libusb1" = "xyes"]) AM_CONDITIONAL([IS_CYGWIN], [test "x$is_cygwin" = "xyes"]) diff --git a/src/jtag/drivers/Makefile.am b/src/jtag/drivers/Makefile.am index e404afe9f..8be834859 100644 --- a/src/jtag/drivers/Makefile.am +++ b/src/jtag/drivers/Makefile.am @@ -143,7 +143,7 @@ endif if ARMJTAGEW DRIVERFILES += %D%/arm-jtag-ew.c endif -if BUSPIRATE +if BUS_PIRATE DRIVERFILES += %D%/buspirate.c endif if REMOTE_BITBANG diff --git a/src/jtag/interfaces.c b/src/jtag/interfaces.c index c24ead8cd..67f0838e3 100644 --- a/src/jtag/interfaces.c +++ b/src/jtag/interfaces.c @@ -102,7 +102,7 @@ struct adapter_driver *adapter_drivers[] = { #if BUILD_ARMJTAGEW == 1 &armjtagew_adapter_driver, #endif -#if BUILD_BUSPIRATE == 1 +#if BUILD_BUS_PIRATE == 1 &buspirate_adapter_driver, #endif #if BUILD_REMOTE_BITBANG == 1 From 2627f8ce6daebd4ab148165a8797595c04052695 Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Sun, 3 Nov 2024 10:52:45 +0100 Subject: [PATCH 20/26] configure.ac: show the linuxgpiod adapter in the configuration summary List AC_ARG_ADAPTERS was missing a comma separating two of the elements. Also verify that each adapter is set to either 'auto', 'yes' or 'no', which should prevent such issues from going unnoticed in the future. Change-Id: I0d407e03b1e5a3edc61d7dc93d5ffa70fe079b3c Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8534 Tested-by: jenkins Reviewed-by: R. Diez Reviewed-by: Antonio Borneo --- configure.ac | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 687c5c974..31e57a3cd 100644 --- a/configure.ac +++ b/configure.ac @@ -283,7 +283,7 @@ AC_ARG_ADAPTERS([ HIDAPI_ADAPTERS, HIDAPI_USB1_ADAPTERS, LIBFTDI_ADAPTERS, - LIBFTDI_USB1_ADAPTERS + LIBFTDI_USB1_ADAPTERS, LIBGPIOD_ADAPTERS, SERIAL_PORT_ADAPTERS, PCIE_ADAPTERS, @@ -372,10 +372,6 @@ AS_CASE([$host_os], AC_MSG_ERROR([sysfsgpio is only available on linux]) ]) - AS_IF([test "x$enable_linuxgpiod" = "xyes"], [ - AC_MSG_ERROR([linuxgpiod is only available on linux]) - ]) - AS_CASE([$host_os], [freebsd*], [], [ AS_IF([test "x$build_rshim" = "xyes"], [ @@ -722,7 +718,7 @@ PROCESS_ADAPTERS([HIDAPI_ADAPTERS], ["x$use_hidapi" = "xyes"], [hidapi]) PROCESS_ADAPTERS([HIDAPI_USB1_ADAPTERS], ["x$use_hidapi" = "xyes" -a "x$use_libusb1" = "xyes"], [hidapi and libusb-1.x]) PROCESS_ADAPTERS([LIBFTDI_ADAPTERS], ["x$use_libftdi" = "xyes"], [libftdi]) PROCESS_ADAPTERS([LIBFTDI_USB1_ADAPTERS], ["x$use_libftdi" = "xyes" -a "x$use_libusb1" = "xyes"], [libftdi and libusb-1.x]) -PROCESS_ADAPTERS([LIBGPIOD_ADAPTERS], ["x$use_libgpiod" = "xyes"], [libgpiod]) +PROCESS_ADAPTERS([LIBGPIOD_ADAPTERS], ["x$use_libgpiod" = "xyes"], [Linux libgpiod]) PROCESS_ADAPTERS([LIBJAYLINK_ADAPTERS], ["x$use_internal_libjaylink" = "xyes" -o "x$use_libjaylink" = "xyes"], [libjaylink-0.2]) PROCESS_ADAPTERS([PCIE_ADAPTERS], ["x$is_linux" = "xyes"], [Linux build]) PROCESS_ADAPTERS([SERIAL_PORT_ADAPTERS], ["x$can_build_buspirate" = "xyes"], @@ -889,6 +885,11 @@ m4_foreach([adapter], [USB1_ADAPTERS, ], [no], [ echo "$s"no + ], + [ + AC_MSG_ERROR(m4_normalize([ + Error in [adapter] "ADAPTER_ARG([adapter])": Variable "ADAPTER_VAR([adapter])" + has invalid value "$ADAPTER_VAR([adapter])".])) ]) ]) echo From 11f24fc2f2bef1868e17f9b3b53ec8264b164611 Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Tue, 5 Nov 2024 14:40:50 +0100 Subject: [PATCH 21/26] configure.ac: improve validation of some --enable-xxx options Catch an invalid option like "--enable-buspirate=rubbish". Also mention all valid values in the help text for those options. Change-Id: Ib0fb8904132d07cc5cde421aa816ca6971a08769 Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8540 Reviewed-by: R. Diez Reviewed-by: Antonio Borneo Tested-by: jenkins --- configure.ac | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 31e57a3cd..567152b0a 100644 --- a/configure.ac +++ b/configure.ac @@ -272,9 +272,13 @@ AC_ARG_ENABLE([dmem], m4_define([AC_ARG_ADAPTERS], [ m4_foreach([adapter], [$1], [AC_ARG_ENABLE(ADAPTER_OPT([adapter]), - AS_HELP_STRING([--enable-ADAPTER_OPT([adapter])], + AS_HELP_STRING([--enable-ADAPTER_OPT([adapter])[[[=yes/no/auto]]]], [Enable building support for the ]ADAPTER_DESC([adapter])[ (default is $2)]), - [], [ADAPTER_VAR([adapter])=$2]) + [case "${enableval}" in + yes|no|auto) ;; + *) AC_MSG_ERROR([Option --enable-ADAPTER_OPT([adapter]) has invalid value "${enableval}".]) ;; + esac], + [ADAPTER_VAR([adapter])=$2]) ]) ]) From f5036aff3a83f9b92ee5939e5a32a13396b53dd5 Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Tue, 22 Oct 2024 17:32:37 +0200 Subject: [PATCH 22/26] target/xtensa: Remove 'ERROR: ' prefix in error log Remove the prefix since it is redundant. Change-Id: I9c23c0479ba40be24e471309e720060cd03763ee Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8577 Tested-by: jenkins Reviewed-by: Ian Thompson Reviewed-by: Antonio Borneo --- src/target/xtensa/xtensa.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/target/xtensa/xtensa.c b/src/target/xtensa/xtensa.c index 8369cc4e5..284bfc9c6 100644 --- a/src/target/xtensa/xtensa.c +++ b/src/target/xtensa/xtensa.c @@ -3000,13 +3000,13 @@ static int xtensa_build_reg_cache(struct target *target) /* Construct empty-register list for handling unknown register requests */ xtensa->empty_regs = calloc(xtensa->dbregs_num, sizeof(struct reg)); if (!xtensa->empty_regs) { - LOG_TARGET_ERROR(target, "ERROR: Out of memory"); + LOG_TARGET_ERROR(target, "Out of memory"); goto fail; } for (unsigned int i = 0; i < xtensa->dbregs_num; i++) { xtensa->empty_regs[i].name = calloc(8, sizeof(char)); if (!xtensa->empty_regs[i].name) { - LOG_TARGET_ERROR(target, "ERROR: Out of memory"); + LOG_TARGET_ERROR(target, "Out of memory"); goto fail; } sprintf((char *)xtensa->empty_regs[i].name, "?0x%04x", i & 0x0000FFFF); @@ -3024,7 +3024,7 @@ static int xtensa_build_reg_cache(struct target *target) if (xtensa->regmap_contiguous && xtensa->contiguous_regs_desc) { xtensa->contiguous_regs_list = calloc(xtensa->total_regs_num, sizeof(struct reg *)); if (!xtensa->contiguous_regs_list) { - LOG_TARGET_ERROR(target, "ERROR: Out of memory"); + LOG_TARGET_ERROR(target, "Out of memory"); goto fail; } for (unsigned int i = 0; i < xtensa->total_regs_num; i++) { From 69736131754c1b742beb8bf3058b72ae4ffc3d32 Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Wed, 23 Oct 2024 15:02:10 +0200 Subject: [PATCH 23/26] rtos: Remove 'ERROR: ' prefix in error log Remove the prefix since it is redundant. While at it, also get rid of the useless exclamation mark. Change-Id: I8707342c602cea735c5a423b37ebe40a3aafb137 Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8578 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/rtos/rtos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c index cc0fecebe..0dd538e8f 100644 --- a/src/rtos/rtos.c +++ b/src/rtos/rtos.c @@ -309,7 +309,7 @@ int rtos_qsymbol(struct connection *connection, char const *packet, int packet_s reply_len += 2 * strlen(next_suffix); /* hexify(..., next_suffix, ...) */ reply_len += 1; /* Terminating NUL */ if (reply_len > sizeof(reply)) { - LOG_ERROR("ERROR: RTOS symbol '%s%s' name is too long for GDB!", next_sym->symbol_name, next_suffix); + LOG_ERROR("RTOS symbol '%s%s' name is too long for GDB", next_sym->symbol_name, next_suffix); goto done; } From c837beaf5d70685a4c3236b1680897985adafe5f Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Tue, 22 Oct 2024 17:31:20 +0200 Subject: [PATCH 24/26] target/breakpoints: Use LOG_TARGET_ERROR() Use LOG_TARGET_xxx() for the remaining log messages. Change-Id: I4b86b206d17dead0662388e827204b40a7d29edd Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8579 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/target/breakpoints.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c index 9378abcb1..a08041629 100644 --- a/src/target/breakpoints.c +++ b/src/target/breakpoints.c @@ -114,7 +114,7 @@ static int context_breakpoint_add_internal(struct target *target, * breakpoint" ... check all the parameters before * succeeding. */ - LOG_ERROR("Duplicate Breakpoint asid: 0x%08" PRIx32 " (BP %" PRIu32 ")", + LOG_TARGET_ERROR(target, "Duplicate Breakpoint asid: 0x%08" PRIx32 " (BP %" PRIu32 ")", asid, breakpoint->unique_id); return ERROR_TARGET_DUPLICATE_BREAKPOINT; } @@ -643,8 +643,7 @@ int watchpoint_remove(struct target *target, target_addr_t address) int watchpoint_clear_target(struct target *target) { - LOG_DEBUG("Delete all watchpoints for target: %s", - target_name(target)); + LOG_TARGET_DEBUG(target, "Delete all watchpoints"); struct watchpoint *watchpoint = target->watchpoints; int retval = ERROR_OK; From 76e228f733af8f685cfd96a25c92f17256562d1c Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Tue, 22 Oct 2024 18:22:36 +0200 Subject: [PATCH 25/26] target/cortex_m: Use LOG_TARGET_xxx() Use LOG_TARGET_xxx() for the remaining log messages. Change-Id: If52e3935b57e4c39212ce6b5111ff65159de1373 Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8580 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/target/cortex_m.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/target/cortex_m.c b/src/target/cortex_m.c index e78d2e29b..fa95fcbc7 100644 --- a/src/target/cortex_m.c +++ b/src/target/cortex_m.c @@ -1469,7 +1469,7 @@ static int cortex_m_resume(struct target *target, int current, if (target->smp && !debug_execution) { retval = cortex_m_restore_smp(target, !!handle_breakpoints); if (retval != ERROR_OK) - LOG_WARNING("resume of a SMP target failed, trying to resume current one"); + LOG_TARGET_WARNING(target, "resume of a SMP target failed, trying to resume current one"); } cortex_m_restart_one(target, !!debug_execution); @@ -2553,7 +2553,7 @@ static bool cortex_m_has_tz(struct target *target) int retval = target_read_u32(target, DAUTHSTATUS, &dauthstatus); if (retval != ERROR_OK) { - LOG_WARNING("Error reading DAUTHSTATUS register"); + LOG_TARGET_WARNING(target, "Error reading DAUTHSTATUS register"); return false; } return (dauthstatus & DAUTHSTATUS_SID_MASK) != 0; @@ -2602,7 +2602,7 @@ int cortex_m_examine(struct target *target) } else { armv7m->debug_ap = dap_get_ap(swjdp, cortex_m->apsel); if (!armv7m->debug_ap) { - LOG_ERROR("Cannot get AP"); + LOG_TARGET_ERROR(target, "Cannot get AP"); return ERROR_FAIL; } } @@ -2659,9 +2659,9 @@ int cortex_m_examine(struct target *target) LOG_TARGET_WARNING(target, "Erratum 3092511: Cortex-M7 can halt in an incorrect address when breakpoint and exception occurs simultaneously"); cortex_m->incorrect_halt_erratum = true; if (armv7m->is_hla_target) - LOG_WARNING("No erratum 3092511 workaround on hla adapter"); + LOG_TARGET_WARNING(target, "No erratum 3092511 workaround on hla adapter"); else - LOG_INFO("The erratum 3092511 workaround will resume after an incorrect halt"); + LOG_TARGET_INFO(target, "The erratum 3092511 workaround will resume after an incorrect halt"); } LOG_TARGET_DEBUG(target, "cpuid: 0x%8.8" PRIx32 "", cpuid); From 133dd9d669e5b8beb7c7787b0be677621808e72d Mon Sep 17 00:00:00 2001 From: Henrik Mau Date: Wed, 13 Nov 2024 15:43:23 +0000 Subject: [PATCH 26/26] target/xtensa: add maskisr command support for NX Add maskisr command support to Xtensa NX targets allowing masking of interrupts during single stepping. Change-Id: I3835479de8015f1a2842afd1aeab24829e385031 Signed-off-by: Henrik Mau Reviewed-on: https://review.openocd.org/c/openocd/+/8575 Reviewed-by: Ian Thompson Reviewed-by: Antonio Borneo Tested-by: jenkins --- src/target/xtensa/xtensa.c | 41 +++++++++++++++++++++----------------- src/target/xtensa/xtensa.h | 1 + 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/target/xtensa/xtensa.c b/src/target/xtensa/xtensa.c index 284bfc9c6..fe1b83bc3 100644 --- a/src/target/xtensa/xtensa.c +++ b/src/target/xtensa/xtensa.c @@ -1727,7 +1727,7 @@ int xtensa_do_step(struct target *target, int current, target_addr_t address, in xtensa_reg_val_t dbreakc[XT_WATCHPOINTS_NUM_MAX]; xtensa_reg_val_t icountlvl, cause; xtensa_reg_val_t oldps, oldpc, cur_pc; - bool ps_lowered = false; + bool ps_modified = false; LOG_TARGET_DEBUG(target, "current=%d, address=" TARGET_ADDR_FMT ", handle_breakpoints=%i", current, address, handle_breakpoints); @@ -1783,14 +1783,23 @@ int xtensa_do_step(struct target *target, int current, target_addr_t address, in * RFI >= DBGLEVEL. */ if (xtensa->stepping_isr_mode == XT_STEPPING_ISR_OFF) { - if (!xtensa->core_config->high_irq.enabled) { - LOG_TARGET_WARNING( - target, - "disabling IRQs while stepping is not implemented w/o high prio IRQs option!"); - return ERROR_FAIL; + if (xtensa->core_config->core_type == XT_LX) { + if (!xtensa->core_config->high_irq.enabled) { + LOG_TARGET_WARNING(target, + "disabling IRQs while stepping is not implemented w/o high prio IRQs option!"); + return ERROR_FAIL; + } + /* Update ICOUNTLEVEL accordingly */ + icountlvl = MIN((oldps & 0xF) + 1, xtensa->core_config->debug.irq_level); + } else { + /* Xtensa NX does not have the ICOUNTLEVEL feature present in Xtensa LX + * and instead disable interrupts while stepping. This could change + * the timing of the system while under debug */ + xtensa_reg_val_t newps = oldps | XT_PS_DI_MSK; + xtensa_reg_set(target, XT_REG_IDX_PS, newps); + icountlvl = xtensa->core_config->debug.irq_level; + ps_modified = true; } - /* Update ICOUNTLEVEL accordingly */ - icountlvl = MIN((oldps & 0xF) + 1, xtensa->core_config->debug.irq_level); } else { icountlvl = xtensa->core_config->debug.irq_level; } @@ -1815,7 +1824,7 @@ int xtensa_do_step(struct target *target, int current, target_addr_t address, in xtensa_cause_clear(target); /* so we don't recurse into the same routine */ if (xtensa->core_config->core_type == XT_LX && ((oldps & 0xf) >= icountlvl)) { /* Lower interrupt level to allow stepping, but flag eps[dbglvl] to be restored */ - ps_lowered = true; + ps_modified = true; uint32_t newps = (oldps & ~0xf) | (icountlvl - 1); xtensa_reg_set(target, xtensa->eps_dbglevel_idx, newps); LOG_TARGET_DEBUG(target, @@ -1916,11 +1925,12 @@ int xtensa_do_step(struct target *target, int current, target_addr_t address, in } /* Restore int level */ - if (ps_lowered) { + if (ps_modified) { LOG_DEBUG("Restoring %s after stepping: 0x%08" PRIx32, - xtensa->core_cache->reg_list[xtensa->eps_dbglevel_idx].name, - oldps); - xtensa_reg_set(target, xtensa->eps_dbglevel_idx, oldps); + xtensa->core_cache->reg_list[(xtensa->core_config->core_type == XT_LX) ? + xtensa->eps_dbglevel_idx : XT_REG_IDX_PS].name, oldps); + xtensa_reg_set(target, (xtensa->core_config->core_type == XT_LX) ? + xtensa->eps_dbglevel_idx : XT_REG_IDX_PS, oldps); } /* write ICOUNTLEVEL back to zero */ @@ -4191,11 +4201,6 @@ COMMAND_HELPER(xtensa_cmd_mask_interrupts_do, struct xtensa *xtensa) return ERROR_OK; } - if (xtensa->core_config->core_type == XT_NX) { - command_print(CMD, "ERROR: ISR step mode only supported on Xtensa LX"); - return ERROR_FAIL; - } - /* Masking is ON -> interrupts during stepping are OFF, and vice versa */ if (!strcasecmp(CMD_ARGV[0], "off")) state = XT_STEPPING_ISR_ON; diff --git a/src/target/xtensa/xtensa.h b/src/target/xtensa/xtensa.h index 1d56f8368..419277675 100644 --- a/src/target/xtensa/xtensa.h +++ b/src/target/xtensa/xtensa.h @@ -45,6 +45,7 @@ /* PS register bits (NX) */ #define XT_PS_DIEXC_MSK BIT(2) +#define XT_PS_DI_MSK BIT(3) /* MS register bits (NX) */ #define XT_MS_DE_MSK BIT(5)