From patchwork Fri Aug 16 23:54:15 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 19273 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ye0-f200.google.com (mail-ye0-f200.google.com [209.85.213.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id DB9C5246AB for ; Fri, 16 Aug 2013 23:54:31 +0000 (UTC) Received: by mail-ye0-f200.google.com with SMTP id r14sf1800860yen.7 for ; Fri, 16 Aug 2013 16:54:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-gm-message-state:delivered-to:from:to:cc:subject :date:message-id:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=4U8URBGm6LI2e9I8FdWUklvWgDfvUoaeU2I7wLQwcWs=; b=T68d8BkgMuCXgKTm1cuRS1cHfUs5hxMsXwYs2jx11ur11/GGSL0qC6+KAoz7Hd+7e1 F8vPS8vacUWjqewxtTAQUPzjVKvWIBrcM2aZd+klg5mOKJt80hge8lEIdUo3etyy4qXK fDMP6u0D4ImJW/6x5g4Wkj5LAoIxe6GzcNw21XRIbomKNoNwNRriXt6xUPpQCnt8uG3K DFf5qc7o+7WoWeGdZlnZIVNuUB1VyUd2HGDUMEi+OmcTG7v5p7D08CtLDjN1LC7PsmAw 09F2VA5ngeFx8X2HTsaUFuTVoc0PBm5qQIOCryrPd43HPCOqNRZ43yRIFo+ze9d+woxY D0Hw== X-Received: by 10.236.128.113 with SMTP id e77mr1496247yhi.6.1376697271170; Fri, 16 Aug 2013 16:54:31 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.27.195 with SMTP id v3ls1021322qeg.6.gmail; Fri, 16 Aug 2013 16:54:31 -0700 (PDT) X-Received: by 10.58.152.3 with SMTP id uu3mr3500216veb.16.1376697271014; Fri, 16 Aug 2013 16:54:31 -0700 (PDT) Received: from mail-ve0-f180.google.com (mail-ve0-f180.google.com [209.85.128.180]) by mx.google.com with ESMTPS id h3si43768vca.148.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 16 Aug 2013 16:54:30 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.128.180 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.180; Received: by mail-ve0-f180.google.com with SMTP id pb11so256915veb.39 for ; Fri, 16 Aug 2013 16:54:30 -0700 (PDT) X-Gm-Message-State: ALoCoQlONRdhto5cwRNq/IJ2NdiIrZahZomQpKCrWHwSQ7VUP7eDXo6QcZi30+KvR1tWqBKnQR4+ X-Received: by 10.52.227.6 with SMTP id rw6mr202902vdc.19.1376697270541; Fri, 16 Aug 2013 16:54:30 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp95227vcz; Fri, 16 Aug 2013 16:54:29 -0700 (PDT) X-Received: by 10.112.14.102 with SMTP id o6mr174424lbc.28.1376697268865; Fri, 16 Aug 2013 16:54:28 -0700 (PDT) Received: from mail-lb0-f175.google.com (mail-lb0-f175.google.com [209.85.217.175]) by mx.google.com with ESMTPS id nw5si61088lbb.41.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 16 Aug 2013 16:54:28 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.217.175 is neither permitted nor denied by best guess record for domain of linus.walleij@linaro.org) client-ip=209.85.217.175; Received: by mail-lb0-f175.google.com with SMTP id 13so1800807lba.34 for ; Fri, 16 Aug 2013 16:54:28 -0700 (PDT) X-Received: by 10.152.8.115 with SMTP id q19mr170714laa.16.1376697267824; Fri, 16 Aug 2013 16:54:27 -0700 (PDT) Received: from localhost.localdomain (c83-249-208-76.bredband.comhem.se. [83.249.208.76]) by mx.google.com with ESMTPSA id l10sm72210lbh.13.1969.12.31.16.00.00 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 16 Aug 2013 16:54:26 -0700 (PDT) From: Linus Walleij To: linux-gpio@vger.kernel.org Cc: Linus Walleij , devicetree@vger.kernel.org, Javier Martinez Canillas , Enric Balletbo i Serra , Grant Likely , Jean-Christophe PLAGNIOL-VILLARD , Santosh Shilimkar , Kevin Hilman , Balaji T K , Tony Lindgren , Jon Hunter , Lars Poeschel Subject: [PATCH 1/3] gpio: interrupt consistency check for OF GPIO IRQs Date: Sat, 17 Aug 2013 01:54:15 +0200 Message-Id: <1376697255-18806-1-git-send-email-linus.walleij@linaro.org> X-Mailer: git-send-email 1.8.1.4 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: linus.walleij@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.180 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Currently the kernel is ambigously treating GPIOs and interrupts from a GPIO controller: GPIOs and interrupts are treated as orthogonal. This unfortunately makes it unclear how to actually retrieve and request a GPIO line or interrupt from a GPIO controller in the device tree probe path. In the non-DT probe path it is clear that the GPIO number has to be passed to the consuming device, and if it is to be used as an interrupt, the consumer shall performa a gpio_to_irq() mapping and request the resulting IRQ number. In the DT boot path consumers may have been given one or more interrupts from the interrupt-controller side of the GPIO chip in an abstract way, such that the driver is not aware of the fact that a GPIO chip is backing this interrupt, and the GPIO side of the same line is never requested with gpio_request(). A typical case for this is ethernet chips which just take some interrupt line which may be a "hard" interrupt line (such as an external interrupt line from an SoC) or a cascaded interrupt connected to a GPIO line. This has the following undesired effects: - The GPIOlib subsystem is not aware that the line is in use and willingly lets some other consumer perform gpio_request() on it, leading to a complex resource conflict if it occurs. - The GPIO debugfs file claims this GPIO line is "free". - The line direction of the interrupt GPIO line is not explicitly set as input, even though it is obvious that such a line need to be set up in this way, often making the system depend on boot-on defaults for this kind of settings. To solve this dilemma, perform an interrupt consistency check when adding a GPIO chip: if the chip is both gpio-controller and interrupt-controller, walk all children of the device tree, check if these in turn reference the interrupt-controller, and if they do, loop over the interrupts used by that child and perform gpio_request() and gpio_direction_input() on these, making them unreachable from the GPIO side. The patch has been devised by Linus Walleij and Lars Poeschel. Cc: devicetree@vger.kernel.org Cc: Javier Martinez Canillas Cc: Enric Balletbo i Serra Cc: Grant Likely Cc: Jean-Christophe PLAGNIOL-VILLARD Cc: Santosh Shilimkar Cc: Kevin Hilman Cc: Balaji T K Cc: Tony Lindgren Cc: Jon Hunter Signed-off-by: Lars Poeschel Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib-of.c | 161 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 160 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 665f953..216e939 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -10,7 +10,6 @@ * the Free Software Foundation; either version 2 of the License, or * (at your option) any later version. */ - #include #include #include @@ -19,6 +18,7 @@ #include #include #include +#include #include #include @@ -127,6 +127,161 @@ int of_gpio_simple_xlate(struct gpio_chip *gc, EXPORT_SYMBOL(of_gpio_simple_xlate); /** + * of_gpio_scan_irq_lines() - internal function to recursively scan the device + * tree and request or free the GPIOs that are to be used as IRQ lines + * @node: node to start the scanning at + * @gcn: device node of the GPIO chip + * @gc: GPIO chip instantiated from same node + * @request: wheter the function should request(true) or free(false) the + * irq lines + * + * This is a internal function that calls itself to recursively scan the device + * tree. It scans for uses of the device_node gcn as an interrupt-controller. + * If it finds some, it requests the corresponding gpio lines that are to be + * used as irq lines and sets them as input. + * + * If the request parameter is 0 it frees the gpio lines. + * For more information see documentation of of_gpiochip_reserve_irq_lines + * function. + */ +static void of_gpio_scan_irq_lines(const struct device_node *const node, + struct device_node * const gcn, + const struct gpio_chip * const gc, + bool request) +{ + struct device_node *child; + struct device_node *irq_parent; + const __be32 *intspec; + u32 intlen; + u32 offset; + int ret; + int num_irq; + int i; + + if (node == NULL) + return; + + for_each_child_of_node(node, child) { + of_gpio_scan_irq_lines(child, gcn, gc, request); + /* Check if we have an IRQ parent, else continue */ + irq_parent = of_irq_find_parent(child); + if (!irq_parent) + continue; + + /* Is it so that this very GPIO chip is the parent? */ + if (irq_parent != gcn) { + of_node_put(irq_parent); + continue; + } + of_node_put(irq_parent); + + pr_debug("gpiochip OF: node %s references GPIO interrupt lines\n", + child->name); + + /* Get the interrupts property */ + intspec = of_get_property(child, "interrupts", &intlen); + if (intspec == NULL) + continue; + intlen /= sizeof(*intspec); + + num_irq = of_irq_count(gcn); + for (i = 0; i < num_irq; i++) { + /* + * The first cell is always the local IRQ number, and + * this corresponds to the GPIO line offset for a GPIO + * chip. + * + * FIXME: take interrupt-cells into account here. + */ + offset = of_read_number(intspec + i, 1); + pr_debug("gpiochip: OF node %s references GPIO %d (%d)\n", + child->name, gc->base + offset, offset); + + if (request) { + /* + * This child is making a reference to this + * chip through the interrupts property, so + * reserve these GPIO lines and set them as + * input. + */ + ret = gpio_request(gc->base + offset, child->name); + if (ret) + pr_err("gpiolib OF: could not request IRQ GPIO %d (%d) for node %s (%d)\n", + gc->base + offset, offset, child->name, ret); + ret = gpio_direction_input(gc->base + offset); + if (ret) + pr_err("gpiolib OF: could not set IRQ GPIO %d (%d) as input for node %s (%d)\n", + gc->base + offset, offset, child->name, ret); + } else { + gpio_free(gc->base + offset); + } + } + } +} + +/** + * of_gpiochip_reserve_irq_lines() - request or free GPIO IRQ lines + * @np: device node of the GPIO chip + * @gc: GPIO chip instantiated from same node + * @request: wheter the function should request(1) or free(0) the irq lines + * + * This function should not be used directly, use the macros + * of_gpiochip_request_irq_lines or of_gpiochip_free_gpio_lines instead. + * + * For the case of requesting the irq lines (request == 1) this function is + * called after instantiating a GPIO chip from a device tree node to assert + * that all interrupts derived from the chip are consistently requested as + * GPIO lines, if the GPIO chip is BOTH a gpio-controller AND an + * interrupt-controller. + * + * If another node in the device tree is referencing the interrupt-controller + * portions of the GPIO chip, such that it is using a GPIO line as some + * arbitrary interrupt source, the following holds: + * + * - That line must NOT be used anywhere else in the device tree as a + * <&gpio N> reference, or GPIO and interrupt usage may conflict. + * + * Conversely, if a node is using a line as a direct reference to a GPIO line, + * no node in the tree may use that line as an interrupt. + * + * If another node is referencing a GPIO line, and also want to use that line + * as an interrupt source, it is necessary for this driver to use the + * gpio_to_irq() kernel interface. + * + * For the case of freeing the irq lines (request == 0) this function simply + * uses the same device tree information used to request the irq lines to call + * gpiochip_free on that GPIOs. + */ +static void of_gpiochip_reserve_irq_lines(struct device_node *np, + struct gpio_chip *gc, bool request) +{ + struct device_node *root; + + /* + * If this chip is not tagged as interrupt-controller, there is + * no problem so we just exit. + */ + if (!of_property_read_bool(np, "interrupt-controller")) + return; + + /* + * Proceed to check the consistency of all references to this + * GPIO chip. + */ + root = of_find_node_by_path("/"); + if (!root) + return; + of_gpio_scan_irq_lines(root, np, gc, request); + of_node_put(root); +} + +#define of_gpiochip_request_irq_lines(np, gc) \ + of_gpiochip_reserve_irq_lines(np, gc, true) + +#define of_gpiochip_free_irq_lines(np, gc) \ + of_gpiochip_reserve_irq_lines(np, gc, false) + +/** * of_mm_gpiochip_add - Add memory mapped GPIO chip (bank) * @np: device node of the GPIO chip * @mm_gc: pointer to the of_mm_gpio_chip allocated structure @@ -170,6 +325,8 @@ int of_mm_gpiochip_add(struct device_node *np, if (ret) goto err2; + of_gpiochip_request_irq_lines(np, gc); + return 0; err2: iounmap(mm_gc->regs); @@ -231,12 +388,14 @@ void of_gpiochip_add(struct gpio_chip *chip) chip->of_xlate = of_gpio_simple_xlate; } + of_gpiochip_request_irq_lines(chip->of_node, chip); of_gpiochip_add_pin_range(chip); of_node_get(chip->of_node); } void of_gpiochip_remove(struct gpio_chip *chip) { + of_gpiochip_free_irq_lines(chip->of_node, chip); gpiochip_remove_pin_ranges(chip); if (chip->of_node)