From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A737C18D636; Sat, 25 Jul 2026 15:14:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784992487; cv=none; b=kapgWh2OBT12fvmIUFS9HPZuO0PI6Eeq4QlWCeTU4yQuRuXo+jOxvkt5zPSKXKT4BloESGrWUbns/SnaH8qVnr2GQzsl+8bXcCqFKPkcZJ13+CkZUEeLYc2E6GwQhM5JFdTRHthl5aLBwDKiJQyClmjRRtbAy3RmL/iDt++8Xbg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784992487; c=relaxed/simple; bh=Pd5+gZ916x4UoYwMUSBK1X9hHU3kpd6+8DKd3ddiB3M=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=BRKkp2urjQJfQRDCnRZ/H7qE2D6dYHu3XGHw45/FUyMBeb3vSov0ebnWqgQ5SMFyjAio9Vtt3tL6Gex2Py41WzGkFam+MhtP9Pkbw0swbqSeWGaxdgObSYm2ZS4jcljrc7uWzEYh7mHXGBxIShBZxqwv1Ye+LCN2hYClOg62/nk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Qs8VJcit; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Qs8VJcit" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E854E1F000E9; Sat, 25 Jul 2026 15:14:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1784992486; bh=0M+yE6prQ2OKvNbO+KreUV7CnRFCglGHnMiB1NRoSSU=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=Qs8VJcitSlfSx7SaCq70xOpBgS3cIy0UjVAuOarFnkbeqXGsJLp4kUApHMx7p8otr jDiyXUU6kw2Vr8ab4M+m84o/jk/+OzelU0yYfCtPdUGsQpxXDKzI7Reslu5NuDrWQH G+NY0GmOMs1ML5elKv9J+/ycpzfhD/RgF7lPX6XXM2d1x/K217DoKAMC9dE3wy1jOo vRPF4irDUwaMNcytJM++c3sZdkiAi8FrkMDELIPk7EkdSD9mYo4TOw8Utg59wObYXp CXfuyo+E5iZv0EnHaAv+zSeh0VkMKVobt7Af2TXvItviGZ7Tn2a/7zq4v43og+O+fY tW2AIzSmskSHA== From: sashiko-bot@kernel.org Subject: Re: [PATCH v2 3/8] rtc: abx80x: properly handle shared IRQs Reply-To: sashiko-reviews@lists.linux.dev To: "Antoni Pokusinski" Cc: conor+dt@kernel.org, linux-rtc@vger.kernel.org, robh@kernel.org, "Alexandre Belloni" , devicetree@vger.kernel.org In-Reply-To: <20260725145718.14733-4-apokusinski01@gmail.com> References: <20260725145718.14733-1-apokusinski01@gmail.com> <20260725145718.14733-4-apokusinski01@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Sat, 25 Jul 2026 15:14:45 +0000 Message-Id: <20260725151445.E854E1F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [High] Read-Modify-Write race condition causes lost hardware interrupts o= n a Write-0-to-Clear (W0C) register. - [High] Returning IRQ_NONE for unhandled RTC interrupts causes a hard inte= rrupt storm. -- --- Patch [3]: [PATCH v2 3/8] rtc: abx80x: properly handle shared IRQs --- commit 3fccf3aa21cff336c32ceacf1b3d453203dbb5b8 Author: Antoni Pokusinski rtc: abx80x: properly handle shared IRQs The abx80x RTC driver registers its interrupt handler with IRQF_SHARED, but previously always returned IRQ_HANDLED and cleared the status register even when no interrupt source from the RTC was active. This patch updates the handler to only handle the interrupt when alarm or watchdog flags are set, clearing only those handled bits. It returns IRQ_NONE when the interrupt was not generated by the RTC. > diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c > index ca3af91c29977..fd9e887bb9c56 100644 > --- a/drivers/rtc/rtc-abx80x.c > +++ b/drivers/rtc/rtc-abx80x.c > @@ -261,25 +261,32 @@ static irqreturn_t abx80x_handle_irq(int irq, void = *dev_id) [ ... ] > - i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, 0); > + if (handled =3D=3D IRQ_HANDLED) > + i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, > + status & ~(ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT)); [Severity: High] Does this read-modify-write operation introduce a regression by creating a race condition that could lose hardware interrupts? The ABX8XX_REG_STATUS register has Write-0-to-Clear (W0C) semantics, meaning writing 0 clears a bit and writing 1 preserves it. Because this writes status & ~(ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT), it writes 0 to any bit that was 0 at the time of the earlier i2c_smbus_read_byte_data() call. If a new status event is asserted by the hardware between the read and the write, won't this write of 0 unintentionally clear it? > =20 > - return IRQ_HANDLED; > + return handled; > } [Severity: High] Could returning IRQ_NONE here without clearing the status register cause a regression where an interrupt storm occurs if an unhandled RTC interrupt fi= res? Since abx80x_probe() does not initialize the interrupt mask register (ABX8XX_REG_IRQ), a bootloader could leave an unknown interrupt enabled.=20 If one of these unhandled interrupts fires, handled remains IRQ_NONE. Because the register is only cleared when handled =3D=3D IRQ_HANDLED, the unhandled interrupt is never cleared from the RTC. Returning IRQ_NONE would then cause the IRQ core to re-trigger the handler in an infinite loop until the spurious IRQ detector permanently disables the shared IRQ line. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260725145718.1473= 3-1-apokusinski01@gmail.com?part=3D3