From patchwork Sun Nov 11 12:22:19 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 12803 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 7B39E23FC2 for ; Sun, 11 Nov 2012 12:23:11 +0000 (UTC) Received: from mail-ie0-f180.google.com (mail-ie0-f180.google.com [209.85.223.180]) by fiordland.canonical.com (Postfix) with ESMTP id 0480CA196BC for ; Sun, 11 Nov 2012 12:23:10 +0000 (UTC) Received: by mail-ie0-f180.google.com with SMTP id e10so7489025iej.11 for ; Sun, 11 Nov 2012 04:23:10 -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=EEOtNNpOW/eKZHlYR4j+J1YNQaFN2bh2mGs71YnbZH4=; b=aYxmXsrIoKd8Zx/rrI+Jd9SDMMBl60vUfnX5zAL1Nixz2dYoOY0PQVNvqDS7kl2SkE RBAiD1GiAR0A7FdFggspSCfe53IDHXR8xI0IAe+4OnP2+J0iSIeCL10zjV6DExJNia1D EgqNDRh4P8xrZEoT/+PGuGHv9jbk8pPmAljZHi6rS8D1WYxjfrhuOlQXVXgnp9HNs2O7 MX7YbOS+g6BkaZ1zKZsv5keKjCcQ69t2bFY6AlafP/Ba20ftruekA5XQWvE4LeSQcub6 GSg5MVA836g+MaNjFpUywyTTWysxD7bT9Yr5pDeoiue4PGA4DxupMZeJ1DSv/BvDYt0S DPlQ== Received: by 10.50.161.169 with SMTP id xt9mr5212434igb.62.1352636590354; Sun, 11 Nov 2012 04:23:10 -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 n20csp243178igt; Sun, 11 Nov 2012 04:23:09 -0800 (PST) Received: by 10.14.0.68 with SMTP id 44mr53687391eea.1.1352636589169; Sun, 11 Nov 2012 04:23:09 -0800 (PST) Received: from eu1sys200aog107.obsmtp.com (eu1sys200aog107.obsmtp.com [207.126.144.123]) by mx.google.com with SMTP id k8si7058830eed.0.2012.11.11.04.22.50 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 11 Nov 2012 04:23:08 -0800 (PST) Received-SPF: neutral (google.com: 207.126.144.123 is neither permitted nor denied by best guess record for domain of linus.walleij@stericsson.com) client-ip=207.126.144.123; Authentication-Results: mx.google.com; spf=neutral (google.com: 207.126.144.123 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-ap.st.com ([138.198.100.35]) (using TLSv1) by eu1sys200aob107.postini.com ([207.126.147.11]) with SMTP ID DSNKUJ+YkEl/KCCDJs3K8Kblpf5jDmp6KS7I@postini.com; Sun, 11 Nov 2012 12:23:08 UTC Received: from zeta.dmz-ap.st.com (ns6.st.com [138.198.234.13]) by beta.dmz-ap.st.com (STMicroelectronics) with ESMTP id B7EC8C2; Sun, 11 Nov 2012 12:14:17 +0000 (GMT) Received: from relay2.stm.gmessaging.net (unknown [10.230.100.18]) by zeta.dmz-ap.st.com (STMicroelectronics) with ESMTP id A2FE185A; Sun, 11 Nov 2012 12:22:28 +0000 (GMT) Received: from exdcvycastm004.EQ1STM.local (alteon-source-exch [10.230.100.61]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (Client CN "exdcvycastm004", Issuer "exdcvycastm004" (not verified)) by relay2.stm.gmessaging.net (Postfix) with ESMTPS id 23445A8083; Sun, 11 Nov 2012 13:22:23 +0100 (CET) Received: from steludxu4075.lud.stericsson.com (10.230.100.153) by smtp.stericsson.com (10.230.100.2) with Microsoft SMTP Server (TLS) id 8.3.83.0; Sun, 11 Nov 2012 13:22:27 +0100 From: Linus Walleij To: , Cc: Stephen Warren , Anmar Oueja , 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 , Greg Kroah-Hartman , Russell King Subject: [PATCH] RFC: pinctrl: grab default handler with bus notifiers Date: Sun, 11 Nov 2012 13:22:19 +0100 Message-ID: <1352636539-6318-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: ALoCoQmwsnqWvcNAPJIbp3TmPPhtCKrkIJSIk58C7EmQRQ9z9qe4Q61AVgu/2rDFdSGEP49HI6T3 From: Linus Walleij This makes the pinctrl subsystem auto-grab the pinctrl handle and set the "default" (PINCTRL_STATE_DEFAULT) state for every device that is present on the platform or AMBA (PrimeCell) bus right before probe. This will account for the lion's share of embedded silicon devcies. The behaviour is achieved using bus notifiers on the platform and AMBA (PrimeCell) busses. 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 Notice that the patch disregards deferral as per above: if the pin controller returns -EPROBE_DEFER, there is no way to send that back to the device core using notifiers, so in cases where the probe ordering is to be controlled, drivers still need to add the above pin control handling code. So this mechanism will basically be optimistic, and to be really sure that you don't have a deferred probe on your pins, you still need to explicitly request the pins. 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. Another solution that was discussed was whether to move the default pinctrl handle and state grab to the device core as an optional field in struct device itself, but I'd like to first propose this less intrusive mechanism. 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 --- Documentation/pinctrl.txt | 24 +++++++++++++++++-- drivers/pinctrl/core.c | 60 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt index da40efb..b6c27b1 100644 --- a/Documentation/pinctrl.txt +++ b/Documentation/pinctrl.txt @@ -972,6 +972,19 @@ pinmux core. Pin control requests from drivers ================================= +When a device driver is about to probe on the platform or AMBA (PrimeCell) +bus, the pinctrl core will automatically attempt to issue +pinctrl_get_select_default() on these devices using the bus notification +mechanism. 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 platform or AMBA 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,8 +1110,8 @@ 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, +The above can be hidden: using device notifiers, the pinctrl core may be +setting up the config and muxing for the pins when the device is probing, nevertheless orthogonal to the GPIO subsystem. But there are also situations where it makes sense for the GPIO subsystem @@ -1144,6 +1157,13 @@ 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. For platform and AMBA (PrimeCell) devices, such as found +in many SoC:s, the pinctrl core will listen to notifications from these +buses and attempt to do pinctrl_get_select_default() for these devices +right before their device drivers are probed, so hogging these will just +make the model look strange. + Runtime pinmuxing ================= diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index 71db586..bdb2300 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -26,6 +26,9 @@ #include #include #include +#include +#include +#include #include "core.h" #include "devicetree.h" #include "pinmux.h" @@ -684,9 +687,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 pinctrl core, using the + * notifiers) 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); } @@ -1026,6 +1036,52 @@ void pinctrl_unregister_map(struct pinctrl_map const *map) } } +static int pinctrl_device_notify(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct device *dev = data; + struct pinctrl *p; + + switch (action) { + case BUS_NOTIFY_BIND_DRIVER: + /* + * Hog pins right before a driver is about to be bound to + * the device + */ + p = devm_pinctrl_get_select_default(dev); + if (IS_ERR_OR_NULL(p)) { + dev_dbg(dev, "no pinctrl handle or default state\n"); + return 0; + } + dev_dbg(dev, "pinctrl core obtained default pinctrl\n"); + break; + case BUS_NOTIFY_UNBOUND_DRIVER: + break; + } + + return 0; +} + +static struct notifier_block pinctrl_device_nb = { + .notifier_call = pinctrl_device_notify, +}; + +static int __init pinctrl_notifiers_init(void) +{ + bus_register_notifier(&platform_bus_type, &pinctrl_device_nb); +#ifdef CONFIG_ARM_AMBA + bus_register_notifier(&amba_bustype, &pinctrl_device_nb); +#endif + return 0; +} + +/* + * The AMBA bus registers in the postcore_initcall() so we need to + * register our notifiers right after that so we can fetch pins for + * e.g. TTY drivers that will be registered in the arch_initcall() next. + */ +postcore_initcall_sync(pinctrl_notifiers_init); + #ifdef CONFIG_DEBUG_FS static int pinctrl_pins_show(struct seq_file *s, void *what)