linux-kernel - [RFT PATCH] gpio: eic-sprd: use atomic notifiers to notify all chips about irqs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230904123320.93980-1-brgl@bgdev.pl>
Date:   Mon,  4 Sep 2023 14:33:20 +0200
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Andy Shevchenko <andy@...nel.org>,
        Orson Zhai <orsonzhai@...il.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        Chunyan Zhang <zhang.lyra@...il.com>
Cc:     linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: [RFT PATCH] gpio: eic-sprd: use atomic notifiers to notify all chips about irqs

From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>

Calling gpiochip_find() from interrupt handler in this driver is an
abuse of the GPIO API. It only happens to work because nobody added a
might_sleep() to it and the lock used by GPIOLIB is a spinlock.

Both will soon be changed as we're limiting both the number of
interfaces allowed to be called from atomic context as well as making
struct gpio_chip private to the GPIO code that owns it. We'll also
switch to protecting the global GPIO device list with a mutex as there
is no reason to allow changes to it from interrupt handlers.

Instead of iterating over all SPRD chips and looking up each
corresponding GPIO chip, let's make each SPRD GPIO controller register
with a notifier chain. The chain will be called at interrupt so that
every chip that already probed will be notified. The rest of the
interrupt handling remains the same. This should result in faster code as
we're avoiding iterating over the list of all GPIO devices.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
---
I only build-tested it. Please take it for a ride, I hope this works.

 drivers/gpio/gpio-eic-sprd.c | 44 ++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
index 5320cf1de89c..21a1afe358d6 100644
--- a/drivers/gpio/gpio-eic-sprd.c
+++ b/drivers/gpio/gpio-eic-sprd.c
@@ -9,6 +9,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
@@ -91,12 +92,20 @@ enum sprd_eic_type {
 
 struct sprd_eic {
 	struct gpio_chip chip;
+	struct notifier_block irq_nb;
 	void __iomem *base[SPRD_EIC_MAX_BANK];
 	enum sprd_eic_type type;
 	spinlock_t lock;
 	int irq;
 };
 
+static ATOMIC_NOTIFIER_HEAD(sprd_eic_irq_notifier);
+
+static struct sprd_eic *to_sprd_eic(struct notifier_block *nb)
+{
+	return container_of(nb, struct sprd_eic, irq_nb);
+}
+
 struct sprd_eic_variant_data {
 	enum sprd_eic_type type;
 	u32 num_eics;
@@ -494,13 +503,6 @@ static void sprd_eic_toggle_trigger(struct gpio_chip *chip, unsigned int irq,
 	sprd_eic_irq_unmask(data);
 }
 
-static int sprd_eic_match_chip_by_type(struct gpio_chip *chip, void *data)
-{
-	enum sprd_eic_type type = *(enum sprd_eic_type *)data;
-
-	return !strcmp(chip->label, sprd_eic_label_name[type]);
-}
-
 static void sprd_eic_handle_one_type(struct gpio_chip *chip)
 {
 	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
@@ -546,27 +548,29 @@ static void sprd_eic_handle_one_type(struct gpio_chip *chip)
 static void sprd_eic_irq_handler(struct irq_desc *desc)
 {
 	struct irq_chip *ic = irq_desc_get_chip(desc);
-	struct gpio_chip *chip;
-	enum sprd_eic_type type;
 
 	chained_irq_enter(ic, desc);
 
 	/*
 	 * Since the digital-chip EIC 4 sub-modules (debounce, latch, async
-	 * and sync) share one same interrupt line, we should iterate each
-	 * EIC module to check if there are EIC interrupts were triggered.
+	 * and sync) share one same interrupt line, we should notify all of
+	 * them to let them check if there are EIC interrupts were triggered.
 	 */
-	for (type = SPRD_EIC_DEBOUNCE; type < SPRD_EIC_MAX; type++) {
-		chip = gpiochip_find(&type, sprd_eic_match_chip_by_type);
-		if (!chip)
-			continue;
-
-		sprd_eic_handle_one_type(chip);
-	}
+	atomic_notifier_call_chain(&sprd_eic_irq_notifier, 0, NULL);
 
 	chained_irq_exit(ic, desc);
 }
 
+static int sprd_eic_irq_notify(struct notifier_block *nb, unsigned long action,
+			       void *data)
+{
+	struct sprd_eic *sprd_eic = to_sprd_eic(nb);
+
+	sprd_eic_handle_one_type(&sprd_eic->chip);
+
+	return NOTIFY_OK;
+}
+
 static const struct irq_chip sprd_eic_irq = {
 	.name		= "sprd-eic",
 	.irq_ack	= sprd_eic_irq_ack,
@@ -653,7 +657,9 @@ static int sprd_eic_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	return 0;
+	sprd_eic->irq_nb.notifier_call = sprd_eic_irq_notify;
+	return atomic_notifier_chain_register(&sprd_eic_irq_notifier,
+					      &sprd_eic->irq_nb);
 }
 
 static const struct of_device_id sprd_eic_of_match[] = {
-- 
2.39.2