Re: [PATCH 1/2] dt-bindings: hwmon: Add D3-323-AA

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 12, 2024 at 12:24:09PM +0800, Hermes Zhang wrote:
> Add Devicetree binding documentation for Nicera D3-323-AA Pyroelectric
> IR sensor.
> 
> Signed-off-by: Hermes Zhang <Hermes.Zhang@xxxxxxxx>
> ---
>  .../bindings/hwmon/nicera,d3-323-aa.yaml      | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/nicera,d3-323-aa.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/nicera,d3-323-aa.yaml b/Documentation/devicetree/bindings/hwmon/nicera,d3-323-aa.yaml
> new file mode 100644
> index 000000000000..31690e630b5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/nicera,d3-323-aa.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/nicera,d3-323-aa.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nicera D3-323-AA Pyroelectric IR sensor
> +
> +maintainers:
> +  - Hermes Zhang <Hermes.Zhang@xxxxxxxx>
> +
> +description: |
> +  Nicera D3-323-AA Pyroelectric IR sensor
> +
> +  datasheet:
> +    https://www.nicera.co.jp/wordpress/wp-content/uploads/2022/01/D3-323-AA_e.pdf
> +
> +properties:
> +  compatible:
> +    const: nicera,d3-323-aa

Undocumented vendor prefix.

It does not look like you tested the bindings, at least after quick
look. Please run 'make dt_binding_check' (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.

It does not look like you tested the DTS against bindings. Please run
'make dtbs_check W=1' (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.

> +
> +  reset-gpios:
> +    description: The GPIO pin connected to the reset pin on the sensor
> +    maxItems: 1
> +
> +  clk-gpios:
> +    description: The GPIO pin connected to the clk pin on the sensor

What is a clk pin? Usually clock pins are clocks, not GPIOs. This needs
explanation


> +    maxItems: 1
> +
> +  si-gpios:
> +    description: The GPIO pin connected to the si pin on the sensor

In all description please drop redundant pieces. It cannot be anything
else than GPIO pin and cannot be connected to something else than the
sensor.

So basically the only useful information you wrote above - after
dropping obvious and redundant pieces - is "si".

Write something useful, what is "si"? This applies also to "clk".
reset-gpios can be without description, because it is obvious - cannot
be anything else than reset pin. OTOH, you could say if pin is active
low or high.

> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reset-gpios
> +  - clk-gpios
> +  - si-gpios
> +

Just one blank line.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    sensor {
> +        compatible = "nicera,d3-323-aa";
> +        reset-gpios = <&gpio4 12 0>;

Include the header and use standard defines for GPIO flags.

Best regards,
Krzysztof





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux