From patchwork Tue Oct 1 02:43:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jamin Lin X-Patchwork-Id: 13818083 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3B101CEACC5 for ; Tue, 1 Oct 2024 14:12:45 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1svdc4-0007fn-Mv; Tue, 01 Oct 2024 10:12:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1svdbe-0007SB-1V; Tue, 01 Oct 2024 10:12:02 -0400 Received: from mail.aspeedtech.com ([211.20.114.72] helo=TWMBX01.aspeed.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1svdbc-0002zy-Cg; Tue, 01 Oct 2024 10:12:01 -0400 Received: from TWMBX01.aspeed.com (192.168.0.62) by TWMBX01.aspeed.com (192.168.0.62) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1258.12; Tue, 1 Oct 2024 10:43:35 +0800 Received: from localhost.localdomain (192.168.10.10) by TWMBX01.aspeed.com (192.168.0.62) with Microsoft SMTP Server id 15.2.1258.12 via Frontend Transport; Tue, 1 Oct 2024 10:43:35 +0800 To: =?utf-8?q?C=C3=A9dric_Le_Goater?= , Peter Maydell , Steven Lee , Troy Lee , Andrew Jeffery , "Joel Stanley" , Thomas Huth , Laurent Vivier , Paolo Bonzini , "open list:ASPEED BMCs" , "open list:All patches CC here" CC: , , Subject: [PATCH v7 4/8] hw/gpio/aspeed: Fix clear incorrect interrupt status for GPIO index mode Date: Tue, 1 Oct 2024 10:43:30 +0800 Message-ID: <20241001024334.834807-5-jamin_lin@aspeedtech.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20241001024334.834807-1-jamin_lin@aspeedtech.com> References: <20241001024334.834807-1-jamin_lin@aspeedtech.com> MIME-Version: 1.0 Received-SPF: pass client-ip=211.20.114.72; envelope-from=jamin_lin@aspeedtech.com; helo=TWMBX01.aspeed.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_FAIL=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-to: Jamin Lin X-Patchwork-Original-From: Jamin Lin via From: Jamin Lin Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The interrupt status field is W1C, where a set bit on read indicates an interrupt is pending. If the bit extracted from data is set it should clear the corresponding bit in reg_value. However, if the extracted bit is clear then the value of the corresponding bit in reg_value should be unchanged. SHARED_FIELD_EX32() extracts the interrupt status bit from the write (data). reg_value is set to the set's interrupt status, which means that for any pin with an interrupt pending, the corresponding bit is set. The deposit32() call updates the bit at pin_idx in the reg_value, using the value extracted from the write (data). The result is that if multiple interrupt status bits were pending and the write was acknowledging specific one bit, then the all interrupt status bits will be cleared. However, it is index mode and should only clear the corresponding bit. For example, say we have an interrupt pending for GPIOA0, where the following statements are true: set->int_status == 0b01 s->pending == 1 Before it is acknowledged, an interrupt becomes pending for GPIOA1: set->int_status == 0b11 s->pending == 2 A write is issued to acknowledge the interrupt for GPIOA0. This causes the following sequence: reg_value == 0b11 pending == 2 s->pending == 0 set->int_status == 0b00 It should only clear bit 0 in index mode and the correct result should be as following. set->int_status == 0b11 s->pending == 2 pending == 1 s->pending == 1 set->int_status == 0b10 Signed-off-by: Jamin Lin Suggested-by: Andrew Jeffery Reviewed-by: Andrew Jeffery --- hw/gpio/aspeed_gpio.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index 8725606aec..16c18ea2f7 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -641,7 +641,7 @@ static void aspeed_gpio_write_index_mode(void *opaque, hwaddr offset, uint32_t pin_idx = reg_idx_number % ASPEED_GPIOS_PER_SET; uint32_t group_idx = pin_idx / GPIOS_PER_GROUP; uint32_t reg_value = 0; - uint32_t cleared; + uint32_t pending = 0; set = &s->sets[set_idx]; props = &agc->props[set_idx]; @@ -703,16 +703,23 @@ static void aspeed_gpio_write_index_mode(void *opaque, hwaddr offset, FIELD_EX32(data, GPIO_INDEX_REG, INT_SENS_2)); set->int_sens_2 = update_value_control_source(set, set->int_sens_2, reg_value); - /* set interrupt status */ - reg_value = set->int_status; - reg_value = deposit32(reg_value, pin_idx, 1, - FIELD_EX32(data, GPIO_INDEX_REG, INT_STATUS)); - cleared = ctpop32(reg_value & set->int_status); - if (s->pending && cleared) { - assert(s->pending >= cleared); - s->pending -= cleared; + /* interrupt status */ + if (FIELD_EX32(data, GPIO_INDEX_REG, INT_STATUS)) { + /* pending is either 1 or 0 for a 1-bit field */ + pending = extract32(set->int_status, pin_idx, 1); + + assert(s->pending >= pending); + + /* No change to s->pending if pending is 0 */ + s->pending -= pending; + + /* + * The write acknowledged the interrupt regardless of whether it + * was pending or not. The post-condition is that it mustn't be + * pending. Unconditionally clear the status bit. + */ + set->int_status = deposit32(set->int_status, pin_idx, 1, 0); } - set->int_status &= ~reg_value; break; case gpio_reg_idx_debounce: reg_value = set->debounce_1;