On Tue, Feb 06, 2024 at 09:11:17AM +0530, Thangaraj Samynathan wrote:
> pci1xxxx_spi_transfer_with_dma is registered as transfer_one callback
> when DMA can be supported. This function adds DMA read operation which
> copies the data from host cpu buffer to SPI Tx Buffer.
> On DMA Read Completion interrupt, SPI transaction is initiated in isr.
> Helper functions pci1xxxx_spi_setup, pci1xxxx_spi_setup_dma_read and
> pci1xxxx_start_spi_xfer are added for starting spi transfer, setting up
> spi and dma read operation. In the existing implementation, codes are
> replaced with helper wherever applicable.
This description is kind of hard to follow and it seems like the
hardware is quite weirdly designed which doesn't help. What you appear
to be saying here is that the DMA here is transmit only as far as SPI is
concerned (it's a bit confusing that you say it's read DMA but AFAICT
it's reading from CPU memory and writing to the SPI controller). This
is a bit off in terms of the core DMA support, the core is assuming that
DMA will be bidirectional and mapping both the TX and RX buffers for DMA
which isn't great if the RX path is PIO. If that's the case then you
might be better off open coding the mapping of the buffers.
> +static irqreturn_t pci1xxxx_spi_isr_dma(int irq, void *dev)
> +{
> + if (regval & SPI_INTR) {
> + rx_buf = p->rx_buf;
> + memcpy_fromio(rx_buf + p->bytes_recvd, p->parent->reg_base +
> + SPI_MST_RSP_BUF_OFFSET(p->hw_inst), p->tx_sgl_len);
> + p->bytes_recvd += p->tx_sgl_len;
> +
> + p->tx_sgl = sg_next(p->tx_sgl);
If we're doing DMA why do we need to have a memcpy() here? That would
tie in with the DMA being transmit only.
Attachment:
signature.asc
Description: PGP signature
