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;
Message-ID: <4af1abfb-9889-af13-7460-903eb94833bf@quicinc.com>
Date:   Fri, 21 Jul 2023 12:03:17 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.2
Subject: Re: [V3,09/11] remoteproc: qcom: Add Hexagon based multipd rproc
 driver
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        <agross@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>, <mathieu.poirier@linaro.org>,
        <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <conor+dt@kernel.org>, <mturquette@baylibre.com>,
        <sboyd@kernel.org>, <quic_eberman@quicinc.com>, <kvalo@kernel.org>,
        <loic.poulain@linaro.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-remoteproc@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-clk@vger.kernel.org>
CC:     <quic_srichara@quicinc.com>, <quic_sjaganat@quicinc.com>,
        <quic_kathirav@quicinc.com>, <quic_anusha@quicinc.com>
References: <20230718120501.3205661-1-quic_mmanikan@quicinc.com>
 <20230718120501.3205661-10-quic_mmanikan@quicinc.com>
 <c53c8fc5-a898-df1d-4626-696ffd990796@linaro.org>
Content-Language: en-US
From:   Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
In-Reply-To: <c53c8fc5-a898-df1d-4626-696ffd990796@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 7/20/2023 6:43 PM, Krzysztof Kozlowski wrote:
> On 18/07/2023 14:04, Manikanta Mylavarapu wrote:
>> It adds support to bring up remoteproc's on multipd model.
>> Pd means protection domain. It's similar to process in Linux.
>> Here QDSP6 processor runs each wifi radio functionality on a
>> separate process. One process can't access other process
>> resources, so this is termed as PD i.e protection domain.
> 
> ...
> 
>> + * User pd boot-info format mentioned below
>> + * <Version> <No of elements passing over smem> <Header type> <Header Length>
>> + * <Process Id> <Load address> <firmware mem Size>
>> + *
>> + * Returns 0 on success else negative value on failure.
>> + */
>> +static int share_upd_bootinfo_to_q6(struct rproc *rproc)
>> +{
>> +	int ret;
>> +	size_t size;
>> +	u16 cnt = 0, version;
>> +	void *ptr;
>> +	struct q6_wcss *wcss = rproc->priv, *upd_wcss;
>> +	struct device_node *upd_np;
>> +	struct platform_device *upd_pdev;
>> +	struct rproc *upd_rproc;
>> +	struct userpd_boot_info upd_bootinfo = {0};
>> +	const struct firmware *fw;
>> +
>> +	ret = qcom_smem_alloc(REMOTE_PID, UPD_BOOT_INFO_SMEM_ID,
>> +			      UPD_BOOT_INFO_SMEM_SIZE);
>> +	if (ret && ret != -EEXIST) {
>> +		dev_err(wcss->dev,
>> +			"failed to allocate q6 bootinfo smem segment\n");
>> +		return ret;
>> +	}
>> +
>> +	ptr = qcom_smem_get(REMOTE_PID, UPD_BOOT_INFO_SMEM_ID, &size);
>> +	if (IS_ERR(ptr) || size != UPD_BOOT_INFO_SMEM_SIZE) {
>> +		dev_err(wcss->dev,
>> +			"Unable to acquire smp2p item(%d) ret:%ld\n",
>> +			UPD_BOOT_INFO_SMEM_ID, PTR_ERR(ptr));
>> +		return PTR_ERR(ptr);
>> +	}
>> +
>> +	/* print physical address */
>> +	dev_info(wcss->dev,
>> +		 "smem phyiscal address:0x%lX\n",
>> +		 (uintptr_t)qcom_smem_virt_to_phys(ptr));
> 
> One more thought. Why do you need it? Even if this is not a kernel or
> user-space address, why would like to disclose the memory layout? I
> think this is contradictory to the kptr_restrict concept.
> 

Sure, I will remove.

Thanks & Regards,
Manikanta.

>> +
>> +	/*Version*/
> 
> 
> 
> Best regards,
> Krzysztof
>