linux-kernel - Re: [PATCH 1/2] dt-bindings: iio: adc: ti-ads1298: Add driver

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: <02cbc31e-ec6e-4b3b-940d-da51567c197b@topic.nl>
Date: Wed, 20 Dec 2023 11:58:00 +0100
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
CC: devicetree@...r.kernel.org, linux-iio@...r.kernel.org,
 Conor Dooley <conor+dt@...nel.org>, Jonathan Cameron <jic23@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Lars-Peter Clausen <lars@...afoo.de>, Rob Herring <robh+dt@...nel.org>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: ti-ads1298: Add driver


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@...ic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail
On 14-12-2023 12:11, Jonathan Cameron wrote:
> On Wed, 13 Dec 2023 10:47:21 +0100
> Mike Looijmans <mike.looijmans@...ic.nl> wrote:
> 
>> Skeleton driver for the TI ADS1298 medical ADC. This device is
>> typically used for ECG and similar measurements. Supports data
>> acquisition at configurable scale and sampling frequency.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
>>
>> ---
>>
>>   .../bindings/iio/adc/ti,ads1298.yaml          | 80 +++++++++++++++++++
>>   1 file changed, 80 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
>> new file mode 100644
>> index 000000000000..7a160ba721eb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
>> @@ -0,0 +1,80 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1298.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments' ads1298 medical ADC chips
>> +
>> +maintainers:
>> +  - Mike Looijmans <mike.looijmans@...ic.nl>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,ads1298
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  spi-cpha: true
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>> +  avdd-supply:
>> +    description:
>> +      Analog power supply, voltage between AVDD and AVSS. When providing a
>> +      symmetric +/- 2.5V, the regulator should report 5V.
> Commented on in other thread.
>> +
>> +  vref-supply:
>> +    description:
>> +      Optional reference voltage. If omitted, internal reference is used,
>> +      depending on analog supply this is 2.4 or 4V.
>> +
> There is a dvdd-supply as well.  Might be others. Makes sure to document them all.

Extra supplies make sense, I'll add them.

> 
> Should probably also document the gpios as the binding should attempt to be as complete
> as possible independent of what the driver currently supports.
> 
> Lots of complex options for this device and the use of those pins, so maybe something
> that can be left for now - but the patch description should then mention that is intentional.

The device has so many options for connecting stuff... It's indeed possible to 
(also) use it as a GPIO expander and as a clock source and more...

I'll put it in the patch description that the definition is incomplete by design.

The main reason I'm submitting is that this is about the third time I've 
written a driver for this chip, and I'm sure that other companies are writing 
their own as well. I'm hoping this will result in some joint effort to 
properly support it...


> 
> Jonathan
> 
>> +  clocks:
>> +    description: Optional 2.048 MHz external source clock on CLK pin
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    const: clk
>> +
>> +  interrupts:
>> +    description: Interrupt on DRDY pin, triggers on falling edge
>> +    maxItems: 1
>> +
>> +  label: true
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - avdd-supply
>> +  - interrupts
>> +
>> +allOf:
>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    spi {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        adc@1 {
>> +          reg = <1>;
>> +          compatible = "ti,ads1298";
>> +          label = "ads1298-1-ecg";
>> +          avdd-supply = <&reg_iso_5v_a>;
>> +          clock-names = "clk";
>> +          clocks = <&clk_ads1298>;
>> +          interrupt-parent = <&gpio0>;
>> +          interrupts = <78 IRQ_TYPE_EDGE_FALLING>;
>> +          spi-max-frequency = <20000000>;
>> +          spi-cpha;
>> +        };
>> +    };
>> +...
>