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
In-Reply-To: <6e498021-bf08-3fd8-a3b8-2fcba054f4a0@quicinc.com>
References: <1645716828-15305-1-git-send-email-quic_srivasam@quicinc.com>
 <1645716828-15305-7-git-send-email-quic_srivasam@quicinc.com>
 <CAE-0n53K3Em52tTFK4zmcsf_ZzFbX4uQQ00_uH9=pq44chwyeg@mail.gmail.com> <6e498021-bf08-3fd8-a3b8-2fcba054f4a0@quicinc.com>
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.10
Date:   Mon, 28 Feb 2022 13:15:34 -0800
Message-ID: <CAE-0n508Wcb7mnqx1TQN0vndOFRcJuyfvo_4YtJri+Kh-GfSbQ@mail.gmail.com>
Subject: Re: [PATCH v16 6/9] ASoC: qcom: Add support for codec dma driver
To:     Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>,
        agross@kernel.org, alsa-devel@alsa-project.org,
        bgoswami@codeaurora.org, bjorn.andersson@linaro.org,
        broonie@kernel.org, devicetree@vger.kernel.org,
        judyhsiao@chromium.org, lgirdwood@gmail.com,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        perex@perex.cz, quic_plai@quicinc.com, robh+dt@kernel.org,
        rohitkr@codeaurora.org, srinivas.kandagatla@linaro.org,
        tiwai@suse.com
Cc:     Venkata Prasad Potturu <quic_potturu@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Quoting Srinivasa Rao Mandadapu (2022-02-25 21:58:03)
>
> On 2/25/2022 5:10 AM, Stephen Boyd wrote:
> Thanks for your time Stephen!!!
> > Quoting Srinivasa Rao Mandadapu (2022-02-24 07:33:45)
> >> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> >> index 198f27c..b3af971 100644
[...]
> >> +
> >> +       buf->area = (unsigned char * __force)memremap(buf->addr, buf->bytes, MEMREMAP_WT);
> > What's the cast and __force for now? MEMREMAP_WT is almost never used so
> Here dma_buffer structure has virtual address(buf->area) of unsigned
> char pointer but memremap returns void pointer.
> > this looks wrong. Why can't MEMREMAP_WC be used? But if it's DMA then
> Okay. Will update the flag MEMREMAP_WT
> > why isn't dma_map_resource() being used?
>
> I am sorry bit confused here. You mean some thing like below.
>
> For Physical address mapping: buf->addr = dma_map_resource(pcm->card->dev, drvdata->va_cdc_dma_lpm_buf,
>                              buf->bytes, DMA_BIDIRECTIONAL, 0);

Yes, but I'm not sure about DMA_BIDIRECTIONAL. How does this
va_cdc_dma_lpm_buf work? Does userspace fill it up with bytes and the
DSP reads it out? More details please.

> For virtual address mapping. buf->area = (unsigned char * __force)memremap(buf->addr, buf->bytes, MEMREMAP_WC);

No. I'd expect dma_mmap_attrs() or dma_mmap_coherent() to be used to
mmap it into userspace.  The dma_map_resource() API only returns a
dma_addr_t type, which needs to be mapped via DMA APIs into a virtual
address. Passing it to memremap is incorrect, because it may be possible
to map it into an IOMMU, for which the DMA APIs know what to do in that
situation.