| Message ID | 20220915133742.115218-6-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dt-bindings: display/msm: rework MDSS and DPU bindings | expand |
On 15/09/2022 15:37, Dmitry Baryshkov wrote: > Move properties common to all MDSS DT nodes to the mdss-common.yaml. > > This extends qcom,msm8998-mdss schema to allow interconnect nodes, which > will be added later, once msm8998 gains interconnect support. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- (...) > - "#interrupt-cells": > - const: 1 > - > iommus: > - items: > - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 > - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port1 > - > - ranges: true > + maxItems: 2 > > interconnects: > - items: > - - description: Interconnect path from mdp0 port to the data bus > - - description: Interconnect path from mdp1 port to the data bus > + maxItems: 2 I think this is not equivalent now, because you have in total minItems:1 and maxItems:2, while in past minItems was 2. The same might apply to iommus. clocks look good. Best regards, Krzysztof
On Thu, 22 Sept 2022 at 10:05, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 15/09/2022 15:37, Dmitry Baryshkov wrote: > > Move properties common to all MDSS DT nodes to the mdss-common.yaml. > > > > This extends qcom,msm8998-mdss schema to allow interconnect nodes, which > > will be added later, once msm8998 gains interconnect support. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > .../bindings/display/msm/dpu-msm8998.yaml | 41 +-------- > > .../bindings/display/msm/dpu-qcm2290.yaml | 51 ++---------- > > .../bindings/display/msm/dpu-sc7180.yaml | 50 ++--------- > > .../bindings/display/msm/dpu-sc7280.yaml | 50 ++--------- > > .../bindings/display/msm/dpu-sdm845.yaml | 54 ++---------- > > .../bindings/display/msm/mdss-common.yaml | 83 +++++++++++++++++++ > > 6 files changed, 111 insertions(+), 218 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/display/msm/mdss-common.yaml > > > > diff --git a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml > > index 200eeace1c71..67791dbc3b5d 100644 > > --- a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml > > +++ b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml > > @@ -14,20 +14,13 @@ description: | > > sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree > > bindings of MDSS and DPU are mentioned for MSM8998 target. > > > > missing allOf Rob asked to remove this while reviewing v6 ([1]). And indeed the allOf's around a single $ref do not seem to be necessary > > > +$ref: /schemas/display/msm/mdss-common.yaml# > > + > > properties: [1] https://lore.kernel.org/dri-devel/20220907195904.GA98468-robh@kernel.org/
On 22/09/2022 14:43, Krzysztof Kozlowski wrote: > On 22/09/2022 09:53, Dmitry Baryshkov wrote: >> On Thu, 22 Sept 2022 at 10:05, Krzysztof Kozlowski >> <krzysztof.kozlowski@linaro.org> wrote: >>> >>> On 15/09/2022 15:37, Dmitry Baryshkov wrote: >>>> Move properties common to all MDSS DT nodes to the mdss-common.yaml. >>>> >>>> This extends qcom,msm8998-mdss schema to allow interconnect nodes, which >>>> will be added later, once msm8998 gains interconnect support. >>>> >>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>> --- >>>> .../bindings/display/msm/dpu-msm8998.yaml | 41 +-------- >>>> .../bindings/display/msm/dpu-qcm2290.yaml | 51 ++---------- >>>> .../bindings/display/msm/dpu-sc7180.yaml | 50 ++--------- >>>> .../bindings/display/msm/dpu-sc7280.yaml | 50 ++--------- >>>> .../bindings/display/msm/dpu-sdm845.yaml | 54 ++---------- >>>> .../bindings/display/msm/mdss-common.yaml | 83 +++++++++++++++++++ >>>> 6 files changed, 111 insertions(+), 218 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/display/msm/mdss-common.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml >>>> index 200eeace1c71..67791dbc3b5d 100644 >>>> --- a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml >>>> +++ b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml >>>> @@ -14,20 +14,13 @@ description: | >>>> sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree >>>> bindings of MDSS and DPU are mentioned for MSM8998 target. >>>> >>> >>> missing allOf >> >> Rob asked to remove this while reviewing v6 ([1]). And indeed the >> allOf's around a single $ref do not seem to be necessary > > He commented on one of properties, not top-level, maybe it is different > case for dtschema. In the past it was required, so are you sure > something changed in dtschema? I do not know if something has changed or not. But judging from the fact that unevaluatedProperties:false do not generate any warnings, the referenced schema is processed and applied. > > Best regards, > Krzysztof >
On 22/09/2022 13:46, Dmitry Baryshkov wrote: >>> - ranges: true >>> + maxItems: 2 >>> >>> interconnects: >>> - items: >>> - - description: Interconnect path from mdp0 port to the data bus >>> - - description: Interconnect path from mdp1 port to the data bus >>> + maxItems: 2 >> >> I think this is not equivalent now, because you have in total minItems:1 >> and maxItems:2, while in past minItems was 2. > > This means that I should have minItems:2, maxItems:2, which, if I got it > right, is frowned upon. Let me doublecheck though if it works as expected. It is frowned upon only if it is alone, because for missing minItems, maxItems implies minItems. Here you have minItems in other schema, so there is no such case Best regards, Krzysztof
On 22/09/2022 15:28, Krzysztof Kozlowski wrote: > On 22/09/2022 13:46, Dmitry Baryshkov wrote: >>>> - ranges: true >>>> + maxItems: 2 >>>> >>>> interconnects: >>>> - items: >>>> - - description: Interconnect path from mdp0 port to the data bus >>>> - - description: Interconnect path from mdp1 port to the data bus >>>> + maxItems: 2 >>> >>> I think this is not equivalent now, because you have in total minItems:1 >>> and maxItems:2, while in past minItems was 2. >> >> This means that I should have minItems:2, maxItems:2, which, if I got it >> right, is frowned upon. Let me doublecheck though if it works as expected. > > It is frowned upon only if it is alone, because for missing minItems, > maxItems implies minItems. Here you have minItems in other schema, so > there is no such case Well, I just checked, the schema will throw an error if I put a single interconnects or iommus entry. If I understand correctly these two clauses are evaluated separately. So, the dpu-common's clause tells minItems:1, maxItems:2. The platform schema file contains just maxItems:2, which implicitly adds minItems:2 to _this_ clause. Thus I think I'll leave this part as is. For the reference (with single-entry iommus and interconnects properties): /home/lumag/Projects/Qcomm/build-64/Documentation/devicetree/bindings/display/msm/dpu-sdm845.example.dtb: display-subsystem@ae00000: iommus: [[4294967295, 2176, 8]] is too short From schema: /home/lumag/Projects/Qcomm/kernel/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml /home/lumag/Projects/Qcomm/build-64/Documentation/devicetree/bindings/display/msm/dpu-sdm845.example.dtb: display-subsystem@ae00000: interconnects: [[4294967295, 1, 0, 4294967295, 1, 0]] is too short From schema: /home/lumag/Projects/Qcomm/kernel/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
On 23/09/2022 22:32, Dmitry Baryshkov wrote: > On 22/09/2022 15:28, Krzysztof Kozlowski wrote: >> On 22/09/2022 13:46, Dmitry Baryshkov wrote: >>>>> - ranges: true >>>>> + maxItems: 2 >>>>> >>>>> interconnects: >>>>> - items: >>>>> - - description: Interconnect path from mdp0 port to the data bus >>>>> - - description: Interconnect path from mdp1 port to the data bus >>>>> + maxItems: 2 >>>> >>>> I think this is not equivalent now, because you have in total minItems:1 >>>> and maxItems:2, while in past minItems was 2. >>> >>> This means that I should have minItems:2, maxItems:2, which, if I got it >>> right, is frowned upon. Let me doublecheck though if it works as expected. >> >> It is frowned upon only if it is alone, because for missing minItems, >> maxItems implies minItems. Here you have minItems in other schema, so >> there is no such case > > Well, I just checked, the schema will throw an error if I put a single > interconnects or iommus entry. If I understand correctly these two > clauses are evaluated separately. So, the dpu-common's clause tells > minItems:1, maxItems:2. The platform schema file contains just > maxItems:2, which implicitly adds minItems:2 to _this_ clause. > > Thus I think I'll leave this part as is. Thanks for checking. It's good then. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml index 200eeace1c71..67791dbc3b5d 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml @@ -14,20 +14,13 @@ description: | sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree bindings of MDSS and DPU are mentioned for MSM8998 target. +$ref: /schemas/display/msm/mdss-common.yaml# + properties: compatible: items: - const: qcom,msm8998-mdss - reg: - maxItems: 1 - - reg-names: - const: mdss - - power-domains: - maxItems: 1 - clocks: items: - description: Display AHB clock @@ -40,23 +33,8 @@ properties: - const: bus - const: core - interrupts: - maxItems: 1 - - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": - const: 1 - iommus: - items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - ranges: true + maxItems: 1 patternProperties: "^display-controller@[0-9a-f]+$": @@ -100,18 +78,7 @@ patternProperties: - const: core - const: vsync -required: - - compatible - - reg - - reg-names - - power-domains - - clocks - - interrupts - - interrupt-controller - - iommus - - ranges - -additionalProperties: false +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml b/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml index d5f1d16b13d3..42e676bdda4e 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml @@ -14,20 +14,13 @@ description: | sub-blocks like DPU display controller and DSI. Device tree bindings of MDSS and DPU are mentioned for QCM2290 target. +$ref: /schemas/display/msm/mdss-common.yaml# + properties: compatible: items: - const: qcom,qcm2290-mdss - reg: - maxItems: 1 - - reg-names: - const: mdss - - power-domains: - maxItems: 1 - clocks: items: - description: Display AHB clock from gcc @@ -40,35 +33,14 @@ properties: - const: bus - const: core - interrupts: - maxItems: 1 - - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": - const: 1 - iommus: - items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port1 - - ranges: true + maxItems: 2 interconnects: - items: - - description: Interconnect path specifying the port ids for data bus + maxItems: 1 interconnect-names: - const: mdp0-mem - - resets: - items: - - description: MDSS_CORE reset + maxItems: 1 patternProperties: "^display-controller@[0-9a-f]+$": @@ -108,18 +80,7 @@ patternProperties: - const: lut - const: vsync -required: - - compatible - - reg - - reg-names - - power-domains - - clocks - - interrupts - - interrupt-controller - - iommus - - ranges - -additionalProperties: false +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml index 2ac10664d79a..99d6bbd45faf 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml @@ -14,20 +14,13 @@ description: | sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree bindings of MDSS and DPU are mentioned for SC7180 target. +$ref: /schemas/display/msm/mdss-common.yaml# + properties: compatible: items: - const: qcom,sc7180-mdss - reg: - maxItems: 1 - - reg-names: - const: mdss - - power-domains: - maxItems: 1 - clocks: items: - description: Display AHB clock from gcc @@ -40,34 +33,14 @@ properties: - const: ahb - const: core - interrupts: - maxItems: 1 - - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": - const: 1 - iommus: - items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - ranges: true + maxItems: 1 interconnects: - items: - - description: Interconnect path specifying the port ids for data bus + maxItems: 1 interconnect-names: - const: mdp0-mem - - resets: - items: - - description: MDSS_CORE reset + maxItems: 1 patternProperties: "^display-controller@[0-9a-f]+$": @@ -109,18 +82,7 @@ patternProperties: - const: core - const: vsync -required: - - compatible - - reg - - reg-names - - power-domains - - clocks - - interrupts - - interrupt-controller - - iommus - - ranges - -additionalProperties: false +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml index 4ca7bc7f0185..01ff88c06c51 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml @@ -14,19 +14,12 @@ description: | sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree bindings of MDSS and DPU are mentioned for SC7280. +$ref: /schemas/display/msm/mdss-common.yaml# + properties: compatible: const: qcom,sc7280-mdss - reg: - maxItems: 1 - - reg-names: - const: mdss - - power-domains: - maxItems: 1 - clocks: items: - description: Display AHB clock from gcc @@ -39,34 +32,14 @@ properties: - const: ahb - const: core - interrupts: - maxItems: 1 - - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": - const: 1 - iommus: - items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - ranges: true + maxItems: 1 interconnects: - items: - - description: Interconnect path specifying the port ids for data bus + maxItems: 1 interconnect-names: - const: mdp0-mem - - resets: - items: - - description: MDSS_CORE reset + maxItems: 1 patternProperties: "^display-controller@[0-9a-f]+$": @@ -107,18 +80,7 @@ patternProperties: - const: core - const: vsync -required: - - compatible - - reg - - reg-names - - power-domains - - clocks - - interrupts - - interrupt-controller - - iommus - - ranges - -additionalProperties: false +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml index de193ca11265..ae649bb6aa81 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml @@ -14,20 +14,13 @@ description: | sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree bindings of MDSS and DPU are mentioned for SDM845 target. +$ref: /schemas/display/msm/mdss-common.yaml# + properties: compatible: items: - const: qcom,sdm845-mdss - reg: - maxItems: 1 - - reg-names: - const: mdss - - power-domains: - maxItems: 1 - clocks: items: - description: Display AHB clock from gcc @@ -38,38 +31,14 @@ properties: - const: iface - const: core - interrupts: - maxItems: 1 - - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": - const: 1 - iommus: - items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port1 - - ranges: true + maxItems: 2 interconnects: - items: - - description: Interconnect path from mdp0 port to the data bus - - description: Interconnect path from mdp1 port to the data bus + maxItems: 2 interconnect-names: - items: - - const: mdp0-mem - - const: mdp1-mem - - resets: - items: - - description: MDSS_CORE reset + maxItems: 2 patternProperties: "^display-controller@[0-9a-f]+$": @@ -109,18 +78,7 @@ patternProperties: - const: core - const: vsync -required: - - compatible - - reg - - reg-names - - power-domains - - clocks - - interrupts - - interrupt-controller - - iommus - - ranges - -additionalProperties: false +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml new file mode 100644 index 000000000000..2a476bd0215e --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/mdss-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display MDSS common properties + +maintainers: + - Krishna Manikandan <quic_mkrishn@quicinc.com> + - Dmitry Baryshkov <dmitry.baryshkov@linaro.org> + - Rob Clark <robdclark@gmail.com> + +description: + Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates + sub-blocks like DPU display controller, DSI and DP interfaces etc. + +properties: + reg: + maxItems: 1 + + reg-names: + const: mdss + + power-domains: + maxItems: 1 + + clocks: + minItems: 2 + maxItems: 3 + + clock-names: + minItems: 2 + maxItems: 3 + + interrupts: + maxItems: 1 + + interrupt-controller: true + + "#address-cells": true + + "#size-cells": true + + "#interrupt-cells": + const: 1 + + iommus: + minItems: 1 + items: + - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 + - description: Phandle to apps_smmu node with SID mask for Hard-Fail port1 + + ranges: true + + interconnects: + minItems: 1 + items: + - description: Interconnect path from mdp0 (or a single mdp) port to the data bus + - description: Interconnect path from mdp1 port to the data bus + + interconnect-names: + minItems: 1 + items: + - const: mdp0-mem + - const: mdp1-mem + + resets: + items: + - description: MDSS_CORE reset + +required: + - compatible + - reg + - reg-names + - power-domains + - clocks + - interrupts + - interrupt-controller + - iommus + - ranges + +additionalProperties: true
Move properties common to all MDSS DT nodes to the mdss-common.yaml. This extends qcom,msm8998-mdss schema to allow interconnect nodes, which will be added later, once msm8998 gains interconnect support. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- .../bindings/display/msm/dpu-msm8998.yaml | 41 +-------- .../bindings/display/msm/dpu-qcm2290.yaml | 51 ++---------- .../bindings/display/msm/dpu-sc7180.yaml | 50 ++--------- .../bindings/display/msm/dpu-sc7280.yaml | 50 ++--------- .../bindings/display/msm/dpu-sdm845.yaml | 54 ++---------- .../bindings/display/msm/mdss-common.yaml | 83 +++++++++++++++++++ 6 files changed, 111 insertions(+), 218 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/mdss-common.yaml