Re: [PATCH v2 01/12] dt-binding: mediatek: add bindings for MediaTek 8195 MDP3 components

From: Rob Herring <robh@kernel.org>
To: Moudy Ho <moudy.ho@mediatek.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Chun-Kuang Hu <chunkuang.hu@kernel.org>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Project_Global_Chrome_Upstream_Group@mediatek.com
Subject: Re: [PATCH v2 01/12] dt-binding: mediatek: add bindings for MediaTek 8195 MDP3 components
Date: Fri, 9 Dec 2022 15:06:31 -0600	[thread overview]
Message-ID: <20221209210631.GA3840519-robh@kernel.org> (raw)
In-Reply-To: <20221207084412.14075-2-moudy.ho@mediatek.com>

On Wed, Dec 07, 2022 at 04:44:01PM +0800, Moudy Ho wrote:
> This patch adds support for MT8195 MDP3 RDMA, and introduce more
> MDP3 components present in MT8195.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
>  .../bindings/media/mediatek,mdp3-aal.yaml     | 64 ++++++++++++++++
>  .../bindings/media/mediatek,mdp3-color.yaml   | 64 ++++++++++++++++
>  .../bindings/media/mediatek,mdp3-fg.yaml      | 64 ++++++++++++++++
>  .../bindings/media/mediatek,mdp3-hdr.yaml     | 63 +++++++++++++++
>  .../bindings/media/mediatek,mdp3-merge.yaml   | 64 ++++++++++++++++
>  .../bindings/media/mediatek,mdp3-ovl.yaml     | 64 ++++++++++++++++
>  .../bindings/media/mediatek,mdp3-pad.yaml     | 64 ++++++++++++++++
>  .../bindings/media/mediatek,mdp3-rdma.yaml    |  5 +-
>  .../bindings/media/mediatek,mdp3-split.yaml   | 76 +++++++++++++++++++
>  .../bindings/media/mediatek,mdp3-stitch.yaml  | 64 ++++++++++++++++
>  .../bindings/media/mediatek,mdp3-tcc.yaml     | 63 +++++++++++++++
>  .../bindings/media/mediatek,mdp3-tdshp.yaml   | 64 ++++++++++++++++
>  12 files changed, 717 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-aal.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-color.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-fg.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-hdr.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-merge.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-ovl.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-pad.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-split.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-stitch.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-tcc.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-tdshp.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-aal.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-aal.yaml
> new file mode 100644
> index 000000000000..5b59e0802357
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-aal.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/mediatek,mdp3-aal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Media Data Path 3 AAL Device Tree Bindings

Drop ' Device Tree Bindings' from titles.

> +
> +maintainers:
> +  - Matthias Brugger <matthias.bgg@gmail.com>
> +  - Moudy Ho <moudy.ho@mediatek.com>
> +
> +description: |

Don't need '|'

> +  One of Media Data Path 3 (MDP3) components is responsible for backlight
> +  power saving and sunlight visibility improving.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,mt8195-mdp3-aal
> +
> +  reg:
> +    maxItems: 1
> +
> +  mediatek,gce-client-reg:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: phandle of GCE
> +        - description: GCE subsys id
> +        - description: register offset
> +        - description: register size
> +    description: The register of client driver can be configured by gce with
> +      4 arguments defined in this property. Each GCE subsys id is mapping to
> +      a client defined in the header include/dt-bindings/gce/<chip>-gce.h.

How many 4 cell entries? This allows any number. If 1, then you need 
'maxItems: 1' with the outer 'items'.

> +
> +  clocks:
> +    minItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - mediatek,gce-client-reg
> +  - clocks
> +  - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/gce/mt8195-gce.h>
> +    #include <dt-bindings/power/mt8195-power.h>
> +
> +    mdp3-aal0@14005000 {
> +      compatible = "mediatek,mt8195-mdp3-aal";
> +      reg = <0x14005000 0x1000>;
> +      mediatek,gce-client-reg = <&gce1 SUBSYS_1400XXXX 0x5000 0x1000>;
> +      clocks = <&vppsys0 CLK_VPP0_MDP_AAL>;
> +      power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS0>;

Indent by 4 spaces here.

> +    };

Same comments for the rest...