* [PATCH v8 1/3] dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible string
2026-05-19 2:17 [PATCH v8 0/3] Mediatek MT8189 JPEG support Jianhua Lin
@ 2026-05-19 2:17 ` Jianhua Lin
2026-05-19 2:26 ` sashiko-bot
2026-05-19 2:17 ` [PATCH v8 2/3] dt-bindings: media: mediatek-jpeg-encoder: " Jianhua Lin
2026-05-19 2:17 ` [PATCH v8 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC Jianhua Lin
2 siblings, 1 reply; 8+ messages in thread
From: Jianhua Lin @ 2026-05-19 2:17 UTC (permalink / raw)
To: nicolas, mchehab, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno
Cc: devicetree, linux-kernel, linux-media, linux-arm-kernel,
linux-mediatek, Project_Global_Chrome_Upstream_Group, sirius.wang,
vince-wl.liu, jh.hsu, 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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v8 1/3] dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible string
2026-05-19 2:17 ` [PATCH v8 1/3] dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible string Jianhua Lin
@ 2026-05-19 2:26 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-05-19 2:26 UTC (permalink / raw)
To: Jianhua Lin; +Cc: conor+dt, robh, devicetree
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v8 2/3] dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible string
2026-05-19 2:17 [PATCH v8 0/3] Mediatek MT8189 JPEG support Jianhua Lin
2026-05-19 2:17 ` [PATCH v8 1/3] dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible string Jianhua Lin
@ 2026-05-19 2:17 ` Jianhua Lin
2026-05-19 2:35 ` sashiko-bot
2026-05-19 2:17 ` [PATCH v8 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC Jianhua Lin
2 siblings, 1 reply; 8+ messages in thread
From: Jianhua Lin @ 2026-05-19 2:17 UTC (permalink / raw)
To: nicolas, mchehab, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno
Cc: devicetree, linux-kernel, linux-media, linux-arm-kernel,
linux-mediatek, Project_Global_Chrome_Upstream_Group, sirius.wang,
vince-wl.liu, jh.hsu, 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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v8 2/3] dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible string
2026-05-19 2:17 ` [PATCH v8 2/3] dt-bindings: media: mediatek-jpeg-encoder: " Jianhua Lin
@ 2026-05-19 2:35 ` sashiko-bot
2026-05-19 17:17 ` Conor Dooley
0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-05-19 2:35 UTC (permalink / raw)
To: Jianhua Lin; +Cc: robh, conor+dt, devicetree
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v8 2/3] dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible string
2026-05-19 2:35 ` sashiko-bot
@ 2026-05-19 17:17 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2026-05-19 17:17 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Jianhua Lin, robh, conor+dt, devicetree
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v8 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC
2026-05-19 2:17 [PATCH v8 0/3] Mediatek MT8189 JPEG support Jianhua Lin
2026-05-19 2:17 ` [PATCH v8 1/3] dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible string Jianhua Lin
2026-05-19 2:17 ` [PATCH v8 2/3] dt-bindings: media: mediatek-jpeg-encoder: " Jianhua Lin
@ 2026-05-19 2:17 ` Jianhua Lin
2026-05-19 3:02 ` sashiko-bot
2 siblings, 1 reply; 8+ messages in thread
From: Jianhua Lin @ 2026-05-19 2:17 UTC (permalink / raw)
To: nicolas, mchehab, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno
Cc: devicetree, linux-kernel, linux-media, linux-arm-kernel,
linux-mediatek, Project_Global_Chrome_Upstream_Group, sirius.wang,
vince-wl.liu, jh.hsu, 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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v8 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC
2026-05-19 2:17 ` [PATCH v8 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC Jianhua Lin
@ 2026-05-19 3:02 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-05-19 3:02 UTC (permalink / raw)
To: Jianhua Lin; +Cc: conor+dt, robh, devicetree
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
^ permalink raw reply [flat|nested] 8+ messages in thread