greybus: gpio: create irqdomain before registering gpio controller

If a gpio line is getting used for an irq, gpio core will create
/proc/irq/X/gpiolib/ directory for it. This directory gets removed while
the gpio controller is unregistered.

In case of greybus, gb_gpio_irqchip_add() creates an irqdomain and
creates irq mappings for the gpio lines. Currently they are added after
registering the gpio controller and removed before the gpio controller
is removed. On the removal path, while the core tries to remove the irq
directory (/proc/irq/X), it finds that the irq is still getting used and
a "gpiolib" directory is present within it and so it gives this warning:

Steps to reproduce:

    $ cd /sys/class/gpio
    $ echo X > export
    $ echo both > gpioX/edge
    $ echo <interface-number> > /sys/bus/greybus/devices/1-svc/intf_eject

[  139.171436] ------------[ cut here ]------------
[  139.171468] WARNING: at /home/vireshk/all/work/repos/ara/arche/kernel/arche/fs/proc/generic.c:552 remove_proc_entry+0x154/0x188()
[  139.171476] remove_proc_entry: removing non-empty directory 'irq/683', leaking at least 'gpiolib'
[  139.171589] Modules linked in: gb_vibrator(O) gb_usb(O) gb_uart(O) gb_spi(O) gb_sdio(O) gb_raw(O) gb_pwm(O) gb_power_supply(O) gb_loopback(O) gb_log(O) gb_light(O) gb_i2c(O) gb_hid(O) gb_gpio(O) gb_gbphy(O) gb_firmware(O) gb_spilib(O) gb_es2(O) gb_camera(O) gb_bootrom(O) gb_audio_module(O) gb_audio_manager(O) gb_audio_codec(O) gb_audio_gb(O) gb_audio_apbridgea(O) gb_arche(O) greybus(O)
[  139.171605] CPU: 1 PID: 280 Comm: kworker/u16:4 Tainted: G        W  O 3.10.83-g9771b10cbeed #107
[  139.171652] Workqueue: greybus1:svc gb_svc_intf_set_power_mode [greybus]
[  139.171657] Call trace:
[  139.171677] [<ffffffc000207b40>] dump_backtrace+0x0/0x268
[  139.171689] [<ffffffc000207db8>] show_stack+0x10/0x1c
[  139.171707] [<ffffffc000ccad78>] dump_stack+0x1c/0x28
[  139.171723] [<ffffffc00021f9dc>] warn_slowpath_common+0x74/0x9c
[  139.171735] [<ffffffc00021fa60>] warn_slowpath_fmt+0x5c/0x80
[  139.171747] [<ffffffc00035fa38>] remove_proc_entry+0x150/0x188
[  139.171763] [<ffffffc00027464c>] unregister_irq_proc+0xb4/0xdc
[  139.171779] [<ffffffc00026e3f4>] free_desc+0x2c/0x70
[  139.171791] [<ffffffc00026e48c>] irq_free_descs+0x54/0x9c
[  139.171802] [<ffffffc000273448>] irq_dispose_mapping+0x54/0x64
[  139.171814] [<ffffffbffc0621e8>] 0xffffffbffc0621e8
[  139.171825] [<ffffffbffc05e01c>] 0xffffffbffc05e01c
[  139.171843] [<ffffffc0005d4e30>] __device_release_driver+0x90/0xe4
[  139.171854] [<ffffffc0005d4ea4>] device_release_driver+0x20/0x38
[  139.171867] [<ffffffc0005d4634>] bus_remove_device+0x12c/0x148
[  139.171878] [<ffffffc0005d1cb0>] device_del+0x108/0x16c
[  139.171888] [<ffffffc0005d1d64>] device_unregister+0x50/0x68
[  139.171901] [<ffffffbffc05e2bc>] gb_gbphy_deregister_driver+0xf0/0x4ec [gb_gbphy]
[  139.171924] [<ffffffbffc0014c4>] greybus_disabled+0x14c4/0x1760 [greybus]
[  139.171936] [<ffffffc0005d4e30>] __device_release_driver+0x90/0xe4
[  139.171948] [<ffffffc0005d4ea4>] device_release_driver+0x20/0x38
[  139.171959] [<ffffffc0005d4634>] bus_remove_device+0x12c/0x148
[  139.171969] [<ffffffc0005d1cb0>] device_del+0x108/0x16c
[  139.171992] [<ffffffbffc004cec>] gb_bundle_destroy+0x7c/0x1b0 [greybus]
[  139.172017] [<ffffffbffc004074>] gb_interface_disable+0xb4/0x178 [greybus]
[  139.172040] [<ffffffbffc002ae4>] gb_module_del+0x5c/0xf8 [greybus]
[  139.172063] [<ffffffbffc008418>] gb_svc_intf_set_power_mode+0xea0/0xfe8 [greybus]
[  139.172078] [<ffffffc00023888c>] process_one_work+0x268/0x3c8
[  139.172089] [<ffffffc000239a64>] worker_thread+0x204/0x358
[  139.172108] [<ffffffc00023f43c>] kthread+0xb8/0xc4
[  139.172114] ---[ end trace 6fa3314e8c6157ca ]---

Also note that registering the gpio controller before creating irqdomain
is incorrect as well and may lead to kernel panic, as a gpio may get
requested as an interrupt source right after the controller is
registered, and the greybus gpio driver wouldn't be fully ready by then.

This patch changes the sequence in both probe() and remove() to fix it.

Fixes: 426e88a47d39 ("greybus: gpio: add interrupt handling support")
Reported-by: David Hsu <davidhsu@google.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c
index 8fa9998b4c3d79b94e3a42c4d999640510b50775..294e2f52f1d41facd9b7400c49a0c4eca65df8db 100644 (file)
--- a/drivers/staging/greybus/gpio.c
+++ b/drivers/staging/greybus/gpio.c
@@ -708,26 +708,26 @@ static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
        if (ret)
                goto exit_line_free;
 
-       ret = gpiochip_add(gpio);
+       ret = gb_gpio_irqchip_add(gpio, irqc, 0,
+                                  handle_level_irq, IRQ_TYPE_NONE);
        if (ret) {
                dev_err(&connection->bundle->dev,
-                       "failed to add gpio chip: %d\n", ret);
+                       "failed to add irq chip: %d\n", ret);
                goto exit_line_free;
        }
 
-       ret = gb_gpio_irqchip_add(gpio, irqc, 0,
-                                  handle_level_irq, IRQ_TYPE_NONE);
+       ret = gpiochip_add(gpio);
        if (ret) {
                dev_err(&connection->bundle->dev,
-                       "failed to add irq chip: %d\n", ret);
-               goto exit_gpiochip_remove;
+                       "failed to add gpio chip: %d\n", ret);
+               goto exit_gpio_irqchip_remove;
        }
 
        gbphy_runtime_put_autosuspend(gbphy_dev);
        return 0;
 
-exit_gpiochip_remove:
-       gb_gpiochip_remove(gpio);
+exit_gpio_irqchip_remove:
+       gb_gpio_irqchip_remove(ggc);
 exit_line_free:
        kfree(ggc->lines);
 exit_connection_disable:
@@ -750,8 +750,8 @@ static void gb_gpio_remove(struct gbphy_device *gbphy_dev)
                gbphy_runtime_get_noresume(gbphy_dev);
 
        gb_connection_disable_rx(connection);
-       gb_gpio_irqchip_remove(ggc);
        gb_gpiochip_remove(&ggc->chip);
+       gb_gpio_irqchip_remove(ggc);
        gb_connection_disable(connection);
        gb_connection_destroy(connection);
        kfree(ggc->lines);