From a0a8bcf4670c2c696e6e83742539a5e0dd7a62d6 Mon Sep 17 00:00:00 2001 From: Grygorii Strashko Date: Mon, 17 Aug 2015 15:35:23 +0300 Subject: gpiolib: irqchip: use different lockdep class for each gpio irqchip Since IRQ chip helpers were introduced drivers lose ability to register separate lockdep classes for each registered GPIO IRQ chip and the gpiolib now is using shared lockdep class for all GPIO IRQ chips (gpiochip_irq_lock_class). As result, lockdep will produce warning when there are min two stacked GPIO chips and all of them are interrupt controllers. HW configuration which generates lockdep warning (TI dra7-evm): [SOC GPIO bankA.gpioX] <- irq - [pcf875x.gpioY] <- irq - DevZ.enable_irq_wake(pcf_gpioY_irq); The issue was reported in [1] and discussed [2]. ============================================= [ INFO: possible recursive locking detected ] 4.2.0-rc6-00013-g5d050ed-dirty #55 Not tainted --------------------------------------------- sh/63 is trying to acquire lock: (class){......}, at: [] __irq_get_desc_lock+0x50/0x94 but task is already holding lock: (class){......}, at: [] __irq_get_desc_lock+0x50/0x94 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(class); lock(class); *** DEADLOCK *** May be due to missing lock nesting notation 7 locks held by sh/63: #0: (sb_writers#4){.+.+.+}, at: [] vfs_write+0x13c/0x164 #1: (&of->mutex){+.+.+.}, at: [] kernfs_fop_write+0x4c/0x1a0 #2: (s_active#36){.+.+.+}, at: [] kernfs_fop_write+0x54/0x1a0 #3: (pm_mutex){+.+.+.}, at: [] pm_suspend+0xec/0x4c4 #4: (&dev->mutex){......}, at: [] __device_suspend+0xd4/0x398 #5: (&gpio->lock){+.+.+.}, at: [] __irq_get_desc_lock+0x74/0x94 #6: (class){......}, at: [] __irq_get_desc_lock+0x50/0x94 stack backtrace: CPU: 0 PID: 63 Comm: sh Not tainted 4.2.0-rc6-00013-g5d050ed-dirty #55 Hardware name: Generic DRA74X (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x84/0x9c) [] (dump_stack) from [] (__lock_acquire+0x19c0/0x1e20) [] (__lock_acquire) from [] (lock_acquire+0xa8/0x128) [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x38/0x4c) [] (_raw_spin_lock_irqsave) from [] (__irq_get_desc_lock+0x50/0x94) [] (__irq_get_desc_lock) from [] (irq_set_irq_wake+0x20/0xfc) [] (irq_set_irq_wake) from [] (pcf857x_irq_set_wake+0x24/0x54) [] (pcf857x_irq_set_wake) from [] (irq_set_irq_wake+0x8c/0xfc) [] (irq_set_irq_wake) from [] (gpio_keys_suspend+0x70/0xd4) [] (gpio_keys_suspend) from [] (dpm_run_callback+0x50/0x124) [] (dpm_run_callback) from [] (__device_suspend+0x10c/0x398) [] (__device_suspend) from [] (dpm_suspend+0x134/0x2f4) [] (dpm_suspend) from [] (suspend_devices_and_enter+0xa8/0x728) [] (suspend_devices_and_enter) from [] (pm_suspend+0x32c/0x4c4) [] (pm_suspend) from [] (state_store+0x64/0xb8) [] (state_store) from [] (kernfs_fop_write+0xbc/0x1a0) [] (kernfs_fop_write) from [] (__vfs_write+0x20/0xd8) [] (__vfs_write) from [] (vfs_write+0x90/0x164) [] (vfs_write) from [] (SyS_write+0x44/0x9c) [] (SyS_write) from [] (ret_fast_syscall+0x0/0x54) Lets fix it by using separate lockdep class for each registered GPIO IRQ Chip. This is done by wrapping gpiochip_irqchip_add call into macros. The implementation of this patch inspired by solution done by Nicolas Boichat for regmap [3] [1] http://www.spinics.net/lists/linux-gpio/msg05844.html [2] http://www.spinics.net/lists/linux-gpio/msg06021.html [3] http://www.spinics.net/lists/arm-kernel/msg429834.html Cc: Geert Uytterhoeven Cc: Roger Quadros Reported-by: Roger Quadros Tested-by: Roger Quadros Signed-off-by: Grygorii Strashko Signed-off-by: Linus Walleij --- include/linux/gpio/driver.h | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) (limited to 'include/linux/gpio/driver.h') diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index c8393cd4d44f..0c7004dab437 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -6,6 +6,7 @@ #include #include #include +#include #include struct device; @@ -126,6 +127,7 @@ struct gpio_chip { irq_flow_handler_t irq_handler; unsigned int irq_default_type; int irq_parent; + struct lock_class_key *lock_key; #endif #if defined(CONFIG_OF_GPIO) @@ -171,11 +173,25 @@ void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip, int parent_irq, irq_flow_handler_t parent_handler); -int gpiochip_irqchip_add(struct gpio_chip *gpiochip, - struct irq_chip *irqchip, - unsigned int first_irq, - irq_flow_handler_t handler, - unsigned int type); +int _gpiochip_irqchip_add(struct gpio_chip *gpiochip, + struct irq_chip *irqchip, + unsigned int first_irq, + irq_flow_handler_t handler, + unsigned int type, + struct lock_class_key *lock_key); + +#ifdef CONFIG_LOCKDEP +#define gpiochip_irqchip_add(...) \ +( \ + ({ \ + static struct lock_class_key _key; \ + _gpiochip_irqchip_add(__VA_ARGS__, &_key); \ + }) \ +) +#else +#define gpiochip_irqchip_add(...) \ + _gpiochip_irqchip_add(__VA_ARGS__, NULL) +#endif #endif /* CONFIG_GPIOLIB_IRQCHIP */ -- cgit v1.2.3 From 41d6bb4c890c8db01248b1bdd512a18e7bd29ca3 Mon Sep 17 00:00:00 2001 From: Grygorii Strashko Date: Mon, 17 Aug 2015 15:35:24 +0300 Subject: gpiolib: add description for gpio irqchip fields in struct gpio_chip Add missed description for GPIO irqchip fields in struct gpio_chip. Signed-off-by: Grygorii Strashko Signed-off-by: Linus Walleij --- include/linux/gpio/driver.h | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'include/linux/gpio/driver.h') diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 0c7004dab437..1aed31c5ffba 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -65,6 +65,17 @@ struct seq_file; * registers. * @irq_not_threaded: flag must be set if @can_sleep is set but the * IRQs don't need to be threaded + * @irqchip: GPIO IRQ chip impl, provided by GPIO driver + * @irqdomain: Interrupt translation domain; responsible for mapping + * between GPIO hwirq number and linux irq number + * @irq_base: first linux IRQ number assigned to GPIO IRQ chip (deprecated) + * @irq_handler: the irq handler to use (often a predefined irq core function) + * for GPIO IRQs, provided by GPIO driver + * @irq_default_type: default IRQ triggering type applied during GPIO driver + * initialization, provided by GPIO driver + * @irq_parent: GPIO IRQ chip parent/bank linux irq number, + * provided by GPIO driver + * @lock_key: per GPIO IRQ chip lockdep class * * A gpio_chip can help platforms abstract various sources of GPIOs so * they can all be accessed through a common programing interface. -- cgit v1.2.3