Re: [PATCH 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 1d779eb91a256c3147a2ce9798c783f5f76b1f50
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 Tegra20 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 be represented by a separate "nvidia,tegra20-i2c-target"
compatible string?

The three generic Tegra20 I2C controllers (such as i2c@7000c500 in
arch/arm/boot/dts/nvidia/tegra20.dtsi) are the same hardware blocks already
described by "nvidia,tegra20-i2c", with controller and target registers in
the same register aperture.  Defining a second compatible string to bind a
separate i2c-tegra-target driver encodes Linux driver structure and runtime
mode into the Devicetree ABI.  In standard I2C bindings
(i2c-controller.yaml), dual-role controllers keep their hardware compatible
string and select target mode via child nodes marked with
I2C_OWN_SLAVE_ADDRESS in reg.

> @@ -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]
If "nvidia,tegra20-i2c-target" is kept, should it also be added to the
allOf conditional that constrains clocks and clock-names for Tegra20 and
Tegra30?

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
}

Without "nvidia,tegra20-i2c-target" in that enum, nodes using the new
compatible fall back to the top-level clocks definition (minItems: 1) and
do not validate the required "div-clk" and "fast-clk" entries.

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