From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f48.google.com (mail-ot1-f48.google.com [209.85.210.48]) (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 1294F212F98 for ; Mon, 16 Jun 2025 20:41:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750106478; cv=none; b=DEdX5Lv2y+0ReFBGo1/RVbzyCY4eV58O+qXDZu15rZ92vetkQFtPFLFxEXNqsbqSksStvqMBu/Z8MywXB/IDgCgMMX0kb3/eT3sE3f0h13OZEDieIwDx/cn07xMw15HvzyZsWDoHDmuc4B0RHbcpJK0zNeQVc5ULrzZyUkAR8rI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750106478; c=relaxed/simple; bh=I1ADj1YYA/0P9VYww4mRdxVOFiO6OjCF4bRvlcPDmRY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=aY2Yn88AlC9L21VZd30RQmYHU6Sl+y/A/vTYxmCroRj3qpvdTeV7PZcMGMxayrUBESlfOd+iR/q6JEkNDoGYgRBcmbtpoTgBWBAw5gXRYFKpUgakLTGzAGMtm/A8ou1wdYoQy9aUQwdoph/1bGUBuWXp7x9gj3t7M6kzXohehSE= 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=NkKjk9OS; arc=none smtp.client-ip=209.85.210.48 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="NkKjk9OS" Received: by mail-ot1-f48.google.com with SMTP id 46e09a7af769-73a43d327d6so1273151a34.2 for ; Mon, 16 Jun 2025 13:41:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1750106475; x=1750711275; 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=NkKjk9OSqOMquQrtrmoBK9Q8zRtB7THF2t0rfWNJ91qFmWWMuxJv4T6L2rZcEdA3xb zauMl23COGYv+EtnL8IiIh8nDNwu2/6vZ0c5DMXAW60zt9ZIrQjqFkuj+IE/isK+NQec M7eeqRbmV+rLT/fhBcgkFfJB0FmcwDgMqJkfk1IoURc0wcfX6CG7/TMy7JcvV/Gwea4M nrmz1EsQkjGoYAwaO+rS3yEIoC7Eg6eL0yaZy3oiYUip3q+1sOtM0IRB1ma7fZP4CYOg w7LWv5kAEX/ZJlCGGSqqINWS2clusGIb88SY+zJg77MmtEe3Winh4t+JJTYkgdNN4/yA 43Vg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750106475; x=1750711275; 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=f8Dxkks/I88aiEgbzSF1CWKhlQArKZCy0a6fU56wX/xDYsn9apTE0Pi6gBFnBWq7nI IOqgOEKgPHFuzsRWKtglRzCo7JnqG83Ig5Ukp76wLquifI6p2RjMqLSwZBRCcxoZQ0DD ZL5Y+bj349c2yZubiGNINycMdCfabCP0tNEqX4SOG530CdTOdxl1tWP68RZorLc0TtvN +FmZVizqJ4V2NXQ/qc2ZVo+Ayfeyhs4vxtZuCOzg56xfUsfkghF8gCr+hc0DtzRV33Vo +wodSgDkv4xlrdzxQ0LVwxhxeh4Wm24usnFua0u3ZhsOBCHmUePBiPtKk0lL2KipFDa3 wl9w== X-Forwarded-Encrypted: i=1; AJvYcCWLhVrePTHIZe3EV7atskH8RwcBN+VJ8JEnLCR82q4TAZnE0qmD5dY8GLdOZdrEX2Euxfk5ptpGHri9@vger.kernel.org X-Gm-Message-State: AOJu0YzBPXJPTsyMfgQIJviJvXyFhE+vLYPISp5LgpgepdGIdVxEZWLi J4wL1PGtcBEVKgi4Ty9MAMo1+7jFArTiCSpemZ6iDMmKHO3QPWOLe2OY432zR6/IkdI= X-Gm-Gg: ASbGncsrlH1gDNZjZ/hGGVNF33OfRxvVDwi7y0mBDLXL6HgdKbzCx/vZy8EtR+w2BpY YBpBDkk6fBq/HGT6RamoafR6pGKFzkM+qfu4KYtep4OtcfxjE4eyIP+FnCo7xB2KNteQ6AU1jov NaxG17apmKyUFd7FRIAXmNKK1H3/xZaPows/q6EyVryx8m6vdSLYUtH1Wf7MyEPTehWAgIENYSn jr+nfQy3ZcZvxXkACV5Ro0aGIijRcewywGTXm4KJAdrYsW3U19pfumv2vaDpMibD6/eZbqC46pU /tvyftViF5ZTOhe3RbfxAKF1AxYnF/QvUVCYdF2pY9LUeaemnSOQdP1+0fgi2yHJl2O5AKVtwP7 UrWOxipyxP3u1NGw3nsochLFUcyeIbBqBsfnjXIfTh107FFQYhA== 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: devicetree@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.