Re: [PATCH] arm64: dts: qcom: qcm6490-idp: Enable PCIe1

[Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>]

On 2/19/2026 7:02 PM, Bjorn Andersson wrote:
> On Thu, Feb 12, 2026 at 05:36:29PM +0530, Sushrut Shree Trivedi wrote:
>> Remove PCIe1 clocks from protected-list and enable PCIe1 controller
>> and its corresponding PHY nodes on qcm6490-idp platform.
>>
> Does this mean that the firmware has changed or does it mean that the
> protected-clocks list has been wrong all along? Is the remaining part
> of the list correct?
No, there is no change in firmware. During initial chipset bring-up, 
clocks related
to unused peripherals are marked as protected to prevent them from being
turned on by mistake. If there is a need to enable a particular peripheral
later on (e.g. PCIe1 here), the respective clocks can be removed from 
protected
list so they can be enabled by respective drivers.

The remaining list contains clocks for currently unused peripherals.
>> PCIe1 is used to connect NVMe based SSD's on this platform.
>>
> "used to connect" sounds like it's optional. If the NVMe is soldered to
> the board then say that in the commit message.
Ack'd
>
>> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 39 +++++++++++++++++++++++++++-----
>>   1 file changed, 33 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> index 089a027c57d5..1fac051ab3e7 100644
>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> @@ -533,12 +533,7 @@ vreg_bob_3p296: bob {
>>   };
>>   
>>   &gcc {
>> -	protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
>> -			   <GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
>> -			   <GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
>> -			   <GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
>> -			   <GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
>> -			   <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
>> +	protected-clocks = <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
>>   			   <GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
>>   			   <GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
>>   			   <GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
>> @@ -615,6 +610,22 @@ &mdss_dsi_phy {
>>   	status = "okay";
>>   };
>>   
>> +&pcie1 {
>> +	perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>> +
>> +	pinctrl-0 = <&pcie1_reset_n>, <&pcie1_wake_n>, <&pcie1_clkreq_n>;
>> +	pinctrl-names = "default";
>> +
>> +	status = "okay";
>> +};
>> +
>> +&pcie1_phy {
>> +	vdda-phy-supply = <&vreg_l10c_0p88>;
>> +	vdda-pll-supply = <&vreg_l6b_1p2>;
>> +
>> +	status = "okay";
>> +};
>> +
>>   &pm7250b_gpios {
>>   	lcd_disp_bias_en: lcd-disp-bias-en-state {
>>   		pins = "gpio2";
>> @@ -920,6 +931,22 @@ &tlmm {
>>   	gpio-reserved-ranges = <32 2>, /* ADSP */
>>   			       <48 4>; /* NFC */
>>   
>> +	pcie1_reset_n: pcie1-reset-n-state {
>> +		pins = "gpio2";
>> +		function = "gpio";
>> +		drive-strength = <16>;
>> +		output-low;
>> +		bias-disable;
>> +		};
> That doesn't look right.
There seems to be multiple default pinctrl configurations for perst.
On rb3 industrial kit we used the same output-low & bias-disable
configuration while for other targets, I see we have used bias-pull-up.
I feel the current config is more appropriate since it ensures the endpoint
remains under reset and doesn't try to link-up with host until the
controller driver initiates link training.

Do you suggest a different configuration ?
>> +
>> +	pcie1_wake_n: pcie1-wake-n-state {
>> +		pins = "gpio3";
>> +		function = "gpio";
>> +		drive-strength = <2>;
>> +		bias-pull-up;
>> +	};
>> +
>> +
> Extra white space.
Ack'd
>
> Thanks,
> Bjorn
>
>>   	sd_cd: sd-cd-state {
>>   		pins = "gpio91";
>>   		function = "gpio";
>>
>> ---
>> base-commit: 4f938c7d3b25d87b356af4106c2682caf8c835a2
>> change-id: 20260212-qcm6490-idp-24f7b6a1812d
>>
>> Best regards,
>> -- 
>> Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>>