linux-kernel - Re: [PATCH 1/2] dt-bindings: iio: adc: adding MCP3564 ADC

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: <3aa1b03c-2b1f-998c-3cbd-4fabfb436dda@topic.nl>
Date:   Wed, 24 May 2023 12:37:43 +0200
From:   Mike Looijmans <mike.looijmans@...ic.nl>
To:     marius.cristea@...rochip.com, jic23@...nel.org, lars@...afoo.de,
        robh+dt@...nel.org
CC:     linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: adding MCP3564 ADC

On 19-05-2023 18:01, marius.cristea@...rochip.com wrote:
> From: Marius Cristea <marius.cristea@...rochip.com>
> 
> This is the device tree schema for iio driver for
> Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
> Delta-Sigma ADCs with an SPI interface.
> 
> Signed-off-by: Marius Cristea <marius.cristea@...rochip.com>
> ---
>   .../bindings/iio/adc/microchip,mcp3564.yaml   | 88 +++++++++++++++++++
>   1 file changed, 88 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml
> new file mode 100644
> index 000000000000..407a125e3776
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/microchip,mcp3564.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MCP346X and MCP356X ADC Family
> +
> +maintainers:
> +  - Marius Cristea <marius.cristea@...rochip.com>
> +
> +description: |
> +   Bindings for the Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
> +   Delta-Sigma ADCs with an SPI interface.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,mcp3461
> +      - microchip,mcp3462
> +      - microchip,mcp3464
> +      - microchip,mcp3461r
> +      - microchip,mcp3462r
> +      - microchip,mcp3464r
> +      - microchip,mcp3561
> +      - microchip,mcp3562
> +      - microchip,mcp3564
> +      - microchip,mcp3561r
> +      - microchip,mcp3562r
> +      - microchip,mcp3564r
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency: true
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  vref-supply:
> +    description:
> +      Some devices have a specific reference voltage supplied on a different
> +      pin to the other supplies. Needed to be able to establish channel scaling
> +      unless there is also an internal reference available (e.g. mcp3564r)
> +
> +  microchip,hw-device-address:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3
> +    description:
> +      The address is set on a per-device basis by fuses in the factory,
> +      configured on request. If not requested, the fuses are set for 0x1.
> +      The device address is part of the device markings to avoid
> +      potential confusion. This address is coded on two bits, so four possible
> +      addresses are available when multiple devices are present on the same
> +      SPI bus with only one Chip Select line for all devices.
> +
> +  "#io-channel-cells":

That's a typo, "io" should be "iio", right?


> +    const: 1
> +
> +dependencies:
> +  spi-cpol: [ spi-cpha ]
> +  spi-cpha: [ spi-cpol ]
> +
> +required:
> +  - compatible
> +  - microchip,hw-device-address

Should be optional, as the default "1" is going to be what most people will be 
using.

> +
> +additionalProperties: false

Missing properties:

- Interrupt (even though the driver doesn't use it yet)
- push-pull or open-drain config of interrupt pin
- dvdd-supply
- avdd-supply

I suggest using this:

+  clocks:
+    description:
+      Phandle and clock identifier for external sampling clock.
+      If not specified, the internal crystal oscillator will be used.
+    maxItems: 1
+
+  interrupts:
+    description: IRQ line of the ADC
+    maxItems: 1
+
+  drive-open-drain:
+    description:
+      Whether to drive the IRQ signal as push-pull (default) or open-drain. Note
+      that the device requires this pin to become "high", otherwise it will stop
+      converting.
+    type: boolean


> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "microchip,mcp3564r";
> +            reg = <0>;
> +            vref-supply = <&vref_reg>;
> +            spi-cpha;
> +            spi-cpol;
> +            spi-max-frequency = <10000000>;
> +            microchip,hw-device-address = <1>;
> +        };
> +    };
> +...

-- 
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