Re: [PATCH v2 1/2] dt-bindings: i2c: Add Tegra20 I2C target controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] dt-bindings: i2c: tegra20: driver-specific compatible for
  target mode
- [Low] dt-bindings: i2c: tegra20: missing clock constraints for target
  compatible
--

commit 6d4b197a67e97b02efd062ed9f1c49931c4c0fce
Author: Marc Dietrich <marvin24@gmx.de>

dt-bindings: i2c: Add Tegra20 I2C target controller

This commit adds the "nvidia,tegra20-i2c-target" compatible string and an
example for the Tegra20 I2C target controller to the Tegra I2C binding
schema.

> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> index 51241c1293e3e..2d7ee8e8750d4 100644
> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> @@ -13,11 +13,15 @@ title: NVIDIA Tegra I2C controller driver
>  properties:
>    compatible:
>      oneOf:
> -      - description: Tegra20 has 4 generic I2C controller. This can support
> -          master and slave mode of I2C communication. The i2c-tegra driver
> -          only support master mode of I2C communication. Driver of I2C
> -          controller is only compatible with "nvidia,tegra20-i2c".
> +      - description: Tegra20 has 3 instances of a generic I2C controller.
> +          It supports controller and target mode of I2C communication.
> +          The i2c-tegra driver only supports master mode of I2C communication.
> +          Driver of I2C controller is only compatible with "nvidia,tegra20-i2c".
>          const: nvidia,tegra20-i2c
> +      - description: The i2c-tegra-target driver supports the target mode of
> +          the generic Tegra20 I2C controller. Driver of the I2C target
> +          controller is only compatible with "nvidia,tegra20-i2c-target".
> +        const: nvidia,tegra20-i2c-target

[Severity: Medium]
Should target mode use the existing "nvidia,tegra20-i2c" compatible rather
than introducing a separate driver-specific compatible string?

The three generic Tegra20 I2C instances are identical hardware blocks that
include both master and target registers within the same 0x100 register
aperture, and are already described by "nvidia,tegra20-i2c".  Defining
"nvidia,tegra20-i2c-target" encodes Linux driver partitioning and operating
mode selection into the Devicetree ABI, whereas standard I2C bindings keep
the hardware compatible string and select target mode via child nodes using
I2C_OWN_SLAVE_ADDRESS in their reg property.

[ ... ]

> @@ -222,3 +226,18 @@ examples:
>          #address-cells = <1>;
>          #size-cells = <0>;
>      };
> +
> +  - |
> +    i2c@7000c500 {
> +        compatible = "nvidia,tegra20-i2c-target";
> +        reg = <0x7000c500 0x100>;
> +        interrupts = <0 92 0x04>;
> +        clocks = <&tegra_car 67>, <&tegra_car 124>;
> +        clock-names = "div-clk", "fast-clk";

[Severity: Low]
Does the allOf conditional for Tegra20 and Tegra30 clocks also need to
match "nvidia,tegra20-i2c-target"?

The top-level clocks and clock-names properties only require minItems: 1
without constraining the names, and the allOf rule requiring two clocks
named "div-clk" and "fast-clk" only checks for "nvidia,tegra20-i2c" and
"nvidia,tegra30-i2c":

Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml:
  - if:
      properties:
        compatible:
          contains:
            enum:
              - nvidia,tegra20-i2c
              - nvidia,tegra30-i2c
    then:
      properties:
        clocks:
          minItems: 2
        clock-names:
          items:
            - const: div-clk
            - const: fast-clk

Nodes using "nvidia,tegra20-i2c-target" bypass this check, allowing wrong
clock counts or names to pass dtbs_check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115204522.360063-1-marvin24@gmx.de?part=1