From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-f41.google.com (mail-oa1-f41.google.com [209.85.160.41]) (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 792B827FD5A for ; Mon, 1 Dec 2025 21:53:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764626004; cv=none; b=Af7MK+OaWZJqBcff0XBA19iuc0RyIBTs8BbqAArDJhhctnM71/T61gKX04IVyBQ+QsTdoi7C77uMTnPuXS27ndec1M6he0qy0u0KCeYolrpePURHu2noqRNblyunZWiwF/77614rUw66KcXj1hFx//EYJ+dc/dJh8eTfmFjKV2s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764626004; c=relaxed/simple; bh=5q9Iv//mIpypUaynw8++s+lccd4+l1jE8IYnVDYU7vE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=huLdS4sapOKG8MLTi6rcF9JbDPKIg1ZCiywRCK+P5IGH/SC0VeSPJPQlRGPex3XdZqtOtdwVm98h9eRz6sJCAskDHD3zI9hrd5MqbEKWmNzMfeqVIrkP2iAX+pV6k/8qrHXCiA1L4647Z3c+opm3wrQ1zbaniNMbxfwzqA+ny0s= 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=XRK5sdeB; arc=none smtp.client-ip=209.85.160.41 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="XRK5sdeB" Received: by mail-oa1-f41.google.com with SMTP id 586e51a60fabf-3f11ad6e76fso1475865fac.1 for ; Mon, 01 Dec 2025 13:53:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1764626001; x=1765230801; 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=rFhnDBg0kXeD51glLwE29J4X5kSMJanTCsRTgbxRyOo=; b=XRK5sdeBGipUVk9GL2VDNJRXpH6y+lixsSeXsVf0z3fkYYbW3mjLwcVmiSa3cu8W1g kMpD5YcCSicygQ4JALuLnonzEW9irJMmfgMZM/YwGBWAsS8+WCQXbUpLFLFj5TXZmtgq wYQbE6b5XwZXVg4uWBMXT7ypzV/tqki9bgjVJzSXYV3+AKt47AfynXPK0OMiX5SnDN/g vr4IA3MBLaH9bJKqttnTjRwm+jZTWt1MkMnfFV8TvJf55pwObWR+lavOzGJyZvmGHDOm iAGn7XUJ6CFVsHRo+qi+5BXMTOSfeQh/M5K5a2gbTpzI4G2bpVllYX38/YwTfC7oy8Zb 6DfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764626001; x=1765230801; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=rFhnDBg0kXeD51glLwE29J4X5kSMJanTCsRTgbxRyOo=; b=mpGIsZefTlKCgAH/dwk432cKy/aQkny3V2GjKSxD5Int2SJl73l2h6qAM9mlSDxeQU aCq/DKnLui7/nWfFEudPYXS4koTLC1Ap+1InGFcGzLXpjN9mqoJpbqpMFn1xQPIjI7W0 hKO3rctnJ+qbMQK1JCaVCuQctZK5Dml5CJ/q42a6UDfqOdnsthZ1mVW0I+B80QjN2uyA B6CAZFYU0SFw034gE8s9FbvlBn1dKJ8OomB6SyLwWLZsEfnZwkz7P11jsInOmCwV3jbB 1DGI1sY7i8GljaOGy/8PKn/3Tve1j9p4REQceDn4pwuL7/bsPAE+Z0Kg8WtE6I5TUXq8 2aQg== X-Forwarded-Encrypted: i=1; AJvYcCUK4LEhyhaAp66U+shdQM4h1hcdq+SmX9xNcpOODFPjEKif0Tv0+kB+JzunXIeeAbzpSyg1e8Gm/FernvQ=@vger.kernel.org X-Gm-Message-State: AOJu0YxW21+c15AxpK8QzFSam4w193bq/Es/0yNMjTpNuac+ifk01gA8 j0pTJfnL711092F57YU4bxWNJAQ0eh9RLouN3FU1b5xbx+a2VHfeoWEGKTd6l/axZwc= X-Gm-Gg: ASbGncvZsBmiyO7rzhTRlkXJTdiVrgrtk7GcK88kLXIOAFOXixgG6qf518NodZaAaxs 7OjNYhcaDYSH0igIT1v6iVyF+Z/rt3yOlaV5xVXe7E//a3qz6d4V4yDRSf0kNm2mb0mmGlvvpu0 GTBuPG/iahB+4W3dqT/k9VwTxOJQfd7pN/BqhBQ8ZdPaJuJLMdKfCNtDQzjA27/KhSnpldrGL/d BMcvE5OMiGaTxtV85XUOuhc1CTXOnKf6VKNAR9fQg0WBRyi3N4Lb+t3X8BWWfA5dcJph2Frzj+8 b2Gr619wiOtTJKbhxSuQyfVd+xIXQt5W7eJKeGG3zNXIh33UhQIGMow1NEA7P1OSFVFUyPTJ7ey hg3bg6mEWDySzxy/JgAHkeSWCPhwRdFXv4pfk/5zDtt3EbUc4cx9IEme5zdA2wGE2gy+DSt/eqB y3D+vl2KYoxdnKuRHQFqgQpMQyIc/u9mO1pnHjlxaXICcyyjpyWJOPsZ3qRQ== X-Google-Smtp-Source: AGHT+IEUi4HQD4MeWgmtjcJxdxnSWi94Lov5ZIkoWcpAPeNP+EfY38zUZPLeRZvSH7fA6B55qw8ZqA== X-Received: by 2002:a05:6870:e0c6:b0:3d4:b76:5080 with SMTP id 586e51a60fabf-3ed1fd9424emr12729233fac.28.1764626001435; Mon, 01 Dec 2025 13:53:21 -0800 (PST) Received: from ?IPV6:2600:8803:e7e4:500:b67b:16c:f7ae:4908? ([2600:8803:e7e4:500:b67b:16c:f7ae:4908]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-3f0dd05bc9asm6083995fac.22.2025.12.01.13.53.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 01 Dec 2025 13:53:20 -0800 (PST) Message-ID: <248b009e-0401-4531-b9f0-56771e16bdef@baylibre.com> Date: Mon, 1 Dec 2025 15:53:19 -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 v3 2/2] iio: adc: Add ti-ads1018 driver To: Kurt Borja , Andy Shevchenko Cc: Jonathan Cameron , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Tobias Sperling , =?UTF-8?Q?Nuno_S=C3=A1?= , Andy Shevchenko , linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Jonathan Cameron References: <20251128-ads1x18-v3-0-a6ebab815b2d@gmail.com> <20251128-ads1x18-v3-2-a6ebab815b2d@gmail.com> <18fbf486-c1cc-4cd2-af12-ffa093fa9ce7@baylibre.com> Content-Language: en-US From: David Lechner In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 12/1/25 1:47 PM, Kurt Borja wrote: > On Mon Dec 1, 2025 at 11:07 AM -05, David Lechner wrote: > > ... > >>>>> + 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 = ads1018_read_unlocked(ads1018, &scan.conv, true); >>>>> + enable_irq(ads1018->drdy_irq); >>>>> + } else { >>>>> + ret = spi_read(ads1018->spi, ads1018->rx_buf, sizeof(ads1018->rx_buf)); >>>>> + scan.conv = 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->timestamp); >>>>> + >>>>> +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? >> >> No, ACQUIRE() is not scoped, just conditional. I don't think it >> will improve anything here. > > Maybe I'm not understanding the problem fully? > > I interpreted "ACQUIRE() class" as a general GUARD class, i.e. > > guard(iio_trigger_notify)(indio_dev->trig); > > This way drivers may use other cleanup.h helpers cleaner, because of the > goto problem? > > I do think it's a good idea, like a `defer` keyword. But it is a bit > unorthodox using guard for non locks. > > To take a simple example first: static int ads1018_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) { int ret; if (!iio_device_claim_direct(indio_dev)) return -EBUSY; ret = ads1018_read_raw_unlocked(indio_dev, chan, val, val2, mask); iio_device_release_direct(indio_dev); return ret; } using ACQUIRE would look like: static int ads1018_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) { int ret; ACQUIRE(iio_device_claim_direct_mode, claim)(indio_dev); if ((ret = ACQUIRE_ERR(iio_device_claim_direct_mode, &claim))) return ret; return ads1018_read_raw_unlocked(indio_dev, chan, val, val2, mask); } It makes it quite more verbose IMHO with little benefit (the direct return is nice, but comes at at an expense of the rest being less readable). And when we need it to be scoped, it adds indent and we have to do some unusual things still to avoid using goto. static irqreturn_t ads1018_trigger_handler(int irq, void *p) { struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct ads1018 *ads1018 = iio_priv(indio_dev); struct { __be16 conv; aligned_s64 ts; } scan = {}; int ret; do { ACQUIRE(iio_device_claim_direct_mode, claim)(indio_dev); if ((ret = ACQUIRE_ERR(iio_device_claim_direct_mode, &claim))) break; if (iio_trigger_using_own(indio_dev)) { disable_irq(ads1018->drdy_irq); ret = ads1018_read_unlocked(ads1018, &scan.conv, true); enable_irq(ads1018->drdy_irq); } else { ret = spi_read(ads1018->spi, ads1018->rx_buf, sizeof(ads1018->rx_buf)); scan.conv = ads1018->rx_buf[0]; } } while (0); if (!ret) iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), pf->timestamp); iio_trigger_notify_done(ads1018->indio_trig); return IRQ_HANDLED; } So unless Jonathan says this is what he wants, I would avoid it.