Re: [PATCH 2/4] dt-bindings: display: mediatek: Add compatibles for MediaTek mt8167

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 15/02/26 09:53, Luca Leonardo Scorcia ha scritto:
> Add compatibles for various display-related blocks of MediaTek mt8167.
> 
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>

Awesome. I'd give you my R-b seal of approval, but there's a few nitpicks that
you have to address. Please check below.

> ---
>   .../devicetree/bindings/display/mediatek/mediatek,aal.yaml   | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,ccorr.yaml | 4 +++-
>   .../bindings/display/mediatek/mediatek,dither.yaml           | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,dsi.yaml   | 5 ++++-
>   .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,ovl.yaml   | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,rdma.yaml  | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,wdma.yaml  | 4 +++-
>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml  | 1 +
>   9 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> index daf90ebb39bf..4bbea72b292a 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml

...snip...

> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
> index 27ffbccc2a08..bcbde16648c0 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
> @@ -25,11 +25,14 @@ properties:
>         - enum:
>             - mediatek,mt2701-dsi
>             - mediatek,mt7623-dsi
> -          - mediatek,mt8167-dsi
>             - mediatek,mt8173-dsi
>             - mediatek,mt8183-dsi
>             - mediatek,mt8186-dsi
>             - mediatek,mt8188-dsi
> +      - items:
> +          - enum:
> +              - mediatek,mt8167-dsi
> +          - const: mediatek,mt2701-dsi

This needs its own patch with its own commit description - as this change is not
really "normal".

You have to mention that you're removing the "mediatek,mt8167-dsi" compatible from
that list and that it is safe to do so because:
  - Bootloader doesn't rely on this single compatible; and
  - There was never any upstreamed devicetree using this single compatible; and
  - The MT8167 DSI Controller is fully compatible with the one found in MT2701.

So, please, split this one out of the entire batch.

>         - items:
>             - enum:
>                 - mediatek,mt6795-dsi
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> index 48542dc7e784..ec1054bb06d4 100644

...snip...

> diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> index acdbce937b0a..c6d0bbdbe0e2 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> @@ -23,6 +23,7 @@ properties:
>         - items:
>             - enum:
>                 - mediatek,mt7623-mipi-tx
> +              - mediatek,mt8167-mipi-tx
>             - const: mediatek,mt2701-mipi-tx
>         - items:
>             - enum:

The PHY bindings are not applied by the same maintainer as the display bindings:
please split that out in a different patch, so that each maintainer can apply
the relevant patch(es).

Besides - apart from those nits, everything in here looks great.

Cheers,
Angelo