From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f45.google.com (mail-ot1-f45.google.com [209.85.210.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6895D136349 for ; Mon, 16 Jun 2025 20:41:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750106474; cv=none; b=qv6F4rVqVD7qw7AWi1g57p5gPzktAMFogm8yokdmO3YVLUqieZFVdbpUwZ+YPwhx/G0nlHIrg5UgttTBSFHALWq3T8f1OLPy1p388Ujiep3+JTYnjbn/9YhWlf/iTEabH41ZYSWop3MCpJFDfZhIKHKsaaMmPd5+Lj37YKiAokY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750106474; c=relaxed/simple; bh=I1ADj1YYA/0P9VYww4mRdxVOFiO6OjCF4bRvlcPDmRY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Kj8838mv2uUBaSOUOqh4yTSM5yLpluQ3HBl82tnk5+9NfqVF/dnWS8sI/lKdvPuryKflKebPEuEEI1XFA6i6GlZ0bw5+KqfMwQHY9GhWLgE0krxbb9MNZgILrjRCLARCVMVbd6zp4AYhqrVY05A7n5BzJQj7HNcSZAagpoO2kf4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b=XuHMbROX; arc=none smtp.client-ip=209.85.210.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="XuHMbROX" Received: by mail-ot1-f45.google.com with SMTP id 46e09a7af769-73a43d327d6so1273115a34.2 for ; Mon, 16 Jun 2025 13:41:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1750106470; x=1750711270; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=im5kNIHpg71oIpCldz19fuuBQyresJi1tem3UV0h1IQ=; b=XuHMbROXNaoCbeHPU9sw/Q84sLVt+xnwqOWM+u0WpJLZOyLBHz8Ka+3nl+JTxssMLG pmRCwu2PKecsy0rFNc2Jc28ZFQV5+Qia1uGB7to4uRzev/rykrj7ruQzpPwAJw5dsrB9 av5eemvB8fzUtIk6R1+P8Y2sZMEAhSF8ZGpgjDvGZt5c02ey/YUBOdlKwG7sU2Xl6TWn yq2ALoVYhpDWYONEzjrkUO9Tbql9d40V8sFzXKs7yAaFtK4cw1mMG6npFhII1WORQbd7 ZqIq6jkxNEWa0zfLrKGFWSGz3lHB/2ztudXKJ6STpQS47CJhl0UN1Z6U0EbDr6hnMi6q oAmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750106470; x=1750711270; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=im5kNIHpg71oIpCldz19fuuBQyresJi1tem3UV0h1IQ=; b=Rma0P6ODfqEht9o7tjmkqaQBVGJF66ZSYlIu8ZfkcupvV/Bt3YG9lNR7S1vHvuZByy yCNmpaO03o65CEG0SOrvdhanG75i6MWPY4y9a0eXkm7btT68z9OUAsTzkLBLdMdJmi4L QMFEqDaMl6FzZeSZL44MyR7yvcrppi9CpyQ8pgW9odCuriXsMbWi1xBh3w8gRCNxer2G RmAi19KXwFvpU/LBVx4G+O3IMzzS3J7+RwXYp3oamY2W9SN+AOAtnVVSMzY/Oh6fDxj8 gQyqtfVlfYfKTdyD5DnZTZBEyBOkc9Bn1TTxYShIuBle/9/rBN+mLFkSAm+TPKBJzLSO w2kA== X-Forwarded-Encrypted: i=1; AJvYcCUoIG7VjUbZeX0ZwEjDW/eAkgkyxRVwpicYo95yZ3gKofMWFgLe0DJFulgpsaAI8xKzYBMo/cqAKvgg@vger.kernel.org X-Gm-Message-State: AOJu0YwITCYfumCGmZxM5eSDMnzD2wonFYogyPwQpxtEM0ss3R1c5Ooq 72hgfkDvR7pBpVrEDEubWjkQuRY5dARZbk4i/McFnZ2iJ5eXuvfgL31Sc6mwNaBeuOA= X-Gm-Gg: ASbGnctqQJ1ggMPwf7iANFz/geIUN7CK/Cxjg5qBJMgHaufmwpa/UF1pVz4s58tZVzq GEifl9aBLDWu6QacCCYpnEFPimVcxXs63sfJ7Oo3t4+I7f83Cz0hl+cu/8fI84hILEziNVnYujP HL6gxlATU/dgFYYS/WFEaolVEk34GKmDwvv+snF7e3i7Yf/iUSQSzahd2b8jvQXYiJVe7RNB+n2 dw8t2XEHYiVCUo4WVfMugpwuq1T1G3lLYKh6NJgnD3+LVbVRS2wSawL3xwwOELaqqB2zQtdA9mI 1RmMiu0dJ3VeuYrJObklCMph5bGxHQgOQEZjGbe2lUhK3Kn1fNaDu6FRUJYc0XfwSauCOcbWKYy XFqwA9FXIqjIivCzY+AoTwn7/5o3m0j7ScF4ygA+JapJdUXAt4A== X-Google-Smtp-Source: AGHT+IEsH+GjWx5DBPXhNJpG9GJzxKD3U06ehvPiNJ6zYbwHj4bcCVL5kJnMon+0twMR1UaMdanrvA== X-Received: by 2002:a05:6830:2c06:b0:72c:320c:d898 with SMTP id 46e09a7af769-73a363f5a90mr6912233a34.22.1750106470433; Mon, 16 Jun 2025 13:41:10 -0700 (PDT) Received: from ?IPV6:2600:8803:e7e4:1d00:9583:1e37:58ed:10ae? ([2600:8803:e7e4:1d00:9583:1e37:58ed:10ae]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-73a28402beesm1275336a34.19.2025.06.16.13.41.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Jun 2025 13:41:10 -0700 (PDT) Message-ID: Date: Mon, 16 Jun 2025 15:41:08 -0500 Precedence: bulk X-Mailing-List: linux-gpio@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170 To: Conor Dooley , Marcelo Schmitt Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, jic23@kernel.org, lars@metafoo.de, Michael.Hennerich@analog.com, nuno.sa@analog.com, andy@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, linus.walleij@linaro.org, brgl@bgdev.pl, marcelo.schmitt1@gmail.com References: <4df9d4d0de83090300b6870afc8ae7b22279cd22.1749582679.git.marcelo.schmitt@analog.com> <20250616-neurology-explicit-ec2a829bd718@spud> Content-Language: en-US From: David Lechner In-Reply-To: <20250616-neurology-explicit-ec2a829bd718@spud> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 6/16/25 10:41 AM, Conor Dooley wrote: > On Tue, Jun 10, 2025 at 05:31:04PM -0300, Marcelo Schmitt wrote: >> Add device tree documentation for AD4170 and similar sigma-delta ADCs. >> The AD4170 is a 24-bit, multichannel, sigma-delta ADC. >> >> Signed-off-by: Marcelo Schmitt >> --- >> Change log v4 -> v5 >> - Dropped interrupt maxItems constraint. >> - Spelled out RC acronym in reference-buffer description. >> - Require to specify interrupt-names when using interrupts. >> - Added interrupt-names to the examples. >> - Made adi,excitation-pin properties identical to adi,ad4130. >> - Removed interrupt-parent props from the examples. >> >> Proposing new types and ways of describing hardware for weigh scale load cells >> and related sensors external to ADCs can lead to potential better description of >> how those components connect to the ADC. However, we must use what already >> exists for properties documenting features that are the same across different >> devices. >> >> Maybe, we could use generic defs to define adi,excitation-current-n-microamp and >> adi,excitation-pin and avoid repetition with those. Though, that triggers a >> dt_binding_check warning. Also, having mixed notation (some prop declarations >> using defines and others not) seems to not be desirable. >> >> It looks like the only option left is making adi,excitation-pin properties >> identical to adi,ad4130. >> >> On one hand, dropping adi,excitation-pin defs and making those properties >> identical to adi,ad4130 preserves their syntax and semantics accross >> dt-bindings. OTOH, we end up with more text repetition in the doc. >> >> >> .../bindings/iio/adc/adi,ad4170.yaml | 564 ++++++++++++++++++ >> MAINTAINERS | 7 + >> 2 files changed, 571 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml >> new file mode 100644 >> index 000000000000..e3249ec56a14 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml >> @@ -0,0 +1,564 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4170.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Analog Devices AD4170 and similar Analog to Digital Converters >> + >> +maintainers: >> + - Marcelo Schmitt >> + >> +description: | >> + Analog Devices AD4170 series of Sigma-delta Analog to Digital Converters. >> + Specifications can be found at: >> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4170-4.pdf >> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4190-4.pdf >> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4195-4.pdf >> + >> +$ref: /schemas/spi/spi-peripheral-props.yaml# >> + >> +$defs: >> + reference-buffer: >> + description: | >> + Enable precharge buffer, full buffer, or skip reference buffering of >> + the positive/negative voltage reference. Because the output impedance >> + of the source driving the voltage reference inputs may be dynamic, >> + resistive/capacitive combinations of those inputs can cause DC gain >> + errors if the reference inputs go unbuffered into the ADC. Enable >> + reference buffering if the provided reference source has dynamic high >> + impedance output. Note the absolute voltage allowed on REFINn+ and REFINn- >> + inputs is from AVSS - 50 mV to AVDD + 50 mV when the reference buffers are >> + disabled but narrows to AVSS to AVDD when reference buffering is enabled >> + or in precharge mode. The valid options for this property are: >> + 0: Reference precharge buffer. >> + 1: Full reference buffering. >> + 2: Bypass reference buffers (buffering disabled). >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + enum: [0, 1, 2] >> + default: 1 > > Why make this property a uint32, rather than a string where you can use > something like "precharge", "full" and "bypass" (or "disabled")? The > next similar device could use something slightly different then the > binding becomes pretty clunky. > Can you explain why this is a dt property rather than something > adjustable at runtime? > > Otherwise, what you have here looks sane enough to me - but I'd like to > see some comments from Jonathan or David etc about your approach to the > excitation properties. This looks like something that should be in the devicetree to me. For example if the external reference supply is high impedance, buffering is pretty much required. And using precharge is an application design choice to reduce THD at the expense of other limitations. > > Cheers, > Conor. > >> + >> +properties: ... >> +allOf: >> + # Some devices don't have integrated DAC >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - adi,ad4190 >> + - adi,ad4195 >> + then: >> + properties: >> + ldac-gpios: false >> + >> + # Require to specify the interrupt pin when using interrupts >> + - if: >> + required: >> + - interrupts >> + then: >> + required: >> + - interrupt-names >> + >> + # If an external clock is set, the internal clock cannot go out and vice versa >> + - oneOf: >> + - required: [clocks] >> + properties: >> + '#clock-cells': false >> + - required: ['#clock-cells'] >> + properties: >> + clocks: false >> + >> +patternProperties: ... >> +required: >> + - compatible >> + - reg >> + - avdd-supply >> + - iovdd-supply >> + - spi-cpol >> + - spi-cpha >> + >> +unevaluatedProperties: false >> + It would be more logical to place these before patternProperties (actually really before allOf) so that they are close to the properties that they are referencing.