fix PIO gpio compatibility checks, docs and simplify/optimize implementation (#2989)
Co-authored-by: Andrew Scheller <lurch@durge.org>
Co-authored-by: will-v-pi <108662275+will-v-pi@users.noreply.github.com>
diff --git a/src/common/pico_base_headers/include/pico/assert.h b/src/common/pico_base_headers/include/pico/assert.h
index cc33779..a9af542 100644
--- a/src/common/pico_base_headers/include/pico/assert.h
+++ b/src/common/pico_base_headers/include/pico/assert.h
@@ -34,7 +34,7 @@
#define invalid_params_if(x, test) ({if (PARAM_ASSERTIONS_ENABLED(x)) assert(!(test));})
#define valid_params_if(x, test) ({if (PARAM_ASSERTIONS_ENABLED(x)) assert(test);})
#define hard_assert_if(x, test) ({if (PARAM_ASSERTIONS_ENABLED(x)) hard_assert(!(test));})
-#define invalid_params_if_and_return(x, test, rc) ({/*if (PARAM_ASSERTIONS_ENABLED(x)) assert(!(test)); */ if (test) return rc; })
+#define invalid_params_if_and_return(x, test, rc) ({if (PARAM_ASSERTIONS_ENABLED(x)) assert(!(test)); if (test) return rc; })
#ifdef NDEBUG
extern void hard_assertion_failure(void);
diff --git a/src/rp2_common/hardware_pio/include/hardware/pio.h b/src/rp2_common/hardware_pio/include/hardware/pio.h
index 9d3d6ba..a67a014 100644
--- a/src/rp2_common/hardware_pio/include/hardware/pio.h
+++ b/src/rp2_common/hardware_pio/include/hardware/pio.h
@@ -837,8 +837,8 @@
// configs that use pins 32-47
uint32_t gpio_over_32 = (config->pinhi >> 1) & used; // checks if bit 1 of any field is 1
uint gpio_base = pio_get_gpio_base(pio);
- invalid_params_if_and_return(PIO, gpio_under_16 && gpio_base, PICO_ERROR_BAD_ALIGNMENT);
- invalid_params_if_and_return(PIO, gpio_over_32 && !gpio_base, PICO_ERROR_BAD_ALIGNMENT);
+ invalid_params_if_and_return(HARDWARE_PIO, gpio_under_16 && gpio_base, PICO_ERROR_BAD_ALIGNMENT);
+ invalid_params_if_and_return(HARDWARE_PIO, gpio_over_32 && !gpio_base, PICO_ERROR_BAD_ALIGNMENT);
// flip the top bit of any used (execctrl/pinctrl) values to turn:
// bit6(32) + 0-15 -> base(16) + 16-31
// bit6(0) + 16-31 -> base(16) + 0-15
@@ -2027,13 +2027,13 @@
* \ingroup hardware_pio
*
* \param program PIO program to add
- * \param pio Returns the PIO hardware instance or NULL if no PIO is available
- * \param sm Returns the index of the PIO state machine that was claimed
- * \param offset Returns the instruction memory offset of the start of the program
+ * \param pio_out Returns the PIO hardware instance or NULL if no PIO is available
+ * \param sm_out Returns the index of the PIO state machine that was claimed
+ * \param offset_out Returns the instruction memory offset of the start of the program
* \return true on success, false otherwise
* \see pio_remove_program_and_unclaim_sm
*/
-bool pio_claim_free_sm_and_add_program(const pio_program_t *program, PIO *pio, uint *sm, uint *offset);
+bool pio_claim_free_sm_and_add_program(const pio_program_t *program, PIO *pio_out, uint *sm_out, uint *offset_out);
/*! \brief Finds a PIO and statemachine and adds a program into PIO memory
* \ingroup hardware_pio
@@ -2046,18 +2046,22 @@
* and optionally will set the GPIO base (see \ref pio_set_gpio_base) of an unused PIO instance if necessary
*
* \param program PIO program to add
- * \param pio Returns the PIO hardware instance or NULL if no PIO is available
- * \param sm Returns the index of the PIO state machine that was claimed
- * \param offset Returns the instruction memory offset of the start of the program
+ * \param pio_out Returns the PIO hardware instance or NULL if no PIO is available
+ * \param sm_out Returns the index of the PIO state machine that was claimed
+ * \param offset_out Returns the instruction memory offset of the start of the program
* \param gpio_start the lowest GPIO number required (0-47 on RP2350B, 0-31 otherwise)
* \param gpio_count the count of consecutive GPIOs required
* \param set_gpio_base if there is no free SM on a PIO instance with the right GPIO base, and there IS an unused PIO
- * instance, then that PIO will be reconfigured so that this method can succeed
+ * instance, then that PIO will be reconfigured so that this method can succeed. Note
+ * this parameter is ignored when PICO_PIO_USE_GPIO_BASE=0; i.e. by default on anything other than RP2350B
*
* \return true on success, false otherwise
* \see pio_remove_program_and_unclaim_sm
+ *
+ * \note on RP2040 or RP2350A (strictly when PICO_PIO_USE_GPIO_BASE == 0), gpio_start + gpio_count must be <= 32
+ * for success, and the set_gpio_base parameter is ignored
*/
-bool pio_claim_free_sm_and_add_program_for_gpio_range(const pio_program_t *program, PIO *pio, uint *sm, uint *offset, uint gpio_start, uint gpio_count, bool set_gpio_base);
+bool pio_claim_free_sm_and_add_program_for_gpio_range(const pio_program_t *program, PIO *pio_out, uint *sm_out, uint *offset_out, uint gpio_start, uint gpio_count, bool set_gpio_base);
/*! \brief Removes a program from PIO memory and unclaims the state machine
* \ingroup hardware_pio
diff --git a/src/rp2_common/hardware_pio/pio.c b/src/rp2_common/hardware_pio/pio.c
index 95d607d..976d7e5 100644
--- a/src/rp2_common/hardware_pio/pio.c
+++ b/src/rp2_common/hardware_pio/pio.c
@@ -51,7 +51,7 @@
uint base = which * NUM_PIO_STATE_MACHINES;
int index = hw_claim_unused_from_range((uint8_t*)&claimed[0], required, base,
base + NUM_PIO_STATE_MACHINES - 1, "No PIO state machines are available");
- return index >= (int)base ? index - (int)base : -1;
+ return index >= (int)base ? index - (int)base : PICO_ERROR_GENERIC;
}
bool pio_sm_is_claimed(PIO pio, uint sm) {
@@ -81,18 +81,15 @@
}
}
+#if PICO_PIO_USE_GPIO_BASE
static int pio_set_gpio_base_unsafe(PIO pio, uint gpio_base) {
invalid_params_if_and_return(HARDWARE_PIO, gpio_base != 0 && (!PICO_PIO_VERSION || gpio_base != 16), PICO_ERROR_BAD_ALIGNMENT);
-#if PICO_PIO_USE_GPIO_BASE
uint32_t used_mask = _used_instruction_space[pio_get_index(pio)];
invalid_params_if_and_return(HARDWARE_PIO, used_mask, PICO_ERROR_INVALID_STATE);
pio->gpiobase = gpio_base;
-#else
- ((void)pio);
- ((void)gpio_base);
-#endif
return PICO_OK;
}
+#endif
int pio_set_gpio_base(PIO pio, uint gpio_base) {
int rc = PICO_OK;
@@ -107,22 +104,19 @@
return rc;
}
+#if PICO_PIO_VERSION > 0 || PICO_PIO_USE_GPIO_BASE
static bool is_gpio_compatible(PIO pio, uint32_t used_gpio_ranges) {
-#if PICO_PIO_USE_GPIO_BASE
bool gpio_base = pio_get_gpio_base(pio);
return !((gpio_base && (used_gpio_ranges & 1)) ||
(!gpio_base && (used_gpio_ranges & 4)));
-#else
- ((void)pio);
- ((void)used_gpio_ranges);
- return true;
-#endif
}
+#endif
static bool is_program_gpio_compatible(PIO pio, const pio_program_t *program) {
#if PICO_PIO_VERSION > 0
return is_gpio_compatible(pio, program->used_gpio_ranges);
#else
+ // there are no stored gpio_ranges, so we assume we're good
((void)pio);
((void)program);
return true;
@@ -178,7 +172,6 @@
return (int)offset;
}
-// these assert if unable
int pio_add_program(PIO pio, const pio_program_t *program) {
uint32_t save = hw_claim_lock();
int offset = find_offset_for_program(pio, program);
@@ -411,36 +404,48 @@
}
}
-bool pio_claim_free_sm_and_add_program(const pio_program_t *program, PIO *pio, uint *sm, uint *offset) {
- return pio_claim_free_sm_and_add_program_for_gpio_range(program, pio, sm, offset, 0, 0, false);
+bool pio_claim_free_sm_and_add_program(const pio_program_t *program, PIO *pio_out, uint *sm_out, uint *offset_out) {
+ int pio_num = NUM_PIOS;
+ while (pio_num--) {
+ PIO pio = pio_get_instance((uint)pio_num);
+ int sm_or_error = pio_claim_unused_sm(pio, false);
+ if (sm_or_error >= 0) {
+ int offset_or_error = pio_add_program(pio, program);
+ if (offset_or_error >= 0) {
+ *pio_out = pio;
+ *sm_out = sm_or_error;
+ *offset_out = offset_or_error;
+ return true;
+ }
+ pio_sm_unclaim(pio, sm_or_error);
+ }
+ }
+ return false;
}
-bool pio_claim_free_sm_and_add_program_for_gpio_range(const pio_program_t *program, PIO *pio, uint *sm, uint *offset, uint gpio_start, uint gpio_count, bool set_gpio_base) {
- invalid_params_if(HARDWARE_PIO, (gpio_start + gpio_count) > NUM_BANK0_GPIOS);
-
+bool pio_claim_free_sm_and_add_program_for_gpio_range(const pio_program_t *program, PIO *pio_out, uint *sm_out, uint *offset_out, uint gpio_start, uint gpio_count, bool set_gpio_base) {
+ invalid_params_if_and_return(HARDWARE_PIO, (gpio_start + gpio_count) > MAX(32, NUM_BANK0_GPIOS), false);
#if !PICO_PIO_USE_GPIO_BASE
- // short-circuit some logic when not using GPIO_BASE
- set_gpio_base = 0;
- gpio_count = 0;
-#endif
+ invalid_params_if(HARDWARE_PIO, (gpio_start + gpio_count) > 32);
+ (void)set_gpio_base;
+ return pio_claim_free_sm_and_add_program(program, pio_out, sm_out, offset_out);
+#else
+ invalid_params_if_and_return(HARDWARE_PIO, !gpio_count, false); // 0 gpio_count breaks logic below, so return false
- // note if gpio_count == 0, we don't care about GPIOs so use a zero mask for what we require
- // if gpio_count > 0, then we just set used mask for the ends, since that is all that is checked at the moment
- uint32_t required_gpio_ranges;
- if (gpio_count) required_gpio_ranges = (1u << (gpio_start >> 4)) | (1u << ((gpio_start + gpio_count - 1) >> 4));
- else required_gpio_ranges = 0;
+ // we just set used mask for the ends, since that is all that is checked at the moment
+ uint32_t required_gpio_ranges = (1u << (gpio_start >> 4)) | (1u << ((gpio_start + gpio_count - 1) >> 4));
int passes = set_gpio_base ? 2 : 1;
for(int pass = 0; pass < passes; pass++) {
int pio_num = NUM_PIOS;
while (pio_num--) {
- *pio = pio_get_instance((uint)pio_num);
+ PIO pio = pio_get_instance((uint)pio_num);
// We need to claim an SM on the PIO
int8_t sm_index[NUM_PIO_STATE_MACHINES];
// on second pass, if there is one, we try and claim all the state machines so that we can change the GPIO base
int num_claimed;
for(num_claimed = 0; num_claimed < (pass ? (int)NUM_PIO_STATE_MACHINES : 1) ; num_claimed++) {
- sm_index[num_claimed] = (int8_t)pio_claim_unused_sm(*pio, false);
+ sm_index[num_claimed] = (int8_t)pio_claim_unused_sm(pio, false);
if (sm_index[num_claimed] < 0) break;
}
// rc = 0 if we claimed all the required state machines for the pass, <0 otherwise
@@ -448,29 +453,30 @@
if (rc >= 0) {
uint32_t save = hw_claim_lock();
if (pass) {
- pio_set_gpio_base_unsafe(*pio, required_gpio_ranges & 4 ? 16 : 0);
+ pio_set_gpio_base_unsafe(pio, required_gpio_ranges & 4 ? 16 : 0);
}
- rc = is_gpio_compatible(*pio, required_gpio_ranges) ? 0 : -1;
- if (rc >= 0) rc = find_offset_for_program(*pio, program);
- if (rc >= 0) rc = add_program_at_offset(*pio, program, (uint)rc);
+ rc = is_gpio_compatible(pio, required_gpio_ranges) ? 0 : -1;
+ if (rc >= 0) rc = find_offset_for_program(pio, program);
+ if (rc >= 0) rc = add_program_at_offset(pio, program, (uint)rc);
if (rc >= 0) {
- *sm = (uint) sm_index[0];
- *offset = (uint) rc;
+ *pio_out = pio;
+ *sm_out = (uint) sm_index[0];
+ *offset_out = (uint) rc;
}
hw_claim_unlock(save);
}
// always un-claim all SMs other than the one we need (array index 0),
// or all of them if we had an error
for (int i = (rc >= 0); i < num_claimed; i++) {
- pio_sm_unclaim(*pio, (uint) sm_index[i]);
+ pio_sm_unclaim(pio, (uint) sm_index[i]);
}
if (rc >= 0) {
return true;
}
}
}
- *pio = NULL;
return false;
+#endif
}
void pio_remove_program_and_unclaim_sm(const pio_program_t *program, PIO pio, uint sm, uint offset) {
