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

[Krzysztof Kozlowski]

On 05/06/2026 05:56, SP_ISW1_AT@xxxxxxxxxxxxxx wrote:
> From: Ben Huang <Ben_Huang@xxxxxxxxxxxxxx>
> 
> Add device tree documentation for Novatek NT726xx SoC i2c controller.
> 
> Signed-off-by: Ben Huang <Ben_Huang@xxxxxxxxxxxxxx>
> Signed-off-by: Novatek i2c <SP_ISW1_AT@xxxxxxxxxxxxxx>

This won't work. Please see submitting patches and explanation of DCO.


> ---
> v2:


Please version your patches correctly, e.g. use b4 or git format-patch
-vX, and add changelog in cover letter or under '---' of individual
patches describing changes from previous version.


Limited review.

> - Add Novatek i2c to Signed-off-by email list.
> - Remove confidential related statements and HTML messages.
> - Fix the potential issues in the device tree document and
>   i2c driver source codes.
> - Fix typos.
> 
> v1:
> https://lore.kernel.org/lkml/20260604060411.355675-1-SP_ISW1_AT@xxxxxxxxxxxxxx/T/#t
> ---
>  .../bindings/i2c/novatek,nt726xx-i2c.yaml     | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
> 
> 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 000000000000..d9dfdaaec205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/novatek,nt726xx-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +maintainers:
> +  - Ben Huang <ben_huang@xxxxxxxxxxxxxx>
> +  - Jason JJ Wu <jason_jj_wu@xxxxxxxxxxxxxx>
> +
> +title: Novatek NT726xx SoC I2C master controller
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: novatek,nt726xx-i2c
> +
> +  reg:
> +    maxItems: 4

You need to list the items instead.

> +
> +  interrupts:
> +    maxItems: 3

You need to list the items instead.

> +
> +  clock-frequency:
> +    description: Operation clock frequency of i2c in Hz.

Drop

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

There is no such syntax. Please do not invent own style.

Anyway, I don't understand the property - drop.

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

No clue what does that mean. What is stbc? Acronym of something? Why
would that be a property of a board?

> +    minItems: 1

Please open existing code. Do you see anywhere uint32 with minItems?
Like anywhere? I really wonder what process lead to such code...



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

Missing example. Please open existing bindings and look how it is done.
do you see i2c controllers without examples?

This binding has several trivial issues which means you did not really
use existing code as starting point, did not read the docs or tutorials
from OSS conferences. I encourage to start with these.

Best regards,
Krzysztof