Re: [PATCH v8 3/4] arm64: dts: qcom: sc7280: Add PCIe nodes for IDP board

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-09-21 01:21, Stephen Boyd wrote:
Quoting Prasad Malisetty (2021-09-17 10:15:46)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 99f9ee5..ee00df0 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -199,6 +199,39 @@
        modem-init;
 };

+&pcie1 {
+       status = "okay";
+
+       perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
+       pinctrl-0 = <&pcie1_default_state &nvme_ldo_enable_pin>;
+};
+
+&pcie1_phy {
+       status = "okay";
+
+       vdda-phy-supply = <&vreg_l10c_0p8>;
+       vdda-pll-supply = <&vreg_l6b_1p2>;
+};
+
+&pcie1_default_state {
+       reset-n {
+               pins = "gpio2";
+               function = "gpio";
+
+               drive-strength = <16>;
+               output-low;
+               bias-disable;
+       };
+
+       wake-n {
+               pins = "gpio3";
+               function = "gpio";
+
+               drive-strength = <2>;
+               bias-pull-up;
+       };

I think the previous round of this series Bjorn was saying that these
should be different nodes and tacked onto the pinctrl-0 list for the
pcie1 device instead of adding them as subnodes of the "default state".


Hi Stephen,

Here NVMe gpio entry is endpoint related where as wake-n and reset-n are PCIe controller gpio's. I think Bjorn was saying keep endpoint related gpio (NVMe) in separate state entry in pinctrl-0 list.

Thanks
-Prasad.

+};
+
 &pmk8350_vadc {
        pmk8350_die_temp {
                reg = <PMK8350_ADC7_DIE_TEMP>;
@@ -343,3 +376,10 @@
                bias-pull-up;
        };
 };
+
+&tlmm {
+       nvme_ldo_enable_pin: nvme_ldo_enable_pin {

Please use dashes where you use underscores in node names

       nvme_ldo_enable_pin: nvme-ldo-enable-pin {

+               function = "gpio";
+               bias-pull-up;

Of course with that said, the name of this node makes it sound like this is a gpio controlled regulator. Why not use that binding then and enable
the regulator either by default with regulator properties like
regulator-always-on and regulator-boot-enable and/or reference it from
the pcie device somehow so that it can be turned off during suspend?

Agree, I will add in next patch series.

+       };
+};



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux