RP2040 + RP2350 HAL: support for PIO units

[electronic_eel]

Hi emulator,

I have seen that you added HAL support for the PIO units of the RP2040 and RP2350. Thank you for implementing that.

It took me some time, but I now have a PIO unit working with it. I'm eager to use your PIO HAL functions, because I had function/variable name clashes with PIO when using the pico-sdk in the past. Getting the PIO unit to run with the functions and CMSIS defines was straightforward.

But there was one issue that wasn't obvious to me from reading the datasheet or the code I previously used with the pico-sdk: When you want to output data on a pin, you have to set a PIO-specific output enable flag for the pin you want to output on. Unfortunately there is no register for this you can access from the CPU core, instead you have to execute a SET instruction with the PINDIRS destination for this. The pico-sdk does this for you as part of it's pio_sm_set_consecutive_pindirs() function.

What I now did was:

Code: Select all

    pioSmSetExecctrlX(sm_ptr, PIO_SM_EXECCTRL_WRAP(offset+freq_out_wrap_target, offset+freq_out_wrap) | PIO_SM_EXECCTRL_SIDE_EN );
    // one side-set output to pin 20, with side-set enable
    pioSmSetPinctrlX(sm_ptr, (2U << PIO_SM_PINCTRL_SIDESET_COUNT_Pos) | \
                             (20U  << PIO_SM_PINCTRL_SIDESET_BASE_Pos) | \
                             (1U  << PIO_SM_PINCTRL_SET_COUNT_Pos) | \
                             (20U  << PIO_SM_PINCTRL_SET_BASE_Pos));

#define PIO_INSTRUCTION_SET_CMD 0xe000U
#define PIO_INSTRUCTION_SET_DST_PINDIRS 0x04U
#define PIO_INSTRUCTION_SET_DST_Pos 5

    // execute set pindirs once
    // required to set the pin direction to output for sideset, this can't be controlled in another way
    // this is also why the SET_BASE + SET_COUNT must be set above
    pioSmExecX(sm_ptr, PIO_INSTRUCTION_SET_CMD | \
                       (PIO_INSTRUCTION_SET_DST_PINDIRS << PIO_INSTRUCTION_SET_DST_Pos) | \
                       1U);

Since this is something that I guess will be needed by most users of PIO, I suggest to add a function like

Code: Select all

void pioSmPindirI(const rp_pio_sm_t *smp, uint32_t gpionr, bool output);

This would save the current value of the Pinctrl register, set the SET_BASE to the gpionr, execute the SET PINDIRS instruction on the PIO state machine, and restore the Pinctrl register. It would of course also need a pioSmPindir companion function.

Do you think this is a good idea? Or do you have some other way of implementing this in mind?

[Giovanni]

Hi,

You should talk with emulator/Buzz/Ellerbach, they are working on the full RP support. Most discussion is on our discord if you want to propose changes/improvements.

Giovanni

[electronic_eel]

You should talk with emulator/Buzz/Ellerbach, they are working on the full RP support.

Yes, I wanted to talk to him. I hope he sees the message here.

Most discussion is on our discord if you want to propose changes/improvements.

I would prefer not having to use discord. The last time I tried to use it, they immediately locked my account and wanted all kinds of intrusive checks to reopen it. Also it is a closed platform that can't be indexed by search engines.

[emulator]

Yes, the PIO LLD is missing a few functions, a patch would be great. If not I will make a pass at is likely later this week.

The existing implementation was primarily motivated as a POC for the CMSIS-DAP example. But that also means most of the support is currently catered to output.

[electronic_eel]

Yes, the PIO LLD is missing a few functions, a patch would be great. If not I will make a pass at is likely later this week.

Very good. I'm currently migrating some more of my PIO functions and take notes where things could be improved. I guess I will have some patches for PIO ready for your review next week.

[electronic_eel]

Hi,

when porting my pio projects, the most annoying issue was that building the header files out of the pio assembler files was not automated in the build process and I had to manually call pioasm all the time. This is why I addressed this first.

I have published a branch here: https://github.com/electroniceel/ChibiO ... 2-pio-make but also attached a patch file, zipped because of forum restrictions, take whatever you prefer.

How to use this:

- install pioasm from the Raspberry Pi pico-sdk
- write some pio assembler and store it in a file with .pio extension, just like you'd do with the pico-sdk. For example foo.pio.
- use this in your program by including it like this:

#include "build/obj/foo.pio.h"

- when creating the rp_pio_program_t struct, reference the "foo_program_instructions". Use sizeof to get the program size.
- modify your project Makefile to include pio_rp2.mk after rules.mk like this:

Code: Select all

RULESPATH = $(CHIBIOS)/os/common/startup/ARMCMx/compilers/GCC/mk
include $(RULESPATH)/arm-none-eabi.mk
include $(RULESPATH)/rules.mk
include $(RULESPATH)/pio_rp2.mk

About my design choices:
I chose to define PICO_NO_HARDWARE 1. That means from the pioasm just the basic assembler output is used. pioasm outputs a struct pio_program too that now can't be used and you have to create a similar rp_pio_program_t instead. But pioasm also always adds C code that calls several functions of the pico-sdk, like pio_get_default_sm_config(), sm_config_set_wrap(),... - these would have to be provided for the program to compile and they would clutter the namespace. The only really interesting info in struct pio_program is the size, and that is something you can get via sizeof too.

For my Makefile rules to work, they must be executed only after CSRC is filled, so I can't just add the code to driver.mk of the pio lld or similar. This is why I created the new pio_rp2.mk and put it in the same path as the other .mk files relevant for the RP2 arch.

It was a bit difficult to create the correct dependency instructions for make to work. When everything is built, there is a .d file that contains everything and is properly created. But this is too late. To start the build, make must know beforehand which pio files are needed where. This is why I resorted to calling grep, a bit similar to what the Makefiles already do for HALCONF. I know that this part isn't perfect, but I don't know of a better way of doing this automatically.

The generated .pio.h files are generated and must be cleaned by "make clean" and so on. I didn't want to create a separate dir for them, so I put them into "./build/obj/", although they are technically not object files. I can of course create a new directory below build for this if you or Giovanni prefer it.

Please review, I'm happy to further explain my patch or adjust it to your suggestions.

Do you also want me to create some test code for this? This would be no problem for me, but for building it you would need to have pioasm from the pico-sdk installed.

[emulator]

I will take a look tomorrow after I finish cleaning up the PAL LLD.

- E

[electronic_eel]

Before tackling PIO further, I did some more Makefile-work to ease working with the RP2 MCUs.

Makefiles are very sensitive to ordering. Currently the generic ARM GCC rules.mk does not allow you to add something to $(OUTFILES). You can't add something beforehand because it overwrites it and you can't add anything afterwards because then "all:" is already defined with the previous $(OUTFILES). I want to add the .uf2 file for the bootloader to $(OUTFILES) so that it is always built automatically with all.

So the first patch is preparing the generic ARM GCC rules.mk to allow adding things to $(OUTFILES) before rules.mk. Also I explicitly define that the default goal is called "all". This makes it easier to define additional custom goals like the "upload" goal for directly flashing to the bootloader from the Makefile.

This change is something to the generic rules.mk, so maybe Giovanni also wants to take a look.

Building on this is a new Make rules file that builds the .uf2 files and allows to directly flash to a RP2 MCU in bootloader mode.

As I said ordering is important, so this is how things should be ordered in a Makefile:

Code: Select all

RULESPATH = $(CHIBIOS)/os/common/startup/ARMCMx/compilers/GCC/mk
include $(RULESPATH)/arm-none-eabi.mk
include $(RULESPATH)/flash_rp2.mk
include $(RULESPATH)/rules.mk
include $(RULESPATH)/pio_rp2.mk

Patches attached, I also added them to the github branch linked in the previous post.

My next goal is to work on creating defines and macros in CMSIS-style for the RP2 PIO assembler commands, to be used with pioSmExecX(). I'd say when doing things properly, this should be done before starting to work with pioSmExecX() for initializing PIO output enables.

[emulator]

So far I've avoided adding flashing in any of the Makefiles as I think its too opinionated and should be left outside of ChibiOS. Putting it in ChibiOS means getting opinionated about things like BOOTSEL + PicoBoot, BOOTSEL + UF2 copy, debug probe, production flash programming, etc.

(Their is a contra argument BTW, the ChibiOS AVR port does include support for multiple options to flash...)

For now though I would remove the flash support from this MR and lets move it to the Development and Feedback forum as this is mostly a generic question about ARM Makefiles and not RP specific. If you still want to raise the RP Flash support MR post just that change in a new thread in this forum and lets talk about it independent of PIO or the generic Makefile changes.

[electronic_eel]

Sorry for mixing up the two issues. I reset my branch at https://github.com/electroniceel/ChibiO ... 2-pio-make to just contain the PIO specifics and created a new thread at viewtopic.php?t=6691 about the generic rules.mk change.

I concur that automatic flashing from the Makefiles makes some assumptions about the setup. So I plan to just create a .mk file that allows the users to always build the .uf2 file. But this is for another thread, let's keep to PIO here.