Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 17 Jun 2023 13:55:32 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Waqar Hameed <waqar.hameed@axis.com>
Cc:     Lars-Peter Clausen <lars@metafoo.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <kernel@axis.com>
Subject: Re: [PATCH 1/2] dt-bindings: iio: proximity: Add bindings for
 Murata IRS-D200
Message-ID: <20230617135532.328dc3c8@jic23-huawei>
In-Reply-To: <9487391b0565434761055b39ba04900bd839580a.1686926857.git.waqarh@axis.com>
References: <cover.1686926857.git.waqarh@axis.com>
        <9487391b0565434761055b39ba04900bd839580a.1686926857.git.waqarh@axis.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 16 Jun 2023 17:10:42 +0200
Waqar Hameed <waqar.hameed@axis.com> wrote:

> Murata IRS-D200 is a PIR sensor for human detection. It uses the I2C bus
> for communication with interrupt support. Add devicetree bindings
> requiring the compatible string, I2C slave address (reg) and interrupts.
> 
> Signed-off-by: Waqar Hameed <waqar.hameed@axis.com>

This device will have some power supplies, so I'd expect those to be both
listed and marked as required (maybe some are optional?)

Other than that and the points in the other review one thing inline about interrupts.

Jonathan

> ---
>  .../iio/proximity/murata,irsd200.yaml         | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/proximity/murata,irsd200.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/proximity/murata,irsd200.yaml b/Documentation/devicetree/bindings/iio/proximity/murata,irsd200.yaml
> new file mode 100644
> index 000000000000..d317fbe7bd50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/murata,irsd200.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/proximity/murata,irsd200.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Murata IRS-D200 PIR sensor
> +
> +maintainers:
> +  - Waqar Hameed <waqar.hameed@axis.com>
> +
> +description: |
> +  PIR sensor for human detection.
> +
> +properties:
> +  compatible:
> +    const: murata,irsd200
> +
> +  reg:
> +    items:
> +      - enum:
> +          - 0x48
> +          - 0x49
> +        description: |
> +          When the AD pin is connected to GND, the slave address is 0x48.
> +          When the AD pin is connected to VDD, the slave address is 0x49.
> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Type should be IRQ_TYPE_EDGE_RISING.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts

If it is possible to remove interrupts from requires  - and hence have
at least a partly functional driver doing basic reading of the sensor
then that is usually a good idea.   Far too many board designers seem
to decide that they don't need to wire up interrupt lines.

If it's really hard then don't worry too much.

Jonathan

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pir@48 {
> +            compatible = "murata,irsd200";
> +            reg = <0x48>;
> +            interrupts = <24 IRQ_TYPE_EDGE_RISING>;
> +        };
> +    };
> +...