pinctrl: refactor struct pinctrl handling in core.c vs pinmux.c

This change separates two aspects of struct pinctrl:

a) The data representation of the parsed mapping table, into:

   1) The top-level struct pinctrl object, a single entity returned
      by pinctrl_get().

   2) The parsed version of each mapping table entry, struct
      pinctrl_setting, of which there is one per mapping table entry.

b) The code that handles this; the code for (1) above is in core.c, and
   the code to parse/execute each entry in (2) above is in pinmux.c, while
   the iteration over multiple settings is lifted to core.c.

This will allow the following future changes:

1) pinctrl_get() API rework, so that struct pinctrl represents all states
   for the device, and the device can select between them without calling
   put()/get() again.

2) To support that, a struct pinctrl_state object will be inserted into
   the data model between the struct pinctrl and struct pinctrl_setting.

3) The mapping table will be extended to allow specification of pin config
   settings too. To support this, struct pinctrl_setting will be enhanced
   to store either mux settings or config settings, and functions will be
   added to pinconf.c to parse/execute pin configuration settings.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

drivers/pinctrl/core.c

@@ -460,14 +460,15 @@ EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output);
 
 static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
 {
-	struct pinctrl_dev *pctldev = NULL;
+	struct pinctrl_dev *pctldev;
 	const char *devname;
 	struct pinctrl *p;
 	unsigned num_maps = 0;
-	int ret = -ENODEV;
+	int ret;
 	struct pinctrl_maps *maps_node;
 	int i;
 	struct pinctrl_map const *map;
+	struct pinctrl_setting *setting;
 
 	/* We must have both a dev and state name */
 	if (WARN_ON(!dev || !name))
@@ -487,39 +488,50 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
 		dev_err(dev, "failed to alloc struct pinctrl\n");
 		return ERR_PTR(-ENOMEM);
 	}
-	pinmux_init_pinctrl_handle(p);
+	p->dev = dev;
+	p->state = name;
+	INIT_LIST_HEAD(&p->settings);
 
 	/* Iterate over the pin control maps to locate the right ones */
 	for_each_maps(maps_node, i, map) {
+		/* Map must be for this device */
+		if (strcmp(map->dev_name, devname))
+			continue;
+
+		/* State name must be the one we're looking for */
+		if (strcmp(map->name, name))
+			continue;
+
 		/*
-		 * First, try to find the pctldev given in the map
+		 * Try to find the pctldev given in the map
 		 */
 		pctldev = get_pinctrl_dev_from_devname(map->ctrl_dev_name);
 		if (!pctldev) {
 			dev_err(dev, "unknown pinctrl device %s in map entry",
 				map->ctrl_dev_name);
-			pinmux_put(p);
-			kfree(p);
 			/* Eventually, this should trigger deferred probe */
-			return ERR_PTR(-ENODEV);
+			ret = -ENODEV;
+			goto error;
 		}
 
 		dev_dbg(dev, "in map, found pctldev %s to handle function %s",
 			dev_name(pctldev->dev), map->function);
 
-		/* Map must be for this device */
-		if (strcmp(map->dev_name, devname))
-			continue;
+		setting = kzalloc(sizeof(*setting), GFP_KERNEL);
+		if (setting == NULL) {
+			dev_err(dev,
+				"failed to alloc struct pinctrl_setting\n");
+			ret = -ENOMEM;
+			goto error;
+		}
 
-		/* State name must be the one we're looking for */
-		if (strcmp(map->name, name))
-			continue;
+		setting->pctldev = pctldev;
+		ret = pinmux_map_to_setting(map, setting);
+		if (ret < 0)
+			goto error;
+
+		list_add_tail(&setting->node, &p->settings);
 
-		ret = pinmux_apply_muxmap(pctldev, p, dev, devname, map);
-		if (ret) {
-			kfree(p);
-			return ERR_PTR(ret);
-		}
 		num_maps++;
 	}
 
@@ -541,6 +553,14 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
 	list_add_tail(&p->node, &pinctrl_list);
 
 	return p;
+
+error:
+	list_for_each_entry(setting, &p->settings, node)
+		pinmux_free_setting(setting);
+
+	kfree(p);
+
+	return ERR_PTR(ret);
 }
 
 /**
@@ -564,13 +584,18 @@ EXPORT_SYMBOL_GPL(pinctrl_get);
 
 static void pinctrl_put_locked(struct pinctrl *p)
 {
+	struct pinctrl_setting *setting, *n;
+
 	if (p == NULL)
 		return;
 
 	if (p->usecount)
 		pr_warn("releasing pin control handle with active users!\n");
-	/* Free the groups and all acquired pins */
-	pinmux_put(p);
+	list_for_each_entry_safe(setting, n, &p->settings, node) {
+		pinmux_free_setting(setting);
+		list_del(&setting->node);
+		kfree(setting);
+	}
 
 	/* Remove from list */
 	list_del(&p->node);
@@ -592,18 +617,24 @@ EXPORT_SYMBOL_GPL(pinctrl_put);
 
 static int pinctrl_enable_locked(struct pinctrl *p)
 {
-	int ret = 0;
+	struct pinctrl_setting *setting;
+	int ret;
 
 	if (p == NULL)
 		return -EINVAL;
 
 	if (p->usecount++ == 0) {
-		ret = pinmux_enable(p);
-		if (ret)
-			p->usecount--;
+		list_for_each_entry(setting, &p->settings, node) {
+			ret = pinmux_enable_setting(setting);
+			if (ret < 0) {
+				/* FIXME: Difficult to return to prev state */
+				p->usecount--;
+				return ret;
+			}
+		}
 	}
 
-	return ret;
+	return 0;
 }
 
 /**
@@ -622,11 +653,14 @@ EXPORT_SYMBOL_GPL(pinctrl_enable);
 
 static void pinctrl_disable_locked(struct pinctrl *p)
 {
+	struct pinctrl_setting *setting;
+
 	if (p == NULL)
 		return;
 
 	if (--p->usecount == 0) {
-		pinmux_disable(p);
+		list_for_each_entry(setting, &p->settings, node)
+			pinmux_disable_setting(setting);
 	}
 }
 
@@ -857,27 +891,20 @@ static int pinctrl_maps_show(struct seq_file *s, void *what)
 static int pinctrl_show(struct seq_file *s, void *what)
 {
 	struct pinctrl *p;
+	struct pinctrl_setting *setting;
 
 	seq_puts(s, "Requested pin control handlers their pinmux maps:\n");
 
 	mutex_lock(&pinctrl_mutex);
 
 	list_for_each_entry(p, &pinctrl_list, node) {
-		struct pinctrl_dev *pctldev = p->pctldev;
+		seq_printf(s, "device: %s state: %s users: %u\n",
+			   dev_name(p->dev), p->state, p->usecount);
 
-		if (!pctldev) {
-			seq_puts(s, "NO PIN CONTROLLER DEVICE\n");
-			continue;
+		list_for_each_entry(setting, &p->settings, node) {
+			seq_printf(s, "  ");
+			pinmux_dbg_show(s, setting);
 		}
-
-		seq_printf(s, "device: %s",
-			   pinctrl_dev_get_name(p->pctldev));
-
-		pinmux_dbg_show(s, p);
-
-		seq_printf(s, " users: %u map-> %s\n",
-			   p->usecount,
-			   p->dev ? dev_name(p->dev) : "(system)");
 	}
 
 	mutex_unlock(&pinctrl_mutex);

drivers/pinctrl/core.h

@@ -49,22 +49,31 @@ struct pinctrl_dev {
  * struct pinctrl - per-device pin control state holder
  * @node: global list node
  * @dev: the device using this pin control handle
+ * @state: the state name passed to pinctrl_get()
  * @usecount: the number of active users of this pin controller setting, used
  *	to keep track of nested use cases
- * @pctldev: pin control device handling this pin control handle
- * @groups: the group selectors for the pinmux device and
- *	selector combination handling this pinmux, this is a list that
- *	will be traversed on all pinmux operations such as
- *	get/put/enable/disable
+ * @settings: a list of settings for this device/state
  */
 struct pinctrl {
 	struct list_head node;
 	struct device *dev;
+	const char *state;
 	unsigned usecount;
+	struct list_head settings;
+};
+
+/**
+ * struct pinctrl_setting - an individual mux setting
+ * @node: list node for struct pinctrl's @settings field
+ * @pctldev: pin control device handling to be programmed
+ * @group_selector: the group selector to program
+ * @func_selector: the function selector to program
+ */
+struct pinctrl_setting {
+	struct list_head node;
 	struct pinctrl_dev *pctldev;
-#ifdef CONFIG_PINMUX
-	struct list_head groups;
-#endif
+	unsigned group_selector;
+	unsigned func_selector;
 };
 
 /**

drivers/pinctrl/pinmux.h

@@ -13,6 +13,7 @@
 #ifdef CONFIG_PINMUX
 
 int pinmux_check_ops(struct pinctrl_dev *pctldev);
+
 int pinmux_request_gpio(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range,
 			unsigned pin, unsigned gpio);
@@ -21,21 +22,16 @@ void pinmux_free_gpio(struct pinctrl_dev *pctldev, unsigned pin,
 int pinmux_gpio_direction(struct pinctrl_dev *pctldev,
 			  struct pinctrl_gpio_range *range,
 			  unsigned pin, bool input);
-static inline void pinmux_init_pinctrl_handle(struct pinctrl *p)
-{
-	INIT_LIST_HEAD(&p->groups);
-}
-int pinmux_apply_muxmap(struct pinctrl_dev *pctldev,
-			struct pinctrl *p,
-			struct device *dev,
-			const char *devname,
-			struct pinctrl_map const *map);
-void pinmux_put(struct pinctrl *p);
-int pinmux_enable(struct pinctrl *p);
-void pinmux_disable(struct pinctrl *p);
+
+int pinmux_map_to_setting(struct pinctrl_map const *map,
+			  struct pinctrl_setting *setting);
+void pinmux_free_setting(struct pinctrl_setting const *setting);
+int pinmux_enable_setting(struct pinctrl_setting const *setting);
+void pinmux_disable_setting(struct pinctrl_setting const *setting);
+
+void pinmux_dbg_show(struct seq_file *s, struct pinctrl_setting const *setting);
 void pinmux_init_device_debugfs(struct dentry *devroot,
 				struct pinctrl_dev *pctldev);
-void pinmux_dbg_show(struct seq_file *s, struct pinctrl *p);
 
 #else
 
@@ -64,28 +60,23 @@ static inline int pinmux_gpio_direction(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
-static inline void pinmux_init_pinctrl_handle(struct pinctrl *p)
-{
-}
-
-static inline int pinmux_apply_muxmap(struct pinctrl_dev *pctldev,
-				      struct pinctrl *p,
-				      struct device *dev,
-				      const char *devname,
-				      struct pinctrl_map const *map)
+static inline int pinmux_map_to_setting(struct pinctrl_map const *map,
+			  struct pinctrl_setting *setting)
 {
 	return 0;
 }
 
-static inline void pinmux_put(struct pinctrl *p)
+static inline void pinmux_free_setting(struct pinctrl_setting const *setting)
 {
 }
 
-static inline int pinmux_enable(struct pinctrl *p)
+static inline int pinmux_enable_setting(struct pinctrl_setting const *setting)
 {
+	return 0;
 }
 
-static inline void pinmux_disable(struct pinctrl *p)
+static inline void pinmux_disable_setting(
+			struct pinctrl_setting const *setting)
 {
 }