On Tue, Apr 16, 2024 at 02:35:47PM +0200, Wolfram Sang wrote: > In the RX DMA completion handler, the hrtimer was restarted before DMA > was set up. If DMA failed, for some reason, it would clean up and the > hrtimer would run into a NULL-pointer. Restart the timer after DMA was > successfully set up. Further investigations, please review: Originally, I thought this was the race condition Dirk encountered. But I didn't take locking into account. sci_dma_rx_timer_fn() is protected by the uart_port_lock. sci_dma_rx_complete() is also protected by the uart_port_lock. So, the position of restarting the hrtimer should not matter. However, I still think it is good coding practice (and easier to understand) if we cancel the hrtimer at the begin of sci_dma_rx_complete() and reenable it only if setting DMA was successful. Because that basically means the timer only runs when DMA was scheduled and has not finished yet. There is some unnecessary unlock/lock in the error handling of sci_dma_rx_complete(). I'll simplify this by moving the dev_err downwards.
Attachment:
signature.asc
Description: PGP signature