Add the missing SC8280XP/SA8540P "pcie-mem" and "cpu-pcie" interconnect
paths to the bindings.
Fixes: 76d777ae045e ("dt-bindings: PCI: qcom: Add SC8280XP to binding")
Fixes: 76c4207f4085 ("dt-bindings: PCI: qcom: Add SA8540P to binding")
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
.../devicetree/bindings/pci/qcom,pcie.yaml | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 22a2aac4c23f..a55434f95edd 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -62,6 +62,12 @@ properties:
minItems: 3
maxItems: 12
+ interconnects:
+ maxItems: 2
+
+ interconnect-names:
+ maxItems: 2
+
resets:
minItems: 1
maxItems: 12
@@ -629,6 +635,25 @@ allOf:
items:
- const: pci # PCIe core reset
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,pcie-sa8540p
+ - qcom,pcie-sc8280xp
+ then:
+ properties:
+ interconnects:
+ maxItems: 2
+ interconnect-names:
+ items:
+ - const: pcie-mem
+ - const: cpu-pcie
+ required:
+ - interconnects
+ - interconnect-names
+
- if:
not:
properties:
--
2.37.3On 17/10/2022 07:24, Johan Hovold wrote:
> Add the missing SC8280XP/SA8540P "pcie-mem" and "cpu-pcie" interconnect
> paths to the bindings.
>
> Fixes: 76d777ae045e ("dt-bindings: PCI: qcom: Add SC8280XP to binding")
> Fixes: 76c4207f4085 ("dt-bindings: PCI: qcom: Add SA8540P to binding")
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
> .../devicetree/bindings/pci/qcom,pcie.yaml | 25 +++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 22a2aac4c23f..a55434f95edd 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -62,6 +62,12 @@ properties:
> minItems: 3
> maxItems: 12
>
> + interconnects:
> + maxItems: 2
> +
> + interconnect-names:
> + maxItems: 2
> +
> resets:
> minItems: 1
> maxItems: 12
> @@ -629,6 +635,25 @@ allOf:
> items:
> - const: pci # PCIe core reset
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,pcie-sa8540p
> + - qcom,pcie-sc8280xp
> + then:
> + properties:
> + interconnects:
> + maxItems: 2
No need for this.
> + interconnect-names:
> + items:
> + - const: pcie-mem
> + - const: cpu-pcie
> + required:
> + - interconnects
> + - interconnect-names
else:
??
Otherwise, you allow any names for other variants.
Best regards,
Krzysztof
On Wed, Oct 19, 2022 at 10:37:31AM -0400, Krzysztof Kozlowski wrote:
> On 17/10/2022 07:24, Johan Hovold wrote:
> > Add the missing SC8280XP/SA8540P "pcie-mem" and "cpu-pcie" interconnect
> > paths to the bindings.
> >
> > Fixes: 76d777ae045e ("dt-bindings: PCI: qcom: Add SC8280XP to binding")
> > Fixes: 76c4207f4085 ("dt-bindings: PCI: qcom: Add SA8540P to binding")
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> > .../devicetree/bindings/pci/qcom,pcie.yaml | 25 +++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index 22a2aac4c23f..a55434f95edd 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -62,6 +62,12 @@ properties:
> > minItems: 3
> > maxItems: 12
> >
> > + interconnects:
> > + maxItems: 2
> > +
> > + interconnect-names:
> > + maxItems: 2
> > +
> > resets:
> > minItems: 1
> > maxItems: 12
> > @@ -629,6 +635,25 @@ allOf:
> > items:
> > - const: pci # PCIe core reset
> >
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - qcom,pcie-sa8540p
> > + - qcom,pcie-sc8280xp
> > + then:
> > + properties:
> > + interconnects:
> > + maxItems: 2
>
> No need for this.
>
> > + interconnect-names:
> > + items:
> > + - const: pcie-mem
> > + - const: cpu-pcie
> > + required:
> > + - interconnects
> > + - interconnect-names
>
> else:
> ??
>
> Otherwise, you allow any names for other variants.
Are you suggesting something like moving the names to the common
constraints for now:
interconnects:
maxItems: 2
interconnect-names:
items:
- const: pcie-mem
- const: cpu-pcie
and then in the allOf:
- if:
properties:
compatible:
contains:
enum:
- qcom,pcie-sa8540p
- qcom,pcie-sc8280xp
then:
required:
- interconnects
- interconnect-names
else:
properties:
interconnects: false
interconnect-names: false
This way we'd catch anyone adding interconnects to a DTS without first
updating the bindings, but it also seems to go against the idea of
bindings fully describing the hardware by saying that no other platforms
have interconnects (when they actually do even if we don't describe it
just yet).
Or should we do the above but without the else clause to have some
constraints in place on the names at least?
Johan
On 20/10/2022 03:57, Johan Hovold wrote:
> On Wed, Oct 19, 2022 at 10:37:31AM -0400, Krzysztof Kozlowski wrote:
>> On 17/10/2022 07:24, Johan Hovold wrote:
>>> Add the missing SC8280XP/SA8540P "pcie-mem" and "cpu-pcie" interconnect
>>> paths to the bindings.
>>>
>>> Fixes: 76d777ae045e ("dt-bindings: PCI: qcom: Add SC8280XP to binding")
>>> Fixes: 76c4207f4085 ("dt-bindings: PCI: qcom: Add SA8540P to binding")
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>> ---
>>> .../devicetree/bindings/pci/qcom,pcie.yaml | 25 +++++++++++++++++++
>>> 1 file changed, 25 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> index 22a2aac4c23f..a55434f95edd 100644
>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> @@ -62,6 +62,12 @@ properties:
>>> minItems: 3
>>> maxItems: 12
>>>
>>> + interconnects:
>>> + maxItems: 2
>>> +
>>> + interconnect-names:
>>> + maxItems: 2
>>> +
>>> resets:
>>> minItems: 1
>>> maxItems: 12
>>> @@ -629,6 +635,25 @@ allOf:
>>> items:
>>> - const: pci # PCIe core reset
>>>
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - qcom,pcie-sa8540p
>>> + - qcom,pcie-sc8280xp
>>> + then:
>>> + properties:
>>> + interconnects:
>>> + maxItems: 2
>>
>> No need for this.
>>
>>> + interconnect-names:
>>> + items:
>>> + - const: pcie-mem
>>> + - const: cpu-pcie
>>> + required:
>>> + - interconnects
>>> + - interconnect-names
>>
>> else:
>> ??
>>
>> Otherwise, you allow any names for other variants.
>
> Are you suggesting something like moving the names to the common
> constraints for now:
>
> interconnects:
> maxItems: 2
>
> interconnect-names:
> items:
> - const: pcie-mem
> - const: cpu-pcie
>
> and then in the allOf:
>
> - if:
> properties:
> compatible:
> contains:
> enum:
> - qcom,pcie-sa8540p
> - qcom,pcie-sc8280xp
> then:
> required:
> - interconnects
> - interconnect-names
> else:
> properties:
> interconnects: false
> interconnect-names: false
>
> This way we'd catch anyone adding interconnects to a DTS without first
> updating the bindings, but it also seems to go against the idea of
> bindings fully describing the hardware by saying that no other platforms
> have interconnects (when they actually do even if we don't describe it
> just yet).
You can add a comment to the else like "TODO: Not described yet". I
would prefer to have specific but incomplete bindings, instead of loose
one which later might cause people adding whatever names they like.
> Or should we do the above but without the else clause to have some
> constraints in place on the names at least?
This would work as well if you think the names are applicable for other
devices.
Best regards,
Krzysztof
On Thu, Oct 20, 2022 at 08:29:02AM -0400, Krzysztof Kozlowski wrote:
> On 20/10/2022 03:57, Johan Hovold wrote:
> > On Wed, Oct 19, 2022 at 10:37:31AM -0400, Krzysztof Kozlowski wrote:
> >> On 17/10/2022 07:24, Johan Hovold wrote:
> >>> Add the missing SC8280XP/SA8540P "pcie-mem" and "cpu-pcie" interconnect
> >>> paths to the bindings.
> >>>
> >>> Fixes: 76d777ae045e ("dt-bindings: PCI: qcom: Add SC8280XP to binding")
> >>> Fixes: 76c4207f4085 ("dt-bindings: PCI: qcom: Add SA8540P to binding")
> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >>> ---
> >>> .../devicetree/bindings/pci/qcom,pcie.yaml | 25 +++++++++++++++++++
> >>> 1 file changed, 25 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>> index 22a2aac4c23f..a55434f95edd 100644
> >>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > Are you suggesting something like moving the names to the common
> > constraints for now:
> >
> > interconnects:
> > maxItems: 2
> >
> > interconnect-names:
> > items:
> > - const: pcie-mem
> > - const: cpu-pcie
> >
> > and then in the allOf:
> >
> > - if:
> > properties:
> > compatible:
> > contains:
> > enum:
> > - qcom,pcie-sa8540p
> > - qcom,pcie-sc8280xp
> > then:
> > required:
> > - interconnects
> > - interconnect-names
> > else:
> > properties:
> > interconnects: false
> > interconnect-names: false
> >
> > This way we'd catch anyone adding interconnects to a DTS without first
> > updating the bindings, but it also seems to go against the idea of
> > bindings fully describing the hardware by saying that no other platforms
> > have interconnects (when they actually do even if we don't describe it
> > just yet).
>
> You can add a comment to the else like "TODO: Not described yet". I
> would prefer to have specific but incomplete bindings, instead of loose
> one which later might cause people adding whatever names they like.
>
> > Or should we do the above but without the else clause to have some
> > constraints in place on the names at least?
>
> This would work as well if you think the names are applicable for other
> devices.
I think that's a reasonable assumption so I'll go with this alternative.
Thanks!
Johan
© 2016 - 2026 Red Hat, Inc.