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

[Rob Herring <robh@kernel.org>]

On Fri, Sep 04, 2026 at 01:40:25PM +0800, nina_kuo@novatek.com.tw wrote:
> From: Ben Huang <Ben_Huang@novatek.com.tw>
> 
> Hi,
> 
> Thanks for the review.
> 
> On Friday, September 4, 2026 12:05 AM, Rob Herring <robh@kernel.org> 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.

Different interrupts and clocks is every IP block. That's why we have 
'interrupts' and 'clocks' properties.

For ownership switch, that can be based on having the 
novatek,stbc-syscon property or not.

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

Okay.

Rob