From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vs1-f43.google.com (mail-vs1-f43.google.com [209.85.217.43]) (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 14F9A36D507 for ; Sun, 30 Nov 2025 03:31:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.217.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764473509; cv=none; b=dYkH4FCMqXpc+uRuCSn0NznM92k9sWOW9E7uakSQsoaTY2a+tmPUcRc8ywEj7Ju0N6e3CT1kuGZkPljDHEdkxEoyv5LKkr8sXSedXgyOxLwGVhrrTUjminQdvrElxYS0SqHRmnwEZBFN6CpCX9oTb7QyhzvR2Omon9XgSssFjPI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764473509; c=relaxed/simple; bh=0Jmug8C9Wt13WRpSx0oCy5aLyOWe463+u5m1tiDTA6A=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:From:To:Cc: References:In-Reply-To; b=CA/S2wixMiyCruHJTe7Nkr+ht0NL5+gy1lkl6nymRhL7vQUCZL+tz+3z9cYaOuGlljMQRbQuRcFI08T2ho/sv9ly5QTW0GxPBC37sFEOMbWhlzPvSQkg52PwC7BNxyAc8bIoy82RllA+phgxlAkySMxwdb0XowXizVn5+WZoIHw= 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=Ax0zY2bw; arc=none smtp.client-ip=209.85.217.43 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="Ax0zY2bw" Received: by mail-vs1-f43.google.com with SMTP id ada2fe7eead31-5dfd2148bf3so1015559137.1 for ; Sat, 29 Nov 2025 19:31:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764473507; x=1765078307; darn=vger.kernel.org; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=CNXcJkrLq97RKGghE/tDEV9K/VHLf6K0n2K5yQgJjy8=; b=Ax0zY2bwa43jnuapddZFeGsSNmuFp4QWxJ2rKCIRGf/G9x1F62uuz3Y23/az9m/PFh YemYypHaDvQDNNERa26IvMa87xwdxyivD8REjUseQ9nQX+uK8Ww3ijEuG0lXdDzh0t1r nw15o/umJHVw7cxBeZfwpnv+ULGuj4Eg0RA7jqS99XZg1DwGNxnhunnuIdzTn6nedKh8 Qg8xlW9qUu6/MzJAoklyCPPfTjgMaR3ZJhxlxSy2bjHITJCSj+oWfi8v1+D3vrVfKtZT ihbv0aZHUN5UoatNWRIcdzOjx/Ww735qdyDBdihl/1u/dm0twn9bWh1rQSzPsZs1i+UK 0EQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764473507; x=1765078307; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=CNXcJkrLq97RKGghE/tDEV9K/VHLf6K0n2K5yQgJjy8=; b=kbvw+X2G3stbxbv8wdoQAnPE1oOfhgufLncdKcCiqlTx3nE1qzl/DtepY+yJiUm233 vcmFJ4ZqEkELrtP3vdgTAlCnHm1YuEq0c7/3+/slsAJ824MEjSx8/xLhNZRgVr7/AOkI CAQOPGi37C/G8H0W51dB6GP3Qk9ExDlh7bIIh8Z4GSWJQL47+8AA2xZfUhNQkn3NSsBr y3vvPk9rQbV12SHM7ndTqKcOj5BRfV1dO5dqzEGgYHwRCAwbbtyAQ11LSQM+JuRhyXkb G0BajPWcwOPuq9xQVIByIX78kdkFKmluXDo5/pGlcW9fGIpO84xl6i3SXJEPe4vILo68 L6VQ== X-Forwarded-Encrypted: i=1; AJvYcCWatwyuDiof/33ZuEIwbOxJk5b+b5jQWVljPUdl8VzGhjr4Ne6yYnj94p7IEK6PCdJMfOvwbxhDKm4xHcc=@vger.kernel.org X-Gm-Message-State: AOJu0YyiFJekc1MiZ2vXeR8BWe3ApiG2WP6lp5Dwna3yg2qoFEdxtu1T MJEzohnQXYc0yorj7hig+u/9VjiSSj2DiiLCQ/SarU/vmtOhtxVviqaQ X-Gm-Gg: ASbGnctlLCj3Wud7bC+repZPzV5OthNFDR+ZxR+mAoUIYoVhpeAdcZ7tKikj3QSqGTy BtEKQONsc+lzkBAObt3naAJmw2chRR8tQ/N9KvKvVRk5MbE9ZN7ARUCgAfhBWJAFkHyEoISsOhQ s7xXq+PZiuPXagR/P0AtEEqxzjmK0ycbV2bBxUIbPgdkSRmeFlO6JaSEabF2EvlF9VO3r8xRzSx O/NlPhFLIr4bQzUFwHEGhc8GL5CASK/U5sfqo3sbx6K6H5om3ylTR1XECBFKc5dqjkMEb9bY/+0 q5iaufdtEGzYA1dnRlW7cpTSu9fRDlgQOi9OTXyTy7ysv82/nSLc2LQBPhKIHVeX5tSQUowlog+ iLAAw5GLjX3i1pFlnoFO8rv0ky0b1dnqRVxqqirz65NW3QugQXE2ZNkW65tf1vYEYOwD7K9Mou8 zrueM= X-Google-Smtp-Source: AGHT+IHP1Gk+IT+glfYKtU0OYJDb+d/EEpIw57u9DDRyfDuHa31Js4CEqSYzL6VsQPevKFPxoAG2Kw== X-Received: by 2002:a05:6102:334c:b0:5dd:b69a:cdce with SMTP id ada2fe7eead31-5e1de0eccc9mr9460400137.1.1764473506918; Sat, 29 Nov 2025 19:31:46 -0800 (PST) Received: from localhost ([2800:bf0:82:3d2:875c:6c76:e06b:3095]) by smtp.gmail.com with ESMTPSA id a1e0cc1a2514c-93cd6ce2180sm4130942241.5.2025.11.29.19.31.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 29 Nov 2025 19:31:46 -0800 (PST) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Sat, 29 Nov 2025 22:31:44 -0500 Message-Id: Subject: Re: [PATCH v3 2/2] iio: adc: Add ti-ads1018 driver From: "Kurt Borja" To: "Andy Shevchenko" , "Kurt Borja" Cc: "Jonathan Cameron" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Tobias Sperling" , "David Lechner" , =?utf-8?q?Nuno_S=C3=A1?= , "Andy Shevchenko" , , , , "Jonathan Cameron" X-Mailer: aerc 0.21.0-0-g5549850facc2 References: <20251128-ads1x18-v3-0-a6ebab815b2d@gmail.com> <20251128-ads1x18-v3-2-a6ebab815b2d@gmail.com> In-Reply-To: On Sat Nov 29, 2025 at 9:21 AM -05, Andy Shevchenko wrote: ... >> + * @ad1018: Device data >> + * >> + * Calculates an appropriate delay for a single shot reading, assuming = the >> + * device's maximum data-rate is used. >> + * >> + * Context: Expects iio_device_claim_direct() is held. >> + * >> + * Return: Delay in microseconds. > > Does 0 have any special meaning? This function is never 0. ... >> + /* We subtract 10% data-rate error */ >> + hz -=3D DIV_ROUND_UP(hz, 10); > > Hmm... For delays I expect to see adding 10% to have a good margin. hz goes in the denomitor bellow, so less hz is more delay. Makes sense because worst case sample rate is less sample rate. ... >> + * Context: Expects spi_bus_lock() is held. > > Do we have a lockdep assert for this? Lockdep checks lock is held on the same thread right? Thinking about it, this context is wrong because we don't hold the bus lock on the same thread. Rather we hold it to avoid other devices from transfering while we hold our CS line. I should probably remove this context and mention this peculiarity in the long description. ... >> +static int ads1018_read_unlocked(struct ads1018 *ads1018, __be16 *cnv, = bool hold_cs) > > Hmm... Don't we want to return value in CPU order? I don't know the answe= r > here, and IIRC IIO triggers might be actually good with endianess convers= ion > done, if required, in user space. I specified IIO_BE endianness in each channel's .scan_type, so this works. However, I don't have issue especifying IIO_CPU and just returning CPU order values. ... >> + * Context: Expects iio_device_claim_direct() is held. > > Jonathan et al., do we have lockdep assert available for this? > I really prefer to see the code for it, while comment is good, > it is not good enough. This would be nice. ... >> + if (iio_device_claim_buffer_mode(indio_dev)) >> + goto out_notify_done; >> + >> + if (iio_trigger_using_own(indio_dev)) { >> + disable_irq(ads1018->drdy_irq); >> + ret =3D ads1018_read_unlocked(ads1018, &scan.conv, true); >> + enable_irq(ads1018->drdy_irq); >> + } else { >> + ret =3D spi_read(ads1018->spi, ads1018->rx_buf, sizeof(ads1018->rx_bu= f)); >> + scan.conv =3D ads1018->rx_buf[0]; >> + } >> + >> + iio_device_release_buffer_mode(indio_dev); >> + >> + if (ret) >> + goto out_notify_done; >> + >> + iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), pf->timest= amp); >> + >> +out_notify_done: >> + iio_trigger_notify_done(ads1018->indio_trig); > > Jonathan et al., maybe we need an ACQUIRE() class for this? It will solve > the conditional scoped guard case, no? ... If no one prefers to do it, I can submit a patch implementing this. Same for the lockdep issue above. >> +/** >> + * ADS1018_FSR_TO_SCALE - Converts FSR into scale >> + * @_fsr: Full-scale range in millivolts >> + * @_res: ADC resolution > > Add here something like this: > > * > * The macro is crafted to avoid potential overflows on 32-bit machines. > * This imposes restrictions to the possible values for @_fsr (less > * than 274878), and @_res (great or equal to 6 bits). > * Sure. I'll address the rest of the comments in v4. Thanks, Andy! --=20 ~ Kurt