From patchwork Sat Sep 1 11:22:40 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shiraz hashim X-Patchwork-Id: 1395121 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 13A81DFABE for ; Sat, 1 Sep 2012 11:26:06 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1T7lmr-0005VH-9S; Sat, 01 Sep 2012 11:22:49 +0000 Received: from mail-lpp01m010-f49.google.com ([209.85.215.49]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1T7lmm-0005V3-UO for linux-arm-kernel@lists.infradead.org; Sat, 01 Sep 2012 11:22:47 +0000 Received: by lagu2 with SMTP id u2so2644543lag.36 for ; Sat, 01 Sep 2012 04:22:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=/ed4xloHz0r2kmU7Z5ZnlvRs178EaMmNQ3lpqJCq3Ag=; b=YyB05av/lfUU97GgXwANR70hCh9mIKXqVQYwSvFNeC7Gj1F+zXfYcqjHeqRc/K8txH MTHtu7jeKPWELk5uhmaIYEmHG7kJKV/PJDi9/r7yY0hHlJ4oooArGVzbJojVUngFVpb9 ymYGnxoR0V8N8rN90ZpcYQ2SvuMsb6NdvcRyq+IqTKy5bFOTD+VxZogy4moe14yUyhjA LwM8k6EqNnj0GphBRFrZzHeBVTeNuSKtLDAu2ohQJ0fmtVAaIhFqpI+7d+IvCXaVFkwR NioCxooBxuBMr4rwlS16Qgx1op1rcvnCPpv6sqPlvBsUtMZF/SIR/gQjiceHbhbZvnVU Z5Tg== MIME-Version: 1.0 Received: by 10.112.99.37 with SMTP id en5mr3580009lbb.25.1346498560993; Sat, 01 Sep 2012 04:22:40 -0700 (PDT) Received: by 10.112.102.136 with HTTP; Sat, 1 Sep 2012 04:22:40 -0700 (PDT) In-Reply-To: References: <1336562136-24498-1-git-send-email-viresh.kumar@st.com> <201205122142.02318.arnd@arndb.de> <4FB47DE2.4070704@st.com> Date: Sat, 1 Sep 2012 16:52:40 +0530 Message-ID: Subject: Re: [GIT PULL] SPEAr pinctrl updates for v-3.5 From: shiraz hashim To: Linus Walleij , "grant.likely@secretlab.ca" X-Spam-Note: CRM114 invocation failed X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (shiraz.linux.kernel[at]gmail.com) -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.215.49 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: Arnd Bergmann , spear-devel , "arm@kernel.org" , viresh kumar , Dong Aisheng , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Hi Linus, Grant, On Wed, Jun 20, 2012 at 1:55 PM, Linus Walleij wrote: > On Tue, Jun 19, 2012 at 4:11 PM, viresh kumar wrote: > >> @Grant: Can we have your comments here please. Or any other better >> solution to the problem >> we are facing. I do hope, my last suggestion is not that bad. > > I did hunt Grant down in Hong Kong (this make me sound like some > secret agent...) and we discussed this with Dong Aisheng. > > The outcome was basically this work item from Linaro: > https://blueprints.launchpad.net/linux-linaro/+spec/pinctrl-gpiorange-makeover > > So the idea is that GPIO drivers should register their pin ranges > instead of the other way around (pinctrl drivers register them, as it > is done today), and that we need to move the mapping to be > GPIO-driver local, which in turn mandates stashing the > struct gpio_chip into the GPIO range mapping. > > Not a simple refactoring but probably necessary (all current users need > to be switched over). > > Interested? ;-) I tried to prepare a basic patch based upon this, can you please see if the approach is fine. 8<-------------------------------------------------------------------------- gpiolib: provide provision for gpios to register pin range pinctrl subsystem needs gpio chip base to prepare set of gpio pin ranges which a given pinctrl driver can handle. This is important to handle pinctrl gpio request calls in order to program a given pin properly for gpio operation. As gpio base is allocated dynamically while gpiochip registration, presently there exists no clean way to pass this information to the pinctrl subsystem. After few discussions from [1], it was concluded that may be gpio reporting the pin range it supports is a better way than pinctrl subsystem directly registering it. [1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/166668 Signed-off-by: Shiraz Hashim #else -------------------------------------------------------------------------->8 diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt index 4e16ba4..70d9e2e 100644 --- a/Documentation/devicetree/bindings/gpio/gpio.txt +++ b/Documentation/devicetree/bindings/gpio/gpio.txt @@ -75,4 +75,38 @@ Example of two SOC GPIO banks defined as gpio-controller nodes: gpio-controller; }; +2.1) gpio-controller and pinctrl subsystem +------------------------------------------ +gpio-controller on a SOC might be tightly coupled with the pinctrl +subsystem, in the sense that the pins can be used by other functions +together with optional gpio feature. + +While the pin allocation is totally managed by the pin ctrl subsystem, +gpio (under gpiolib) is still maintained by gpio drivers. It may happen +that different pin ranges in a SoC is managed by different gpio drivers. + +This makes it logical to let gpio drivers announce their pin ranges to +the pin ctrl subsystem and call 'pinctrl_request_gpio' in order to +request the corresponding pin before any gpio usage. + +For this, the gpio controller can use a pinctrl phandle and pins to +announce the pinrange to the pin ctrl subsystem. For example, + + qe_pio_e: gpio-controller@1460 { + #gpio-cells = <2>; + compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank"; + reg = <0x1460 0x18>; + gpio-controller; + pins = <&pinctrl1 20 10>, + <&pinctrl2 50 20>; + + } + +where, + &pinctrl1 and &pinctrl2 is the phandle to the pinctrl DT node. + + Next values specify the base pin and number of pins for the range + handled by 'qe_pio_e' gpio. In the given example from base pin 20 to + pin 29 under pinctrl1 and pin 50 to pin 69 under pinctrl2 is handled + by this gpio controller. diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index d18068a..cadbbff 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -19,6 +19,7 @@ #include #include #include +#include #include /* Private data structure for of_gpiochip_is_match */ @@ -213,6 +214,64 @@ err0: } EXPORT_SYMBOL(of_mm_gpiochip_add); +#ifdef CONFIG_PINCTRL +void of_gpiochip_add_pin_range(struct gpio_chip *chip) +{ + struct device_node *np = chip->of_node; + struct gpio_pin_range *pin_range; + struct of_phandle_args pinspec; + int index = 0; + + if (!chip->of_node) + return; + + INIT_LIST_HEAD(&chip->pin_ranges); + + do { + int ret; + + ret = of_parse_phandle_with_args(np, "gpio-ranges", "#gpio-range-cells", + index, &pinspec); + + if (ret) + break; + + pin_range = kzalloc(sizeof(*pin_range), GFP_KERNEL); + if (!pin_range) { + pr_err("%s: GPIO chip: failed to allocate pin ranges\n", + chip->label); + break; + } + + pin_range->range.name = chip->label; + pin_range->range.base = chip->base; + pin_range->range.pin_base = pinspec.args[0]; + pin_range->range.npins = pinspec.args[1]; + pin_range->pctldev = of_pinctrl_add_gpio_range(pinspec.np, + &pin_range->range); + + list_add_tail(&pin_range->node, &chip->pin_ranges); + + } while (index++); + +} + +void of_gpiochip_remove_pin_range(struct gpio_chip *chip) +{ + struct gpio_pin_range *pin_range, *tmp; + + list_for_each_entry_safe(pin_range, tmp, &chip->pin_ranges, node) { + pinctrl_remove_gpio_range(pin_range->pctldev, + &pin_range->range); + list_del(&pin_range->node); + kfree(pin_range); + } +} +#else +void of_gpiochip_add_pin_range(struct gpio_chip *chip) {} +void of_gpiochip_remove_pin_range(struct gpio_chip *chip) {} +#endif + void of_gpiochip_add(struct gpio_chip *chip) { if ((!chip->of_node) && (chip->dev)) @@ -226,11 +285,14 @@ void of_gpiochip_add(struct gpio_chip *chip) chip->of_xlate = of_gpio_simple_xlate; } + of_gpiochip_add_pin_range(chip); of_node_get(chip->of_node); } void of_gpiochip_remove(struct gpio_chip *chip) { + of_gpiochip_remove_pin_range(chip); + if (chip->of_node) of_node_put(chip->of_node); } diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c index fcb1de4..6728ec7 100644 --- a/drivers/pinctrl/devicetree.c +++ b/drivers/pinctrl/devicetree.c @@ -106,6 +106,19 @@ static struct pinctrl_dev *find_pinctrl_by_of_node(struct device_node *np) return NULL; } +struct pinctrl_dev *of_pinctrl_add_gpio_range(struct device_node *np, + struct pinctrl_gpio_range *range) +{ + struct pinctrl_dev *pctldev; + + pctldev = find_pinctrl_by_of_node(np); + if (!pctldev) + return NULL; + + pinctrl_add_gpio_range(pctldev, range); + return pctldev; +} + static int dt_to_map_one_config(struct pinctrl *p, const char *statename, struct device_node *np_config) { diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 365ea09..f7e0648 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -5,6 +5,7 @@ #include #include #include +#include #ifdef CONFIG_GPIOLIB @@ -47,6 +48,21 @@ struct seq_file; struct module; struct device_node; +#ifdef CONFIG_PINCTRL +/** + * struct gpio_pin_range - pin range controlled by a gpio chip + * @head: list for maintaining set of pin ranges, used internally + * @pctldev: pinctrl device which handles corresponding pins + * @range: actual range of pins controlled by a gpio controller + */ + +struct gpio_pin_range { + struct list_head node; + struct pinctrl_dev *pctldev; + struct pinctrl_gpio_range range; +}; +#endif + /** * struct gpio_chip - abstract a GPIO controller * @label: for diagnostics @@ -132,6 +148,14 @@ struct gpio_chip { int (*of_xlate)(struct gpio_chip *gc, const struct of_phandle_args *gpiospec, u32 *flags); #endif +#ifdef CONFIG_PINCTRL + /* If CONFIG_PINCTRL is enabled, then gpio controllers can + * optionally describe the actual pin range which they serve in + * an SoC. This information would be used by pinctrl subsystem + * to configure corresponding pins for gpio usage. + */ + struct list_head pin_ranges; +#endif }; extern const char *gpiochip_is_requested(struct gpio_chip *chip, diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h index 3b894a6..4222b83 100644 --- a/include/linux/pinctrl/pinctrl.h +++ b/include/linux/pinctrl/pinctrl.h @@ -133,6 +133,12 @@ extern void pinctrl_add_gpio_range(struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range); extern void pinctrl_remove_gpio_range(struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range); +#ifdef CONFIG_OF +extern struct pinctrl_dev *of_pinctrl_add_gpio_range(struct device_node *np, + struct pinctrl_gpio_range *range); + +#endif + extern const char *pinctrl_dev_get_name(struct pinctrl_dev *pctldev); extern void *pinctrl_dev_get_drvdata(struct pinctrl_dev *pctldev);