[PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Nancy.Lin]

Add vdosys1 ETHDR definition.

Signed-off-by: Nancy.Lin <nanc...@mediatek.com>
Reviewed-by: Chun-Kuang Hu <chunku...@kernel.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacc...@collabora.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof...@linaro.org>
Tested-by: AngeloGioacchino Del Regno <angelogioacc...@collabora.com>
---
.../display/mediatek/mediatek,ethdr.yaml | 188 ++++++++++++++++++
1 file changed, 188 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
new file mode 100644
index 000000000000..3b11e47a8834
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
@@ -0,0 +1,188 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,ethdr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Ethdr Device
+
+maintainers:
+ - Chun-Kuang Hu <chunku...@kernel.org>
+ - Philipp Zabel <p.z...@pengutronix.de>
+
+description:
+ ETHDR (ET High Dynamic Range) is a MediaTek internal HDR engine and is
+ designed for HDR video and graphics conversion in the external display path.
+ It handles multiple HDR input types and performs tone mapping, color
+ space/color format conversion, and then combine different layers,
+ output the required HDR or SDR signal to the subsequent display path.
+ This engine is composed of two video frontends, two graphic frontends,
+ one video backend and a mixer. ETHDR has two DMA function blocks, DS and ADL.
+ These two function blocks read the pre-programmed registers from DRAM and
+ set them to HW in the v-blanking period.
+
+properties:
+ compatible:
+ const: mediatek,mt8195-disp-ethdr
+
+ reg:
+ maxItems: 7
+
+ reg-names:
+ items:
+ - const: mixer
+ - const: vdo_fe0
+ - const: vdo_fe1
+ - const: gfx_fe0
+ - const: gfx_fe1
+ - const: vdo_be
+ - const: adl_ds
+
+ interrupts:
+ maxItems: 1
+
+ iommus:
+ minItems: 1
+ maxItems: 2
+
+ clocks:
+ items:
+ - description: mixer clock
+ - description: video frontend 0 clock
+ - description: video frontend 1 clock
+ - description: graphic frontend 0 clock
+ - description: graphic frontend 1 clock
+ - description: video backend clock
+ - description: autodownload and menuload clock
+ - description: video frontend 0 async clock
+ - description: video frontend 1 async clock
+ - description: graphic frontend 0 async clock
+ - description: graphic frontend 1 async clock
+ - description: video backend async clock
+ - description: ethdr top clock
+
+ clock-names:
+ items:
+ - const: mixer
+ - const: vdo_fe0
+ - const: vdo_fe1
+ - const: gfx_fe0
+ - const: gfx_fe1
+ - const: vdo_be
+ - const: adl_ds
+ - const: vdo_fe0_async
+ - const: vdo_fe1_async
+ - const: gfx_fe0_async
+ - const: gfx_fe1_async
+ - const: vdo_be_async
+ - const: ethdr_top
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ items:
+ - description: video frontend 0 async reset
+ - description: video frontend 1 async reset
+ - description: graphic frontend 0 async reset
+ - description: graphic frontend 1 async reset
+ - description: video backend async reset
+
+ reset-names:
+ items:
+ - const: vdo_fe0_async
+ - const: vdo_fe1_async
+ - const: gfx_fe0_async
+ - const: gfx_fe1_async
+ - const: vdo_be_async
+
+ mediatek,gce-client-reg:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: The register of display function block to be set by gce.
+ There are 4 arguments in this property, gce node, subsys id, offset and
+ register size. The subsys id is defined in the gce header of each chips
+ include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
+ function block.
+ items:
+ items:
+ - description: phandle of GCE
+ - description: GCE subsys id
+ - description: register offset
+ - description: register size
+ minItems: 7
+ maxItems: 7
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - power-domains
+ - resets
+ - mediatek,gce-client-reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/mt8195-clk.h>
+ #include <dt-bindings/gce/mt8195-gce.h>
+ #include <dt-bindings/memory/mt8195-memory-port.h>
+ #include <dt-bindings/power/mt8195-power.h>
+ #include <dt-bindings/reset/mt8195-resets.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ hdr-engine@1c114000 {
+ compatible = "mediatek,mt8195-disp-ethdr";
+ reg = <0 0x1c114000 0 0x1000>,
+ <0 0x1c115000 0 0x1000>,
+ <0 0x1c117000 0 0x1000>,
+ <0 0x1c119000 0 0x1000>,
+ <0 0x1c11a000 0 0x1000>,
+ <0 0x1c11b000 0 0x1000>,
+ <0 0x1c11c000 0 0x1000>;
+ reg-names = "mixer", "vdo_fe0", "vdo_fe1", "gfx_fe0", "gfx_fe1",
+ "vdo_be", "adl_ds";
+ mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX 0x4000 0x1000>,
+ <&gce0 SUBSYS_1c11XXXX 0x5000 0x1000>,
+ <&gce0 SUBSYS_1c11XXXX 0x7000 0x1000>,
+ <&gce0 SUBSYS_1c11XXXX 0x9000 0x1000>,
+ <&gce0 SUBSYS_1c11XXXX 0xa000 0x1000>,
+ <&gce0 SUBSYS_1c11XXXX 0xb000 0x1000>,
+ <&gce0 SUBSYS_1c11XXXX 0xc000 0x1000>;
+ clocks = <&vdosys1 CLK_VDO1_DISP_MIXER>,
+ <&vdosys1 CLK_VDO1_HDR_VDO_FE0>,
+ <&vdosys1 CLK_VDO1_HDR_VDO_FE1>,
+ <&vdosys1 CLK_VDO1_HDR_GFX_FE0>,
+ <&vdosys1 CLK_VDO1_HDR_GFX_FE1>,
+ <&vdosys1 CLK_VDO1_HDR_VDO_BE>,
+ <&vdosys1 CLK_VDO1_26M_SLOW>,
+ <&vdosys1 CLK_VDO1_HDR_VDO_FE0_DL_ASYNC>,
+ <&vdosys1 CLK_VDO1_HDR_VDO_FE1_DL_ASYNC>,
+ <&vdosys1 CLK_VDO1_HDR_GFX_FE0_DL_ASYNC>,
+ <&vdosys1 CLK_VDO1_HDR_GFX_FE1_DL_ASYNC>,
+ <&vdosys1 CLK_VDO1_HDR_VDO_BE_DL_ASYNC>,
+ <&topckgen CLK_TOP_ETHDR>;
+ clock-names = "mixer", "vdo_fe0", "vdo_fe1", "gfx_fe0", "gfx_fe1",
+ "vdo_be", "adl_ds", "vdo_fe0_async", "vdo_fe1_async",
+ "gfx_fe0_async", "gfx_fe1_async","vdo_be_async",
+ "ethdr_top";
+ power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
+ iommus = <&iommu_vpp M4U_PORT_L3_HDR_DS>,
+ <&iommu_vpp M4U_PORT_L3_HDR_ADL>;
+ interrupts = <GIC_SPI 517 IRQ_TYPE_LEVEL_HIGH 0>; /* disp mixer */
+ resets = <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE0_DL_ASYNC>,
+ <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE1_DL_ASYNC>,
+ <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE0_DL_ASYNC>,
+ <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE1_DL_ASYNC>,
+ <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_BE_DL_ASYNC>;
+ reset-names = "vdo_fe0_async", "vdo_fe1_async", "gfx_fe0_async",
+ "gfx_fe1_async", "vdo_be_async";
+ };
+ };
+...
--
2.18.0

[Nancy Lin (林欣螢)]

>
> +
> +description:
> +  ETHDR (ET High Dynamic Range) is a MediaTek internal HDR engine
> and is
> +  designed for HDR video and graphics conversion in the external
> display path.
> +  It handles multiple HDR input types and performs tone mapping,
> color
> +  space/color format conversion, and then combine different layers,
> +  output the required HDR or SDR signal to the subsequent display
> path.
> +  This engine is composed of two video frontends, two graphic
> frontends,
> +  one video backend and a mixer. ETHDR has two DMA function blocks,
> DS and ADL.
> +  These two function blocks read the pre-programmed registers from
> DRAM and
> +  set them to HW in the v-blanking period.
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8195-disp-ethdr
> +
> +  reg:
> +    maxItems: 7
> +
> +  reg-names:
> +    items:
> +      - const: mixer
> +      - const: vdo_fe0
> +      - const: vdo_fe1
> +      - const: gfx_fe0
> +      - const: gfx_fe1
> +      - const: vdo_be
> +      - const: adl_ds
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  iommus:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clocks:
> +    items:
> +      - description: mixer clock
> +      - description: video frontend 0 clock
> +      - description: video frontend 1 clock
> +      - description: graphic frontend 0 clock
> +      - description: graphic frontend 1 clock
> +      - description: video backend clock
> +      - description: autodownload and menuload clock
> +      - description: video frontend 0 async clock
> +      - description: video frontend 1 async clock
> +      - description: graphic frontend 0 async clock
> +      - description: graphic frontend 1 async clock
> +      - description: video backend async clock
> +      - description: ethdr top clock
> +
> +  clock-names:
> +    items:
> +      - const: mixer
> +      - const: vdo_fe0
> +      - const: vdo_fe1
> +      - const: gfx_fe0
> +      - const: gfx_fe1
> +      - const: vdo_be
> +      - const: adl_ds
> +      - const: vdo_fe0_async
> +      - const: vdo_fe1_async
> +      - const: gfx_fe0_async
> +      - const: gfx_fe1_async
> +      - const: vdo_be_async
> +      - const: ethdr_top
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: video frontend 0 async reset
> +      - description: video frontend 1 async reset
> +      - description: graphic frontend 0 async reset
> +      - description: graphic frontend 1 async reset
> +      - description: video backend async reset
> +
> +  reset-names:
> +    items:
> +      - const: vdo_fe0_async
> +      - const: vdo_fe1_async
> +      - const: gfx_fe0_async
> +      - const: gfx_fe1_async
> +      - const: vdo_be_async
> +
> +  mediatek,gce-client-reg:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: The register of display function block to be set by
> gce.
> +      There are 4 arguments in this property, gce node, subsys id,
> offset and
> +      register size. The subsys id is defined in the gce header of
> each chips
> +      include/dt-bindings/gce/<chip>-gce.h, mapping to the register
> of display
> +      function block.
> +    items:
> +      items:
> +        - description: phandle of GCE
> +        - description: GCE subsys id
> +        - description: register offset
> +        - description: register size
> +    minItems: 7
> +    maxItems: 7
> +

Hi Rob and krzysztof,

I got the two messages when running dt_binding_check [1]. This binding
patch was sent previously in [2]. 

If I remove the following items/minItems/maxItems in the mediatek,gce-
client property, the two message disappear. I don't know what's wrong
with the original syntax. Do you have any suggestions for this?

-    items:
-      items:
-        - description: phandle of GCE
-        - description: GCE subsys id
-        - description: register offset
-        - description: register size
-    minItems: 7
-    maxItems: 7


[1].
Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.examp
le.dtb
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096] is too long
        From schema:
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.yaml
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096]] is too short
        From schema:
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.yaml

[2] [RESEND,v5,3/3] dt-bindings: mediatek: add ethdr definition for
mt8195

https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220608043852.4...@mediatek.com/


Regards,
Nancy















> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - power-domains
> +  - resets
> +  - mediatek,gce-client-reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/gce/mt8195-gce.h>
> +    #include <dt-bindings/memory/mt8195-memory-port.h>
> +    #include <dt-bindings/power/mt8195-power.h>
> +    #include <dt-bindings/reset/mt8195-resets.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        hdr-engine@1c114000 {
> +                compatible = "mediatek,mt8195-disp-ethdr";
> +                reg = <0 0x1c114000 0 0x1000>,
> +                      <0 0x1c115000 0 0x1000>,
> +                      <0 0x1c117000 0 0x1000>,
> +                      <0 0x1c119000 0 0x1000>,
> +                      <0 0x1c11a000 0 0x1000>,
> +                      <0 0x1c11b000 0 0x1000>,
> +                      <0 0x1c11c000 0 0x1000>;
> +                reg-names = "mixer", "vdo_fe0", "vdo_fe1",
> "gfx_fe0", "gfx_fe1",
> +                            "vdo_be", "adl_ds";
> +                mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX
> 0x4000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x5000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x7000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x9000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xa000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xb000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xc000 0x1000>;
> +                clocks = <&vdosys1 CLK_VDO1_DISP_MIXER>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_BE>,
> +                         <&vdosys1 CLK_VDO1_26M_SLOW>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_BE_DL_ASYNC>,
> +                         <&topckgen CLK_TOP_ETHDR>;
> +                clock-names = "mixer", "vdo_fe0", "vdo_fe1",
> "gfx_fe0", "gfx_fe1",
> +                              "vdo_be", "adl_ds", "vdo_fe0_async",
> "vdo_fe1_async",
> +                              "gfx_fe0_async",
> "gfx_fe1_async","vdo_be_async",
> +                              "ethdr_top";
> +                power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
> +                iommus = <&iommu_vpp M4U_PORT_L3_HDR_DS>,
> +                         <&iommu_vpp M4U_PORT_L3_HDR_ADL>;
> +                interrupts = <GIC_SPI 517 IRQ_TYPE_LEVEL_HIGH 0>; /*
> disp mixer */
> +                resets = <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE0_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE1_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE0_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE1_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_BE_DL_ASYNC>;
> +                reset-names = "vdo_fe0_async", "vdo_fe1_async",
> "gfx_fe0_async",
> +                              "gfx_fe1_async", "vdo_be_async";
> +        };
> +    };
> +...

************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!

[Krzysztof Kozlowski]

On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:

Trim the replies and remove unneeded context. You want to get the
attention of other people, not force them to read entire email.

This looks like known issue with phandles with variable number of
arguments. Either we add it to the exceptions or just define it in
reduced way like in other cases - only maxItems: 1 without describing items.


Best regards,
Krzysztof

[Nancy Lin (林欣螢)]

: mediatek,gce-client-reg:0: [4294967295, 7,
> > 16384,
> > 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> > 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
> > 7,
> > 45056, 4096, 4294967295, 7, 49152, 4096] is too long
> >         From schema:
> 
> This looks like known issue with phandles with variable number of
> arguments. Either we add it to the exceptions or just define it in
> reduced way like in other cases - only maxItems: 1 without describing
> items.
> 
> 
> Best regards,
> Krzysztof


Hi Krzysztof,

Thanks for the comment.

But I have several items for this vendor property in the binding
example.
Can I remove maxItems? Change the mediatek,gce-client-reg as [1].

[1]
  mediatek,gce-client-reg:
    $ref: /schemas/types.yaml#/definitions/phandle-array
    description: The register of display function block to be set by
gce.
      There are 4 arguments in this property, gce node, subsys id,
offset and
      register size. The subsys id is defined in the gce header of each
chips
      include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
      function block.

Regards,
Nancy

> 

[Krzysztof Kozlowski]

Do you? I thought you have one phandle?

> Can I remove maxItems? Change the mediatek,gce-client-reg as [1].
>
> [1]
> mediatek,gce-client-reg:
> $ref: /schemas/types.yaml#/definitions/phandle-array
> description: The register of display function block to be set by
> gce.
> There are 4 arguments in this property, gce node, subsys id,
> offset and
> register size. The subsys id is defined in the gce header of each
> chips
> include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
> display
> function block.

No, this needs some constraints.

Best regards,
Krzysztof

[AngeloGioacchino Del Regno]

Il 16/03/23 07:31, Krzysztof Kozlowski ha scritto:

> On 16/03/2023 07:19, Nancy Lin (林欣螢) wrote:
>> On Wed, 2023-03-15 at 08:16 +0100, Krzysztof Kozlowski wrote:
>>> On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:
>>>

..snip..

>>>>
>>>>
>>>> [1].
>>>> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.e
>>>> xamp
>>>> le.dtb
>>>> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
>>>> cetr
>>>> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
>>>> hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7,
>>>> 16384,
>>>> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
>>>> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
>>>> 7,
>>>> 45056, 4096, 4294967295, 7, 49152, 4096] is too long
>>>> From schema:
>>>
>>> This looks like known issue with phandles with variable number of
>>> arguments. Either we add it to the exceptions or just define it in
>>> reduced way like in other cases - only maxItems: 1 without describing
>>> items.
>>>

...

>>
>> But I have several items for this vendor property in the binding
>> example.
>
> Do you? I thought you have one phandle?
>
>> Can I remove maxItems? Change the mediatek,gce-client-reg as [1].
>>
>> [1]
>> mediatek,gce-client-reg:
>> $ref: /schemas/types.yaml#/definitions/phandle-array
>> description: The register of display function block to be set by
>> gce.
>> There are 4 arguments in this property, gce node, subsys id,
>> offset and
>> register size. The subsys id is defined in the gce header of each
>> chips
>> include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
>> display
>> function block.
>
> No, this needs some constraints.

Hello Krzysztof, Nancy,

Since this series has reached v29, can we please reach an agreement on the bindings
to use here, so that we can get this finally upstreamed?

I will put some examples to try to get this issue resolved.

### Example 1: Constrain the number of GCE entries to *seven* array elements (7x4!)

mediatek,gce-client-reg:
$ref: /schemas/types.yaml#/definitions/phandle-array
maxItems: 1

description: The register of display function block to be set by gce.
There are 4 arguments in this property, gce node, subsys id, offset and
register size. The subsys id is defined in the gce header of each chips
include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
function block.

items:
minItems: 28
maxItems: 28
items: <----- this block doesn't seem to get checked :\

- description: phandle of GCE
- description: GCE subsys id

- description: register offset
- description: register size


### Example 2: Don't care about constraining the number of arguments

mediatek,gce-client-reg:
$ref: /schemas/types.yaml#/definitions/phandle-array
maxItems: 1

description: The register of display function block to be set by gce.
There are 4 arguments in this property, gce node, subsys id, offset and
register size. The subsys id is defined in the gce header of each chips
include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
function block.

Regards,
Angelo

[Krzysztof Kozlowski]

On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:

> Hello Krzysztof, Nancy,
>
> Since this series has reached v29, can we please reach an agreement on the bindings
> to use here, so that we can get this finally upstreamed?
>
> I will put some examples to try to get this issue resolved.
>
> ### Example 1: Constrain the number of GCE entries to *seven* array elements (7x4!)
>
> mediatek,gce-client-reg:
> $ref: /schemas/types.yaml#/definitions/phandle-array
> maxItems: 1
> description: The register of display function block to be set by gce.
> There are 4 arguments in this property, gce node, subsys id, offset and
> register size. The subsys id is defined in the gce header of each chips
> include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
> function block.
> items:
> minItems: 28
> maxItems: 28
> items: <----- this block doesn't seem to get checked :\
> - description: phandle of GCE
> - description: GCE subsys id
> - description: register offset
> - description: register size

This is what we would like to have but it requires exception in
dtschema. Thus:

>
>
> ### Example 2: Don't care about constraining the number of arguments
>
> mediatek,gce-client-reg:
> $ref: /schemas/types.yaml#/definitions/phandle-array
> maxItems: 1
> description: The register of display function block to be set by gce.
> There are 4 arguments in this property, gce node, subsys id, offset and
> register size. The subsys id is defined in the gce header of each chips
> include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
> function block.

use this.

Best regards,
Krzysztof

[Nancy Lin (林欣螢)]

On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> 
> > Hello Krzysztof, Nancy,
> > 
> > Since this series has reached v29, can we please reach an agreement
> > on the bindings
> > to use here, so that we can get this finally upstreamed?
> > 
> > I will put some examples to try to get this issue resolved.
> > 
> > ### Example 1: Constrain the number of GCE entries to *seven* array
> > elements (7x4!)
> > 
> >    mediatek,gce-client-reg:
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >      maxItems: 1
> >      description: The register of display function block to be set
> > by gce.
> >        There are 4 arguments in this property, gce node, subsys id,
> > offset and
> >        register size. The subsys id is defined in the gce header of
> > each chips
> >        include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of display
> >        function block.
> >      items:
> >        minItems: 28
> >        maxItems: 28
> >        items:                     <----- this block doesn't seem to
> > get checked :\
> >          - description: phandle of GCE
> >          - description: GCE subsys id
> >          - description: register offset
> >          - description: register size
> 
> This is what we would like to have but it requires exception in
> dtschema. Thus:
> 
> > 
> > 
> > ### Example 2: Don't care about constraining the number of
> > arguments
> > 
> >    mediatek,gce-client-reg:
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >      maxItems: 1
> >      description: The register of display function block to be set
> > by gce.
> >        There are 4 arguments in this property, gce node, subsys id,
> > offset and
> >        register size. The subsys id is defined in the gce header of
> > each chips
> >        include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of display
> >        function block.
> 
> use this.
> 
> Best regards,
> Krzysztof


Hi Krzysztof, Angelo,

Thanks for the comment.
The Example 2 can pass dt_binding_check. 

But the example in the binding has 7 items [1] and dts [2]. Does the
"maxItems: 1" affect any other schema or dts check? 

[1]
+    mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX 0x4000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x5000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x7000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x9000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xa000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xb000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xc000 0x1000>;
> 

[2] [v21,25/25] arm64: dts: mt8195: add display node for vdosys1

https://patchwork.kernel.org/project/linux-mediatek/patch/20220504091440.2...@mediatek.com/

Regards,
Nancy

[Krzysztof Kozlowski]

Ah, then it should be maxItems: 7, not 1.

Best regards,
Krzysztof

[AngeloGioacchino Del Regno]

Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:

Keep in mind for your v30:

maxItems: 7 will pass - but only if minItems is *not* 7 :-)

-> (so, do not declare minItems, as default is 1) <-

Regards,
Angelo

> Best regards,
> Krzysztof
>

[Nancy Lin (林欣螢)]

On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> > On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> > > On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> > > > On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> > > > 
> > > > > Hello Krzysztof, Nancy,
> > > > > 
> > > > > Since this series has reached v29, can we please reach an
> > > > > agreement
> > > > > on the bindings
> > > > > to use here, so that we can get this finally upstreamed?
> > > > > 
> > > > > I will put some examples to try to get this issue resolved.
> > > > > 
> > > > > ### Example 1: Constrain the number of GCE entries to *seven*
> > > > > array
> > > > > elements (7x4!)
> > > > > 
> > > > >     mediatek,gce-client-reg:
> > > > >       $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > >       maxItems: 1
> > > > >       description: The register of display function block to
> > > > > be set
> > > > > by gce.
> > > > >         There are 4 arguments in this property, gce node,
> > > > > subsys id,
> > > > > offset and
> > > > >         register size. The subsys id is defined in the gce
> > > > > header of
> > > > > each chips
> > > > >         include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > > > > register of display
> > > > >         function block.
> > > > >       items:
> > > > >         minItems: 28
> > > > >         maxItems: 28
> > > > >         items:                     <----- this block doesn't
> > > > > seem to
> > > > > get checked :\
> > > > >           - description: phandle of GCE
> > > > >           - description: GCE subsys id
> > > > >           - description: register offset
> > > > >           - description: register size
> > > > 
> > > > This is what we would like to have but it requires exception in
> > > > dtschema. Thus:
> > > > 
> > > > > 
> > > > > 
> > > > > ### Example 2: Don't care about constraining the number of
> > > > > arguments
> > > > > 
> > > > >     mediatek,gce-client-reg:
> > > > >       $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > >       maxItems: 1
> > > > >       description: The register of display function block to
> > > > > be set
> > > > > by gce.
> > > > >         There are 4 arguments in this property, gce node,
> > > > > subsys id,
> > > > > offset and
> > > > >         register size. The subsys id is defined in the gce
> > > > > header of
> > > > > each chips
> > > > >         include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > > > > register of display
> > > > >         function block.
> > > > 
> > > > use this.
> > > > 
> > > > Best regards,
> > > > Krzysztof
> > > 
> > > 
> > > Hi Krzysztof, Angelo,
> > > 
> > > Thanks for the comment.
> > > The Example 2 can pass dt_binding_check.
> > > 
> > > But the example in the binding has 7 items [1] and dts [2]. Does
> > > the
> > > "maxItems: 1" affect any other schema or dts check?
> > 
> > Ah, then it should be maxItems: 7, not 1.
> > 
> 
> Keep in mind for your v30:
> 
> maxItems: 7 will pass - but only if minItems is *not* 7 :-)
> 
> -> (so, do not declare minItems, as default is 1) <-
> 
> Regards,
> Angelo
> 
Hi Angelo,

I still have one message [1] when runing dt_binding_check for "example
2 + maxItems: 7" [2].

[1]
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096]] is too short


[2]
   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 7
     description: The register of display function block to be set by
gce.
       There are 4 arguments in this property, gce node, subsys id,
offset and
       register size. The subsys id is defined in the gce header of
each chips
       include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
       function block.

Regards,
Nancy


> > Best regards,
> > Krzysztof
> > 

[AngeloGioacchino Del Regno]

Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:

Maybe I'm wrong about the "do not declare minItems"... try with

minItems: 1
maxItems: 7


...does it work now?

[Nancy Lin (林欣螢)]

Dear Angelo,

Sorry for late reply.

On Fri, 2023-03-17 at 10:58 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:
> > On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> > > > On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> > > > > On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> > > > > > On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> > > > > > 
> > > > > > > Hello Krzysztof, Nancy,
> > > > > > > 
> > > > > > > Since this series has reached v29, can we please reach an
> > > > > > > agreement
> > > > > > > on the bindings
> > > > > > > to use here, so that we can get this finally upstreamed?
> > > > > > > 
> > > > > > > I will put some examples to try to get this issue
> > > > > > > resolved.
> > > > > > > 
> > > > > > > ### Example 1: Constrain the number of GCE entries to
> > > > > > > *seven*
> > > > > > > array
> > > > > > > elements (7x4!)
> > > > > > > 
> > > > > > >      mediatek,gce-client-reg:
> > > > > > >        $ref: /schemas/types.yaml#/definitions/phandle-
> > > > > > > array
> > > > > > >        maxItems: 1
> > > > > > >        description: The register of display function
> > > > > > > block to
> > > > > > > be set
> > > > > > > by gce.
> > > > > > >          There are 4 arguments in this property, gce
> > > > > > > node,
> > > > > > > subsys id,
> > > > > > > offset and
> > > > > > >          register size. The subsys id is defined in the
> > > > > > > gce
> > > > > > > header of
> > > > > > > each chips
> > > > > > >          include/dt-bindings/gce/<chip>-gce.h, mapping to
> > > > > > > the
> > > > > > > register of display
> > > > > > >          function block.
> > > > > > >        items:
> > > > > > >          minItems: 28
> > > > > > >          maxItems: 28
> > > > > > >          items:                     <----- this block
> > > > > > > doesn't
> > > > > > > seem to
> > > > > > > get checked :\
> > > > > > >            - description: phandle of GCE
> > > > > > >            - description: GCE subsys id
> > > > > > >            - description: register offset
> > > > > > >            - description: register size
> > > > > > 
> > > > > > This is what we would like to have but it requires
> > > > > > exception in
> > > > > > dtschema. Thus:
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > ### Example 2: Don't care about constraining the number
> > > > > > > of
> > > > > > > arguments
> > > > > > > 
> > > > > > >      mediatek,gce-client-reg:
> > > > > > >        $ref: /schemas/types.yaml#/definitions/phandle-
> > > > > > > array
> > > > > > >        maxItems: 1
> > > > > > >        description: The register of display function
> > > > > > > block to
> > > > > > > be set
> > > > > > > by gce.
> > > > > > >          There are 4 arguments in this property, gce
> > > > > > > node,
> > > > > > > subsys id,
> > > > > > > offset and
> > > > > > >          register size. The subsys id is defined in the
> > > > > > > gce
> > > > > > > header of
> > > > > > > each chips
> > > > > > >          include/dt-bindings/gce/<chip>-gce.h, mapping to
> > > > > > > the
> > > > > > > register of display
> > > > > > >          function block.
> > > > > > 
> > > > > > use this.
> > > > > > 
> > > > > > Best regards,
> > > > > > Krzysztof
> > > > > 
> > > > > 
> > > > > Hi Krzysztof, Angelo,
> > > > > 
> > > > > Thanks for the comment.
> > > > > The Example 2 can pass dt_binding_check.
> > > > > 
> > > > > But the example in the binding has 7 items [1] and dts [2].
> > > > > Does
> > > > > the
> > > > > "maxItems: 1" affect any other schema or dts check?
> > > > 
> > > > Ah, then it should be maxItems: 7, not 1.
> > > > 
> > > 
> > > Keep in mind for your v30:
> > > 
> > > maxItems: 7 will pass - but only if minItems is *not* 7 :-)
> > > 
> > > -> (so, do not declare minItems, as default is 1) <-
> > > 
> > > Regards,
> > > Angelo
> > > 
> > 
> > Hi Angelo,
> > 
> > I still have one message [1] when runing dt_binding_check for
> > "example
> > 2 + maxItems: 7" [2].
> > 
> > [1]
> > /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
> > cetr
> > ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
> > hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7,
> > 16384,
> > 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> > 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
> > 7,
> > 45056, 4096, 4294967295, 7, 49152, 4096]] is too short
> > 
> > 
> > [2]
> >     mediatek,gce-client-reg:
> >       $ref: /schemas/types.yaml#/definitions/phandle-array
> >       maxItems: 7
> >       description: The register of display function block to be set
> > by
> > gce.
> >         There are 4 arguments in this property, gce node, subsys
> > id,
> > offset and
> >         register size. The subsys id is defined in the gce header
> > of
> > each chips
> >         include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of
> > display
> >         function block.
> > 
> 
> Maybe I'm wrong about the "do not declare minItems"... try with
> 
> minItems: 1
> maxItems: 7
> 
> 
> ...does it work now?
> 

Yes, It works well with "example2 + minItems:1 + maxItems: 7" [1]

[1]
  mediatek,gce-client-reg:
    $ref: /schemas/types.yaml#/definitions/phandle-array
    minItems: 1
    maxItems: 7
    description: The register of display function block to be set by
gce.
      There are 4 arguments in this property, gce node, subsys id,
offset and
      register size. The subsys id is defined in the gce header of each
chips
      include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
      function block.

Regards,
Nancy


> > Regards,
> > Nancy
> > 
> > 
> > > > Best regards,
> > > > Krzysztof
> > > > 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-ar...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[AngeloGioacchino Del Regno]

Il 21/03/23 06:33, Nancy Lin (林欣螢) ha scritto:

Please send a v30 with that solution ASAP then, so that we may perhaps *finally*
get it in for v6.4.

Regards,
Angelo