From patchwork Wed Dec 12 20:25:07 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 13517 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 7FF7C23FC0 for ; Wed, 12 Dec 2012 20:25:51 +0000 (UTC) Received: from mail-ie0-f178.google.com (mail-ie0-f178.google.com [209.85.223.178]) by fiordland.canonical.com (Postfix) with ESMTP id 01E1EA18673 for ; Wed, 12 Dec 2012 20:25:50 +0000 (UTC) Received: by mail-ie0-f178.google.com with SMTP id c12so2646140ieb.37 for ; Wed, 12 Dec 2012 12:25:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:from:to:cc :subject:date:message-id:x-mailer:mime-version:content-type :x-gm-message-state; bh=tfjX0EguUQSuyBghyPWigN2HG9H/uPXaHHnpmo+3VWM=; b=NdXuIePPfsyLWc+tG6KS8YuT5Le9FbTNpkIesZtMizhdRFcnSNOHdOZOL0gVYhgo6m xpcD0Q8g06S4pHZ8DseJQ11e89Isi6+/NgKMvip1MT7qp8tbGJ7yOyGg6speYmz0WJlv FrxGy6dWcdsLqvGm6C//lKuH8g+a9Q3fTi7UXgWlw+TMvM3ZgOAVPZAoVapwo5b/QroA 9kESWgEktGs5y1OoJj+m0CEz7TnOUNuBQ2fyyLP7nleadda8xXDj3E2236w0qXMmDqpQ vZ34P2SO+yWQ6U1bRaxT2Jro8qH2GKd2yr7IIKOWklsFjn0fHTMgMcMtTFU06aGepWLO LTjA== Received: by 10.50.185.166 with SMTP id fd6mr14374453igc.62.1355343950216; Wed, 12 Dec 2012 12:25:50 -0800 (PST) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.50.67.148 with SMTP id n20csp223317igt; Wed, 12 Dec 2012 12:25:49 -0800 (PST) Received: by 10.14.215.194 with SMTP id e42mr5753721eep.32.1355343948295; Wed, 12 Dec 2012 12:25:48 -0800 (PST) Received: from eu1sys200aog105.obsmtp.com (eu1sys200aog105.obsmtp.com [207.126.144.119]) by mx.google.com with SMTP id l8si49968879eem.17.2012.12.12.12.25.38 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 12 Dec 2012 12:25:48 -0800 (PST) Received-SPF: neutral (google.com: 207.126.144.119 is neither permitted nor denied by best guess record for domain of linus.walleij@stericsson.com) client-ip=207.126.144.119; Authentication-Results: mx.google.com; spf=neutral (google.com: 207.126.144.119 is neither permitted nor denied by best guess record for domain of linus.walleij@stericsson.com) smtp.mail=linus.walleij@stericsson.com Received: from beta.dmz-eu.st.com ([164.129.1.35]) (using TLSv1) by eu1sys200aob105.postini.com ([207.126.147.11]) with SMTP ID DSNKUMjoPEV0H93Nmd9dxmWPXlkBfNlpT6ch@postini.com; Wed, 12 Dec 2012 20:25:47 UTC Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 94CC6E6; Wed, 12 Dec 2012 20:25:14 +0000 (GMT) Received: from relay2.stm.gmessaging.net (unknown [10.230.100.18]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id C71505744; Wed, 12 Dec 2012 20:25:13 +0000 (GMT) Received: from exdcvycastm022.EQ1STM.local (alteon-source-exch [10.230.100.61]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (Client CN "exdcvycastm022", Issuer "exdcvycastm022" (not verified)) by relay2.stm.gmessaging.net (Postfix) with ESMTPS id C3D55A8072; Wed, 12 Dec 2012 21:25:05 +0100 (CET) Received: from steludxu4075.lud.stericsson.com (10.230.100.153) by smtp.stericsson.com (10.230.100.30) with Microsoft SMTP Server (TLS) id 8.3.83.0; Wed, 12 Dec 2012 21:25:12 +0100 From: Linus Walleij To: , , Greg Kroah-Hartman Cc: Linus Walleij , Felipe Balbi , Benoit Cousson , Dmitry Torokhov , Thomas Petazzoni , Mitch Bradley , Mark Brown , Ulf Hansson , "Rafael J. Wysocki" , Kevin Hilman , Jean-Christophe PLAGNIOL-VILLARD , Rickard Andersson , Russell King Subject: [PATCH v2] drivers/pinctrl: grab default handles from device core Date: Wed, 12 Dec 2012 21:25:07 +0100 Message-ID: <1355343907-11535-1-git-send-email-linus.walleij@stericsson.com> X-Mailer: git-send-email 1.7.11.3 MIME-Version: 1.0 X-Gm-Message-State: ALoCoQkF+gLZcuz7qvq2fFVwHgUelOcJSCwD28HCudiUjkgZxGOzT47lME307lg5OvdcqfF631ik From: Linus Walleij This makes the device core auto-grab the pinctrl handle and set the "default" (PINCTRL_STATE_DEFAULT) state for every device that is present in the device model right before probe. This will account for the lion's share of embedded silicon devcies. A modification of the semantics for pinctrl_get() is also done: previously if the pinctrl handle for a certain device was already taken, the pinctrl core would return an error. Now, since the core may have already default-grabbed the handle and set its state to "default", if the handle was already taken, this will be disregarded and the located, previously instanitated handle will be returned to the caller. This way all code in drivers explicitly requesting their pinctrl handlers will still be functional, and drivers that want to explicitly retrieve and switch their handles can still do that. But if the desired functionality is just boilerplate of this type in the probe() function: struct pinctrl *p; p = devm_pinctrl_get_select_default(&dev); if (IS_ERR(p)) { if (PTR_ERR(p) == -EPROBE_DEFER) return -EPROBE_DEFER; dev_warn(&dev, "no pinctrl handle\n"); } The discussion began with the addition of such boilerplate to the omap4 keypad driver: http://marc.info/?l=linux-input&m=135091157719300&w=2 A previous approach using notifiers was discussed: http://marc.info/?l=linux-kernel&m=135263661110528&w=2 This failed because it could not handle deferred probes. This patch alone does not solve the entire dilemma faced: whether code should be distributed into the drivers or if it should be centralized to e.g. a PM domain. But it solves the immediate issue of the addition of boilerplate to a lot of drivers that just want to grab the default state. As mentioned, they can later explicitly retrieve the handle and set different states, and this could as well be done by e.g. PM domains as it is only related to a certain struct device * pointer. Cc: Felipe Balbi Cc: Benoit Cousson Cc: Dmitry Torokhov Cc: Thomas Petazzoni Cc: Mitch Bradley Cc: Mark Brown Cc: Ulf Hansson Cc: Rafael J. Wysocki Cc: Kevin Hilman Cc: Jean-Christophe PLAGNIOL-VILLARD Cc: Rickard Andersson Cc: Greg Kroah-Hartman Cc: Russell King Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Only store a pointer in the device struct, and only allocate this if it's really used by the device. - Seeking ACK on this from Greg (and others who like it) so I can merge it through the pinctrl subsystem. --- Documentation/pinctrl.txt | 24 ++++++++++-- drivers/base/Makefile | 1 + drivers/base/dd.c | 7 ++++ drivers/base/pinctrl.c | 81 +++++++++++++++++++++++++++++++++++++++++ drivers/pinctrl/core.c | 11 +++++- include/linux/device.h | 7 ++++ include/linux/pinctrl/devinfo.h | 45 +++++++++++++++++++++++ 7 files changed, 171 insertions(+), 5 deletions(-) create mode 100644 drivers/base/pinctrl.c create mode 100644 include/linux/pinctrl/devinfo.h diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt index da40efb..68836e5 100644 --- a/Documentation/pinctrl.txt +++ b/Documentation/pinctrl.txt @@ -972,6 +972,18 @@ pinmux core. Pin control requests from drivers ================================= +When a device driver is about to probe the device core will automatically +attempt to issue pinctrl_get_select_default() on these devices. +This way driver writers do not need to add any of the boilerplate code +of the type found below. However when doing fine-grained state selection +and not using the "default" state, you may have to do some device driver +handling of the pinctrl handles and states. + +So if you just want to put the pins for a certain device into the default +state and be done with it, there is nothing you need to do besides +providing the proper mapping table. The device core will take care of +the rest. + Generally it is discouraged to let individual drivers get and enable pin control. So if possible, handle the pin control in platform code or some other place where you have access to all the affected struct device * pointers. In @@ -1097,9 +1109,9 @@ situations that can be electrically unpleasant, you will certainly want to mux in and bias pins in a certain way before the GPIO subsystems starts to deal with them. -The above can be hidden: using pinctrl hogs, the pin control driver may be -setting up the config and muxing for the pins when it is probing, -nevertheless orthogonal to the GPIO subsystem. +The above can be hidden: using the device core, the pinctrl core may be +setting up the config and muxing for the pins right before the device is +probing, nevertheless orthogonal to the GPIO subsystem. But there are also situations where it makes sense for the GPIO subsystem to communicate directly with with the pinctrl subsystem, using the latter @@ -1144,6 +1156,12 @@ PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-foo", NULL /* group */, "power_func") This gives the exact same result as the above construction. +This should not be used for any kind of device which is represented in +the device model, as the pinctrl core will attempt to do the equal of +pinctrl_get_select_default() for these devices right before their device +drivers are probed, so hogging these will just make the model look +strange. Instead put in proper map entries. + Runtime pinmuxing ================= diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 5aa2d70..4e22ce3 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -21,6 +21,7 @@ endif obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o obj-$(CONFIG_REGMAP) += regmap/ obj-$(CONFIG_SOC_BUS) += soc.o +obj-$(CONFIG_PINCTRL) += pinctrl.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG diff --git a/drivers/base/dd.c b/drivers/base/dd.c index e3bbed8..65631015 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "base.h" #include "power/power.h" @@ -269,6 +270,12 @@ static int really_probe(struct device *dev, struct device_driver *drv) WARN_ON(!list_empty(&dev->devres_head)); dev->driver = drv; + + /* If using pinctrl, bind pins now before probing */ + ret = pinctrl_bind_pins(dev); + if (ret) + goto probe_failed; + if (driver_sysfs_add(dev)) { printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n", __func__, dev_name(dev)); diff --git a/drivers/base/pinctrl.c b/drivers/base/pinctrl.c new file mode 100644 index 0000000..edbf8e1 --- /dev/null +++ b/drivers/base/pinctrl.c @@ -0,0 +1,81 @@ +/* + * Driver core interface to the pinctrl subsystem. + * + * Copyright (C) 2012 ST-Ericsson SA + * Written on behalf of Linaro for ST-Ericsson + * Based on bits of regulator core, gpio core and clk core + * + * Author: Linus Walleij + * + * License terms: GNU General Public License (GPL) version 2 + */ + +#include +#include +#include +#include + +/** + * pinctrl_bind_pins() - called by the device core before probe + * @dev: the device that is just about to probe + */ +int pinctrl_bind_pins(struct device *dev) +{ + struct dev_pin_info *dpi; + int ret; + + /* Allocate a pin state container on-the-fly */ + if (!dev->pins) { + dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL); + if (!dpi) + return -ENOMEM; + } else + dpi = dev->pins; + + /* + * Check if we already have a pinctrl handle, as we may arrive here + * after a deferral in the state selection below + */ + if (!dpi->p) { + dpi->p = devm_pinctrl_get(dev); + if (IS_ERR_OR_NULL(dpi->p)) { + int ret = PTR_ERR(dpi->p); + + dev_dbg(dev, "no pinctrl handle\n"); + /* Only return deferrals */ + if (ret == -EPROBE_DEFER) + return ret; + return 0; + } + } + + /* + * For a newly allocated info struct, here is where we keep it, + * since at this point it actually contains something. + */ + dev->pins = dpi; + + /* + * We may have looked up the state earlier as well. + */ + if (!dpi->default_state) { + dpi->default_state = pinctrl_lookup_state(dpi->p, + PINCTRL_STATE_DEFAULT); + if (IS_ERR(dpi->default_state)) { + dev_dbg(dev, "no default pinctrl state\n"); + return 0; + } + } + + ret = pinctrl_select_state(dpi->p, dpi->default_state); + if (ret) { + dev_dbg(dev, "failed to activate default pinctrl state\n"); + + /* Only return deferrals */ + if (ret == -EPROBE_DEFER) + return ret; + return 0; + } + + return 0; +} diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index 5b4885c..6947bf0 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -734,9 +734,16 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev) if (WARN_ON(!dev)) return ERR_PTR(-EINVAL); + /* + * See if somebody else (such as the device core) has already + * obtained a handle to the pinctrl for this device. In that case, + * return another pointer to it. + */ p = find_pinctrl(dev); - if (p != NULL) - return ERR_PTR(-EBUSY); + if (p != NULL) { + dev_dbg(dev, "obtain a copy of previously claimed pinctrl\n"); + return p; + } return create_pinctrl(dev); } diff --git a/include/linux/device.h b/include/linux/device.h index 86ef6ab..86d11f7 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -605,6 +606,8 @@ struct device_dma_parameters { * @pm_domain: Provide callbacks that are executed during system suspend, * hibernation, system resume and during runtime PM transitions * along with subsystem-level and driver-level callbacks. + * @pins: For device pin management. + * See Documentation/pinctrl.txt for details. * @numa_node: NUMA node this device is close to. * @dma_mask: Dma mask (if dma'ble device). * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all @@ -656,6 +659,10 @@ struct device { struct dev_pm_info power; struct dev_pm_domain *pm_domain; +#ifdef CONFIG_PINCTRL + struct dev_pin_info *pins; +#endif + #ifdef CONFIG_NUMA int numa_node; /* NUMA node this device is close to */ #endif diff --git a/include/linux/pinctrl/devinfo.h b/include/linux/pinctrl/devinfo.h new file mode 100644 index 0000000..6e5f8a9 --- /dev/null +++ b/include/linux/pinctrl/devinfo.h @@ -0,0 +1,45 @@ +/* + * Per-device information from the pin control system. + * This is the stuff that get included into the device + * core. + * + * Copyright (C) 2012 ST-Ericsson SA + * Written on behalf of Linaro for ST-Ericsson + * This interface is used in the core to keep track of pins. + * + * Author: Linus Walleij + * + * License terms: GNU General Public License (GPL) version 2 + */ + +#ifndef PINCTRL_DEVINFO_H +#define PINCTRL_DEVINFO_H + +#ifdef CONFIG_PINCTRL + +/* The device core acts as a consumer toward pinctrl */ +#include + +/** + * struct dev_pin_info - pin state container for devices + * @p: pinctrl handle for the containing device + * @default_state: the default state for the handle, if found + */ +struct dev_pin_info { + struct pinctrl *p; + struct pinctrl_state *default_state; +}; + +extern int pinctrl_bind_pins(struct device *dev); + +#else + +/* Stubs if we're not using pinctrl */ + +static inline int pinctrl_bind_pins(struct device *dev) +{ + return 0; +} + +#endif /* CONFIG_PINCTRL */ +#endif /* PINCTRL_DEVINFO_H */