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 4b1ea8511a7da9d7201df40302e3341c6e97ffdd from upstream #967

Merged
merged 53 commits into from
Nov 29, 2023

Conversation

timsifive
Copy link
Collaborator

@timsifive timsifive commented Nov 21, 2023

There was a messy conflict in breakpoints.c. @kr-sc, can you take a look? It's a combination of your changes upstream and my changes here that caused the conflict.

As usual, upstream changes have checkpatch issues which we'll ignore.

karlp and others added 30 commits August 26, 2023 11:39
These cores are advertised as M23 and M33 compatible, but are identified
by the Realtek implementor id.  These cores are found on the RTL872xD
family, at least.

Raw CPUIDs:
Real-M200 (KM0): 721cd200
Real-M300 (KM4): 721fd220

Change-Id: I4106ccb7e8c562f98072a71e9e818f57999d664e
Signed-off-by: Karl Palsson <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7846
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Sufficient to probe both cores via multiple APs.
No support listed for jtag in the datasheet or usermanual.
Tested against a BW-16 board:
  https://www.amebaiot.com/en/amebad/#partner_bw16

Change-Id: Idf82085e7b7327fdf3d6d668e6fb59eff6e0431b
Signed-off-by: Karl Palsson <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7847
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
Since zephyrproject-rtos/zephyr@c3eeae8,
Zephyr OS exposes offset of mode_exc_return in the arch struct for ARM.

Accounting for this allows for consistency and enables
logic with further offsets that may be added after this.

Signed-off-by: Bruno Mendes <[email protected]>
Change-Id: Id53ebd80c5d98a7d94eb6b00ad638ce51e719822
Reviewed-on: https://review.openocd.org/c/openocd/+/7851
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
Add links for the SoCs are supported by the conf file for future
reference.

Change-Id: Ic5b7786ef3ac31414fe2ce56c1237a18ce99aaa1
Signed-off-by: Jason Kacines <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7853
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Reviewed-by: Tomas Vanek <[email protected]>
Add support for the TI K3 family AM62A7 SoC.

For further details, see https://www.ti.com/lit/pdf/spruj16a

Change-Id: Ie69bde4895f34b04f9967f63d1ca9c8149c50b8a
Signed-off-by: Jason Kacines <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7854
Reviewed-by: Antonio Borneo <[email protected]>
Reviewed-by: Tomas Vanek <[email protected]>
Tested-by: jenkins
Add basic connection details with am625 SK/EVM

For further details, see https://www.ti.com/tool/SK-AM62A-LP

Change-Id: I0b6b4004f3a04be7a90207e44c588a4f68aff47a
Signed-off-by: Jason Kacines <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7855
Reviewed-by: Antonio Borneo <[email protected]>
Reviewed-by: Tomas Vanek <[email protected]>
Tested-by: jenkins
Direct memory driver support for CoreSight Access Port(AP).

Even though we emulate SWD (serial wire debug), we aren't actually
using swd. Instead, we are using a direct memory access to get to the
register set. This is similar in approach to other fast access native
drivers such as am335xgpio drivers.

Example operation on Texas Instrument's AM62x K3 SoC:

+-----------+
|  OpenOCD  |   SoC mem map
|    on     |--------------+
| Cortex-A53|              |
+-----------+              |
                           |
+-----------+        +-----v-----+
|Cortex-M4F |<───────|           |
+-----------+        |           |
                     |  DebugSS  |
+-----------+        |           |
|Cortex-M4F |<───────|           |
+-----------+        +-----------+

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Jason Peck <[email protected]>
Change-Id: I8470cb15348863dd844b2c0e3f63a9063cb032c6
Reviewed-on: https://review.openocd.org/c/openocd/+/7088
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
This emulation mode supports software translation of an AP request
into an address mapped transaction that does not rely on physical AP
hardware. This is necessary in some hardware such as K3 SoCs since the
hardware architecture anticipates a potential race condition between
AP doing direct memory access generating transactions back to system
bus and firewalls that data path out.

This emulation mode allows direct memory driver to emulate CoreSight
Access Port (AP) and reuse the SoC configuration meant for JTAG
debuggers.

Since the address ranges are flat in nature, the requisite memory base
and size will need to be provided a-priori to the driver for mapping.
The other design alternative would be to map requested memory map for
every register operation, but, that would defeat our intent of getting
max debug performance.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Jason Peck <[email protected]>
Change-Id: I2d3c5f7833f1973e90b4f6b247827f62fc2905d0
Reviewed-on: https://review.openocd.org/c/openocd/+/7089
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Direct memory driver swd native configuration for am625.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Jason Peck <[email protected]>
Change-Id: I6cf521fe9af0a4b8f8ab4853bc25722368b713e6
Reviewed-on: https://review.openocd.org/c/openocd/+/7091
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
Direct memory driver swd native configuration for j721E

Signed-off-by: Kaelin Laundry <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
Change-Id: I27455040f48c47271ae110afd114fce005824969
Reviewed-on: https://review.openocd.org/c/openocd/+/7259
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
stlink v2 on Nucleo-64 board (e.g. NUCLEO-L476RG) has target SWO signal
connected to STM32F103CB'S PA10, which is UART1_RX. UART1 within this
MCU in theory can be configured to 4.5 Mbps baudrate, which means this
is the upper limit supported by HW. As a confirmation BMP (Black Magic
Probe) project also states in documentation that UART1 can be used with
up to 4.5 Mbps baudrate.

Tests have shown that configuring 4.5 Mbps baudrate on stlink v2
available on NUCLEO-L476RG board results in receiving corrupted data.
Using 2.25 Mbps however allows to successfully receive all data from
SWO. This makes sense in terms of STM32F103CB capabilities, since 2.25
Mbps is the next supported baudrate due to division by 2.

Increase supported stlink v2 SWO speed from 2 to 2.25 Mbps.

Tested with NUCLEO-L476RG:

  $ stm32l4x.tpiu configure -protocol uart \
    -traceclk 80000000 -pin-freq 2250000 \
    -output /dev/stdout
  $ stm32l4x.tpiu enable

2.25 Mbps speed confirmed with logic analyzer.

Signed-off-by: Marcin Niestroj <[email protected]>
Change-Id: Icbec04585664aba8b217e8f9a75458e577f7617f
Reviewed-on: https://review.openocd.org/c/openocd/+/7848
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Introduce the ability to detect CPUs based on CP0 PRId register and
apply cpu specific quirks, which alter the default ejtag behavior.

First of those is EJTAG_QUIRK_PAD_DRET, which makes sure extra NOPs are
placed after the DRET instruction on exit from debug mode. This fixes
resume behavior on Ingenic JZ4780 SoC.

The proper detection of some (currently unsupported) CPUs becomes quite
complicated, so please consult the following Linux kernel code when
adding new CPUs:
* arch/mips/include/asm/cpu.h
* arch/mips/kernel/cpu-probe.c

Change-Id: I0f413d5096cd43ef346b02cea85024985b7face6
Signed-off-by: Artur Rojek <[email protected]>
Signed-off-by: Paul Fertser <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7859
Tested-by: jenkins
…ned value

add a line that checks the returned value of set signals function

add two VIDs of other original boards (have onboard angie architecture)
so angie driver can connect to them and change their VID after
renumeration.

Change-Id: Ide4f1f6f38168a410191bf3ff75bcd59dcf7ef50
Signed-off-by: Ahmed BOUDJELIDA <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7795
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
add new i2c bit-banging feature, we can now connect in JTAG with the SoC
target and in i2c with the main board components at the same time.

Change-Id: I8e4516fe1ad5238e0373444f1c3c9bc0814d0f52
Signed-off-by: Ahmed BOUDJELIDA <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7796
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Previously, if a pin was configured as ADAPTER_GPIO_INIT_STATE_INPUT and
its drive value was ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL, initialize_gpio
would configure the pin as an output.

The set_gpio_value function is optimized to not set the direction for
pins configured as ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL as it only needs to
be set once. When initialize_gpio performs this setup, it checked only
that the drive value was ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL to set the
output direction but did not exclude input pins which have already had
their direction set.

Now, input pins are ignored when initialize_gpio checks for
ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL to set the mode to output.

Change-Id: I4fc7a8132a6b00c7f213ec9fd05c7bbb37ee5f20
Fixes: 0dd969d ("drivers/bcm2835gpio: Migrate to adapter gpio commands")
Signed-off-by: Brandon Pupp <[email protected]>
[vfazio: update commit message]
Signed-off-by: Vincent Fazio <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7862
Tested-by: jenkins
Reviewed-by: Tomas Vanek <[email protected]>
There was a typo in the register numbering.

Signed-off-by: Robert Kovacsics <[email protected]>
Change-Id: Ie5d306725962c42f1bce976b80968145e6d0a177
Reviewed-on: https://review.openocd.org/c/openocd/+/7860
Tested-by: jenkins
Reviewed-by: Oleksij Rempel <[email protected]>
Now internal watch/breakpoint will not be removed in case
of error during removing triggers from hardware.

Also change signature of some functions (for deletion
bp/wp) to print message in case of some error.

Change-Id: I71cd1f556a33975005d0ee372fc384fddfddc3bf
Signed-off-by: Kirill Radkin <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7738
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
This patch unifies the lines printed by the "bp" command
so that different types of breakpoints are printed in
the same format.

Change-Id: Ic1335eda1c58072a334aed9cf0011431c8ec86a4
Signed-off-by: Marek Vrbka <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7861
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Enumerator's case were not specified in the coding style.

Add enumerators together with macros for upper-case preference.
While there, add the word CamelCase beside the less common, but
already used, MixedCaps. This could help linking this chapter to
the output of checkpatch script.

Change-Id: I6d4af06cc6f4bc46f525e99e9a74ecc167606c49
Signed-off-by: Antonio Borneo <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7875
Reviewed-by: Tomas Vanek <[email protected]>
Tested-by: jenkins
Added NPCX flash driver to support the Nuvoton NPCX4/K3 series
microcontrollers. Add config file for these series.

Change-Id: I0b6e128fa51146b561f422e23a98260594b1f138
Signed-off-by: Luca Hung <[email protected]>
Signed-off-by: Mulin CHao <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7794
Tested-by: jenkins
Reviewed-by: Tomas Vanek <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
Previously, if the image file was less than 9 bytes long,
it was assumed to be an error when it could be a binary
image file. This patch makes OpenOCD detect these cases
as binary files.

Change-Id: I5b4dad2b547786246887812ac75907378fe58671
Signed-off-by: Marek Vrbka <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7880
Tested-by: jenkins
Reviewed-by: Tomas Vanek <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
Previously, if a pin was configured as ADAPTER_GPIO_INIT_STATE_INPUT and
its drive value was ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL, initialize_gpio
would configure the pin as an output.

The set_gpio_value function is optimized to not set the direction for
pins configured as ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL as it only needs to
be set once. When initialize_gpio performs this setup, it checked only
that the drive value was ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL to set the
output direction but did not exclude input pins which have already had
their direction set.

Now, input pins are ignored when initialize_gpio checks for
ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL to set the mode to output.

Fixes: ace0282 ("drivers/am335xgpio: Migrate to adapter gpio commands")
Change-Id: I9ea502c400ea4ffae37080b9cee891ca9176a47d
Signed-off-by: Vincent Fazio <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7877
Reviewed-by: Tomas Vanek <[email protected]>
Tested-by: jenkins
Reviewed-by: Steve Marple <[email protected]>
According to gdb documentation, `g` and `p` packets can report a
register being unavailable by a string of 'x' instead of register's
value.

Change-Id: I8ef279f1357c2e612f5d3290eb0022c1b47d9fa7
Signed-off-by: Evgeniy Naydanov <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7876
Reviewed-by: Marek Vrbka <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
Reviewed-by: Anatoly Parshintsev <[email protected]>
Tested-by: jenkins
Add SWD_EN and SWDIO_OE config for Digilent HS2

Change-Id: I3f7479bbe2e518ad6f84bf9eb729b54fee4a0f9b
Signed-off-by: Joshua Nekl <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7863
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Change-Id: I75d6aa0292bf7ff4ebee8752a5e7a3516500cd04
Signed-off-by: Joshua Nekl <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7881
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
For ARMv8, add AArch64 mdd and mwd support. AArch32 not supported.

Change-Id: I25490471e16943e5a67d7649595d77643aa9a095
Signed-off-by: Daniel Goehring <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7192
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
Recent commit 62f76b2 ("flash/nor: add support for Nuvoton
NPCX4/K3 series flash") introduces a memory leak for a missing
free() on early return for an error.

Add the free() on the return path on error.

Change-Id: Ica8568a986802e23df2ab7bed4e8cc4bbb6305a5
Signed-off-by: Antonio Borneo <[email protected]>
Fixes: 62f76b2 ("flash/nor: add support for Nuvoton NPCX4/K3 series flash")
Reviewed-on: https://review.openocd.org/c/openocd/+/7894
Tested-by: jenkins
Reviewed-by: Tomas Vanek <[email protected]>
According to Arm®v8-M Architecture Reference Manual:
section D1.2.59 DWT_DEVARCH, DWT Device Architecture Register,
the field REVISION bits [19:16] defines two DWT architectures revision

Signed-off-by: Fedi Bouzazi <[email protected]>
Change-Id: I948dae0710ac921a7f0fbcef3ccacdae99184fe4
Reviewed-on: https://review.openocd.org/c/openocd/+/7800
Tested-by: jenkins
Reviewed-by: Tarek BOCHKATI <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
STM32WBA5x have a single bank flash up to 1MB

Change-Id: I3d720e202f0fdd89ecd8aa7224653ca5a7ae187b
Signed-off-by: Tarek BOCHKATI <[email protected]>
Signed-off-by: Erwan Gouriou <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7694
Tested-by: jenkins
Reviewed-by: Tomas Vanek <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
at91sam7 flash driver allocates a flash bank based on detected flash
structure.
Use calloc() instead of malloc() - struct flash_bank has to be zeroed.

While on this:
Return error in case of struct flash_bank or driver_priv allocation fail.
Set default_padded_value and erased_value.
Use strdup() on bank->name, pointer is freed in flash_free_all_banks()

Signed-off-by: Tomas Vanek <[email protected]>
Change-Id: Id890496bfbadb7970ef583256aa4f30a7bff832f
Reviewed-on: https://review.openocd.org/c/openocd/+/7884
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
danselmi and others added 14 commits September 23, 2023 14:33
Jtagspi is using a proxy bitstream to "connect" JTAG to the
SPI pins. This is not possible with all FPGA vendors/families.
In this cases a dedicated procedure is needed to establish such
a connection.

This patch adds a jtagspi-mode for these cases. It also adds the
needed interfaces to jtagspi and the pld-driver so the driver
can select the mode and provide the necessary procedures.

For the cases where a proxy bitstream is needed, the pld driver
will select the mode and provide instruction code needed in this
case.

Change-Id: I9563f26739589157b39a3664a73d91152cd13f77
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7822
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Provide jtagspi with specific procedures to be able to
use jtagspi for programming spi-flash devices on lattice
ecp2 and ecp3 devices.

Change-Id: I39028aba47a74a0479be16d52d318f4bff7f2ed4
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7823
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Provide	jtagspi	with specific procedures to be able to
use jtagspi for	programming spi-flash devices on lattice
ecp5 devices.

Change-Id: I4a4a60f21d7e8685a5b8320b9c6ebdc2693bbd21
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7824
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
Provide	jtagspi	with specific procedures to be able to
use jtagspi for	programming spi-flash devices on lattice
certus and certus po devices.

Change-Id: I6a8ec16be78f86073a4ef5302f6241185b08e1c6
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7825
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Provide	jtagspi	with information to use jtagspi for
programming spi-flash devices on efinix trion and
titanium devices using a proxy bitstream.

Change-Id: I4a851fcaafe832c35bd7b825d95a3d08e4d57a7b
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7826
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
Provide jtagspi with information to use jtagspi for
programming spi-flash devices on xilinx devices
using a proxy bitstream.

Change-Id: I68000d71de25118ed8a8603e544cff1dc69bd9ba
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7836
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Provide jtagspi with information to use jtagspi for
programming spi-flash devices on intel devices using
a proxy bitstream.

Change-Id: Ib947b8c0dd61e2c6fa8beeb30074606131b1480f
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7837
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
Provide	jtagspi	with specific procedures to be able to
use jtagspi for	programming spi-flash devices on cologne
chip gatemate devices.

Change-Id: Ifa1c4ca6e215d7f49bd21620898991af213812e9
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7838
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
add refresh command for lattice devices
rename gowin reprogram to refresh
rename virtex2 program to refresh

Change-Id: I9da83a614b96da3e947ac4608b0a291b1d126914
Signed-off-by: Daniel Anselmi <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7839
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
DP_SELECT_APSEL and DP_SELECT_APBANK is no more used in ADIv6.

Change-Id: I4176574d46c6dc8eb3fe3aef6daab6e33492c050
Signed-off-by: Tomas Vanek <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7538
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
and DP_SELECT_DPBANK.
Use the defined symbols instead of magic numbers.

Change-Id: I19c86b183e57e42b96f76eed180c0492cd67bee1
Signed-off-by: Tomas Vanek <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7539
Tested-by: jenkins
Reviewed-by: Antonio Borneo <[email protected]>
The patch adds configuration files for the following XLP 300-series
processors: XLP304, XLP308, XLP316.

Change-Id: Iaf2b807abf9fc4d7b51222fd40bdb18c6aca7d9c
Signed-off-by: Aleksey Kuleshov <[email protected]>
Signed-off-by: Peter Mamonov <[email protected]>
CC: Antony Pavlov <[email protected]>
CC: Dongxue Zhang <[email protected]>
CC: Oleksij Rempel <[email protected]>
CC: Paul Fertser <[email protected]>
CC: Salvador Arroyo <[email protected]>
CC: Spencer Oliver <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/2323
Tested-by: jenkins
Reviewed-by: Oleksij Rempel <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
Change-Id: Ia14854bc64f5a31b6591be69be4edee9cd1310c3
Signed-off-by: Peter Mamonov <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/5249
Reviewed-by: Oleksij Rempel <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
Tested-by: jenkins
…tream

Change-Id: I05cd5ef9b04fa61a27321ae9b6a4fecabe3dee80
@@ -293,36 +293,49 @@ static void breakpoint_free(struct target *data_target, struct target *breakpoin
}

if (!breakpoint)
return;
return ERROR_OK;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
return ERROR_OK;
return ERROR_BREAKPOINT_NOT_FOUND;

Copy link
Collaborator

Choose a reason for hiding this comment

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

This if block is dead code; discussed here: #967 (comment)

@timsifive timsifive force-pushed the from_upstream branch 2 times, most recently from 556d7fb to 975baa3 Compare November 21, 2023 20:40
}

if (num_found_watchpoints == 0)
LOG_TARGET_ERROR(target, "no watchpoint at address " TARGET_ADDR_FMT " found", address);
Copy link
Contributor

@kr-sc kr-sc Nov 22, 2023

Choose a reason for hiding this comment

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

We should add return ERROR_WATCHPOINT_NOT_FOUND; here because if wp is missing at address we will get ERROR_OK (on smp targets). Fixed in OS here: https://review.openocd.org/c/openocd/+/7910

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Done.

@kr-sc
Copy link
Contributor

kr-sc commented Nov 22, 2023

@timsifive, also, I think that you should also add this one https://review.openocd.org/c/openocd/+/7940.
It fixes endless loop in case of clearing target from wp/bp.

@JanMatCodasip
Copy link
Collaborator

JanMatCodasip commented Nov 24, 2023

I have started checking the changes and am halfway through.

I expect to finish it early next week, and will also look at the suggestions posted so far.

…tream

There was a big conflict with this one incoming change. This version of
the code passes all the tests (which don't test for the changed behavior
at all AFAIK), and at least passes errors back everywhere.

Conflicts:
	src/target/breakpoints.c

Change-Id: I72f75a3e08deda7e624e8bb82e1a9ea07a7a9276
…tream

Change-Id: I59366e08a4ac7e443e426b5fd6727c649f1ac9d5
If we can't remove bp/wp, we will stuck in endless loop

Change-Id: I44c0a164db1d15c0a0637d33c75087a49cf5c0f4
Signed-off-by: Kirill Radkin <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/7940
Tested-by: jenkins
Reviewed-by: Anatoly P <[email protected]>
Reviewed-by: Tim Newsome <[email protected]>
Reviewed-by: Jan Matyas <[email protected]>
Reviewed-by: Antonio Borneo <[email protected]>
@timsifive
Copy link
Collaborator Author

@timsifive, also, I think that you should also add this one https://review.openocd.org/c/openocd/+/7940.

I cherry-picked that one.

Comment on lines 578 to +579
if (!watchpoint)
return;
return ERROR_WATCHPOINT_NOT_FOUND;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this if is redundant - the condition can't ever be true. That's because both the callers of watchpoint_free() already ensure that the argument is a valid watchpoint of the given target.

An assert should be used here instead.

I will submit a change to the vanilla upstream by the end of the week. No need to change anything in this merge request.

Comment on lines 295 to +296
if (!breakpoint)
return;
return ERROR_BREAKPOINT_NOT_FOUND;
Copy link
Collaborator

@JanMatCodasip JanMatCodasip Nov 28, 2023

Choose a reason for hiding this comment

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

This is the same case as in watchpoint_free(). The condition of the if can't be true, and so the if should be replaced by an assert. I will submit a short patch to the upstream shortly. No need to change anything in this merge request.

Copy link
Collaborator

@JanMatCodasip JanMatCodasip left a comment

Choose a reason for hiding this comment

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

I have finished my (visual) review and all looks well, including breakpoints.c after cherry-picking the two upstream changes suggested by @kr-sc (thanks).

The two cosmetic suggestion (removal of pointless if) block I will submit to openocd.org separately.

Thank you.

@timsifive timsifive merged commit e9484ba into riscv Nov 29, 2023
4 of 5 checks passed
@timsifive timsifive deleted the from_upstream branch November 29, 2023 17:24
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.