| Message ID | 20221017112449.2146-2-johan+linaro@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: qcom: Add basic interconnect support | expand |
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. 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
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:
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(+)