diff options
Diffstat (limited to 'libhw_cr/rp2040_hwspi.c')
| -rw-r--r-- | libhw_cr/rp2040_hwspi.c | 47 |
1 files changed, 37 insertions, 10 deletions
diff --git a/libhw_cr/rp2040_hwspi.c b/libhw_cr/rp2040_hwspi.c index d4adb11..646d8ba 100644 --- a/libhw_cr/rp2040_hwspi.c +++ b/libhw_cr/rp2040_hwspi.c @@ -30,8 +30,8 @@ #error config.h must define CONFIG_RP2040_SPI_DEBUG (bool) #endif -LO_IMPLEMENTATION_C(io_duplex_readwriter, struct rp2040_hwspi, rp2040_hwspi, static) -LO_IMPLEMENTATION_C(spi, struct rp2040_hwspi, rp2040_hwspi, static) +LO_IMPLEMENTATION_C(io_duplex_readwriter, struct rp2040_hwspi, rp2040_hwspi, static); +LO_IMPLEMENTATION_C(spi, struct rp2040_hwspi, rp2040_hwspi, static); static void rp2040_hwspi_intrhandler(void *_self, enum dmairq LM_UNUSED(irq), uint LM_UNUSED(channel)) { struct rp2040_hwspi *self = _self; @@ -133,6 +133,8 @@ void _rp2040_hwspi_init(struct rp2040_hwspi *self, self->sema = (cr_sema_t){0}; /* Initialize the interrupt handler. */ + /* We do this on (just) the rx channel, because the way the + * SSP works reads necessarily complete *after* writes. */ dmairq_set_and_enable_exclusive_handler(DMAIRQ_0, self->dma_rx_data, rp2040_hwspi_intrhandler, self); } @@ -163,23 +165,43 @@ static void rp2040_hwspi_readwritev(struct rp2040_hwspi *self, const struct dupl if (!pruned_iovcnt) return; - /* For tx_data_blocks, it doesn't really matter which aliases - * we choose: + /* It doesn't *really* matter which aliases we choose: + * * - None of our fields can be NULL (so no * false-termination). + * * - Moving const fields first so they don't have to be - * re-programmed each time isn't possible for us there need - * to be at least 2 const fields, and we only have 1 + * re-programmed each time isn't possible for us; there + * need to be at least 2 const fields, and we only have 1 * (read_addr for rx_data_blocks, and write_addr for * tx_data_blocks). * - * But for rx_data_blocks, we need ctrl to be the trigger - * register so that the DMA_CTRL_IRQ_QUIET flag isn't cleared - * before we get to the trigger; and while for tx_data_blocks - * it doesn't really matter, the inverse would be nice. + * The code following this initial declaration is generic to + * the alias, so changing which alias is used is easy. + * + * Since we have no hard requirements, here are some mild + * preferences: + * + * - I like the aliases being different for each channel, + * because it helps prevent alias-specific code from + * sneaking in. + * + * - I like the rx channel (the channel the interrupt handler + * is wired to) having ctrl be the trigger, so that we + * don't have to worry about DMA_CTRL_IRQ_QUIET being + * cleared before the trigger, and at the end the control + * block is clean and zeroed-out. + * + * - Conversely, I like the tx channel (the non-interrupt + * channel) having ctrl *not* be the trigger, so that + * DMA_CTRL_IRQ_QUIET is cleared by the time the trigger + * happens, so the IRQ machinery doesn't need to be engaged + * at all. */ struct dma_alias1 *tx_data_blocks = alloca(sizeof(struct dma_alias1)*(pruned_iovcnt+1)); struct dma_alias0 *rx_data_blocks = alloca(sizeof(struct dma_alias0)*(pruned_iovcnt+1)); + static_assert(!DMA_IS_TRIGGER(typeof(tx_data_blocks[0]), ctrl)); + static_assert(DMA_IS_TRIGGER(typeof(rx_data_blocks[0]), ctrl)); for (int i = 0, j = 0; i < iovcnt; i++) { if (!iov[i].iov_len) @@ -210,6 +232,11 @@ static void rp2040_hwspi_readwritev(struct rp2040_hwspi *self, const struct dupl } tx_data_blocks[pruned_iovcnt] = (typeof(tx_data_blocks[0])){0}; rx_data_blocks[pruned_iovcnt] = (typeof(rx_data_blocks[0])){0}; + /* If ctrl isn't the trigger then we need to make sure that + * DMA_CTRL_IRQ_QUIET isn't cleared before the trigger + * happens. */ + if (!DMA_IS_TRIGGER(typeof(rx_data_blocks[0]), ctrl)) + rx_data_blocks[pruned_iovcnt].ctrl = DMA_CTRL_IRQ_QUIET; /* Set up ctrl. */ DMA_NONTRIGGER(self->dma_tx_ctrl, read_addr) = tx_data_blocks; |