Re: [PATCH v8 1/3] dt-bindings: i2c: Add Novatek NT72676 I2C controller

[nina_kuo]

From: Ben Huang <Ben_Huang@xxxxxxxxxxxxxx>

Hi,

Thanks for the review.

On Friday, September 4, 2026 12:05 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
>> +description:
>> +  Novatek NT72676 SoC provides two I2C controller variants, generic I2C and
>> +  STBC I2C.
>
> blank line between paragraphs and you need a '>' modifier on 
> 'description'.

I will change the top-level description to use the folded block style and
add blank lines between paragraphs.

>> +  The generic I2C controllers are accessible only by the host CPU and driven
>> +  by a 96 MHz AHB clock.
>> +  On the other hand, the STBC I2C controllers can be controlled by either the
>> +  host CPU or STBC (Standby Controller, an 8051-based microcontroller) and
>> +  driven by a 12 MHz STBC clock. Authentication with STBC is required before
>> +  the host CPU can access the STBC I2C controller registers.
>
> Is the programming model of the I2C controller different? If not, the 
> compatible should be the same.

The register layout and transfer programming model are the same, but these
are separate physical IP blocks with different hardware integration. They
have different interrupt sources and input clocks, and the STBC instance
requires an authentication and ownership-switch sequence before its
registers can be accessed by the host CPU. These differences are fixed
properties of the two IP blocks, so separate compatible strings are used.

>> +  novatek,stbc-syscon:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      Phandle to the STBC system controller registers used to authenticate
>> +      host access and switch ownership of the STBC I2C controller.
>
> Sounds like this could be modelled as a hwlock? That's other platforms 
> mediate access to shared h/w. We really only want to have these syscon 
> phandles as a last resort.

The ownership mechanism is a one-time switch rather than a hardware lock.
The presence of the STBC I2C node in the Device Tree means that the
controller is assigned to the host CPU. The driver authenticates and switches
ownership during initialization, after which the controller remains under
host CPU control. The STBC does not access the controller concurrently, and
there is no runtime acquire/release or lock arbitration. Therefore, the
generic hwlock binding does not match this hardware mechanism.
 
I will update the binding description to make these hardware characteristics
explicit.

Sincerely,
Ben Huang