From 88f6ba61f25bfe6eb92cb8f511b0879cdfbb64d3 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 3 Aug 2016 15:52:10 -0700 Subject: [PATCH] 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 > /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] [] dump_backtrace+0x0/0x268 [ 139.171689] [] show_stack+0x10/0x1c [ 139.171707] [] dump_stack+0x1c/0x28 [ 139.171723] [] warn_slowpath_common+0x74/0x9c [ 139.171735] [] warn_slowpath_fmt+0x5c/0x80 [ 139.171747] [] remove_proc_entry+0x150/0x188 [ 139.171763] [] unregister_irq_proc+0xb4/0xdc [ 139.171779] [] free_desc+0x2c/0x70 [ 139.171791] [] irq_free_descs+0x54/0x9c [ 139.171802] [] irq_dispose_mapping+0x54/0x64 [ 139.171814] [] 0xffffffbffc0621e8 [ 139.171825] [] 0xffffffbffc05e01c [ 139.171843] [] __device_release_driver+0x90/0xe4 [ 139.171854] [] device_release_driver+0x20/0x38 [ 139.171867] [] bus_remove_device+0x12c/0x148 [ 139.171878] [] device_del+0x108/0x16c [ 139.171888] [] device_unregister+0x50/0x68 [ 139.171901] [] gb_gbphy_deregister_driver+0xf0/0x4ec [gb_gbphy] [ 139.171924] [] greybus_disabled+0x14c4/0x1760 [greybus] [ 139.171936] [] __device_release_driver+0x90/0xe4 [ 139.171948] [] device_release_driver+0x20/0x38 [ 139.171959] [] bus_remove_device+0x12c/0x148 [ 139.171969] [] device_del+0x108/0x16c [ 139.171992] [] gb_bundle_destroy+0x7c/0x1b0 [greybus] [ 139.172017] [] gb_interface_disable+0xb4/0x178 [greybus] [ 139.172040] [] gb_module_del+0x5c/0xf8 [greybus] [ 139.172063] [] gb_svc_intf_set_power_mode+0xea0/0xfe8 [greybus] [ 139.172078] [] process_one_work+0x268/0x3c8 [ 139.172089] [] worker_thread+0x204/0x358 [ 139.172108] [] 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 Signed-off-by: Viresh Kumar Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 8fa9998b4c3d..294e2f52f1d4 100644 --- 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); -- 2.39.5