rpi_pico: Added Generic PIO support and a PIO based UART driver

[iocapa]

Needed some extra uarts some time ago (interrupt capable).

Knew that #44316 existed, but I needed something fast and decided to roll my own.

@yonsch and everyone else.
Feedback is appreciated.

Cleaned the code up a bit and opened this PR, as it was sitting on my drive for some time.
Leaving this here, maybe someone could use it in their own project or maybe it would be helpful in current/future PIO implementations.

Some technical details:
Uses the existing bus / on-bus pattern. The PIO is the bus and manages resources / interrupts. The child devices implement the functionality.

Does not use the pico sdk or pioasm. This was intended, as the PIO is not really a complicated piece of hardware at the register level (program design is so-so). The programs are crafted by the peripheral implementing the function using whatever is fit for that implementation (there are some helper macros provided). This allows for more flexibility, for example, in the case of UART, a patch operation (dynamically changing instructions during runtime) is desired to change the number of stop bits.

Instruction / State machine allocation is performed at driver init. A static allocation scheme was experimented with in the beginning, but it falls short for devices sharing program space.

The rule on usage depends on the driver. The idea is that the child device should take care not to access anything (SMs / instructions / registers) not specifically allocated to it, including IRQ flags. For example, a driver using only SM0 should only use IRQ flags 0 and 4, a driver using all 4 SMs could use all flags.

Uart:
Not fully tested in all modes (data/stop lengths) but looks like it works as it should for shell / logging operations.
Experiments were done with adding TX parity suport / wide (9 bit) support, but I don't think it's useful ATM.

Pretty sure I omitted some technical details, but if anyone has any question, feel free to ask.

/ {
	chosen {
		zephyr,console = &pio0_uart0;
		zephyr,shell-uart = &pio0_uart0;
	};
};

&uart0 {
	status = "disabled";
};

&pio0 {
	status = "okay";

	pio0_uart0: pio0_uart0 {
		compatible = "raspberrypi,pico-pio-uart";

		status = "okay";

		pio,interrupts = <0>;
		pio,interrupt-names = "shared";

		pinctrl-0 = <&pio0_uart0_default>;
		pinctrl-names = "default";

		current-speed = <115200>;
	};
};

#include <zephyr/dt-bindings/pinctrl/rpi-pico-rp2040-pinctrl.h>

&pinctrl {
	pio0_uart0_default: pio0_uart0_default {
		tx_gpio {
			pinmux = <PIO0_P0>;
		};

		rx_gpio {
			pinmux = <PIO0_P1>;
			input-enable;
			bias-pull-up;
		};
	};
};

*** Booting Zephyr OS build 1ac10e9d44d0 ***
[00:00:01.001,000] <inf> main: One second passed...
[00:00:02.001,000] <inf> main: One second passed...
[00:00:03.001,000] <inf> main: One second passed...
[00:00:04.001,000] <inf> main: One second passed...
uart:~$ help
Please press the <Tab> button to see all available commands.
You can also use the <Tab> button to prompt or auto-complete all commands or its subcommands.
You can try to call commands with <-h> or <--help> parameter for more information.

Shell supports following meta-keys:
  Ctrl + (a key from: abcdefklnpuw)
  Alt  + (a key from: bf)
Please refer to shell documentation for more details.

Available commands:
  clear    :Clear screen.
  device   :Device commands
  devmem   :Read/write physical memory
            Usage:
            Read memory at address with optional width:
            devmem address [width]
            Write memory at address with mandatory width and value:
            devmem address <width> <value>
  help     :Prints the help message.
  history  :Command history.
  kernel   :Kernel commands
  log      :Commands for controlling logger
  resize   :Console gets terminal screen size or assumes default in case the
            readout fails. It must be executed after each terminal width change
            to ensure correct text display.
  shell    :Useful, not Unix-like shell commands.
[00:00:05.002,000] <inf> main: One second passed...
[00:00:06.002,000] <inf> main: One second passed...

[soburi]

Instruction / State machine allocation is performed at driver init. A static allocation scheme was > experimented with in the beginning, but it falls short for devices sharing program space.

I think static allocation seems possible in theory. It would be helpful if you could give me more details about the problem.

[soburi]

I don't think there is a need to distribute IRQs.

It's good to treat it as a bus on the devicetree syntax.
But I think PIO is not 'Actual bus', because it is not bus controller.
The protocol driver can use the interrupt defined in the PIO node directly.
Two devices cannot use one PIO simultaneously, and dts statically configure devices, so a particular device occupies PIO. It not cause problem.

               IRQ_CONNECT( \
                       DT_IRQ_BY_IDX(DT_INST_PARENT(idx), 0, irq), \
                       DT_IRQ_BY_IDX(DT_INST_PARENT(idx), 0, priority), \
                       irq_handler, NULL, (0)); \

It is easy to handle as it follows standard IRQ practices.
And from other viewpoint, ISR should be better not have intermediate processing.

[iocapa]

@soburi

Actually you can have as many devices as you want (can) on a PIO instance. See below for how I'm adding 4 extra uarts (2 per each PIO instance). In practice, you are only limited by the number of SMs / Instruction memory availability as there's no real inter dependency between SMs.

The 'bus' / 'on-bus' thing is mostly for static checking as you are required to be on a PIO parent device. This could be however implemented with a phandle property for each device, but it makes the DTS and checks more confusing.

The intermediate IRQ routing is done for potential load sharing purposes. You could for example overlay PIO IRQ0 to a higher prio and assign it to a child device that requires that higher prio, or you could have one or more child devices that need separate IRQ lines.

&pio0 {
	status = "okay";

	pio0_uart0: pio0_uart0 {
		compatible = "raspberrypi,pico-pio-uart";

		pio,interrupts = <0>;
		pio,interrupt-names = "shared";

		pinctrl-0 = <&pio0_uart0_default>;
		pinctrl-names = "default";

		current-speed = <115200>;
	};

	pio0_uart1: pio0_uart1 {
		compatible = "raspberrypi,pico-pio-uart";

		pio,interrupts = <1>;
		pio,interrupt-names = "shared";

		pinctrl-0 = <&pio0_uart1_default>;
		pinctrl-names = "default";

		current-speed = <115200>;
	};
};

&pio1 {
	status = "okay";

	pio1_uart0: pio1_uart0 {
		compatible = "raspberrypi,pico-pio-uart";

		pio,interrupts = <0>;
		pio,interrupt-names = "shared";

		pinctrl-0 = <&pio1_uart0_default>;
		pinctrl-names = "default";

		current-speed = <115200>;
	};

	pio1_uart1: pio1_uart1 {
		compatible = "raspberrypi,pico-pio-uart";

		pio,interrupts = <1>;
		pio,interrupt-names = "shared";

		pinctrl-0 = <&pio1_uart1_default>;
		pinctrl-names = "default";

		current-speed = <115200>;
	};
};

[soburi]

Thank you for the explanation. I understood it.

In this case, since there is only one PIO program, I would expect multiple instances of the exact driver.
If so, I think it would be good to distribute interrupts for each driver. However, in this case, duplicate implementations may increase.
Providing this as a simple framework for client drivers would be nice.

(Simply, I think it is better that driver developers need not care about any special rules.)

[soburi]

This is a much interesting implementation.
It should be useful for manage pio code even if it is back-translated from pioasm output.

Could you please consider the following two points?

• Separating PIO_ASM_... macros to standalone file.
• Adding NOP instructions (I understand it can implements by MOV, but frequently appeared.)

[iocapa]

Instruction / State machine allocation is performed at driver init. A static allocation scheme was > experimented with in the beginning, but it falls short for devices sharing program space.

I think static allocation seems possible in theory. It would be helpful if you could give me more details about the problem.

It's a bit of a hack, but the idea is as follows:
Each device has the default sm-count and instr-count properties in their bindings.
After that, you expand all all children and compute offsets based on the parent index and the sum of all props till the device index.
This works, but to share program space you would need to expand based on compat, and I'm not sure if this is possible at all.

Here is a snippet:

/**
 * @brief Compute resource offset based on other child nodes
 * 
 * Generates an expression in the following form:
 * ((N >= M) ? 0 : DT_PROP(res))
 * 
 * Will throw an error if other nodes do not have the 'res' property
 * This is required as a device not requiring the PIO should not be the
 * child of a PIO instance.
 * 
 * Where:
 * N           = Index of a node in the parent binding
 * M           = Index of the current node in the parent binding
 * res         = Value of the res property for the child_node
 */
#define RPI_PICO_PIO_COUNT_RES_OFFSET(child_node, node_id, res)                                        \
       ((DT_NODE_CHILD_IDX(child_node) >= DT_NODE_CHILD_IDX(node_id))                          \
               ? 0 : DT_PROP_OR(child_node, res, 0))

/**
 * @brief Gets one resource ID and offsets it
 * 
 * Generates a sum of offsets base on the 'res' property of all nodes:
 * 
 * ((N[0] >= M) ? 0 : res) + ((N[1] >= M) ? 0 : res) // ...
 */
#define RPI_PICO_PIO_GET_RES_ID(node_id, res, offset)                                          \
       ((DT_FOREACH_CHILD_STATUS_OKAY_SEP_VARGS(                                               \
               DT_PARENT(node_id), RPI_PICO_PIO_COUNT_RES_OFFSET, (+), node_id, res)           \
        ) + offset)

/**
 * @brief Assign a unique state machine ID from the available pool
 * 
 * State machines are allocated based on the DT structure.
 * Each peripheral implemented by PIO must have the 'sm_count'
 * property. This property must have a value equal to the number
 * of state machines required by the peripheral.
 * 
 * Example devicetree fragment:
 * 
 * @code{.dts}
 *     pio {
               status = "okay";
 *             c1: child-1 {
 *                     sm-count = <2>; // Fixed, based on peripheral requirements
 *             };
 *             c2: child-2 {
 *                     sm-count = <2>;
 *             };
 *     };
 * @endcode
 * 
 * Example bindings fragment:
 * 
 * @code{.yaml}
 *     include: "raspberrypi,pico-pio-device.yaml"
 *     properties:
 *       sm-count:
 *         type: int
 *         default: 2
 *         const: 2
 * @endcode
 * 
 * Example usage:
 * 
 * @code{.c}
 *     // child_2.c
 *     DT_RPI_PICO_PIO_ASSIGN_SM(node, 0)      // 2
 *     DT_RPI_PICO_PIO_ASSIGN_SM(node, 1)      // 3
 *
 *     // child_1.c
 *     DT_RPI_PICO_PIO_ASSIGN_SM(node, 0)      // 0
 *     DT_RPI_PICO_PIO_ASSIGN_SM(node, 1)      // 1
 * @endcode
 * 
 * The offset must be contained in the number of state machines assigned
 * to the peripheral. For example, requesting an ofsset of 2 for a sm-count
 * of 2, will result in an error.
 * 
 * @param node_id node identifier
 * @param offset The offset of the state machine in the assigned group
 * @return the assigned state machine's index
 */
#define DT_RPI_PICO_PIO_ASSIGN_SM(node_id, offset)                                             \
       RPI_PICO_PIO_GET_RES_ID(node_id, sm_count, offset)

/**
 * @brief Assign a unique state machine ID from the available pool
 * 
 * @param inst instance number
 * @param offset The offset of the state machine in the assigned group
 * @return the assigned state machine's index
 * 
 * @see DT_RPI_PICO_PIO_ASSIGN_SM
 */
#define DT_INST_RPI_PICO_PIO_ASSIGN_SM(inst, offset)                                           \
       DT_RPI_PICO_PIO_ASSIGN_SM(DT_DRV_INST(inst), offset)

/**
 * @brief Assign a unique program instruction ID from the available pool
 * 
 * Program instructions are allocated based on the DT structure.
 * Each peripheral implemented by PIO must have the 'sm_count'
 * property. This property must have a value equal to the number
 * of instructions required by the peripheral.
 * 
 * Example devicetree fragment:
 * 
 * @code{.dts}
 *     pio {
               status = "okay";
               clocks = <&system_clk>;
 *             c1: child-1 {
 *                     instr-count = <16>; // Fixed, based on peripheral requirements
 *             };
 *             c2: child-2 {
 *                     instr-count = <16>;
 *             };
 *     };
 * @endcode
 * 
 * Example usage:
 * 
 * @code{.c}
 *     // child_2.c
 *     DT_RPI_PICO_PIO_ASSIGN_INSTR(node, 0)   // 16
 *     DT_RPI_PICO_PIO_ASSIGN_INSTR(node, 8)   // 24
 *
 *     // child_1.c
 *     DT_RPI_PICO_PIO_ASSIGN_INSTR(node, 0)   // 0
 *     DT_RPI_PICO_PIO_ASSIGN_INSTR(node, 8)   // 8
 * @endcode
 * 
 * The offset must be contained in the number of program instructions assigned
 * to the peripheral. For example, requesting an ofsset of 16 for a instr-count
 * of 16, will result in an error.
 * 
 * @param node_id node identifier
 * @param offset The offset of the program instruction in the assigned group
 * @return the assigned program instruction's index
 */
#define DT_RPI_PICO_PIO_ASSIGN_INSTR(node_id, offset)                                          \
       RPI_PICO_PIO_GET_RES_ID(node_id, instr_count, offset)

/**
 * @brief Assign a unique instruction location from the available pool
 * 
 * @param inst instance number
 * @param offset The offset of the instruction in the assigned group
 * @return the assigned instructions's index
 * 
 * @see DT_RPI_PICO_PIO_ASSIGN_INSTR
 */
#define DT_INST_RPI_PICO_PIO_ASSIGN_INSTR(inst, offset)                                                \
       DT_RPI_PICO_PIO_ASSIGN_INSTR(DT_DRV_INST(inst), offset)

[galak]

The interrupts for PIO should be done as a nested interrupt controller and thus their representation in devicetree would utilize normal interrupt properties.

[galak]

uart-controller.yaml now has stop-bits and data-bits, so you can remove them here

[iocapa]

Done

[iocapa]

@galak

The interrupts for PIO should be done as a nested interrupt controller and thus their representation in devicetree would utilize normal interrupt properties.

Open to suggestions, maybe I'm missing some things.
Thought about this initially, however, I think there are some issues in moving this to a 2nd level interrupt scheme.

• You still need to know the index of the PIO interrupt as you need to set one of 2 (future IP revisions could have more) internal PIO registers to enable/disable. This still requires some kind of mapping from the assigned IRQ id to the actual PIO IRQ id.

• Since lines are shared, you need to filter events inside the ISR, as all of them will be called if enabled, this has to be done by the child device. Filtering could be done at the initial handler level, but it creates runtime dependencies since you need to know the SM indexes, and these are allocated at init.

• You could potentially have a maximum of 2*4 IRQs per PIO (for example, 4 devices). And, depending on use-case you can mix the assignments to satisfy some system requirements (maybe timing). I'm not really sure how this could be accomplished with a 2nd level INTC.

&pio0 {
	status = "okay";
	device0: device0 {
		compatible = "raspberrypi,pico-pio-future-device";
		pio,interrupts = <0>, <1>;
		pio,interrupt-names = "tx", "rx";
	};
	device1: device1 {
		compatible = "raspberrypi,pico-pio-future-device";
		pio,interrupts = <1>, <1>;
		pio,interrupt-names = "tx", "rx";
	};
        // .....
	deviceN: deviceN {
		compatible = "raspberrypi,pico-pio-future-device";
		pio,interrupts = <1>, <1>;
		pio,interrupt-names = "tx", "rx";
	};

[iocapa]

Hey @yonsch @soburi @galak @dcpleung. Can you have another look at this one?

I think the major point of debate around this was about interrupt handling.
I explained in the comments above why I feel this is necessary and why IMO there is no way around it.
Any other ideas are welcomed.

Another topic was the use of the raspberry pi API.
While I have nothing against it, again, IMO, it creates more issues than it solves. It has a lot of convoluted APIs for a pretty simple peripheral at the register level. Also future issues could be introduced by rpi SDK updates.

The PIO has 6 registers for each SM, and 3 of them (CLKDIV, ADDR, INSTR) are pretty self explanatory, the others (EXECCTRL, SHIFTCTRL, PINCTRL) are pretty well documented in the DS. So it's pretty easy to understand and configure. There are also other issues described in the conversations above.

Now, architecturally defining a PIO peripheral might not be that easy, but this is true also by using the SDK. Programs and flows still need to be thought of, and there has to be a pretty good understanding on how the PIO works (the DS is pretty good on this side).

Rpi provided examples are a good starting point, but they are pretty limited in scope and most of the times expanding them to a full featured peripheral requires a major rewrite.

If anyone has any questions/suggestions, please let me know.

P.S. The proposed API (including the UART drivers) was pretty well tested for some time in a limited production environment. No obvious issues till now. Also, I've used this api for another PIO based driver (JTAG/SWD) with quite interesting results.

Ionut

[soburi]

IMO, It is good to be prudent about adding peripheral-specific API.
If possible, using an existing API or a similar configuration would be preferable. The shared_irq previously mentioned has a dts interface, so isn't it possible to extend and use it?
And some PR proposes to extend shared_irq. (#61331, #57146)
Is it possible to solve this problem by extending shared_irq?
(I think this problem is essentially not PIO specific problem. It should abstract the problem and apply it to generic cases.)

[soburi]

@yonsch Could you tell us your opinion, please?

[iocapa]

Is it possible to solve this problem by extending shared_irq?
(I think this problem is essentially not PIO specific problem. It should abstract the problem and apply it to generic cases.)

@soburi shared-irq could technically be adapted to support pio shared IRQs, but it would mean adding PIO only code inside it, which would break it's generic approach IMO.

[soburi]

@yonsch Could you tell us your opinion, please?

[soburi]

Another idea is overwriting the "compatible" line by overlay file to replace the PIO driver completely.
We can think of PIO as a separate device for each loaded program. From this point of view, it can be a method suitable for Zephyr design.

[iocapa]

Another idea is overwriting the "compatible" line by overlay file to replace the PIO driver completely. We can think of PIO as a separate device for each loaded program. From this point of view, it can be a method suitable for Zephyr design.

@soburi
Not sure I understand your idea. Can you give more details?
Currently, multiple devices can share the same program. This has a lot of benefits for many use-cases.

@ALL
Since the PIO SPI driver was committed as part of:
85cbc7a
Compliance checks will fail as the commit is not compatible with this PR.

Most likely, to make this PR pass all checks again and maintain the SPI driver, me (or someone else) needs to recreate the driver using the framework in this PR (either by a new commit on this PR or chaining another PR starting from this one as base and temporary disabling the existing SPI driver).

It's not a massive amount of work, but it's work nonetheless.

So, I would appreciate some feedback on this PR to see where it's going. As more PIO based drivers will be created this will become harder to keep up to date.

[yonsch]

@iocapa @soburi
Sorry for taking so long to get to this PR.
As far as I can tell (tell me if I missed anything), it consists of the following:

1. Reimplementing the PIO device without pico-sdk
2. Adding an "assembler" implemented with macros
3. Adding support for interrupts
4. Adding more UART functionality
5. Making the pico-device binding a bus rather than a compatible
6. Converting the uart_pio sample to a shell
7. Renaming stuff (e.g. rpi pico pio to pio rpi pico)

Many of which are a topic for a discussion.

Reimplementing the PIO device without pico-sdk

This is a dilemma we're running into with most of the pico drivers. pico-sdk sometimes does pretty annoying things internally, which forces us to reimplement low level code. I generally think we should start migrating away from the pico-sdk code, but this is a big decision and requires a more thorough discussion.

Adding an "assembler" implemented with macros

This is a cool idea, but I wonder why we'd prefer it over using pioasm. I know that currently it's not available, but supposedly we'll one day have it as an installable package (raspberrypi/pico-sdk#1345), which we could then use to assemble the programs on every build. It might require some more work around the build system, but that's less work than maintaining an assembler. Just think of debugging a PIO program and discovering that it's a "compiler" bug...

Adding support for interrupts

This should be very carefully planned, both on the PIO side and core side. I really should dive deeper into this before commenting further.

4, 5 and 6

Sounds good overall

Renaming stuff (e.g. rpi pico pio to pio rpi pico)

Not sure if this was intentional or just an artifact. If it's intentional, and you have good reasoning to change a name, we can probably still change it. If it's just a matter of taste, I prefer to leave it as is.

[iocapa]

@yonsch
Please see my responses.

Reimplementing the PIO device without pico-sdk

At least on the PIO side, the sdk is pretty messy. Sure it's nice to read a few examples and make a really limited implementation for a peripheral. This falls apart pretty fast when you get into the more advanced PIO usage scenarios (interrupts, DMA, dynamic patching, execution from FIFO streams, dynamic reconfiguration, etc...).
The PIO is a pretty capable accelerator, it can do some really nice things, the SDK doesn't really help with that.

Adding an "assembler" implemented with macros

The macros are not meant to replace pioasm, they're just to avoid random hardcoded hex numbers in source files. I have no strong feeling on any approach. The idea was to make it clear (and easily changeable) without the need to propagate *.pio files or the assembler. For example, there are some cases when you need to inject PIO instruction through the FIFOs, and this: pio->fifo = PIO_ASM_IRQ(0, 1, PIO_ASM_INDEX(true, 0), PIO_SS(0, 0, 0)) looks better than pio->fifo = 0xBEEF.

Adding support for interrupts

There are quite a lot of comments in this PR that explain why this cannot be avoided if maximum PIO flexibility is desired.

Renaming stuff (e.g. rpi pico pio to pio rpi pico)

Files were not renamed from the names you had in your original PR (only initially, renaming was reverted quite some time ago).

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[soburi]

Regarding the issue of IRQ dispatching, it seems necessary to see the progress of the discussion in #66505.
(I don't think it should be considered as a problem only to the PIO.)

As @yonsch shows, I think this PR can split into some parts. We can discuss start with 'light' issues.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.