Rebase onto latest upstream master about openocd HOT 24 CLOSED

Note: i have created an rp2040-master branch, it would be great to make a new patch of the rp2040 specific changes to upstream (in a form acceptable to them)

from openocd.

These are the missing changes

* 71510a77a 5 days ago Luke Wren tcl/boards: add pico-debug.cfg
* e32e11eae 6 days ago GitHub Exit and enter XIP for erase and write (#29)
* b0f34c772 3 weeks ago Graham Sanderson Prevent Glitches on SWCLK
* 92898d055 3 weeks ago Graham Sanderson cmsis_dsp_usb: add support for SWD multi-drop
* 41f11c841 3 weeks ago Graham Sanderson rp2040: add .cfg files for single core targets

from openocd.

@kilograham, I am NOT a git expert and don't really know what you are talking about. What specifically are you asking me for?

from openocd.

Looking at the raw logs, it looks like your build is choking on use of libusb.

from openocd.

Unless I am mistaken, you took new files from this fork, and up-streamed them (editing them along the way), but didn't tell us or submit a PR to merge your changes back into here. As a result, there are now subsequent changes here some of which have merge conflicts with the code you edited, meaning we can't simply re-base on top of the latest upstream.

Since you made major edits to rp2040.c fixing these conflicts is non trivial. I am asking you to fix up these 5 changes to cleanly apply on top the modifications you made... this can be done by cherry-picking them into the rp2040-master branch which has the latest upstream master, and the other rp2040 changes up to these last 5 applied.

from openocd.

Looking at the raw logs, it looks like your build is choking on use of libusb.

I assume by "raw logs" you mean e.g. https://github.com/raspberrypi/openocd/runs/2855380845
It's only "choking" on libusb there because https://github.com/raspberrypi/openocd/blob/rp2040/.github/workflows/snapshot.yml was setting the LIBUSB1_SRC variable using the ::set-env syntax, which GitHub has now blocked. However this has already been updated in https://github.com/raspberrypi/openocd/blob/rp2040-master/.github/workflows/snapshot.yml to use syntax that GitHub does allow.

TL;DR "choking on libusb" is a red herring 😉

from openocd.

Since you made major edits to rp2040.c fixing these conflicts is non trivial

Correct me if I'm wrong, but the only commit with changes to rp2040.c is this one, which I didn't author:

e32e11e 6 days ago GitHub Exit and enter XIP for erase and write (#29)

The others that you cited have no connection to rp2040.c:

71510a7 5 days ago Luke Wren tcl/boards: add pico-debug.cfg
b0f34c7 3 weeks ago Graham Sanderson Prevent Glitches on SWCLK
92898d0 3 weeks ago Graham Sanderson cmsis_dsp_usb: add support for SWD multi-drop
41f11c8 3 weeks ago Graham Sanderson rp2040: add .cfg files for single core targets

from openocd.

yeah i don't think all 5 have issues, those are just the ones since things were last in sync.

I'm referring to this

Author: Peter Lawrence <[email protected]>
Date:   Tue Mar 2 15:21:28 2021 -0600

    tcl/board: add pico-debug support
    
    pico-debug is not a board; it is a virtual CMSIS-DAP adapter that
    runs on the same RP2040 also being debugged.  This is possible due
    to pico-debug running on the normally-dormant second Cortex-M0+
    core (Core1), providing debugging of the first core (Core0).
    As such, it could be used on a variety of RP2040-based boards.
    
    Since a flash driver is useful (if not essential), a flash driver
    is included.  This driver code originated on RPi's bespoke OpenOCD
    fork; lipstick was added to this particular pig to make it more
    presentable on OpenOCD proper.
    
    no new Clang analyzer warnings
    
    Change-Id: I31f98b5ea1664f0adfbc184b57efba963acfb958
    Signed-off-by: Peter Lawrence <[email protected]>
    Reviewed-on: http://openocd.zylin.com/6075
    Tested-by: jenkins
    Reviewed-by: Tomas Vanek <[email protected]>

from openocd.

I'm referring to this

Got it. That was a lot of work that I was forced to do on RPI's behalf to jump through all the hoops to make RPi's rp2040.c presentable to OpenOCD by following the OpenOCD coding conventions, fixing memory leaks, etc., etc.

What specifically are you looking for? I can't "fix up" changes that have nothing to do with rp2040.c

from openocd.

I'm referring to this

Got it. That was a lot of work that I was forced to do on RPI's behalf to jump through all the hoops to make RPi's rp2040.c presentable to OpenOCD by following the OpenOCD coding conventions, fixing memory leaks, etc., etc.

I feel your pain; i had done the same with the 2 PRs I wrote for SWD-multidrop & CortexM0+SMP which still haven't been upstream-ed (which didn't have any memory leaks etc). I didn't have anything to do with rp2040.c

What specifically are you looking for? I can't "fix up" changes that have nothing to do with rp2040.c

Ok i guess there were some other conflicting changes other than yours (outside of the SWD/SMP stuff I was focused on) then since 0.11.0 (which was fine for rebasing).

I will fix up these other ones, then leave you anything that touches rp2040.c

thanks.

from openocd.

OK, i have re-merged everything onto a (now force-pushed) p2040-master

This is now missing only the conflicting * e32e11eae 6 days ago GitHub Exit and enter XIP for erase and write (#29)

If you can take a look at doing that, I will continue what I was trying to do which is making progress on getting the SWD/multicore SMP stuff upstreamed. Note I have made two other branches (master+swdetc which has non RP2040 specific if you will commits, and master+rp2040 which has the rp2040 changes only, so won't actually work on its own)

from openocd.

The stated objective of e32e11e was to remedy #25. However, as it happens the upstream openocd already fixed #25 through the use of stack_grab_and_prep(), which is employed equally by both rp2040_flash_write() and rp2040_flash_erase(), and said function performs a superset of e32e11e’s rp2040_flash_exit_xip().

The remaining addition in e32e11e was to enable XIP mode after an erase, consistent with the legacy code on a write.

I’m in the dark as to what the right git-ness is needed to adapt another user’s commit, but here is a suggested diff to achieve e32e11e:

diff --git a/src/flash/nor/rp2040.c b/src/flash/nor/rp2040.c
index 5b4c16bb9..aec3f806e 100644
--- a/src/flash/nor/rp2040.c
+++ b/src/flash/nor/rp2040.c
@@ -167,6 +167,20 @@ static int stack_grab_and_prep(struct flash_bank *bank)
 	return ERROR_OK;
 }
 
+static int rp2040_flash_enter_xip(struct flash_bank *bank) 
+{
+	struct rp2040_flash_bank *priv = bank->driver_priv;
+	int err = ERROR_OK;
+
+	LOG_DEBUG("Configuring SSI for execute-in-place");
+	err = rp2040_call_rom_func(bank->target, priv, priv->jump_enter_cmd_xip, NULL, 0);
+	if (err != ERROR_OK)
+	{
+		LOG_ERROR("RP2040 enter xip: failed to enter flash XIP mode");
+	}
+	return err;
+}
+
 static int rp2040_flash_write(struct flash_bank *bank, const uint8_t *buffer, uint32_t offset, uint32_t count)
 {
 	LOG_DEBUG("Writing %d bytes starting at 0x%" PRIx32, count, offset);
@@ -223,10 +237,9 @@ static int rp2040_flash_write(struct flash_bank *bank, const uint8_t *buffer, ui
 		LOG_ERROR("RP2040 write: failed to flush flash cache");
 		return err;
 	}
-	LOG_DEBUG("Configuring SSI for execute-in-place");
-	err = rp2040_call_rom_func(bank->target, priv, priv->jump_enter_cmd_xip, NULL, 0);
-	if (err != ERROR_OK)
-		LOG_ERROR("RP2040 write: failed to flush flash cache");
+
+	err = rp2040_flash_enter_xip(bank);
+
 	return err;
 }
 
@@ -262,6 +275,8 @@ static int rp2040_flash_erase(struct flash_bank *bank, unsigned int first, unsig
 
 	err = rp2040_call_rom_func(bank->target, priv, priv->jump_flash_range_erase, args, ARRAY_SIZE(args));
 
+	err = rp2040_flash_enter_xip(bank);
+
 	return err;
 }
 

from openocd.

Cool thanks, much appreciated, will take a look

from openocd.

This gets me curious: was there a reason to keep this outside of the main OpenOCD repository?
Mind if I propose the picoprobe support upstream?

from openocd.

This gets me curious: was there a reason to keep this outside of the main OpenOCD repository? Mind if I propose the picoprobe support upstream?

@josuah, you might as well ask why picoprobe even exists in the first place. Raspberry Pi chose to invent a proprietary protocol (Picoprobe) that only works with their openocd fork. They could have saved time and money by adopting an existing driver-less protocol (i.e. CMSIS-DAP) that already has existing open source implementations, and then RP2040 would have worked out of the box with the cornucopia of free and commercial ARM IDEs.

from openocd.

@majbthrd yes their choice is confusing.

pico-debug uses the CMSIS-DAP standard

I understand this project better now.

from openocd.

@majbthrd I'm slowly and finally starting to get a hang of what is going on here, and maybe you already understood it well since the beginning, but here I am:

RaspberryPi chose the CMSIS-DAPv2 protocol for debugging

https://developer.arm.com/architectures/system-architectures/system-components/coresight/serial-wire-debug

Single Debug Port for Multi-Chip Products

Arm Multi-drop SWD technology brings the benefits of SWD to complex, multi-processor-based SoCs by enabling simultaneous access to an unlimited number of devices through a single connection, providing complex-device developers with a low power 2-pin debug and trace solution. This is particularly important for connectivity-constrained products, such as mobile phones, where multi-die and multiple chips are common.

The Multi-drop SWD is fully backwards compatible, retaining existing single point-to-point host equipment connections, and permits a device to power down completely while that device is not selected, reducing power consumption. Serial Wire debug with multi drop support is available to all CoreSight licensees under their maintenance program.

As indicated in the RP2040 datasheet :

Debug access is via independent DAPs (one per core) attached to a shared multidrop SWD bus (SWD v2). Each DAP will only respond to debug commands if correctly addressed by a SWD TARGETSEL command; all others tristate their outputs. Additionally, a Rescue DP (see Section 2.3.4.2) is available which is connected to system control features. Default addresses of each debug port are given below:

That is as much a standard as the one used by well-known ST-Link and J-Link, but is also much less frequently implemented by these. That is why they offered to use One RP2040 to debug another.

from openocd.

RaspberryPi chose the CMSIS-DAPv2 protocol for debugging

NO. This is not the case. CMSIS-DAP is a USB protocol to connect PC software running ARM debug software to an ARM debug unit on an ARM MCU. You are mistakenly interpreting descriptions about SWD as somehow being a description of CMSIS-DAP.

RaspberryPi chose to create their own proprietary protocol that is layered on top a USB virtual serial UART port and uses that UART protocol to conduct SWD transfers. They did this instead of adopting an open standard like CMSIS-DAPv2.

from openocd.

Thank you for taking the time to correct me.

Much of the confusion came between "DAP" (used often through the RP2040 datasheet) vs "CMSIS-DAP" (standard for exposing the DAP over USB) 1. Apologies for the distraction.

There start to be more clashing changes from upstream:

#define NTAP_OPT_IRLEN     0
#define NTAP_OPT_IRMASK    1
#define NTAP_OPT_IRCAPTURE 2
#define NTAP_OPT_ENABLED   3
#define NTAP_OPT_DISABLED  4
#define NTAP_OPT_EXPECTED_ID 5
#define NTAP_OPT_VERSION   6
<<<<<<< HEAD
#define NTAP_OPT_BYPASS    7
=======
#define NTAP_OPT_DP_ID 7
#define NTAP_OPT_INSTANCE_ID 8
>>>>>>> 25ce9cea7 (swd: Add support for SWD multi-drop)

These seems like cosmetic changes and could be ignored.

I tried rebasing raspberrypi/openocd/rp2040-master (branch with the ) over master, and compiling stops there:

libtool: compile:  cc -DHAVE_CONFIG_H -I. -I./src -I./src -DPKGDATADIR=\"/usr/local/share/openocd\" -DBINDIR=\"/usr/local/bin\" -I./jimtcl -I./jimtcl -isystem /usr/local/include -I/usr/local/include/libusb-1.0 -I/usr/local/include/libftdi1 -I/usr/local/include/libusb-1.0 -I/usr/local/include/hidapi -I./src/jtag/drivers/libjaylink/libjaylink -I./src/jtag/drivers/libjaylink -Wall -Wstrict-prototypes -Wformat-security -Wshadow -Wextra -Wno-unused-parameter -Wbad-function-cast
-Wcast-align -Wredundant-decls -Wpointer-arith -Wundef -Wno-error=deprecated-declarations -g -O2 -MT src/jtag/drivers/libocdjtagdrivers_la-picoprobe.lo -MD -MP -MF src/jtag/drivers/.deps/libocdjtagdrivers_la-picoprobe.Tpo -c src/jtag/drivers/picoprobe.c -o src/jtag/drivers/libocdjtagdrivers_la-picoprobe.o
src/jtag/drivers/picoprobe.c:501:35: error: too many arguments to function call, expected 4, have 5
                        &picoprobe_handle->usb_handle, NULL) != ERROR_OK) {
                                                       ^~~~
/usr/include/sys/_null.h:10:14: note: expanded from macro 'NULL'
#define NULL    ((void *)0)
                ^~~~~~~~~~~
src/jtag/drivers/libusb_helper.h:30:5: note: 'jtag_libusb_open' declared here
int jtag_libusb_open(const uint16_t vids[], const uint16_t pids[],
    ^
1 error generated.
gmake[2]: *** [Makefile:3807: src/jtag/drivers/libocdjtagdrivers_la-picoprobe.lo] Error 1

The code for the picoprobe seems made for an older OpenOCD that what it is today.

What else than picoprobe was needed to upstream to OpenOCD? Is the work on multidrop an adaptation of it for the RP2040 or a project to bring Multidrop support to OpenOCD?

from openocd.

The code for the picoprobe seems made for an older OpenOCD that what it is today.

@josuah, introducing patches to the authentic OpenOCD requires acceptance of those changes from that project's maintainers. Those maintainers have a very particular code review process.

This is just my opinion, but I think RaspberryPi found it easier to control their own fate by going their own way. Since a walled garden suits Raspberry Pi products just fine, there was no incentive to interoperate and the code diverged.

Furthermore, to given credit where credit is due, @kilograham proposed multi-drop SWD changes to the authentic OpenOCD almost two years before the RP2040 was announced, but his particular patch was ultimately never accepted.

from openocd.

@majbthrd
SWD multi-drop has been added to openocd upstream.
Ref: https://review.openocd.org/c/openocd/+/6141/
And the merged code is based on @kilograham 's code (but the merged code does not have all the features)
http://review.openocd.org/4935

But I think the reviews are still open -- not so sure any further works will be done to get them merged.
https://review.openocd.org/c/openocd/+/4935/
https://review.openocd.org/c/openocd/+/4936/

from openocd.

@josuah, you might as well ask why picoprobe even exists in the first place. Raspberry Pi chose to invent a proprietary protocol (Picoprobe) that only works with their openocd fork. They could have saved time and money by adopting an existing driver-less protocol (i.e. CMSIS-DAP) that already has existing open source implementations, and then RP2040 would have worked out of the box with the cornucopia of free and commercial ARM IDEs.

FYI, picoprobe has been updated recently to include a CMSIS compatible implementation: raspberrypi/debugprobe#31

I've tested it recently and it works fine with the upstream openocd v0.11.0, albeit the target I tested was not RP2040.

from openocd.

@vulpes2 this suggests this issue can be closed soon?

In #56, @P33M closed with the note that the upstream OpenOCD would work with the picoprobe.

This means the work would continue on OpenOCD rather than here? Is this fork meant to be closed?

from openocd.

I've now switched the default branch to upstream 0.12 plus extra upstream development branch commits. This fork will eventually be closed, yes, but for now there are some long-tail bits and pieces.

from openocd.