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 <seraphin.bonnaffe@stericsson.com>
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

drivers/pinctrl/core.h

@@ -33,6 +33,7 @@ struct pinctrl_gpio_range;
  * @p: result of pinctrl_get() for this device
  * @hog_default: default state for pins hogged by this device
  * @hog_sleep: sleep state for pins hogged by this device
+ * @mutex: mutex taken on each pin controller specific action
  * @device_root: debugfs root for this device
  */
 struct pinctrl_dev {
@@ -46,6 +47,7 @@ struct pinctrl_dev {
 	struct pinctrl *p;
 	struct pinctrl_state *hog_default;
 	struct pinctrl_state *hog_sleep;
+	struct mutex mutex;
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *device_root;
 #endif
@@ -168,6 +170,7 @@ struct pinctrl_maps {
 };
 
 struct pinctrl_dev *get_pinctrl_dev_from_devname(const char *dev_name);
+struct pinctrl_dev *get_pinctrl_dev_from_of_node(struct device_node *np);
 int pin_get_from_name(struct pinctrl_dev *pctldev, const char *name);
 const char *pin_get_name(struct pinctrl_dev *pctldev, const unsigned pin);
 int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
@@ -186,8 +189,7 @@ void pinctrl_unregister_map(struct pinctrl_map const *map);
 extern int pinctrl_force_sleep(struct pinctrl_dev *pctldev);
 extern int pinctrl_force_default(struct pinctrl_dev *pctldev);
 
-extern struct mutex pinctrl_mutex;
-extern struct list_head pinctrldev_list;
+extern struct mutex pinctrl_maps_mutex;
 extern struct list_head pinctrl_maps;
 
 #define for_each_maps(_maps_node_, _i_, _map_) \

drivers/pinctrl/devicetree.c

@@ -95,22 +95,11 @@ static int dt_remember_or_free_map(struct pinctrl *p, const char *statename,
 	return pinctrl_register_map(map, num_maps, false, true);
 }
 
-static struct pinctrl_dev *find_pinctrl_by_of_node(struct device_node *np)
-{
-	struct pinctrl_dev *pctldev;
-
-	list_for_each_entry(pctldev, &pinctrldev_list, node)
-		if (pctldev->dev->of_node == np)
-			return pctldev;
-
-	return NULL;
-}
-
 struct pinctrl_dev *of_pinctrl_get(struct device_node *np)
 {
 	struct pinctrl_dev *pctldev;
 
-	pctldev = find_pinctrl_by_of_node(np);
+	pctldev = get_pinctrl_dev_from_of_node(np);
 	if (!pctldev)
 		return NULL;
 
@@ -138,7 +127,7 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
 			/* OK let's just assume this will appear later then */
 			return -EPROBE_DEFER;
 		}
-		pctldev = find_pinctrl_by_of_node(np_pctldev);
+		pctldev = get_pinctrl_dev_from_of_node(np_pctldev);
 		if (pctldev)
 			break;
 		/* Do not defer probing of hogs (circular loop) */

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;
 }

drivers/pinctrl/pinmux.c

@@ -506,7 +506,7 @@ static int pinmux_functions_show(struct seq_file *s, void *what)
 	if (!pmxops)
 		return 0;
 
-	mutex_lock(&pinctrl_mutex);
+	mutex_lock(&pctldev->mutex);
 	nfuncs = pmxops->get_functions_count(pctldev);
 	while (func_selector < nfuncs) {
 		const char *func = pmxops->get_function_name(pctldev,
@@ -530,7 +530,7 @@ static int pinmux_functions_show(struct seq_file *s, void *what)
 		func_selector++;
 	}
 
-	mutex_unlock(&pinctrl_mutex);
+	mutex_unlock(&pctldev->mutex);
 
 	return 0;
 }
@@ -548,7 +548,7 @@ 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): mux_owner gpio_owner hog?\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++) {
@@ -583,7 +583,7 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 			seq_printf(s, "\n");
 	}
 
-	mutex_unlock(&pinctrl_mutex);
+	mutex_unlock(&pctldev->mutex);
 
 	return 0;
 }