Move the phy, phy-names, wake-gpio's to the pcie root port node instead of
the bridge node, as agreed upon in multiple places one instance is[1].
Update the qcom,pcie-common.yaml to include the phy, phy-names, and
wake-gpios properties in the root port node. There is already reset-gpio
defined for PERST# in pci-bus-common.yaml, start using that property
instead of perst-gpio.
For backward compatibility, do not remove any existing properties in the
bridge node.
[1] https://lore.kernel.org/linux-pci/20241211192014.GA3302752@bhelgaas/
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
.../devicetree/bindings/pci/qcom,pcie-common.yaml | 18 ++++++++++++++++++
.../devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 17 +++++++++++++----
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
index 0480c58f7d998adbac4c6de20cdaec945b3bab21..16e9acba1559b457da8a8a9dda4a22b226808f86 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
@@ -85,6 +85,24 @@ properties:
opp-table:
type: object
+patternProperties:
+ "^pcie@":
+ type: object
+ $ref: /schemas/pci/pci-pci-bridge.yaml#
+
+ properties:
+ reg:
+ maxItems: 1
+
+ phys:
+ maxItems: 1
+
+ wake-gpios:
+ description: GPIO controlled connection to WAKE# signal
+ maxItems: 1
+
+ unevaluatedProperties: false
+
required:
- reg
- reg-names
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
index 76cb9fbfd476fb0412217c68bd8db44a51c7d236..beb092f53019c31861460570cd2142506e05d8ef 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
@@ -162,9 +162,6 @@ examples:
iommu-map = <0x0 &apps_smmu 0x1c80 0x1>,
<0x100 &apps_smmu 0x1c81 0x1>;
- phys = <&pcie1_phy>;
- phy-names = "pciephy";
-
pinctrl-names = "default";
pinctrl-0 = <&pcie1_clkreq_n>;
@@ -173,7 +170,19 @@ examples:
resets = <&gcc GCC_PCIE_1_BCR>;
reset-names = "pci";
- perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
vddpe-3v3-supply = <&pp3300_ssd>;
+ pcie1_port0: pcie@0 {
+ device_type = "pci";
+ reg = <0x0 0x0 0x0 0x0 0x0>;
+ bus-range = <0x01 0xff>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ phys = <&pcie1_phy>;
+
+ reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
+ };
+
};
};
--
2.34.1On Mon, Apr 14, 2025 at 11:09:12AM +0530, Krishna Chaitanya Chundru wrote:
> Move the phy, phy-names, wake-gpio's to the pcie root port node instead of
> the bridge node, as agreed upon in multiple places one instance is[1].
>
> Update the qcom,pcie-common.yaml to include the phy, phy-names, and
> wake-gpios properties in the root port node. There is already reset-gpio
> defined for PERST# in pci-bus-common.yaml, start using that property
> instead of perst-gpio.
>
> For backward compatibility, do not remove any existing properties in the
> bridge node.
>
> [1] https://lore.kernel.org/linux-pci/20241211192014.GA3302752@bhelgaas/
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> .../devicetree/bindings/pci/qcom,pcie-common.yaml | 18 ++++++++++++++++++
> .../devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 17 +++++++++++++----
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
> index 0480c58f7d998adbac4c6de20cdaec945b3bab21..16e9acba1559b457da8a8a9dda4a22b226808f86 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
> @@ -85,6 +85,24 @@ properties:
> opp-table:
> type: object
>
> +patternProperties:
> + "^pcie@":
> + type: object
> + $ref: /schemas/pci/pci-pci-bridge.yaml#
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + phys:
> + maxItems: 1
> +
> + wake-gpios:
> + description: GPIO controlled connection to WAKE# signal
> + maxItems: 1
> +
> + unevaluatedProperties: false
Please mark old properties as deprecated.
> +
> required:
> - reg
> - reg-names
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> index 76cb9fbfd476fb0412217c68bd8db44a51c7d236..beb092f53019c31861460570cd2142506e05d8ef 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> @@ -162,9 +162,6 @@ examples:
> iommu-map = <0x0 &apps_smmu 0x1c80 0x1>,
> <0x100 &apps_smmu 0x1c81 0x1>;
>
> - phys = <&pcie1_phy>;
> - phy-names = "pciephy";
> -
> pinctrl-names = "default";
> pinctrl-0 = <&pcie1_clkreq_n>;
>
> @@ -173,7 +170,19 @@ examples:
> resets = <&gcc GCC_PCIE_1_BCR>;
> reset-names = "pci";
>
> - perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> vddpe-3v3-supply = <&pp3300_ssd>;
> + pcie1_port0: pcie@0 {
> + device_type = "pci";
The rest of the file uses 4 spaces to indent the next level. Any reason
for breaking this custom?
> + reg = <0x0 0x0 0x0 0x0 0x0>;
> + bus-range = <0x01 0xff>;
> +
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges;
> + phys = <&pcie1_phy>;
> +
> + reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> + };
> +
Drop extra empty liines
> };
> };
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
On 4/14/25 10:04, Dmitry Baryshkov wrote:
> On Mon, Apr 14, 2025 at 11:09:12AM +0530, Krishna Chaitanya Chundru wrote:
>> Move the phy, phy-names, wake-gpio's to the pcie root port node instead of
>> the bridge node, as agreed upon in multiple places one instance is[1].
>>
>> Update the qcom,pcie-common.yaml to include the phy, phy-names, and
>> wake-gpios properties in the root port node. There is already reset-gpio
>> defined for PERST# in pci-bus-common.yaml, start using that property
>> instead of perst-gpio.
>>
>> For backward compatibility, do not remove any existing properties in the
>> bridge node.
>>
>> [1] https://lore.kernel.org/linux-pci/20241211192014.GA3302752@bhelgaas/
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>> .../devicetree/bindings/pci/qcom,pcie-common.yaml | 18 ++++++++++++++++++
>> .../devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 17 +++++++++++++----
>> 2 files changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
>> index 0480c58f7d998adbac4c6de20cdaec945b3bab21..16e9acba1559b457da8a8a9dda4a22b226808f86 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
>> @@ -85,6 +85,24 @@ properties:
>> opp-table:
>> type: object
>>
>> +patternProperties:
>> + "^pcie@":
>> + type: object
>> + $ref: /schemas/pci/pci-pci-bridge.yaml#
>> +
>> + properties:
>> + reg:
>> + maxItems: 1
>> +
>> + phys:
>> + maxItems: 1
>> +
>> + wake-gpios:
>> + description: GPIO controlled connection to WAKE# signal
>> + maxItems: 1
>> +
>> + unevaluatedProperties: false
>
> Please mark old properties as deprecated.
Since this is a trivial change, just moving two properties, I don't see
why it makes sense to deprecate -- just remove the old properties, and
move over all the platforms at once.
>
>> +
>> required:
>> - reg
>> - reg-names
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
>> index 76cb9fbfd476fb0412217c68bd8db44a51c7d236..beb092f53019c31861460570cd2142506e05d8ef 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
>> @@ -162,9 +162,6 @@ examples:
>> iommu-map = <0x0 &apps_smmu 0x1c80 0x1>,
>> <0x100 &apps_smmu 0x1c81 0x1>;
>>
>> - phys = <&pcie1_phy>;
>> - phy-names = "pciephy";
>> -
>> pinctrl-names = "default";
>> pinctrl-0 = <&pcie1_clkreq_n>;
>>
>> @@ -173,7 +170,19 @@ examples:
>> resets = <&gcc GCC_PCIE_1_BCR>;
>> reset-names = "pci";
>>
>> - perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>> vddpe-3v3-supply = <&pp3300_ssd>;
>> + pcie1_port0: pcie@0 {
>> + device_type = "pci";
>
> The rest of the file uses 4 spaces to indent the next level. Any reason
> for breaking this custom?
>
>> + reg = <0x0 0x0 0x0 0x0 0x0>;
>> + bus-range = <0x01 0xff>;
>> +
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> + ranges;
>> + phys = <&pcie1_phy>;
>> +
>> + reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>> + };
>> +
>
> Drop extra empty liines
>
>> };
>> };
>>
>> --
>> 2.34.1
>>
>
--
Caleb (they/them)
On Mon, Apr 14, 2025 at 02:50:19PM +0200, Caleb Connolly wrote: > > > On 4/14/25 10:04, Dmitry Baryshkov wrote: > > On Mon, Apr 14, 2025 at 11:09:12AM +0530, Krishna Chaitanya Chundru wrote: > > > Move the phy, phy-names, wake-gpio's to the pcie root port node instead of > > > the bridge node, as agreed upon in multiple places one instance is[1]. > > > > > > Update the qcom,pcie-common.yaml to include the phy, phy-names, and > > > wake-gpios properties in the root port node. There is already reset-gpio > > > defined for PERST# in pci-bus-common.yaml, start using that property > > > instead of perst-gpio. > > > > > > For backward compatibility, do not remove any existing properties in the > > > bridge node. > > > > > > [1] https://lore.kernel.org/linux-pci/20241211192014.GA3302752@bhelgaas/ > > > > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> > > > --- > > > .../devicetree/bindings/pci/qcom,pcie-common.yaml | 18 ++++++++++++++++++ > > > .../devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 17 +++++++++++++---- > > > 2 files changed, 31 insertions(+), 4 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml > > > index 0480c58f7d998adbac4c6de20cdaec945b3bab21..16e9acba1559b457da8a8a9dda4a22b226808f86 100644 > > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml > > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml > > > @@ -85,6 +85,24 @@ properties: > > > opp-table: > > > type: object > > > +patternProperties: > > > + "^pcie@": > > > + type: object > > > + $ref: /schemas/pci/pci-pci-bridge.yaml# > > > + > > > + properties: > > > + reg: > > > + maxItems: 1 > > > + > > > + phys: > > > + maxItems: 1 > > > + > > > + wake-gpios: > > > + description: GPIO controlled connection to WAKE# signal > > > + maxItems: 1 > > > + > > > + unevaluatedProperties: false > > > > Please mark old properties as deprecated. > > Since this is a trivial change, just moving two properties, I don't see why > it makes sense to deprecate -- just remove the old properties, and move over > all the platforms at once. > This will be an ABI break. You should not remove properties all of a sudden without first deprecating them (even if you convert all upstream DTS at once). ABI is for older DTS also. - Mani -- மணிவண்ணன் சதாசிவம்
On Tue, Apr 15, 2025 at 01:11:35PM +0530, Manivannan Sadhasivam wrote: > On Mon, Apr 14, 2025 at 02:50:19PM +0200, Caleb Connolly wrote: > > > > > > On 4/14/25 10:04, Dmitry Baryshkov wrote: > > > On Mon, Apr 14, 2025 at 11:09:12AM +0530, Krishna Chaitanya Chundru wrote: > > > > Move the phy, phy-names, wake-gpio's to the pcie root port node instead of > > > > the bridge node, as agreed upon in multiple places one instance is[1]. > > > > > > > > Update the qcom,pcie-common.yaml to include the phy, phy-names, and > > > > wake-gpios properties in the root port node. There is already reset-gpio > > > > defined for PERST# in pci-bus-common.yaml, start using that property > > > > instead of perst-gpio. > > > > > > > > For backward compatibility, do not remove any existing properties in the > > > > bridge node. > > > > > > > > [1] https://lore.kernel.org/linux-pci/20241211192014.GA3302752@bhelgaas/ > > > > > > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> > > > > --- > > > > .../devicetree/bindings/pci/qcom,pcie-common.yaml | 18 ++++++++++++++++++ > > > > .../devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 17 +++++++++++++---- > > > > 2 files changed, 31 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml > > > > index 0480c58f7d998adbac4c6de20cdaec945b3bab21..16e9acba1559b457da8a8a9dda4a22b226808f86 100644 > > > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml > > > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml > > > > @@ -85,6 +85,24 @@ properties: > > > > opp-table: > > > > type: object > > > > +patternProperties: > > > > + "^pcie@": > > > > + type: object > > > > + $ref: /schemas/pci/pci-pci-bridge.yaml# > > > > + > > > > + properties: > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + phys: > > > > + maxItems: 1 > > > > + > > > > + wake-gpios: > > > > + description: GPIO controlled connection to WAKE# signal > > > > + maxItems: 1 > > > > + > > > > + unevaluatedProperties: false > > > > > > Please mark old properties as deprecated. > > > > Since this is a trivial change, just moving two properties, I don't see why > > it makes sense to deprecate -- just remove the old properties, and move over > > all the platforms at once. > > > > This will be an ABI break. You should not remove properties all of a sudden > without first deprecating them (even if you convert all upstream DTS at once). > ABI is for older DTS also. +1 Rob
© 2016 - 2025 Red Hat, Inc.