linux-kernel - Re: [PATCH v2 1/2] dt-bindings: hwmon: (lm75) Add AMS AS6200 temperature 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: <20231219-mascot-semester-7d2c492b99bc@spud>
Date: Tue, 19 Dec 2023 15:18:24 +0000
From: Conor Dooley <conor@...nel.org>
To: Abdel Alkuor <alkuor@...il.com>
Cc: Jean Delvare <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Jonathan Corbet <corbet@....net>, linux-hwmon@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: (lm75) Add AMS AS6200
 temperature sensor

On Sun, Dec 17, 2023 at 11:52:27PM -0500, Abdel Alkuor wrote:
> as6200 is a temperature sensor with a range between -40°C to
> 125°C degrees and an accuracy of ±0.4°C degree between 0
> and 65°C and ±1°C for the other ranges.
> 
> Signed-off-by: Abdel Alkuor <alkuor@...il.com>
> ---
> Changes in v2:
>   - Incorporate as6200 into lm75 bindings
> 
>  .../devicetree/bindings/hwmon/lm75.yaml        | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> index 0b69897f0c63..63b85a83ac18 100644
> --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> @@ -14,6 +14,7 @@ properties:
>    compatible:
>      enum:
>        - adi,adt75
> +      - ams,as6200
>        - atmel,at30ts74
>        - dallas,ds1775
>        - dallas,ds75
> @@ -48,6 +49,9 @@ properties:
>    vs-supply:
>      description: phandle to the regulator that provides the +VS supply
>  
> +  interrupts:
> +    maxItems: 1

Do the other devices here have interrupts? If not, you just allowed
interrupts for them. You, at the very least, need to add something like:

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index 63b85a83ac18..d7ce96606400 100644
--- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
+++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
@@ -56,6 +56,17 @@ required:
   - compatible
   - reg
 
+allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: ams,as6200
+    then:
+      properties:
+        interrupts: false
+
 additionalProperties: false
 
 examples:

I had a brief look at the driver though, but I could not immediately
tell if the interrupt was required on the ams,as6200 or if the driver
continued on without that functionality. It seemed like an additional
feature that the interrupt was required for, but if not you should make
the interrupt required for the as6200.

> +
>  required:
>    - compatible
>    - reg
> @@ -66,3 +70,17 @@ examples:
>          vs-supply = <&vs>;
>        };
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        temperature-sensor@48 {
> +            compatible = "ams,as6200";
> +            reg = <0x48>;
> +            vs-supply = <&vs>;
> +            interrupt-parent = <&gpio1>;
> +            interrupts = <17 IRQ_TYPE_EDGE_BOTH>;
> +        };
> +    };

Can you make the indent here match that in the other example in this
file please?

Thanks,
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)