Re: [PATCH v11 3/6] media: dt-bindings: Add Amlogic V4L2 video decoder

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: media: amlogic,s4-vdec: incomplete validation for interrupts array and names
--

commit 0c8109e65ab164df82b0c800ca621bbe98ed15f7
Author: Zhentao Guo <zhentao.guo@amlogic.com>

media: dt-bindings: Add Amlogic V4L2 video decoder

This patch adds the device tree binding schema for the Amlogic hardware
decoder accelerator. The device supports stateless H.264 decoding on
Amlogic SoCs.

> diff --git a/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
> new file mode 100644
> index 0000000000000..75b9ba3cb08d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
> @@ -0,0 +1,114 @@
[ ... ]
> +  interrupts:
> +    maxItems: 2
> +
> +  interrupt-names:
> +    items:
> +      - const: hvdec
> +      - const: vdec

[Severity: Low]
Does the schema need minItems: 2 here for the interrupts array? Without it,
an incomplete node with only 1 interrupt will pass schema checks while still
failing at the driver level when index 1 is requested.

[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - power-domain-names
> +  - resets
> +  - amlogic,canvas
> +  - amlogic,secure-monitor

[Severity: Low]
Is interrupt-names missing from this required list?

Leaving it out permits incomplete configurations to silently pass schema
validation (dt_binding_check). If the driver fetches the interrupt purely
by index (e.g. platform_get_irq(pdev, IRQ_VDEC) where IRQ_VDEC is enum 1),
omitting interrupt-names from the requirements allows device tree authors
to easily miss the correct ordering or provide only one interrupt, leading
directly to driver probe failures or incorrect interrupt routing at runtime.

[ ... ]
> +      reg-names = "dos",
> +                  "dmc";
> +      interrupts = <GIC_SPI 91 IRQ_TYPE_EDGE_RISING>,
> +                   <GIC_SPI 92 IRQ_TYPE_EDGE_RISING>;
> +      clocks = <&clkc_periphs CLKID_DOS>,

[Severity: Low]
Should interrupt-names be included in this example?

The example currently omits interrupt-names completely, demonstrating how a
functionally incomplete node is permitted by the schema.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-b4-s4-vdec-upstream-v11-0-25939f85ada5@amlogic.com?part=3

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic