Rx timeout interrupt should be controlled individually #568

New issue

Open

opened 2021-09-20 22:36:53 -05:00 by xiongyu0523 · 3 comments

xiongyu0523 commented

2021-09-20 22:36:53 -05:00

(Migrated from github.com)

I saw this PR https://github.com/raspberrypi/pico-sdk/pull/504/commits/6e6cb52192b3a9455d0714e606633d82283062b4 and some discussion in issue https://github.com/raspberrypi/pico-sdk/issues/500.

It is not flexible to have both RX and RX timeout interrupts enabled together. In some cases when UART is used together with DMA, we just want to have the RX timeout (IDLE) to tell CPU that a complete frame is received without triggering a DMA interrupt, and definitely, we do not want each character or FIFO threshold trigger a interrupt

I would suggest using a single function to control each interrupt bit. Even the original one is breaking the single-purpose design principle.

Will be something like, what's your thoughts?

uart_set_tx_irq_enable
uart_set_rx_irq_enable
uart_set_idle_irq_enable
uart_set_error_irq_enable

I saw this PR https://github.com/raspberrypi/pico-sdk/pull/504/commits/6e6cb52192b3a9455d0714e606633d82283062b4 and some discussion in issue https://github.com/raspberrypi/pico-sdk/issues/500. It is not flexible to have both RX and RX timeout interrupts enabled together. In some cases when UART is used together with DMA, we just want to have the RX timeout (IDLE) to tell CPU that a complete frame is received without triggering a DMA interrupt, and definitely, we do not want each character or FIFO threshold trigger a interrupt I would suggest using a single function to control each interrupt bit. Even the original one is breaking the single-purpose design principle. Will be something like, what's your thoughts? `uart_set_tx_irq_enable` `uart_set_rx_irq_enable` `uart_set_idle_irq_enable` `uart_set_error_irq_enable`

👍 1

xiongyu0523 commented

2021-09-20 22:39:26 -05:00

(Migrated from github.com)

@Wren6991 @MustBeArt

Wren6991 commented

2021-09-21 08:53:20 -05:00

(Migrated from github.com)

The RX IRQ function is trying to give a simplified view of RX interrupts, satisfying these rules:

When at least one character has been received and not read, there will eventually be an interrupt
When all characters have been read, there will be no more interrupts until more are received

Which can be serviced with simple interrupt handler logic, "when I get an interrupt, read all characters and return". It's not perfect but it hides some of the surprising behaviour details of the PL011, and we could write similar functions for another UART (say, PIO) which satisfy the same conditions above.

The functions you suggest give better control, but they seem quite specific to the way PL011 interrupts work.

It is not flexible to have both RX and RX timeout interrupts enabled together. In some cases when UART is used together with DMA, we just want to have the RX timeout (IDLE) to tell CPU that a complete frame is received without triggering a DMA interrupt, and definitely, we do not want each character or FIFO threshold trigger a interrupt

Sorry, could you give a little more detail here? I'm not sure I fully understand the use case. The DREQ glue does handle SINGLE requests, so there should be no need for the processor to come and tidy up odd bytes from the FIFO when the receiver goes idle.

Are you saying the DMA is handling all of the data movement, and you want to use the idle IRQ to prompt the processor to scan through the DMA RX buffer at meaningful times?

At some point, if you are using PL011-specific functionality, you may just want to write your own wrappers

void pl011_set_idle_irq(uart_hw_t *hw, bool irq) {
    if (irq) {
        hw_set_bits(hw->imsc, UART_UARTIMSC_RTIM_BITS);
    } else {
        hw_clear_bits(hw->imsc, UART_UARTIMSC_RTIM_BITS);
   }
}

The RX IRQ function is trying to give a simplified view of RX interrupts, satisfying these rules: - When at least one character has been received and not read, there will eventually be an interrupt - When all characters have been read, there will be no more interrupts until more are received Which can be serviced with simple interrupt handler logic, "when I get an interrupt, read all characters and return". It's not perfect but it hides some of the surprising behaviour details of the PL011, and we could write similar functions for another UART (say, PIO) which satisfy the same conditions above. The functions you suggest give better control, but they seem quite specific to the way PL011 interrupts work. > It is not flexible to have both RX and RX timeout interrupts enabled together. In some cases when UART is used together with DMA, we just want to have the RX timeout (IDLE) to tell CPU that a complete frame is received without triggering a DMA interrupt, and definitely, we do not want each character or FIFO threshold trigger a interrupt Sorry, could you give a little more detail here? I'm not sure I fully understand the use case. The DREQ glue does handle SINGLE requests, so there should be no need for the processor to come and tidy up odd bytes from the FIFO when the receiver goes idle. Are you saying the DMA is handling all of the data movement, and you want to use the idle IRQ to prompt the processor to scan through the DMA RX buffer at meaningful times? At some point, if you are using PL011-specific functionality, you may just want to write your own wrappers ```c void pl011_set_idle_irq(uart_hw_t *hw, bool irq) { if (irq) { hw_set_bits(hw->imsc, UART_UARTIMSC_RTIM_BITS); } else { hw_clear_bits(hw->imsc, UART_UARTIMSC_RTIM_BITS); } } ````

xiongyu0523 commented

2021-09-24 09:37:21 -05:00

(Migrated from github.com)

Are you saying the DMA is handling all of the data movement, and you want to use the idle IRQ to prompt the processor to scan through the DMA RX buffer at meaningful times?

Yes exactly.

When I got a DMA interrupt, I know the buffer is full, I readout all data within the data buffer
When I get an Idle interrupt, I know the a complete frame is received, I read out the amount of data from the last position of the DMA buffer to the current position.

`Are you saying the DMA is handling all of the data movement, and you want to use the idle IRQ to prompt the processor to scan through the DMA RX buffer at meaningful times?` Yes exactly. - When I got a DMA interrupt, I know the buffer is full, I readout all data within the data buffer - When I get an Idle interrupt, I know the a complete frame is received, I read out the amount of data from the last position of the DMA buffer to the current position.

👍 1