From: Linus Walleij <linus.walleij@linaro.org>
To: linux-gpio@vger.kernel.org,
Alexandre Courbot <acourbot@nvidia.com>,
Alexander Stein <alexander.stein@systec-electronic.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
linux-input@vger.kernel.org,
Tomeu Vizoso <tomeu.vizoso@collabora.com>,
Guenter Roeck <linux@roeck-us.net>
Subject: [PATCH] gpiolib: handle probe deferrals better
Date: Fri, 1 Apr 2016 13:44:08 +0200 [thread overview]
Message-ID: <1459511048-24084-1-git-send-email-linus.walleij@linaro.org> (raw)
The gpiolib does not currently return probe deferrals from the
.to_irq() hook while the GPIO drivers are being initialized.
Further: it keeps returning -EPROBE_DEFER for gpio[d]_get()
even after all builtin drivers have initialized.
Fix this thusly:
- Move the assignment of .to_irq() to the last step when
using gpiolib_irqchip_add() so we can't get spurious calls
into the .to_irq() function until all set-up is finished.
- Put in a late_initcall_sync() to set a boolean state variable to
indicate that we're not gonna defer any longer. Since deferred
probe happens at late_initcall() time, using late_initcall_sync()
should be fine.
- After this point, return hard errors (-ENXIO) from both
gpio[d]_get() and .to_irq().
This way we should (at least for all drivers using GPIOLIB_IRQCHIP)
be getting proper deferrals from both gpio[d]_get() and .to_irq()
until the irqchip side is properly set up, and then proper
errors after all drivers should have been probed.
This problem was first seen with gpio-keys.
Cc: linux-input@vger.kernel.org
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Alexander: please test this, you need Guether's patches too,
I have it all on my "fixes" branch in the GPIO git:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=fixes
Tomeu: I think you're the authority on deferred probe these days.
Is this the right way for a subsystem to switch from returning
-EPROBE_DEFER to instead returning an unrecoverable error?
Guenther: I applied this on top of your patches, please check it
if you can, you're smarter than me with this stuff.
---
drivers/gpio/gpiolib.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b747c76fd2b1..426a93f9d79e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -68,7 +68,9 @@ LIST_HEAD(gpio_devices);
static void gpiochip_free_hogs(struct gpio_chip *chip);
static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip);
+/* These keep the state of the library */
static bool gpiolib_initialized;
+static bool gpiolib_builtin_ready;
static inline void desc_set_label(struct gpio_desc *d, const char *label)
{
@@ -1093,7 +1095,6 @@ int _gpiochip_irqchip_add(struct gpio_chip *gpiochip,
gpiochip->irqchip = irqchip;
gpiochip->irq_handler = handler;
gpiochip->irq_default_type = type;
- gpiochip->to_irq = gpiochip_to_irq;
gpiochip->lock_key = lock_key;
gpiochip->irqdomain = irq_domain_add_simple(of_node,
gpiochip->ngpio, first_irq,
@@ -1129,6 +1130,12 @@ int _gpiochip_irqchip_add(struct gpio_chip *gpiochip,
}
acpi_gpiochip_request_interrupts(gpiochip);
+ /*
+ * Wait with this until last, as someone may be asynchronously
+ * calling .to_irq() and needs to be getting probe deferrals until
+ * this point.
+ */
+ gpiochip->to_irq = gpiochip_to_irq;
return 0;
}
@@ -1366,12 +1373,21 @@ done:
int gpiod_request(struct gpio_desc *desc, const char *label)
{
- int status = -EPROBE_DEFER;
+ int status;
struct gpio_device *gdev;
VALIDATE_DESC(desc);
gdev = desc->gdev;
+ /*
+ * Defer requests until all built-in drivers have had a chance
+ * to probe, then give up and return a hard error.
+ */
+ if (!gpiolib_builtin_ready)
+ status = -EPROBE_DEFER;
+ else
+ status = -ENXIO;
+
if (try_module_get(gdev->owner)) {
status = __gpiod_request(desc, label);
if (status < 0)
@@ -1993,18 +2009,27 @@ EXPORT_SYMBOL_GPL(gpiod_cansleep);
* gpiod_to_irq() - return the IRQ corresponding to a GPIO
* @desc: gpio whose IRQ will be returned (already requested)
*
- * Return the IRQ corresponding to the passed GPIO, or an error code in case of
- * error.
+ * Return the IRQ corresponding to the passed GPIO, or an error code.
*/
int gpiod_to_irq(const struct gpio_desc *desc)
{
- struct gpio_chip *chip;
- int offset;
+ struct gpio_chip *chip;
+ int offset;
VALIDATE_DESC(desc);
chip = desc->gdev->chip;
offset = gpio_chip_hwgpio(desc);
- return chip->to_irq ? chip->to_irq(chip, offset) : -ENXIO;
+
+ if (chip->to_irq)
+ return chip->to_irq(chip, offset);
+ /*
+ * We will wait for new GPIO drivers to arrive until the
+ * late initcalls. After that we stop deferring and return
+ * a hard error.
+ */
+ if (!gpiolib_builtin_ready)
+ return -EPROBE_DEFER;
+ return -ENXIO;
}
EXPORT_SYMBOL_GPL(gpiod_to_irq);
@@ -2883,6 +2908,14 @@ static int __init gpiolib_dev_init(void)
}
core_initcall(gpiolib_dev_init);
+static int __init gpiolib_late_done(void)
+{
+ /* Flag that we're not deferring probes anymore */
+ gpiolib_builtin_ready = true;
+ return 0;
+}
+late_initcall_sync(gpiolib_late_done);
+
#ifdef CONFIG_DEBUG_FS
static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
--
2.4.3
next reply other threads:[~2016-04-01 11:44 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-01 11:44 Linus Walleij [this message]
2016-04-01 12:16 ` [PATCH] gpiolib: handle probe deferrals better Alexander Stein
2016-04-01 13:03 ` Linus Walleij
2016-04-01 13:42 ` Alexander Stein
2016-04-01 14:03 ` Grygorii Strashko
2016-04-01 14:35 ` Alexander Stein
2016-04-01 14:04 ` Guenter Roeck
2016-04-01 17:52 ` Bjorn Andersson
2016-04-01 12:42 ` Guenter Roeck
2016-04-01 13:28 ` Grygorii Strashko
2016-04-04 16:21 ` Grygorii Strashko
2016-04-06 13:39 ` Linus Walleij
2016-04-06 15:42 ` Grygorii Strashko
2016-04-07 17:09 ` Linus Walleij
2016-04-11 6:10 ` Alexander Stein
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1459511048-24084-1-git-send-email-linus.walleij@linaro.org \
--to=linus.walleij@linaro.org \
--cc=acourbot@nvidia.com \
--cc=alexander.stein@systec-electronic.com \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=tomeu.vizoso@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).