From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oo1-f54.google.com (mail-oo1-f54.google.com [209.85.161.54]) (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 D19C53081A4 for ; Tue, 23 Dec 2025 17:23:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766510609; cv=none; b=P8KT5toAOOqvl9E5sHaLYZsnYqD4y3LE6eK6NINqwFSNFkQEn7uLQjx0Pm66vSt/9RT1UP2PkcAhMJ5y2ZJ5UN501eRATKdx6pvTliFTS9+b60OgBn2Mh5OsfH0xUWaKVDX+FyG0QJZUWp5Hg5vhsFP9H1b9nM8qL1aTFTqmqLo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766510609; c=relaxed/simple; bh=sUecvfF8Cm+RDTkmMhY0GrrV5SoBCyMriYZBdj4xepw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=S4KGiGQ3RK7tPxfEhWgzuKtLXjt7wTXIG9kCoqIakEygTpXc7cEWdHCA3+WngH2Z9GAHijoKz2Ak7dhtzl16tRwdRNZ8jDVLwWyPRx545W/5fdCagCKXtwmsFuUFEJRCI7ShApYiFXuSSRoULKYAN0cXBpJ99Xp/e60jsxcfhMQ= 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=bIBlbaNC; arc=none smtp.client-ip=209.85.161.54 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="bIBlbaNC" Received: by mail-oo1-f54.google.com with SMTP id 006d021491bc7-65cf050a5cdso3315399eaf.1 for ; Tue, 23 Dec 2025 09:23:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1766510606; x=1767115406; 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=TzGkwjiXlIuI2e0ZltEyYghW6SSYBvcawlzgy/fyiHc=; b=bIBlbaNCDn9beTPQU+/D4B51MpYH0OJbW4qJStCRBR3CTnFA1YIdJw2gzQCJzM/h19 fHJtTESRbjCjivemBwZFeJ5LSpN64wR1xvKXYrJ2JN/yrM7KSvosm5Z5hdGGpbnHa6Pq +C4ave63geYCUR7VXH2qXxVpUSlR/liYqTZncKfsQ6T7sEnogAIQeLdNFmug9DthPLdd SX7zelzexgzCRPIcxbO9ksUDzTyo0lZxtGEjVnaNRcO6hUvACyY+YFcRE4S6PCxqJftN BeDOvTmEpCJTWXYLGxsxZcnv7dw4KiUEiY+mC3Jyw9wFOE3Bbxq6sHvz7mfNhmylHfw3 K/oQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1766510606; x=1767115406; 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=TzGkwjiXlIuI2e0ZltEyYghW6SSYBvcawlzgy/fyiHc=; b=rn9B5sworlzfMfyJzG0h1Da6kxtyOz1mDbWh/7DSbDRfd/QdPM+8iYxj/cFcWUR1wT FHoi7wEWyy5kPNE2N8tSkns+ipwntRre8EQ7oKUEecrrbw8gWFLKivniV7joMymK5GkY e/O/r+Sc093VjkV8fwqV4s+fYIkdbNdH52e0sfuC1hkvahUvKtTfjSPCsheLHCZWs4dB q3MGDUnagGTH8ISGYOX5wUeY7ZoD7rT8/VBwZuABLXZPAOwRpSL4M6G9qha8vLd/43/l 99Evz4mwa43ieYc5s7Nst3/56gY35fru45ryQvxXzZvR/P4oj4iykPU+xthZUngY9jjF /+wA== X-Forwarded-Encrypted: i=1; AJvYcCUfKcmeFM6FS8eD6INcu7PeWDmHAczh8NNda4V3FudEiHlmiHPAZ+DKS4OuxSrO2opvG2+xTdFTlTsG2/Y=@vger.kernel.org X-Gm-Message-State: AOJu0Yw30I7D5fq27Ohf1nEOqSQKvIxRaW0OKlLG1WAuo4JeG8boH62j l/iMLgJo4N6YCpMEwJ2i4Qfir6lnsB2UaGYGNEhP3hdH5IHSuVjkKFxladV1uLyusuw= X-Gm-Gg: AY/fxX5mKIGkhpkiSZN14SrDjBoDp/e6qQ4EsUvcUVQvP6virrjIdt89AksykZvLUZi RMY2Vtiiptan5j+v+fui1Ho4WVWoiEvgpkSZzs5oP11hz88HXYhUCL3tNUz1XXQgC1QLL70N/rF EqzDPE2iG942F6ryEs/J2aKOZ6yN2bJOT2EJlmYbz1HXaw0AjCTIATBLhRcx/3FQBdSbz/FOmqJ Br6/rkLDsoZAx2BDhZcwxn0Q83HANNXW1K5fx4aTTVDjxUEk80rtY5M0oeQAeI0unp/4MqPIkbD CjRIJ4S3tP2tWNm8qyYsKosRo2atHcWe8UGsaipeJpdf3QQrwiu0xzP9kUnDB4Iuoe91yPumsBT Sx2x5Li78230kHeRBvNAhfZ8IhUrPMer3tlAVXtMY1i1ExM0u+cKSUzcIwv5cD1kWN83Kannp63 2ZjcD2fBlNMquDA7M5N2PWede9uXOMMRTLID5dUuNMnPKoKaFojbFXB6Pi50tC X-Google-Smtp-Source: AGHT+IF6eNAmKWUZLe2B5K3E7g3i1rg1hxCty9Nbyk5DlxGfbQw0sm3r0j1+ICUtiDqdkuvUNTE5SA== X-Received: by 2002:a05:6820:4989:b0:65c:f869:1343 with SMTP id 006d021491bc7-65d0e1eefb8mr3656806eaf.11.1766510605628; Tue, 23 Dec 2025 09:23:25 -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 006d021491bc7-65d0f69ae7esm8989800eaf.9.2025.12.23.09.23.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Dec 2025 09:23:25 -0800 (PST) Message-ID: Date: Tue, 23 Dec 2025 11:23:24 -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 4/7] iio: core: Add cleanup.h support for iio_device_claim_*() 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-4-6fb47bdaaf24@gmail.com> Content-Language: en-US From: David Lechner In-Reply-To: <20251211-lock-impr-v2-4-6fb47bdaaf24@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 12/11/25 8:45 PM, Kurt Borja wrote: > Add guard classes for iio_device_claim_*() conditional locks. This will > aid drivers write safer and cleaner code when dealing with some common > patterns. > > These classes are not meant to be used directly by drivers (hence the > __priv__ prefix). Instead, documented wrapper macros are provided to > enforce the use of ACQUIRE() or guard() semantics and avoid the > problematic scoped guard. Would be useful to repeat this in a comment in the code. > > Suggested-by: Andy Shevchenko > Signed-off-by: Kurt Borja > --- > include/linux/iio/iio.h | 83 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 83 insertions(+) > > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > index f8a7ef709210..c84853c7a37f 100644 > --- a/include/linux/iio/iio.h > +++ b/include/linux/iio/iio.h > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -739,6 +740,88 @@ static inline void iio_device_release_buffer_mode(struct iio_dev *indio_dev) > __iio_dev_mode_unlock(indio_dev); > } > > +DEFINE_GUARD(__priv__iio_dev_mode_lock, struct iio_dev *, > + __iio_dev_mode_lock(_T), __iio_dev_mode_unlock(_T)); > +DEFINE_GUARD_COND(__priv__iio_dev_mode_lock, _try_buffer, > + iio_device_claim_buffer_mode(_T)); > +DEFINE_GUARD_COND(__priv__iio_dev_mode_lock, _try_direct, > + iio_device_claim_direct(_T)); > + > +/** > + * IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) - Tries to acquire the direct mode > + * lock with automatic release > + * @_dev: IIO device instance > + * @_var: Dummy variable identifier to store acquire result It's not a dummy if it does something. :-) (so we can drop that word) Also, I would call it _claim instead of _var to to match the example and encourage people to use the same name everywhere. And for that matter, we don't really need the leading underscores in either parameter since there are no name conflicts. > + * > + * Tries to acquire the direct mode lock with cleanup ACQUIRE() semantics and > + * automatically releases it at the end of the scope. It most be always paired > + * with IIO_DEV_ACQUIRE_ERR(), for example:: > + * > + * IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim); > + * if (IIO_DEV_ACQUIRE_ERR(&claim)) > + * return -EBUSY; > + * > + * ...or a more common scenario (notice scope the braces):: > + * > + * switch() { > + * case IIO_CHAN_INFO_RAW: { > + * IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim); > + * if (IIO_DEV_ACQUIRE_ERR(&claim)) > + * return -EBUSY; > + * > + * ... > + * } > + * case IIO_CHAN_INFO_SCALE: > + * ... > + * ... > + * } > + * > + * Context: Can sleep > + */ > +#define IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) \ > + ACQUIRE(__priv__iio_dev_mode_lock_try_direct, _var)(_dev) > + > +/** > + * IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) - Tries to acquire the buffer mode > + * lock with automatic release > + * @_dev: IIO device instance > + * @_var: Dummy variable identifier to store acquire result > + * > + * Tries to acquire the direct mode lock and automatically releases it at the > + * end of the scope. It most be paired with IIO_DEV_ACQUIRE_ERR(), for example:: > + * > + * IIO_DEV_ACQUIRE_BUFFER_MODE(indio_dev, claim); > + * if (IIO_DEV_ACQUIRE_ERR(&claim)) > + * return IRQ_HANDLED; > + * > + * Context: Can sleep > + */ > +#define IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) \ > + ACQUIRE(__priv__iio_dev_mode_lock_try_buffer, _var)(_dev) > + > +/** > + * IIO_DEV_ACQUIRE_ERR() - ACQUIRE_ERR() wrapper > + * @_var: Dummy variable passed to IIO_DEV_ACQUIRE_*_MODE() > + * > + * Return: true on success, false on error This could be clarified more. Based on the example, this sounds backwards. Returns: true if acquiring the mode failed, otherwise false. > + */ > +#define IIO_DEV_ACQUIRE_ERR(_var_ptr) \ > + ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_buffer, _var_ptr) There is no error code here, so calling it "ERR" seems wrong. Maybe IIO_DEV_ACQUIRE_FAILED()? > + > +/** > + * IIO_DEV_GUARD_ANY_MODE - Acquires the mode lock with automatic release > + * @_dev: IIO device instance It would be helpful to explain more about the use case here and that this is used rarely. The point to get across is that it is used when we need to do something that doesn't depend on the current mode, but would be affected by a mode switch. So it guards against changing the mode without caring what the current mode is. If it is in buffer mode, it stays in buffer mode, otherwise direct mode is claimed. > + * > + * Acquires the mode lock with cleanup guard() semantics. It is usually paired > + * with iio_buffer_enabled(). > + * > + * This should *not* be used to protect internal driver state and it's use in > + * general is *strongly* discouraged. Use any of the IIO_DEV_ACQUIRE_*_MODE() > + * variants. Might as well add Context: here like the others. > + */ > +#define IIO_DEV_GUARD_ANY_MODE(_dev) \ Accordingly, I would be inclined to call it IIO_DEV_GUARD_CURRENT_MODE() > + guard(__priv__iio_dev_mode_lock)(_dev) > + > extern const struct bus_type iio_bus_type; > > /** >