| Message ID | 20220422114841.1854138-4-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | dt-bindings: YAMLify pci/qcom,pcie schema | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 22/04/2022 13:48, Dmitry Baryshkov wrote: > Instead of specifying the enum of possible reg-names, specify them > explicitly. This allows us to specify which chipsets need the "atu" > regions, which do not. Also it clearly describes which platforms > enumerate PCIe cores using the dbi region and which use parf region for > that. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > .../devicetree/bindings/pci/qcom,pcie.yaml | 96 ++++++++++++++++--- > 1 file changed, 81 insertions(+), 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > index 7210057d1511..e78e63ea4b25 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > @@ -35,21 +35,6 @@ properties: > - qcom,pcie-ipq6018 > - const: snps,dw-pcie > > - reg: > - minItems: 4 > - maxItems: 5 This should stay. > - > - reg-names: > - minItems: 4 > - maxItems: 5 > - items: > - enum: > - - parf # Qualcomm specific registers > - - dbi # DesignWare PCIe registers > - - elbi # External local bus interface registers > - - config # PCIe configuration space > - - atu # ATU address space (optional) Move one of your lists for specific compatibles here and name last element optional (minItems: 4). You will need to fix the order of regs in DTS to match the one defined here. > - > interrupts: > maxItems: 1 > > @@ -108,6 +93,87 @@ required: > > allOf: > - $ref: /schemas/pci/pci-bus.yaml# > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,pcie-apq8064 > + - qcom,pcie-ipq4019 > + - qcom,pcie-ipq8064 > + - qcom,pcie-ipq8064v2 > + - qcom,pcie-ipq8074 > + - qcom,pcie-qcs404 > + then: > + properties: > + reg: > + minItems: 4 > + maxItems: 4 Only maxItems: 4 > + reg-names: > + items: > + - const: dbi # DesignWare PCIe registers > + - const: elbi # External local bus interface registers > + - const: parf # Qualcomm specific registers > + - const: config # PCIe configuration space No need for this, instead only maxItems:4 > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,pcie-ipq6018 > + then: > + properties: > + reg: > + minItems: 5 > + maxItems: 5 Only minItems:5 should be needed. > + reg-names: > + items: > + - const: dbi # DesignWare PCIe registers > + - const: elbi # External local bus interface registers > + - const: atu # ATU address space (optional) > + - const: parf # Qualcomm specific registers > + - const: config # PCIe configuration space This can be removed. All other cases should be merged with the ones here. Best regards, Krzysztof
On Fri, 22 Apr 2022 at 15:55, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 22/04/2022 13:48, Dmitry Baryshkov wrote: > > Instead of specifying the enum of possible reg-names, specify them > > explicitly. This allows us to specify which chipsets need the "atu" > > regions, which do not. Also it clearly describes which platforms > > enumerate PCIe cores using the dbi region and which use parf region for > > that. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > .../devicetree/bindings/pci/qcom,pcie.yaml | 96 ++++++++++++++++--- > > 1 file changed, 81 insertions(+), 15 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > > index 7210057d1511..e78e63ea4b25 100644 > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > > @@ -35,21 +35,6 @@ properties: > > - qcom,pcie-ipq6018 > > - const: snps,dw-pcie > > > > - reg: > > - minItems: 4 > > - maxItems: 5 > > This should stay. > > > - > > - reg-names: > > - minItems: 4 > > - maxItems: 5 > > - items: > > - enum: > > - - parf # Qualcomm specific registers > > - - dbi # DesignWare PCIe registers > > - - elbi # External local bus interface registers > > - - config # PCIe configuration space > > - - atu # ATU address space (optional) > > Move one of your lists for specific compatibles here and name last > element optional (minItems: 4). > > You will need to fix the order of regs in DTS to match the one defined here. I see your idea. I wanted to be explicit, which platforms need atu and which do not. You'd prefer not to. Let's probably drop this for now. The bindings proposed in patch 1 work for now. I will work on updating reg-names later. > > > - > > interrupts: > > maxItems: 1 > > > > @@ -108,6 +93,87 @@ required: > > > > allOf: > > - $ref: /schemas/pci/pci-bus.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - qcom,pcie-apq8064 > > + - qcom,pcie-ipq4019 > > + - qcom,pcie-ipq8064 > > + - qcom,pcie-ipq8064v2 > > + - qcom,pcie-ipq8074 > > + - qcom,pcie-qcs404 > > + then: > > + properties: > > + reg: > > + minItems: 4 > > + maxItems: 4 > > Only maxItems: 4 > > > + reg-names: > > + items: > > + - const: dbi # DesignWare PCIe registers > > + - const: elbi # External local bus interface registers > > + - const: parf # Qualcomm specific registers > > + - const: config # PCIe configuration space > > No need for this, instead only maxItems:4 > > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - qcom,pcie-ipq6018 > > + then: > > + properties: > > + reg: > > + minItems: 5 > > + maxItems: 5 > > Only minItems:5 should be needed. > > > + reg-names: > > + items: > > + - const: dbi # DesignWare PCIe registers > > + - const: elbi # External local bus interface registers > > + - const: atu # ATU address space (optional) > > + - const: parf # Qualcomm specific registers > > + - const: config # PCIe configuration space > > This can be removed. > > All other cases should be merged with the ones here. > > Best regards, > Krzysztof
On 22/04/2022 17:47, Dmitry Baryshkov wrote: > On Fri, 22 Apr 2022 at 15:55, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 22/04/2022 13:48, Dmitry Baryshkov wrote: >>> Instead of specifying the enum of possible reg-names, specify them >>> explicitly. This allows us to specify which chipsets need the "atu" >>> regions, which do not. Also it clearly describes which platforms >>> enumerate PCIe cores using the dbi region and which use parf region for >>> that. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> .../devicetree/bindings/pci/qcom,pcie.yaml | 96 ++++++++++++++++--- >>> 1 file changed, 81 insertions(+), 15 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>> index 7210057d1511..e78e63ea4b25 100644 >>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>> @@ -35,21 +35,6 @@ properties: >>> - qcom,pcie-ipq6018 >>> - const: snps,dw-pcie >>> >>> - reg: >>> - minItems: 4 >>> - maxItems: 5 >> >> This should stay. >> >>> - >>> - reg-names: >>> - minItems: 4 >>> - maxItems: 5 >>> - items: >>> - enum: >>> - - parf # Qualcomm specific registers >>> - - dbi # DesignWare PCIe registers >>> - - elbi # External local bus interface registers >>> - - config # PCIe configuration space >>> - - atu # ATU address space (optional) >> >> Move one of your lists for specific compatibles here and name last >> element optional (minItems: 4). >> >> You will need to fix the order of regs in DTS to match the one defined here. > > I see your idea. I wanted to be explicit, which platforms need atu and > which do not. You'd prefer not to. Opposite, I wish platforms to be specific, which need atu which not. However I wish the strictly defined, same order for everyone because it looks possible. > Let's probably drop this for now. The bindings proposed in patch 1 > work for now. I will work on updating reg-names later. Best regards, Krzysztof
On 22/04/2022 18:51, Krzysztof Kozlowski wrote: > On 22/04/2022 17:47, Dmitry Baryshkov wrote: >> On Fri, 22 Apr 2022 at 15:55, Krzysztof Kozlowski >> <krzysztof.kozlowski@linaro.org> wrote: >>> >>> On 22/04/2022 13:48, Dmitry Baryshkov wrote: >>>> Instead of specifying the enum of possible reg-names, specify them >>>> explicitly. This allows us to specify which chipsets need the "atu" >>>> regions, which do not. Also it clearly describes which platforms >>>> enumerate PCIe cores using the dbi region and which use parf region for >>>> that. >>>> >>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>> --- >>>> .../devicetree/bindings/pci/qcom,pcie.yaml | 96 ++++++++++++++++--- >>>> 1 file changed, 81 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>>> index 7210057d1511..e78e63ea4b25 100644 >>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>>> @@ -35,21 +35,6 @@ properties: >>>> - qcom,pcie-ipq6018 >>>> - const: snps,dw-pcie >>>> >>>> - reg: >>>> - minItems: 4 >>>> - maxItems: 5 >>> >>> This should stay. >>> >>>> - >>>> - reg-names: >>>> - minItems: 4 >>>> - maxItems: 5 >>>> - items: >>>> - enum: >>>> - - parf # Qualcomm specific registers >>>> - - dbi # DesignWare PCIe registers >>>> - - elbi # External local bus interface registers >>>> - - config # PCIe configuration space >>>> - - atu # ATU address space (optional) >>> >>> Move one of your lists for specific compatibles here and name last >>> element optional (minItems: 4). >>> >>> You will need to fix the order of regs in DTS to match the one defined here. >> >> I see your idea. I wanted to be explicit, which platforms need atu and >> which do not. You'd prefer not to. > > Opposite, I wish platforms to be specific, which need atu which not. > However I wish the strictly defined, same order for everyone because it > looks possible. Well, the same order is not possible, since for some devices the first, address-defining reg is "parf", for others it is "dbi". So, there will be two "families" of the devices. Unless we want to change the DT address of the unit. >> Let's probably drop this for now. The bindings proposed in patch 1 >> work for now. I will work on updating reg-names later. > > > Best regards, > Krzysztof
On 22/04/2022 21:09, Dmitry Baryshkov wrote: >> >> Opposite, I wish platforms to be specific, which need atu which not. >> However I wish the strictly defined, same order for everyone because it >> looks possible. > > Well, the same order is not possible, since for some devices the first, > address-defining reg is "parf", for others it is "dbi". So, there will > be two "families" of the devices. Unless we want to change the DT > address of the unit. I missed that, so my feedback is not correct. Better to not change the unit addresses. You can still leave the "reg" and "reg-names" with generic constraints in the main properties part, but the rest looks actually correct. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml index 7210057d1511..e78e63ea4b25 100644 --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml @@ -35,21 +35,6 @@ properties: - qcom,pcie-ipq6018 - const: snps,dw-pcie - reg: - minItems: 4 - maxItems: 5 - - reg-names: - minItems: 4 - maxItems: 5 - items: - enum: - - parf # Qualcomm specific registers - - dbi # DesignWare PCIe registers - - elbi # External local bus interface registers - - config # PCIe configuration space - - atu # ATU address space (optional) - interrupts: maxItems: 1 @@ -108,6 +93,87 @@ required: allOf: - $ref: /schemas/pci/pci-bus.yaml# + - if: + properties: + compatible: + contains: + enum: + - qcom,pcie-apq8064 + - qcom,pcie-ipq4019 + - qcom,pcie-ipq8064 + - qcom,pcie-ipq8064v2 + - qcom,pcie-ipq8074 + - qcom,pcie-qcs404 + then: + properties: + reg: + minItems: 4 + maxItems: 4 + reg-names: + items: + - const: dbi # DesignWare PCIe registers + - const: elbi # External local bus interface registers + - const: parf # Qualcomm specific registers + - const: config # PCIe configuration space + - if: + properties: + compatible: + contains: + enum: + - qcom,pcie-ipq6018 + then: + properties: + reg: + minItems: 5 + maxItems: 5 + reg-names: + items: + - const: dbi # DesignWare PCIe registers + - const: elbi # External local bus interface registers + - const: atu # ATU address space (optional) + - const: parf # Qualcomm specific registers + - const: config # PCIe configuration space + - if: + properties: + compatible: + contains: + enum: + - qcom,pcie-apq8084 + - qcom,pcie-msm8996 + - qcom,pcie-sdm845 + then: + properties: + reg: + minItems: 4 + maxItems: 4 + reg-names: + items: + - const: parf # Qualcomm specific registers + - const: dbi # DesignWare PCIe registers + - const: elbi # External local bus interface registers + - const: config # PCIe configuration space + - if: + properties: + compatible: + contains: + enum: + - qcom,pcie-sc7280 + - qcom,pcie-sc8180x + - qcom,pcie-sm8250 + - qcom,pcie-sm8450-pcie0 + - qcom,pcie-sm8450-pcie1 + then: + properties: + reg: + minItems: 5 + maxItems: 5 + reg-names: + items: + - const: parf # Qualcomm specific registers + - const: dbi # DesignWare PCIe registers + - const: elbi # External local bus interface registers + - const: atu # ATU address space (optional) + - const: config # PCIe configuration space - if: properties: compatible:
Instead of specifying the enum of possible reg-names, specify them explicitly. This allows us to specify which chipsets need the "atu" regions, which do not. Also it clearly describes which platforms enumerate PCIe cores using the dbi region and which use parf region for that. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- .../devicetree/bindings/pci/qcom,pcie.yaml | 96 ++++++++++++++++--- 1 file changed, 81 insertions(+), 15 deletions(-)