From patchwork Fri Sep 5 15:22:04 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Octavian Purdila X-Patchwork-Id: 386408 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 08CA11400B9 for ; Sat, 6 Sep 2014 01:22:36 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756095AbaIEPWe (ORCPT ); Fri, 5 Sep 2014 11:22:34 -0400 Received: from mga09.intel.com ([134.134.136.24]:64644 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754920AbaIEPWe (ORCPT ); Fri, 5 Sep 2014 11:22:34 -0400 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 05 Sep 2014 08:16:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,473,1406617200"; d="scan'208";a="568912997" Received: from opurdila-mobl1.rb.intel.com ([10.237.104.133]) by orsmga001.jf.intel.com with ESMTP; 05 Sep 2014 08:22:14 -0700 From: Octavian Purdila To: linus.walleij@linaro.org, gnurou@gmail.com Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, Octavian Purdila Subject: [PATCH] gpiolib: fix a few issues with gpiochip_remove Date: Fri, 5 Sep 2014 18:22:04 +0300 Message-Id: <1409930524-1529-1-git-send-email-octavian.purdila@intel.com> X-Mailer: git-send-email 1.9.1 Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org The current implementation of gpiochip_remove() does not check to see if the GPIO pins are busy before removing the associated irqchip and this is causing the following warning: WARNING: CPU: 3 PID: 553 at fs/proc/generic.c:521 remove_proc_entry+0x19f/0x1b0() remove_proc_entry: removing non-empty directory 'irq/24', leaking at least 'bmc150_accel_event' Call Trace: [] dump_stack+0x4e/0x7a [] warn_slowpath_common+0x7d/0xa0 [] warn_slowpath_fmt+0x4c/0x50 [] remove_proc_entry+0x19f/0x1b0 [] unregister_irq_proc+0xce/0xf0 [] free_desc+0x31/0x70 [] irq_free_descs+0x3c/0x80 [] irq_dispose_mapping+0x36/0x50 [] gpiochip_remove+0x5a/0x160 [] dln2_do_remove+0x18/0x80 [] dln2_gpio_remove+0x2a/0x30 [] platform_drv_remove+0x1d/0x40 ... and bug: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97 in_atomic(): 1, irqs_disabled(): 1, pid: 553, name: khubd Preemption disabled at:[] gpiochip_remove+0x22/0x160 Call Trace: [] dump_stack+0x4e/0x7a [] __might_sleep+0x10f/0x180 [] mutex_lock+0x20/0x3d [] free_desc+0x3d/0x70 [] irq_free_descs+0x3c/0x80 [] irq_dispose_mapping+0x36/0x50 [] gpiochip_remove+0x5a/0x160 [] dln2_do_remove+0x18/0x80 [] dln2_gpio_remove+0x2a/0x30 [] platform_drv_remove+0x1d/0x40 ... The current implementaion also does a partial cleanup if one of the pins is busy, which makes it impossible to retry the remove operation later. A retry operation is needed in the case of MFD devices that bundles a GPIO device and another device that is an indirect consumer of the GPIO device (typical an I2C bus). In this case, when the MFD device is removed, if an I2C device associated with the I2C bus of the MFD device is using a GPIO pin (as an interrupt source for example), and the remove routine for the GPIO device is called first, then the removal of the gpio chip will fail. However, we can later retry the gpio chip removal, as the I2C bus will eventually be removed which will cause the I2C device to release the GPIO pin. This patch modifies gpiochip_remove to be atomic (i.e. if it fails no partial cleanup is done) and it also moves gpiochip_irqchip_remove() out of the spinlock to avoid the bug above. Signed-off-by: Octavian Purdila --- drivers/gpio/gpiolib.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 15cc0bb..0f53bef 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -314,14 +314,8 @@ int gpiochip_remove(struct gpio_chip *chip) int status = 0; unsigned id; - acpi_gpiochip_remove(chip); - spin_lock_irqsave(&gpio_lock, flags); - gpiochip_irqchip_remove(chip); - gpiochip_remove_pin_ranges(chip); - of_gpiochip_remove(chip); - for (id = 0; id < chip->ngpio; id++) { if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) { status = -EBUSY; @@ -337,8 +331,13 @@ int gpiochip_remove(struct gpio_chip *chip) spin_unlock_irqrestore(&gpio_lock, flags); - if (status == 0) + if (status == 0) { + gpiochip_irqchip_remove(chip); + gpiochip_remove_pin_ranges(chip); + of_gpiochip_remove(chip); + acpi_gpiochip_remove(chip); gpiochip_unexport(chip); + } return status; }