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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: media: hardcoded M4U iommus description applied to SMMU
--

commit 4d170d7f6a62a55e32063d072935c6eaecadd30f
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 MediaTek
JPEG encoder dt-binding document. The commit message notes that the MT8196 uses
an SMMU and supports features like 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
>  
>    power-domains:
>      maxItems: 1

[Severity: Low]
Since the commit message states that MT8196 uses an SMMU instead of an IOMMU,
is it correct to retain the existing iommus description for this new
compatible?

Looking at properties:iommus in this file, the top-level description is:

  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.

Because MT8196 uses an SMMU, the "master port" and M4U reference
(bindings/iommu/mediatek,iommu.yaml) seem misleading for the newly added
mediatek,mt8196-jpgenc compatible. While a later patch in this series adds
SMMU configuration properties, it does not appear to fix this descriptive
discrepancy.

Could this be updated or conditionally overridden for MT8196?

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

[Severity: Low]
The child node iommus property (under
patternProperties:^jpgenc@[0-9a-f]+$:properties:iommus) also has an
M4U-specific description:

      iommus:
        minItems: 1
        maxItems: 32
        description:
          List of the hardware port in respective IOMMU block for current Socs.
          Refer to bindings/iommu/mediatek,iommu.yaml.

Would it make sense to update this description to reflect the SMMU stream IDs
used by the newly added mediatek,mt8196-jpgenc-hw compatible?

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