lkml.org 
[lkml]   [2023]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RESEND v3 01/13] dt-binding: mediatek: add bindings for MediaTek mt8195 MDP3 components
From
On 16/01/2023 04:21, Moudy Ho wrote:
> This patch adds support for MT8195 MDP3 RDMA, and introduce more
> MDP3 components present in MT8195.

Do not use "This commit/patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

Subject: drop second/last, redundant "bindings for". The "dt-bindings"
prefix is already stating that these are bindings.

>
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
> .../bindings/media/mediatek,mdp3-aal.yaml | 63 ++++++++++++++++
> .../bindings/media/mediatek,mdp3-color.yaml | 63 ++++++++++++++++
> .../bindings/media/mediatek,mdp3-fg.yaml | 63 ++++++++++++++++
> .../bindings/media/mediatek,mdp3-hdr.yaml | 62 +++++++++++++++
> .../bindings/media/mediatek,mdp3-merge.yaml | 63 ++++++++++++++++
> .../bindings/media/mediatek,mdp3-ovl.yaml | 63 ++++++++++++++++
> .../bindings/media/mediatek,mdp3-pad.yaml | 63 ++++++++++++++++
> .../bindings/media/mediatek,mdp3-rdma.yaml | 30 +++++---
> .../bindings/media/mediatek,mdp3-rsz.yaml | 11 ++-
> .../bindings/media/mediatek,mdp3-split.yaml | 75 +++++++++++++++++++
> .../bindings/media/mediatek,mdp3-stitch.yaml | 63 ++++++++++++++++
> .../bindings/media/mediatek,mdp3-tcc.yaml | 62 +++++++++++++++
> .../bindings/media/mediatek,mdp3-tdshp.yaml | 63 ++++++++++++++++
> 13 files changed, 731 insertions(+), 13 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..d2e1b5245778
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-aal.yaml

Filename should match compatible, unless you already expect this binding
will cover other devices. If so, why not adding them now?

> @@ -0,0 +1,63 @@
> +# 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
> +
> +maintainers:
> + - Matthias Brugger <matthias.bgg@gmail.com>
> + - Moudy Ho <moudy.ho@mediatek.com>
> +
> +description:
> + 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:
> + description:
> + The register of client driver can be configured by gce with 4 arguments
> + defined in this property, such as phandle of gce, subsys id,
> + register offset and size.
> + Each subsys id is mapping to a base address of display function blocks
> + register which is defined in the gce header
> + include/dt-bindings/gce/<chip>-gce.h.
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + maxItems: 1

items with items syntax instead:

https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42

> +
> + clocks:
> + minItems: 1

Nope, maxItems.

> +
> + 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 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Definitelly "3" and "0" are not generic suffixes.

All comments above apply to your other files here.

> + 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>;
> + };
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-color.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-color.yaml
> new file mode 100644
> index 000000000000..1d8aa5dc76b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-color.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/mediatek,mdp3-color.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Media Data Path 3 COLOR
> +
> +maintainers:
> + - Matthias Brugger <matthias.bgg@gmail.com>
> + - Moudy Ho <moudy.ho@mediatek.com>
> +
> +description:
> + One of Media Data Path 3 (MDP3) components used to adjust hue, luma and
> + saturation to get better picture quality.
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-mdp3-color

This is exactly the same as previous file. Why do you split the binding?
It really looks unnecessary.

Probably all other files should be also squashed.

> +
> + reg:
> + maxItems: 1
> +
> + mediatek,gce-client-reg:
> + description:
> + The register of client driver can be configured by gce with 4 arguments
> + defined in this property, such as phandle of gce, subsys id,
> + register offset and size.
> + Each subsys id is mapping to a base address of display function blocks
> + register which is defined in the gce header
> + include/dt-bindings/gce/<chip>-gce.h.

Full, real path please, so it could be validated with tools.


Best regards,
Krzysztof

\
 
 \ /
  Last update: 2023-03-26 23:42    [W:0.204 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site