Message-ID: <64fc529a-3250-e655-e06e-952f25b1ac2b@quicinc.com>
Date:   Thu, 9 Mar 2023 11:38:19 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v3 08/28] ASoC: qcom: Add USB backend ASoC driver for Q6
Content-Language: en-US
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        <mathias.nyman@intel.com>, <perex@perex.cz>, <broonie@kernel.org>,
        <lgirdwood@gmail.com>, <krzysztof.kozlowski+dt@linaro.org>,
        <agross@kernel.org>, <Thinh.Nguyen@synopsys.com>,
        <bgoswami@quicinc.com>, <andersson@kernel.org>,
        <robh+dt@kernel.org>, <gregkh@linuxfoundation.org>,
        <tiwai@suse.com>
CC:     <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <alsa-devel@alsa-project.org>, <devicetree@vger.kernel.org>,
        <linux-usb@vger.kernel.org>, <quic_jackp@quicinc.com>,
        <quic_plai@quicinc.com>
References: <20230308235751.495-1-quic_wcheng@quicinc.com>
 <20230308235751.495-9-quic_wcheng@quicinc.com>
 <ad7dd26d-0ee6-675f-72a5-a93bb0240121@linaro.org>
From:   Wesley Cheng <quic_wcheng@quicinc.com>
In-Reply-To: <ad7dd26d-0ee6-675f-72a5-a93bb0240121@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Srinivas,

On 3/9/2023 1:01 AM, Srinivas Kandagatla wrote:
> 
> 
> On 08/03/2023 23:57, Wesley Cheng wrote:
>> Create a USB BE component that will register a new USB port to the 
>> ASoC USB
>> framework.  This will handle determination on if the requested audio
>> profile is supported by the USB device currently selected.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> 
> Thanks Wesley for the patch, I have few minor comments.
> 

Thanks for the review!

>> ---
>>   include/sound/q6usboffload.h  |  20 ++++
>>   sound/soc/qcom/Kconfig        |   4 +
>>   sound/soc/qcom/qdsp6/Makefile |   1 +
>>   sound/soc/qcom/qdsp6/q6usb.c  | 208 ++++++++++++++++++++++++++++++++++
>>   4 files changed, 233 insertions(+)
>>   create mode 100644 include/sound/q6usboffload.h
>>   create mode 100644 sound/soc/qcom/qdsp6/q6usb.c
>>
>> diff --git a/include/sound/q6usboffload.h b/include/sound/q6usboffload.h
>> new file mode 100644
>> index 000000000000..4fb1912d9f55
>> --- /dev/null
>> +++ b/include/sound/q6usboffload.h
>> @@ -0,0 +1,20 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * linux/sound/q6usboffload.h -- QDSP6 USB offload
>> + *
>> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All 
>> rights reserved.
>> + */
>> +
>> +/**
>> + * struct q6usb_offload
>> + * @dev - dev handle to usb be
>> + * @sid - streamID for iommu
>> + * @intr_num - usb interrupter number
>> + * @domain - allocated iommu domain
>> + **/
>> +struct q6usb_offload {
>> +    struct device *dev;
>> +    long long sid;
>> +    u32 intr_num;
>> +    struct iommu_domain *domain;
> Why do we need to store this domain, You can remove this along with the 
> one line that gets domain in probe function.
> 

We'll need a reference to the iommu domain, because the QC USB offload 
driver will be the one that is going to map the XHCI interrupter and 
transfer ring regions for the audio DSP.  This happens when a USB QMI 
enable stream request is received in the USB offload driver.  Please 
refer to:

static int prepare_qmi_response(struct snd_usb_substream *subs,
		struct qmi_uaudio_stream_req_msg_v01 *req_msg,
		struct qmi_uaudio_stream_resp_msg_v01 *resp, int info_idx)
{
...
	xhci_pa = xhci_get_ir_resource(subs->dev, ir);
	if (!xhci_pa) {
		dev_err(uaudio_qdev->dev,
			"failed to get sec event ring address\n");
		ret = -ENODEV;
		goto free_sec_ring;
	}
...
	va = uaudio_iommu_map(MEM_EVENT_RING, dma_coherent, xhci_pa, PAGE_SIZE,
			NULL);
	if (!va) {
		ret = -ENOMEM;
		goto free_sec_ring;
	}

This is just an example for mapping the XHCI secondary interrupter.  We 
will also do the same for the transfer ring.

Thanks
Wesley Cheng