[v3,1/2] dt-bindings: PCI: qcom: Add SM8550 compatible
Commit Message
Add the SM8550 platform to the binding.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
The v2 was here:
https://lore.kernel.org/all/20230118111704.3553542-1-abel.vesa@linaro.org/
Changes since v2:
* dropped the pipe from clock-names
* removed the pcie instance number from aggre clock-names comment
* renamed aggre clock-names to noc_aggr
* dropped the _pcie infix from cnoc_pcie_sf_axi
* renamed pcie_1_link_down_reset to simply link_down
* added enable-gpios back, since pcie1 node will use it
Changes since v1:
* Switched to single compatible for both PCIes (qcom,pcie-sm8550)
* dropped enable-gpios property
* dropped interconnects related properties, the power-domains
* properties
and resets related properties the sm8550 specific allOf:if:then
* dropped pipe_mux, phy_pipe and ref clocks from the sm8550 specific
allOf:if:then clock-names array and decreased the minItems and
maxItems for clocks property accordingly
* added "minItems: 1" to interconnects, since sm8550 pcie uses just one,
same for interconnect-names
.../devicetree/bindings/pci/qcom,pcie.yaml | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
Comments
On 19/01/2023 12:24, Abel Vesa wrote:
> Add the SM8550 platform to the binding.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>
> The v2 was here:
> https://lore.kernel.org/all/20230118111704.3553542-1-abel.vesa@linaro.org/
>
> Changes since v2:
> * dropped the pipe from clock-names
> * removed the pcie instance number from aggre clock-names comment
> * renamed aggre clock-names to noc_aggr
> * dropped the _pcie infix from cnoc_pcie_sf_axi
> * renamed pcie_1_link_down_reset to simply link_down
> * added enable-gpios back, since pcie1 node will use it
>
> Changes since v1:
> * Switched to single compatible for both PCIes (qcom,pcie-sm8550)
> * dropped enable-gpios property
> * dropped interconnects related properties, the power-domains
> * properties
> and resets related properties the sm8550 specific allOf:if:then
> * dropped pipe_mux, phy_pipe and ref clocks from the sm8550 specific
> allOf:if:then clock-names array and decreased the minItems and
> maxItems for clocks property accordingly
> * added "minItems: 1" to interconnects, since sm8550 pcie uses just one,
> same for interconnect-names
>
>
> .../devicetree/bindings/pci/qcom,pcie.yaml | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a5859bb3dc28..93e86dfdd6fe 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -34,6 +34,7 @@ properties:
> - qcom,pcie-sm8250
> - qcom,pcie-sm8450-pcie0
> - qcom,pcie-sm8450-pcie1
> + - qcom,pcie-sm8550
> - qcom,pcie-ipq6018
>
> reg:
> @@ -65,9 +66,11 @@ properties:
> dma-coherent: true
>
> interconnects:
> + minItems: 1
> maxItems: 2
1. Why do you skip cpu-pcie interconnect on SM8550?
2. This should not be allowed on other variants.
>
> interconnect-names:
> + minItems: 1
> items:
> - const: pcie-mem
> - const: cpu-pcie
> @@ -102,6 +105,10 @@ properties:
> power-domains:
> maxItems: 1
>
> + enable-gpios:
> + description: GPIO controlled connection to ENABLE# signal
> + maxItems: 1
> +
> perst-gpios:
> description: GPIO controlled connection to PERST# signal
> maxItems: 1
> @@ -197,6 +204,7 @@ allOf:
> - qcom,pcie-sm8250
> - qcom,pcie-sm8450-pcie0
> - qcom,pcie-sm8450-pcie1
> + - qcom,pcie-sm8550
> then:
> properties:
> reg:
> @@ -611,6 +619,41 @@ allOf:
> items:
> - const: pci # PCIe core reset
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,pcie-sm8550
> + then:
> + properties:
> + clocks:
> + minItems: 7
> + maxItems: 8
> + clock-names:
> + minItems: 7
> + items:
> + - const: aux # Auxiliary clock
> + - const: cfg # Configuration clock
> + - const: bus_master # Master AXI clock
> + - const: bus_slave # Slave AXI clock
> + - const: slave_q2a # Slave Q2A clock
> + - const: ddrss_sf_tbu # PCIe SF TBU clock
> + - const: noc_aggr # Aggre NoC PCIe AXI clock
> + - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock
> + iommus:
> + maxItems: 1
> + iommu-map:
> + maxItems: 2
1. Don't define new properties in allOf. It makes the binding
unmaintainable.
2. Why only SM8550?
> + resets:
> + minItems: 1
Why second reset is optional?
> + maxItems: 2
> + reset-names:
> + minItems: 1
> + items:
> + - const: pci # PCIe core reset
> + - const: link_down # PCIe link down reset
> +
> - if:
> properties:
> compatible:
> @@ -694,6 +737,7 @@ allOf:
> - qcom,pcie-sm8250
> - qcom,pcie-sm8450-pcie0
> - qcom,pcie-sm8450-pcie1
> + - qcom,pcie-sm8550
> then:
> oneOf:
> - properties:
Best regards,
Krzysztof
On 23-01-20 09:37:32, Krzysztof Kozlowski wrote:
> On 19/01/2023 12:24, Abel Vesa wrote:
> > Add the SM8550 platform to the binding.
> >
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> >
> > The v2 was here:
> > https://lore.kernel.org/all/20230118111704.3553542-1-abel.vesa@linaro.org/
> >
> > Changes since v2:
> > * dropped the pipe from clock-names
> > * removed the pcie instance number from aggre clock-names comment
> > * renamed aggre clock-names to noc_aggr
> > * dropped the _pcie infix from cnoc_pcie_sf_axi
> > * renamed pcie_1_link_down_reset to simply link_down
> > * added enable-gpios back, since pcie1 node will use it
> >
> > Changes since v1:
> > * Switched to single compatible for both PCIes (qcom,pcie-sm8550)
> > * dropped enable-gpios property
> > * dropped interconnects related properties, the power-domains
> > * properties
> > and resets related properties the sm8550 specific allOf:if:then
> > * dropped pipe_mux, phy_pipe and ref clocks from the sm8550 specific
> > allOf:if:then clock-names array and decreased the minItems and
> > maxItems for clocks property accordingly
> > * added "minItems: 1" to interconnects, since sm8550 pcie uses just one,
> > same for interconnect-names
> >
> >
> > .../devicetree/bindings/pci/qcom,pcie.yaml | 44 +++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index a5859bb3dc28..93e86dfdd6fe 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -34,6 +34,7 @@ properties:
> > - qcom,pcie-sm8250
> > - qcom,pcie-sm8450-pcie0
> > - qcom,pcie-sm8450-pcie1
> > + - qcom,pcie-sm8550
> > - qcom,pcie-ipq6018
> >
> > reg:
> > @@ -65,9 +66,11 @@ properties:
> > dma-coherent: true
> >
> > interconnects:
> > + minItems: 1
> > maxItems: 2
>
> 1. Why do you skip cpu-pcie interconnect on SM8550?
> 2. This should not be allowed on other variants.
That is a good point. Will add the cpu-pcie in v5.
>
> >
> > interconnect-names:
> > + minItems: 1
> > items:
> > - const: pcie-mem
> > - const: cpu-pcie
> > @@ -102,6 +105,10 @@ properties:
> > power-domains:
> > maxItems: 1
> >
> > + enable-gpios:
> > + description: GPIO controlled connection to ENABLE# signal
> > + maxItems: 1
> > +
> > perst-gpios:
> > description: GPIO controlled connection to PERST# signal
> > maxItems: 1
> > @@ -197,6 +204,7 @@ allOf:
> > - qcom,pcie-sm8250
> > - qcom,pcie-sm8450-pcie0
> > - qcom,pcie-sm8450-pcie1
> > + - qcom,pcie-sm8550
> > then:
> > properties:
> > reg:
> > @@ -611,6 +619,41 @@ allOf:
> > items:
> > - const: pci # PCIe core reset
> >
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - qcom,pcie-sm8550
> > + then:
> > + properties:
> > + clocks:
> > + minItems: 7
> > + maxItems: 8
> > + clock-names:
> > + minItems: 7
> > + items:
> > + - const: aux # Auxiliary clock
> > + - const: cfg # Configuration clock
> > + - const: bus_master # Master AXI clock
> > + - const: bus_slave # Slave AXI clock
> > + - const: slave_q2a # Slave Q2A clock
> > + - const: ddrss_sf_tbu # PCIe SF TBU clock
> > + - const: noc_aggr # Aggre NoC PCIe AXI clock
> > + - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock
> > + iommus:
> > + maxItems: 1
> > + iommu-map:
> > + maxItems: 2
>
> 1. Don't define new properties in allOf. It makes the binding
> unmaintainable.
>
> 2. Why only SM8550?
Good point again. Will make both iommus and iommu-map properties global
as even SM8450 has them.
>
> > + resets:
> > + minItems: 1
>
> Why second reset is optional?
link_down reset is needed only by g4x2 pcie, AFAICT.
>
> > + maxItems: 2
> > + reset-names:
> > + minItems: 1
> > + items:
> > + - const: pci # PCIe core reset
> > + - const: link_down # PCIe link down reset
> > +
> > - if:
> > properties:
> > compatible:
> > @@ -694,6 +737,7 @@ allOf:
> > - qcom,pcie-sm8250
> > - qcom,pcie-sm8450-pcie0
> > - qcom,pcie-sm8450-pcie1
> > + - qcom,pcie-sm8550
> > then:
> > oneOf:
> > - properties:
>
> Best regards,
> Krzysztof
>
On 23/01/2023 17:40, Abel Vesa wrote:
>
>>
>>> + resets:
>>> + minItems: 1
>>
>> Why second reset is optional?
>
> link_down reset is needed only by g4x2 pcie, AFAICT.
OK
Best regards,
Krzysztof
@@ -34,6 +34,7 @@ properties:
- qcom,pcie-sm8250
- qcom,pcie-sm8450-pcie0
- qcom,pcie-sm8450-pcie1
+ - qcom,pcie-sm8550
- qcom,pcie-ipq6018
reg:
@@ -65,9 +66,11 @@ properties:
dma-coherent: true
interconnects:
+ minItems: 1
maxItems: 2
interconnect-names:
+ minItems: 1
items:
- const: pcie-mem
- const: cpu-pcie
@@ -102,6 +105,10 @@ properties:
power-domains:
maxItems: 1
+ enable-gpios:
+ description: GPIO controlled connection to ENABLE# signal
+ maxItems: 1
+
perst-gpios:
description: GPIO controlled connection to PERST# signal
maxItems: 1
@@ -197,6 +204,7 @@ allOf:
- qcom,pcie-sm8250
- qcom,pcie-sm8450-pcie0
- qcom,pcie-sm8450-pcie1
+ - qcom,pcie-sm8550
then:
properties:
reg:
@@ -611,6 +619,41 @@ allOf:
items:
- const: pci # PCIe core reset
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,pcie-sm8550
+ then:
+ properties:
+ clocks:
+ minItems: 7
+ maxItems: 8
+ clock-names:
+ minItems: 7
+ items:
+ - const: aux # Auxiliary clock
+ - const: cfg # Configuration clock
+ - const: bus_master # Master AXI clock
+ - const: bus_slave # Slave AXI clock
+ - const: slave_q2a # Slave Q2A clock
+ - const: ddrss_sf_tbu # PCIe SF TBU clock
+ - const: noc_aggr # Aggre NoC PCIe AXI clock
+ - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock
+ iommus:
+ maxItems: 1
+ iommu-map:
+ maxItems: 2
+ resets:
+ minItems: 1
+ maxItems: 2
+ reset-names:
+ minItems: 1
+ items:
+ - const: pci # PCIe core reset
+ - const: link_down # PCIe link down reset
+
- if:
properties:
compatible:
@@ -694,6 +737,7 @@ allOf:
- qcom,pcie-sm8250
- qcom,pcie-sm8450-pcie0
- qcom,pcie-sm8450-pcie1
+ - qcom,pcie-sm8550
then:
oneOf:
- properties: