Re: [PATCH 1/2] dt-bindings: iio: temperature: Add NXP P3T175x support.

[Krzysztof Kozlowski <krzk@kernel.org>]

On 24/07/2025 10:39, Lakshay Piplani wrote:
> Add bindings for the NXP P3T175x (P3T1755/P3T1750)
> digital temperature sensor, supporting both I2C &
> I3C interfaces.
> 


Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

Subject: drop full stop.

> +properties:
> +  compatible:
> +    enum:
> +      - nxp,p3t1755
> +      - nxp,p3t1750

Keep the list sorted.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +    description: |
> +      In I2C mode, the device supports up to 32 static addresses.
> +      In I3C mode, the 'reg' property encodes a triplet of
> +      <static-address BCR PID> used for device matching.
> +      Static address is optional if matching is done via PID.
> +
> +  nxp,interrupt-mode:
> +    type: boolean
> +    description: |
> +      Enables interrupt mode (TM = 1), where alerts are latched until
> +      cleared by a register read.
> +      Required for IBI support over I3C. On I2C, both interrupt and
> +      comparator mode support events.

Both properties are redundant because they are implied by the bus, no?

> +
> +  nxp,alert-active-high:
> +    type: boolean
> +    description: |
> +      Only applicable for I2C mode.
> +      Sets the polarity of ALERT pin to active high, if true.

Why are you encoding standard interrupt flags as a new property?

> +      Ignored in I3C mode (which uses IBI signaling).
> +
> +  nxp,fault-queue:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 4, 6]
> +    description: |
> +      Number of consecutive temperature limit
> +      violations required before an alert is triggered.
> +      valid values:- 1, 2, 4 or 6.
> +      If unspecified, hardware default (2) is used.

Why would that be board level configuration?

> +
> +  assigned-address:

:true

and that's it... unless you want to make sure it has a type also for I2C
case? How other I3C device binding solve it?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0x1
> +    maximum: 0xff
> +    description: |
> +      Dynamic address to be assigned to this device. In case static address is
> +      present (first cell of the reg property != 0), this address is assigned
> +      through SETDASA. If static address is not present, this address is assigned
> +      through SETNEWDA after assigning a temporary address via ENTDAA.

But for sure no need to duplicate common schema.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        temp-sensor@48 {
> +            compatible = "nxp,p3t1755";
> +            reg = <0x48>;
> +            nxp,interrupt-mode;
> +            nxp,fault-queue = <6>;
> +            interrupt-parent = <&gpio2>;
> +            interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> +        };
> +    };
> +
> +  - |
> +    i3c {
> +      #address-cells = <3>;
> +      #size-cells = <0>;
> +      temp-sensor@48,236152a00 {
> +        reg = <0x48 0x236 0x152a00>;
> +        assigned-address = <0x50>;
> +      };
> +    };
> +
> +  - |
> +    i3c {

Drop this example.


Best regards,
Krzysztof