[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <184bf60f-f803-48a0-a854-badc14584e53@kernel.org>
Date: Thu, 24 Jul 2025 11:02:58 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Lakshay Piplani <lakshay.piplani@....com>, linux-kernel@...r.kernel.org,
linux-iio@...r.kernel.org, jic23@...nel.org, dlechner@...libre.com,
nuno.sa@...log.com, andy@...nel.org, marcelo.schmitt1@...il.com,
gregkh@...uxfoundation.org, viro@...iv.linux.org.uk, peterz@...radead.org,
jstephan@...libre.com, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, devicetree@...r.kernel.org
Cc: vikash.bansal@....com, priyanka.jain@....com,
shashank.rebbapragada@....com, Frank.Li@....com, carlos.song@....com,
xiaoning.wang@....com, haibo.chen@....com
Subject: Re: [PATCH 1/2] dt-bindings: iio: temperature: Add NXP P3T175x
support.
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
Powered by blists - more mailing lists