Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1670509544-15977-1-git-send-email-quic_vnivarth@quicinc.com>
In-Reply-To: <1670509544-15977-1-git-send-email-quic_vnivarth@quicinc.com>
From:   Doug Anderson <dianders@chromium.org>
Date:   Tue, 13 Dec 2022 11:41:31 -0800
Message-ID: <CAD=FV=VTkpZ84RzaKYmb15AG+hiVUM0mk0ieGmDHWYVLQgsrnA@mail.gmail.com>
Subject: Re: [V4] spi: spi-geni-qcom: Add support for SE DMA mode
To:     Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
Cc:     agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org,
        broonie@kernel.org, linux-arm-msm@vger.kernel.org,
        linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
        quic_msavaliy@quicinc.com, mka@chromium.org, swboyd@chromium.org,
        quic_vtanuku@quicinc.com, vkoul@kernel.org,
        quic_arandive@quicinc.com, quic_ramkri@quicinc.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

On Thu, Dec 8, 2022 at 6:25 AM Vijaya Krishna Nivarthi
<quic_vnivarth@quicinc.com> wrote:
>
> SE DMA mode can be used for larger transfers and FIFO mode
> for smaller transfers.
>
> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
> ---
> v4 -> v3:
> - remove cur_m_cmd field
> - add dma_addr fields to spi_geni_master
> - dont use dma_addr from xfer, use above
> - dev_warn instead of dev_err in handle_timeout
> - set xfer_mode to FIFO in set_cs
>
> v2 -> v3:
> - indentation change
> - readability change
>
> v1 -> v2:
> - replace writel_relaxed with writel
> - corrected order of performing FSM reset and dma_unprep
> - added more comments in geni_can_dma
> - removed redundant change in spi_geni_init(DMA as default xfer_mode)
> - removed redundant initialisations in setup_se_xfer
> - removed redundant null checks in setup_se_xfer
> - added dma_tx_prep failure handling(rx un_map) in setup_se_xfer
> - move handing return of setip_se_xfer to transfer_one
> - apply irq error handling for DMA mode too in geni_spi_isr
> ---
>  drivers/spi/spi-geni-qcom.c | 211 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 165 insertions(+), 46 deletions(-)

This looks reasonable to me now, thanks.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

A note to Mark since he was also concerned about the fact that we
aren't using the SPI framework to manage the DMA [1] and Vijay didn't
respond directly to him on this issue. The issue here has to do with
the fact that the driver wanted to use geni_se_tx_dma_prep() /
geni_se_rx_dma_prep() and that function overlaps with what the SPI
framework does. Eventually it seems like geni_se_tx_dma_prep() /
geni_se_rx_dma_prep() should be reworked so as not to overlap, but I
agree with Vijay that it can be future work. I'm at least happy now
that the driver is using its own storage for the dma_addr_t variables
now.Obviously, though, the final say is yours. :-)

[1] https://lore.kernel.org/r/Y4kxVP97C66oi0Bi@sirena.org.uk