Messages in this thread | | | Date | Tue, 28 Sep 2021 19:06:41 +0530 | From | Prasad Malisetty <> | Subject | Re: [PATCH v8 3/4] arm64: dts: qcom: sc7280: Add PCIe nodes for IDP board |
| |
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.
>> + }; >> +};
| |