Re: [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] The device tree binding schema is entirely missing the mandatory `examples` section.
- [Low] The `novatek,hwmods` property uses incorrect type constraints, lacks pattern enforcement, and has ambiguous semantics.
- [Low] The `novatek,stbc` property is defined as a `uint32` but its description indicates it is a boolean flag.
- [Low] The `reg` and `interrupts` properties permit multiple items without documenting their meaning or expected order.
--

commit 1cba013f2a2d800236543516887a86edb943712e
Author: Ben Huang <Ben_Huang@novatek.com.tw>

dt-bindings: i2c: add Novatek NT726xx SoC i2c controller
    
Add device tree documentation for Novatek NT726xx SoC i2c controller.

> diff --git a/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
> new file mode 100644
> index 0000000000000..d9dfdaaec205e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
> @@ -0,0 +1,48 @@

[ ... ]

> +properties:
> +  compatible:
> +    const: novatek,nt726xx-i2c
> +
> +  reg:
> +    maxItems: 4
> +
> +  interrupts:
> +    maxItems: 3

[Severity: Low]
These properties permit multiple items without documenting their meaning or
expected order. Should we use an items list with descriptions so that the
purpose and expected order of each entry is defined?

> +
> +  clock-frequency:
> +    description: Operation clock frequency of i2c in Hz.
> +    default: 100000
> +    enum: [ 100000, 400000 ]
> +
> +  novatek,hwmods:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: Name of each i2c pin, must be named with "i2cX". (X is
> +                 an integer starting from 0)
> +    minItems: 1

[Severity: Low]
This property references a scalar string definition, but minItems: 1 is an
array constraint, and the description "Name of each i2c pin" suggests it
accepts multiple names. Should this be defined as a string-array instead?

Also, should we add a pattern regex (like ^i2c[0-9]+$) to enforce the
documented naming rule?

> +
> +  novatek,stbc:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Set if this i2c master controlled by stbc.
> +    minItems: 1

[Severity: Low]
The description "Set if..." outlines a boolean state, but it is defined as a
uint32. Should this be declared using /schemas/types.yaml#/definitions/flag
instead?

Applying minItems: 1 is also redundant for a single scalar type.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - novatek,hwmods
> +
> +unevaluatedProperties: false

[Severity: Low]
Is the mandatory examples block missing from the end of this file?

All new hardware device tree binding YAML schemas require an examples block
to demonstrate how the device tree node should be written and to allow
dt_binding_check validation testing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605035614.383387-1-SP_ISW1_AT@novatek.com.tw?part=2