linux-kernel - Re: [PATCH 2/3] dt-bindings: iio: proximity: Add Nicera D3-323-AA PIR sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <02b0dbc2-e8fd-419a-b546-5910b3fd0e0c@kernel.org>
Date: Fri, 9 May 2025 17:06:56 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Waqar Hameed <waqar.hameed@...s.com>, Jonathan Cameron
 <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: kernel@...s.com, linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: iio: proximity: Add Nicera D3-323-AA PIR
 sensor

On 09/05/2025 17:03, Waqar Hameed wrote:
> Nicera D3-323-AA is a PIR sensor for human detection. It has support for
> raw data measurements and detection notification. The communication
> protocol is custom made and therefore needs to be GPIO bit banged.
> 
> Add devicetree bindings requiring the compatible string and the various
> GPIOs needed for operation. Some of the GPIOs have multiple use-cases
> depending on device state. Describe these thoroughly.


Drop redundant parts of description. Describe the hardware. Entire last
paragraph is pretty pointless.

> +
> +properties:
> +  compatible:
> +    const: nicera,d3323aa
> +
> +  vdd-gpio:
> +    maxItems: 1
> +    description:
> +      GPIO for supply voltage (1.8 to 5.5 V).

This is not how pins are represented in the kernel. Either you have here
regulator (supply) or reset gpios. Plus 'gpio' suffix is not valid, btw.

Datasheet says this is a supply.

> +      This GPIO will be driven low by the driver in order to cut the supply and
> +      reset the device (driving it then back to high to power it on).
> +
> +  clk-vout-gpio:

No, for the similar reasons. Which pin is this?

> +    maxItems: 1
> +    description:
> +      GPIO for clock and detection.
> +      After reset, the device signals with two falling edges on this pin that it
> +      is ready for configuration (within 1.2 s), which the driver listens for as
> +      interrupts.
> +      During configuration, it is used as clock for data reading and writing (on
> +      data-gpio). The driver drives this pin with the frequency of 1 kHz (bit
> +      banging).
> +      After all this, the device is now in operational mode and signals on this
> +      pin for any detections. The driver listens for this as interrupts.
> +
> +  data-gpio:

There is no such pin.

> +    maxItems: 1
> +    description:
> +      GPIO for data reading and writing.
> +      During configuration, configuration data will be written and read back
> +      with bit banging (together with clk-vout-gpio as clock).
> +      After this, during operational mode, the device will output serial data on
> +      this GPIO. However, the driver currently doesn't read this.
> +
> +required:
> +  - compatible
> +  - vdd-gpio
> +  - clk-vout-gpio
> +  - data-gpio
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>

So you included that header

> +
> +    proximity {
> +        compatible = "nicera,d3323aa";
> +        vdd-gpio = <&gpio 73 0>;
> +        clk-vout-gpio = <&gpio 78 0>;
> +        data-gpio = <&gpio 76 0>;

But where are you using it?

> +    };
> +...


Best regards,
Krzysztof