All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kurt Borja" <kuurtb@gmail.com>
To: "David Lechner" <dlechner@baylibre.com>,
	"Kurt Borja" <kuurtb@gmail.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Tobias Sperling" <tobias.sperling@softing.com>
Cc: "Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Jonathan Cameron" <Jonathan.Cameron@huawei.com>
Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1018/ADS1118
Date: Fri, 21 Nov 2025 19:26:24 -0500	[thread overview]
Message-ID: <DEESPC8XZSXI.3G1JISSNEBC3Q@gmail.com> (raw)
In-Reply-To: <09d8aa0c-8713-4673-8d70-ff5d59392c12@baylibre.com>

On Fri Nov 21, 2025 at 5:32 PM -05, David Lechner wrote:
> On 11/21/25 11:16 AM, Kurt Borja wrote:
>> Add documentation for Texas Instruments ADS1018 and ADS1118
>> analog-to-digital converters.
>> 
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>  .../devicetree/bindings/iio/adc/ti,ads1118.yaml    | 132 +++++++++++++++++++++
>>  1 file changed, 132 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1118.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1118.yaml
>> new file mode 100644
>> index 000000000000..eb7228ed6ddb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1118.yaml
>
> I like to make the file name match the lowest number. That way it matches
> the first item in the compatible list.
>
>> @@ -0,0 +1,132 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>
> Better to be specific with GPL-2.0-only or GPL-2.0-or-later (your choice).

I will go for GPL-2.0-only on both files.

>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1118.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TI ADS1018/ADS1118 SPI analog to digital converter
>> +
>> +maintainers:
>> +  - Kurt Borja <kuurtb@gmail.com>
>> +
>> +description: |
>> +  The ADS1018/ADS1118 is a precision, low-power, 12-bit or 16-bit, noise-free,
>> +  analog-to-digital converter (ADC). It integrates a programmable gain amplifier
>> +  (PGA), voltage reference, oscillator and high-accuracy temperature sensor.
>> +
>> +  Datasheets:
>> +    - ADS1018: https://www.ti.com/lit/ds/symlink/ads1018.pdf
>> +    - ADS1118: https://www.ti.com/lit/ds/symlink/ads1118.pdf
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,ads1018
>> +      - ti,ads1118
>> +
>> +  reg:
>> +    maxitems: 1
>
> We know the max SCLK rate from the datasheet, so we can add:
>
> spi-max-frequency:
>   maximum: 4000000
>
> And we need to explicitly mention this one (it isn't part of
> spi-peripheral-props.yaml but rather comes from spi-controller.yaml):
>
> spi-cpha: true
>
>> +
>> +  interrupts:
>> +    description: DOUT/DRDY (Data Out/Data Ready) line.
>> +    maxitems: 1
>> +
>> +  drdy-gpios:
>> +    description:
>> +      Extra GPIO line connected to DOUT/DRDY (Data Out/Data Ready). This allows
>> +      distinguishing between latched and real DRDY IRQs.
>> +    maxitems: 1
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +  '#io-channel-cells':
>> +    const: 1
>> +
>
> All chips need power, so we should have:
>
> vdd-supply: true
>
> and make it required.
>
>> +required:
>> +  - compatible
>> +  - reg
>> +  - drdy-gpios
>
> The chip can be used without needing the DRDY signal, so drdy-gpios and
> interrupts should not be required.
>
>> +  - '#address-cells'
>> +  - '#size-cells'
>> +
>> +patternProperties:
>> +  "^channel@[0-7]$":
>> +    type: object
>> +    $ref: /schemas/iio/adc/adc.yaml#
>> +    description: Properties for a single ADC channel.
>> +
>> +    properties:
>> +      reg:
>> +        minimum: 0
>> +        maximum: 7
>> +        description: The channel index (0-7).
>> +
>> +      ti,gain:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 0
>> +        maximum: 5
>> +        description:
>> +          Programmable gain amplifier configuration, as described in the PGA
>> +          Config Register Field description. If not present, the default is
>> +          used.
>
> The gain is programmable, so normally, we would set that by writing
> to the in_voltageY_scale attribute rather than hard-coding it in the
> devicetree.
>
>> +
>> +      ti,datarate:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 0
>> +        maximum: 7
>> +        description:
>> +          Data rate configuration, as described in the DR Config Register Field
>> +          description. If not present, the default is used.
>
> Likewise, the data rate should be set at runtime using a
> sampling_frequency attribute rather than being hard-coded
> in the devicetree.

I actually took this approach from ti-ads1015. I was debating this
myself.

I'll gladly drop these.

>
>> +
>> +    required:
>> +      - reg
>> +
>> +    additionalProperties: false
>> +
> So we shouldn't need explicit channel properties at this time.

Ack for the rest of comments.


-- 
 ~ Kurt


  reply	other threads:[~2025-11-22  0:26 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-21 17:16 [PATCH 0/2] iio: Add support for TI ADS1X18 ADCs Kurt Borja
2025-11-21 17:16 ` [PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1018/ADS1118 Kurt Borja
2025-11-21 18:30   ` Rob Herring (Arm)
2025-11-21 19:10   ` Krzysztof Kozlowski
2025-11-21 20:56     ` Kurt Borja
2025-11-21 22:40       ` David Lechner
2025-11-22  0:25         ` Kurt Borja
2025-11-22  9:34         ` Krzysztof Kozlowski
2025-11-22 15:09           ` David Lechner
2025-11-22 16:14             ` Krzysztof Kozlowski
2025-11-21 22:32   ` David Lechner
2025-11-22  0:26     ` Kurt Borja [this message]
2025-11-21 17:16 ` [PATCH 2/2] iio: adc: Add ti-ads1x18 driver Kurt Borja
2025-11-21 22:33   ` David Lechner
2025-11-22  0:24     ` Kurt Borja
2025-11-22 15:56       ` David Lechner
2025-11-26 20:41         ` Kurt Borja
2025-11-22 10:31   ` kernel test robot
2025-11-21 22:32 ` [PATCH 0/2] iio: Add support for TI ADS1X18 ADCs David Lechner
2025-11-21 23:28   ` Kurt Borja
2025-11-22  0:02     ` David Lechner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DEESPC8XZSXI.3G1JISSNEBC3Q@gmail.com \
    --to=kuurtb@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    --cc=tobias.sperling@softing.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.