From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f42.google.com (mail-lf1-f42.google.com [209.85.167.42]) (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 0396E39B97E for ; Mon, 18 May 2026 05:21:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779081698; cv=none; b=ex4c3WG2DvNFIGcRvoAao/oUdNBM+v5ISTAVdOuSb2UwryOpEU3JUZcTvLFs5UTG/DYJ22YC/lXfSuPG3LS+sM6agUAdbE5or/4NnOR4z5suhCgL50yFKfNkbjRlArJ1EcUwr9gPn0T4M+OnCgvYK9+nveQbtYhT2LnpCvJlOls= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779081698; c=relaxed/simple; bh=vNdKnsYYZlKkKhRJ8ehWd4hcpi7UO7QeQ3RXo2YHBqw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=E3YSVvoLFP9h23aq2G36b6mY+sL+3no9Fxqi0bH0mHR8eWYBbTNmLRmHbsviM0/NslD+Ei3cm/R8O8TJn7NZn1NBcKnSA/3tIP6G5crkJh9Ea35HUzEDaYFisHr/4qalNuOPK74J2a9oestkRteDUcqP/+qF5lfMEDZnH3SeiLk= 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=O4+i1Ho2; arc=none smtp.client-ip=209.85.167.42 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="O4+i1Ho2" Received: by mail-lf1-f42.google.com with SMTP id 2adb3069b0e04-5a40cfab24dso2386967e87.2 for ; Sun, 17 May 2026 22:21:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779081680; x=1779686480; 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=COvqQNzUKLRtL3QdO0fWmfEMf02deV0mUukamXhTeL4=; b=O4+i1Ho2ZPlGtLDsYR83zal+utYP1A/8ULtF3LhaUZm1ZSrLId4v1xiuH0tL7TundM mCCcKMvLWP/N0uHWhBl+HOyGZw8jgLLij5y/QDPGYJl1cylLXXYyeESBqSLrAKZ/fAfl SCqUHmJOtYexiOkSxPsXdXruFC6excpJcNubHoC1I4TNN64FWyki5qjiHq/y6/+uTSRI 8NL9+e1c4XsfXaMnaXUp3Y9bwMFLbc1+6vSavocVV3oaHUS9U5eRySQDpO9FYfHHkXtI 6DOL4nr7UsrVi0wZ/DFZR1FB159hTz5Bf5aoDwtuZxj2vunEMiXHglBu4akFXY+ncbEe 9HdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779081680; x=1779686480; 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=COvqQNzUKLRtL3QdO0fWmfEMf02deV0mUukamXhTeL4=; b=f6LrfSMbcgCkKbDyLY4IyKlXKpHmfxC256kDy2w8a9iLQCvRv39jywchk6xq9p0+aS fT1GQ8ao/FBNAlHsgNWRPBJRDI7R2bYyqT674vug8AK0gn3rF1KsKaRVDmueH/ZWiMTE 7h6x4wrBC04aht4++6UvezwfUwUoIy6wqKj1/Bs5TUYVtuzPldb6DTyC7rUleAkC7ojX ecP5Mk0fu8J4g5JMoZfO5ujtAw10uj5fIgu/kiBYaP3Lg/rmYP88wZcRN6tWjoajYhR8 jDwiOnFSRSWmSH0vWdF6kADpB9Dugs+kIXiv9JkVEoF451GA6G4q/fYoDkacRccCyxHj dhbw== X-Forwarded-Encrypted: i=1; AFNElJ+rrJHG7boeS3Ssn2/g67uVkASdK1M/bnn6AHYSpPzE13794D07XUZIgp29mvRiIGM+0qUW4gEJFhmUQRE=@vger.kernel.org X-Gm-Message-State: AOJu0YycZncFV3btNzGIYt6M38sLpo+LG4Ez9Wmn19MgZlNVj8MQFRzy uqJnT5qxKIqFB4YZ/kbs+Mqso0lZKdw5duTKbJpM5teshBHRnGPBExU3 X-Gm-Gg: Acq92OHuU5dDHm2C+0rVCwBHwIZXHWjNmvIoSauAChR1KxXwfm6ry9wPZkG1H6O3rJl 8djCepS3VdSzGgKivV/YxpS/y/VdydqxUm18asaKcgo7r7AjwCEdk/phDeDiGbLQTViy4RUSIzQ Isg9ffhAQurjEL4NZBk9ytMjIc/wRBT+McuSX6pukwFYfGptAtAteC7Zyu7YIl8ipyw0ITDXkMg bXzElv0GbJpTMh4q3/nAiEldETqwQ1+pRQ6qIu5vqevr0DE5mB+kkuUtHR6vkswPkLRWKS52VJi 73v81PXAnd5MKzoLH1byieMegcs5Sjmr0UQel/s6lBEi0rmdII1k+upZJqviizuoCNQZXxhFimQ x8iGG1tbG47w5DMut4SIrngVQVXlK6Y6efsQCrTz3e2qMorS5+1nlnGmmHjTaU1ZGgaLFDab49L lWyI76hfJQgSE2Dtd9bDnZg65oNG/YX8vAt86FxdikltRDpsMonZPvUwvaUlFzMsbSOlMBXDiC0 NJiGNMzZGpOG4bvkL4= X-Received: by 2002:a05:6512:1193:b0:5a8:fc93:85c1 with SMTP id 2adb3069b0e04-5aa0e73c325mr3744302e87.32.1779081679717; Sun, 17 May 2026 22:21:19 -0700 (PDT) Received: from ?IPV6:2a10:a5c0:800d:dd00:8fdf:935a:2c85:d703? ([2a10:a5c0:800d:dd00:8fdf:935a:2c85:d703]) by smtp.gmail.com with ESMTPSA id 38308e7fff4ca-395882cf497sm10171221fa.16.2026.05.17.22.21.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 17 May 2026 22:21:18 -0700 (PDT) Message-ID: <3cb30f12-8b4f-415f-9a1d-823d8ff8c33b@gmail.com> Date: Mon, 18 May 2026 08:21:17 +0300 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] iio: pressure: rohm-bm1390: notify trigger on all error paths To: David Lechner , Stepan Ionichev Cc: jic23@kernel.org, nuno.sa@analog.com, andy@kernel.org, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org References: <20260517160801.269-1-sozdayvek@gmail.com> <54ee1fba-3209-4192-82c3-674a1ae3ca8f@baylibre.com> Content-Language: en-US, en-AU, en-GB, en-BW From: Matti Vaittinen In-Reply-To: <54ee1fba-3209-4192-82c3-674a1ae3ca8f@baylibre.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 17/05/2026 20:12, David Lechner wrote: > On 5/17/26 11:08 AM, Stepan Ionichev wrote: >> bm1390_trigger_handler() has three error returns: >> >> if (ret || !status) >> return IRQ_NONE; /* status read failed */ >> ... >> if (ret) { >> dev_warn(...); >> return IRQ_NONE; /* pressure read failed */ >> } >> ... >> if (ret) { >> dev_warn(...); >> return IRQ_HANDLED; /* temp read failed */ >> } >> >> None of them call iio_trigger_notify_done(). The success path at the >> end does, so on a single transient regmap or pressure-read error the >> trigger never sees its use_count decremented, and the >> !atomic_read(&trig->use_count) guard in iio_trigger_poll_chained() >> drops every subsequent dispatch for that trigger. The buffered-data >> flow stays wedged until the trigger is detached. I don't really know the intended logic of the use_count, so I'll leave this to those who understand it better. I'll just add some thoughts this invoked. I think it is not really nice to require (or trust) drivers to call the "iio_trigger_notify_done()" if the handler fails. Maybe it would be better to do something like: void iio_trigger_poll_nested(struct iio_trigger *trig) { int i; if (!atomic_read(&trig->use_count)) { atomic_set(&trig->use_count, CONFIG_IIO_CONSUMERS_PER_TRIGGER); for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) { if (trig->subirqs[i].enabled) handle_nested_irq(trig->subirq_base + i); else iio_trigger_notify_done(trig); } atomic_set(&trig->use_count, 0); /* Clear the use_count if drivers didn't */ } } to prevent this class of problems once and for all. But yeah, wiser minds have designed this - so let's hear some other opinions as well :) >> >> The IRQ_HANDLED return on the temperature path additionally leaves >> the temp branch's last partial state in &data->buf.temp without >> pushing the sample, which is the existing intended behaviour; only >> the missing notify_done() needs fixing. >> >> Funnel all returns through a single 'done' label that calls >> iio_trigger_notify_done() before returning the saved irqreturn_t. >> >> Fixes: 81ca5979b6ed ("iio: pressure: Support ROHM BU1390") >> Cc: stable@vger.kernel.org >> Signed-off-by: Stepan Ionichev >> --- >> drivers/iio/pressure/rohm-bm1390.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/iio/pressure/rohm-bm1390.c b/drivers/iio/pressure/rohm-bm1390.c >> index 08146ca0f..c18352399 100644 >> --- a/drivers/iio/pressure/rohm-bm1390.c >> +++ b/drivers/iio/pressure/rohm-bm1390.c >> @@ -626,12 +626,15 @@ static irqreturn_t bm1390_trigger_handler(int irq, void *p) >> struct iio_poll_func *pf = p; >> struct iio_dev *idev = pf->indio_dev; >> struct bm1390_data *data = iio_priv(idev); >> + irqreturn_t result = IRQ_HANDLED; >> int ret, status; >> >> /* DRDY is acked by reading status reg */ >> ret = regmap_read(data->regmap, BM1390_REG_STATUS, &status); >> - if (ret || !status) >> - return IRQ_NONE; >> + if (ret || !status) { >> + result = IRQ_NONE; > > IRQ_NONE means that the interrupt wasn't handled, so it won't be cleared > and the handler will likely just run again immediately. So it probably > isn't the right thing to be returning in the first place. This is exactly why IRQ-none is returned, and what it is used for. If the problem with bus-access / device persists, the kernel will (after XXXX fails indicated by IRQ_NONE - don't remember exact numbers) disable the IRQ from the host side, and emit the, ass-saving, "nobody cared" -print. This is (in my opinion) the only RightThing(tm). (Especially so, if the device is accessed from the fast handler, and is system is single-core). There is a tremendous difference when debugging a system which just hangs in IRQ loop forever (and you can't get no contact to it), and when debugging a system which, after a relatively short hang-up, let's you see the magic "nobody cared" -print telling a misbehaving IRQ was disabled. Furthermore, if the status register read failure was a temporary one, then we should be getting new IRQ as soon as the handler exists. This should then successfully handle the IRQ. Yours, -- Matti -- --- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~