lkml.org 
[lkml]   [2021]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

>> + };
>> +};

\
 
 \ /
  Last update: 2021-09-28 15:37    [W:0.141 / U:0.552 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site