lkml.org 
[lkml]   [2023]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v2 1/2] dt-bindings: hwmon: (lm75) Add AMS AS6200 temperature sensor
On Tue, Dec 19, 2023 at 03:18:24PM +0000, Conor Dooley wrote:
> On Sun, Dec 17, 2023 at 11:52:27PM -0500, Abdel Alkuor wrote:
>
> 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
>
No, not all of them support the interrupt. Just tmp101, tmp102, tmp112, and as6200.
For now, I'll add the check for ams,as6200.
> 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.
>
It is an additional feature. The interrupt basically notifies the user space when the
alarm state changes through temp1_alarm sysfs using poll on the file for example. That
being said, we should still be able to read the alarm state for as6200 without the
interrupt present.
> > +
> > 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?
Sure.

Thanks,
Abdel


\
 
 \ /
  Last update: 2023-12-19 17:28    [W:0.066 / U:1.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site