Hi Stephen,
static void geni_se_select_dma_mode(struct geni_se *se){ + u32 proto = geni_se_read_proto(se); u32 val;geni_se_irq_clear(se); + val = readl_relaxed(se->base + SE_GENI_M_IRQ_EN);+ if (proto != GENI_SE_UART) {Not a problem with this patch but it would be great if there was a comment here (and probably in geni_se_select_fifo_mode() too) indicating why GENI_SE_UART is special. Is it because GENI_SE_UART doesn't use the main sequencer? I think that is the reason, but I forgot and reading this code doesn't tell me that. Splitting the driver in this way where the logic is in the geni wrapper and in the engine driver leads to this confusion.
GENI_SE_UART uses main sequencer for TX and secondary for RX transfers because it is asynchronous in nature.
That's why RX related bits (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN) are not enable in main sequencer for UART.
(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN) bits are controlled from UART driver, it's gets enabled and disabled multiple times from start_tx ,stop_tx respectively.
Regards, Akash
+ val &= ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN); + val &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN); + } + writel_relaxed(val, se->base + SE_GENI_M_IRQ_EN); + + val = readl_relaxed(se->base + SE_GENI_S_IRQ_EN); + if (proto != GENI_SE_UART) + val &= ~S_CMD_DONE_EN; + writel_relaxed(val, se->base + SE_GENI_S_IRQ_EN); + val = readl_relaxed(se->base + SE_GENI_DMA_MODE_EN); val |= GENI_DMA_MODE_EN; writel_relaxed(val, se->base + SE_GENI_DMA_MODE_EN);
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

