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

[Marc Dietrich <marvin24@gmx.de>]

Hi,

On Sun, 8 Nov 2026, sashiko-bot@kernel.org wrote:

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

it is the intended behaviour to separate the two drivers in order to keep 
the code mantainable. The controller driver is already very large in order 
to support the various tegra models. If we also add the target driver to 
the controller driver (potentially adding more variants in the future), 
the driver would grow even more.
That said, the limitation is that we cannot support both controller and 
target on the same instance, but I guess there are only limited uses cases 
for the scenario.

>> @@ -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?

not sure if this is required. The machine binding typically includes the
SoC binding, which already defines all the required properties.

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

see above.

Marc