summaryrefslogtreecommitdiffstats
path: root/drivers/pinctrl/pinmux.c
diff options
context:
space:
mode:
authorStephen Warren <swarren@nvidia.com>2012-03-02 13:05:44 -0700
committerLinus Walleij <linus.walleij@linaro.org>2012-03-05 11:19:49 +0100
commit57b676f9c1b7cd84397fe5a86c9bd2788ac4bd32 (patch)
treee07d87bba28678aa80d9325a9c48eac0f57a7fe2 /drivers/pinctrl/pinmux.c
parent962bcbc57aa244eeb1176fa2e9f65ac865cca68a (diff)
downloadlinux-57b676f9c1b7cd84397fe5a86c9bd2788ac4bd32.tar.gz
linux-57b676f9c1b7cd84397fe5a86c9bd2788ac4bd32.tar.bz2
linux-57b676f9c1b7cd84397fe5a86c9bd2788ac4bd32.zip
pinctrl: fix and simplify locking
There are many problems with the current pinctrl locking: struct pinctrl_dev's gpio_ranges_lock isn't effective; pinctrl_match_gpio_range() only holds this lock while searching for a gpio range, but the found range is return and manipulated after releading the lock. This could allow pinctrl_remove_gpio_range() for that range while it is in use, and the caller may very well delete the range after removing it, causing pinctrl code to touch the now-free range object. Solving this requires the introduction of a higher-level lock, at least a lock per pin controller, which both gpio range registration and pinctrl_get()/put() will acquire. There is missing locking on HW programming; pin controllers may pack the configuration for different pins/groups/config options/... into one register, and hence have to read-modify-write the register. This needs to be protected, but currently isn't. Related, a future change will add a "complete" op to the pin controller drivers, the idea being that each state's programming will be programmed into the pinctrl driver followed by the "complete" call, which may e.g. flush a register cache to HW. For this to work, it must not be possible to interleave the pinctrl driver calls for different devices. As above, solving this requires the introduction of a higher-level lock, at least a lock per pin controller, which will be held for the duration of any pinctrl_enable()/disable() call. However, each pinctrl mapping table entry may affect a different pin controller if necessary. Hence, with a per-pin-controller lock, almost any pinctrl API may need to acquire multiple locks, one per controller. To avoid deadlock, these would need to be acquired in the same order in all cases. This is extremely difficult to implement in the case of pinctrl_get(), which doesn't know which pin controllers to lock until it has parsed the entire mapping table, since it contains somewhat arbitrary data. The simplest solution here is to introduce a single lock that covers all pin controllers at once. This will be acquired by all pinctrl APIs. This then makes struct pinctrl's mutex irrelevant, since that single lock will always be held whenever this mutex is currently held. Signed-off-by: Stephen Warren <swarren@nvidia.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Diffstat (limited to 'drivers/pinctrl/pinmux.c')
-rw-r--r--drivers/pinctrl/pinmux.c21
1 files changed, 9 insertions, 12 deletions
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index f409f161ea1d..7342c26f4246 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -19,8 +19,6 @@
#include <linux/radix-tree.h>
#include <linux/err.h>
#include <linux/list.h>
-#include <linux/mutex.h>
-#include <linux/spinlock.h>
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/debugfs.h>
@@ -96,15 +94,12 @@ static int pin_request(struct pinctrl_dev *pctldev,
goto out;
}
- spin_lock(&desc->lock);
if (desc->owner && strcmp(desc->owner, owner)) {
- spin_unlock(&desc->lock);
dev_err(pctldev->dev,
"pin already requested\n");
goto out;
}
desc->owner = owner;
- spin_unlock(&desc->lock);
/* Let each pin increase references to this module */
if (!try_module_get(pctldev->owner)) {
@@ -131,11 +126,8 @@ static int pin_request(struct pinctrl_dev *pctldev,
dev_err(pctldev->dev, "->request on device %s failed for pin %d\n",
pctldev->desc->name, pin);
out_free_pin:
- if (status) {
- spin_lock(&desc->lock);
+ if (status)
desc->owner = NULL;
- spin_unlock(&desc->lock);
- }
out:
if (status)
dev_err(pctldev->dev, "pin-%d (%s) status %d\n",
@@ -178,10 +170,8 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
else if (ops->free)
ops->free(pctldev, pin);
- spin_lock(&desc->lock);
owner = desc->owner;
desc->owner = NULL;
- spin_unlock(&desc->lock);
module_put(pctldev->owner);
return owner;
@@ -580,6 +570,8 @@ static int pinmux_functions_show(struct seq_file *s, void *what)
const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
unsigned func_selector = 0;
+ mutex_lock(&pinctrl_mutex);
+
while (pmxops->list_functions(pctldev, func_selector) >= 0) {
const char *func = pmxops->get_function_name(pctldev,
func_selector);
@@ -600,9 +592,10 @@ static int pinmux_functions_show(struct seq_file *s, void *what)
seq_puts(s, "]\n");
func_selector++;
-
}
+ mutex_unlock(&pinctrl_mutex);
+
return 0;
}
@@ -614,6 +607,8 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
seq_puts(s, "Pinmux settings per pin\n");
seq_puts(s, "Format: pin (name): owner\n");
+ mutex_lock(&pinctrl_mutex);
+
/* The pin number can be retrived from the pin controller descriptor */
for (i = 0; i < pctldev->desc->npins; i++) {
struct pin_desc *desc;
@@ -635,6 +630,8 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
is_hog ? " (HOG)" : "");
}
+ mutex_unlock(&pinctrl_mutex);
+
return 0;
}