From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f50.google.com (mail-ot1-f50.google.com [209.85.210.50]) (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 6C82D2D0C63 for ; Tue, 23 Dec 2025 17:20:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766510422; cv=none; b=ESlbm356rMDjalITJZgoO0JV+ChjJ3wJfkG9Wj0xVZMnrAyogixrN//5nFpFpGA5OHdaeyJUiq7sG2+T/gh77n1Pq+OvLn9DD5QNJwCdAfqQpF8r78eTNQs9UfvNAeRcIGPBVfIzIai322/uIp2gF6r0C0LM/iRtWh26vLGCTPM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766510422; c=relaxed/simple; bh=BOangCFpjJ52aOedJ3g06NZWOG6nu/2RmNPphZdciHY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=p+N+g6UoDIO/NdGOuTnUQyZWxu9mVY+0ZJGFJ8r6Y5BskPiXcCu6NXntKi1f2HQdlHce+RGUqGJ90rYG5S6/T8WbTfbmowGtM4UGS6HQlnfgyQNOnoBNglW/prEtuXFW3JgUhz29gGynENMzqLJ7dV1kwQECoZI5LJWsZ4Vrr4k= 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=ZJCIwaCt; arc=none smtp.client-ip=209.85.210.50 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="ZJCIwaCt" Received: by mail-ot1-f50.google.com with SMTP id 46e09a7af769-7c75387bb27so2156983a34.1 for ; Tue, 23 Dec 2025 09:20:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1766510419; x=1767115219; 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=HqjoHDMp6UnfGET/5eFXYzvi2pMQ1f0rv1DZdq1dHzY=; b=ZJCIwaCtCX2dSWRKlMVB1kphCIgBnt7fxIXvdPXKgS35JHn8pp8VuWCG7VZkN6fG7a ux72e1Akd/T9etfDlpRfhWcBzF2EzL7BidNAadg/CUIDc/E/2S7iwmp+bzW9TD8QhUTF mjvwzKjbs7gZxaH2AXbFYfMLx0K4GxOu6dDBLJCHqy7dlHcyO0lcp++M1j3f4h0bODVa 81A05f9dtTCBdUuS8N32oYrsutxmBhyfqgenypKNREU5JNfFp8wisQCEOBb5UM1ZPD5N xUvpPIHYC06KEKjAV9jkuaVnvws4vlfOkN2MaEyW2NW4SYCH1OaNe4WoUdxClSDpGdaq k62g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1766510419; x=1767115219; 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=HqjoHDMp6UnfGET/5eFXYzvi2pMQ1f0rv1DZdq1dHzY=; b=qD4N9mq6R2S9oMVdwGpAIjA6qDFxmUTvjMYubOvwVuVkofxZJNLYYXrIV0sUILMLww TncF/uIH2a94AfYNYGX3NUnQBHy3qNpejYWFGKM6kcIN1DS7wzevpK69VtT3r1g9zDlY 1hBtlGfEodql5kwVsTYxMhRH5e+pgz6cNHypdkb0nko35TnGXqst0HxH02E7YwIh6zyk WCcd182x6s196N5S7tyawaZDQwrjR4lFP1R0pXdxa7fYKENZ/twV6YYl1eR5h1gD2eo7 Po5W8oRn27JzNLN4O78X8tHaS3+sPW0AvT0cBoxKCBYpwnuRxIzHlyhdhb50Lvw70ZM3 hwoA== X-Forwarded-Encrypted: i=1; AJvYcCWR6dpNzNTOwdiZEgBKrsnwBoG2l1ctVoHXaUGKFK2QU4oF4wqArQ5Su7/kdG6QCkvx91sC1JpxLJtqhBk=@vger.kernel.org X-Gm-Message-State: AOJu0YxtRfU4OfFUd7Cys6stZcunQo0FnpSMnka2BLphGydfjlf8x0rD 2MlDnComGTRoiTI5Ox+UDueS1s43MsQFxewhfJnCUua7QTGMJVGDNlGfqNBRtjw2VUA= X-Gm-Gg: AY/fxX6Nhkp4bBqZmPxTSH14CxpQ8UUWOL8ScQ1kiLFPrbufv1yEtCDY0NLfAN06xJ4 VqjI8ctZenmkp9rf3QbAiXLWaLPOE1smvEcz0pnFXXF6bHsCX0qJZ2RRkLyJ2bB2Wl+7xmh6sbF 4m6ROX1tkKdu7AT46Qn/mTof45VgG2vclyItgRMxeQiHSfb09PKIi0+Kt3XEfJx2AgZPoxAGHbP F2m6diTkIUvTFQgtwuxjrHz/EIRXiulbLJr1kNaJmC0lNuoTMa/qzg2iq9zmYx9tt7BmxXvrCMw Gos6QGLApgALdb9m/0fRf6FUpP3HAthykAYOTmRbry8gDXRqxviGjfpfUkLHfUwgLEAYV+STwcR rnspSFoZvL8MouIieNq/06jjIoFsaP6w2TwgwkruenfU6ZmXhYpkSM8BaItpr1pjgxRrSuqCP7i tQGbXAxpUbH05V7wubPpskMOAjg99s2A7TNHXlF8RaObxw/x6By1QW8h0vWtQw X-Google-Smtp-Source: AGHT+IFKHrsIz88QlZ38cn6wj+NStTXlLJClZOJ8CAkdAaG6odFkTXTwTPo7/oejo9fhLpdux+pQCA== X-Received: by 2002:a05:6830:7199:b0:7c2:8937:5d2e with SMTP id 46e09a7af769-7cc66901f28mr7039103a34.15.1766510419383; Tue, 23 Dec 2025 09:20:19 -0800 (PST) Received: from ?IPV6:2600:8803:e7e4:500:fe29:88f1:f763:378b? ([2600:8803:e7e4:500:fe29:88f1:f763:378b]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-7cc667ebe98sm9756043a34.21.2025.12.23.09.20.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Dec 2025 09:20:18 -0800 (PST) Message-ID: <6ad0f764-91f2-4946-81cd-9363ec87722a@baylibre.com> Date: Tue, 23 Dec 2025 11:20:17 -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 2/7] iio: core: Refactor iio_device_claim_direct() implementation To: Kurt Borja , Andy Shevchenko , Lars-Peter Clausen , Michael Hennerich , Jonathan Cameron , Benson Leung , Antoniu Miclaus , Gwendal Grignou , Shrikant Raskar , Per-Daniel Olsson Cc: =?UTF-8?Q?Nuno_S=C3=A1?= , Andy Shevchenko , Guenter Roeck , Jonathan Cameron , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, chrome-platform@lists.linux.dev References: <20251211-lock-impr-v2-0-6fb47bdaaf24@gmail.com> <20251211-lock-impr-v2-2-6fb47bdaaf24@gmail.com> Content-Language: en-US From: David Lechner In-Reply-To: <20251211-lock-impr-v2-2-6fb47bdaaf24@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 12/11/25 8:45 PM, Kurt Borja wrote: > In order to eventually unify the locking API, implement > iio_device_claim_direct() fully inline, with the use of > __iio_dev_mode_lock(), which takes care of sparse annotations. > > Signed-off-by: Kurt Borja > --- > drivers/iio/industrialio-core.c | 44 ----------------------------------------- > include/linux/iio/iio.h | 38 +++++++++++++++++++++++------------ > 2 files changed, 25 insertions(+), 57 deletions(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 1cce2d1ecef1..cffc6efb6617 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -2201,50 +2201,6 @@ void __iio_dev_mode_unlock(struct iio_dev *indio_dev) > } > EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock); > > -/** > - * __iio_device_claim_direct - Keep device in direct mode > - * @indio_dev: the iio_dev associated with the device > - * > - * If the device is in direct mode it is guaranteed to stay > - * that way until __iio_device_release_direct() is called. > - * > - * Use with __iio_device_release_direct(). > - * > - * Drivers should only call iio_device_claim_direct(). > - * > - * Returns: true on success, false on failure. > - */ > -bool __iio_device_claim_direct(struct iio_dev *indio_dev) > -{ > - struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > - > - mutex_lock(&iio_dev_opaque->mlock); > - > - if (iio_buffer_enabled(indio_dev)) { > - mutex_unlock(&iio_dev_opaque->mlock); > - return false; > - } > - return true; > -} > -EXPORT_SYMBOL_GPL(__iio_device_claim_direct); > - > -/** > - * __iio_device_release_direct - releases claim on direct mode > - * @indio_dev: the iio_dev associated with the device > - * > - * Release the claim. Device is no longer guaranteed to stay > - * in direct mode. > - * > - * Drivers should only call iio_device_release_direct(). > - * > - * Use with __iio_device_claim_direct() > - */ > -void __iio_device_release_direct(struct iio_dev *indio_dev) > -{ > - mutex_unlock(&to_iio_dev_opaque(indio_dev)->mlock); > -} > -EXPORT_SYMBOL_GPL(__iio_device_release_direct); > - > /** > * iio_device_claim_buffer_mode - Keep device in buffer mode > * @indio_dev: the iio_dev associated with the device > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > index aecda887d833..76398dbfa5ca 100644 > --- a/include/linux/iio/iio.h > +++ b/include/linux/iio/iio.h > @@ -664,30 +664,42 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp); > > void __iio_dev_mode_lock(struct iio_dev *indio_dev) __acquires(indio_dev); > void __iio_dev_mode_unlock(struct iio_dev *indio_dev) __releases(indio_dev); > -bool __iio_device_claim_direct(struct iio_dev *indio_dev); > -void __iio_device_release_direct(struct iio_dev *indio_dev); > > -/* > - * Helper functions that allow claim and release of direct mode > - * in a fashion that doesn't generate many false positives from sparse. > - * Note this must remain static inline in the header so that sparse > - * can see the __acquire() marking. Revisit when sparse supports > - * __cond_acquires() I think we should leave a comment that says these functions need to stay as static inline so that sparse works. Although I'm wondering how sparse could still work since __acquire() and __release() are removed here. IIRC it was a bit finicky about that. > +/** > + * iio_device_claim_direct - Keep device in direct mode > + * @indio_dev: the iio_dev associated with the device > + * > + * If the device is in direct mode it is guaranteed to stay > + * that way until iio_device_release_direct() is called. > + * > + * Use with iio_device_release_direct(). > + * > + * Returns: true on success, false on failure. > */ > static inline bool iio_device_claim_direct(struct iio_dev *indio_dev) > { > - if (!__iio_device_claim_direct(indio_dev)) > - return false; > + __iio_dev_mode_lock(indio_dev); > > - __acquire(iio_dev); > + if (iio_buffer_enabled(indio_dev)) { > + __iio_dev_mode_unlock(indio_dev); > + return false; > + } > > return true; > } > > +/** > + * iio_device_release_direct - Releases claim on direct mode > + * @indio_dev: the iio_dev associated with the device > + * > + * Release the claim. Device is no longer guaranteed to stay > + * in direct mode. > + * > + * Use with iio_device_claim_direct() > + */ > static inline void iio_device_release_direct(struct iio_dev *indio_dev) It could make more sense to make this: #define iio_device_release_direct __iio_dev_mode_unlock To make it clear that the auto cleanup functions that come later don't call iio_device_release_direct() but rather call __iio_dev_mode_unlock() directly. Otherwise, someone could be tempted to modify the iio_device_release_direct() function and the changes would not have an effect when auto cleanup is used. (Same applies to the release buffer mode function.) > { > - __iio_device_release_direct(indio_dev); > - __release(indio_dev); > + __iio_dev_mode_unlock(indio_dev); > } > > int iio_device_claim_buffer_mode(struct iio_dev *indio_dev); >