| 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
| ||
|
Date: Sat, 15 Jun 2024 10:23:31 -0500
From: David Lechner <dlechner@...libre.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Michael Hennerich <michael.hennerich@...log.com>,
Nuno Sá <nuno.sa@...log.com>,
Jonathan Corbet <corbet@....net>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs
On 6/15/24 7:41 AM, Jonathan Cameron wrote:
> On Wed, 12 Jun 2024 14:20:40 -0500
> David Lechner <dlechner@...libre.com> wrote:
>
>> Add device tree bindings for AD4695 and similar ADCs.
>>
...
>> +
>> + avdd-supply:
>> + description: A 2.7 V to 5.5 V supply that powers the analog circuitry.
>
> I'm a cynic. Do we care about the supported voltages in this binding doc?
> Feels just somewhere we might make a mistake.
Not especially useful, I suppose. I'll clean these up a bit.
>
>> +
>> + ldo-in-supply:
>> + description: A 2.4 V to 5.5 V supply connected to the internal LDO input.
>> +
>> + vdd-supply:
>> + description: A 1.8V supply that powers the core circuitry.
>> +
>> + vio-supply:
>> + description: A 1.2V to 1.8V supply for the digital inputs and outputs.
>> +
>> + ref-supply:
>> + description: A 2.4 V to 5.1 V supply for the external reference voltage.
>> +
>> + refin-supply:
>> + description: A 2.4 V to 5.1 V supply for the internal reference buffer input.
>> +
>> + com-supply:
>> + description: Common voltage supply for pseudo-differential analog inputs.
>
> These last few have more info in them so definitely good to have the descriptions
>
...
>> +
>> +patternProperties:
>> + "^channel@[0-9a-f]$":
>> + type: object
>> + $ref: adc.yaml
>> + unevaluatedProperties: false
>> + description:
>> + Describes each individual channel. In addition the properties defined
>> + below, bipolar from adc.yaml is also supported.
>> +
>> + properties:
>> + reg:
>> + maximum: 15
>> + description: Input pin number (INx).
>
> I'd drop this description as the pin pairing makes it messy.
> If you switch to diff-channels etc, just leave it as a index value not
> connected to the pin numbers.
>
>> +
>> + adi,pin-pairing:
>> + description: |
>> + The input pin pairing for the negative input. This can be:
>> + - REFGND, normally 0V (single-ended)
>> + - COM, normally V_REF/2, see com-supply (pseudo-differential)
>> + - For even numbered pins, the next odd numbered pin (differential)
>> + $ref: /schemas/types.yaml#/definitions/string
>> + enum: [refgnd, com, next]
>
> Next is full on differential, just provide both channels via
> diff-channels. You can constrain the particular combinations in the binding.
>
> Refcnd is normal single ended. Probably want to use the new single-channel
> property for that as we are mixing differential and single ended channels
> so reg is pretty much just an index.
>
> Hmm. For comm we haven't had done a recent binding for a chip with the option
> of pseudo differential that is per channel, they've been whole device only.
> That feels like it will be common enough we need to support it cleanly
> with a 'general' scheme.
I think we have. :-)
https://lore.kernel.org/linux-iio/adc6cba9-2e79-475f-9c24-039fe9d3345d@baylibre.com/T/#mcbc1ce3a2541db502bf7870b7ea8574626a46312
>
> Problem is I know someone will have a chip with 2 vincom pins and selecting
> between them, so we can't just have pseudo-differential as a boolean and adc.yaml
>
> There are horrible solutions like a magic channel number that changes the
> meaning of diff-channels but that's ugly.
> Maybe pseudo-differential for now and we have to later we add
> pseudo-differential-comm = <0> etc?
>
I was trying to keep things simple with 1 property instead of 3, but we
can drop adi,pin-pairing and use the standard diff-channels, single-channel
and common-mode-channel properties.
>
>> + default: refgnd
>> +
>> + adi,no-high-z:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description: |
>
> Do we need the | given not really formatted?
No. Was probably copy/paste.
>
>> + Enable this flag if the input pin requires the Analog Input High-Z
>> + Mode to be disabled for proper operation.
>> +
...
>> +examples:
>> + - |
>> + #include <dt-bindings/gpio/gpio.h>
>> +
>> + spi {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + adc@0 {
>> + compatible = "adi,ad4695";
>> + reg = <0>;
>> + spi-cpol;
>> + spi-cpha;
>> + spi-max-frequency = <80000000>;
>> + avdd-supply = <&supply_2_5V>;
>> + vdd-supply = <&supply_1_8V>;
>> + vio-supply = <&supply_1_2V>;
>> + ref-supply = <&supply_5V>;
>> + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
Using the standard adc.yaml properties, these would now be:
>> + /* Differential channel between IN0 and IN1. */
>> + channel@0 {
>> + reg = <0>;
diff-channels = <0>, <1>;
>> + bipolar;
>> + };
>> +
>> + /* Single-ended channel between IN2 and REFGND. */
>> + channel@2 {
>> + reg = <2>;
single-channel = <2>;
common-mode-channel = <0>;
>> + };
>> +
>> + /* Pseudo-differential channel between IN3 and COM. */
>> + channel@f {
>> + reg = <3>;
single-channel = <3>;
common-mode-channel = <1>;
>> + bipolar;
>> + };
>> + };
>> + };
>
And I will add a header file with macros for the common mode
channel values.
Powered by blists - more mailing lists