From patchwork Thu Sep 5 04:17:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 13791728 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 27BEACD4F58 for ; Thu, 5 Sep 2024 04:31:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=JXmUmnslyZHyZJLR+Zy+r2EKZKWq2y74P1oax/HM5cE=; b=dAd3cTwdYG+9xjKGJvsY+nOlc9 blMhexm+EyWMWiFodJk/rLhG76T9/2uSKQNTqvm+DvcpBEgSiD/iQEguQPpIcOMbUBL/d3dBHbTAK E/hkrRWDsMupm2ff1gm7ptnwegArExgTvLEBaoQ50RZsSMFBvvv2WQb9qE1fVTobfO5voU/uBT6+L GSFgLQwEi80NciZTUQPYC3rikzpO9VTIPu5zbTQueCMIkW6yp4FVYIrdUqF3XO9CPLCwQpmVcxo4E 6IR1NoDtjt7Q3onuh/E7yBtfAuCimTnHcHahZB8AerxLdUFNEcnP7GGEMMlMKTmpFaLqCM7XTFqN5 taVl9YWw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sm49D-00000006wAv-10ma; Thu, 05 Sep 2024 04:31:07 +0000 Received: from mail-ot1-x32b.google.com ([2607:f8b0:4864:20::32b]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sm3wZ-00000006tRu-3wcz for linux-arm-kernel@lists.infradead.org; Thu, 05 Sep 2024 04:18:05 +0000 Received: by mail-ot1-x32b.google.com with SMTP id 46e09a7af769-70f6e65d605so210181a34.3 for ; Wed, 04 Sep 2024 21:18:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725509883; x=1726114683; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=JXmUmnslyZHyZJLR+Zy+r2EKZKWq2y74P1oax/HM5cE=; b=erXoKqyyMsuCP2QYE3GqRQ5c7e0zfUuXS+v/HoXl377m9jr7vGa/prCN2PJthx+4eb NHDaeRsjlqlDgzveoNCUHg0xiORxB7fRNVYmQb6l6/PJzGkS7O9Yd8mQJKDMb+szlyXj 9Ur3cBQs7nxuQF6PwJ9H4Tfj0kJK3nBZGinvFscMzgiv8qXW8zR9LizexGSGV2fIb/DW jyKkDongf1DNbRHngIp9xPY5rAxyEnWSS5nC/nyCR/Dub2nSSzV/vdZJDVd2NFkc/P2C u6QtsV9kfIWUtiiT1flMU9rO97k59cYk/BhMCpnvxbFx/pT4tuyMU/Zg0eYjHEfjHzr1 vIMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725509883; x=1726114683; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JXmUmnslyZHyZJLR+Zy+r2EKZKWq2y74P1oax/HM5cE=; b=lGyrQH7nKWTaZNZbm3d3elLySzsjusNcwfqpjhk/97jpYrRyRYJXevWsKLD+Gfr8qK 7jbQ9C03hGlkfjfoLTmsEOqV3nBWtITTzhia/iphcRCHJA+3E5oLgEZrvdkgTi3z4aO8 3XVjdWRxjOdGRtXRBOTxp4npKi+ySgs7E89zy5huG0RgmZPJE8ck1NEoWyKcHw74aOW7 e+17pO4nKk6FT4+NdlWeqAcnPLbrsml+8VjWVhuWKkRMjtaSQ+KqEqsN86SQfFAb7nZp LvnJ32URpCtR0RlNTuHimnuPv7eoVAPDSr61RcjhIzo9KEZPKQvQkHfOfCj9n/YbA5Jq mNRQ== X-Forwarded-Encrypted: i=1; AJvYcCVU/ByxvVlLVGOzpwo0ht7oS8gZa+Io4AMLlplWXtNd8DzsnjcpiyVb84iPUiqvXZLpJsp00qvWQTIO3I1I04EA@lists.infradead.org X-Gm-Message-State: AOJu0YxMEO2ZfvhY94xJVPgLl2KXBBPOnBAk1RROFTIspOM0ExTG7g3R s+7q6zOfp6dK5ra4GuvoiJGpZIfLpY/ImaGGfjKRpKlCMhNYFGlh X-Google-Smtp-Source: AGHT+IHcMsyUFlTpAfo6lJFBaVdiWg5Gnvhy6xBpLteR2E7XriaKtgwaYzGHtMaMdLGv9PUE1ZycOA== X-Received: by 2002:a05:6830:4111:b0:709:4060:1093 with SMTP id 46e09a7af769-70f5c467082mr29658656a34.22.1725509882956; Wed, 04 Sep 2024 21:18:02 -0700 (PDT) Received: from dtor-ws.sjc.corp.google.com ([2620:15c:9d:2:13bd:b4e:4c0f:4c37]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7d4fbd8d52esm2450216a12.32.2024.09.04.21.18.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Sep 2024 21:18:02 -0700 (PDT) From: Dmitry Torokhov To: linux-input@vger.kernel.org Cc: =?utf-8?q?Pali_Roh=C3=A1r?= , Helge Deller , "K. Y. Srinivasan" , Wei Liu , Dexuan Cui , Samuel Holland , Lyude Paul , Michal Simek , Hans de Goede , linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev Subject: [PATCH 13/24] Input: i8042 - use guard notation when acquiring spinlock Date: Wed, 4 Sep 2024 21:17:18 -0700 Message-ID: <20240905041732.2034348-14-dmitry.torokhov@gmail.com> X-Mailer: git-send-email 2.46.0.469.g59c65b2a67-goog In-Reply-To: <20240905041732.2034348-1-dmitry.torokhov@gmail.com> References: <20240905041732.2034348-1-dmitry.torokhov@gmail.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240904_211804_010928_7AE8D403 X-CRM114-Status: GOOD ( 20.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section. Signed-off-by: Dmitry Torokhov --- drivers/input/serio/i8042.c | 204 +++++++++++++++--------------------- 1 file changed, 82 insertions(+), 122 deletions(-) diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index 674cd155ec8f..86916fe3925f 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -197,42 +197,26 @@ EXPORT_SYMBOL(i8042_unlock_chip); int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str, struct serio *serio)) { - unsigned long flags; - int ret = 0; + guard(spinlock_irqsave)(&i8042_lock); - spin_lock_irqsave(&i8042_lock, flags); - - if (i8042_platform_filter) { - ret = -EBUSY; - goto out; - } + if (i8042_platform_filter) + return -EBUSY; i8042_platform_filter = filter; - -out: - spin_unlock_irqrestore(&i8042_lock, flags); - return ret; + return 0; } EXPORT_SYMBOL(i8042_install_filter); int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str, struct serio *port)) { - unsigned long flags; - int ret = 0; - - spin_lock_irqsave(&i8042_lock, flags); + guard(spinlock_irqsave)(&i8042_lock); - if (i8042_platform_filter != filter) { - ret = -EINVAL; - goto out; - } + if (i8042_platform_filter != filter) + return -EINVAL; i8042_platform_filter = NULL; - -out: - spin_unlock_irqrestore(&i8042_lock, flags); - return ret; + return 0; } EXPORT_SYMBOL(i8042_remove_filter); @@ -271,28 +255,22 @@ static int i8042_wait_write(void) static int i8042_flush(void) { - unsigned long flags; unsigned char data, str; int count = 0; - int retval = 0; - spin_lock_irqsave(&i8042_lock, flags); + guard(spinlock_irqsave)(&i8042_lock); while ((str = i8042_read_status()) & I8042_STR_OBF) { - if (count++ < I8042_BUFFER_SIZE) { - udelay(50); - data = i8042_read_data(); - dbg("%02x <- i8042 (flush, %s)\n", - data, str & I8042_STR_AUXDATA ? "aux" : "kbd"); - } else { - retval = -EIO; - break; - } - } + if (count++ >= I8042_BUFFER_SIZE) + return -EIO; - spin_unlock_irqrestore(&i8042_lock, flags); + udelay(50); + data = i8042_read_data(); + dbg("%02x <- i8042 (flush, %s)\n", + data, str & I8042_STR_AUXDATA ? "aux" : "kbd"); + } - return retval; + return 0; } /* @@ -349,17 +327,12 @@ static int __i8042_command(unsigned char *param, int command) int i8042_command(unsigned char *param, int command) { - unsigned long flags; - int retval; - if (!i8042_present) return -1; - spin_lock_irqsave(&i8042_lock, flags); - retval = __i8042_command(param, command); - spin_unlock_irqrestore(&i8042_lock, flags); + guard(spinlock_irqsave)(&i8042_lock); - return retval; + return __i8042_command(param, command); } EXPORT_SYMBOL(i8042_command); @@ -369,19 +342,18 @@ EXPORT_SYMBOL(i8042_command); static int i8042_kbd_write(struct serio *port, unsigned char c) { - unsigned long flags; - int retval = 0; + int error; - spin_lock_irqsave(&i8042_lock, flags); + guard(spinlock_irqsave)(&i8042_lock); - if (!(retval = i8042_wait_write())) { - dbg("%02x -> i8042 (kbd-data)\n", c); - i8042_write_data(c); - } + error = i8042_wait_write(); + if (error) + return error; - spin_unlock_irqrestore(&i8042_lock, flags); + dbg("%02x -> i8042 (kbd-data)\n", c); + i8042_write_data(c); - return retval; + return 0; } /* @@ -460,9 +432,8 @@ static int i8042_start(struct serio *serio) device_set_wakeup_enable(&serio->dev, true); } - spin_lock_irq(&i8042_lock); + guard(spinlock_irq)(&i8042_lock); port->exists = true; - spin_unlock_irq(&i8042_lock); return 0; } @@ -476,10 +447,10 @@ static void i8042_stop(struct serio *serio) { struct i8042_port *port = serio->port_data; - spin_lock_irq(&i8042_lock); - port->exists = false; - port->serio = NULL; - spin_unlock_irq(&i8042_lock); + scoped_guard(spinlock_irq, &i8042_lock) { + port->exists = false; + port->serio = NULL; + } /* * We need to make sure that interrupt handler finishes using @@ -583,45 +554,41 @@ static bool i8042_handle_data(int irq) { struct i8042_port *port; struct serio *serio; - unsigned long flags; unsigned char str, data; unsigned int dfl; unsigned int port_no; bool filtered; - spin_lock_irqsave(&i8042_lock, flags); - - str = i8042_read_status(); - if (unlikely(~str & I8042_STR_OBF)) { - spin_unlock_irqrestore(&i8042_lock, flags); - return false; - } - - data = i8042_read_data(); + scoped_guard(spinlock_irqsave, &i8042_lock) { + str = i8042_read_status(); + if (unlikely(~str & I8042_STR_OBF)) + return false; - if (i8042_mux_present && (str & I8042_STR_AUXDATA)) { - port_no = i8042_handle_mux(str, &data, &dfl); - } else { + data = i8042_read_data(); - dfl = (str & I8042_STR_PARITY) ? SERIO_PARITY : 0; - if ((str & I8042_STR_TIMEOUT) && !i8042_notimeout) - dfl |= SERIO_TIMEOUT; + if (i8042_mux_present && (str & I8042_STR_AUXDATA)) { + port_no = i8042_handle_mux(str, &data, &dfl); + } else { - port_no = (str & I8042_STR_AUXDATA) ? - I8042_AUX_PORT_NO : I8042_KBD_PORT_NO; - } + dfl = (str & I8042_STR_PARITY) ? SERIO_PARITY : 0; + if ((str & I8042_STR_TIMEOUT) && !i8042_notimeout) + dfl |= SERIO_TIMEOUT; - port = &i8042_ports[port_no]; - serio = port->exists ? port->serio : NULL; + port_no = (str & I8042_STR_AUXDATA) ? + I8042_AUX_PORT_NO : I8042_KBD_PORT_NO; + } - filter_dbg(port->driver_bound, data, "<- i8042 (interrupt, %d, %d%s%s)\n", - port_no, irq, - dfl & SERIO_PARITY ? ", bad parity" : "", - dfl & SERIO_TIMEOUT ? ", timeout" : ""); + port = &i8042_ports[port_no]; + serio = port->exists ? port->serio : NULL; - filtered = i8042_filter(data, str, serio); + filter_dbg(port->driver_bound, + data, "<- i8042 (interrupt, %d, %d%s%s)\n", + port_no, irq, + dfl & SERIO_PARITY ? ", bad parity" : "", + dfl & SERIO_TIMEOUT ? ", timeout" : ""); - spin_unlock_irqrestore(&i8042_lock, flags); + filtered = i8042_filter(data, str, serio); + } if (likely(serio && !filtered)) serio_interrupt(serio, data, dfl); @@ -779,24 +746,22 @@ static bool i8042_irq_being_tested; static irqreturn_t i8042_aux_test_irq(int irq, void *dev_id) { - unsigned long flags; unsigned char str, data; - int ret = 0; - spin_lock_irqsave(&i8042_lock, flags); + guard(spinlock_irqsave)(&i8042_lock); + str = i8042_read_status(); - if (str & I8042_STR_OBF) { - data = i8042_read_data(); - dbg("%02x <- i8042 (aux_test_irq, %s)\n", - data, str & I8042_STR_AUXDATA ? "aux" : "kbd"); - if (i8042_irq_being_tested && - data == 0xa5 && (str & I8042_STR_AUXDATA)) - complete(&i8042_aux_irq_delivered); - ret = 1; - } - spin_unlock_irqrestore(&i8042_lock, flags); + if (!(str & I8042_STR_OBF)) + return IRQ_NONE; + + data = i8042_read_data(); + dbg("%02x <- i8042 (aux_test_irq, %s)\n", + data, str & I8042_STR_AUXDATA ? "aux" : "kbd"); + + if (i8042_irq_being_tested && data == 0xa5 && (str & I8042_STR_AUXDATA)) + complete(&i8042_aux_irq_delivered); - return IRQ_RETVAL(ret); + return IRQ_HANDLED; } /* @@ -837,7 +802,6 @@ static int i8042_check_aux(void) int retval = -1; bool irq_registered = false; bool aux_loop_broken = false; - unsigned long flags; unsigned char param; /* @@ -921,18 +885,15 @@ static int i8042_check_aux(void) if (i8042_enable_aux_port()) goto out; - spin_lock_irqsave(&i8042_lock, flags); - - init_completion(&i8042_aux_irq_delivered); - i8042_irq_being_tested = true; - - param = 0xa5; - retval = __i8042_command(¶m, I8042_CMD_AUX_LOOP & 0xf0ff); - - spin_unlock_irqrestore(&i8042_lock, flags); + scoped_guard(spinlock_irqsave, &i8042_lock) { + init_completion(&i8042_aux_irq_delivered); + i8042_irq_being_tested = true; - if (retval) - goto out; + param = 0xa5; + retval = __i8042_command(¶m, I8042_CMD_AUX_LOOP & 0xf0ff); + if (retval) + goto out; + } if (wait_for_completion_timeout(&i8042_aux_irq_delivered, msecs_to_jiffies(250)) == 0) { @@ -1020,7 +981,6 @@ static int i8042_controller_selftest(void) static int i8042_controller_init(void) { - unsigned long flags; int n = 0; unsigned char ctr[2]; @@ -1057,14 +1017,14 @@ static int i8042_controller_init(void) * Handle keylock. */ - spin_lock_irqsave(&i8042_lock, flags); - if (~i8042_read_status() & I8042_STR_KEYLOCK) { - if (i8042_unlock) - i8042_ctr |= I8042_CTR_IGNKEYLOCK; - else - pr_warn("Warning: Keylock active\n"); + scoped_guard(spinlock_irqsave, &i8042_lock) { + if (~i8042_read_status() & I8042_STR_KEYLOCK) { + if (i8042_unlock) + i8042_ctr |= I8042_CTR_IGNKEYLOCK; + else + pr_warn("Warning: Keylock active\n"); + } } - spin_unlock_irqrestore(&i8042_lock, flags); /* * If the chip is configured into nontranslated mode by the BIOS, don't