From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) (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 4D35735F185 for ; Tue, 25 Aug 2026 13:44:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787665495; cv=none; b=Ah/XdMLkyp9T823XvgdIE4TGosWrnTwZJ2aZpBgO6ely4hSar8aNENJ8mTpWjzEZgOa2dqHNYRhoZZeRwpvCnlr3QvP27aFQT30A4vD/8pBRb2sjBtujh4n7wV24Dl66Hk6AZsLoOSFLwXAioFxuYAirllUvBo7IJ5kECUUU8E0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787665495; c=relaxed/simple; bh=P5WYgFdBB2WI7NKSLc+xkQveSUZZ4oR4RAnxRZ3sOk4=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=iNpR+lmOFHAyb1iMMYVBxiaidVqtnTUeoWUiKMifSJUcFiY/RPFjgkSqF8AQlu2M9hGS6JIbgMAWYtgScoktJjeBgg2fNv+76h32C7AhoLSvmaAWZ4tzo8ZWABiJujp4bDKtmjDAHFxmOAtkmMxFCsq8vzJ6TUJ9kqzecAj7CY4= 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=dK32DNjg; arc=none smtp.client-ip=209.85.218.44 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="dK32DNjg" Received: by mail-ej1-f44.google.com with SMTP id a640c23a62f3a-c15d111ca99so577103866b.0 for ; Tue, 25 Aug 2026 06:44:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787665491; x=1788270291; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=FWi5OWCgDbdxS/z+vG1uBzNcpx684WDjQnVfCGs5vMo=; b=dK32DNjg2scjX673JNJzWSquArEbN6lfZ+BV7iTjzCBBg3Plg74V1BLyPqifApojqr Lb9yDXxmFAg/nkVntEc8pW4aP4BopdzVENDYZHFoqoPVVuVwQv0U85+iqSx4AuNkacfW tW9mOrehmpbSwn24G30MyiLEBC2oK+iCzdaT88imi24lPKBJNnZGkdbrJyb1z9GdKQZg lWihaeiBhDs5MFqTGkS1RX3llc8PCaoTiMcYuCzHKqHww6uPcN0fGSfhe1koQ/ZJenOO gSQmKBdexhX6+nDTWm18mzqH0p783MoyQs02S37ionpGGMtM93pXRl+x1K4AeichmhqK Tzsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787665491; x=1788270291; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=FWi5OWCgDbdxS/z+vG1uBzNcpx684WDjQnVfCGs5vMo=; b=nr/92gXNI/nRDxx1+cuFInCIF9cTqtHpfIJmEiYp3P9LqC9bAlI1JB3vEwsrfWdLI9 mTxhhMQTh7k2AIzm5IdTms9nu6v4bwWiKXiqAs2BuVX3/+Uf6nvo32YvXsP2k60gpFJN p9Ta6m27WA/P0yLvDXf/pXCCB9+cV2Hvlcsl/RUDTj9Pajx0Y91FhENb0Kv+SOe8Ai9k 8o9fAxe+hP8Ar8z1+5hwhXby/jb1aCMAh6EhfTbAvOzJuMcCKRAnahckfGyexbMJSDxD 0bsGQlOruz3rd2czQv/lCWZSo1/8AXG8Bb5nnQhh2t5JLxX76F0FAwUmKZOqPh32heC6 KiSA== X-Forwarded-Encrypted: i=1; AHgh+RrNs36k6Tju6A5p3h0GUl5z/M3zfRk81yeLXf86BUB9yUGjGKWnRgXyxRnYDxGQ2XMe83s7zHoxtKM/duo=@vger.kernel.org X-Gm-Message-State: AFuF++m/IfF7q4uR5iizf5I9xp6B/i9qDJnbHdWldzzRQ1buiuBFyHP9 hcwJiFlwDlBiUqgB39I40VGN2EDhgp+he3keWK5klIzkY8/Zdyn+Ozyx X-Gm-Gg: AR+sD11UIH5AWyHO0fVK0152REz+Ayz05t0y0QvpPEg14eBZ7SLMdD4PLMQkga8KK8Y VD9IIWYs63gBps9kual3tjpt2LkwHBraPHQApdmp+WQ2CFxRCkppgRMmYNxgsg4BEPkVJs4xHbL hu6jpDL+niOLWbKCUgJirTxDfvZv6lr/78n2Qiea9BDeFAa1AIQmwQWqmekAFHjDR7yXLXZKtiN 1RdlwOX9eh5gBqvAvagY9ZlCqm7XIDDrK8AfMAbq5xTSTvsGxQlSaGU9M8zD7DxbhpHjX8COSU/ 8CIK+TwQ3Jc7iTfbJ5wrUyP2rIj5OkUKlJZWm5y0oFk/lIfWf3GLSTSSlUkjq+ePVmIn9RgkHi9 VTHIeY6fm41Ew8WhRp2lsaXVYk9in9SQEuBPQu5oXKmRWxDcyPNasoj3sxMbg1oLrLzJESe+wnO sR7fzdoRIAnA6AtKPsNp/0xmkndcdONlzRkpbxO/1FEUTebJEFajy+yTUvT8aC3rgCINjma10Hn gPYufPI9HssbaUeEftOrtIZcgxh2KZemLPI1MBV9WJeMoGe0u+e3IR2UdVzS3s4KRTIitfm/Hpo QdmrGex9bbak37xfQzyfFFkQuNiQqDb2M+QKazxTVrpGQySOLuUlDhdTXH4bdLsKYZ2jY0pkjyD 8R+Xgq8RNLd4A0I/gsWUsx1sbZesbf0xE4Ue+nabUjCdRG2rP9H24HqWI/5Q2JBI4xBqcAV3taG wzt17J X-Received: by 2002:a17:907:9701:b0:c24:8af3:c92 with SMTP id a640c23a62f3a-c2504e504bbmr243031766b.3.1787665491183; Tue, 25 Aug 2026 06:44:51 -0700 (PDT) Received: from localhost (90-182-112-124.rcp.o2.cz. [90.182.112.124]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-c249606b59fsm2031244466b.12.2026.08.25.06.44.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Aug 2026 06:44:51 -0700 (PDT) Date: Tue, 25 Aug 2026 15:44:49 +0200 From: Joshua Crofts To: Esben Haabendal Cc: , , "Jonathan Cameron" , "Lars-Peter Clausen" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Martin Kepplinger" , "Sean Nyekjaer" , "David Lechner" , Nuno =?ISO-8859-1?Q?S=E1?= , "Andy Shevchenko" , "Martin Kepplinger" , Subject: Re: [PATCH v6 7/9] iio: accel: mma8452: Drop unneeded lock acquire on read Message-ID: <20260825154449.00002f6f@gmail.com> In-Reply-To: <87y0dumiga.fsf@geanix.com> References: <20260825-mma8452-open-drain-v6-0-9b252804ee80@geanix.com> <20260825-mma8452-open-drain-v6-7-9b252804ee80@geanix.com> <20260825121704.00004ed3@gmail.com> <87y0dumiga.fsf@geanix.com> X-Mailer: Claws Mail 4.4.0 (GTK 3.24.51; x86_64-w64-mingw32) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Tue, 25 Aug 2026 15:35:17 +0200 Esben Haabendal wrote: > "Joshua Crofts" writes: > > > On Tue, 25 Aug 2026 10:27:45 +0200 > > Esben Haabendal wrote: > > > >> There is no need to acquire data->lock when calling mma8452_read(), and > >> dropping that makes it less likely to end up in an AB-BA deadlock > >> situation. > >> > >> Signed-off-by: Esben Haabendal > >> --- > >> drivers/iio/accel/mma8452.c | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c > >> index 7ef1a9a91c31..9ae2c3e60576 100644 > >> --- a/drivers/iio/accel/mma8452.c > >> +++ b/drivers/iio/accel/mma8452.c > >> @@ -504,9 +504,7 @@ static int mma8452_read_raw(struct iio_dev *indio_dev, > >> if (!iio_device_claim_direct(indio_dev)) > >> return -EBUSY; > >> > >> - mutex_lock(&data->lock); > >> ret = mma8452_read(data, buffer); > >> - mutex_unlock(&data->lock); > >> iio_device_release_direct(indio_dev); > >> if (ret < 0) > >> return ret; > >> > > > > Sashiko has something to say and I tend to agree at the moment: > > > > Could removing this lock expose mma8452_read() to race conditions with PM > > auto-suspend and event configuration? > > Yes. I tend to agree as well. > > > mma8452_read() can be interrupted by the PM auto-suspend worker, which puts > > the device in STANDBY and disables regulators while mma8452_drdy() is actively > > polling over I2C. This can lead to I/O timeouts or errors. > > Yes, and causing pain for other I2C devices on same bus :( > > > Additionally, concurrent sysfs writes to event configurations invoke > > mma8452_change_config(), which puts the hardware into STANDBY to modify > > registers. The Standby transition flushes the hardware FIFO. If this occurs > > between the mma8452_drdy() check and the i2c_smbus_read_i2c_block_data() in > > mma8452_read(), the block read will fetch flushed or stale data. > > Argh. Yet another level of trouble. > > Maybe we should extend the use of data->lock instead. Holding it > 1. whenever doing read-modify-write actions > 2. while holding the device in standby mode for changing registers > 3. while doing suspend/resume > > I was just adding support for a open-drain mode irq sharing you know. > While sashiko-bot definitely is catching lots of valid problems, and > fixing them is a good thing, this is starting to feel like I opened > Pandoras box by touching this driver :D > > I will try to wrap up a patch with the above described extended usage of > data->lock, but hope we can find a way to get the irq sharing and open > drain mode support merged, without necessarily having to fixing all and every > possible existing bugs as a pre-condition, but maybe delay some work to > later work. > If they're pre-existing conditions then it's not required to fix those in the same patch series, you can come back to it another time or someone else can fix those. Nevertheless this is an issue that will be caused directly by this patch if applied. If your only goal is to add support for open-drain irq sharing, you can probably just drop this for the time being and just focus on that. -- Kind regards, Joshua Crofts