Re: [PATCH v1 1/3] dt-bindings: i2c: Add Qualcomm I2C slave controller

[Viken Dadhaniya]

On 6/29/2026 12:02 PM, Krzysztof Kozlowski wrote:
> On 28/06/2026 16:39, Viken Dadhaniya wrote:
>> +description:
>> +  The Qualcomm I2C slave controller is a dedicated hardware IP found on
>> +  Qualcomm SoCs that operates exclusively as an I2C slave device on the bus.
>> +  It supports FIFO (PIO) mode for data transfer and does not function as an
>> +  I2C master.
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,i2c-slave
> 
> Such compatible makes little sense in upstream. Please look at other
> bindings how to properly create a compatible. This has to be device
> specific.
> 

Sure, will update with an SoC-specific compatible string
(e.g., qcom,qdu1000-i2c-target) in the next revision.

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: XO clock for I2C slave
>> +      - description: AHB clock for I2C slave
>> +
>> +  clock-names:
>> +    items:
>> +      - const: sm_bus_xo_clk
>> +      - const: sm_bus_ahb_clk
> 
> Look at other bindings.
> 

Sure, updated the clock names to xo and ahb in the next revision.

>> +
>> +  qcom,slave-addr:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      The 7-bit I2C slave address programmed into the controller's device
>> +      address register. This controller operates exclusively as an I2C slave
>> +      and has no child nodes, so the standard 'reg' property cannot be used
>> +      to convey the I2C address (it is already used for the MMIO register
>> +      range). The address is configurable at runtime via a hardware register.
>> +    minimum: 0x08
>> +    maximum: 0x77
>> +
>> +  interconnects:
>> +    maxItems: 1
>> +
>> +  interconnect-names:
>> +    const: i2c-slave-config
> 
> i2c
> 

Sure, will update to i2c in the next revision.

>> +
>> +  pinctrl-0: true
>> +  pinctrl-1: true
>> +
>> +  pinctrl-names:
>> +    minItems: 1
>> +    items:
>> +      - const: default
>> +      - const: sleep
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - qcom,slave-addr
>> +  - interconnects
>> +  - interconnect-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/qcom,qdu1000-gcc.h>
>> +    #include <dt-bindings/interconnect/qcom,qdu1000-rpmh.h>
>> +
>> +    i2c-slave@88ca000 {
> 
> You need to fix your terminology everywhere...
> 

Will update in the next revision.

> 
> Best regards,
> Krzysztof