Messages in this thread | | | Subject | Re: [PATCH 1/3] soc: qcom: geni: More properly switch to DMA mode | From | Akash Asthana <> | Date | Mon, 12 Oct 2020 14:35:23 +0530 |
| |
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
| |