Re: [PATCH 2/2] spi: spi-geni-qcom: Do not do DMA map/unmap inside driver, use framework instead

[Doug Anderson <dianders@xxxxxxxxxxxx>]

Hi,

On Fri, May 12, 2023 at 10:07 AM Vijaya Krishna Nivarthi
<quic_vnivarth@xxxxxxxxxxx> wrote:
>
> @@ -836,35 +858,24 @@ static int setup_se_xfer(struct spi_transfer *xfer,
>         geni_se_setup_m_cmd(se, m_cmd, FRAGMENTATION);
>
>         if (mas->cur_xfer_mode == GENI_SE_DMA) {
> +               dma_addr_t dma_ptr_sg;
> +               unsigned int dma_len_sg;
> +
>                 if (m_cmd & SPI_RX_ONLY) {
> -                       ret =  geni_se_rx_dma_prep(se, xfer->rx_buf,
> -                               xfer->len, &mas->rx_se_dma);
> -                       if (ret) {
> -                               dev_err(mas->dev, "Failed to setup Rx dma %d\n", ret);
> -                               mas->rx_se_dma = 0;
> -                               goto unlock_and_return;
> -                       }
> +                       dma_ptr_sg = sg_dma_address(xfer->rx_sg.sgl);
> +                       dma_len_sg = sg_dma_len(xfer->rx_sg.sgl);
> +                       geni_se_rx_init_dma(se, &dma_ptr_sg, dma_len_sg);

nit: probably don't need local variables if you change patch set #1
like I suggested and don't pass in a pointer for the iova.


One last question: should you call:

dma_set_max_seg_size(dev, INT_MAX)

...in your probe function? I don't think you have any limitations of
maximum segment size, right? Right now if you don't set anything it
looks as if it considers your max to be 64K. That would cause the SPI
framework to break things up into multiple chunks which would make you
fall back to FIFO mode, right?

Other than that this looks good to me.

-Doug