Received-SPF: pass (google.com: domain of linux-kernel+bounces-196478-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <205de8b7-507f-45c9-83ce-6eceb1466cb2@quicinc.com>
Date: Fri, 31 May 2024 16:35:38 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v9 4/4] arm64: dts: qcom: aim300: add AIM300 AIoT
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
CC: <andersson@kernel.org>, <konrad.dybcio@linaro.org>, <robh@kernel.org>,
        <krzk+dt@kernel.org>, <conor+dt@kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <kernel@quicinc.com>,
        Qiang Yu <quic_qianyu@quicinc.com>,
        Ziyue Zhang
	<quic_ziyuzhan@quicinc.com>, <quic_chenlei@quicinc.com>
References: <20240529100926.3166325-1-quic_tengfan@quicinc.com>
 <20240529100926.3166325-5-quic_tengfan@quicinc.com>
 <s5gt3p6zsd5ebrkop4dhd33tykln33f6ahu3pibymecxsmakyd@lg5wfgec6dat>
From: Tengfei Fan <quic_tengfan@quicinc.com>
In-Reply-To: <s5gt3p6zsd5ebrkop4dhd33tykln33f6ahu3pibymecxsmakyd@lg5wfgec6dat>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 5/29/2024 11:18 PM, Dmitry Baryshkov wrote:
> On Wed, May 29, 2024 at 06:09:26PM +0800, Tengfei Fan wrote:
>> Add AIM300 AIoT Carrier board DTS support, including usb, UART, PCIe,
>> I2C functions support.
>> Here is a diagram of AIM300 AIoT Carrie Board and SoM
>>   +--------------------------------------------------+
>>   |             AIM300 AIOT Carrier Board            |
>>   |                                                  |
>>   |           +-----------------+                    |
>>   |power----->| Fixed regulator |---------+          |
>>   |           +-----------------+         |          |
>>   |                                       |          |
>>   |                                       v VPH_PWR  |
>>   | +----------------------------------------------+ |
>>   | |                          AIM300 SOM |        | |
>>   | |                                     |VPH_PWR | |
>>   | |                                     v        | |
>>   | |   +-------+       +--------+     +------+    | |
>>   | |   | UFS   |       | QCS8550|     |PMIC  |    | |
>>   | |   +-------+       +--------+     +------+    | |
>>   | |                                              | |
>>   | +----------------------------------------------+ |
>>   |                                                  |
>>   |                    +----+          +------+      |
>>   |                    |USB |          | UART |      |
>>   |                    +----+          +------+      |
>>   +--------------------------------------------------+
>>
>> Co-developed-by: Qiang Yu <quic_qianyu@quicinc.com>
>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>> Co-developed-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/Makefile             |   1 +
>>   .../boot/dts/qcom/qcs8550-aim300-aiot.dts     | 322 ++++++++++++++++++
>>   2 files changed, 323 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/qcom/qcs8550-aim300-aiot.dts
> 
> [trimmed]
> 
>> +&remoteproc_adsp {
>> +	firmware-name = "qcom/qcs8550/adsp.mbn",
>> +			"qcom/qcs8550/adsp_dtbs.elf";
> 
> Please excuse me, I think I missed those on the previous run.
> 
> adsp_dtb.mbn

Currently, waht we have released is adsp_dtbs.elf. If we modify it to 
adsp_dtb.mbn, it may cause the ADSP functionality can not boot normally.

> 
>> +	status = "okay";
>> +};
>> +
>> +&remoteproc_cdsp {
>> +	firmware-name = "qcom/qcs8550/cdsp.mbn",
>> +			"qcom/qcs8550/cdsp_dtbs.elf";
> 
> cdsp_dtb.mbn

CDSP also as above ADSP.

> 
>> +	status = "okay";
>> +};
>> +
>> +&swr1 {
>> +	status = "okay";
>> +};
>> +
>> +&swr2 {
>> +	status = "okay";
>> +};
>> +
>> +&tlmm {
>> +	gpio-reserved-ranges = <32 8>;
>> +
>> +	dsi_active: dsi-active-state {
>> +		pins = "gpio133";
>> +		function = "gpio";
>> +		drive-strength = <8>;
>> +		bias-disable;
>> +	};
> 
> s/dsi/panel[-_]reset/

I will update this (like: "dsi_active" to "panel_resest_active") as your 
recommendation.

> 
>> +
>> +	dsi_suspend: dsi-suspend-state {
>> +		pins = "gpio133";
>> +		function = "gpio";
>> +		drive-strength = <2>;
>> +		bias-pull-down;
>> +	};

This also do update as "s/dsi/panel[-_]reset/".

>> +
>> +	te_active: te-active-state {
>> +		pins = "gpio86";
>> +		function = "mdp_vsync";
>> +		drive-strength = <2>;
>> +		bias-pull-down;
>> +	};
>> +
>> +	te_suspend: te-suspend-state {
>> +		pins = "gpio86";
>> +		function = "mdp_vsync";
>> +		drive-strength = <2>;
>> +		bias-pull-down;
>> +	};
> 
> What is the difference between these two?

TE pin needs to be pulled down for both active and suspend states. There 
is no difference.

> 
>> +};
> 

-- 
Thx and BRs,
Tengfei Fan