From 42fed7ba44e4e8c1fb27b28ad14490cb1daff3c7 Mon Sep 17 00:00:00 2001 From: Patrice Chotard Date: Thu, 11 Apr 2013 11:01:27 +0200 Subject: pinctrl: move subsystem mutex to pinctrl_dev struct This mutex avoids deadlock in case of use of multiple pin controllers. Before this modification, by using a global mutex, deadlock appeared when, for example, a call to pinctrl_pins_show() locked the pinctrl_mutex, called the ops->pin_dbg_show of a particular pin controller. If this pin controller needs I2C access to retrieve configuration information and I2C driver is using pinctrl to drive its pins, a call to pinctrl_select_state() try to lock again pinctrl_mutex which leads to a deadlock. Notice that the mutex grab from the two direction functions was moved into pinctrl_gpio_direction(). For several cases, we can't replace pinctrl_mutex by pctldev->mutex, because at this stage, pctldev is not accessible : - pinctrl_get()/pinctrl_put() - pinctrl_register_maps() So add respectively pinctrl_list_mutex and pinctrl_maps_mutex in order to protect pinctrl_list and pinctrl_maps list instead. Reintroduce pinctrldev_list_mutex in find_pinctrl_by_of_node(), pinctrl_find_and_add_gpio_range() pinctrl_request_gpio(), pinctrl_free_gpio(), pinctrl_gpio_direction(), pinctrl_devices_show(), pinctrl_register() and pinctrl_unregister() to protect pinctrldev_list. Changes v2->v3: - Fix a missing EXPORT_SYMBOL_GPL() for pinctrl_select_state(). Changes v1->v2: - pinctrl_select_state_locked() is removed, all lock mechanism is located inside pinctrl_select_state(). When parsing the state->setting list, take the per-pin-controller driver lock. (Patrice). - Introduce pinctrldev_list_mutex to protect pinctrldev_list in all functions which parse or modify pictrldev_list. (Patrice). - move find_pinctrl_by_of_node() from pinctrl/devicetree.c to pinctrl/core.c in order to protect pinctrldev_list. (Patrice). - Sink mutex:es into some functions and remove some _locked variants down to where the lists are actually accessed to make things simpler. (Linus) - Drop *all* mutexes completely from pinctrl_lookup_state() and pinctrl_select_state() - no relevant mutex was taken and it was unclear what this was protecting against. (Linus) Reported by : Seraphin Bonnaffe Signed-off-by: Patrice Chotard Signed-off-by: Linus Walleij --- drivers/pinctrl/pinconf.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) (limited to 'drivers/pinctrl/pinconf.c') diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c index 32f96808202a..c67c37e23dd7 100644 --- a/drivers/pinctrl/pinconf.c +++ b/drivers/pinctrl/pinconf.c @@ -89,14 +89,14 @@ int pin_config_get(const char *dev_name, const char *name, struct pinctrl_dev *pctldev; int pin; - mutex_lock(&pinctrl_mutex); - pctldev = get_pinctrl_dev_from_devname(dev_name); if (!pctldev) { pin = -EINVAL; - goto unlock; + return pin; } + mutex_lock(&pctldev->mutex); + pin = pin_get_from_name(pctldev, name); if (pin < 0) goto unlock; @@ -104,7 +104,7 @@ int pin_config_get(const char *dev_name, const char *name, pin = pin_config_get_for_pin(pctldev, pin, config); unlock: - mutex_unlock(&pinctrl_mutex); + mutex_unlock(&pctldev->mutex); return pin; } EXPORT_SYMBOL(pin_config_get); @@ -145,14 +145,14 @@ int pin_config_set(const char *dev_name, const char *name, struct pinctrl_dev *pctldev; int pin, ret; - mutex_lock(&pinctrl_mutex); - pctldev = get_pinctrl_dev_from_devname(dev_name); if (!pctldev) { ret = -EINVAL; - goto unlock; + return ret; } + mutex_lock(&pctldev->mutex); + pin = pin_get_from_name(pctldev, name); if (pin < 0) { ret = pin; @@ -162,7 +162,7 @@ int pin_config_set(const char *dev_name, const char *name, ret = pin_config_set_for_pin(pctldev, pin, config); unlock: - mutex_unlock(&pinctrl_mutex); + mutex_unlock(&pctldev->mutex); return ret; } EXPORT_SYMBOL(pin_config_set); @@ -174,13 +174,14 @@ int pin_config_group_get(const char *dev_name, const char *pin_group, const struct pinconf_ops *ops; int selector, ret; - mutex_lock(&pinctrl_mutex); - pctldev = get_pinctrl_dev_from_devname(dev_name); if (!pctldev) { ret = -EINVAL; - goto unlock; + return ret; } + + mutex_lock(&pctldev->mutex); + ops = pctldev->desc->confops; if (!ops || !ops->pin_config_group_get) { @@ -200,7 +201,7 @@ int pin_config_group_get(const char *dev_name, const char *pin_group, ret = ops->pin_config_group_get(pctldev, selector, config); unlock: - mutex_unlock(&pinctrl_mutex); + mutex_unlock(&pctldev->mutex); return ret; } EXPORT_SYMBOL(pin_config_group_get); @@ -217,13 +218,14 @@ int pin_config_group_set(const char *dev_name, const char *pin_group, int ret; int i; - mutex_lock(&pinctrl_mutex); - pctldev = get_pinctrl_dev_from_devname(dev_name); if (!pctldev) { ret = -EINVAL; - goto unlock; + return ret; } + + mutex_lock(&pctldev->mutex); + ops = pctldev->desc->confops; pctlops = pctldev->desc->pctlops; @@ -279,7 +281,7 @@ int pin_config_group_set(const char *dev_name, const char *pin_group, ret = 0; unlock: - mutex_unlock(&pinctrl_mutex); + mutex_unlock(&pctldev->mutex); return ret; } @@ -487,7 +489,7 @@ static int pinconf_pins_show(struct seq_file *s, void *what) seq_puts(s, "Pin config settings per pin\n"); seq_puts(s, "Format: pin (name): configs\n"); - mutex_lock(&pinctrl_mutex); + mutex_lock(&pctldev->mutex); /* The pin number can be retrived from the pin controller descriptor */ for (i = 0; i < pctldev->desc->npins; i++) { @@ -507,7 +509,7 @@ static int pinconf_pins_show(struct seq_file *s, void *what) seq_printf(s, "\n"); } - mutex_unlock(&pinctrl_mutex); + mutex_unlock(&pctldev->mutex); return 0; } @@ -608,7 +610,7 @@ static int pinconf_dbg_config_print(struct seq_file *s, void *d) bool found = false; unsigned long config; - mutex_lock(&pinctrl_mutex); + mutex_lock(&pctldev->mutex); /* Parse the pinctrl map and look for the elected pin/state */ for_each_maps(maps_node, i, map) { @@ -657,7 +659,7 @@ static int pinconf_dbg_config_print(struct seq_file *s, void *d) confops->pin_config_config_dbg_show(pctldev, s, config); exit: - mutex_unlock(&pinctrl_mutex); + mutex_unlock(&pctldev->mutex); return 0; } @@ -747,7 +749,7 @@ static int pinconf_dbg_config_write(struct file *file, return -EINVAL; strncpy(config, token, MAX_NAME_LEN); - mutex_lock(&pinctrl_mutex); + mutex_lock(&pinctrl_maps_mutex); /* Parse the pinctrl map and look for the selected dev/state/pin */ for_each_maps(maps_node, i, map) { @@ -785,7 +787,7 @@ static int pinconf_dbg_config_write(struct file *file, } exit: - mutex_unlock(&pinctrl_mutex); + mutex_unlock(&pinctrl_maps_mutex); return count; } -- cgit v1.2.3