From 9b91754ecde05b757e9a3a19d6d5ee35cd787f5c Mon Sep 17 00:00:00 2001
From: Daniel Garcia Briseno <dg.briseno@gmail.com>
Date: Wed, 24 Aug 2022 21:46:12 -0400
Subject: [PATCH 1/6] Add isr example with the pio

---
 pio/isr/CMakeLists.txt |  18 ++++
 pio/isr/main.c         | 193 +++++++++++++++++++++++++++++++++++++++++
 pio/isr/simply_isr.pio |  72 +++++++++++++++
 3 files changed, 283 insertions(+)
 create mode 100644 pio/isr/CMakeLists.txt
 create mode 100644 pio/isr/main.c
 create mode 100644 pio/isr/simply_isr.pio

diff --git a/pio/isr/CMakeLists.txt b/pio/isr/CMakeLists.txt
new file mode 100644
index 000000000..5f5839e23
--- /dev/null
+++ b/pio/isr/CMakeLists.txt
@@ -0,0 +1,18 @@
+add_executable(isr)
+
+pico_generate_pio_header(isr ${CMAKE_CURRENT_LIST_DIR}/simply_isr.pio)
+
+target_sources(isr PRIVATE main.c)
+
+pico_enable_stdio_usb(isr 1)
+pico_enable_stdio_uart(isr 0)
+
+target_link_libraries(isr PRIVATE
+        pico_stdlib
+        hardware_pio
+        )
+
+pico_add_extra_outputs(isr)
+
+# add url via pico_set_program_url
+example_auto_set_url(isr)
diff --git a/pio/isr/main.c b/pio/isr/main.c
new file mode 100644
index 000000000..d0521f233
--- /dev/null
+++ b/pio/isr/main.c
@@ -0,0 +1,193 @@
+/**
+ * Copyright 2022 (c) Daniel Garcia-Briseno
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are permitted provided that the
+ * following conditions are met:
+ * 
+ * 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following
+ *    disclaimer.
+ * 
+ * 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials provided with the distribution.
+ * 
+ * 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ * 
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include "pico/stdlib.h"
+
+#include "simply_isr.pio.h"
+// the sdk doesn't give us functions for figuring out which
+// isr source we're using, so we have to do it manually.
+#include "hardware/regs/pio.h"
+
+/**
+ * These defines represent each state machine.
+ * The value is the bit in the IRQ register that
+ * will be set by each state machine thanks to "irq wait 0 rel"
+ */
+#define PIO_SM_0_IRQ 0b0001
+#define PIO_SM_1_IRQ 0b0010
+#define PIO_SM_2_IRQ 0b0100
+#define PIO_SM_3_IRQ 0b1000
+
+/**
+ * This variable will shadow the IRQ flags set by the PIO state machines.
+ * Typically you do not want to do work in ISRs because the main thread
+ * has more important things to do. Because of that, when we get the ISR
+ * I'm simply going to copy the state machine that fired the ISR into
+ * this variable.
+ *
+ * Variable is volatile so that it doesn't get cached in a CPU register
+ * in the main thread. Without this it's possible that you never see
+ * irq_flags get set even though the ISR is firing all the time.
+ *
+ * Of course, you can really do whatever you want in the ISR, it's up to you.
+ */
+volatile uint32_t irq_flags = 0;
+
+/**
+ * Reads a hardware register
+ *
+ * @note See datasheet and/or pico-sdk/src/rp2040/include/hardware/regs
+ *       for base addresses and offsets
+ * @param[in] bank The base address of the register set usually <MODULE>_BASE
+ * @param[in] offset The register offset, usually <REG_NAME>_OFFSET
+ */
+uint32_t read_register(uint32_t bank, uint32_t offset) {
+    return *((volatile io_rw_32*)(bank + offset));
+}
+
+/**
+ * Writes a hardware register
+ *
+ * @note See datasheet and/or pico-sdk/src/rp2040/include/hardware/regs
+ *       for base addresses and offsets
+ * @param[in] bank The base address of the register set usually <MODULE>_BASE
+ * @param[in] offset The register offset, usually <REG_NAME>_OFFSET
+ * @param[in] value The value to write to the register
+ */
+void write_register(uint32_t bank, uint32_t offset, uint32_t value) {
+    *((volatile io_rw_32*)(bank + offset)) = value;
+}
+
+/**
+ * This function is called when the IRQ is fired by the state machine.
+ * @note See enable_pio_isrs for how to register this function to be called
+ */
+void simply_isr_handler() {
+    // Read the IRQ register to get the IRQ flags from the state machine
+    // This tells me which state machine sent the IRQ
+    irq_flags = read_register(PIO0_BASE, PIO_IRQ_OFFSET);
+    
+    // IRQ_OFFSET is write 1 to clear, so by writing back the
+    // value, we're acknowledging that we've serviced the interrupt.
+    write_register(PIO0_BASE, PIO_IRQ_OFFSET, irq_flags);
+}
+
+/**
+ * Lets the pico know that we want it to notify us of the PIO ISRs.
+ * @note in simply_isr.pio we enable irq0. This tells the state machine
+ *       to send the ISRs to the core, we still need to tell the core
+ *       to send them to our program.
+ */
+void enable_pio_isrs() {
+    // Set the function that will be called when the PIO IRQ comes in.
+    irq_set_exclusive_handler(PIO0_IRQ_0, simply_isr_handler);
+
+    // Once that function is set, we can go ahead and allow the interrupts
+    // to come in. You want to set the function before enabling the interrupt
+    // just in case. The docs say if an IRQ comes in and there's no handler
+    // then it will work like a breakpoint, which seems bad.
+    irq_set_enabled(PIO0_IRQ_0, true);
+}
+
+/**
+ * Loads simply_isr pio program into PIO memory
+ */
+void load_pio_programs() {
+    PIO pio = pio0;
+
+    // Load the program into PIO memory
+    uint offset = pio_add_program(pio, &simply_isr_program);
+
+    // Load the program to run in each state machine.
+    // They are allowed to run the same program in memory.
+    simply_isr_program_init(pio, 0, offset);
+    simply_isr_program_init(pio, 1, offset);
+    simply_isr_program_init(pio, 2, offset);
+    simply_isr_program_init(pio, 3, offset);
+}
+
+/**
+ * Writes to the tx fifo of the given state machine.
+ * This will make the simply_isr program send an ISR to us!
+ */
+void trigger_isr(int sm) {
+    printf("Triggering ISR from state machine %d\n", sm);
+    pio_sm_put_blocking(pio0, sm, 1);
+    // ISR will fire from the pio right here thanks to above function.
+
+    // Print the irq we expect based on the given state machine
+    printf("Expected IRQ flags: 0x%08X\n", (1 << sm));
+    printf("Actual IRQ Flags: 0x%08X\n", irq_flags);
+
+    // Here you could do work for the isr depending on which one it is.
+    // Something like
+    if (irq_flags & PIO_SM_0_IRQ) {
+        // handle_sm0_irq();
+    }
+    if (irq_flags & PIO_SM_1_IRQ) {
+        // handle_sm1_irq();
+    }
+    if (irq_flags & PIO_SM_2_IRQ) {
+        // handle_sm2_irq();
+    }
+    if (irq_flags & PIO_SM_3_IRQ) {
+        // handle_sm3_irq();
+    }
+    
+    // clear irq flags now.
+    irq_flags = 0;
+}
+
+int main() {
+    // Init stdio
+    stdio_init_all();
+
+    // Load simply_isr into memory
+    load_pio_programs();
+
+    // Enable IRQs to respond to simply_isr
+    enable_pio_isrs();
+
+    // simply_isr is programmed to fire an ISR when we write
+    // to their tx fifo. So let's do that now.
+    while (true) {
+        // Fire state machine 0
+        trigger_isr(0);
+        sleep_ms(1000);
+
+        // Fire state machine 1
+        trigger_isr(1);
+        sleep_ms(1000);
+
+        // Fire state machine 2
+        trigger_isr(2);
+        sleep_ms(1000);
+
+        // Fire state machine 3
+        trigger_isr(3);
+        sleep_ms(1000);
+    }
+}
+
diff --git a/pio/isr/simply_isr.pio b/pio/isr/simply_isr.pio
new file mode 100644
index 000000000..4fd25d725
--- /dev/null
+++ b/pio/isr/simply_isr.pio
@@ -0,0 +1,72 @@
+; Copyright (c) 2022 Daniel Garcia-Briseno
+;
+; Redistribution and use in source and binary forms, with or without modification, are permitted provided that the
+; following conditions are met:
+; 
+; 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following
+;    disclaimer.
+; 
+; 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following
+;    disclaimer in the documentation and/or other materials provided with the distribution.
+; 
+; 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products
+;    derived from this software without specific prior written permission.
+; 
+; THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+; INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+; DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+; SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+; SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+; WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+; THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+.program simply_isr
+
+; Stall on OSR. Wait for software to send a signal to continue
+; by writing to the OSR
+out x, 1
+
+; Do work here
+nop
+
+; Notify the software via irq that some work has been done
+; irq wait means to wait for software to acknowledge the irq before
+; continuing. rel means let the irq be relative to the state machine.
+; by using "0 rel" on all state machines, software will see a different
+; interrupt source for each state machine. Technically state machines
+; can set any irq source, so in order to know where the irq is coming from
+; it's best to set 0 rel
+irq wait 0 rel
+
+% c-sdk {
+    /**
+     * Initializer for the fake isr program program
+     * @param[in] pio the PIO instance to use
+     * @param[in] sm state machine to use for the PIO instance
+     * @param[in] offset Offset into PIO memory to place the program into
+     */
+    static inline void simply_isr_program_init(PIO pio, uint sm, uint offset) {
+        // Enable the IRQ source
+        // The reason for doing interrupt0 + sm:
+        // IRQ sources are enabled per irq flag. Since the irq flag being set depends on the state
+        // machine because of the "0 rel", we want to make sure we're enabling the correct interrupt
+        // source for the state machine the program is loaded into. 
+        pio_set_irq0_source_enabled(pio, (pis_interrupt0 + sm), true);
+        // Make sure the interrupt starts cleared. It should already be cleared, so this should
+        // basically be a no-op. I call it defensive programming.
+        pio_interrupt_clear(pio, sm);
+
+        // Build the configuration for the state machine
+        pio_sm_config config = simply_isr_program_get_default_config(offset);
+
+        // Set up autopull to pull the TX Fifo into the OSR
+        // This is what actually makes the "out" instruction wait
+        // for input from software.
+        // params are (config, shift_right (ignored here), autopull (true), pull threshold (1 bit))
+        sm_config_set_out_shift(&config, true, true, 1);
+
+        // Load the config and execute the state machine
+        pio_sm_init(pio, sm, offset, &config);
+        pio_sm_set_enabled(pio, sm, true);
+    }
+%}

From e65d58ff4bd04c6d0de547a473d7c9a8d9dc2359 Mon Sep 17 00:00:00 2001
From: Daniel Garcia Briseno <dg.briseno@gmail.com>
Date: Thu, 25 Aug 2022 15:47:45 -0400
Subject: [PATCH 2/6] Rename simply -> simpl. Change program name to pio_isr

---
 pio/isr/CMakeLists.txt                     | 16 ++++++++--------
 pio/isr/{simply_isr.pio => simple_isr.pio} |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)
 rename pio/isr/{simply_isr.pio => simple_isr.pio} (95%)

diff --git a/pio/isr/CMakeLists.txt b/pio/isr/CMakeLists.txt
index 5f5839e23..576652992 100644
--- a/pio/isr/CMakeLists.txt
+++ b/pio/isr/CMakeLists.txt
@@ -1,18 +1,18 @@
-add_executable(isr)
+add_executable(pio_isr)
 
-pico_generate_pio_header(isr ${CMAKE_CURRENT_LIST_DIR}/simply_isr.pio)
+pico_generate_pio_header(pio_isr ${CMAKE_CURRENT_LIST_DIR}/simple_isr.pio)
 
-target_sources(isr PRIVATE main.c)
+target_sources(pio_isr PRIVATE main.c)
 
-pico_enable_stdio_usb(isr 1)
-pico_enable_stdio_uart(isr 0)
+pico_enable_stdio_usb(pio_isr 1)
+pico_enable_stdio_uart(pio_isr 0)
 
-target_link_libraries(isr PRIVATE
+target_link_libraries(pio_isr PRIVATE
         pico_stdlib
         hardware_pio
         )
 
-pico_add_extra_outputs(isr)
+pico_add_extra_outputs(pio_isr)
 
 # add url via pico_set_program_url
-example_auto_set_url(isr)
+example_auto_set_url(pio_isr)
diff --git a/pio/isr/simply_isr.pio b/pio/isr/simple_isr.pio
similarity index 95%
rename from pio/isr/simply_isr.pio
rename to pio/isr/simple_isr.pio
index 4fd25d725..10f15a6ab 100644
--- a/pio/isr/simply_isr.pio
+++ b/pio/isr/simple_isr.pio
@@ -20,7 +20,7 @@
 ; WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 ; THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-.program simply_isr
+.program simple_isr
 
 ; Stall on OSR. Wait for software to send a signal to continue
 ; by writing to the OSR
@@ -45,7 +45,7 @@ irq wait 0 rel
      * @param[in] sm state machine to use for the PIO instance
      * @param[in] offset Offset into PIO memory to place the program into
      */
-    static inline void simply_isr_program_init(PIO pio, uint sm, uint offset) {
+    static inline void simple_isr_program_init(PIO pio, uint sm, uint offset) {
         // Enable the IRQ source
         // The reason for doing interrupt0 + sm:
         // IRQ sources are enabled per irq flag. Since the irq flag being set depends on the state
@@ -57,7 +57,7 @@ irq wait 0 rel
         pio_interrupt_clear(pio, sm);
 
         // Build the configuration for the state machine
-        pio_sm_config config = simply_isr_program_get_default_config(offset);
+        pio_sm_config config = simple_isr_program_get_default_config(offset);
 
         // Set up autopull to pull the TX Fifo into the OSR
         // This is what actually makes the "out" instruction wait

From 688f6d7473dd12f82453acdd084e942db266e4bb Mon Sep 17 00:00:00 2001
From: Daniel Garcia Briseno <dg.briseno@gmail.com>
Date: Thu, 25 Aug 2022 16:44:21 -0400
Subject: [PATCH 3/6] Update main.c to use pio_interrupt_get/clear

---
 pio/CMakeLists.txt |  1 +
 pio/isr/main.c     | 91 +++++++++++++++++++---------------------------
 2 files changed, 39 insertions(+), 53 deletions(-)

diff --git a/pio/CMakeLists.txt b/pio/CMakeLists.txt
index 0b9d0b3ac..e187e18f4 100644
--- a/pio/CMakeLists.txt
+++ b/pio/CMakeLists.txt
@@ -18,4 +18,5 @@ if (NOT PICO_NO_HARDWARE)
     add_subdirectory(uart_rx)
     add_subdirectory(uart_tx)
     add_subdirectory(ws2812)
+    add_subdirectory(isr)
 endif ()
diff --git a/pio/isr/main.c b/pio/isr/main.c
index d0521f233..7e0ed80a0 100644
--- a/pio/isr/main.c
+++ b/pio/isr/main.c
@@ -25,27 +25,24 @@
 #include <stdio.h>
 #include "pico/stdlib.h"
 
-#include "simply_isr.pio.h"
-// the sdk doesn't give us functions for figuring out which
-// isr source we're using, so we have to do it manually.
-#include "hardware/regs/pio.h"
+#include "simple_isr.pio.h"
 
 /**
  * These defines represent each state machine.
- * The value is the bit in the IRQ register that
- * will be set by each state machine thanks to "irq wait 0 rel"
+ * I'll use these to flag which state machine fired the ISR
  */
 #define PIO_SM_0_IRQ 0b0001
 #define PIO_SM_1_IRQ 0b0010
 #define PIO_SM_2_IRQ 0b0100
 #define PIO_SM_3_IRQ 0b1000
 
+
 /**
  * This variable will shadow the IRQ flags set by the PIO state machines.
  * Typically you do not want to do work in ISRs because the main thread
  * has more important things to do. Because of that, when we get the ISR
- * I'm simply going to copy the state machine that fired the ISR into
- * this variable.
+ * I'm going to copy the state machine that fired the ISR into
+ * this variable using the above defines.
  *
  * Variable is volatile so that it doesn't get cached in a CPU register
  * in the main thread. Without this it's possible that you never see
@@ -55,54 +52,42 @@
  */
 volatile uint32_t irq_flags = 0;
 
-/**
- * Reads a hardware register
- *
- * @note See datasheet and/or pico-sdk/src/rp2040/include/hardware/regs
- *       for base addresses and offsets
- * @param[in] bank The base address of the register set usually <MODULE>_BASE
- * @param[in] offset The register offset, usually <REG_NAME>_OFFSET
- */
-uint32_t read_register(uint32_t bank, uint32_t offset) {
-    return *((volatile io_rw_32*)(bank + offset));
-}
-
-/**
- * Writes a hardware register
- *
- * @note See datasheet and/or pico-sdk/src/rp2040/include/hardware/regs
- *       for base addresses and offsets
- * @param[in] bank The base address of the register set usually <MODULE>_BASE
- * @param[in] offset The register offset, usually <REG_NAME>_OFFSET
- * @param[in] value The value to write to the register
- */
-void write_register(uint32_t bank, uint32_t offset, uint32_t value) {
-    *((volatile io_rw_32*)(bank + offset)) = value;
-}
-
 /**
  * This function is called when the IRQ is fired by the state machine.
  * @note See enable_pio_isrs for how to register this function to be called
  */
-void simply_isr_handler() {
-    // Read the IRQ register to get the IRQ flags from the state machine
-    // This tells me which state machine sent the IRQ
-    irq_flags = read_register(PIO0_BASE, PIO_IRQ_OFFSET);
-    
-    // IRQ_OFFSET is write 1 to clear, so by writing back the
-    // value, we're acknowledging that we've serviced the interrupt.
-    write_register(PIO0_BASE, PIO_IRQ_OFFSET, irq_flags);
+void simple_isr_handler() {
+    // Check the interrupt source and set the flag accordingly
+    // Using if for each one means that if multiple interrupts came in, this
+    // ISR would flag each one. That means the ISR would execute once instead
+    // of being executed for each interrupt individually.
+    // Another alternative intead of using pio_interrupt_get would be to
+    // read the PIO0_IRQ register directly to get these flags
+    for (uint i = 0; i < 4; i++) {
+        // Check if the interrupt is set, flags 0-3 are used for system interrupts
+        // These are the ones that will be set by the isr instruction in pio
+        bool isr_set = pio_interrupt_get(pio0, i);
+
+        if (isr_set) {
+            // Set a bit flagging which state machine set the interrupt
+            irq_flags |= (1 << i);
+
+            // Clear/Acknowledge the ISR. If you use "isr wait" this is what
+            // tells the sm to continue.
+            pio_interrupt_clear(pio0, i);
+        }
+    }
 }
 
 /**
  * Lets the pico know that we want it to notify us of the PIO ISRs.
- * @note in simply_isr.pio we enable irq0. This tells the state machine
+ * @note in simple_isr.pio we enable irq0. This tells the state machine
  *       to send the ISRs to the core, we still need to tell the core
  *       to send them to our program.
  */
 void enable_pio_isrs() {
     // Set the function that will be called when the PIO IRQ comes in.
-    irq_set_exclusive_handler(PIO0_IRQ_0, simply_isr_handler);
+    irq_set_exclusive_handler(PIO0_IRQ_0, simple_isr_handler);
 
     // Once that function is set, we can go ahead and allow the interrupts
     // to come in. You want to set the function before enabling the interrupt
@@ -112,25 +97,25 @@ void enable_pio_isrs() {
 }
 
 /**
- * Loads simply_isr pio program into PIO memory
+ * Loads simple_isr pio program into PIO memory
  */
 void load_pio_programs() {
     PIO pio = pio0;
 
     // Load the program into PIO memory
-    uint offset = pio_add_program(pio, &simply_isr_program);
+    uint offset = pio_add_program(pio, &simple_isr_program);
 
     // Load the program to run in each state machine.
     // They are allowed to run the same program in memory.
-    simply_isr_program_init(pio, 0, offset);
-    simply_isr_program_init(pio, 1, offset);
-    simply_isr_program_init(pio, 2, offset);
-    simply_isr_program_init(pio, 3, offset);
+    simple_isr_program_init(pio, 0, offset);
+    simple_isr_program_init(pio, 1, offset);
+    simple_isr_program_init(pio, 2, offset);
+    simple_isr_program_init(pio, 3, offset);
 }
 
 /**
  * Writes to the tx fifo of the given state machine.
- * This will make the simply_isr program send an ISR to us!
+ * This will make the simple_isr program send an ISR to us!
  */
 void trigger_isr(int sm) {
     printf("Triggering ISR from state machine %d\n", sm);
@@ -164,13 +149,13 @@ int main() {
     // Init stdio
     stdio_init_all();
 
-    // Load simply_isr into memory
+    // Load simple_isr into memory
     load_pio_programs();
 
-    // Enable IRQs to respond to simply_isr
+    // Enable IRQs to respond to simple_isr
     enable_pio_isrs();
 
-    // simply_isr is programmed to fire an ISR when we write
+    // simple_isr is programmed to fire an ISR when we write
     // to their tx fifo. So let's do that now.
     while (true) {
         // Fire state machine 0

From 65e984f7eaffe994aba78b7170a006c2b7439aee Mon Sep 17 00:00:00 2001
From: Daniel Garcia Briseno <dg.briseno@gmail.com>
Date: Thu, 25 Aug 2022 20:03:02 -0400
Subject: [PATCH 4/6] Update readme

---
 README.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/README.md b/README.md
index c74a4c266..82431d42e 100644
--- a/README.md
+++ b/README.md
@@ -156,6 +156,7 @@ App|Description
 [uart_tx](pio/uart_tx)| Implement the transmit component of a UART serial port, and print hello world.
 [ws2812](pio/ws2812)| Examples of driving WS2812 addressable RGB LEDs.
 [addition](pio/addition)| Add two integers together using PIO. Only around 8 billion times slower than Cortex-M0+.
+[pio_isr](pio/isr)| Respond to interrupts generated by a PIO state machine.
 
 ### PWM
 

From 0ff4e62a95d6235bb810b46c2a28623283084a42 Mon Sep 17 00:00:00 2001
From: Daniel Garcia Briseno <dg.briseno@gmail.com>
Date: Fri, 26 Aug 2022 07:19:55 -0400
Subject: [PATCH 5/6] rename isr->irq. Use direct register access.

---
 pio/CMakeLists.txt                            |  2 +-
 pio/irq/CMakeLists.txt                        | 18 ++++
 pio/{isr => irq}/main.c                       | 90 +++++++++----------
 .../simple_isr.pio => irq/simple_irq.pio}     |  8 +-
 pio/isr/CMakeLists.txt                        | 18 ----
 5 files changed, 66 insertions(+), 70 deletions(-)
 create mode 100644 pio/irq/CMakeLists.txt
 rename pio/{isr => irq}/main.c (67%)
 rename pio/{isr/simple_isr.pio => irq/simple_irq.pio} (94%)
 delete mode 100644 pio/isr/CMakeLists.txt

diff --git a/pio/CMakeLists.txt b/pio/CMakeLists.txt
index e187e18f4..8684fc519 100644
--- a/pio/CMakeLists.txt
+++ b/pio/CMakeLists.txt
@@ -18,5 +18,5 @@ if (NOT PICO_NO_HARDWARE)
     add_subdirectory(uart_rx)
     add_subdirectory(uart_tx)
     add_subdirectory(ws2812)
-    add_subdirectory(isr)
+    add_subdirectory(irq)
 endif ()
diff --git a/pio/irq/CMakeLists.txt b/pio/irq/CMakeLists.txt
new file mode 100644
index 000000000..c16185ee1
--- /dev/null
+++ b/pio/irq/CMakeLists.txt
@@ -0,0 +1,18 @@
+add_executable(pio_irq)
+
+pico_generate_pio_header(pio_irq ${CMAKE_CURRENT_LIST_DIR}/simple_irq.pio)
+
+target_sources(pio_irq PRIVATE main.c)
+
+pico_enable_stdio_usb(pio_irq 1)
+pico_enable_stdio_uart(pio_irq 0)
+
+target_link_libraries(pio_irq PRIVATE
+        pico_stdlib
+        hardware_pio
+        )
+
+pico_add_extra_outputs(pio_irq)
+
+# add url via pico_set_program_url
+example_auto_set_url(pio_irq)
diff --git a/pio/isr/main.c b/pio/irq/main.c
similarity index 67%
rename from pio/isr/main.c
rename to pio/irq/main.c
index 7e0ed80a0..341c6862a 100644
--- a/pio/isr/main.c
+++ b/pio/irq/main.c
@@ -25,7 +25,7 @@
 #include <stdio.h>
 #include "pico/stdlib.h"
 
-#include "simple_isr.pio.h"
+#include "simple_irq.pio.h"
 
 /**
  * These defines represent each state machine.
@@ -41,8 +41,8 @@
  * This variable will shadow the IRQ flags set by the PIO state machines.
  * Typically you do not want to do work in ISRs because the main thread
  * has more important things to do. Because of that, when we get the ISR
- * I'm going to copy the state machine that fired the ISR into
- * this variable using the above defines.
+ * I'm going to copy the state machine that fired the ISR into this
+ * variable.
  *
  * Variable is volatile so that it doesn't get cached in a CPU register
  * in the main thread. Without this it's possible that you never see
@@ -50,44 +50,36 @@
  *
  * Of course, you can really do whatever you want in the ISR, it's up to you.
  */
-volatile uint32_t irq_flags = 0;
+volatile uint8_t irq_flags = 0;
 
 /**
  * This function is called when the IRQ is fired by the state machine.
- * @note See enable_pio_isrs for how to register this function to be called
+ * @note See enable_pio_irqs for how to register this function to be called
  */
-void simple_isr_handler() {
+void simple_irq_handler() {
     // Check the interrupt source and set the flag accordingly
-    // Using if for each one means that if multiple interrupts came in, this
-    // ISR would flag each one. That means the ISR would execute once instead
+    // This method can handle multiple simultaneous interrupts from the PIO
+    // since it checks all the irq flags. The advantage to this is that the
+    // ISR would execute only once when simultaneous interrupts happen instead
     // of being executed for each interrupt individually.
-    // Another alternative intead of using pio_interrupt_get would be to
-    // read the PIO0_IRQ register directly to get these flags
-    for (uint i = 0; i < 4; i++) {
-        // Check if the interrupt is set, flags 0-3 are used for system interrupts
-        // These are the ones that will be set by the isr instruction in pio
-        bool isr_set = pio_interrupt_get(pio0, i);
-
-        if (isr_set) {
-            // Set a bit flagging which state machine set the interrupt
-            irq_flags |= (1 << i);
-
-            // Clear/Acknowledge the ISR. If you use "isr wait" this is what
-            // tells the sm to continue.
-            pio_interrupt_clear(pio0, i);
-        }
-    }
+    // For other applications, make sure to check the correct pio (pio0/pio1)
+    irq_flags = pio0_hw->irq;
+    // Clear the flags since we've saved them to check later.
+    hw_clear_bits(&pio0_hw->irq, irq_flags);
+
+    // alternatively you could use pio_interrupt_get(pio0, #) to check a specific
+    // irq flag.
 }
 
 /**
  * Lets the pico know that we want it to notify us of the PIO ISRs.
- * @note in simple_isr.pio we enable irq0. This tells the state machine
+ * @note in simple_irq.pio we enable irq0. This tells the state machine
  *       to send the ISRs to the core, we still need to tell the core
  *       to send them to our program.
  */
-void enable_pio_isrs() {
+void enable_pio_irqs() {
     // Set the function that will be called when the PIO IRQ comes in.
-    irq_set_exclusive_handler(PIO0_IRQ_0, simple_isr_handler);
+    irq_set_exclusive_handler(PIO0_IRQ_0, simple_irq_handler);
 
     // Once that function is set, we can go ahead and allow the interrupts
     // to come in. You want to set the function before enabling the interrupt
@@ -97,37 +89,41 @@ void enable_pio_isrs() {
 }
 
 /**
- * Loads simple_isr pio program into PIO memory
+ * Loads simple_irq pio program into PIO memory
  */
 void load_pio_programs() {
     PIO pio = pio0;
 
     // Load the program into PIO memory
-    uint offset = pio_add_program(pio, &simple_isr_program);
+    uint offset = pio_add_program(pio, &simple_irq_program);
 
-    // Load the program to run in each state machine.
+    // Tell each state machine to run the program.
     // They are allowed to run the same program in memory.
-    simple_isr_program_init(pio, 0, offset);
-    simple_isr_program_init(pio, 1, offset);
-    simple_isr_program_init(pio, 2, offset);
-    simple_isr_program_init(pio, 3, offset);
+    simple_irq_program_init(pio, 0, offset);
+    simple_irq_program_init(pio, 1, offset);
+    simple_irq_program_init(pio, 2, offset);
+    simple_irq_program_init(pio, 3, offset);
 }
 
 /**
  * Writes to the tx fifo of the given state machine.
- * This will make the simple_isr program send an ISR to us!
+ * This will make the simple_irq program send an ISR to us!
  */
-void trigger_isr(int sm) {
+void trigger_irq(int sm) {
     printf("Triggering ISR from state machine %d\n", sm);
     pio_sm_put_blocking(pio0, sm, 1);
-    // ISR will fire from the pio right here thanks to above function.
+    // Interrupt will fire from the pio right here since they trigger
+    // whenever data goes into the TX FIFO, see simple_irq.pio
 
     // Print the irq we expect based on the given state machine
     printf("Expected IRQ flags: 0x%08X\n", (1 << sm));
     printf("Actual IRQ Flags: 0x%08X\n", irq_flags);
 
-    // Here you could do work for the isr depending on which one it is.
-    // Something like
+    // Here you could dispatch work for the irq depending one
+    // flagged it. The work should be done here rather than inside
+    // the irq function. While executing code in an irq, no other
+    // irq's can execute. By having the processing code outside of
+    // the irq, we can still be responsive to new irqs.
     if (irq_flags & PIO_SM_0_IRQ) {
         // handle_sm0_irq();
     }
@@ -149,29 +145,29 @@ int main() {
     // Init stdio
     stdio_init_all();
 
-    // Load simple_isr into memory
+    // Load simple_irq into memory
     load_pio_programs();
 
-    // Enable IRQs to respond to simple_isr
-    enable_pio_isrs();
+    // Enable IRQs to respond to simple_irq
+    enable_pio_irqs();
 
-    // simple_isr is programmed to fire an ISR when we write
+    // simple_irq is programmed to fire an ISR when we write
     // to their tx fifo. So let's do that now.
     while (true) {
         // Fire state machine 0
-        trigger_isr(0);
+        trigger_irq(0);
         sleep_ms(1000);
 
         // Fire state machine 1
-        trigger_isr(1);
+        trigger_irq(1);
         sleep_ms(1000);
 
         // Fire state machine 2
-        trigger_isr(2);
+        trigger_irq(2);
         sleep_ms(1000);
 
         // Fire state machine 3
-        trigger_isr(3);
+        trigger_irq(3);
         sleep_ms(1000);
     }
 }
diff --git a/pio/isr/simple_isr.pio b/pio/irq/simple_irq.pio
similarity index 94%
rename from pio/isr/simple_isr.pio
rename to pio/irq/simple_irq.pio
index 10f15a6ab..934ef2eb6 100644
--- a/pio/isr/simple_isr.pio
+++ b/pio/irq/simple_irq.pio
@@ -20,7 +20,7 @@
 ; WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 ; THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-.program simple_isr
+.program simple_irq
 
 ; Stall on OSR. Wait for software to send a signal to continue
 ; by writing to the OSR
@@ -40,12 +40,12 @@ irq wait 0 rel
 
 % c-sdk {
     /**
-     * Initializer for the fake isr program program
+     * Initializer for the fake irq program program
      * @param[in] pio the PIO instance to use
      * @param[in] sm state machine to use for the PIO instance
      * @param[in] offset Offset into PIO memory to place the program into
      */
-    static inline void simple_isr_program_init(PIO pio, uint sm, uint offset) {
+    static inline void simple_irq_program_init(PIO pio, uint sm, uint offset) {
         // Enable the IRQ source
         // The reason for doing interrupt0 + sm:
         // IRQ sources are enabled per irq flag. Since the irq flag being set depends on the state
@@ -57,7 +57,7 @@ irq wait 0 rel
         pio_interrupt_clear(pio, sm);
 
         // Build the configuration for the state machine
-        pio_sm_config config = simple_isr_program_get_default_config(offset);
+        pio_sm_config config = simple_irq_program_get_default_config(offset);
 
         // Set up autopull to pull the TX Fifo into the OSR
         // This is what actually makes the "out" instruction wait
diff --git a/pio/isr/CMakeLists.txt b/pio/isr/CMakeLists.txt
deleted file mode 100644
index 576652992..000000000
--- a/pio/isr/CMakeLists.txt
+++ /dev/null
@@ -1,18 +0,0 @@
-add_executable(pio_isr)
-
-pico_generate_pio_header(pio_isr ${CMAKE_CURRENT_LIST_DIR}/simple_isr.pio)
-
-target_sources(pio_isr PRIVATE main.c)
-
-pico_enable_stdio_usb(pio_isr 1)
-pico_enable_stdio_uart(pio_isr 0)
-
-target_link_libraries(pio_isr PRIVATE
-        pico_stdlib
-        hardware_pio
-        )
-
-pico_add_extra_outputs(pio_isr)
-
-# add url via pico_set_program_url
-example_auto_set_url(pio_isr)

From 5ca07585b8db11f03d942f951b05cb9ec78da495 Mon Sep 17 00:00:00 2001
From: Daniel Garcia Briseno <dg.briseno@gmail.com>
Date: Fri, 26 Aug 2022 07:38:49 -0400
Subject: [PATCH 6/6] Update readme with isr -> irq

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 82431d42e..6916d99d6 100644
--- a/README.md
+++ b/README.md
@@ -156,7 +156,7 @@ App|Description
 [uart_tx](pio/uart_tx)| Implement the transmit component of a UART serial port, and print hello world.
 [ws2812](pio/ws2812)| Examples of driving WS2812 addressable RGB LEDs.
 [addition](pio/addition)| Add two integers together using PIO. Only around 8 billion times slower than Cortex-M0+.
-[pio_isr](pio/isr)| Respond to interrupts generated by a PIO state machine.
+[irq](pio/irq)| Respond to interrupts generated by a PIO state machine.
 
 ### PWM