Re: [PATCH v2] dt-bindings: display: mediatek: Compatible list cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu, Mar 2 2023 at 05:08:30 PM +01:00:00, AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
Il 02/03/23 15:12, Yassine Oudjana ha scritto:
From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>

Several DT bindings of MediaTek display blocks make unnecessary use of
"oneOf" and "items". Remove them and replace them with enums where
necessary.

Signed-off-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
---
Changes since v1:
- Leave enums with 1 element as they are.

  .../bindings/display/mediatek/mediatek,ccorr.yaml   |  7 +++----
  .../bindings/display/mediatek/mediatek,color.yaml   | 10 ++++------
  .../bindings/display/mediatek/mediatek,dither.yaml  |  3 +--
  .../bindings/display/mediatek/mediatek,dsc.yaml     |  4 +---
  .../bindings/display/mediatek/mediatek,gamma.yaml   |  7 +++----
  .../bindings/display/mediatek/mediatek,merge.yaml   |  8 +++-----
  .../bindings/display/mediatek/mediatek,od.yaml      |  8 +++-----
  .../bindings/display/mediatek/mediatek,ovl-2l.yaml  |  7 +++----
.../bindings/display/mediatek/mediatek,ovl.yaml | 13 +++++--------
  .../display/mediatek/mediatek,postmask.yaml         |  3 +--
.../bindings/display/mediatek/mediatek,rdma.yaml | 13 +++++--------
  .../bindings/display/mediatek/mediatek,split.yaml   |  4 +---
  .../bindings/display/mediatek/mediatek,ufoe.yaml    |  4 +---
  .../bindings/display/mediatek/mediatek,wdma.yaml    |  4 +---
  14 files changed, 35 insertions(+), 60 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml
index b04820c95b22..dc22bd522523 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml
@@ -21,10 +21,9 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt8183-disp-ccorr
-      - items:
-          - const: mediatek,mt8192-disp-ccorr
+      - enum:
+          - mediatek,mt8183-disp-ccorr
+          - mediatek,mt8192-disp-ccorr

This change is ok.

        - items:
            - enum:
                - mediatek,mt8188-disp-ccorr
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
index 62306c88f485..d0ea77fc4b06 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
@@ -22,12 +22,10 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt2701-disp-color
-      - items:
-          - const: mediatek,mt8167-disp-color
-      - items:
-          - const: mediatek,mt8173-disp-color
+      - enum:
+          - mediatek,mt2701-disp-color
+          - mediatek,mt8167-disp-color
+          - mediatek,mt8173-disp-color

OK.

        - items:
            - enum:
                - mediatek,mt7623-disp-color
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml
index 5c7445c174e5..9d74de63fe63 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml
@@ -22,8 +22,7 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt8183-disp-dither
+      - const: mediatek,mt8183-disp-dither

OK.

        - items:
            - enum:
                - mediatek,mt8186-disp-dither
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml
index 49248864514b..37bf6bf4a1ab 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml
@@ -19,9 +19,7 @@ description: |
    properties:
    compatible:
-    oneOf:
-      - items:
-          - const: mediatek,mt8195-disp-dsc
+    const: mediatek,mt8195-disp-dsc

This will grow, and you'll get devicetree declaring something like:

compatible = "mediatek,(different-new-chip)-disp-dsc";

and

compatible = "mediatek,(current-chip)-disp-dsc", "mediatek,mt8195-disp-dsc";

Note: Some smartphone SoCs (Dimensity series) do have the same disp-dsc as
the Chromebook variant MT8195! :-)

      reg:
      maxItems: 1
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
index a5c6a91fac71..6c2be9d6840b 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
@@ -21,10 +21,9 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt8173-disp-gamma
-      - items:
-          - const: mediatek,mt8183-disp-gamma
+      - enum:
+          - mediatek,mt8173-disp-gamma
+          - mediatek,mt8183-disp-gamma

This change is ok.

        - items:
            - enum:
                - mediatek,mt8186-disp-gamma
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
index 69ba75777dac..c474ee6fa05b 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
@@ -20,11 +20,9 @@ description: |
    properties:
    compatible:
-    oneOf:
-      - items:
-          - const: mediatek,mt8173-disp-merge
-      - items:
-          - const: mediatek,mt8195-disp-merge
+    enum:
+      - mediatek,mt8173-disp-merge
+      - mediatek,mt8195-disp-merge

I'm mostly sure that you (yes, you, Yassine) will sooner or later write a
devicetree node containing the following compatible string on merge0:

compatible = "mediatek,mt6735-disp-merge", "mediatek,mt8173-disp-merge";

Am I wrong? :-)

Actually MT6735 doesn't have this block so no, I won't be doing that :)


In that case, that `oneOf:` will have to return back to this file, so it's
not worth removing it.

      reg:
      maxItems: 1
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml
index 853fcb9db2be..7e6bbf8b5c60 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml
@@ -20,11 +20,9 @@ description: |
    properties:
    compatible:
-    oneOf:
-      - items:
-          - const: mediatek,mt2712-disp-od
-      - items:
-          - const: mediatek,mt8173-disp-od
+    enum:
+      - mediatek,mt2712-disp-od
+      - mediatek,mt8173-disp-od

Same story with this one...

It doesn't have this one either.


    reg:
      maxItems: 1
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml
index 4e94f4e947ad..c7dd0ef02dcf 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml
@@ -21,10 +21,9 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt8183-disp-ovl-2l
-      - items:
-          - const: mediatek,mt8192-disp-ovl-2l
+      - enum:
+          - mediatek,mt8183-disp-ovl-2l
+          - mediatek,mt8192-disp-ovl-2l

This change is ok.

        - items:
            - enum:
                - mediatek,mt8186-disp-ovl-2l
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml
index 065e526f950e..92e320d54ba2 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml
@@ -21,14 +21,11 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt2701-disp-ovl
-      - items:
-          - const: mediatek,mt8173-disp-ovl
-      - items:
-          - const: mediatek,mt8183-disp-ovl
-      - items:
-          - const: mediatek,mt8192-disp-ovl
+      - enum:
+          - mediatek,mt2701-disp-ovl
+          - mediatek,mt8173-disp-ovl
+          - mediatek,mt8183-disp-ovl
+          - mediatek,mt8192-disp-ovl

This is ok too.

        - items:
            - enum:
                - mediatek,mt7623-disp-ovl
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml
index 27de64495401..12ec410bb921 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml
@@ -21,8 +21,7 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt8192-disp-postmask
+      - const: mediatek,mt8192-disp-postmask

Should be fine as well.

        - items:
            - enum:
                - mediatek,mt8186-disp-postmask
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml
index 3ade2ece3fed..42059efad45d 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml
@@ -23,14 +23,11 @@ description: |
  properties:
    compatible:
      oneOf:
-      - items:
-          - const: mediatek,mt2701-disp-rdma
-      - items:
-          - const: mediatek,mt8173-disp-rdma
-      - items:
-          - const: mediatek,mt8183-disp-rdma
-      - items:
-          - const: mediatek,mt8195-disp-rdma
+      - enum:
+          - mediatek,mt2701-disp-rdma
+          - mediatek,mt8173-disp-rdma
+          - mediatek,mt8183-disp-rdma
+          - mediatek,mt8195-disp-rdma

OK.

        - items:
            - enum:
                - mediatek,mt8188-disp-rdma
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
index 35ace1f322e8..54b7b0531144 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
@@ -20,9 +20,7 @@ description: |
    properties:
    compatible:
-    oneOf:
-      - items:
-          - const: mediatek,mt8173-disp-split
+    const: mediatek,mt8173-disp-split


compatible = "mediatek,mt6795-disp-split", "mediatek,mt8173-disp-split";

I have exactly that in my local devicetree for Xperia M5 (not upstream yet) so
no, I wouldn't do that :-)

Would only removing "items" and keeping "oneOf" be alright then?


    reg:
      maxItems: 1
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml
index b8bb135fe96b..87523b45a210 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml
@@ -21,9 +21,7 @@ description: |
    properties:
    compatible:
-    oneOf:
-      - items:
-          - const: mediatek,mt8173-disp-ufoe
+    const: mediatek,mt8173-disp-ufoe

Same, and I think UFOE is present on your MT6735 as well.

It isn't. In fact its input and output muxes are present, but the UFOE block itself is absent. My guess is Mediatek reused the dispsys skeleton in several SoCs and plugged in processing blocks as needed. Removing the muxes would've probably required additional work to reroute the connections between blocks before and after UFOE, so
they just kept the muxes untouched.

BTW, this patch comes in preparation for adding bindings for almost all MT6735 display blocks, so I haven't gotten rid of something that I'd need later. Although I didn't reuse MT8173 and MT8167 compatibles (the 2 SoCs functionally closest to MT6735 in terms of display) but instead added new MT6735 compatibles to the drivers (none of this is sent yet
of course).


    reg:
      maxItems: 1
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
index 7d7cc1ab526b..52f233fe1c0f 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
@@ -20,9 +20,7 @@ description: |
    properties:
    compatible:
-    oneOf:
-      - items:
-          - const: mediatek,mt8173-disp-wdma
+    const: mediatek,mt8173-disp-wdma

This is present on all MediaTek SoCs - literally. The issue is that the driver currently does not support command mode panels for real, so this binding is...
...well, somehow in forgotten-land...

What does it have to do with command mode panels? I thought it was used for off-screen
image processing. Or was that MDP WDMA?

Also with all the command mode stuff the driver currently has, it doesn't properly support command mode panels? I mean it even switches to command mode to send configuration commands to video mode panels rather than using VM_CMD (something that took me a while to get right on MT6735, because its DSI controller acts funny when modes are switched frequently), so I
imagined it had better support for command mode.


I would hope that command mode panels get implemented soon(er-than-later), but I'll leave the choice to you and Krzysztof - I'm only providing the information
here. :-)

    reg:
      maxItems: 1

Thanks for the effort!

Cheers,
Angelo


Anyway, I don't mind dropping the changes you NACKed, especially since they don't affect me at all, but I do want to know the motive behind discouraging simplifying
the bindings now and adding these things back if needed in the future.

Thanks,
Yassine






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux