[PATCH v8 0/3] Mediatek MT8189 JPEG support

[PATCH v8 0/3] Mediatek MT8189 JPEG support

[Jianhua Lin]

This series is based on linux-next tag next-20260508.

This series depends on commit 7560349ee0d9 ("media: mediatek: jpeg:
support 34bits"), which introduced the `support_34bit` field in struct
mtk_jpeg_variant. That commit has already been merged via the media
tree and is present in linux-next as of next-20260508.

Changes compared with v7:
- Patches 1/3 (dt-bindings: decoder):
  - In the allOf constraints, only keep maxItems: 1 in the then 
    branch and minItems: 2 in the else branch; remove duplicated
    constraints already defined at the top level

Changes compared with v6:
- Patches 1/3 (dt-bindings: decoder):
  update the existing `allOf` condition for mediatek,mt8189-jpgdec to
  make the 'mediatek,larb' property strictly required for MT8189 SoC.
- Patches 2/3 (dt-bindings: encoder):
  Add an `allOf` condition to enforce that the `mediatek,larb` property
  is strictly required when the compatible string contains
  mediatek,mt8189-jpgenc.

Changes compared with v5:
- Patches 1/3 (dt-bindings: decoder):
  - Drop top-level minItems/maxItems for clock-names per Krzysztof's
    review.
  - Refine allOf block to strictly enforce clock constraints.

Changes compared with v4:
- Refines the device tree bindings for JPEG decoder and encoder.
  - Patches 1/3 (dt-bindings: decoder):
    Moved the standalone compatible string mediatek,mt8189-jpgdec
    into the first oneOf entry along with mt2701 and mt8173, as
    suggested by Rob Herring. This correctly groups all independent
    ICs and removes the redundant items wrapper.
  - Patches 2/3 (dt-bindings: encoder):
    Applied the same logic suggested by Rob Herring to the encoder
    binding. Restructured the compatible property to clearly
    distinguish between the standalone IC (mediatek,mt8189-jpgenc)
    and the ICs that must fallback to mediatek,mtk-jpgenc.

Changes compared with v3:
- The v4 is resending the cover-letter, because the v3 cover-letter was
  not sent successfully.

Changes compared with v2:
- Dropped the dts patch (arm64: dts: mt8188: update JPEG encoder/decoder
  compatible) as it belongs to a different tree/series.
- Patches 1/3 (dt-bindings: decoder):
  - Changed the MT8189 compatible to be a standalone `const` instead of
    an `enum`.
  - Added an `allOf` block with conditional checks to enforce the single
    clock ("jpgdec") requirement for MT8189, while preserving the
    two-clock requirement for older SoCs.
  - Updated commit message to reflect the schema structure changes and
    hardware differences.
- Patches 2/3 (dt-bindings: encoder):
  - Changed the MT8189 compatible to be a standalone `const` instead of
    an `enum` inside the `items` list, as it does not fallback to
    "mediatek,mtk-jpgenc" due to 34-bit IOVA requirements.
  - Updated commit message to explain the standalone compatible design.
- Patches 3/3 (media: mediatek: jpeg):
  - Refined commit message for better clarity regarding 34-bit IOVA and
    single clock configuration.

Changes compared with v1:
- Patches 1/4:
  - Updating commit message
- Patches 2/4, 3/4: 
  - Updating commit message
  - Adjusted property descriptions acorrding to hardware requirements
  - Improved formatting for better readability and consistency
- Patches 4/4:
  - Updating commit message

Jianhua Lin (3):
  dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible
    string
  dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible
    string
  media: mediatek: jpeg: add compatible for MT8189 SoC

 .../bindings/media/mediatek-jpeg-decoder.yaml | 44 +++++++++++++++----
 .../bindings/media/mediatek-jpeg-encoder.yaml | 30 ++++++++++---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 44 +++++++++++++++++++
 3 files changed, 104 insertions(+), 14 deletions(-)

-- 
2.45.2

[PATCH v8 1/3] dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible string

[Jianhua Lin]

Add the compatible string for the JPEG decoder block found in the
MediaTek MT8189 SoC.

Compared to previous generation ICs, the MT8189 JPEG decoder requires
34-bit IOVA address space support and only needs a single clock
("jpgdec") instead of two. Therefore, it is added as a standalone
compatible string without falling back to older SoCs.

Update the binding schema to include the new compatible string and add
an `allOf` block with conditional checks. This enforces the single clock
requirement for MT8189 while preserving the two-clock requirement
("jpgdec-smi", "jpgdec") for older SoCs.

Signed-off-by: Jianhua Lin <jianhua.lin@mediatek.com>
---
 .../bindings/media/mediatek-jpeg-decoder.yaml | 44 +++++++++++++++----
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
index a4aacd3eb189..a152c874b53b 100644
--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
@@ -15,10 +15,10 @@ description: |-
 properties:
   compatible:
     oneOf:
-      - items:
-          - enum:
-              - mediatek,mt8173-jpgdec
-              - mediatek,mt2701-jpgdec
+      - enum:
+          - mediatek,mt2701-jpgdec
+          - mediatek,mt8173-jpgdec
+          - mediatek,mt8189-jpgdec
       - items:
           - enum:
               - mediatek,mt7623-jpgdec
@@ -32,13 +32,16 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
     maxItems: 2
-    minItems: 2
 
   clock-names:
-    items:
-      - const: jpgdec-smi
-      - const: jpgdec
+    oneOf:
+      - items:
+          - const: jpgdec
+      - items:
+          - const: jpgdec-smi
+          - const: jpgdec
 
   power-domains:
     maxItems: 1
@@ -51,6 +54,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
@@ -60,6 +67,27 @@ required:
   - power-domains
   - iommus
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8189-jpgdec
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names:
+          maxItems: 1
+      required:
+        - mediatek,larb
+    else:
+      properties:
+        clocks:
+          minItems: 2
+        clock-names:
+          minItems: 2
+
 additionalProperties: false
 
 examples:
-- 
2.45.2

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

[Jianhua Lin]

Add the compatible string for the JPEG encoder block found in the
MediaTek MT8189 SoC.

Unlike some previous SoCs, the MT8189 JPEG encoder requires 34-bit IOVA
address space support. Therefore, it is added as a standalone compatible
string without falling back to the generic "mediatek,mtk-jpgenc" to
ensure the driver applies the correct hardware-specific configurations.

Signed-off-by: Jianhua Lin <jianhua.lin@mediatek.com>
---
 .../bindings/media/mediatek-jpeg-encoder.yaml | 30 +++++++++++++++----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
index 5b15f8977f67..f75871e72633 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
+      - items:
+          - enum:
+              - mediatek,mt2701-jpgenc
+              - mediatek,mt8183-jpgenc
+              - mediatek,mt8186-jpgenc
+              - mediatek,mt8188-jpgenc
+          - const: mediatek,mtk-jpgenc
+
   reg:
     maxItems: 1
 
@@ -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
+
 additionalProperties: false
 
 examples:
-- 
2.45.2

[PATCH v8 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC

[Jianhua Lin]

Compared to the previous generation ICs, the MT8189 uses a 34-bit IOVA
address space (16GB) and requires a single clock configuration.

Therefore, add new compatible strings ("mediatek,mt8189-jpgenc" and
"mediatek,mt8189-jpgdec") along with their specific driver data to
support the JPEG encoder and decoder of the MT8189 SoC.

Signed-off-by: Jianhua Lin <jianhua.lin@mediatek.com>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index d147ec483081..14f2991a4053 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1868,6 +1868,10 @@ static struct clk_bulk_data mt8173_jpeg_dec_clocks[] = {
 	{ .id = "jpgdec" },
 };
 
+static struct clk_bulk_data mtk_jpeg_dec_clocks[] = {
+	{ .id = "jpgdec" },
+};
+
 static const struct mtk_jpeg_variant mt8173_jpeg_drvdata = {
 	.clks = mt8173_jpeg_dec_clocks,
 	.num_clks = ARRAY_SIZE(mt8173_jpeg_dec_clocks),
@@ -1899,6 +1903,38 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
 	.multi_core = false,
 };
 
+static const struct mtk_jpeg_variant mtk8189_jpegenc_drvdata = {
+	.clks = mtk_jpeg_clocks,
+	.num_clks = ARRAY_SIZE(mtk_jpeg_clocks),
+	.formats = mtk_jpeg_enc_formats,
+	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
+	.qops = &mtk_jpeg_enc_qops,
+	.irq_handler = mtk_jpeg_enc_irq,
+	.hw_reset = mtk_jpeg_enc_reset,
+	.m2m_ops = &mtk_jpeg_enc_m2m_ops,
+	.dev_name = "mtk-jpeg-enc",
+	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+	.support_34bit = true,
+};
+
+static const struct mtk_jpeg_variant mtk8189_jpegdec_drvdata = {
+	.clks = mtk_jpeg_dec_clocks,
+	.num_clks = ARRAY_SIZE(mtk_jpeg_dec_clocks),
+	.formats = mtk_jpeg_dec_formats,
+	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
+	.qops = &mtk_jpeg_dec_qops,
+	.irq_handler = mtk_jpeg_dec_irq,
+	.hw_reset = mtk_jpeg_dec_reset,
+	.m2m_ops = &mtk_jpeg_dec_m2m_ops,
+	.dev_name = "mtk-jpeg-dec",
+	.ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
+	.support_34bit = true,
+};
+
 static struct mtk_jpeg_variant mtk8195_jpegenc_drvdata = {
 	.formats = mtk_jpeg_enc_formats,
 	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
@@ -1938,6 +1974,14 @@ static const struct of_device_id mtk_jpeg_match[] = {
 		.compatible = "mediatek,mtk-jpgenc",
 		.data = &mtk_jpeg_drvdata,
 	},
+	{
+		.compatible = "mediatek,mt8189-jpgenc",
+		.data = &mtk8189_jpegenc_drvdata,
+	},
+	{
+		.compatible = "mediatek,mt8189-jpgdec",
+		.data = &mtk8189_jpegdec_drvdata,
+	},
 	{
 		.compatible = "mediatek,mt8195-jpgenc",
 		.data = &mtk8195_jpegenc_drvdata,
-- 
2.45.2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch silently adds the `mediatek,larb` property and makes it required for the `mediatek,mt8189-jpgdec` compatible without documenting this change in the commit message.
--

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

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

This commit adds the compatible string for the MT8189 JPEG decoder, updating
the clock requirements to a single clock and enforcing conditional checks
for the new hardware.

> Compared to previous generation ICs, the MT8189 JPEG decoder requires
> 34-bit IOVA address space support and only needs a single clock
> ("jpgdec") instead of two. Therefore, it is added as a standalone
> compatible string without falling back to older SoCs.
> 
> Update the binding schema to include the new compatible string and add
> an `allOf` block with conditional checks. This enforces the single clock
> requirement for MT8189 while preserving the two-clock requirement
> ("jpgdec-smi", "jpgdec") for older SoCs.

The commit message details the changes to the clock requirements and IOVA
address space, but it omits the introduction of a new hardware dependency.

> diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
> --- a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml

[ ... ]

> @@ -60,6 +67,27 @@ required:
>    - power-domains
>    - iommus
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8189-jpgdec
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names:
> +          maxItems: 1
> +      required:
> +        - mediatek,larb

Could the commit message be expanded to explain why the mediatek,larb
property is being added and made required for the MT8189 hardware?

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

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

[sashiko-bot]

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

Re: [PATCH v8 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Missing cancellation of `job_timeout_work` in `mtk_jpeg_remove()` leads to a Use-After-Free.
- [High] Variant structures use globally shared static arrays for clock definitions, which are illegally mutated during device probe by `devm_clk_bulk_get()`.
--

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

media: mediatek: jpeg: add compatible for MT8189 SoC

The commit adds support for the MT8189 SoC's JPEG encoder and decoder by
introducing new compatible strings. It also adds the associated driver data
structures configuring the hardware for a 34-bit IOVA space and single
clock setup.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index d147ec4830811..14f2991a40535 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1868,6 +1868,10 @@ static struct clk_bulk_data mt8173_jpeg_dec_clocks[] = {
>  	{ .id = "jpgdec" },
>  };
>  
> +static struct clk_bulk_data mtk_jpeg_dec_clocks[] = {
> +	{ .id = "jpgdec" },
> +};
> +
>  static const struct mtk_jpeg_variant mt8173_jpeg_drvdata = {
>  	.clks = mt8173_jpeg_dec_clocks,

[ ... ]

> @@ -1899,6 +1903,38 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
>  	.multi_core = false,
>  };
>  
> +static const struct mtk_jpeg_variant mtk8189_jpegenc_drvdata = {
> +	.clks = mtk_jpeg_clocks,

Does this code pass a shared static array to devm_clk_bulk_get()?

During device probe, the global mtk_jpeg_clocks array is passed directly:

mtk_jpeg_probe()
    devm_clk_bulk_get(jpeg->dev, jpeg->variant->num_clks, jpeg->variant->clks)

The devm_clk_bulk_get() API modifies the array in-place by writing the
acquired struct clk pointers into it.

If multiple device instances probe, or if unbind/rebind cycles occur, could
the clock pointers be clobbered by subsequent probes? This might lead to
hardware lockups since clocks wouldn't be enabled or disabled properly, or
use-after-free conditions when devres frees the clocks but dangling pointers
remain in the global array.

Should the driver dynamically allocate a per-device clk_bulk_data array
during probe instead?

> +	.num_clks = ARRAY_SIZE(mtk_jpeg_clocks),
> +	.formats = mtk_jpeg_enc_formats,
> +	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
> +	.qops = &mtk_jpeg_enc_qops,
> +	.irq_handler = mtk_jpeg_enc_irq,
> +	.hw_reset = mtk_jpeg_enc_reset,
> +	.m2m_ops = &mtk_jpeg_enc_m2m_ops,
> +	.dev_name = "mtk-jpeg-enc",
> +	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
> +	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
> +	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> +	.support_34bit = true,
> +};
> +

This isn't a bug in this patch, but looking at the single-core implementation
that this variant uses, mtk_jpeg_single_core_init() sets up the workqueue:

mtk_jpeg_single_core_init()
    INIT_DELAYED_WORK(&jpeg->job_timeout_work, mtk_jpeg_job_timeout_work);

However, mtk_jpeg_remove() doesn't cancel this work:

mtk_jpeg_remove()
    pm_runtime_disable(&pdev->dev);
    video_unregister_device(jpeg->vdev);
    v4l2_m2m_release(jpeg->m2m_dev);
    v4l2_device_unregister(&jpeg->v4l2_dev);

If the driver is unbound while a job is active, mtk_jpeg_remove() will
complete and the devm subsystem will free the jpeg structure. If the timeout
work fires afterwards and executes mtk_jpeg_job_timeout_work(), will this
result in a use-after-free when it dereferences the freed jpeg pointer?

> +static const struct mtk_jpeg_variant mtk8189_jpegdec_drvdata = {
> +	.clks = mtk_jpeg_dec_clocks,

Does this structure also pass a shared static array to devm_clk_bulk_get(),
similar to mtk8189_jpegenc_drvdata above?

[ ... ]

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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3314 bytes --]

On Tue, May 19, 2026 at 02:35:51AM +0000, sashiko-bot@kernel.org wrote:
> 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?

No it doesn't. Also "This isn't a bug, but it is a bug" is how this
sentence reads, makes no sense.

It should only be a const if there's not likely to be other devices
joining the enum.

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

Yes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]