Compile with Latest ARM GNU and Pico SDK

[tornupnegatives]

Fixes #60. Yapicoprobe now compiles and OpenOCD detects it, but it fails to initialize the DAP

[rgrr]

do I understand correctly, that the major problem is, that one "net_device.h" declares "tud_network_mac_address" as "const" while the other does not? This would mean that we are using different TinyUSB versions. Could you please update the TinyUSB submodule to the current main?

Is it really necessary to change tu_static -> static?

[tornupnegatives]

Could you please update the TinyUSB submodule to the current main?

The TinyUSB headers you have in src/net/tinyusb are drastically different from the current TinyUSB main, so I'm not sure we want to do that...

Hmmm, drastically is a strong word... only change according to my view is, that I have moved some NCM constants out of the way.

But to be honest I took not into account, that net_device.h is included from somewhere else in TinyUSB (like tusb.h :-/)

Sorry... sorry... did not want to change our comment. Wanted to reply.

[tornupnegatives]

I will say, although these changes do allow me to compile yapicoprobe, it does not actually work. Of course, I cannot compile the master branch as a ground reference...

openocd ➤ sudo src/openocd -f interface/cmsis-dap.cfg -c "adapter speed 5000" -f target/stm32h7x.cfg -s tcl
Open On-Chip Debugger 0.11.0-g8e3c38f (2023-07-19-09:31)
Licensed under GNU GPL v2
For bug reports, read
        http://openocd.org/doc/doxygen/bugs.html
adapter speed: 5000 kHz

Info : auto-selecting first available session transport "swd". To override use 'transport select <transport>'.
Info : Listening on port 6666 for tcl connections
Info : Listening on port 4444 for telnet connections
Info : Using CMSIS-DAPv2 interface with VID:PID=0x2e8a:0x000c, serial=E6616407E34A622D
Info : CMSIS-DAP: SWD  Supported
Info : CMSIS-DAP: FW Version = 2.0.0
Info : CMSIS-DAP: Serial# = E6616407E34A622D
Info : CMSIS-DAP: Interface Initialised (SWD)
Info : SWCLK/TCK = 0 SWDIO/TMS = 0 TDI = 0 TDO = 0 nTRST = 0 nRESET = 0
Info : CMSIS-DAP: Interface ready
Info : clock speed 1800 kHz
Error: Error connecting DP: cannot read IDR
Info : DAP init failed

[rgrr]

I will say, although these changes do allow me to compile yapicoprobe, it does not actually work. Of course, I cannot compile the master branch as a ground reference...

Are the provided images working?

[tornupnegatives]

Are the provided images working?

Using the latest OpenOCD provided by Raspberry Pi and the build image yapicoprobe-0115-pico-defa657.uf2, no...

I have my probe connected with GPIO2 connected to SWCLK and GPIO3 connected to SWDIO, per includes/boards/pico.h

The board registers as a USB device, but it lacks an ID

$ lsusb
...       
Bus 001 Device 046: ID 2e8a:0003
... 

[rgrr]

:

$ lsusb ...
Bus 001 Device 046: ID 2e8a:0003 ...

Oops, mine says

:
Bus 001 Device 003: ID 2e8a:000c RaspberryPi YAPicoprobe CMSIS-DAP
:

[rgrr]

Are the provided images working?

Using the latest OpenOCD provided by Raspberry Pi and the build image yapicoprobe-0115-pico-defa657.uf2, no...

Do you have an RP2040 target at hand?

[tornupnegatives]

Do you have an RP2040 target at hand?

I'm currently targeting an STM327Hxx MCU, but I do have some spare Picos. I just don't have the custom 3-pin SWD cable...

[rgrr]

Do you have an RP2040 target at hand?

I'm currently targeting an STM327Hxx MCU, but I do have some spare Picos. I just don't have the custom 3-pin SWD cable...

I know about that problem. That's why I prefer the headerless versions and one reason why I bought a picoprobe.

Concerning the STM32: you are sure that voltage levels are correct?

[tornupnegatives]

Concerning the STM32: you are sure that voltage levels are correct?

Yes, I am sure. I am powering the Pico and the STM32 from the same USB bus. The official picoprobe fork has no issues connecting to this device, either.

[rgrr]

Concerning the STM32: you are sure that voltage levels are correct?

Yes, I am sure. I am powering the Pico and the STM32 from the same USB bus. The official picoprobe fork has no issues connecting to this device, either.

These are bad news. So the problem is related to yapicoprobe :-/

Have you tried to reduce clock frequency?

[tornupnegatives]

Have you tried to reduce clock frequency?

Yes, I have lowered PROBE_CPU_CLOCK_KHZ to no avail. I peeked at the SWD pins using a logic analyzer, and it is talking to the target. It just isn't having the right conversation, apparently...

These are bad news. So the problem is related to yapicoprobe :-/

Please let me know how I can help!

[rgrr]

:

Please let me know how I can help!

Thanks for the offer. But to be honest I'm a little bit clueless at the moment how to check this.

I think I have to buy an STM32 board. But in the meantime you could perhaps check if there is a yapicoprobe version which can handle the STM correctly.

[tornupnegatives]

Version rg-1.11 does compile and successfully connect to the STM32!

[rgrr]

Version rg-1.11 does compile and successfully connect to the STM32!

What about 1.12?

[tornupnegatives]

Version rg-1.11 does compile and successfully connect to the STM32!

What about 1.12?

Unfortunately not. See #62

[tornupnegatives]

That being said, I do think that this PR solves the original issue, which was mismatches between const qualifiers preventing compilation

[rgrr]

yes, I'm really not very happy with this PR. The current main branch of TinyUSB does not have the "const" qualifier so your patch will be broken again.

In the meantime until the Pico SDK is lifted to a newer version of TinyUSB the problem could be solved via documentation. What do you think?

[tornupnegatives]

I definitely see your point. At present, the Pico SDK does use the latest release of TinyUSB (https://github.com/hathach/tinyusb/releases/tag/0.15.0). As far as declaring tud_network_mac_address as const, I do not think this would cause a problem if it became non-const in TinyUSB. Going from non-const to const is totally legal-just not the other way around.

My preference is that the project builds. Having to clone the repo and then manually make a change in order to build is frustrating.

[rgrr]

And I understand your point, that you want a repo which has to work.

Nevertheless if you have a look at your code: actually it is a cruel hack (don't take it personal) which circumvents a wrong definition within TinyUSB.

If const uint8_t tud_network_mac_address[6]; is put into flash which should be legal to my understanding, the cast and the subsequent write would have no effect.

I think the TinyUSB guys understood, that the tud_network_mac_address[] can't be const and thus changed it.

My idea of correcting this to document it and advise how to update TinyUSB within the SDK

[tornupnegatives]

It does seem that the TinyUSB team has fixed this issue by declaring tud_network_mac_address as non-const (https://github.com/hathach/tinyusb/pull/1971).

[rgrr]

Still not convinced ;-)

Tough luck somehow, that it has been decided to use TinyUSB 0.15.0 as of 10.2.2023 and the fix arrived on 24.3.2023.

tud_network_mac_address must be placed in RAM, so it cannot be const. So the actual definition in get_config.c must use __attribute__ ((section ...)) to put it there. Then one could proceed as you suggested. But I'm not happy with that, because as said before, an update of TinyUSB would break that again.

I've tried now several other ways, but all ended up in either "no success" or in bad hacks nobody will understand in 2 days again.

So my suggestion is still to document this issue and say that TinyUSB has to be upgraded accordingly in the SDK.

[tornupnegatives]

And how is it that you are using a different version of TinyUSB than what is packaged with the Pico SDK?

[rgrr]

And how is it that you are using a different version of TinyUSB than what is packaged with the Pico SDK?

I really fought with ECM/RNDIS/NCM as you can see in the commit log. One of my tries was updating TinyUSB.

Besides: the MAC const topic is a real bug and I think even this one would made me update. No negative consequences are known to me, except one has to do some manual work.

[tornupnegatives]

My opinion is that the user should not have to modify the source in order to get it to compile, beyond a Make/CMake argument or environment variable. Yes, there is value in waiting on the distributors of TinyUSB/Pico SDK to correct their "faulty" implementations, but there is no saying when or if that will happen. In the meantime, users of your code should not be left with a broken build environment.

If you are still vehemently against it, would a CMake variable to enable the change be acceptable?

[rgrr]

I'm almost sure, that it can be determined by cmake, how tud_network_mac_address[] is defined. So no switch should be required at all. I will check that later, perhaps today.

And BTW: the patch can be cleaned up a little bit:

• the Eclipse stuff can be reverted
• tusb_config.h does not need it (my bad)
• get_config.h and net_device.h neither

at least it compiles without those declarations.

[rgrr]

could you please check the master branch if your issue has been fixed?

Solution was much simpler than thought: definition might differ from declaration, the linker actually does not check.

[tornupnegatives]

When building against master, it appears the issue related to the tu_static definition is still present.

error: expected ';' before 'const'
   84 | CFG_TUSB_MEM_SECTION CFG_TUSB_MEM_ALIGN tu_static const ntb_parameters_t ntb_parameters = {

error: 'ncm_interface' undeclared (first use in this function); did you mean 'ncm_interface_t'?
  153 |     if (usbd_edpt_busy(0, ncm_interface.ep_out)) {

I would recommend creating a GitHub action to build the project when you commit to master, as a sort of regression. I'd be happy to help you set that up.

[rgrr]

When building against master, it appears the issue related to the tu_static definition is still present.

Häh!? I thought we were struggling with the several declarations of tud_network_mac_address[]... :-/

:

I would recommend creating a GitHub action to build the project when you commit to master, as a sort of regression. I'd be happy to help you set that up.

I thought about that already as well. But not sure if this will really work, because the target is bare metal Arm. Any help is welcome of course.

I will try to reproduce this issue now locally on my machine.

[rgrr]

Ahhh, now I see it. This is fixed here?:

#ifndef ECLIPSE_GUI
    #define tu_static static
#endif

You don't have to answer, because "yes" is the answer

[rgrr]

could you please post (again) the output of your cmake run?

You don't have to: I have gone back to TinyUSB 0.15.0 and see the exact same. Wondering what they have done to tu_static!?

[rgrr]

could you please try: https://github.com/rgrr/yapicoprobe/tree/feature/60-build-failure

[rgrr]

Aaaarggghhhhhh... tu_static appeared on 22.2.2023

[tornupnegatives]

could you please try: https://github.com/rgrr/yapicoprobe/tree/feature/60-build-failure

It compiles!

[rgrr]

Yippieeeee. Thanks for your support and stubbornness. I close it!