Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge up to a35e254c5383008cdacf7838a777f7f17af5eeb1 from upstream #1036

Merged
merged 29 commits into from
Apr 14, 2024

Conversation

en-sc
Copy link
Collaborator

@en-sc en-sc commented Mar 28, 2024

No description provided.

tom-van and others added 28 commits March 9, 2024 10:59
While on it update table comment and drop
not working URLs.

Change-Id: I9e21c72aa75a908c644460e43c148d3240c49b2d
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8102
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
Use the newer driver name 'nrf5' instead.

While on it set the unused parameters of flash bank
creation to zero.

While on it remove 2 empty comments.

Change-Id: I9cf0eadc5b696e6c8b7e6aec0ea3345967523e87
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8103
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
After 'reset run' or 'reset halt' the loaded application
is expected to manipulate RAMON register to workaround
the known silicon errata.

Moreover, writing to RAMON register from 'reset-end' event
after 'reset run' may collide with application intentions.

Use the workaround in 'reset-init' event only to ensure
correct function of target algorithms.

Change-Id: I7d2d92e6805a05a83676edb46b3163ef39b9a7e4
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8104
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
from Product Specification of nRF52805, 810, 811
820, 833 and 840.

While on it, rename the table to make sure the codes
are valid for nRF52 series only.

Change-Id: Id8f78fd214c5d345d1769378ae546a6be5a183ba
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8105
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
from nRF52 family.

Change-Id: I6d2b4586700bb4014c0b77dbf4ea26d1b5dc9715
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8106
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
nrf5_get_probed_chip_if_halted() was somewhat bizarre
combination of functions:
- test if the target is halted is appropriate for flash
  erase/write only, certainly not for getting chip info
- getting chip pointer takes place more frequently
  and using one temporary variable for dereference
  makes no harm
- probing chip is useless at all as the flash
  infrastructure always calls auto_probe() before
  entering a flash operation

Replace the function by ordinary and readable code.

Change-Id: Ic31f4e33d8b7b36687be3f40bfd0fe913d17b75f
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8107
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
The command substantially complicates support of nRF53/91
series. It was not even properly ported to nRF52.
The informative value is disputable. Who wants to see
e.g. override trim values for radio or unique device ID?

Drop it and simplify the driver.

Change-Id: Ia7fb20ce2ebf16065705c5d18deaf934e58db426
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8108
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
for features flags.

Change-Id: I8bff0f5fac41c50180c847f36c6d2a075eca32ca
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: https://review.openocd.org/c/openocd/+/8109
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
After commit d9b2607 ("gdb_server: support sparse register
maps"), GDB crashes while requesting the value of 'cpsr' because
the fake register is tagged as not existing.

Change the logic and set all register as existing, while still
limiting the list for the initial GDB request at connect.

Change-Id: I1c4e274c06147683db2a59a8920ae5ccd863e15c
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8163
Tested-by: jenkins
Running the GDB command 'flash-erase' triggers sending the remote
GDB commands 'vFlashErase' (one per flash bank) followed by one
single 'vFlashDone', with no 'vFlashWrite' commands in between.
This causes the field 'gdb_connection->vflash_image' to be NULL
during the execution of 'vFlashDone', triggering a segmentation
fault in OpenOCD.

While parsing 'vFlashDone', check if any image to flash has been
received.

Change-Id: I443021c7a531255b60f2c44c2685e52e3c34b5c8
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8164
Tested-by: jenkins
Add access to dsp registers and a command for dsp related operations.
Checkpatch-ignore: MACRO_ARG_REUSE

Change-Id: I30aec0b9e4984896965edb1663f74216ad41101e
Signed-off-by: Walter Ji <walter.ji@oss.cipunited.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7867
Tested-by: jenkins
Reviewed-by: Oleksij Rempel <linux@rempel-privat.de>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Commit [1] added a break on error to the nrf5_erase() sector loop
and the checking of the res value became useless in the for loop condition.

Removing nrf5_get_probed_chip_if_halted() later in [2]
dropped res initialization and clang static analyser complains
  "The left operand of '==' is a garbage value"

Drop the useless test!

Fixes: [1] commit 491636c ("flash/nor/nrf5: check protection before flash erase/write on nRF51")
Fixes: [2] commit 2db325f ("flash/nor/nrf5: drop nrf5_get_probed_chip_if_halted()")
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: Ife6071c509719f8d7dc312fe9a780bdcf2575f69
Reviewed-on: https://review.openocd.org/c/openocd/+/8174
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Unify the error codes returned by adapter drivers in the case
of the received SWD ACK field differs from OK.

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: I29e478390b4b30408054a090ac6a7fac3415ae71
Reviewed-on: https://review.openocd.org/c/openocd/+/8137
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
Allow direct pointer to struct adiv5_private_config
for targets with adiv5_private_config inside of a bigger
private config container. Use it instead of the private_config
pointer toggling hack in aarch64.c

Allow optional use of -dap parameter and use it instead
of the static variable hack in xtensa_chip.c

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: I7260c79332940adfa49d57b45cae39325cdaf432
Reviewed-on: https://review.openocd.org/c/openocd/+/8138
Tested-by: jenkins
Reviewed-by: Ian Thompson <ianst@cadence.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Allow logging at a changeable level.

Add an example of usage in ftdi driver.
Log SWD commands with not OK response at debug level (3).
For commands which responded OK use debug io level (4)
not to flood the log.

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: I67a472b293f7ed9ee84cadb7c081803e9eeb1ad0
Reviewed-on: https://review.openocd.org/c/openocd/+/8151
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
Log SWD commands with not OK response but WAIT retries at debug level.
For commands responded OK and WAIT retries use debug io level
not to flood the log.

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: Idf658e82ed970061c155945df55d06908ed25e09
Reviewed-on: https://review.openocd.org/c/openocd/+/8152
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
The bitbang driver kept retrying a SWD command as long as
the debugged device had been responding by SWD WAIT.
If the DP stalled in WAIT permanently, OpenOCD hanged.

Check 0.5 sec timeout in WAIT retry loop.

While on it insert a short alive_sleep() if the command
is retried 20 or more times.

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: I744e56e21a5a2dc2c4494cc0d7bbcb4be14ddb23
Reviewed-on: https://review.openocd.org/c/openocd/+/8153
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
ADIv6 brought more complicated rules for DP reg 0 banking.
Neither the original implementation [1] nor the later
modification [2] respected that the DP reg 0 is banked
for read only, not for write. Enforcing of an useless
SELECT write before a write to ABORT register may trigger
FAULT (CTRL/STAT bits ORUNDETECT and STICKYORUN are set)
or WAIT (DP is stalled by an outstanding previous operation)
and therefore make ABORT register virtually unusable
on some adapters (bitbang, CMSIS-DAP).

There are DP ABORT specific functions swd_queue_ap_abort()
and swd_clear_sticky_errors() which worked around the problem
using the lowest level swd->write_reg(). Using a specific
write procedure for a single DP register was error prone
(there are other DP_ABORT writes using swd_queue_dp_write_inner())
and also the Tcl command 'xx.dap dpreg 0 value' suffered
from unwanted SELECT write.

Other smaller discords in DP banking probably do not
influence normal DP operation however they may complicate
debugging in corner cases.

Adhere strictly to the DP banking rules for both ADI versions.

Fixes: [1] commit 72fb886 ("adiv6: add low level swd transport")
Fixes: [2] commit ee3fb5a ("target/arm_adi_v5: fix DP SELECT logic")
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: I3328748c1c3e0661c5ecd6eb070ac519b190ace2
Reviewed-on: https://review.openocd.org/c/openocd/+/8154
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Extend the existing code to support Monitor mode in AArch32.

Change-Id: Ia43df98d1497baac48aea67b92d81344c24f0635
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8169
Tested-by: jenkins
When receiving an OpenOCD debug log to investigate about errors or
issues, the first question is often about providing the complete
command line to better understand the use context.
Plus, when OpenOCD is lunched by an IDE, its command line is kept
hidden inside the IDE, adding troubles to the user to recover it.

Add the full command line directly inside the debug log.

It could have been useful to also search and add in the log the
full path of the OpenOCD executable, but this is not an immediate
task due to portability among OS's. See, for example:
	https://stackoverflow.com/questions/933850
This part could be handled in a future change, if really needed.

Change-Id: Ia6c5b838b9b7208bf1ecac7f95b5efc319aeabf5
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8170
Tested-by: jenkins
Reviewed-by: Paul Fertser <fercerpav@gmail.com>
To simplify the ipdbg start/stop command and be able to
add additional commands in the future, we introduce the
concept of a hub which has to be created before a
ipdbg server can be started.
The hub was created on the fly in previous versions.

Change-Id: I55f317542d01a7324990b2cacd496a41fa5ff875
Signed-off-by: Daniel Anselmi <danselmi@gmx.ch>
Reviewed-on: https://review.openocd.org/c/openocd/+/7979
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
Change-Id: I7941de02a968ccab730bfebd3483b8c3b84d7e53
Signed-off-by: Daniel Anselmi <danselmi@gmx.ch>
Reviewed-on: https://review.openocd.org/c/openocd/+/7980
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
Add access to fpr and cp1 registers.
GDB can now check the FPRs with `info reg f` and change them.
Checkpatch-ignore: MACRO_ARG_REUSE

Change-Id: I63896ab6f6737054d8108db105a13a58e1446fbc
Signed-off-by: Walter Ji <walter.ji@oss.cipunited.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7866
Tested-by: jenkins
Reviewed-by: Oleksij Rempel <linux@rempel-privat.de>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
To avoid gdb to timeout, OpenOCD implements a keep-alive mechanism
that consists in sending periodically to gdb empty strings embedded
in the "O" remote reply packet.

The main purpose of "O" packets is to forward in the gdb console
the output of the remote execution; the gdb-remote puts in the "O"
packet the string that gdb will print. It's use is restricted to
few "running/execution" contexts listed in
http://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html
and this currently limits the keep-alive capabilities of OpenOCD.

Long data transfer (memory R/W) can also cause gdb to timeout if
the interface is too slow. In this case the usual keep-alive based
on "O" packet cannot be used and, if used, would trigger a protocol
error that causes the transfer to be dropped.
The slow transfer rate can be simulated by adding some delay in the
main loop of mem_ap_write() and mem_ap_read(), then using the gdb
commands "dump" and "restore".

In the wait loop during a memory R/W, gdb drops any extra character
received from the gdb-remote that is not recognized as a valid
reply to the memory command. Every dropped character re-initializes
the timeout counter and could be used as keep-alive.

From gdb 7.0 (released 2009-10-06), an asynchronous notification
can also be received from gdb-remote during a memory R/W and has
the effect to reset the timeout counter, thus can be used as
keep-alive.
The notification would be treated as "junk" extra characters by any
gdb older than 7.0, being still valid as keep-alive.
Check putpkt_binary() and getpkt_sane() in gdb commit
74531fed1f2d662debc2c209b8b3faddceb55960

Currently, only one notification packet ("Stop") is recognized by
gdb, and gdb documentation reports that notification packets that
are not recognized should be silently dropped.
Use 'set debug remote 1' in gdb to dump the received notifications
and the junk extra characters.

Add a new level in enum gdb_output_flag for using the asynchronous
notifications.
Activate this new level during memory transfers.
Send a custom "oocd_keepalive" notification packet as keep_alive.
While there, drop a useless return in the switch/case, already
managed in case of break.

After this commit, the proper calls to keep_alive() have to be
added in the loops that code the memory transfers. Of course, the
keep_alive() should be placed during the wait for JTAG flush, not
while locally queuing the JTAG elementary transfers.

Change-Id: I9ca8e78630611597d15984bd0e8634c8fc3c32b9
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8165
Tested-by: jenkins
OpenOCD can send it's log to gdb, and gdb replies with 'OK'.
Calls to LOG_XXX() are also present in the code that communicates
with gdb. This can cause infinite nested calls.

OpenOCD uses the flag 'gdb_con->busy' to protect the communication
with gdb and prevent nested calls.

There is no reason to check for 'gdb_con->busy' in the code for
keep-alive, as keep_alive() is never called in this gdb server;
the flag would eventually be set if the current keep_alive() will
send something to gdb.

Drop the flag 'gdb_con->busy' in gdb_keep_client_alive().
While there, document the use of 'gdb_con->busy'.

Change-Id: I1ea20bf96abb5d2f1fcdba1e3861df257c396bb6
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8166
Tested-by: jenkins
The file list.h was originally taken from the Linux kernel code,
thus under license GPL-2.0-only. This locks OpenOCD to follow the
same license, even if the majority of OpenOCD files are licensed
as GPL-2.0-or-later.

A similar file is also present in FreeBSD code base under the more
permissive license BSD-2-Clause.

Drop the code from Linux kernel and replace it with the code from
FreeBSD 13.3.0.
Adapt the code to OpenOCD coding style by fixing the majority of
issues identified by checkpatch.
Add the OpenOCD specific macros and comments.

Unfortunately this causes the lost of all the doxygen comments.

Checkpatch-ignore: MACRO_ARG_REUSE, MACRO_ARG_PRECEDENCE
Change-Id: I6d86752c50158f3174c4e8c4add81e9998d01e0e
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8172
Tested-by: jenkins
The file 'list.h', copied from FreeBSD, does not depend from any
OpenOCD specific include file, but only needs 'stddef.h' for the
type 'size_t'.

Let 'list.h' to include the correct header file, then fix the now
broken dependencies in the other files that were incorrectly
relying on 'list.h' to include 'helper/types.h'

Change-Id: Idd31b5bf607e226cac44ef41b2aa335ae4dbf519
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8173
Tested-by: jenkins
Move setting of do_reconnect flag from swd_run_inner()
to swd_run(). Reconnect is not used at the inner level
and the flag had to be cleared after swd_run_inner()
to prevent recursion.

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: Ib1de80bbdf10d1cbfb1dd351c6a5658e50d12af2
Reviewed-on: https://review.openocd.org/c/openocd/+/8155
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
@en-sc en-sc self-assigned this Mar 28, 2024
Checkpatch-ignore: MACRO_ARG_REUSE, MACRO_ARG_PRECEDENCE
Change-Id: Icd10f44d162054f8f32019a579ccbdda2cee7a91
@en-sc
Copy link
Collaborator Author

en-sc commented Mar 28, 2024

checkpatch fails due to:

  • MACRO_ARG_REUSE and MACRO_ARG_PRECEDENCE in src/helper/list.h (a213afa).
  • MACRO_ARG_REUSE in src/target/mips32.h (561ea48 and 01a797a)

Will adjust commit message to include Checkpatch-ignore

@en-sc
Copy link
Collaborator Author

en-sc commented Mar 28, 2024

Adding appropriate Checkpatch-ignore to the commit message did not fix the issue. Seems like checkpatch does not properly support merge commits.
Please, ignore checkpatch failure.

@en-sc en-sc marked this pull request as ready for review March 28, 2024 09:59
Copy link
Collaborator

@MarekVCodasip MarekVCodasip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Please, see my comment about the use of the BIT() macro.

Comment on lines +98 to +101
NRF5_FEATURE_SERIES_51 = BIT(0),
NRF5_FEATURE_SERIES_52 = BIT(1),
NRF5_FEATURE_BPROT = BIT(2),
NRF5_FEATURE_ACL_PROT = BIT(3),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could start using the BIT() macro in our code as well? What others think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarekVCodasip Thank you for taking the time to review this upstream sync.

As for the BIT() macro, sure, we can use it whenever suitable. Though, I think we can handle most of our bit-masking cases directly by constants from files debug_defines.h and encoding.h.

@en-sc en-sc merged commit 27fbc27 into riscv-collab:riscv Apr 14, 2024
3 of 4 checks passed
@en-sc en-sc deleted the en-sc/from_upstream branch April 14, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants