From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vs1-f45.google.com (mail-vs1-f45.google.com [209.85.217.45]) (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 7E83D19A288 for ; Tue, 2 Dec 2025 14:46:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.217.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764686803; cv=none; b=eKv3FWvaZl6TNgiljAAVVfzWuhiWDpXaCF9DLVG6iALdd0yfp8WPDD6VICs1yTiHV4LHx78XceDOhMT8R07FSsa35OidHnJIbA8Wv2U1ihV4Lf4SY5FU1pPPVdE3ujFH64w+kvfNMwpNX6V5K0Fii/HSsDGu6uTNDNCgDEj/mnI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764686803; c=relaxed/simple; bh=MoS7F23fetzjMdg5mb8OfGPgJPbLAfpu6CaMEvPxbyw=; h=Mime-Version:Content-Type:Date:Message-Id:To:Cc:Subject:From: References:In-Reply-To; b=eVhnTcWven8zuzFRfWXuWE5Y3dMj4+uKh5/zoySMSaGO5YtacuX00ei0h+Yn5DyoAWZI+O3XiTze9GEOov2AyQKv7xdbZA2dg2ReNIWVRQ0P6t9ccZFzDCdKmw3Up8R0/DW/exr92ekVP1Y+3EK1EM/fodLGZUy62kbaNWQu4h0= 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=hJjtyn7i; arc=none smtp.client-ip=209.85.217.45 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="hJjtyn7i" Received: by mail-vs1-f45.google.com with SMTP id ada2fe7eead31-5dbe6304b79so2020924137.3 for ; Tue, 02 Dec 2025 06:46:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764686800; x=1765291600; darn=vger.kernel.org; h=in-reply-to:references:from:subject:cc:to:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=7DY7E3WB0ICOl9V1+gQL7qAH+nekudiHdVslHe/Z11Y=; b=hJjtyn7ibgr/tPL4uemKfTtNOgaKZyzYDpQk8XlaNNTI9VYl3yvr6YGsuRKMWXyGCO 3cboKCZr9ZpYyMvdPUsFVREMHnCCV9JXkIJwcLcKdhRyyjx9Nu/k09Sk93aRQ0OR0cz3 DKkN7efd4+C4kP7rCrjz24dTerBY0rtmMYoqnxAj1zJ1wBwzK6yv5hgvs0xSjywuQWlt Q69j2qDKLiaipIBUMMZvFOvgGX26BB9b4q9qsFWxgwlMKw32hrB3QPl/U0O+TzVJtz/L Bed+CegQaPHPWs4Xe8T7Vmmaljgt+AA0x6Jj9qB1BBGhnevVSp3xZ0aJWKLRXEzUum2W CRJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764686800; x=1765291600; h=in-reply-to:references:from:subject:cc:to: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=7DY7E3WB0ICOl9V1+gQL7qAH+nekudiHdVslHe/Z11Y=; b=faU17kzj2XzwR3Ee8thGc9uJO8Lo2pQuA+MCr90LYhmb+GQJ/4wSiBOOWGVMFQ4cXN 1qHkfOZHKIOMlnWujH2uVdGxy53lOtHqTbSONBV6IzVgSrRww6azWLEKO+OgH6LqE8xA rEk2HABiXzNnYEtyUgyBvxxHKDp3hmYguRLjQR7xjPUcdnkFpwho00yt432LSDebtIg3 dI9xfpDxfDC/hFoP2LTe8fvNg+s4x5WA5hupQVfzHBwjsfTKwkNPxGUYjucFkeF6KAkd yn4cNFK91XyBNL3lS6itoEuCqvLs65kiVJBSFJyPoUlnoLwpPm0AUfrtQmBNYLCdlS/Z CyOw== X-Forwarded-Encrypted: i=1; AJvYcCUwPaLP6leupSrvs3lsI0cZxC9SoprkH0qJUXUxprIr1N6taJzv93isTP2lXDA9IpJHL0OwnBM/PytSyDg=@vger.kernel.org X-Gm-Message-State: AOJu0YyBMFcKI8nhv+YNzowq5LOtT849yolgmDwj5eMxxQ8oEYJqSvOt dPBTjH7ZHVza8lZil22PD68Ziwn5i7D4enjCK7L0D7OBi/v8r859hMCO X-Gm-Gg: ASbGncvg5zhbqIGDu82hZNmj9UnRxkPo4KvPlVU/m1QtDOUtIEjeJGbrTmMf+T38FB9 K/qCOhYqhbal13GPoeLo4vI9NTyTmNvZwcaVxIm1EnB/J/51GVwLR86TkmoLB+UfBuC5Ybmk+6v feyTf6mn4+H4RWekq2pKQCeMkxkEUR9u1o8tCqMeKat7tz9bTe0+TBjqXnBxgWM7NcdP+5FU4ZR HYXN0OarCLJD2ZVQtN4OyqLeQgzSV1e5LjxkRXl9jkvAyFBIrg/zI1htfdLXBXzd/LGIwUISdKM AQi0rrBTc5928AQGVeXd/huAV0cPTqvtQ6JnMUBXddP4lsAy4mWhuFvG/L1L1g1twMyCcfcpZii GquhskIYzu3JWVwt8uJQZUM+mNf4bdLFEowKZgTrCTOeZkg67F2BoQHTjTddlgGWKWrqa+NnTeZ mBDGw= X-Google-Smtp-Source: AGHT+IEzulod+TcRd82XYq5299hF3YYW0eriJP67UXNInTYJ+ZnrHWKSPy0cJtoLv0clmyFSXlS9Ew== X-Received: by 2002:a05:6102:4a94:b0:5db:d60a:6b24 with SMTP id ada2fe7eead31-5e1de342e84mr15526327137.22.1764686800032; Tue, 02 Dec 2025 06:46:40 -0800 (PST) Received: from localhost ([2800:bf0:82:3d2:875c:6c76:e06b:3095]) by smtp.gmail.com with ESMTPSA id a1e0cc1a2514c-93cd7661ae8sm6582676241.12.2025.12.02.06.46.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 02 Dec 2025 06:46:39 -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: Tue, 02 Dec 2025 09:46:37 -0500 Message-Id: To: "David Lechner" , "Kurt Borja" , "Andy Shevchenko" Cc: "Jonathan Cameron" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Tobias Sperling" , =?utf-8?q?Nuno_S=C3=A1?= , "Andy Shevchenko" , , , , "Jonathan Cameron" Subject: Re: [PATCH v3 2/2] iio: adc: Add ti-ads1018 driver From: "Kurt Borja" X-Mailer: aerc 0.21.0-0-g5549850facc2 References: <20251128-ads1x18-v3-0-a6ebab815b2d@gmail.com> <20251128-ads1x18-v3-2-a6ebab815b2d@gmail.com> <18fbf486-c1cc-4cd2-af12-ffa093fa9ce7@baylibre.com> <248b009e-0401-4531-b9f0-56771e16bdef@baylibre.com> In-Reply-To: <248b009e-0401-4531-b9f0-56771e16bdef@baylibre.com> On Mon Dec 1, 2025 at 4:53 PM -05, David Lechner wrote: > On 12/1/25 1:47 PM, Kurt Borja wrote: >> On Mon Dec 1, 2025 at 11:07 AM -05, David Lechner wrote: >>=20 >> ... >>=20 >>>>>> + 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->r= x_buf)); >>>>>> + 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->ti= mestamp); >>>>>> + >>>>>> +out_notify_done: >>>>>> + iio_trigger_notify_done(ads1018->indio_trig); >>>>> >>>>> Jonathan et al., maybe we need an ACQUIRE() class for this? It will s= olve >>>>> the conditional scoped guard case, no? >>> >>> No, ACQUIRE() is not scoped, just conditional. I don't think it >>> will improve anything here. >>=20 >> Maybe I'm not understanding the problem fully? >>=20 >> I interpreted "ACQUIRE() class" as a general GUARD class, i.e. >> =09 >> guard(iio_trigger_notify)(indio_dev->trig); >>=20 >> This way drivers may use other cleanup.h helpers cleaner, because of the >> goto problem? >>=20 >> I do think it's a good idea, like a `defer` keyword. But it is a bit >> unorthodox using guard for non locks. >>=20 >>=20 > > To take a simple example first: > > static int > ads1018_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *c= han, > int *val, int *val2, long mask) > { > int ret; > > if (!iio_device_claim_direct(indio_dev)) > return -EBUSY; > > ret =3D 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 *c= han, > int *val, int *val2, long mask) > { > int ret; > > ACQUIRE(iio_device_claim_direct_mode, claim)(indio_dev); > if ((ret =3D 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). This is verbose yes, but we could avoid having two functions in the first place and implement everything inside ads1018_read_raw() with ACQUIRE(...) on top. > > > > 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 =3D p; > struct iio_dev *indio_dev =3D pf->indio_dev; > struct ads1018 *ads1018 =3D iio_priv(indio_dev); > struct { > __be16 conv; > aligned_s64 ts; > } scan =3D {}; > int ret; > > do { > ACQUIRE(iio_device_claim_direct_mode, claim)(indio_dev); > if ((ret =3D ACQUIRE_ERR(iio_device_claim_direct_mode, &claim))) > break; > > 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_buf= )); > scan.conv =3D ads1018->rx_buf[0]; > } > } while (0); Here we could use scoped_cond_guard() instead, no? > > if (!ret) > iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), pf->timesta= mp); > > iio_trigger_notify_done(ads1018->indio_trig); > > return IRQ_HANDLED; > } > > So unless Jonathan says this is what he wants, I would avoid it. I will submit this as a separate RFC patch. We can continue the discussion there to avoid delaying this series. --=20 ~ Kurt