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

From: Doug Anderson <dianders@chromium.org>
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,
	quic_ptalari@quicinc.com
Subject: Re: [PATCH v2 2/2] spi: spi-geni-qcom: Do not do DMA map/unmap inside driver, use framework instead
Date: Wed, 17 May 2023 07:20:40 -0700	[thread overview]
Message-ID: <CAD=FV=Wi9d0W5pRs9gGGDtjPPNfC5L0wL9YCpfDtFKyQyCONPg@mail.gmail.com> (raw)
In-Reply-To: <1684325894-30252-3-git-send-email-quic_vnivarth@quicinc.com>

Hi,

On Wed, May 17, 2023 at 5:18 AM Vijaya Krishna Nivarthi
<quic_vnivarth@quicinc.com> wrote:
>
> The spi geni driver in SE DMA mode, unlike GSI DMA, is not making use of
> DMA mapping functionality available in the framework.
> The driver does mapping internally which makes dma buffer fields available
> in spi_transfer struct superfluous while requiring additional members in
> spi_geni_master struct.
>
> Conform to the design by having framework handle map/unmap and do only
> DMA transfer in the driver; this also simplifies code a bit.
>
> Fixes: e5f0dfa78ac7 ("spi: spi-geni-qcom: Add support for SE DMA mode")

I'm not 100% sure I'd tag it as a fix. It's certainly a cleanup that
was asked for when thuat patch was landed, but technically it doesn't
fix any known problems. In any case, I'll leave it to Mark to decide
if he's happy with the fixes tag and to remove it if he sees fit. IMO
no need to re-post the patch either way.

> Suggested-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
> ---
> v1 -> v2:
> - pass dma address to new geni interfaces instead of pointer
> - set max_dma_len as per HPG
> - remove expendable local variables
> ---
>  drivers/spi/spi-geni-qcom.c | 103 +++++++++++++++++++++-----------------------
>  1 file changed, 50 insertions(+), 53 deletions(-)

Mark and Bjorn will have to coordinate how they want to land this,
since normally patch #1 would go through the Qualcomm tree and patch
#2 through the SPI tree. In any case:

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