diff mbox series

[v7,05/12] dt-bindings: display/msm: move common MDSS properties to mdss-common.yaml

Message ID 20220915133742.115218-6-dmitry.baryshkov@linaro.org
State Accepted
Commit 5a5c7b35f00f734b38f320ab95b0e571c1874e09
Headers show
Series None | expand

Commit Message

Dmitry Baryshkov Sept. 15, 2022, 1:37 p.m. UTC
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

Comments

Krzysztof Kozlowski Sept. 22, 2022, 7:04 a.m. UTC | #1
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
Dmitry Baryshkov Sept. 22, 2022, 7:53 a.m. UTC | #2
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/
Krzysztof Kozlowski Sept. 22, 2022, 11:43 a.m. UTC | #3
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?

Best regards,
Krzysztof
Dmitry Baryshkov Sept. 22, 2022, 11:46 a.m. UTC | #4
On 22/09/2022 10:04, Krzysztof Kozlowski 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>
>> ---
> 
> (...)
> 
>> -  "#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.

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.

> 
> The same might apply to iommus. clocks look good.
Krzysztof Kozlowski Sept. 22, 2022, 12:28 p.m. UTC | #5
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
Dmitry Baryshkov Sept. 23, 2022, 8:32 p.m. UTC | #6
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
diff mbox series

Patch

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