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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `interrupts` property allows up to 3 items but lacks descriptions (`items:`) and omits an `interrupt-names` definition.
- [Medium] The `resets` property is defined and used in the example but is missing from the `required` list.
--

--- Patch [3]: [PATCH v6 3/6] media: dt-bindings: Add Amlogic V4L2 video decoder ---
commit 289a73f667fca4eaf3038cecf5c55be6082f5801
Author: Zhentao Guo <zhentao.guo@amlogic.com>

media: dt-bindings: Add Amlogic V4L2 video decoder

This commit describes the initial support for the V4L2 stateless video decoder
driver used with the Amlogic S4 (S805X2) platform.

> diff --git a/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

[ ... ]

> +  reg-names:
> +    items:
> +      - const: dos
> +      - const: dmc
> +
> +  interrupts:
> +    maxItems: 3

[Severity: Medium]
Is it possible to add descriptions or an interrupt-names array here?

Without descriptions for each array item or an interrupt-names property,
device tree authors do not have documentation on the required order and
purpose of the 3 interrupts.

Additionally, dt_binding_check cannot enforce the exact number of interrupts
or their order without an items list, meaning a device tree node with only
1 or 2 interrupts could silently pass validation.

> +
> +  clocks:
> +    maxItems: 3

[ ... ]

> +  amlogic,canvas:
> +    description: should point to a canvas provider node
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  secure-monitor:
> +    description: phandle to the secure-monitor node
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - power-domain-names
> +  - amlogic,canvas
> +  - secure-monitor

[Severity: Medium]
Should resets be included in this required properties list?

The resets property is defined above in the schema and used in the example.

Since Amlogic video decoder hardware blocks typically require a hardware
reset line (e.g., the DOS reset) for proper initialization, omitting resets
from the required list might allow an incomplete device tree node to
silently pass schema validation but fail at runtime.

> +
> +additionalProperties: false

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-b4-s4-vdec-upstream-v6-0-6072a7ee7428@amlogic.com?part=3

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