lkml.org 
[lkml]   [2022]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 3/3] dt-bindings: hwmon: Add bindings for max31732
From
On 14/12/2022 15:22, Sinan Divarci wrote:
> Adding bindings for max31732 quad remote temperature sensor

Full stop.

Subject: drop second, redundant "bindings for".

>
> Signed-off-by: Sinan Divarci <Sinan.Divarci@analog.com>
> ---
> .../bindings/hwmon/adi,max31732.yaml | 83 +++++++++++++++++++
> 1 file changed, 83 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/adi,max31732.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,max31732.yaml b/Documentation/devicetree/bindings/hwmon/adi,max31732.yaml
> new file mode 100644
> index 000000000..c701cda95
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/adi,max31732.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2022 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/adi,max31732.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices MAX31732 Temperature Sensor Device Driver

Drop "Device Driver"

> +
> +maintainers:
> + - Sinan Divarci <Sinan.Divarci@analog.com>
> +
> +description: Bindings for the Analog Devices MAX31732 Temperature Sensor Device.

Drop "Bindings for". Actually, either drop entire description or write
something else than title.

> +
> +properties:
> + compatible:
> + enum:
> + - adi,max31732
> +
> + reg:
> + description: I2C address of the Temperature Sensor Device.

Drop description.

> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 2
> +
> + interrupt-names:
> + description: Name of the interrupt pin of max31732 used for IRQ.

Drop description.

> + minItems: 1
> + items:
> + - enum: [ALARM1, ALARM2]
> + - enum: [ALARM1, ALARM2]

This should be fixed, not flexible. Why it's flexible?

lowercase letters only

> +
> + adi,alarm1-interrupt-mode:
> + description: |
> + Enables the ALARM1 output to function in interrupt mode.
> + Default ALARM1 output function is comparator mode.

Why this is a property of DT/hardware? Don't encode policy in DT.

> + type: boolean
> +
> + adi,alarm2-interrupt-mode:
> + description: |
> + Enables the ALARM2 output to function in interrupt mode.
> + Default ALARM2 output function is comparator mode.

Same question.

> + type: boolean
> +
> + adi,alarm1-fault-queue:
> + description: The number of consecutive faults required to assert ALARM1.

Same question - why this number differs with hardware?

> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2, 4, 6]
> + default: 1
> +
> + adi,alarm2-fault-queue:
> + description: The number of consecutive faults required to assert ALARM2.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2, 4, 6]
> + default: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/gpio/gpio.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sensor@1c {
> + compatible = "adi,max31732";
> + reg = <0x1c>;
> + interrupt-parent = <&gpio>;
> + interrupts = <17 IRQ_TYPE_EDGE_BOTH>, <27 IRQ_TYPE_EDGE_BOTH>;
> + interrupt-names = "ALARM1", "ALARM2";
> + adi,alarm1-fault-queue = <4>;
> + adi,alarm2-fault-queue = <2>;
> + adi,alarm2-interrupt-mode;
> + };

Messed indentation.


> + };

Best regards,
Krzysztof

\
 
 \ /
  Last update: 2022-12-14 18:01    [W:1.160 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site