Re: [PATCH v15 10/12] media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgenc compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The schema update adds support for MT8196 but omits definitions for its newly advertised hardware features. While DVFS relies on existing standard `power-domains` properties without needing schema additions, the advertised SMMU support introduces a hard dependency on a new `mediatek,smmu-config` property in the child nodes (implemented in a subsequent patch 'media: mediatek: jpeg: add jpeg smmu sid setting'). Because `additionalProperties: false` is enforced, any device tree attempting to specify this property will fail schema validation. Conversely, if the property is omitted to satisfy validation, the driver will fail to probe.
- [Low] The `iommus` property description incorrectly references the MediaTek IOMMU binding for hardware that explicitly uses an SMMU.
--

commit 375a0f1aaa853e038449c5b8176a2c5ea4a805df
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgenc compatible

This commit adds the mediatek,mt8196-jpgenc compatible string to the binding.
It notes that the MT8196 hardware uses an SMMU instead of an IOMMU and
supports dynamic voltage and frequency scaling.

> diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> index 596186497b684..e2d772ea0fb01 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> @@ -14,7 +14,9 @@ description:
>  
>  properties:
>    compatible:
> -    const: mediatek,mt8195-jpgenc
> +    enum:
> +      - mediatek,mt8195-jpgenc
> +      - mediatek,mt8196-jpgenc

[Severity: Low]
The commit message notes that MT8196 uses an SMMU instead of the proprietary
MediaTek IOMMU. Does the existing description for the iommus property need
updating to reflect this?

Currently, the property definition explicitly points to the IOMMU bindings:

  iommus:
    maxItems: 4
    description:
      Points to the respective IOMMU block with master port as argument, see
      Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.
      Ports are according to the HW.

Could this static reference cause confusion for developers configuring the
new MT8196 hardware?

[ ... ]

> @@ -44,7 +46,9 @@ patternProperties:
>  
>      properties:
>        compatible:
> -        const: mediatek,mt8195-jpgenc-hw
> +        enum:
> +          - mediatek,mt8195-jpgenc-hw
> +          - mediatek,mt8196-jpgenc-hw

[Severity: High]
Since this child node is enforced by additionalProperties: false, will adding
support for MT8196 without defining its new SMMU features cause validation or
probe failures?

The commit message advertises SMMU support, which introduces a dependency
on a new mediatek,smmu-config property in the child nodes. Because the child
node enforces additionalProperties: false:

    required:
      - compatible
      - reg
      - iommus
      - interrupts
      - clocks
      - clock-names
      - power-domains

    additionalProperties: false

If device tree authors provide the mediatek,smmu-config property, it will
cause dtbs_check validation failures.

If they omit it to pass validation, the driver will fail to probe since it
won't be able to look up the syscon regmap. 

Should mediatek,smmu-config be added to the properties list for the child
node to prevent this issue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702072614.10373-1-kyrie.wu@mediatek.com?part=10