From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f169.google.com (mail-oi1-f169.google.com [209.85.167.169]) (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 12E221D5AAD for ; Tue, 28 Jan 2025 15:56:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738079804; cv=none; b=LzgfcQOlLyeVV5zrzRcYMYJTds8KZFWTd7ewb//sjkdahkxLJIK0jmEckHeDXJZNe0+qFt3W88fc3y+t0IofwG88j7N8A/+I1lmPG/3mtkdK4fcJfJLPIeP1jfhyIy5fN4ATPbwflPfh73Lwn2Z50f3iJ6kcSYFQbguG8WoDgNQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738079804; c=relaxed/simple; bh=U1CRXAgspDWLAL9BhnasVoFw3dIoFPyaUQBg3kwcafc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=DRsf4tWhwosj55hczetIf7Qv7gz2AVvZRhysxXUM47iiZ7LEF06YAi+KmXnoLK8DqtSPUThANWgpf3VTVzvA3lzDyiJbMveLkynGX6xZqWCKLJeqR1QvxnXjbC4xmnCw12RAhgrWBD2K6kVsLtQAmkFH8svEYS9Hbosi14lDDFY= 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=YGqO+qe7; arc=none smtp.client-ip=209.85.167.169 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="YGqO+qe7" Received: by mail-oi1-f169.google.com with SMTP id 5614622812f47-3eb3c143727so3330040b6e.1 for ; Tue, 28 Jan 2025 07:56:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1738079801; x=1738684601; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=I2Qw44d6HeXewj2arD8dL/6DqT4U1JV+B9kC0ec4R8I=; b=YGqO+qe7NxqU8msv1DCe7UijfIGza7KAudeUSePl3kCzof2NU5vfKtotqLgW/FeL5z 8oOh6fWygQLVzWr4Hjwl6I4QrBM9oD91XKGFjDP7j3suKW38H3agWCaIdMvN9SnWUax8 chaBCwdnn4JhnGSm/FNn99S+7uo7uqsNygviwyvDrYBC+KQF3nt1fQctNJrrjWk3t2nf xeT2eeCyv8mMH7dEZXPhY0zTx/gtEa6N6YmQLChlsvDEJm7ADn+AcHFgCoLv/lOi5LqA 5quEw0WjnYiCu2NTXTdrU4Z3g7BeBvoab9u76IOMyxyb+qPG9Q8K4PBYz/NlZFsF5pna dybA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738079801; x=1738684601; h=content-transfer-encoding:in-reply-to:content-language:from :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=I2Qw44d6HeXewj2arD8dL/6DqT4U1JV+B9kC0ec4R8I=; b=miPqUKgThP+Xkxf3vK3sDOcb9O63vOnCQgrw/+IJPBNJCH6h4YyuGLMX+APNxewrES Ins9rptgdXIP9/N+Bvbaij5smA34bk/39o5+RNsYsRoexZn4Gc/7AwAmKZviRFLw5qAI 97RCQl0KEVoT6jllEVg9kg3rx+YHnUDeoxybUmzrHp38KWASD4KVyVd0mScP+Zt2Eper GGYRZpt1l+1OpGGtmZ37zM0zgfG92h0vxclMCgOCAiPkyc3HeDVP2AjxUyKdeRuNePqu LAINS8Q8scx3fACE1cOV5d7pmaZEimdwY/iBNSORNrWEQw/SRifmR52gd20tRD3xQXYP zSqA== X-Forwarded-Encrypted: i=1; AJvYcCUlglYBu6KYyFstIqS5gtCIdlN9/p41FQqukII5jL6g8B9If4w4kljsei2Qm5e52zGHR4X4OK+m/r8DkGA=@vger.kernel.org X-Gm-Message-State: AOJu0YxBnUYjBcfY8EyumekTOyBpvZbQD1MRsSnvLXGLa5GHljmzDNDh Z3jyxTquiitncIQrA1c1gBmdJzsk8aWt3U37MnZJuZZmBsXiqdesIoXRPMtZMKs= X-Gm-Gg: ASbGncvW8rX5Da+4vXoRAVIuU+Xof84hD2mimMctC8M+8anAC2Zd5D1cYHWxCLRizXs RcI7/9cTNcx0YZWLbvOVRahZdFkOpUgMvJioKtK8jRK53u67IMpH2JnazZFF+Zkyr32bpVpdBG3 0zk3LS/Olvp9UwsP43dzWTw1HbATy409zpnpBUYZRuYKoTRrgLPwqalYzQcAu78edqfdl4MNvaQ MDVGO3Ubb5mZa8SMtR3fobtWWegtgMCFoyxGREfsi5BLN3KbgrQCtBFRo+XYZhOK68usMs7Y2b4 HZUbDPfqE2swJNrDlGSHrF3HsmxQzqB/8yLdBcp2hNRjtYqRzQ5z X-Google-Smtp-Source: AGHT+IFtfyDR9boDmmfEF591oA/p0B2VQoOq0QZKo6ZB7tolgqw9/zygWP3UxD/QMyhPqXrDRH8u2Q== X-Received: by 2002:a05:6808:14cc:b0:3eb:64a8:837c with SMTP id 5614622812f47-3f313c8e04amr2330880b6e.6.1738079800976; Tue, 28 Jan 2025 07:56:40 -0800 (PST) Received: from [192.168.0.142] (ip98-183-112-25.ok.ok.cox.net. [98.183.112.25]) by smtp.gmail.com with ESMTPSA id 5614622812f47-3f1f088b0ddsm3134959b6e.20.2025.01.28.07.56.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 28 Jan 2025 07:56:39 -0800 (PST) Message-ID: Date: Tue, 28 Jan 2025 09:56:37 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 02/16] dt-bindings: iio: adc: ad7768-1: add trigger-sources property To: 0044dd4b-01ce-4ca0-9855-8c239b9bfb6f@baylibre.com Cc: Jonathan Santos , linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, lars@metafoo.de, Michael.Hennerich@analog.com, marcelo.schmitt@analog.com, jic23@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, marcelo.schmitt1@gmail.com References: <0044dd4b-01ce-4ca0-9855-8c239b9bfb6f@baylibre.com> From: David Lechner Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 1/28/25 9:04 AM, Jonathan Santos wrote: > On 01/27, David Lechner wrote: >> On 1/27/25 9:11 AM, Jonathan Santos wrote: >>> Add a new trigger-sources property to enable synchronization across >>> multiple devices. This property references the main device (or >>> trigger provider) responsible for generating the pulse to drive the >>> SYNC_IN of all devices in the setup. >>> >>> In addition to GPIO synchronization, The AD7768-1 also supports >>> synchronization over SPI, which use is recommended when the GPIO >>> cannot provide a pulse synchronous with the base MCLK signal. It >>> consists of looping back the SYNC_OUT to the SYNC_IN pin and send >>> a command via SPI to trigger the synchronization. >>> >>> SPI-based synchronization is enabled in the absence of adi,sync-in-gpios >>> property. Since adi,sync-in-gpios is not long the only method, remove it >>> from required properties. >>> >>> While at it, add description to the interrupt property. >>> >>> Signed-off-by: Jonathan Santos >>> --- >>> v2 Changes: >>> * Patch added as replacement for adi,sync-in-spi patch. >>> * addressed the request for a description to interrupts property. >>> --- >>> .../bindings/iio/adc/adi,ad7768-1.yaml | 22 +++++++++++++++++-- >>> 1 file changed, 20 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml >>> index 3ce59d4d065f..3e119cf1754b 100644 >>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml >>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml >>> @@ -26,7 +26,17 @@ properties: >>> clock-names: >>> const: mclk >>> >>> + trigger-sources: >>> + description: >>> + References the main device responsible for synchronization. In a single >>> + device setup, reference the own node. >>> + maxItems: 1 >> >> We probably actually need 2 here. One for /SYNC_IN and one for a GPIO3 pin >> acting as the /START signal. >> >>> + >>> interrupts: >>> + description: >>> + Specifies the interrupt line associated with the ADC. This refers >>> + to the DRDY (Data Ready) pin, which signals when conversion results are >>> + available. >>> maxItems: 1 >>> >>> '#address-cells': >>> @@ -46,6 +56,8 @@ properties: >>> sampling. A pulse is always required if the configuration is changed >>> in any way, for example if the filter decimation rate changes. >>> As the line is active low, it should be marked GPIO_ACTIVE_LOW. >>> + In the absence of this property, Synchronization over SPI will be >>> + enabled. >> >> Isn't /SYNC_OUT connected to /SYNC_IN required for synchronization over SPI? >> >> If yes, instead of adding this text, I would make the binding have: >> > > Yes, but the synchronization over SPI is enabled in the absence of the GPIO. > The trigger-sources property would indicate if the sync provider is the > own device or not. As i said below, maybe i misunderstood. > >> oneOf: >> - required: >> - trigger-sources >> - required: >> - adi,sync-in-gpios >> > > Wouldn't be simpler to consider the absence of sync-in-gpio? this way we > have less changes in the ABI. Maybe it is me that missed something, but if I'm reading the datasheet correctly, then sync over SPI only works if /SYNC_IN is wired to /SYNC_OUT. And the chip isn't going to work correctly without some sort of sync. So we need something wired to /SYNC_IN no matter what. In any case, the DT bindings should just say how the chip is wired up and not dictate how the driver should behave. So what I was going for with this is to have the bindings say that something has to be wired to /SYNC_IN and we can leave it up to the driver to decide what to do with this information. > >>> >>> reset-gpios: >>> maxItems: 1 >>> @@ -57,6 +69,9 @@ properties: >>> "#io-channel-cells": >>> const: 1 >>> >>> + "#trigger-source-cells": >>> + const: 0 >>> + >>> required: >>> - compatible >>> - reg >>> @@ -65,7 +80,8 @@ required: >>> - vref-supply >>> - spi-cpol >>> - spi-cpha >>> - - adi,sync-in-gpios >>> + - trigger-sources >>> + - #trigger-source-cells >>> >>> patternProperties: >>> "^channel@([0-9]|1[0-5])$": >>> @@ -99,7 +115,7 @@ examples: >>> #address-cells = <1>; >>> #size-cells = <0>; >>> >>> - adc@0 { >>> + adc0: adc@0 { >>> compatible = "adi,ad7768-1"; >>> reg = <0>; >>> spi-max-frequency = <2000000>; >>> @@ -109,6 +125,8 @@ examples: >>> interrupts = <25 IRQ_TYPE_EDGE_RISING>; >>> interrupt-parent = <&gpio>; >>> adi,sync-in-gpios = <&gpio 22 GPIO_ACTIVE_LOW>; >> >> Don't we need to drop adi,sync-in-gpios here? I don't think we would have two >> things connected to /SYNC_IN at the same time. >> > > I guess i misunderstood the use of trigger-sources. I thought it would > indicate the trigger provider or main device. Like if it points to other > device we should use it to drive the SYNC_IN of all devices. > > Then what happens if the trigger-sources points to other node? we would't be > able to driver the SYNC_IN in case of any configuration change? I think you understand the trigger-source bindings correctly. The driver doesn't have to support everything that the DT bindings allow. This series is big enough already, so we can defer figuring out how to implement triggers other than the loopback case later. :-) We just want to make the DT bindings as complete as we can now. > >>> + trigger-sources = <&adc0>; >>> + #trigger-source-cells = <0>; >>> reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>; >>> clocks = <&ad7768_mclk>; >>> clock-names = "mclk"; >>