From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vk1-f176.google.com (mail-vk1-f176.google.com [209.85.221.176]) (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 B52BD390CAB for ; Sun, 9 Aug 2026 08:27:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.176 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786264027; cv=none; b=F8MivsW4ZB3CdZeSJ5Gn3e5srAMeVhLn4YTtUw9Plop1oPfbcBE27oRDwCnsjBopH77jXXFMYOspLE47hFRgM6f7IXzrnE6eXQwzAEzXlb7OzTpEkPjnmRjOYL/chweRhkB1LxFeplHywP1xDMBsXbKmaYcm1dNdAI8NMbUPNw8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786264027; c=relaxed/simple; bh=F5FT9OwhnJ6YSXO2v4rbSgahxv2Ps6idkBRw2W44LTg=; h=Content-Type:Date:Message-Id:From:To:Cc:Subject:Mime-Version: References:In-Reply-To; b=dmkHjqL6PeCR8jetltoIGzAT7ULxtX+yZgJ7BPx9HvAa7UMe/59qPFe74dTknLWW6NMUOYPt88/vBMn6+En304P+J9XM0bUY+VQNeCSVhAOLQhOj5oNIhcd3071x6BXO60ntA3jASxNZJa5U4Y9fcBqx/u3NLwGriaGB981Jo78= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=myNRbsXo; arc=none smtp.client-ip=209.85.221.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="myNRbsXo" Received: by mail-vk1-f176.google.com with SMTP id 71dfb90a1353d-59b074ec7ceso434221e0c.1 for ; Sun, 09 Aug 2026 01:27:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786264024; x=1786868824; darn=vger.kernel.org; h=in-reply-to:references:content-transfer-encoding:mime-version :subject:cc:to:from:message-id:date:content-type:from:to:cc:subject :date:message-id:reply-to:content-type; bh=SDlibkiHCsiPlgn9bXT0ZRW/tQXXQ2ApH6C6TY57GQc=; b=myNRbsXoOo3JnAHDqTYhIQpOp4VVohRSt07qercWR+A0HdfZEGaMLnA+mUn5BpxHk5 icEJaj2ya7Fh8aZntdr8vJm1InI+bhJNm6OSb8y6OPqD9rmmLuZHsCaENFrjrEQ8oJaN 89PyjdCQxwI+rEEH2vdwswbgep4Q0Y//CvtTj6e1cU84x7jpDzyQsz0mDRo1UnEin4wV eGrdFPe1EiPNWkXeS2OCmkggMsZGuh/SakEdWZID36Zfsqo7bU680qq5EZrxDLVnFg+E nVYG3QHX6FY2qj7NPy9CyhvkzGJGjyc1QWAQvd8gsVzB8XDPc04zjF/UakVcN3ewv2xp 582w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786264024; x=1786868824; h=in-reply-to:references:content-transfer-encoding:mime-version :subject:cc:to:from:message-id:date:content-type:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=SDlibkiHCsiPlgn9bXT0ZRW/tQXXQ2ApH6C6TY57GQc=; b=QHPcWLTMvJbh/cfr60BbaqhvYiB4AwyT9FloxABGV2IJKaLAnUagq9eAHYCHE6OhOK UHSoQxLAgvLH/gU/zrOyIt2jgs5e0PS4QfFbH8INzeIVy+6cE8lPzKqYuUf14Hgy2SKh eJ/3sILnjNtGcve9FxBbNH5PVPoi62cLaFspmfAqYyk2P7/F5hunoTIQ751FZVy1TzRi Gt7XQGQpLJ7wduEFwgq0FouS+z0uU8O8mDOlgC2c8XqRSexGnamYsRDFIlfRYTdgycxG 360gtGOsGemdlhiKwti85OflhgxkGGFSLH3h8dSfDklsWPGhEHnVlOJRSoj833vthCET N/wA== X-Forwarded-Encrypted: i=1; AHgh+RovGp+aFgJgK3oNEYeRQlXbJZNps8t8shzozrAPkbn9qsErEfbcV1vQlsPJk/mTJDcSwpdCN54/zgSN@vger.kernel.org X-Gm-Message-State: AOJu0Yx8taArBbCfpSfGH8VBKzFXiezSckBZVKuE+RInr7/RZdCJuxKz Nmv18IrKUQ8wMgFt2MCAswVXBygVH34mDVWE/ngA9+0m6D/H7CFVnr2V X-Gm-Gg: AR+sD12WOuXZF7TWvsL6AJJSQCO/OkjXimXSLlDcT3NdnK2iw7u5BAmh0FOeIuAhPxp YMVdU1XnuzpYtN/EGMbdNEeGfg19ukxGWbqug8eWKDW1CuKFn1KHT0grAU2ORxeuKKq2pmOuPUZ emhhZ85pWMWDSHmsUzx/voNYvFzLs7586DZRy4SsEUPbKFIC6g7D/IiwNjYt3H4cYl+NgqVJoMG MRRhHFxOfigct/Mi4yVDKe+ZoTb6AmRlcCCPhL07box5axvb5AfPW+RQJql29RiJnpcNnFn316A 9/pvOG3LePguBOH3RbWfOJaW+/ep6xUeLkWvv2enk/hdvOsQTRAdOARUC5bdM12poxE6wxGhkJk O2sfr7ghF9qcyMt9OrFKd6ImDt1GX2HDQMy5b1cIpJfcRKSRmw/BX1sfoQeB1kABNeIJ0z1Vp/S KJO3DpUe8imBpoTgUCST+jPM59S0hLydc3pnd4sE6LhNnNoHnQWZ4= X-Received: by 2002:a05:6102:c10:b0:75f:e517:ec96 with SMTP id ada2fe7eead31-760e3194fc4mr8187153137.0.1786264024434; Sun, 09 Aug 2026 01:27:04 -0700 (PDT) Received: from localhost ([2800:bf0:82:11a2:7ac4:1f2:947b:2b6]) by smtp.gmail.com with ESMTPSA id 71dfb90a1353d-5c40b1db997sm2752812e0c.13.2026.08.09.01.27.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 09 Aug 2026 01:27:04 -0700 (PDT) Content-Type: text/plain; charset=UTF-8 Date: Sun, 09 Aug 2026 03:26:57 -0500 Message-Id: From: "Kurt Borja" To: "David Lechner" , "Kurt Borja" , "Jonathan Cameron" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Linus Walleij" , "Bartosz Golaszewski" Cc: =?utf-8?q?Nuno_S=C3=A1?= , "Andy Shevchenko" , , , , Subject: Re: [PATCH v3 2/9] iio: adc: add the ti-ads1262 driver Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0-0-g5549850facc2 References: <20260807-ads126x-v3-0-f89925d72792@gmail.com> <20260807-ads126x-v3-2-f89925d72792@gmail.com> In-Reply-To: On Sat Aug 8, 2026 at 1:39 PM -05, David Lechner wrote: > On 8/7/26 10:58 PM, Kurt Borja wrote: >> Add the ti-ads1262 driver with initial support for the primary ADC >> (ADC1). The ADS1263 auxiliary ADC (ADC2) is handled by a separate driver >> and interoperability considerations were taken into account. > > Should probably mention here that IIO_CHAN_INFO_SCALE is intentionally > left out here. (Or just implement it using internal reference voltage > to start with.) I'll mention the scale is left out. > >>=20 >> Signed-off-by: Kurt Borja >> --- [...] >> diff --git a/drivers/iio/adc/ti-ads1262.c b/drivers/iio/adc/ti-ads1262.c [...] >> +static int ads1262_dev_reset(struct ads1262 *st) >> +{ >> + int ret; >> + >> + if (st->reset_gpiod) { >> + ret =3D gpiod_set_value_cansleep(st->reset_gpiod, 1); >> + if (ret) >> + return ret; >> + >> + /* >> + * The RESET pulse timing requirement is 4 clock cycles, at the >> + * minimum clock rate this is 4 microseconds. >> + */ >> + fsleep(4); > > How long do we have to hold reset before the chip powers down? For power down 65536 clk cycles. Less than that is simple reset. [...] >> +static int ads1262_wait_for_conversion(struct ads1262 *st) >> +{ >> + u64 max_lat_ms; >> + long ret; >> + >> + /* >> + * The first conversion latency is affected by the channel's data rate= , >> + * filter, the configurable conversion delay and whether chop mode >> + * and/or IDAC rotation mode are enabled. >> + * >> + * The worst possible latency is calculated by taking the lowest data >> + * rate (2.5 SPS) and the sinc4 filter. This gives a latency of 1600 m= s >> + * (Table 9-13). Then we scale it by the actual clock rate and multipl= y >> + * by 4 to account for chop and IDAC rotation modes (Equation 20). >> + */ >> + max_lat_ms =3D 4 * div_u64(mul_u32_u32(1600, 7372800), st->clk_rate); > > These are constant values, so don't need mul_u32_u32(). Also, given the w= ide > range of possible sampling rates, I would include the current sampling ra= te > in the calculation. No need to wate 1.6 seconds for something that should > take a few 10s of microseconds. Can we leave it like this until I implement the settlingtime? That way I can calculate the actual first conversion latency. [...] >> +static int ads1262_debugfs_reg_access(struct iio_dev *indio_dev, unsign= ed int reg, >> + unsigned int writeval, unsigned int *readval) >> +{ >> + struct ads1262 *st =3D iio_priv(indio_dev); >> + >> + guard(mutex)(&st->xfer_lock); >> + >> + if (readval) >> + return regmap_read_bypassed(st->regmap, reg, readval); > > Don't trust the cache? :-) I don't trust myself :-) This was relevant early in development, I'll go with the normal version. [...] >> +static int ads1262_parse_channels(struct iio_dev *indio_dev) >> +{ >> + struct ads1262 *st =3D iio_priv(indio_dev); >> + struct device *dev =3D &st->spi->dev; >> + struct iio_chan_spec *specs; >> + unsigned long used_regs =3D 0; >> + int num_specs; >> + u32 reg; >> + int ret; >> + >> + st->num_channels =3D device_get_named_child_node_count(dev, "channel")= ; >> + if (!st->num_channels) >> + return dev_err_probe(dev, -ENXIO, "no 'channel' nodes configured\n"); >> + if (st->num_channels > ADS1262_MAX_CHANNEL_COUNT) >> + return dev_err_probe(dev, -EINVAL, "too many channels\n"); >> + >> + /* Account for the timestamp channel */ >> + num_specs =3D st->num_channels + 1; >> + specs =3D devm_kcalloc(dev, num_specs, sizeof(*specs), GFP_KERNEL); >> + if (!specs) >> + return -ENOMEM; >> + >> + device_for_each_named_child_node_scoped(dev, node, "channel") { >> + ret =3D fwnode_property_read_u32(node, "reg", ®); >> + if (ret) >> + return dev_err_probe(dev, ret, "%s: failed to read channel reg\n", >> + fwnode_get_name(node)); >> + if (reg >=3D st->num_channels) >> + return dev_err_probe(dev, -EINVAL, "%s: reg out of range\n", >> + fwnode_get_name(node)); >> + >> + static_assert(ADS1262_MAX_CHANNEL_COUNT < BITS_PER_LONG); >> + if (__test_and_set_bit(reg, &used_regs)) >> + return dev_err_probe(dev, -EINVAL, "%s: duplicated channel reg\n", >> + fwnode_get_name(node)); >> + >> + specs[reg].scan_index =3D reg; >> + specs[reg].scan_type =3D (struct iio_scan_type) { >> + .format =3D IIO_SCAN_FORMAT_SIGNED_INT, >> + .realbits =3D ADS1262_ADC1_RESOLUTION, >> + .storagebits =3D 32, >> + .endianness =3D IIO_BE, >> + }; >> + >> + ret =3D ads1262_parse_channel_node(st, &specs[reg], node); >> + if (ret) >> + return ret; >> + >> + if (specs[reg].channel =3D=3D ADS1262_INPMUX_TEMP) >> + specs[reg].type =3D IIO_TEMP; >> + else >> + specs[reg].type =3D IIO_VOLTAGE; >> + >> + if (specs[reg].channel !=3D ADS1262_INPMUX_TEMP) >> + specs[reg].indexed =3D true; >> + >> + specs[reg].info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW); >> + } > > If we are going to use reg to determine the scan index, we need to > make sure there are no holes in specs that didn't get filled in. > > device_for_each_named_child_node_scoped() will skip `status =3D "disabled= "` > channels, so this could be a possibility. Ah, I didn't know some channels could be skipped. I'll check for holes. [...] --=20 Thanks, ~ Kurt