mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Lechner <dlechner@baylibre.com>
To: Lakshay Piplani <lakshay.piplani@nxp.com>,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	jic23@kernel.org, nuno.sa@analog.com, andy@kernel.org,
	marcelo.schmitt1@gmail.com, gregkh@linuxfoundation.org,
	viro@zeniv.linux.org.uk, peterz@infradead.org,
	jstephan@baylibre.com, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, devicetree@vger.kernel.org,
	ilpo.jarvinen@linux.intel.com, jonathan.cameron@huawei.com,
	akpm@linux-foundation.org, chao@kernel.org, jaegeuk@kernel.org
Cc: vikash.bansal@nxp.com, priyanka.jain@nxp.com,
	shashank.rebbapragada@nxp.com, Frank.Li@nxp.com
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: temperature: Add NXP P3T175x support
Date: Wed, 27 Aug 2025 08:59:27 -0500	[thread overview]
Message-ID: <ca078446-7cdf-4922-b550-6dd671d39589@baylibre.com> (raw)
In-Reply-To: <20250827103105.2472328-1-lakshay.piplani@nxp.com>

On 8/27/25 5:31 AM, Lakshay Piplani wrote:
> Add bindings for the NXP P3T175x (P3T1750/P3T1755) temperature
> sensor, supporting both I2C & I3C interfaces.
> 
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
> Changes in v2 (addressed review comments from Krzysztof Kozlowski):
>  - Dropped nxp,alert-active-high: unnecessary as polarity handling is implicit in driver.

>  - Retained nxp,interrupt-mode: required to program TM(thermostat mode) bit; enables interrupt
>    (latched) mode. If not present in DT entry comparator mode is set as default.
>  - Retained nxp,fault-queue: This needs to be configured during device initialization.
>    This property configures the hardware fault queue length. Defines how many consecutive faults
>    are required before ALERT/IBI is asserted, preventing false triggers in noisy environments.

These are not very convincing reasons that these to properties should
be in the devicetree. The devicetree describes how things are wired
up, not how they are used in the driver. For the first one, we already
have the parent node to tell us if we are using I2C or I3C, so the
property is redundant. For the second one, the whole system could be
moved from a less noisy to a more noisy environment and we should not
have to change the devicetree to handle that.

>  - The `reg` property remains required to satisfy `dt_binding_check`.
>  - Fixed YAML formatting, line wrapping, and examples.
>  - Changed compatibles from nxp,p3t1755 to nxp,p3t1755-iio and nxp,p3t1750 to nxp,p3t1750-iio
>    as reported by kernel test robot.
> 
>  .../bindings/iio/temperature/nxp,p3t1755.yaml | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/nxp,p3t1755.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/nxp,p3t1755.yaml b/Documentation/devicetree/bindings/iio/temperature/nxp,p3t1755.yaml
> new file mode 100644
> index 000000000000..4eb6fc5cb247
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/nxp,p3t1755.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/temperature/nxp,p3t1755.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP P3T175x Temperature Sensor
> +
> +maintainers:
> +  - Lakshay Piplani <lakshay.piplani@nxp.com>
> +
> +description: |
> +  Datasheet: https://www.nxp.com/docs/en/data-sheet/P3T1755.pdf
> +
> +  P3T175x (P3T1750/P3T1755) is a digital temperature sensor with a range of -40°C to
> +  +125°C and a 12-bit resolution. It supports communication over
> +  both I2C and I3C interfaces.
> +
> +  The I2C interface supports up to 32 static addresses and provides
> +  an ALERT output to signal when temperature thresholds are crossed.
> +
> +  The I3C interface supports In-Band interrupts (IBI) in interrupt mode,
> +  allowing the device to notify the controller of threshold events without
> +  dedicated alert pin.
> +
> +  The device supports configurable thermostat modes (interrupt or comparator),
> +  fault queue length etc.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,p3t1750-iio
> +      - nxp,p3t1755-iio
> +
> +  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.

As mentioned above, the driver should know best which mode makes the
most sense without having a property to restrict it.

> +      Required for IBI support over I3C. On I2C, both interrupt and
> +      comparator mode support events.
> +
> +  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.

If we kept this, it should have `default: 2`. But as mentioned above,
this doesn't seem like something that would be known when writing
the device tree since it could depend on variable environmental
conditions.

We already have IIO_EV_INFO_RUNNING_COUNT that sounds similar to this
type of control that would allow it to be set at runtime instead.

> +
> +  assigned-address:
> +    true
> +

Missing `vcc-supply: true`, which should also be required.

> +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-iio";
> +            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>;
> +      };
> +    };


  parent reply	other threads:[~2025-08-27 13:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-27 10:31 Lakshay Piplani
2025-08-27 10:31 ` [PATCH v2 2/2] iio: temperature: Add driver for NXP P3T175x temperature sensor Lakshay Piplani
2025-08-27 14:36   ` David Lechner
2025-08-29 15:43   ` Andy Shevchenko
2025-08-31 16:46   ` Krzysztof Kozlowski
2025-09-01 15:47     ` Jonathan Cameron
2025-09-02 11:06       ` Andy Shevchenko
2025-08-27 13:59 ` David Lechner [this message]
2025-08-31 16:01 ` [PATCH v2 1/2] dt-bindings: iio: temperature: Add NXP P3T175x support Jonathan Cameron
2025-08-31 16:47 ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ca078446-7cdf-4922-b550-6dd671d39589@baylibre.com \
    --to=dlechner@baylibre.com \
    --cc=Frank.Li@nxp.com \
    --cc=akpm@linux-foundation.org \
    --cc=andy@kernel.org \
    --cc=chao@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jaegeuk@kernel.org \
    --cc=jic23@kernel.org \
    --cc=jonathan.cameron@huawei.com \
    --cc=jstephan@baylibre.com \
    --cc=krzk+dt@kernel.org \
    --cc=lakshay.piplani@nxp.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo.schmitt1@gmail.com \
    --cc=nuno.sa@analog.com \
    --cc=peterz@infradead.org \
    --cc=priyanka.jain@nxp.com \
    --cc=robh@kernel.org \
    --cc=shashank.rebbapragada@nxp.com \
    --cc=vikash.bansal@nxp.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®