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

cpu/rpx0xx: initial PIO support #17425

Merged
merged 12 commits into from
May 23, 2023
Merged

cpu/rpx0xx: initial PIO support #17425

merged 12 commits into from
May 23, 2023

Conversation

fabian18
Copy link
Contributor

@fabian18 fabian18 commented Dec 19, 2021

Contribution description

This adds a new periph interface for Programmable IO (PIO) and implements PIO
for the rpi-pico. To test basic functionality, there is a test application:
tests/periph_pio/. To test further, there is also a PIO implementation for
RIOTs I2C interface which should allow you to run tests for I2C device drivers.

We need the pioasm from the pico-sdk, to assemble PIO programs. I compiled the assembler and copied the binary to /usr/local/bin/pioasm. It is downloaded from github and compiled during the build process if needed. The bash script should work with all PIO programs from pico-examples.

Testing procedure

$ cd tests/driver_at24cxxx
$ CFLAGS+="-DPIO_I2C_CONFIG=\{.sda=2,.scl=3,.irq=0\}" BOARD=rpi-pico make flash term
2021-12-18 21:03:32,720 # main(): This is RIOT! (Version: 2021.10-devel-1049-gd35a26-rp2040_PIO)
2021-12-18 21:03:32,723 # Starting tests for module at24cxxx
2021-12-18 21:03:32,725 # EEPROM size: 32768 byte
2021-12-18 21:03:32,727 # Page size  : 64 byte
2021-12-18 21:03:32,729 # [SUCCESS] at24cxxx_init
2021-12-18 21:03:32,732 # [SUCCESS] at24cxxx_write_byte
2021-12-18 21:03:32,735 # [SUCCESS] at24cxxx_read_byte
2021-12-18 21:03:32,738 # [SUCCESS] write_byte/read_byte
2021-12-18 21:03:32,744 # [SUCCESS] at24cxxx_write
2021-12-18 21:03:32,747 # [SUCCESS] at24cxxx_read
2021-12-18 21:03:32,749 # [SUCCESS] write/read
2021-12-18 21:03:32,755 # [SUCCESS] at24cxxx_set
2021-12-18 21:03:32,760 # [SUCCESS] set/read
2021-12-18 21:03:32,763 # Finished tests for module at24cxxx

See also logic analyzer output:
at24cxxx.zip

Issues/PRs references

#15822

@github-actions github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework labels Dec 19, 2021
@benpicco benpicco requested a review from maribu December 20, 2021 12:52
@krzysztof-cabaj
Copy link
Contributor

krzysztof-cabaj commented Dec 22, 2021

Great work! I'm waiting for this device as I have some plans for WS2812B project ... and my old Arduino UNO is far too small.
RPi-pico and PIO sound fantastic!

However, I have some doubts if manual downloading of tools outside RIOT directory is a good idea. In my last PR 17348, I added makefiles that automatically download pico-SDK, compile elf2uf2 tool and copy it to the appropriate directory. Maybe such a solution could be useful. Look at RIOT/makefiles/tools/targets.inc.mk (end of file, section concerning elf2uf2) and
RIOT/dist/tools/elf2uf2/Makefile .

@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Jan 1, 2022
@krzysztof-cabaj
Copy link
Contributor

krzysztof-cabaj commented Jan 3, 2022

After w few days working with this code, I have two questions/suggestions.

  1. Why are low-level functions that allow interaction with PIO not accessible for normal RIOT programs? During my experiments I use some "dirty-hack" and copy pio.h from RIOT/cpu/rpx0xx/include/pio/ to RIOT/drivers/include/periph/ renaming it to pio_lowlevel.h. After that, I could use all functions from my code. I think this could be beneficial for other users, and now we cannot imagine for what purpose PIO can be used in various projects.

  2. During my experiments, I have some problems executing PIO code. I suspect this is the best place to describe my issue and ask for help. I plan to use a simple PIO program to blink onboard LED.

.program blink01

.wrap_target
begin:
 set pins, 0
 nop
 set pins, 1
 nop
 jmp begin
.wrap

This code is similar to this micro-python code or a little more complicated from pico-examples blink. I purposely changed LOW and HIGH levels and added jmp instruction - because during my test, I suspected that my code is executed only once. My success is that the presented code switch LED on. However, even adding additional jmp instruction does not blink. When I used the most common idea - first set the pin to HIGH and later to LOW - I haven't noticed anything - maybe LED blink by a very short while - but I didn't notice this. Below is a list of used functions and structures; however, I used them without details. Is there missing some critical function? Any ideas why the PIO program is executed only once?

fill using appropriate data: pio_program_t, pio_instr_t and pio_program_conf
pio_write_program(...)
pio_alloc_program_sm_lock_any(...)
pio_sm_init_common(...)
pio_sm_set_set_pins(...)
pio_sm_set_clkdiv(...) ... using pio_sm_clkdiv(...)
fill using appropriate data: gpio_pad_ctrl_t and gpio_io_ctrl_t
gpio_set_pad_config(...)
gpio_set_io_config(...)
pio_sm_restart(...)
pio_sm_start(...)

@fabian18
Copy link
Contributor Author

fabian18 commented Jan 6, 2022

First of all thank you for testing!

  1. My intention why low level PIO functions should not accessible from normal RIOT programs was that users should not interact directly with CPU specific PIO details (program wrapping, FIFO joining, interrupts).
    Instead, there should be dedicated PIO programs which define their interface, for your example pio_blink_init() which users should interact with. Or a driver for the WS2812B which you are planning to use.
    Only the person who writes the program must know the lowlevel PIO API. The program is written for a dedicated CPU. If there was another CPU with PIO, I´d expect that e.g. the I2C implementation could look very different. Moreover the actual .pio program would look different because of different instruction sets. But both CPUs would have FEATURES_PROVIDED += pio_i2c and share the pio_i2c_*() interface.
    But actually nothing stops you from using the lowlevel API from your main.c if you #include "pio/pio.h" and implement everything in your application. But e.g. the PIO driver for the WS2812B should be in the CPU folder, in my opinion.

  2. I´d say your suspicion that your LED blinks too fast for human eyes is correct. The PIOs run from the system clock (125 MHz) divided by some prescaler that you can configure wtth pio_sm_set_clkdiv().
    The largest possible prescaler is 65536, hence ((125MHz / 65536) ~ 1908 cycles/sec) is as slow as you can get. One period in your program takes 5 cycles. That means your LED would blink ((1908 / 5) ~ 381 times/sec), but most people’s maximum ability to detect flicker ranges between 50 and 90 Hz. The micro-python code that you linked includes a lot of nop instructions and 31 delay cycles after every instruction to make the blinking of the LED visible. Your call sequence looks good except that you first have to allocate instruction memory (pio_alloc_program_sm_lock_any()) and then write the instructions to the allocated memory (pio_write_program()). But I noticed that pio_write_program() does not check if prog->location == PIO_PROGRAM_NOT_LOADED, which I think it better should do.

I believe you set up the pin correctly but just to exclude the case that you didn´t, here is how it worked for me.

static void pio_blink_init_pins(pio_t pio, pio_sm_t sm, gpio_t pin)
{
    const gpio_io_ctrl_t io_ctrl = {
        .function_select = pio ? FUNCTION_SELECT_PIO1 : FUNCTION_SELECT_PIO0
    };
    gpio_set_io_config(pin, io_ctrl); /* set alternate fuction of "pin" */
    pio_sm_set_set_pins(pio, sm, pin, 1); /* set pin mapping starts at "pin" */
    pio_sm_exec_block(pio, sm, pio_inst_set(PIO_INST_SET_DST_PINDIRS, 0x1)); /* make "pin" an output pin */
}

@krzysztof-cabaj
Copy link
Contributor

Thank you for your detailed response.

  1. Yes, I agree that if somebody uses PIO only to add new or more peripheral, access to the low-level functions could be dangerous. However, I still think that PIO is so universal that many interesting solutions can be programmed. Thank you for the hint with #include "pio/pio.h - my program works fine without my previous 'dirty-hack". Moreover, during my longer than expected tests, I developed a program that can be used as an example of PIO low-level access, such as blinking LEDs. If there will be a need, I could clean it and add to the RIOT examples when your PIO code becomes merged to the RIOT master.

  2. During my experiments with code and digging into RP2040 datasheet, I realized a limitation for minimal frequency. I'm confused by the pio_sm_clkdiv function, which takes very low values without errors. There is no clear information in the doxygen documentation that the minimal value of PIO frequency is around 2 kHz. When I loaded a program with many empty cycles, I could see LED blinking. One comment to your code - my program do not use pio_sm_exec_block(pio, sm, pio_inst_set(PIO_INST_SET_DST_PINDIRS, 0x1)); function and everything works well. I suspected, that function pio_sm_set_set do this for PIO set instruction. As there are functions pio_sm_set_out_pins and pio_sm_set_in_pins respectively for in and out PIO instructions.

@Ollrogge
Copy link
Member

Hey,

running the testing procedure as you described I get the following error output:

2022-01-12 21:49:54,166 # main(): This is RIOT! (Version: 2019.10-devel-15051-gc89b5-rp2040_PIO)
2022-01-12 21:49:54,166 # Starting tests for module at24cxxx
2022-01-12 21:49:54,166 # EEPROM size: 32768 byte
2022-01-12 21:49:54,166 # Page size  : 64 byte
2022-01-12 21:49:54,166 # [SUCCESS] at24cxxx_init
2022-01-12 21:49:54,167 # [FAILURE] at24cxxx_write_byte: -6
2022-01-12 21:50:00,400 # Exiting Pyterm

Any idea whats going wrong ?

@fabian18
Copy link
Contributor Author

Any idea whats going wrong ?

You could get this if you for example have not specified: CFLAGS+="-DPIO_I2C_CONFIG=\{.sda=2,.scl=3,.irq=0\}".
Maybe you have mixed up some pins. Which EEPROM do you use?

@Ollrogge
Copy link
Member

u have mixed up some pins. Which EEPROM do you use?

Ahh yes I mixed up some pins ... :S. Works now, thanks !

gpio_set_pad_config(scl, pad_ctrl);
gpio_set_pad_config(sda, pad_ctrl);

pio_sm_exec_block(pio, sm, pio_inst_set(PIO_INST_SET_DST_PINS, 0x3));

Choose a reason for hiding this comment

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

this here needs some text to be understand able, thus why first writenig 3 then 0 again....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also works without set pins 3 and later set pins 0. But the pico SDK does it like that too.
I guess that is what they mean with "avoid glitching". I now have copied their comment.

Choose a reason for hiding this comment

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

my problem has, i did not knew that I have to setup my pio pins as output manually (to just write to them with the "out" instruction), thus after hours in the end I ended up looking into mp and pico sdk what they do - thus some comment here would have helped me a lot. i guess the pio interface should have something like "init pin" as output.
When I have my code in a clean shape, I can drop it as an example, and maybe i also get a clue howto do a init pin function, or even getting this compile pio step bit cleaner... at the moment I believe its can be made more convenient.
I used pio in mp before and i was that clean and easy, some of that must be possible here too ;-)

@fabian18
Copy link
Contributor Author

Btw. I think I am going to squash. These fixup!s are getting messy.

@maribu
Copy link
Member

maribu commented May 13, 2023

bors merge

bors bot added a commit that referenced this pull request May 13, 2023
17425: cpu/rpx0xx: initial PIO support r=maribu a=fabian18



Co-authored-by: Fabian Hüßler <fabian.huessler@st.ovgu.de>
@bors
Copy link
Contributor

bors bot commented May 13, 2023

Build failed:

@maribu
Copy link
Member

maribu commented May 13, 2023

looks like a new issue this time 👍 It's getting closer

@fabian18
Copy link
Contributor Author

fabian18 commented May 13, 2023

It's getting closer

yes ... 😅

I am thinking of two alternatives.
I can either provide a dummy i2c_init() which does nothing for dev >= I2C_NUMOF in cpu/rpx0xx/periph/i2c.c, or
I can move pio_t pio and pio_sm_t sm to pio_i2c_conf_t pio_i2c_config, so they have to be specified by configuration but i2c_init() could for dev >= I2C_NUMOF do the stuff in pio_i2c_init_programs() with the static pio and sm.
Option 2 seems cleaner to me.

@dylad
Copy link
Member

dylad commented May 13, 2023

I think option 2 would be good too.

@benpicco
Copy link
Contributor

bors merge

bors bot added a commit that referenced this pull request May 22, 2023
17425: cpu/rpx0xx: initial PIO support r=benpicco a=fabian18



19611: sys/net/rpl: fix possible NULL dereference r=benpicco a=maribu

### Contribution description

As the title says


19640: core/thread: drop unused thread_arch_t r=benpicco a=maribu

### Contribution description

No architecture makes use of thread_arch_t anymore, so let's drop it.


Co-authored-by: Fabian Hüßler <fabian.huessler@st.ovgu.de>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
@bors
Copy link
Contributor

bors bot commented May 22, 2023

Build failed (retrying...):

@benpicco
Copy link
Contributor

Kconfig is unappy

bors cancel

@bors
Copy link
Contributor

bors bot commented May 22, 2023

Canceled.

@fabian18
Copy link
Contributor Author

Kconfig is unappy

I forgot that there is a Kconfig.features listing all features

@maribu
Copy link
Member

maribu commented May 23, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented May 23, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@bors
Copy link
Contributor

bors bot commented May 23, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit f315012 into RIOT-OS:master May 23, 2023
@maribu
Copy link
Member

maribu commented May 23, 2023

🚀 thx for the PR

@fabian18
Copy link
Contributor Author

Thanks to all for your patience and good feedback!

@dylad
Copy link
Member

dylad commented May 23, 2023

Congrats @fabian18 ! Thanks for your hard work !

@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: drivers Area: Device drivers Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants