Re: [PATCH v8 2/3] dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible string

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The commit silently introduces a new `mediatek,larb` property and makes it mandatory for the MT8189 variant without any mention in the commit message.
- [Low] The new compatible string `mediatek,mt8189-jpgenc` is defined using an `enum` containing a single item instead of `const`.
--

commit de42f55ad078326b54b85cf561d120b901b074be
Author: Jianhua Lin <jianhua.lin@mediatek.com>

dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible string

This commit adds the compatible string for the JPEG encoder block found
in the MediaTek MT8189 SoC. It is added as a standalone compatible string
without falling back to the generic identifier to ensure proper 34-bit IOVA
address space support.

> diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
> index 5b15f8977f67f..f75871e72633b 100644
> --- a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
> @@ -14,13 +14,17 @@ description: |-
>  
>  properties:
>    compatible:
> -    items:
> +    oneOf:
>        - enum:
> -          - mediatek,mt2701-jpgenc
> -          - mediatek,mt8183-jpgenc
> -          - mediatek,mt8186-jpgenc
> -          - mediatek,mt8188-jpgenc
> -      - const: mediatek,mtk-jpgenc
> +          - mediatek,mt8189-jpgenc

This isn't a bug, but dt_binding_check usually warns about using an enum with
a single item. Should this use const instead?

> +      - items:
> +          - enum:
> +              - mediatek,mt2701-jpgenc
> +              - mediatek,mt8183-jpgenc
> +              - mediatek,mt8186-jpgenc
> +              - mediatek,mt8188-jpgenc
> +          - const: mediatek,mtk-jpgenc
> +

[ ... ]

> @@ -45,6 +49,10 @@ properties:
>        Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.
>        Ports are according to the HW.
>  
> +  mediatek,larb:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: A phandle to the smi_larb node.
> +
>  required:
>    - compatible
>    - reg
> @@ -54,6 +62,16 @@ required:
>    - power-domains
>    - iommus
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8189-jpgenc
> +    then:
> +      required:
> +        - mediatek,larb

The commit message mentions the standalone compatible string is needed for
34-bit IOVA address space support, but it doesn't mention adding the new
mediatek,larb property or making it mandatory for the MT8189 variant.

Could the commit message be updated to explain the purpose and necessity of
this new hardware constraint?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519021726.19137-1-jianhua.lin@mediatek.com?part=2