[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <70db6155-a230-4f9c-a2e1-2c872c87cd39@quicinc.com>
Date: Mon, 24 Mar 2025 15:21:53 +0800
From: "Wenbin Yao (Consultant)" <quic_wenbyao@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>, <andersson@...nel.org>,
<konradybcio@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<catalin.marinas@....com>, <will@...nel.org>,
<quic_qianyu@...cinc.com>, <sfr@...b.auug.org.au>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v1 3/3] arm64: dts: qcom: x1e80100-qcp: Add power control
and sideband signals for PCIe3
On 3/21/2025 6:08 AM, Bryan O'Donoghue wrote:
> On 20/03/2025 05:55, Wenbin Yao wrote:
>> From: Qiang Yu <quic_qianyu@...cinc.com>
>>
>> Add perst, wake and clkreq sideband signals and required regulators in
>> PCIe3 controller and PHY device tree node. Describe the voltage rails of
>> the x8 PCI slots for PCIe3 port.
>>
>> Signed-off-by: Qiang Yu <quic_qianyu@...cinc.com>
>> Signed-off-by: Wenbin Yao <quic_wenbyao@...cinc.com>
>> ---
>> arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 119 ++++++++++++++++++++++
>> 1 file changed, 119 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
>> b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
>> index 28086a2bc..9cd313802 100644
>> --- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
>> +++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
>> @@ -318,6 +318,48 @@ vreg_wcn_3p3: regulator-wcn-3p3 {
>> regulator-boot-on;
>> };
>>
>> + vreg_pcie_12v: regulator-pcie_12v {
>> + compatible = "regulator-fixed";
>> +
>> + regulator-name = "VREG_PCIE_12V";
>> + regulator-min-microvolt = <12000000>;
>> + regulator-max-microvolt = <12000000>;
>> +
>> + gpio = <&pm8550ve_8_gpios 8 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> +
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pcie_x8_12v>;
>> + };
>> +
>> + vreg_pcie_3v3_aux: regulator-pcie_3v3_aux {
>> + compatible = "regulator-fixed";
>> +
>> + regulator-name = "VREG_PCIE_3P3_AUX";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> +
>> + gpio = <&pmc8380_3_gpios 8 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> +
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pm_sde7_aux_3p3_en>;
>> + };
>> +
>> + vreg_pcie_3v3: regulator-pcie_3v3 {
>> + compatible = "regulator-fixed";
>> +
>> + regulator-name = "VREG_PCIE_3P3";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> +
>> + gpio = <&pmc8380_3_gpios 6 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> +
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pm_sde7_main_3p3_en>;
>> +};
>> +
>> usb-1-ss0-sbu-mux {
>> compatible = "onnn,fsusb42", "gpio-sbu-mux";
>>
>> @@ -907,6 +949,60 @@ &mdss_dp3_phy {
>> status = "okay";
>> };
>>
>> +&pm8550ve_8_gpios {
>> + pcie_x8_12v: pcie-12v-default-state {
>> + pins = "gpio8";
>> + function = "normal";
>> + output-enable;
>> + output-high;
>> + bias-pull-down;
>> + power-source = <0>;
>> + };
>> +};
>> +
>> +&pmc8380_3_gpios {
>> + pm_sde7_aux_3p3_en: pcie-aux-3p3-default-state {
>> + pins = "gpio8";
>> + function = "normal";
>> + output-enable;
>> + output-high;
>> + bias-pull-down;
>> + power-source = <0>;
>> + };
>> +
>> + pm_sde7_main_3p3_en: pcie-main-3p3-default-state {
>> + pins = "gpio6";
>> + function = "normal";
>> + output-enable;
>> + output-high;
>> + bias-pull-down;
>> + power-source = <0>;
>> + };
>> +};
>> +
>> +&pcie3 {
>> +
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pcie3_default>;
>> + perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
>> + wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
>> + status = "okay";
>> +};
>> +
>> +&pcie3_phy {
>> + vdda-phy-supply = <&vreg_l3j_0p8>;
>> + vdda-pll-supply = <&vreg_l3e_1p2>;
>> + vdda-qref-supply = <&vreg_l3c_0p8>;
>> +
>> + status = "okay";
>> +};
>> +
>> +&pcie3port {
>> + vpcie12v-supply = <&vreg_pcie_12v>;
>> + vpcie3v3-supply = <&vreg_pcie_3v3>;
>> + vpcie3v3aux-supply = <&vreg_pcie_3v3_aux>;
>> +};
>> +
>> &pcie4 {
>> perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
>> wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
>> @@ -1118,6 +1214,29 @@ nvme_reg_en: nvme-reg-en-state {
>> bias-disable;
>> };
>>
>> + pcie3_default: pcie3-default-state {
>> + clkreq-n-pins {
>> + pins = "gpio144";
>> + function = "pcie3_clk";
>> + drive-strength = <2>;
>> + bias-pull-up;
>> + };
>> +
>> + perst-n-pins {
>> + pins = "gpio143";
>> + function = "gpio";
>> + drive-strength = <2>;
>> + bias-pull-down;
>> + };
>> +
>> + wake-n-pins {
>> + pins = "gpio145";
>> + function = "gpio";
>> + drive-strength = <2>;
>> + bias-pull-up;
>> + };
>> + };
>> +
>> pcie4_default: pcie4-default-state {
>> clkreq-n-pins {
>> pins = "gpio147";
>> --
>> 2.34.1
>>
>>
>
> Recommend breaking this patch into at least two patches @ the and
>
> -> Add power control
> -> Add sideband signals
>
> if your patch title requires an and its usually a good indicator of a
> place to break that patch into different parts.
Will fix in the next version.
>
> ---
> bod
--
With best wishes
Wenbin
Powered by blists - more mailing lists