looks ok, touching imxrt driver only
On Wed, Sep 1, 2021 at 7:55 AM Christian Mauderer
<christian.maude...@embedded-brains.de> wrote:
>
> It wasn't possible to keep the CS line low between multiple message
> descriptors in one transfer. This patch reworks the driver so that it is
> possible.
>
> Update #4180
> ---
> bsps/arm/imxrt/spi/imxrt-lpspi.c | 197 +++++++++++++++++++------------
> 1 file changed, 124 insertions(+), 73 deletions(-)
>
> diff --git a/bsps/arm/imxrt/spi/imxrt-lpspi.c
> b/bsps/arm/imxrt/spi/imxrt-lpspi.c
> index 06d8f715d9..80b47f9663 100644
> --- a/bsps/arm/imxrt/spi/imxrt-lpspi.c
> +++ b/bsps/arm/imxrt/spi/imxrt-lpspi.c
> @@ -43,16 +43,19 @@ struct imxrt_lpspi_bus {
> uint32_t src_clock_hz;
> clock_ip_name_t clock_ip;
>
> - uint32_t msg_todo;
> - const spi_ioc_transfer *msg;
> rtems_binary_semaphore sem;
> - uint32_t tcr;
> + bool cs_change_on_last_msg;
>
> + uint32_t rx_msg_todo;
> + const spi_ioc_transfer *rx_msg;
> size_t remaining_rx_size;
> uint8_t *rx_buf;
>
> + uint32_t tx_msg_todo;
> + const spi_ioc_transfer *tx_msg;
> size_t remaining_tx_size;
> const uint8_t *tx_buf;
> +
> uint32_t fifo_size;
> };
>
> @@ -146,11 +149,7 @@ static void imxrt_lpspi_config(
> }
>
> tcr |= LPSPI_TCR_PCS(msg->cs);
> -
> - if (!msg->cs_change) {
> - tcr |= LPSPI_TCR_CONT_MASK;
> - }
> -
> + tcr |= LPSPI_TCR_CONT_MASK;
> tcr |= LPSPI_TCR_FRAMESZ(word_size-1);
>
> if (ccr_orig != ccr) {
> @@ -159,9 +158,13 @@ static void imxrt_lpspi_config(
> regs->CR |= LPSPI_CR_MEN_MASK;
> }
>
> - /* No CONTC on first write. Otherwise upper 8 bits are not written. */
> - regs->TCR = tcr;
> - regs->TCR = tcr | LPSPI_TCR_CONTC_MASK | LPSPI_TCR_CONT_MASK;
> + if (bus->cs_change_on_last_msg) {
> + /* No CONTC on first write. Otherwise upper 8 bits are not written. */
> + regs->TCR = tcr;
> + }
> + regs->TCR = tcr | LPSPI_TCR_CONTC_MASK;
> +
> + bus->cs_change_on_last_msg = msg->cs_change;
> }
>
> static inline bool imxrt_lpspi_rx_fifo_not_empty(
> @@ -184,48 +187,72 @@ static inline bool imxrt_lpspi_tx_fifo_not_full(
> bus->fifo_size - 2;
> }
>
> +static void imxrt_lpspi_next_tx_msg(
> + struct imxrt_lpspi_bus *bus,
> + volatile LPSPI_Type *regs
> +)
> +{
> + if (bus->tx_msg_todo > 0) {
> + const spi_ioc_transfer *msg;
> +
> + msg = bus->tx_msg;
> +
> + imxrt_lpspi_config(bus, regs, msg);
> + bus->remaining_tx_size = msg->len;
> + bus->tx_buf = msg->tx_buf;
> + }
> +}
> +
> static void imxrt_lpspi_fill_tx_fifo(
> struct imxrt_lpspi_bus *bus,
> volatile LPSPI_Type *regs
> )
> {
> while(imxrt_lpspi_tx_fifo_not_full(bus, regs)
> - && bus->remaining_tx_size > 0) {
> - if (bus->remaining_tx_size == 1) {
> - regs->TCR &= ~(LPSPI_TCR_CONT_MASK);
> - }
> + && (bus->tx_msg_todo > 0 || bus->remaining_tx_size > 0)) {
> + if (bus->remaining_tx_size > 0) {
> + if (bus->remaining_tx_size == 1 && bus->tx_msg->cs_change) {
> + /*
> + * Necessary for getting the last data out of the Rx FIFO. See "i.MX
> + * RT1050 Processor Reference Manual Rev. 4" Chapter 47.3.2.2
> "Receive
> + * FIFO and Data Match":
> + *
> + * "During a continuous transfer, if the transmit FIFO is empty, then
> + * the receive data is only written to the receive FIFO after the
> + * transmit FIFO is written or after the Transmit Command Register
> (TCR)
> + * is written to end the frame."
> + */
> + regs->TCR &= ~(LPSPI_TCR_CONT_MASK);
> + }
>
> - if (bus->tx_buf != NULL) {
> - regs->TDR = bus->tx_buf[0];
> - ++bus->tx_buf;
> - } else {
> - regs->TDR = 0;
> + if (bus->tx_buf != NULL) {
> + regs->TDR = bus->tx_buf[0];
> + ++bus->tx_buf;
> + } else {
> + regs->TDR = 0;
> + }
> + --bus->remaining_tx_size;
> + }
> + if (bus->remaining_tx_size == 0) {
> + --bus->tx_msg_todo;
> + ++bus->tx_msg;
> + imxrt_lpspi_next_tx_msg(bus, regs);
> }
> - --bus->remaining_tx_size;
> }
> }
>
> -static void imxrt_lpspi_next_msg(
> +static void imxrt_lpspi_next_rx_msg(
> struct imxrt_lpspi_bus *bus,
> volatile LPSPI_Type *regs
> )
> {
> - if (bus->msg_todo > 0) {
> + if (bus->rx_msg_todo > 0) {
> const spi_ioc_transfer *msg;
>
> - msg = bus->msg;
> + msg = bus->rx_msg;
>
> - imxrt_lpspi_config(bus, regs, msg);
> - bus->remaining_tx_size = msg->len;
> bus->remaining_rx_size = msg->len;
> bus->rx_buf = msg->rx_buf;
> - bus->tx_buf = msg->tx_buf;
> -
> - imxrt_lpspi_fill_tx_fifo(bus, regs);
> - regs->IER = LPSPI_IER_TDIE_MASK;
> - } else {
> - regs->IER = 0;
> - rtems_binary_semaphore_post(&bus->sem);
> }
> }
>
> @@ -234,15 +261,22 @@ static void imxrt_lpspi_pull_data_from_rx_fifo(
> volatile LPSPI_Type *regs
> )
> {
> - while (imxrt_lpspi_rx_fifo_not_empty(regs) && bus->remaining_rx_size > 0) {
> - uint32_t data;
> -
> - data = regs->RDR;
> - if (bus->rx_buf != NULL) {
> - *bus->rx_buf = data;
> - ++bus->rx_buf;
> + uint32_t data;
> + while (imxrt_lpspi_rx_fifo_not_empty(regs)
> + && (bus->rx_msg_todo > 0 || bus->remaining_rx_size > 0)) {
> + if (bus->remaining_rx_size > 0) {
> + data = regs->RDR;
> + if (bus->rx_buf != NULL) {
> + *bus->rx_buf = data;
> + ++bus->rx_buf;
> + }
> + --bus->remaining_rx_size;
> + }
> + if (bus->remaining_rx_size == 0) {
> + --bus->rx_msg_todo;
> + ++bus->rx_msg;
> + imxrt_lpspi_next_rx_msg(bus, regs);
> }
> - --bus->remaining_rx_size;
> }
> }
>
> @@ -257,39 +291,22 @@ static void imxrt_lpspi_interrupt(void *arg)
> imxrt_lpspi_pull_data_from_rx_fifo(bus, regs);
> imxrt_lpspi_fill_tx_fifo(bus, regs);
>
> - if (bus->remaining_tx_size == 0) {
> - if (bus->remaining_rx_size > 0) {
> - regs->IER = LPSPI_IER_RDIE_MASK;
> - } else {
> - --bus->msg_todo;
> - ++bus->msg;
> - imxrt_lpspi_next_msg(bus, regs);
> - }
> + if (bus->tx_msg_todo > 0 || bus->remaining_tx_size > 0) {
> + regs->IER = LPSPI_IER_TDIE_MASK;
> + } else if (bus->rx_msg_todo > 0 || bus->remaining_rx_size > 0) {
> + regs->IER = LPSPI_IER_RDIE_MASK;
> + } else {
> + regs->IER = 0;
> + rtems_binary_semaphore_post(&bus->sem);
> }
> }
>
> static inline int imxrt_lpspi_settings_ok(
> struct imxrt_lpspi_bus *bus,
> - const spi_ioc_transfer *msg
> + const spi_ioc_transfer *msg,
> + const spi_ioc_transfer *prev_msg
> )
> {
> - if (msg->cs_change == 0) {
> - /*
> - * This one most likely would need a bigger workaround if it is
> necessary.
> - * See "i.MX RT1050 Processor Reference Manual Rev. 4" Chapter 47.3.2.2
> - * "Receive FIFO and Data Match":
> - *
> - * "During a continuous transfer, if the transmit FIFO is empty, then the
> - * receive data is only written to the receive FIFO after the transmit
> FIFO
> - * is written or after the Transmit Command Register (TCR) is written to
> end
> - * the frame."
> - *
> - * It might is possible to extend the driver so that it can work with an
> - * empty read buffer.
> - */
> - return -EINVAL;
> - }
> -
> /* most of this is currently just not implemented */
> if (msg->cs > 3 ||
> msg->speed_hz > bus->base.max_speed_hz ||
> @@ -299,6 +316,18 @@ static inline int imxrt_lpspi_settings_ok(
> return -EINVAL;
> }
>
> + if (prev_msg != NULL && !prev_msg->cs_change) {
> + /*
> + * A lot of settings have to be the same in this case because the upper 8
> + * bit of TCR can't be changed if it is a continuous transfer.
> + */
> + if (prev_msg->cs != msg->cs ||
> + prev_msg->speed_hz != msg->speed_hz ||
> + prev_msg->mode != msg->mode) {
> + return -EINVAL;
> + }
> + }
> +
> return 0;
> }
>
> @@ -308,17 +337,29 @@ static int imxrt_lpspi_check_messages(
> uint32_t size
> )
> {
> + const spi_ioc_transfer *prev_msg = NULL;
> +
> while(size > 0) {
> int rv;
> - rv = imxrt_lpspi_settings_ok(bus, msg);
> + rv = imxrt_lpspi_settings_ok(bus, msg, prev_msg);
> if (rv != 0) {
> return rv;
> }
>
> + prev_msg = msg;
> ++msg;
> --size;
> }
>
> + /*
> + * Check whether cs_change is set on last message. Can't work without it
> + * because the last received data is only put into the FIFO if it is the
> end
> + * of a transfer or if another TX byte is put into the FIFO.
> + */
> + if (!prev_msg->cs_change) {
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> @@ -336,10 +377,20 @@ static int imxrt_lpspi_transfer(
> rv = imxrt_lpspi_check_messages(bus, msgs, n);
>
> if (rv == 0) {
> - bus->msg_todo = n;
> - bus->msg = &msgs[0];
> -
> - imxrt_lpspi_next_msg(bus, bus->regs);
> + bus->tx_msg_todo = n;
> + bus->tx_msg = &msgs[0];
> + bus->rx_msg_todo = n;
> + bus->rx_msg = &msgs[0];
> + bus->cs_change_on_last_msg = true;
> +
> + imxrt_lpspi_next_rx_msg(bus, bus->regs);
> + imxrt_lpspi_next_tx_msg(bus, bus->regs);
> + /*
> + * Enable the transmit FIFO empty interrupt which will cause an interrupt
> + * instantly because there is no data in the transmit FIFO. The interrupt
> + * will then fill the FIFO. So nothing else to do here.
> + */
> + bus->regs->IER = LPSPI_IER_TDIE_MASK;
> rtems_binary_semaphore_wait(&bus->sem);
> }
>
> @@ -416,7 +467,7 @@ static int imxrt_lpspi_setup(spi_bus *base)
>
> bus = (struct imxrt_lpspi_bus *) base;
>
> - rv = imxrt_lpspi_settings_ok(bus, &msg);
> + rv = imxrt_lpspi_settings_ok(bus, &msg, NULL);
>
> /*
> * Nothing to do besides checking.
> --
> 2.31.1
>
> _______________________________________________
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
