linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, ...
@ 2012-02-28 20:27 Stephen Warren
  2012-02-28 20:27 ` [PATCH V2 1/6] pinctrl: Fix and simplify locking Stephen Warren
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Stephen Warren @ 2012-02-28 20:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel, Stephen Warren

This is the current set of patches I have outstanding for pinctrl.

The locking rework change is still under discussion; I haven't seen a
nak/ack since I was asked to move it later in the series and I explained
the difficulties in doing so while maintaining correctness.

The other changes have fixes based on review feedback.

The last patch is a fix for the Tegra30 pinctrl driver that showed up
once I was able to test on real HW. You should be able to apply this
out-of-order if you want. I'd even be happy if you squashed it into the
original commit that added the driver, if you branch doesn't need to be
git-stable.

Stephen Warren (6):
  pinctrl: Fix and simplify locking
  pinctrl: Refactor struct pinctrl handling in core.c vs pinmux.c
  pinctrl: Add usecount to pins for muxing
  pinctrl: API changes to support multiple states per device
  pinctrl: Enhance mapping table to support pin config operations
  pinctrl: fix case of Tegra30's foo_groups[] arrays

 Documentation/pinctrl.txt         |  196 +++-
 arch/arm/mach-u300/core.c         |   28 +-
 drivers/pinctrl/core.c            |  630 ++++++++----
 drivers/pinctrl/core.h            |   92 ++-
 drivers/pinctrl/pinconf.c         |  272 +++++-
 drivers/pinctrl/pinconf.h         |   40 +
 drivers/pinctrl/pinctrl-tegra30.c | 1992 ++++++++++++++++++------------------
 drivers/pinctrl/pinmux.c          |  464 +++------
 drivers/pinctrl/pinmux.h          |   59 +-
 drivers/tty/serial/sirfsoc_uart.c |   12 +-
 include/linux/pinctrl/consumer.h  |   55 +-
 include/linux/pinctrl/machine.h   |  148 +++-
 include/linux/pinctrl/pinctrl.h   |    1 -
 13 files changed, 2282 insertions(+), 1707 deletions(-)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V2 1/6] pinctrl: Fix and simplify locking
  2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
@ 2012-02-28 20:27 ` Stephen Warren
  2012-02-28 20:27 ` [PATCH V2 2/6] pinctrl: Refactor struct pinctrl handling in core.c vs pinmux.c Stephen Warren
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-02-28 20:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel, Stephen Warren

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>
---
v2: Rebased. No logical changes.

 drivers/pinctrl/core.c          |  198 ++++++++++++++++++++++++---------------
 drivers/pinctrl/core.h          |   10 +-
 drivers/pinctrl/pinconf.c       |  107 ++++++++++++++++------
 drivers/pinctrl/pinmux.c        |   21 ++---
 include/linux/pinctrl/pinctrl.h |    1 -
 5 files changed, 215 insertions(+), 122 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 6af6d8d..aefc339 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -18,11 +18,8 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/slab.h>
-#include <linux/radix-tree.h>
 #include <linux/err.h>
 #include <linux/list.h>
-#include <linux/mutex.h>
-#include <linux/spinlock.h>
 #include <linux/sysfs.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
@@ -44,16 +41,16 @@ struct pinctrl_maps {
 	unsigned num_maps;
 };
 
-/* Global list of pin control devices */
-static DEFINE_MUTEX(pinctrldev_list_mutex);
+/* Mutex taken by all entry points */
+DEFINE_MUTEX(pinctrl_mutex);
+
+/* Global list of pin control devices (struct pinctrl_dev) */
 static LIST_HEAD(pinctrldev_list);
 
-/* List of pin controller handles */
-static DEFINE_MUTEX(pinctrl_list_mutex);
+/* List of pin controller handles (struct pinctrl) */
 static LIST_HEAD(pinctrl_list);
 
-/* Global pinctrl maps */
-static DEFINE_MUTEX(pinctrl_maps_mutex);
+/* List of pinctrl maps (struct pinctrl_maps) */
 static LIST_HEAD(pinctrl_maps);
 
 #define for_each_maps(_maps_node_, _i_, _map_) \
@@ -90,7 +87,6 @@ struct pinctrl_dev *get_pinctrl_dev_from_devname(const char *devname)
 	if (!devname)
 		return NULL;
 
-	mutex_lock(&pinctrldev_list_mutex);
 	list_for_each_entry(pctldev, &pinctrldev_list, node) {
 		if (!strcmp(dev_name(pctldev->dev), devname)) {
 			/* Matched on device name */
@@ -98,7 +94,6 @@ struct pinctrl_dev *get_pinctrl_dev_from_devname(const char *devname)
 			break;
 		}
 	}
-	mutex_unlock(&pinctrldev_list_mutex);
 
 	return found ? pctldev : NULL;
 }
@@ -143,11 +138,11 @@ bool pin_is_valid(struct pinctrl_dev *pctldev, int pin)
 	if (pin < 0)
 		return false;
 
+	mutex_lock(&pinctrl_mutex);
 	pindesc = pin_desc_get(pctldev, pin);
-	if (pindesc == NULL)
-		return false;
+	mutex_unlock(&pinctrl_mutex);
 
-	return true;
+	return pindesc != NULL;
 }
 EXPORT_SYMBOL_GPL(pin_is_valid);
 
@@ -191,8 +186,6 @@ static int pinctrl_register_one_pin(struct pinctrl_dev *pctldev,
 		return -ENOMEM;
 	}
 
-	spin_lock_init(&pindesc->lock);
-
 	/* Set owner */
 	pindesc->pctldev = pctldev;
 
@@ -243,16 +236,13 @@ pinctrl_match_gpio_range(struct pinctrl_dev *pctldev, unsigned gpio)
 	struct pinctrl_gpio_range *range = NULL;
 
 	/* Loop over the ranges */
-	mutex_lock(&pctldev->gpio_ranges_lock);
 	list_for_each_entry(range, &pctldev->gpio_ranges, node) {
 		/* Check if we're in the valid range */
 		if (gpio >= range->base &&
 		    gpio < range->base + range->npins) {
-			mutex_unlock(&pctldev->gpio_ranges_lock);
 			return range;
 		}
 	}
-	mutex_unlock(&pctldev->gpio_ranges_lock);
 
 	return NULL;
 }
@@ -274,7 +264,6 @@ static int pinctrl_get_device_gpio_range(unsigned gpio,
 	struct pinctrl_dev *pctldev = NULL;
 
 	/* Loop over the pin controllers */
-	mutex_lock(&pinctrldev_list_mutex);
 	list_for_each_entry(pctldev, &pinctrldev_list, node) {
 		struct pinctrl_gpio_range *range;
 
@@ -282,11 +271,9 @@ static int pinctrl_get_device_gpio_range(unsigned gpio,
 		if (range != NULL) {
 			*outdev = pctldev;
 			*outrange = range;
-			mutex_unlock(&pinctrldev_list_mutex);
 			return 0;
 		}
 	}
-	mutex_unlock(&pinctrldev_list_mutex);
 
 	return -EINVAL;
 }
@@ -302,9 +289,9 @@ static int pinctrl_get_device_gpio_range(unsigned gpio,
 void pinctrl_add_gpio_range(struct pinctrl_dev *pctldev,
 			    struct pinctrl_gpio_range *range)
 {
-	mutex_lock(&pctldev->gpio_ranges_lock);
+	mutex_lock(&pinctrl_mutex);
 	list_add_tail(&range->node, &pctldev->gpio_ranges);
-	mutex_unlock(&pctldev->gpio_ranges_lock);
+	mutex_unlock(&pinctrl_mutex);
 }
 EXPORT_SYMBOL_GPL(pinctrl_add_gpio_range);
 
@@ -316,9 +303,9 @@ EXPORT_SYMBOL_GPL(pinctrl_add_gpio_range);
 void pinctrl_remove_gpio_range(struct pinctrl_dev *pctldev,
 			       struct pinctrl_gpio_range *range)
 {
-	mutex_lock(&pctldev->gpio_ranges_lock);
+	mutex_lock(&pinctrl_mutex);
 	list_del(&range->node);
-	mutex_unlock(&pctldev->gpio_ranges_lock);
+	mutex_unlock(&pinctrl_mutex);
 }
 EXPORT_SYMBOL_GPL(pinctrl_remove_gpio_range);
 
@@ -368,14 +355,21 @@ int pinctrl_request_gpio(unsigned gpio)
 	int ret;
 	int pin;
 
+	mutex_lock(&pinctrl_mutex);
+
 	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
-	if (ret)
+	if (ret) {
+		mutex_unlock(&pinctrl_mutex);
 		return -EINVAL;
+	}
 
 	/* Convert to the pin controllers number space */
 	pin = gpio - range->base + range->pin_base;
 
-	return pinmux_request_gpio(pctldev, range, pin, gpio);
+	ret = pinmux_request_gpio(pctldev, range, pin, gpio);
+
+	mutex_unlock(&pinctrl_mutex);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pinctrl_request_gpio);
 
@@ -394,14 +388,20 @@ void pinctrl_free_gpio(unsigned gpio)
 	int ret;
 	int pin;
 
+	mutex_lock(&pinctrl_mutex);
+
 	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
-	if (ret)
+	if (ret) {
+		mutex_unlock(&pinctrl_mutex);
 		return;
+	}
 
 	/* Convert to the pin controllers number space */
 	pin = gpio - range->base + range->pin_base;
 
-	return pinmux_free_gpio(pctldev, pin, range);
+	pinmux_free_gpio(pctldev, pin, range);
+
+	mutex_unlock(&pinctrl_mutex);
 }
 EXPORT_SYMBOL_GPL(pinctrl_free_gpio);
 
@@ -432,7 +432,11 @@ static int pinctrl_gpio_direction(unsigned gpio, bool input)
  */
 int pinctrl_gpio_direction_input(unsigned gpio)
 {
-	return pinctrl_gpio_direction(gpio, true);
+	int ret;
+	mutex_lock(&pinctrl_mutex);
+	ret = pinctrl_gpio_direction(gpio, true);
+	mutex_unlock(&pinctrl_mutex);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_input);
 
@@ -446,7 +450,11 @@ EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_input);
  */
 int pinctrl_gpio_direction_output(unsigned gpio)
 {
-	return pinctrl_gpio_direction(gpio, false);
+	int ret;
+	mutex_lock(&pinctrl_mutex);
+	ret = pinctrl_gpio_direction(gpio, false);
+	mutex_unlock(&pinctrl_mutex);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output);
 
@@ -479,7 +487,6 @@ 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);
 	}
-	mutex_init(&p->mutex);
 	pinmux_init_pinctrl_handle(p);
 
 	/* Iterate over the pin control maps to locate the right ones */
@@ -531,9 +538,7 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
 		num_maps, devname, name ? name : "(undefined)");
 
 	/* Add the pinmux to the global list */
-	mutex_lock(&pinctrl_list_mutex);
 	list_add_tail(&p->node, &pinctrl_list);
-	mutex_unlock(&pinctrl_list_mutex);
 
 	return p;
 }
@@ -549,74 +554,91 @@ struct pinctrl *pinctrl_get(struct device *dev, const char *name)
 {
 	struct pinctrl *p;
 
-	mutex_lock(&pinctrl_maps_mutex);
+	mutex_lock(&pinctrl_mutex);
 	p = pinctrl_get_locked(dev, name);
-	mutex_unlock(&pinctrl_maps_mutex);
+	mutex_unlock(&pinctrl_mutex);
 
 	return p;
 }
 EXPORT_SYMBOL_GPL(pinctrl_get);
 
-/**
- * pinctrl_put() - release a previously claimed pin control handle
- * @p: a pin control handle previously claimed by pinctrl_get()
- */
-void pinctrl_put(struct pinctrl *p)
+static void pinctrl_put_locked(struct pinctrl *p)
 {
 	if (p == NULL)
 		return;
 
-	mutex_lock(&p->mutex);
 	if (p->usecount)
 		pr_warn("releasing pin control handle with active users!\n");
 	/* Free the groups and all acquired pins */
 	pinmux_put(p);
-	mutex_unlock(&p->mutex);
 
 	/* Remove from list */
-	mutex_lock(&pinctrl_list_mutex);
 	list_del(&p->node);
-	mutex_unlock(&pinctrl_list_mutex);
 
 	kfree(p);
 }
-EXPORT_SYMBOL_GPL(pinctrl_put);
 
 /**
- * pinctrl_enable() - enable a certain pin controller setting
- * @p: the pin control handle to enable, previously claimed by pinctrl_get()
+ * pinctrl_put() - release a previously claimed pin control handle
+ * @p: a pin control handle previously claimed by pinctrl_get()
  */
-int pinctrl_enable(struct pinctrl *p)
+void pinctrl_put(struct pinctrl *p)
+{
+	mutex_lock(&pinctrl_mutex);
+	pinctrl_put(p);
+	mutex_unlock(&pinctrl_mutex);
+}
+EXPORT_SYMBOL_GPL(pinctrl_put);
+
+static int pinctrl_enable_locked(struct pinctrl *p)
 {
 	int ret = 0;
 
 	if (p == NULL)
 		return -EINVAL;
-	mutex_lock(&p->mutex);
+
 	if (p->usecount++ == 0) {
 		ret = pinmux_enable(p);
 		if (ret)
 			p->usecount--;
 	}
-	mutex_unlock(&p->mutex);
+
 	return ret;
 }
-EXPORT_SYMBOL_GPL(pinctrl_enable);
 
 /**
- * pinctrl_disable() - disable a certain pin control setting
- * @p: the pin control handle to disable, previously claimed by pinctrl_get()
+ * pinctrl_enable() - enable a certain pin controller setting
+ * @p: the pin control handle to enable, previously claimed by pinctrl_get()
  */
-void pinctrl_disable(struct pinctrl *p)
+int pinctrl_enable(struct pinctrl *p)
+{
+	int ret;
+	mutex_lock(&pinctrl_mutex);
+	ret = pinctrl_enable_locked(p);
+	mutex_unlock(&pinctrl_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pinctrl_enable);
+
+static void pinctrl_disable_locked(struct pinctrl *p)
 {
 	if (p == NULL)
 		return;
 
-	mutex_lock(&p->mutex);
 	if (--p->usecount == 0) {
 		pinmux_disable(p);
 	}
-	mutex_unlock(&p->mutex);
+}
+
+/**
+ * pinctrl_disable() - disable a certain pin control setting
+ * @p: the pin control handle to disable, previously claimed by pinctrl_get()
+ */
+void pinctrl_disable(struct pinctrl *p)
+{
+	mutex_lock(&pinctrl_mutex);
+	pinctrl_disable_locked(p);
+	mutex_unlock(&pinctrl_mutex);
 }
 EXPORT_SYMBOL_GPL(pinctrl_disable);
 
@@ -676,9 +698,9 @@ int pinctrl_register_mappings(struct pinctrl_map const *maps,
 		return -ENOMEM;
 	}
 
-	mutex_lock(&pinctrl_maps_mutex);
+	mutex_lock(&pinctrl_mutex);
 	list_add_tail(&maps_node->node, &pinctrl_maps);
-	mutex_unlock(&pinctrl_maps_mutex);
+	mutex_unlock(&pinctrl_mutex);
 
 	return 0;
 }
@@ -693,6 +715,8 @@ static int pinctrl_pins_show(struct seq_file *s, void *what)
 
 	seq_printf(s, "registered pins: %d\n", pctldev->desc->npins);
 
+	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;
@@ -713,6 +737,8 @@ static int pinctrl_pins_show(struct seq_file *s, void *what)
 		seq_puts(s, "\n");
 	}
 
+	mutex_unlock(&pinctrl_mutex);
+
 	return 0;
 }
 
@@ -726,6 +752,8 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
 	if (!ops)
 		return 0;
 
+	mutex_lock(&pinctrl_mutex);
+
 	seq_puts(s, "registered pin groups:\n");
 	while (ops->list_groups(pctldev, selector) >= 0) {
 		const unsigned *pins;
@@ -748,6 +776,7 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
 		selector++;
 	}
 
+	mutex_unlock(&pinctrl_mutex);
 
 	return 0;
 }
@@ -759,8 +788,9 @@ static int pinctrl_gpioranges_show(struct seq_file *s, void *what)
 
 	seq_puts(s, "GPIO ranges handled:\n");
 
+	mutex_lock(&pinctrl_mutex);
+
 	/* Loop over the ranges */
-	mutex_lock(&pctldev->gpio_ranges_lock);
 	list_for_each_entry(range, &pctldev->gpio_ranges, node) {
 		seq_printf(s, "%u: %s GPIOS [%u - %u] PINS [%u - %u]\n",
 			   range->id, range->name,
@@ -768,7 +798,8 @@ static int pinctrl_gpioranges_show(struct seq_file *s, void *what)
 			   range->pin_base,
 			   (range->pin_base + range->npins - 1));
 	}
-	mutex_unlock(&pctldev->gpio_ranges_lock);
+
+	mutex_unlock(&pinctrl_mutex);
 
 	return 0;
 }
@@ -778,7 +809,9 @@ static int pinctrl_devices_show(struct seq_file *s, void *what)
 	struct pinctrl_dev *pctldev;
 
 	seq_puts(s, "name [pinmux] [pinconf]\n");
-	mutex_lock(&pinctrldev_list_mutex);
+
+	mutex_lock(&pinctrl_mutex);
+
 	list_for_each_entry(pctldev, &pinctrldev_list, node) {
 		seq_printf(s, "%s ", pctldev->desc->name);
 		if (pctldev->desc->pmxops)
@@ -791,7 +824,8 @@ static int pinctrl_devices_show(struct seq_file *s, void *what)
 			seq_puts(s, "no");
 		seq_puts(s, "\n");
 	}
-	mutex_unlock(&pinctrldev_list_mutex);
+
+	mutex_unlock(&pinctrl_mutex);
 
 	return 0;
 }
@@ -804,7 +838,8 @@ static int pinctrl_maps_show(struct seq_file *s, void *what)
 
 	seq_puts(s, "Pinctrl maps:\n");
 
-	mutex_lock(&pinctrl_maps_mutex);
+	mutex_lock(&pinctrl_mutex);
+
 	for_each_maps(maps_node, i, map) {
 		seq_printf(s, "%s:\n", map->name);
 		seq_printf(s, "  device: %s\n", map->dev_name);
@@ -813,7 +848,8 @@ static int pinctrl_maps_show(struct seq_file *s, void *what)
 		seq_printf(s, "  group: %s\n", map->group ? map->group :
 			   "(default)");
 	}
-	mutex_unlock(&pinctrl_maps_mutex);
+
+	mutex_unlock(&pinctrl_mutex);
 
 	return 0;
 }
@@ -823,6 +859,9 @@ static int pinctrl_show(struct seq_file *s, void *what)
 	struct pinctrl *p;
 
 	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;
 
@@ -841,6 +880,8 @@ static int pinctrl_show(struct seq_file *s, void *what)
 			   p->dev ? dev_name(p->dev) : "(system)");
 	}
 
+	mutex_unlock(&pinctrl_mutex);
+
 	return 0;
 }
 
@@ -1008,7 +1049,6 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
 	pctldev->driver_data = driver_data;
 	INIT_RADIX_TREE(&pctldev->pin_desc_tree, GFP_KERNEL);
 	INIT_LIST_HEAD(&pctldev->gpio_ranges);
-	mutex_init(&pctldev->gpio_ranges_lock);
 	pctldev->dev = dev;
 
 	/* If we're implementing pinmuxing, check the ops for sanity */
@@ -1042,12 +1082,16 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
 		goto out_err;
 	}
 
-	mutex_lock(&pinctrldev_list_mutex);
+	mutex_lock(&pinctrl_mutex);
+
 	list_add_tail(&pctldev->node, &pinctrldev_list);
-	mutex_unlock(&pinctrldev_list_mutex);
-	pctldev->p = pinctrl_get(pctldev->dev, PINCTRL_STATE_DEFAULT);
+
+	pctldev->p = pinctrl_get_locked(pctldev->dev, PINCTRL_STATE_DEFAULT);
 	if (!IS_ERR(pctldev->p))
-		pinctrl_enable(pctldev->p);
+		pinctrl_enable_locked(pctldev->p);
+
+	mutex_unlock(&pinctrl_mutex);
+
 	pinctrl_init_device_debugfs(pctldev);
 
 	return pctldev;
@@ -1070,18 +1114,22 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
 		return;
 
 	pinctrl_remove_device_debugfs(pctldev);
+
+	mutex_lock(&pinctrl_mutex);
+
 	if (!IS_ERR(pctldev->p)) {
-		pinctrl_disable(pctldev->p);
-		pinctrl_put(pctldev->p);
+		pinctrl_disable_locked(pctldev->p);
+		pinctrl_put_locked(pctldev->p);
 	}
+
 	/* TODO: check that no pinmuxes are still active? */
-	mutex_lock(&pinctrldev_list_mutex);
 	list_del(&pctldev->node);
-	mutex_unlock(&pinctrldev_list_mutex);
 	/* Destroy descriptor tree */
 	pinctrl_free_pindescs(pctldev, pctldev->desc->pins,
 			      pctldev->desc->npins);
 	kfree(pctldev);
+
+	mutex_unlock(&pinctrl_mutex);
 }
 EXPORT_SYMBOL_GPL(pinctrl_unregister);
 
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 2981b73..d55cb67 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -9,6 +9,8 @@
  * License terms: GNU General Public License (GPL) version 2
  */
 
+#include <linux/mutex.h>
+#include <linux/radix-tree.h>
 #include <linux/pinctrl/pinconf.h>
 
 struct pinctrl_gpio_range;
@@ -22,7 +24,6 @@ struct pinctrl_gpio_range;
  *	this radix tree
  * @gpio_ranges: a list of GPIO ranges that is handled by this pin controller,
  *	ranges are added to this list at runtime
- * @gpio_ranges_lock: lock for the GPIO ranges list
  * @dev: the device entry for this pin controller
  * @owner: module providing the pin controller, used for refcounting
  * @driver_data: driver data for drivers registering to the pin controller
@@ -35,7 +36,6 @@ struct pinctrl_dev {
 	struct pinctrl_desc *desc;
 	struct radix_tree_root pin_desc_tree;
 	struct list_head gpio_ranges;
-	struct mutex gpio_ranges_lock;
 	struct device *dev;
 	struct module *owner;
 	void *driver_data;
@@ -52,7 +52,6 @@ struct pinctrl_dev {
  * @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
- * @mutex: a lock for the pin control state holder
  * @func_selector: the function selector for the pinmux device handling
  *	this pinmux
  * @groups: the group selectors for the pinmux device and
@@ -65,7 +64,6 @@ struct pinctrl {
 	struct device *dev;
 	unsigned usecount;
 	struct pinctrl_dev *pctldev;
-	struct mutex mutex;
 #ifdef CONFIG_PINMUX
 	unsigned func_selector;
 	struct list_head groups;
@@ -78,7 +76,6 @@ struct pinctrl {
  * @name: a name for the pin, e.g. the name of the pin/pad/finger on a
  *	datasheet or such
  * @dynamic_name: if the name of this pin was dynamically allocated
- * @lock: a lock to protect the descriptor structure
  * @mux_requested: whether the pin is already requested by pinmux or not
  * @mux_function: a named muxing function for the pin that will be passed to
  *	subdrivers and shown in debugfs etc
@@ -87,7 +84,6 @@ struct pin_desc {
 	struct pinctrl_dev *pctldev;
 	const char *name;
 	bool dynamic_name;
-	spinlock_t lock;
 	/* These fields only added when supporting pinmux drivers */
 #ifdef CONFIG_PINMUX
 	const char *owner;
@@ -104,3 +100,5 @@ static inline struct pin_desc *pin_desc_get(struct pinctrl_dev *pctldev,
 {
 	return radix_tree_lookup(&pctldev->pin_desc_tree, pin);
 }
+
+extern struct mutex pinctrl_mutex;
diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index 3f018a1..e0a4537 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -64,15 +64,23 @@ 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)
-		return -EINVAL;
+	if (!pctldev) {
+		pin = -EINVAL;
+		goto unlock;
+	}
 
 	pin = pin_get_from_name(pctldev, name);
 	if (pin < 0)
-		return pin;
+		goto unlock;
 
-	return pin_config_get_for_pin(pctldev, pin, config);
+	pin = pin_config_get_for_pin(pctldev, pin, config);
+
+unlock:
+	mutex_unlock(&pinctrl_mutex);
+	return pin;
 }
 EXPORT_SYMBOL(pin_config_get);
 
@@ -110,17 +118,27 @@ int pin_config_set(const char *dev_name, const char *name,
 		   unsigned long config)
 {
 	struct pinctrl_dev *pctldev;
-	int pin;
+	int pin, ret;
+
+	mutex_lock(&pinctrl_mutex);
 
 	pctldev = get_pinctrl_dev_from_devname(dev_name);
-	if (!pctldev)
-		return -EINVAL;
+	if (!pctldev) {
+		ret = -EINVAL;
+		goto unlock;
+	}
 
 	pin = pin_get_from_name(pctldev, name);
-	if (pin < 0)
-		return pin;
+	if (pin < 0) {
+		ret = pin;
+		goto unlock;
+	}
+
+	ret = pin_config_set_for_pin(pctldev, pin, config);
 
-	return pin_config_set_for_pin(pctldev, pin, config);
+unlock:
+	mutex_unlock(&pinctrl_mutex);
+	return ret;
 }
 EXPORT_SYMBOL(pin_config_set);
 
@@ -129,25 +147,36 @@ int pin_config_group_get(const char *dev_name, const char *pin_group,
 {
 	struct pinctrl_dev *pctldev;
 	const struct pinconf_ops *ops;
-	int selector;
+	int selector, ret;
+
+	mutex_lock(&pinctrl_mutex);
 
 	pctldev = get_pinctrl_dev_from_devname(dev_name);
-	if (!pctldev)
-		return -EINVAL;
+	if (!pctldev) {
+		ret = -EINVAL;
+		goto unlock;
+	}
 	ops = pctldev->desc->confops;
 
 	if (!ops || !ops->pin_config_group_get) {
 		dev_err(pctldev->dev, "cannot get configuration for pin "
 			"group, missing group config get function in "
 			"driver\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto unlock;
 	}
 
 	selector = pinctrl_get_group_selector(pctldev, pin_group);
-	if (selector < 0)
-		return selector;
+	if (selector < 0) {
+		ret = selector;
+		goto unlock;
+	}
 
-	return ops->pin_config_group_get(pctldev, selector, config);
+	ret = ops->pin_config_group_get(pctldev, selector, config);
+
+unlock:
+	mutex_unlock(&pinctrl_mutex);
+	return ret;
 }
 EXPORT_SYMBOL(pin_config_group_get);
 
@@ -163,27 +192,34 @@ 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)
-		return -EINVAL;
+	if (!pctldev) {
+		ret = -EINVAL;
+		goto unlock;
+	}
 	ops = pctldev->desc->confops;
 	pctlops = pctldev->desc->pctlops;
 
 	if (!ops || (!ops->pin_config_group_set && !ops->pin_config_set)) {
 		dev_err(pctldev->dev, "cannot configure pin group, missing "
 			"config function in driver\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto unlock;
 	}
 
 	selector = pinctrl_get_group_selector(pctldev, pin_group);
-	if (selector < 0)
-		return selector;
+	if (selector < 0) {
+		ret = selector;
+		goto unlock;
+	}
 
 	ret = pctlops->get_group_pins(pctldev, selector, &pins, &num_pins);
 	if (ret) {
 		dev_err(pctldev->dev, "cannot configure pin group, error "
 			"getting pins\n");
-		return ret;
+		goto unlock;
 	}
 
 	/*
@@ -197,23 +233,30 @@ int pin_config_group_set(const char *dev_name, const char *pin_group,
 		 * pin-by-pin as well, it returns -EAGAIN.
 		 */
 		if (ret != -EAGAIN)
-			return ret;
+			goto unlock;
 	}
 
 	/*
 	 * If the controller cannot handle entire groups, we configure each pin
 	 * individually.
 	 */
-	if (!ops->pin_config_set)
-		return 0;
+	if (!ops->pin_config_set) {
+		ret = 0;
+		goto unlock;
+	}
 
 	for (i = 0; i < num_pins; i++) {
 		ret = ops->pin_config_set(pctldev, pins[i], config);
 		if (ret < 0)
-			return ret;
+			goto unlock;
 	}
 
-	return 0;
+	ret = 0;
+
+unlock:
+	mutex_unlock(&pinctrl_mutex);
+
+	return ret;
 }
 EXPORT_SYMBOL(pin_config_group_set);
 
@@ -236,6 +279,8 @@ 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): pinmux setting array\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;
@@ -254,6 +299,8 @@ static int pinconf_pins_show(struct seq_file *s, void *what)
 		seq_printf(s, "\n");
 	}
 
+	mutex_unlock(&pinctrl_mutex);
+
 	return 0;
 }
 
@@ -280,6 +327,8 @@ static int pinconf_groups_show(struct seq_file *s, void *what)
 	seq_puts(s, "Pin config settings per pin group\n");
 	seq_puts(s, "Format: group (name): pinmux setting array\n");
 
+	mutex_lock(&pinctrl_mutex);
+
 	while (pctlops->list_groups(pctldev, selector) >= 0) {
 		const char *gname = pctlops->get_group_name(pctldev, selector);
 
@@ -290,6 +339,8 @@ static int pinconf_groups_show(struct seq_file *s, void *what)
 		selector++;
 	}
 
+	mutex_unlock(&pinctrl_mutex);
+
 	return 0;
 }
 
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index db5ed86..77ece74 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>
@@ -93,15 +91,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)) {
@@ -128,11 +123,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",
@@ -175,10 +167,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;
@@ -590,6 +580,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);
@@ -610,9 +602,10 @@ static int pinmux_functions_show(struct seq_file *s, void *what)
 		seq_puts(s, "]\n");
 
 		func_selector++;
-
 	}
 
+	mutex_unlock(&pinctrl_mutex);
+
 	return 0;
 }
 
@@ -624,6 +617,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;
@@ -647,6 +642,8 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 			   owner);
 	}
 
+	mutex_unlock(&pinctrl_mutex);
+
 	return 0;
 }
 
diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
index 411fe23..bbdd7e1 100644
--- a/include/linux/pinctrl/pinctrl.h
+++ b/include/linux/pinctrl/pinctrl.h
@@ -15,7 +15,6 @@
 #ifdef CONFIG_PINCTRL
 
 #include <linux/radix-tree.h>
-#include <linux/spinlock.h>
 #include <linux/list.h>
 #include <linux/seq_file.h>
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V2 2/6] pinctrl: Refactor struct pinctrl handling in core.c vs pinmux.c
  2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
  2012-02-28 20:27 ` [PATCH V2 1/6] pinctrl: Fix and simplify locking Stephen Warren
@ 2012-02-28 20:27 ` Stephen Warren
  2012-02-28 20:27 ` [PATCH V2 3/6] pinctrl: Add usecount to pins for muxing Stephen Warren
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-02-28 20:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel, Stephen Warren

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>
---
v2: Rebased. No logical changes.

 drivers/pinctrl/core.c   |  105 ++++++++-----
 drivers/pinctrl/core.h   |   26 ++-
 drivers/pinctrl/pinmux.c |  409 +++++++++++-----------------------------------
 drivers/pinctrl/pinmux.h |   42 ++---
 4 files changed, 195 insertions(+), 387 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index aefc339..535f8d5 100644
--- a/drivers/pinctrl/core.c
+++ b/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);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index d55cb67..de8df86 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -49,25 +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
- * @func_selector: the function selector for the pinmux device handling
- *	this pinmux
- * @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
+	unsigned group_selector;
 	unsigned func_selector;
-	struct list_head groups;
-#endif
 };
 
 /**
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 77ece74..2318d85 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -7,6 +7,8 @@
  *
  * Author: Linus Walleij <linus.walleij@linaro.org>
  *
+ * Copyright (C) 2012 NVIDIA CORPORATION. All rights reserved.
+ *
  * License terms: GNU General Public License (GPL) version 2
  */
 #define pr_fmt(fmt) "pinmux core: " fmt
@@ -28,16 +30,6 @@
 #include "core.h"
 #include "pinmux.h"
 
-/**
- * struct pinmux_group - group list item for pinmux groups
- * @node: pinmux group list node
- * @group_selector: the group selector for this group
- */
-struct pinmux_group {
-	struct list_head node;
-	unsigned group_selector;
-};
-
 int pinmux_check_ops(struct pinctrl_dev *pctldev)
 {
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
@@ -241,164 +233,8 @@ int pinmux_gpio_direction(struct pinctrl_dev *pctldev,
 	return ret;
 }
 
-/**
- * acquire_pins() - acquire all the pins for a certain function on a pinmux
- * @pctldev: the device to take the pins on
- * @owner: a representation of the owner of this pin; typically the device
- *	name that controls its mux function
- * @group_selector: the group selector containing the pins to acquire
- */
-static int acquire_pins(struct pinctrl_dev *pctldev,
-			const char *owner,
-			unsigned group_selector)
-{
-	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
-	const unsigned *pins;
-	unsigned num_pins;
-	int ret;
-	int i;
-
-	ret = pctlops->get_group_pins(pctldev, group_selector,
-				      &pins, &num_pins);
-	if (ret)
-		return ret;
-
-	dev_dbg(pctldev->dev, "requesting the %u pins from group %u\n",
-		num_pins, group_selector);
-
-	/* Try to allocate all pins in this group, one by one */
-	for (i = 0; i < num_pins; i++) {
-		ret = pin_request(pctldev, pins[i], owner, NULL);
-		if (ret) {
-			dev_err(pctldev->dev,
-				"could not get request pin %d on device %s - conflicting mux mappings?\n",
-				pins[i],
-				pinctrl_dev_get_name(pctldev));
-			/* On error release all taken pins */
-			i--; /* this pin just failed */
-			for (; i >= 0; i--)
-				pin_free(pctldev, pins[i], NULL);
-			return -ENODEV;
-		}
-	}
-	return 0;
-}
-
-/**
- * release_pins() - release pins taken by earlier acquirement
- * @pctldev: the device to free the pins on
- * @group_selector: the group selector containing the pins to free
- */
-static void release_pins(struct pinctrl_dev *pctldev,
-			 unsigned group_selector)
-{
-	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
-	const unsigned *pins;
-	unsigned num_pins;
-	int ret;
-	int i;
-
-	ret = pctlops->get_group_pins(pctldev, group_selector,
-				      &pins, &num_pins);
-	if (ret) {
-		dev_err(pctldev->dev, "could not get pins to release for group selector %d\n",
-			group_selector);
-		return;
-	}
-	for (i = 0; i < num_pins; i++)
-		pin_free(pctldev, pins[i], NULL);
-}
-
-/**
- * pinmux_check_pin_group() - check function and pin group combo
- * @pctldev: device to check the pin group vs function for
- * @func_selector: the function selector to check the pin group for, we have
- *	already looked this up in the calling function
- * @pin_group: the pin group to match to the function
- *
- * This function will check that the pinmux driver can supply the
- * selected pin group for a certain function, returns the group selector if
- * the group and function selector will work fine together, else returns
- * negative
- */
-static int pinmux_check_pin_group(struct pinctrl_dev *pctldev,
-				  unsigned func_selector,
-				  const char *pin_group)
-{
-	const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
-	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
-	int ret;
-
-	/*
-	 * If the driver does not support different pin groups for the
-	 * functions, we only support group 0, and assume this exists.
-	 */
-	if (!pctlops || !pctlops->list_groups)
-		return 0;
-
-	/*
-	 * Passing NULL (no specific group) will select the first and
-	 * hopefully only group of pins available for this function.
-	 */
-	if (!pin_group) {
-		char const * const *groups;
-		unsigned num_groups;
-
-		ret = pmxops->get_function_groups(pctldev, func_selector,
-						  &groups, &num_groups);
-		if (ret)
-			return ret;
-		if (num_groups < 1)
-			return -EINVAL;
-		ret = pinctrl_get_group_selector(pctldev, groups[0]);
-		if (ret < 0) {
-			dev_err(pctldev->dev,
-				"function %s wants group %s but the pin controller does not seem to have that group\n",
-				pmxops->get_function_name(pctldev, func_selector),
-				groups[0]);
-			return ret;
-		}
-
-		if (num_groups > 1)
-			dev_dbg(pctldev->dev,
-				"function %s support more than one group, default-selecting first group %s (%d)\n",
-				pmxops->get_function_name(pctldev, func_selector),
-				groups[0],
-				ret);
-
-		return ret;
-	}
-
-	dev_dbg(pctldev->dev,
-		"check if we have pin group %s on controller %s\n",
-		pin_group, pinctrl_dev_get_name(pctldev));
-
-	ret = pinctrl_get_group_selector(pctldev, pin_group);
-	if (ret < 0) {
-		dev_dbg(pctldev->dev,
-			"%s does not support pin group %s with function %s\n",
-			pinctrl_dev_get_name(pctldev),
-			pin_group,
-			pmxops->get_function_name(pctldev, func_selector));
-	}
-	return ret;
-}
-
-/**
- * pinmux_search_function() - check pin control driver for a certain function
- * @pctldev: device to check for function and position
- * @map: function map containing the function and position to look for
- * @func_selector: returns the applicable function selector if found
- * @group_selector: returns the applicable group selector if found
- *
- * This will search the pinmux driver for an applicable
- * function with a specific pin group, returns 0 if these can be mapped
- * negative otherwise
- */
-static int pinmux_search_function(struct pinctrl_dev *pctldev,
-				  struct pinctrl_map const *map,
-				  unsigned *func_selector,
-				  unsigned *group_selector)
+static int pinmux_func_name_to_selector(struct pinctrl_dev *pctldev,
+					const char *function)
 {
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
 	unsigned selector = 0;
@@ -407,168 +243,128 @@ static int pinmux_search_function(struct pinctrl_dev *pctldev,
 	while (ops->list_functions(pctldev, selector) >= 0) {
 		const char *fname = ops->get_function_name(pctldev,
 							   selector);
-		int ret;
-
-		if (!strcmp(map->function, fname)) {
-			/* Found the function, check pin group */
-			ret = pinmux_check_pin_group(pctldev, selector,
-						     map->group);
-			if (ret < 0)
-				return ret;
 
-			/* This function and group selector can be used */
-			*func_selector = selector;
-			*group_selector = ret;
-			return 0;
+		if (!strcmp(function, fname))
+			return selector;
 
-		}
 		selector++;
 	}
 
 	pr_err("%s does not support function %s\n",
-	       pinctrl_dev_get_name(pctldev), map->function);
+	       pinctrl_dev_get_name(pctldev), function);
 	return -EINVAL;
 }
 
-/**
- * pinmux_enable_muxmap() - enable a map entry for a certain pinmux
- */
-static int pinmux_enable_muxmap(struct pinctrl_dev *pctldev,
-				struct pinctrl *p,
-				struct device *dev,
-				const char *devname,
-				struct pinctrl_map const *map)
+int pinmux_map_to_setting(struct pinctrl_map const *map,
+			  struct pinctrl_setting *setting)
 {
-	unsigned func_selector;
-	unsigned group_selector;
-	struct pinmux_group *grp;
+	struct pinctrl_dev *pctldev = setting->pctldev;
+	const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
+	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
+	char const * const *groups;
+	unsigned num_groups;
 	int ret;
+	const char *group;
+	int i;
+	const unsigned *pins;
+	unsigned num_pins;
 
-	/*
-	 * Note that we're not locking the pinmux mutex here, because
-	 * this is only called at pinmux initialization time when it
-	 * has not been added to any list and thus is not reachable
-	 * by anyone else.
-	 */
+	setting->func_selector =
+		pinmux_func_name_to_selector(pctldev, map->function);
+	if (setting->func_selector < 0)
+		return setting->func_selector;
 
-	if (p->pctldev && p->pctldev != pctldev) {
-		dev_err(pctldev->dev,
-			"different pin control devices given for device %s, function %s\n",
-			devname, map->function);
+	ret = pmxops->get_function_groups(pctldev, setting->func_selector,
+					  &groups, &num_groups);
+	if (ret < 0)
+		return ret;
+	if (!num_groups)
 		return -EINVAL;
+
+	if (map->group) {
+		bool found = false;
+		group = map->group;
+		for (i = 0; i < num_groups; i++) {
+			if (!strcmp(group, groups[i])) {
+				found = true;
+				break;
+			}
+		}
+		if (!found)
+			return -EINVAL;
+	} else {
+		group = groups[0];
 	}
-	p->dev = dev;
-	p->pctldev = pctldev;
 
-	/* Now go into the driver and try to match a function and group */
-	ret = pinmux_search_function(pctldev, map, &func_selector,
-				     &group_selector);
-	if (ret < 0)
-		return ret;
+	setting->group_selector =
+		pinctrl_get_group_selector(pctldev, group);
+	if (setting->group_selector < 0)
+		return setting->group_selector;
 
-	/*
-	 * If the function selector is already set, it needs to be identical,
-	 * we support several groups with one function but not several
-	 * functions with one or several groups in the same pinmux.
-	 */
-	if (p->func_selector != UINT_MAX &&
-	    p->func_selector != func_selector) {
+	ret = pctlops->get_group_pins(pctldev, setting->group_selector,
+				      &pins, &num_pins);
+	if (ret) {
 		dev_err(pctldev->dev,
-			"dual function defines in the map for device %s\n",
-		       devname);
-		return -EINVAL;
+			"could not get pins for device %s group selector %d\n",
+			pinctrl_dev_get_name(pctldev), setting->group_selector);
+			return -ENODEV;
 	}
-	p->func_selector = func_selector;
-
-	/* Now add this group selector, we may have many of them */
-	grp = kmalloc(sizeof(*grp), GFP_KERNEL);
-	if (!grp)
-		return -ENOMEM;
-	grp->group_selector = group_selector;
-	ret = acquire_pins(pctldev, devname, group_selector);
-	if (ret) {
-		kfree(grp);
-		return ret;
+
+	/* Try to allocate all pins in this group, one by one */
+	for (i = 0; i < num_pins; i++) {
+		ret = pin_request(pctldev, pins[i], map->dev_name, NULL);
+		if (ret) {
+			dev_err(pctldev->dev,
+				"could not get request pin %d on device %s\n",
+				pins[i], pinctrl_dev_get_name(pctldev));
+			/* On error release all taken pins */
+			i--; /* this pin just failed */
+			for (; i >= 0; i--)
+				pin_free(pctldev, pins[i], NULL);
+			return -ENODEV;
+		}
 	}
-	list_add_tail(&grp->node, &p->groups);
 
 	return 0;
 }
 
-/**
- * pinmux_apply_muxmap() - apply a certain mux mapping entry
- */
-int pinmux_apply_muxmap(struct pinctrl_dev *pctldev,
-			struct pinctrl *p,
-			struct device *dev,
-			const char *devname,
-			struct pinctrl_map const *map)
+void pinmux_free_setting(struct pinctrl_setting const *setting)
 {
+	struct pinctrl_dev *pctldev = setting->pctldev;
+	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
+	const unsigned *pins;
+	unsigned num_pins;
 	int ret;
+	int i;
 
-	ret = pinmux_enable_muxmap(pctldev, p, dev,
-				   devname, map);
+	ret = pctlops->get_group_pins(pctldev, setting->group_selector,
+				      &pins, &num_pins);
 	if (ret) {
-		pinmux_put(p);
-		return ret;
+		dev_err(pctldev->dev,
+			"could not get pins for device %s group selector %d\n",
+			pinctrl_dev_get_name(pctldev), setting->group_selector);
+		return;
 	}
 
-	return 0;
-}
-
-/**
- * pinmux_put() - free up the pinmux portions of a pin controller handle
- */
-void pinmux_put(struct pinctrl *p)
-{
-	struct list_head *node, *tmp;
-
-	list_for_each_safe(node, tmp, &p->groups) {
-		struct pinmux_group *grp =
-			list_entry(node, struct pinmux_group, node);
-		/* Release all pins taken by this group */
-		release_pins(p->pctldev, grp->group_selector);
-		list_del(node);
-		kfree(grp);
-	}
+	for (i = 0; i < num_pins; i++)
+		pin_free(pctldev, pins[i], NULL);
 }
 
-/**
- * pinmux_enable() - enable the pinmux portion of a pin control handle
- */
-int pinmux_enable(struct pinctrl *p)
+int pinmux_enable_setting(struct pinctrl_setting const *setting)
 {
-	struct pinctrl_dev *pctldev = p->pctldev;
+	struct pinctrl_dev *pctldev = setting->pctldev;
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
-	struct pinmux_group *grp;
-	int ret;
 
-	list_for_each_entry(grp, &p->groups, node) {
-		ret = ops->enable(pctldev, p->func_selector,
-				  grp->group_selector);
-		if (ret)
-			/*
-			 * TODO: call disable() on all groups we called
-			 * enable() on to this point?
-			 */
-			return ret;
-	}
-	return 0;
+	return ops->enable(pctldev, setting->func_selector,
+			   setting->group_selector);
 }
 
-/**
- * pinmux_disable() - disable the pinmux portions of a pin control handle
- */
-void pinmux_disable(struct pinctrl *p)
+void pinmux_disable_setting(struct pinctrl_setting const *setting)
 {
-	struct pinctrl_dev *pctldev = p->pctldev;
+	struct pinctrl_dev *pctldev = setting->pctldev;
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
-	struct pinmux_group *grp;
 
-	list_for_each_entry(grp, &p->groups, node) {
-		ops->disable(pctldev, p->func_selector,
-			     grp->group_selector);
-	}
+	ops->disable(pctldev, setting->func_selector, setting->group_selector);
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -647,29 +443,18 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 	return 0;
 }
 
-void pinmux_dbg_show(struct seq_file *s, struct pinctrl *p)
+void pinmux_dbg_show(struct seq_file *s, struct pinctrl_setting const *setting)
 {
-	struct pinctrl_dev *pctldev = p->pctldev;
-	const struct pinmux_ops *pmxops;
-	const struct pinctrl_ops *pctlops;
-	struct pinmux_group *grp;
-
-	pmxops = pctldev->desc->pmxops;
-	pctlops = pctldev->desc->pctlops;
-
-	seq_printf(s, " function: %s (%u),",
-		   pmxops->get_function_name(pctldev,
-					     p->func_selector),
-		   p->func_selector);
-
-	seq_printf(s, " groups: [");
-	list_for_each_entry(grp, &p->groups, node) {
-		seq_printf(s, " %s (%u)",
-			   pctlops->get_group_name(pctldev,
-						   grp->group_selector),
-			   grp->group_selector);
-	}
-	seq_printf(s, " ]");
+	struct pinctrl_dev *pctldev = setting->pctldev;
+	const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
+	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
+
+	seq_printf(s, "controller: %s group: %s (%u) function: %s (%u)\n",
+		   pinctrl_dev_get_name(pctldev),
+		   pctlops->get_group_name(pctldev, setting->group_selector),
+		   setting->group_selector,
+		   pmxops->get_function_name(pctldev, setting->func_selector),
+		   setting->func_selector);
 }
 
 static int pinmux_functions_open(struct inode *inode, struct file *file)
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 84b8fe9..1500ae8 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/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,22 +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)
-{
-	p->func_selector = UINT_MAX;
-	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
 
@@ -65,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)
 {
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V2 3/6] pinctrl: Add usecount to pins for muxing
  2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
  2012-02-28 20:27 ` [PATCH V2 1/6] pinctrl: Fix and simplify locking Stephen Warren
  2012-02-28 20:27 ` [PATCH V2 2/6] pinctrl: Refactor struct pinctrl handling in core.c vs pinmux.c Stephen Warren
@ 2012-02-28 20:27 ` Stephen Warren
  2012-02-28 20:27 ` [PATCH V2 4/6] pinctrl: API changes to support multiple states per device Stephen Warren
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-02-28 20:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel, Stephen Warren

Multiple mapping table entries could reference the same pin, and hence
"own" it. This would be unusual now that pinctrl_get() represents a single
state for a client device, but in the future when it represents all known
states for a device, this is quite likely. Implement reference counting
for pin ownership to handle this.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
---
v2: Added kerneldoc for @usecount and @owner fields - the latter was missing
from a previous patch.

 drivers/pinctrl/core.h   |   12 +++++++++---
 drivers/pinctrl/pinmux.c |   23 +++++++++++++++++++----
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index de8df86..0bc52ec 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -82,9 +82,14 @@ struct pinctrl_setting {
  * @name: a name for the pin, e.g. the name of the pin/pad/finger on a
  *	datasheet or such
  * @dynamic_name: if the name of this pin was dynamically allocated
- * @mux_requested: whether the pin is already requested by pinmux or not
- * @mux_function: a named muxing function for the pin that will be passed to
- *	subdrivers and shown in debugfs etc
+ * @usecount: If zero, the pin is not claimed, and @owner should be NULL.
+ *	If non-zero, this pin is claimed by @owner. This field is an integer
+ *	rather than a boolean, since pinctrl_get() might process multiple
+ *	mapping table entries that refer to, and hence claim, the same group
+ *	or pin, and each of these will increment the @usecount.
+ * @owner: The name of the entity owning the pin. Typically, this is the name
+ *	of the device that called pinctrl_get(). Alternatively, it may be the
+ *	name of the GPIO passed to pinctrl_request_gpio().
  */
 struct pin_desc {
 	struct pinctrl_dev *pctldev;
@@ -92,6 +97,7 @@ struct pin_desc {
 	bool dynamic_name;
 	/* These fields only added when supporting pinmux drivers */
 #ifdef CONFIG_PINMUX
+	unsigned usecount;
 	const char *owner;
 #endif
 };
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 2318d85..2d1aaf8 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -83,11 +83,16 @@ static int pin_request(struct pinctrl_dev *pctldev,
 		goto out;
 	}
 
-	if (desc->owner && strcmp(desc->owner, owner)) {
+	if (desc->usecount && strcmp(desc->owner, owner)) {
 		dev_err(pctldev->dev,
 			"pin already requested\n");
 		goto out;
 	}
+
+	desc->usecount++;
+	if (desc->usecount > 1)
+		return 0;
+
 	desc->owner = owner;
 
 	/* Let each pin increase references to this module */
@@ -111,12 +116,18 @@ static int pin_request(struct pinctrl_dev *pctldev,
 	else
 		status = 0;
 
-	if (status)
+	if (status) {
 		dev_err(pctldev->dev, "->request on device %s failed for pin %d\n",
 		       pctldev->desc->name, pin);
+		module_put(pctldev->owner);
+	}
+
 out_free_pin:
-	if (status)
-		desc->owner = NULL;
+	if (status) {
+		desc->usecount--;
+		if (!desc->usecount)
+			desc->owner = NULL;
+	}
 out:
 	if (status)
 		dev_err(pctldev->dev, "pin-%d (%s) status %d\n",
@@ -150,6 +161,10 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
 		return NULL;
 	}
 
+	desc->usecount--;
+	if (desc->usecount)
+		return NULL;
+
 	/*
 	 * If there is no kind of request function for the pin we just assume
 	 * we got it by default and proceed.
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V2 4/6] pinctrl: API changes to support multiple states per device
  2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
                   ` (2 preceding siblings ...)
  2012-02-28 20:27 ` [PATCH V2 3/6] pinctrl: Add usecount to pins for muxing Stephen Warren
@ 2012-02-28 20:27 ` Stephen Warren
  2012-02-29  6:46   ` Dong Aisheng
  2012-02-28 20:27 ` [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations Stephen Warren
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Stephen Warren @ 2012-02-28 20:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel, Stephen Warren

The API model is changed from:

p = pinctrl_get(dev, "state1");
pinctrl_enable(p);
...
pinctrl_disable(p);
pinctrl_put(p);
p = pinctrl_get(dev, "state2");
pinctrl_enable(p);
...
pinctrl_disable(p);
pinctrl_put(p);

to this:

p = pinctrl_get(dev);
s1 = pinctrl_lookup_state(p, "state1");
s2 = pinctrl_lookup_state(p, "state2");
pinctrl_select_state(p, s1);
...
pinctrl_select_state(p, s2);
...
pinctrl_put(p);

This allows devices to directly transition between states without
disabling the pin controller programming and put()/get()ing the
configuration data each time. This model will also better suit pinconf
programming, which doesn't have a concept of "disable".

The special-case hogging feature of pin controllers is re-written to use
the regular APIs instead of special-case code. Hence, the pinmux-hogs
debugfs file is removed; see the top-level pinctrl-handles files for
equivalent data.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: Make use of PINCTRL_STATE_DEFAULT, split out some documentation
cleanup into an earlier patch. Various minor fixes. Fixes due to
rebasing on updated earlier patches. Remove usecount field from struct
pinctrl; allow only one concurrent pinctrl_get() per device.

 Documentation/pinctrl.txt         |  120 ++++++++----
 arch/arm/mach-u300/core.c         |   16 +--
 drivers/pinctrl/core.c            |  363 ++++++++++++++++++++++---------------
 drivers/pinctrl/core.h            |   23 ++-
 drivers/tty/serial/sirfsoc_uart.c |   12 +-
 include/linux/pinctrl/consumer.h  |   55 +++++-
 include/linux/pinctrl/machine.h   |   11 +-
 7 files changed, 375 insertions(+), 225 deletions(-)

diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
index 558aac5..23426c7 100644
--- a/Documentation/pinctrl.txt
+++ b/Documentation/pinctrl.txt
@@ -847,8 +847,8 @@ As it is possible to map a function to different groups of pins an optional
 This example mapping is used to switch between two positions for spi0 at
 runtime, as described further below under the heading "Runtime pinmuxing".
 
-Further it is possible to match several groups of pins to the same function
-for a single device, say for example in the mmc0 example above, where you can
+Further it is possible for one named state to affect the muxing of several
+groups of pins, say for example in the mmc0 example above, where you can
 additively expand the mmc0 bus from 2 to 4 to 8 pins. If we want to use all
 three groups for a total of 2+2+4 = 8 pins (for an 8-bit MMC bus as is the
 case), we define a mapping like this:
@@ -879,6 +879,7 @@ case), we define a mapping like this:
 	.dev_name = "foo-mmc.0",
 	.name = "8bit"
 	.ctrl_dev_name = "pinctrl-foo",
+	.function = "mmc0",
 	.group = "mmc0_1_grp",
 },
 {
@@ -900,10 +901,16 @@ case), we define a mapping like this:
 The result of grabbing this mapping from the device with something like
 this (see next paragraph):
 
-	p = pinctrl_get(&device, "8bit");
+	p = pinctrl_get(dev);
+	s = pinctrl_lookup_state(p, "8bit");
+	ret = pinctrl_select_state(p, s);
+
+or more simply:
+
+	p = pinctrl_get_select(dev, "8bit");
 
 Will be that you activate all the three bottom records in the mapping at
-once. Since they share the same name, pin controller device, funcion and
+once. Since they share the same name, pin controller device, function and
 device, and since we allow multiple groups to match to a single device, they
 all get selected, and they all get enabled and disable simultaneously by the
 pinmux core.
@@ -925,45 +932,63 @@ default state like this:
 
 struct foo_state {
        struct pinctrl *p;
+       struct pinctrl_state *s;
        ...
 };
 
 foo_probe()
 {
-	/* Allocate a state holder named "state" etc */
-	struct pinctrl p;
+	/* Allocate a state holder named "foo" etc */
+	struct foo_state *foo = ...;
+
+	foo->p = pinctrl_get(&device);
+	if (IS_ERR(foo->p)) {
+		/* FIXME: clean up "foo" here */
+		return PTR_ERR(foo->p);
+	}
 
-	p = pinctrl_get(&device, PINCTRL_STATE_DEFAULT);
-	if IS_ERR(p)
-		return PTR_ERR(p);
-	pinctrl_enable(p);
+	foo->s = pinctrl_lookup_state(foo->p, PINCTRL_STATE_DEFAULT);
+	if (IS_ERR(foo->s)) {
+		pinctrl_put(foo->p);
+		/* FIXME: clean up "foo" here */
+		return PTR_ERR(s);
+	}
 
-	state->p = p;
+	ret = pinctrl_select_state(foo->s);
+	if (ret < 0) {
+		pinctrl_put(foo->p);
+		/* FIXME: clean up "foo" here */
+		return ret;
+	}
 }
 
 foo_remove()
 {
-	pinctrl_disable(state->p);
 	pinctrl_put(state->p);
 }
 
-This get/enable/disable/put sequence can just as well be handled by bus drivers
+This get/lookup/select/put sequence can just as well be handled by bus drivers
 if you don't want each and every driver to handle it and you know the
 arrangement on your bus.
 
-The semantics of the get/enable respective disable/put is as follows:
+The semantics of the pinctrl APIs are:
+
+- pinctrl_get() is called in process context to obtain a handle to all pinctrl
+  information for a given client device. It will allocate a struct from the
+  kernel memory to hold the pinmux state. All mapping table parsing or similar
+  slow operations take place within this API.
 
-- pinctrl_get() is called in process context to reserve the pins affected with
-  a certain mapping and set up the pinmux core and the driver. It will allocate
-  a struct from the kernel memory to hold the pinmux state.
+- pinctrl_lookup_state() is called in process context to obtain a handle to a
+  specific state for a the client device. This operation may be slow too.
 
-- pinctrl_enable()/pinctrl_disable() is quick and can be called from fastpath
-  (irq context) when you quickly want to set up/tear down the hardware muxing
-  when running a device driver. Usually it will just poke some values into a
-  register.
+- pinctrl_select_state() programs pin controller hardware according to the
+  definition of the state as given by the mapping table. In theory this is a
+  fast-path operation, since it only involved blasting some register settings
+  into hardware. However, note that some pin controllers may have their
+  registers on a slow/IRQ-based bus, so client devices should not assume they
+  can call pinctrl_select_state() from non-blocking contexts.
 
-- pinctrl_disable() is called in process context to tear down the pin requests
-  and release the state holder struct for the mux setting etc.
+- pinctrl_put() frees all information associated with a pinctrl handle.
 
 Usually the pin control core handled the get/put pair and call out to the
 device drivers bookkeeping operations, like checking available functions and
@@ -979,12 +1004,12 @@ System pin control hogging
 ==========================
 
 Pin control map entries can be hogged by the core when the pin controller
-is registered. This means that the core will attempt to call pinctrl_get() and
-pinctrl_enable() on it immediately after the pin control device has been
-registered.
+is registered. This means that the core will attempt to call pinctrl_get(),
+lookup_state() and select_state() on it immediately after the pin control
+device has been registered.
 
-This is enabled by simply setting the .dev_name field in the map to the name
-of the pin controller itself, like this:
+This occurs for mapping table entries where the client device name is equal
+to the pin controller device name, and the state name is PINCTRL_STATE_DEFAULT.
 
 {
 	.dev_name = "pinctrl-foo",
@@ -1009,8 +1034,8 @@ It is possible to mux a certain function in and out at runtime, say to move
 an SPI port from one set of pins to another set of pins. Say for example for
 spi0 in the example above, we expose two different groups of pins for the same
 function, but with different named in the mapping as described under
-"Advanced mapping" above. So we have two mappings named "spi0-pos-A" and
-"spi0-pos-B".
+"Advanced mapping" above. So that for an SPI device, we have two states named
+"pos-A" and "pos-B".
 
 This snippet first muxes the function in the pins defined by group A, enables
 it, disables and releases it, and muxes it in on the pins defined by group B:
@@ -1020,23 +1045,36 @@ it, disables and releases it, and muxes it in on the pins defined by group B:
 foo_switch()
 {
 	struct pinctrl *p;
+	struct pinctrl_state *s1, *s2;
+
+	/* Setup */
+	p = pinctrl_get(&device);
+	if (IS_ERR(p))
+		...
+
+	s1 = pinctrl_lookup_state(foo->p, "pos-A");
+	if (IS_ERR(s1))
+		...
+
+	s2 = pinctrl_lookup_state(foo->p, "pos-B");
+	if (IS_ERR(s2))
+		...
 
 	/* Enable on position A */
-	p = pinctrl_get(&device, "spi0-pos-A");
-	if IS_ERR(p)
-		return PTR_ERR(p);
-	pinctrl_enable(p);
+	ret = pinctrl_select_state(s1);
+	if (ret < 0)
+	    ...
 
-	/* This releases the pins again */
-	pinctrl_disable(p);
-	pinctrl_put(p);
+	...
 
 	/* Enable on position B */
-	p = pinctrl_get(&device, "spi0-pos-B");
-	if IS_ERR(p)
-		return PTR_ERR(p);
-	pinctrl_enable(p);
+	ret = pinctrl_select_state(s2);
+	if (ret < 0)
+	    ...
+
 	...
+
+	pinctrl_put(p);
 }
 
 The above has to be done from process context.
diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
index 5b37d84..18973ca 100644
--- a/arch/arm/mach-u300/core.c
+++ b/arch/arm/mach-u300/core.c
@@ -1579,22 +1579,18 @@ static struct pinctrl_map __initdata u300_pinmux_map[] = {
 };
 
 struct u300_mux_hog {
-	const char *name;
 	struct device *dev;
 	struct pinctrl *p;
 };
 
 static struct u300_mux_hog u300_mux_hogs[] = {
 	{
-		.name = "uart0",
 		.dev = &uart0_device.dev,
 	},
 	{
-		.name = "spi0",
 		.dev = &pl022_device.dev,
 	},
 	{
-		.name = "mmc0",
 		.dev = &mmcsd_device.dev,
 	},
 };
@@ -1607,16 +1603,10 @@ static int __init u300_pinctrl_fetch(void)
 		struct pinctrl *p;
 		int ret;
 
-		p = pinctrl_get(u300_mux_hogs[i].dev, PINCTRL_STATE_DEFAULT);
+		p = pinctrl_get_select_default(u300_mux_hogs[i].dev);
 		if (IS_ERR(p)) {
-			pr_err("u300: could not get pinmux hog %s\n",
-			       u300_mux_hogs[i].name);
-			continue;
-		}
-		ret = pinctrl_enable(p);
-		if (ret) {
-			pr_err("u300: could enable pinmux hog %s\n",
-			       u300_mux_hogs[i].name);
+			pr_err("u300: could not get pinmux hog for dev %s\n",
+			       dev_name(u300_mux_hogs[i].dev));
 			continue;
 		}
 		u300_mux_hogs[i].p = p;
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 535f8d5..c6f3ca3 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -458,25 +458,98 @@ int pinctrl_gpio_direction_output(unsigned gpio)
 }
 EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output);
 
-static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
+static struct pinctrl_state *find_state(struct pinctrl *p,
+					const char *name)
 {
-	struct pinctrl_dev *pctldev;
-	const char *devname;
-	struct pinctrl *p;
-	unsigned num_maps = 0;
-	int ret;
-	struct pinctrl_maps *maps_node;
-	int i;
-	struct pinctrl_map const *map;
+	struct pinctrl_state *state;
+
+	list_for_each_entry(state, &p->states, node)
+		if (!strcmp(state->name, name))
+			return state;
+
+	return NULL;
+}
+
+static struct pinctrl_state *create_state(struct pinctrl *p,
+					  const char *name)
+{
+	struct pinctrl_state *state;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (state == NULL) {
+		dev_err(p->dev,
+			"failed to alloc struct pinctrl_state\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	state->name = name;
+	INIT_LIST_HEAD(&state->settings);
+
+	list_add_tail(&state->node, &p->states);
+
+	return state;
+}
+
+static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
+{
+	struct pinctrl_state *state;
 	struct pinctrl_setting *setting;
+	int ret;
 
-	/* We must have both a dev and state name */
-	if (WARN_ON(!dev || !name))
-		return ERR_PTR(-EINVAL);
+	state = find_state(p, map->name);
+	if (!state)
+		state = create_state(p, map->name);
+	if (IS_ERR(state))
+		return PTR_ERR(state);
 
-	devname = dev_name(dev);
+	setting = kzalloc(sizeof(*setting), GFP_KERNEL);
+	if (setting == NULL) {
+		dev_err(p->dev,
+			"failed to alloc struct pinctrl_setting\n");
+		return -ENOMEM;
+	}
 
-	dev_dbg(dev, "pinctrl_get() for device %s state %s\n", devname, name);
+	setting->pctldev = get_pinctrl_dev_from_devname(map->ctrl_dev_name);
+	if (setting->pctldev == NULL) {
+		dev_err(p->dev, "unknown pinctrl device %s in map entry",
+			map->ctrl_dev_name);
+		kfree(setting);
+		/* Eventually, this should trigger deferred probe */
+		return -ENODEV;
+	}
+
+	ret = pinmux_map_to_setting(map, setting);
+	if (ret < 0) {
+		kfree(setting);
+		return ret;
+	}
+
+	list_add_tail(&setting->node, &state->settings);
+
+	return 0;
+}
+
+static struct pinctrl *find_pinctrl(struct device *dev)
+{
+	struct pinctrl *p;
+
+	list_for_each_entry(p, &pinctrldev_list, node)
+		if (p->dev == dev)
+			return p;
+
+	return NULL;
+}
+
+static void pinctrl_put_locked(struct pinctrl *p, bool inlist);
+
+static struct pinctrl *create_pinctrl(struct device *dev)
+{
+	struct pinctrl *p;
+	const char *devname;
+	struct pinctrl_maps *maps_node;
+	int i;
+	struct pinctrl_map const *map;
+	int ret;
 
 	/*
 	 * create the state cookie holder struct pinctrl for each
@@ -489,8 +562,9 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
 		return ERR_PTR(-ENOMEM);
 	}
 	p->dev = dev;
-	p->state = name;
-	INIT_LIST_HEAD(&p->settings);
+	INIT_LIST_HEAD(&p->states);
+
+	devname = dev_name(dev);
 
 	/* Iterate over the pin control maps to locate the right ones */
 	for_each_maps(maps_node, i, map) {
@@ -498,183 +572,179 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
 		if (strcmp(map->dev_name, devname))
 			continue;
 
-		/* State name must be the one we're looking for */
-		if (strcmp(map->name, name))
-			continue;
-
-		/*
-		 * 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);
-			/* Eventually, this should trigger deferred probe */
-			ret = -ENODEV;
-			goto error;
-		}
-
-		dev_dbg(dev, "in map, found pctldev %s to handle function %s",
-			dev_name(pctldev->dev), map->function);
-
-		setting = kzalloc(sizeof(*setting), GFP_KERNEL);
-		if (setting == NULL) {
-			dev_err(dev,
-				"failed to alloc struct pinctrl_setting\n");
-			ret = -ENOMEM;
-			goto error;
+		ret = add_setting(p, map);
+		if (ret < 0) {
+			pinctrl_put_locked(p, false);
+			return ERR_PTR(ret);
 		}
-
-		setting->pctldev = pctldev;
-		ret = pinmux_map_to_setting(map, setting);
-		if (ret < 0)
-			goto error;
-
-		list_add_tail(&setting->node, &p->settings);
-
-		num_maps++;
 	}
 
-	/*
-	 * This may be perfectly legitimate. An IP block may get re-used
-	 * across SoCs. Not all of those SoCs may need pinmux settings for the
-	 * IP block, e.g. if one SoC dedicates pins to that function but
-	 * another doesn't. The driver won't know this, and will always
-	 * attempt to set up the pinmux. The mapping table defines whether any
-	 * HW programming is actually needed.
-	 */
-	if (!num_maps)
-		dev_info(dev, "zero maps found for mapping %s\n", name);
-
-	dev_dbg(dev, "found %u maps for device %s state %s\n",
-		num_maps, devname, name ? name : "(undefined)");
-
 	/* Add the pinmux to the global list */
 	list_add_tail(&p->node, &pinctrl_list);
 
 	return p;
+}
 
-error:
-	list_for_each_entry(setting, &p->settings, node)
-		pinmux_free_setting(setting);
+static struct pinctrl *pinctrl_get_locked(struct device *dev)
+{
+	struct pinctrl *p;
 
-	kfree(p);
+	if (WARN_ON(!dev))
+		return ERR_PTR(-EINVAL);
+
+	p = find_pinctrl(dev);
+	if (p != NULL)
+		return ERR_PTR(-EBUSY);
 
-	return ERR_PTR(ret);
+	p = create_pinctrl(dev);
+	if (IS_ERR(p))
+		return p;
+
+	return p;
 }
 
 /**
- * pinctrl_get() - retrieves the pin controller handle for a certain device
- * @dev: the device to get the pin controller handle for
- * @name: an optional specific control mapping name or NULL, the name is only
- *	needed if you want to have more than one mapping per device, or if you
- *	need an anonymous pin control (not tied to any specific device)
+ * pinctrl_get() - retrieves the pinctrl handle for a device
+ * @dev: the device to obtain the handle for
  */
-struct pinctrl *pinctrl_get(struct device *dev, const char *name)
+struct pinctrl *pinctrl_get(struct device *dev)
 {
 	struct pinctrl *p;
 
 	mutex_lock(&pinctrl_mutex);
-	p = pinctrl_get_locked(dev, name);
+	p = pinctrl_get_locked(dev);
 	mutex_unlock(&pinctrl_mutex);
 
 	return p;
 }
 EXPORT_SYMBOL_GPL(pinctrl_get);
 
-static void pinctrl_put_locked(struct pinctrl *p)
+static void pinctrl_put_locked(struct pinctrl *p, bool inlist)
 {
-	struct pinctrl_setting *setting, *n;
-
-	if (p == NULL)
-		return;
-
-	if (p->usecount)
-		pr_warn("releasing pin control handle with active users!\n");
-	list_for_each_entry_safe(setting, n, &p->settings, node) {
-		pinmux_free_setting(setting);
-		list_del(&setting->node);
-		kfree(setting);
+	struct pinctrl_state *state, *n1;
+	struct pinctrl_setting *setting, *n2;
+
+	list_for_each_entry_safe(state, n1, &p->states, node) {
+		list_for_each_entry_safe(setting, n2, &state->settings, node) {
+			if (state == p->state)
+				pinmux_disable_setting(setting);
+			pinmux_free_setting(setting);
+			list_del(&setting->node);
+			kfree(setting);
+		}
+		list_del(&state->node);
+		kfree(state);
 	}
 
-	/* Remove from list */
-	list_del(&p->node);
-
+	if (inlist)
+		list_del(&p->node);
 	kfree(p);
 }
 
 /**
- * pinctrl_put() - release a previously claimed pin control handle
- * @p: a pin control handle previously claimed by pinctrl_get()
+ * pinctrl_put() - release a previously claimed pinctrl handle
+ * @p: the pinctrl handle to release
  */
 void pinctrl_put(struct pinctrl *p)
 {
 	mutex_lock(&pinctrl_mutex);
-	pinctrl_put(p);
+	pinctrl_put_locked(p, true);
 	mutex_unlock(&pinctrl_mutex);
 }
 EXPORT_SYMBOL_GPL(pinctrl_put);
 
-static int pinctrl_enable_locked(struct pinctrl *p)
+static struct pinctrl_state *pinctrl_lookup_state_locked(struct pinctrl *p,
+							 const char *name)
 {
-	struct pinctrl_setting *setting;
-	int ret;
+	struct pinctrl_state *state;
 
-	if (p == NULL)
-		return -EINVAL;
+	state = find_state(p, name);
+	if (!state)
+		return ERR_PTR(-ENODEV);
 
-	if (p->usecount++ == 0) {
-		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 0;
+	return state;
 }
 
 /**
- * pinctrl_enable() - enable a certain pin controller setting
- * @p: the pin control handle to enable, previously claimed by pinctrl_get()
+ * pinctrl_lookup_state() - retrieves a state handle from a pinctrl handle
+ * @p: the pinctrl handle to retrieve the state from
+ * @name: the state name to retrieve
  */
-int pinctrl_enable(struct pinctrl *p)
+struct pinctrl_state *pinctrl_lookup_state(struct pinctrl *p, const char *name)
 {
-	int ret;
+	struct pinctrl_state *s;
+
 	mutex_lock(&pinctrl_mutex);
-	ret = pinctrl_enable_locked(p);
+	s = pinctrl_lookup_state_locked(p, name);
 	mutex_unlock(&pinctrl_mutex);
-	return ret;
+
+	return s;
 }
-EXPORT_SYMBOL_GPL(pinctrl_enable);
+EXPORT_SYMBOL_GPL(pinctrl_lookup_state);
 
-static void pinctrl_disable_locked(struct pinctrl *p)
+static int pinctrl_select_state_locked(struct pinctrl *p,
+				       struct pinctrl_state *state)
 {
-	struct pinctrl_setting *setting;
+	struct pinctrl_setting *setting, *setting2;
+	int ret;
 
-	if (p == NULL)
-		return;
+	if (p->state == state)
+		return 0;
 
-	if (--p->usecount == 0) {
-		list_for_each_entry(setting, &p->settings, node)
-			pinmux_disable_setting(setting);
+	if (p->state) {
+		/*
+		 * The set of groups with a mux configuration in the old state
+		 * may not be identical to the set of groups with a mux setting
+		 * in the new state. While this might be unusual, it's entirely
+		 * possible for the "user"-supplied mapping table to be written
+		 * that way. For each group that was configured in the old state
+		 * but not in the new state, this code puts that group into a
+		 * safe/disabled state.
+		 */
+		list_for_each_entry(setting, &p->state->settings, node) {
+			bool found = false;
+			list_for_each_entry(setting2, &state->settings, node) {
+				if (setting2->group_selector ==
+						setting->group_selector) {
+					found = true;
+					break;
+				}
+			}
+			if (!found)
+				pinmux_disable_setting(setting);
+		}
+	}
+
+	p->state = state;
+
+	/* Apply all the settings for the new state */
+	list_for_each_entry(setting, &state->settings, node) {
+		ret = pinmux_enable_setting(setting);
+		if (ret < 0) {
+			/* FIXME: Difficult to return to prev state */
+			return ret;
+		}
 	}
+
+	return 0;
 }
 
 /**
- * pinctrl_disable() - disable a certain pin control setting
- * @p: the pin control handle to disable, previously claimed by pinctrl_get()
+ * pinctrl_select() - select/activate/program a pinctrl state to HW
+ * @p: the pinctrl handle for the device that requests configuratio
+ * @state: the state handle to select/activate/program
  */
-void pinctrl_disable(struct pinctrl *p)
+int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
 {
+	int ret;
+
 	mutex_lock(&pinctrl_mutex);
-	pinctrl_disable_locked(p);
+	ret = pinctrl_select_state_locked(p, state);
 	mutex_unlock(&pinctrl_mutex);
+
+	return ret;
 }
-EXPORT_SYMBOL_GPL(pinctrl_disable);
+EXPORT_SYMBOL_GPL(pinctrl_select_state);
 
 /**
  * pinctrl_register_mappings() - register a set of pin controller mappings
@@ -891,6 +961,7 @@ 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_state *state;
 	struct pinctrl_setting *setting;
 
 	seq_puts(s, "Requested pin control handlers their pinmux maps:\n");
@@ -898,12 +969,17 @@ static int pinctrl_show(struct seq_file *s, void *what)
 	mutex_lock(&pinctrl_mutex);
 
 	list_for_each_entry(p, &pinctrl_list, node) {
-		seq_printf(s, "device: %s state: %s users: %u\n",
-			   dev_name(p->dev), p->state, p->usecount);
+		seq_printf(s, "device: %s current state: %s\n",
+			   dev_name(p->dev),
+			   p->state ? p->state->name : "none");
+
+		list_for_each_entry(state, &p->states, node) {
+			seq_printf(s, "  state: %s\n", state->name);
 
-		list_for_each_entry(setting, &p->settings, node) {
-			seq_printf(s, "  ");
-			pinmux_dbg_show(s, setting);
+			list_for_each_entry(setting, &state->settings, node) {
+				seq_printf(s, "    ");
+				pinmux_dbg_show(s, setting);
+			}
 		}
 	}
 
@@ -1113,9 +1189,14 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
 
 	list_add_tail(&pctldev->node, &pinctrldev_list);
 
-	pctldev->p = pinctrl_get_locked(pctldev->dev, PINCTRL_STATE_DEFAULT);
-	if (!IS_ERR(pctldev->p))
-		pinctrl_enable_locked(pctldev->p);
+	pctldev->p = pinctrl_get_locked(pctldev->dev);
+	if (!IS_ERR(pctldev->p)) {
+		struct pinctrl_state *s =
+			pinctrl_lookup_state_locked(pctldev->p,
+						    PINCTRL_STATE_DEFAULT);
+		if (!IS_ERR(s))
+			pinctrl_select_state_locked(pctldev->p, s);
+	}
 
 	mutex_unlock(&pinctrl_mutex);
 
@@ -1144,10 +1225,8 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
 
 	mutex_lock(&pinctrl_mutex);
 
-	if (!IS_ERR(pctldev->p)) {
-		pinctrl_disable_locked(pctldev->p);
-		pinctrl_put_locked(pctldev->p);
-	}
+	if (!IS_ERR(pctldev->p))
+		pinctrl_put_locked(pctldev->p, true);
 
 	/* TODO: check that no pinmuxes are still active? */
 	list_del(&pctldev->node);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 0bc52ec..5691d31 100644
--- a/drivers/pinctrl/core.h
+++ b/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
- * @settings: a list of settings for this device/state
+ * @states: a list of states for this device
+ * @state: the current state
  */
 struct pinctrl {
 	struct list_head node;
 	struct device *dev;
-	const char *state;
-	unsigned usecount;
+	struct list_head states;
+	struct pinctrl_state *state;
+};
+
+/**
+ * struct pinctrl_state - a pinctrl state for a device
+ * @node: list not for struct pinctrl's @states field
+ * @name: the name of this state
+ * @settings: a list of settings for this state
+ */
+struct pinctrl_state {
+	struct list_head node;
+	const char *name;
 	struct list_head settings;
 };
 
 /**
  * struct pinctrl_setting - an individual mux setting
- * @node: list node for struct pinctrl's @settings field
+ * @node: list node for struct pinctrl_settings'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
diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 3cabb65..5b3eda2 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -673,12 +673,10 @@ int sirfsoc_uart_probe(struct platform_device *pdev)
 	port->irq = res->start;
 
 	if (sirfport->hw_flow_ctrl) {
-		sirfport->p = pinctrl_get(&pdev->dev, PINCTRL_STATE_DEFAULT);
+		sirfport->p = pinctrl_get_select_default(&pdev->dev);
 		ret = IS_ERR(sirfport->p);
 		if (ret)
 			goto pin_err;
-
-		pinctrl_enable(sirfport->p);
 	}
 
 	port->ops = &sirfsoc_uart_ops;
@@ -695,10 +693,8 @@ int sirfsoc_uart_probe(struct platform_device *pdev)
 
 port_err:
 	platform_set_drvdata(pdev, NULL);
-	if (sirfport->hw_flow_ctrl) {
-		pinctrl_disable(sirfport->p);
+	if (sirfport->hw_flow_ctrl)
 		pinctrl_put(sirfport->p);
-	}
 pin_err:
 irq_err:
 	devm_iounmap(&pdev->dev, port->membase);
@@ -711,10 +707,8 @@ static int sirfsoc_uart_remove(struct platform_device *pdev)
 	struct sirfsoc_uart_port *sirfport = platform_get_drvdata(pdev);
 	struct uart_port *port = &sirfport->port;
 	platform_set_drvdata(pdev, NULL);
-	if (sirfport->hw_flow_ctrl) {
-		pinctrl_disable(sirfport->p);
+	if (sirfport->hw_flow_ctrl)
 		pinctrl_put(sirfport->p);
-	}
 	devm_iounmap(&pdev->dev, port->membase);
 	uart_remove_one_port(&sirfsoc_uart_drv, port);
 	return 0;
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 3086594..9ad5896 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -12,12 +12,14 @@
 #ifndef __LINUX_PINCTRL_CONSUMER_H
 #define __LINUX_PINCTRL_CONSUMER_H
 
+#include <linux/err.h>
 #include <linux/list.h>
 #include <linux/seq_file.h>
 #include "pinctrl.h"
 
 /* This struct is private to the core and should be regarded as a cookie */
 struct pinctrl;
+struct pinctrl_state;
 
 #ifdef CONFIG_PINCTRL
 
@@ -26,10 +28,13 @@ extern int pinctrl_request_gpio(unsigned gpio);
 extern void pinctrl_free_gpio(unsigned gpio);
 extern int pinctrl_gpio_direction_input(unsigned gpio);
 extern int pinctrl_gpio_direction_output(unsigned gpio);
-extern struct pinctrl * __must_check pinctrl_get(struct device *dev, const char *name);
+
+extern struct pinctrl * __must_check pinctrl_get(struct device *dev);
 extern void pinctrl_put(struct pinctrl *p);
-extern int pinctrl_enable(struct pinctrl *p);
-extern void pinctrl_disable(struct pinctrl *p);
+extern struct pinctrl_state * __must_check pinctrl_lookup_state(
+							struct pinctrl *p,
+							const char *name);
+extern int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *s);
 
 #else /* !CONFIG_PINCTRL */
 
@@ -52,7 +57,7 @@ static inline int pinctrl_gpio_direction_output(unsigned gpio)
 	return 0;
 }
 
-static inline struct pinctrl * __must_check pinctrl_get(struct device *dev, const char *name)
+static inline struct pinctrl * __must_check pinctrl_get(struct device *dev)
 {
 	return NULL;
 }
@@ -61,17 +66,53 @@ static inline void pinctrl_put(struct pinctrl *p)
 {
 }
 
-static inline int pinctrl_enable(struct pinctrl *p)
+static inline struct pinctrl_state * __must_check pinctrl_lookup_state(
+							struct pinctrl *p,
+							const char *name)
 {
-	return 0;
+	return NULL;
 }
 
-static inline void pinctrl_disable(struct pinctrl *p)
+static inline int pinctrl_select_state(struct pinctrl *p,
+				       struct pinctrl_state *s)
 {
+	return 0;
 }
 
 #endif /* CONFIG_PINCTRL */
 
+static inline struct pinctrl * __must_check pinctrl_get_select(
+					struct device *dev, const char *name)
+{
+	struct pinctrl *p;
+	struct pinctrl_state *s;
+	int ret;
+
+	p = pinctrl_get(dev);
+	if (IS_ERR(p))
+		return p;
+
+	s = pinctrl_lookup_state(p, name);
+	if (IS_ERR(s)) {
+		pinctrl_put(p);
+		return ERR_PTR(PTR_ERR(s));
+	}
+
+	ret = pinctrl_select_state(p, s);
+	if (ret < 0) {
+		pinctrl_put(p);
+		return ERR_PTR(ret);
+	}
+
+	return p;
+}
+
+static inline struct pinctrl * __must_check pinctrl_get_select_default(
+					struct device *dev)
+{
+	return pinctrl_get_select(dev, PINCTRL_STATE_DEFAULT);
+}
+
 #ifdef CONFIG_PINCONF
 
 extern int pin_config_get(const char *dev_name, const char *name,
diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
index 20e9735..05d25c8 100644
--- a/include/linux/pinctrl/machine.h
+++ b/include/linux/pinctrl/machine.h
@@ -21,23 +21,22 @@
  *	same name as the pin controllers own dev_name(), the map entry will be
  *	hogged by the driver itself upon registration
  * @name: the name of this specific map entry for the particular machine.
- *	This is the second parameter passed to pinmux_get() when you want
- *	to have several mappings to the same device
+ *	This is the parameter passed to pinmux_lookup_state()
  * @ctrl_dev_name: the name of the device controlling this specific mapping,
  *	the name must be the same as in your struct device*
- * @function: a function in the driver to use for this mapping, the driver
- *	will lookup the function referenced by this ID on the specified
- *	pin control device
  * @group: sometimes a function can map to different pin groups, so this
  *	selects a certain specific pin group to activate for the function, if
  *	left as NULL, the first applicable group will be used
+ * @function: a function in the driver to use for this mapping, the driver
+ *	will lookup the function referenced by this ID on the specified
+ *	pin control device
  */
 struct pinctrl_map {
 	const char *dev_name;
 	const char *name;
 	const char *ctrl_dev_name;
-	const char *function;
 	const char *group;
+	const char *function;
 };
 
 /*
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations
  2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
                   ` (3 preceding siblings ...)
  2012-02-28 20:27 ` [PATCH V2 4/6] pinctrl: API changes to support multiple states per device Stephen Warren
@ 2012-02-28 20:27 ` Stephen Warren
  2012-03-01  8:46   ` Dong Aisheng
  2012-02-28 20:27 ` [PATCH V2 6/6] pinctrl: fix case of Tegra30's foo_groups[] arrays Stephen Warren
  2012-02-29 16:46 ` [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Linus Walleij
  6 siblings, 1 reply; 18+ messages in thread
From: Stephen Warren @ 2012-02-28 20:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel, Stephen Warren

The pinctrl mapping table can now contain entries to:
* Set the mux function of a pin group
* Apply a set of pin config options to a pin or a group

This allows pinctrl_select_state() to apply pin configs settings as well
as mux settings.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: Added numerous extra PIN_MAP_*() special-case macros. Fixed kerneldoc
typo. Delete pinctrl_get_pin_id() and replace it with pin_get_from_name().
Various minor fixes. Updates due to rebase.

 Documentation/pinctrl.txt       |   76 +++++++++++++++---
 arch/arm/mach-u300/core.c       |   12 ++--
 drivers/pinctrl/core.c          |  152 ++++++++++++++++++++++++++++++------
 drivers/pinctrl/core.h          |   35 +++++++-
 drivers/pinctrl/pinconf.c       |  165 +++++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinconf.h       |   40 ++++++++++
 drivers/pinctrl/pinmux.c        |   69 ++++++++++------
 drivers/pinctrl/pinmux.h        |   23 +++++-
 include/linux/pinctrl/machine.h |  145 +++++++++++++++++++++++++++-------
 9 files changed, 610 insertions(+), 107 deletions(-)

diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
index 23426c7..bba7c97 100644
--- a/Documentation/pinctrl.txt
+++ b/Documentation/pinctrl.txt
@@ -206,14 +206,21 @@ using a certain resistor value - pull up and pull down - so that the pin has a
 stable value when nothing is driving the rail it is connected to, or when it's
 unconnected.
 
-For example, a platform may do this:
+Pin configuration can be programmed either using the explicit APIs described
+immediately below, or by adding configuration entries into the mapping table;
+see section "Board/machine configuration" below.
+
+For example, a platform may do the following to pull up a pin to VDD:
 
 #include <linux/pinctrl/consumer.h>
 
 ret = pin_config_set("foo-dev", "FOO_GPIO_PIN", PLATFORM_X_PULL_UP);
 
-To pull up a pin to VDD. The pin configuration driver implements callbacks for
-changing pin configuration in the pin controller ops like this:
+The format and meaning of the configuration parameter, PLATFORM_X_PULL_UP
+above, is entirely defined by the pin controller driver.
+
+The pin configuration driver implements callbacks for changing pin
+configuration in the pin controller ops like this:
 
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinconf.h>
@@ -765,7 +772,7 @@ obtain the function "gpioN" where "N" is the global GPIO pin number if no
 special GPIO-handler is registered.
 
 
-Pinmux board/machine configuration
+Board/machine configuration
 ==================================
 
 Boards and machines define how a certain complete running system is put
@@ -773,9 +780,9 @@ together, including how GPIOs and devices are muxed, how regulators are
 constrained and how the clock tree looks. Of course pinmux settings are also
 part of this.
 
-A pinmux config for a machine looks pretty much like a simple regulator
-configuration, so for the example array above we want to enable i2c and
-spi on the second function mapping:
+A pin controller configuration for a machine looks pretty much like a simple
+regulator configuration, so for the example array above we want to enable i2c
+and spi on the second function mapping:
 
 #include <linux/pinctrl/machine.h>
 
@@ -783,20 +790,23 @@ static const struct pinctrl_map __initdata mapping[] = {
 	{
 		.dev_name = "foo-spi.0",
 		.name = PINCTRL_STATE_DEFAULT,
+		.type = PIN_MAP_TYPE_MUX_GROUP,
 		.ctrl_dev_name = "pinctrl-foo",
-		.function = "spi0",
+		.data.mux.function = "spi0",
 	},
 	{
 		.dev_name = "foo-i2c.0",
 		.name = PINCTRL_STATE_DEFAULT,
+		.type = PIN_MAP_TYPE_MUX_GROUP,
 		.ctrl_dev_name = "pinctrl-foo",
-		.function = "i2c0",
+		.data.mux.function = "i2c0",
 	},
 	{
 		.dev_name = "foo-mmc.0",
 		.name = PINCTRL_STATE_DEFAULT,
+		.type = PIN_MAP_TYPE_MUX_GROUP,
 		.ctrl_dev_name = "pinctrl-foo",
-		.function = "mmc0",
+		.data.mux.function = "mmc0",
 	},
 };
 
@@ -817,7 +827,40 @@ it even more compact which assumes you want to use pinctrl-foo and position
 0 for mapping, for example:
 
 static struct pinctrl_map __initdata mapping[] = {
-	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0", "foo-i2c.0"),
+	PIN_MAP_MUX_GROUP("foo-i2c.o", PINCTRL_STATE_DEFAULT, "pinctrl-foo", NULL, "i2c0"),
+};
+
+The mapping table may also contain pin configuration entries. It's common for
+each pin/group to have a number of configuration entries that affect it, so
+the table entries for configuration reference an array of config parameters
+and values. An example using the convenience macros is shown below:
+
+static unsigned long i2c_grp_configs[] = {
+	FOO_PIN_DRIVEN,
+	FOO_PIN_PULLUP,
+};
+
+static unsigned long i2c_pin_configs[] = {
+	FOO_OPEN_COLLECTOR,
+	FOO_SLEW_RATE_SLOW,
+};
+
+static struct pinctrl_map __initdata mapping[] = {
+	PIN_MAP_MUX_GROUP("foo-i2c.0", PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0", "i2c0"),
+	PIN_MAP_MUX_CONFIGS_GROUP("foo-i2c.0", PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0", i2c_grp_configs),
+	PIN_MAP_MUX_CONFIGS_PIN("foo-i2c.0", PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0scl", i2c_pin_configs),
+	PIN_MAP_MUX_CONFIGS_PIN("foo-i2c.0", PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0sda", i2c_pin_configs),
+};
+
+Finally, some devices expect the mapping table to contain certain specific
+named states. When running on hardware that doesn't need any pin controller
+configuration, the mapping table must still contain those named states, in
+order to explicitly indicate that the states were provided and intended to
+be empty. Table entry macro PIN_MAP_DUMMY_STATE serves the purpose of defining
+a named state without causing any pin controller to be programmed:
+
+static struct pinctrl_map __initdata mapping[] = {
+	PIN_MAP_DUMMY_STATE("foo-i2c.0", PINCTRL_STATE_DEFAULT),
 };
 
 
@@ -831,6 +874,7 @@ As it is possible to map a function to different groups of pins an optional
 {
 	.dev_name = "foo-spi.0",
 	.name = "spi0-pos-A",
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "spi0",
 	.group = "spi0_0_grp",
@@ -838,6 +882,7 @@ As it is possible to map a function to different groups of pins an optional
 {
 	.dev_name = "foo-spi.0",
 	.name = "spi0-pos-B",
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "spi0",
 	.group = "spi0_1_grp",
@@ -857,6 +902,7 @@ case), we define a mapping like this:
 {
 	.dev_name = "foo-mmc.0",
 	.name = "2bit"
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "mmc0",
 	.group = "mmc0_1_grp",
@@ -864,6 +910,7 @@ case), we define a mapping like this:
 {
 	.dev_name = "foo-mmc.0",
 	.name = "4bit"
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "mmc0",
 	.group = "mmc0_1_grp",
@@ -871,6 +918,7 @@ case), we define a mapping like this:
 {
 	.dev_name = "foo-mmc.0",
 	.name = "4bit"
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "mmc0",
 	.group = "mmc0_2_grp",
@@ -878,6 +926,7 @@ case), we define a mapping like this:
 {
 	.dev_name = "foo-mmc.0",
 	.name = "8bit"
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "mmc0",
 	.group = "mmc0_1_grp",
@@ -885,6 +934,7 @@ case), we define a mapping like this:
 {
 	.dev_name = "foo-mmc.0",
 	.name = "8bit"
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "mmc0",
 	.group = "mmc0_2_grp",
@@ -892,6 +942,7 @@ case), we define a mapping like this:
 {
 	.dev_name = "foo-mmc.0",
 	.name = "8bit"
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "mmc0",
 	.group = "mmc0_3_grp",
@@ -1014,6 +1065,7 @@ to the pin controller device name, and the state name is PINCTRL_STATE_DEFAULT.
 {
 	.dev_name = "pinctrl-foo",
 	.name = PINCTRL_STATE_DEFAULT,
+	.type = PIN_MAP_TYPE_MUX_GROUP,
 	.ctrl_dev_name = "pinctrl-foo",
 	.function = "power_func",
 },
@@ -1022,7 +1074,7 @@ Since it may be common to request the core to hog a few always-applicable
 mux settings on the primary pin controller, there is a convenience macro for
 this:
 
-PIN_MAP_SYS_HOG("pinctrl-foo", "power_func")
+PIN_MAP_MUX_GROUP_HOG("pinctrl-foo", NULL /* group */, "power_func")
 
 This gives the exact same result as the above construction.
 
diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
index 18973ca..c965bb5 100644
--- a/arch/arm/mach-u300/core.c
+++ b/arch/arm/mach-u300/core.c
@@ -1569,13 +1569,13 @@ static struct platform_device dma_device = {
 /* Pinmux settings */
 static struct pinctrl_map __initdata u300_pinmux_map[] = {
 	/* anonymous maps for chip power and EMIFs */
-	PIN_MAP_SYS_HOG("pinctrl-u300", "power"),
-	PIN_MAP_SYS_HOG("pinctrl-u300", "emif0"),
-	PIN_MAP_SYS_HOG("pinctrl-u300", "emif1"),
+	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "power"),
+	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif0"),
+	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif1"),
 	/* per-device maps for MMC/SD, SPI and UART */
-	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "mmc0", "mmci"),
-	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "spi0", "pl022"),
-	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "uart0", "uart0"),
+	PIN_MAP_MUX_GROUP_DEFAULT("mmci",  "pinctrl-u300", NULL, "mmc0"),
+	PIN_MAP_MUX_GROUP_DEFAULT("pl022", "pinctrl-u300", NULL, "spi0"),
+	PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0"),
 };
 
 struct u300_mux_hog {
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index c6f3ca3..4eae7e5 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -502,6 +502,9 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
 	if (IS_ERR(state))
 		return PTR_ERR(state);
 
+	if (map->type == PIN_MAP_TYPE_DUMMY_STATE)
+		return 0;
+
 	setting = kzalloc(sizeof(*setting), GFP_KERNEL);
 	if (setting == NULL) {
 		dev_err(p->dev,
@@ -509,6 +512,8 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
 		return -ENOMEM;
 	}
 
+	setting->type = map->type;
+
 	setting->pctldev = get_pinctrl_dev_from_devname(map->ctrl_dev_name);
 	if (setting->pctldev == NULL) {
 		dev_err(p->dev, "unknown pinctrl device %s in map entry",
@@ -518,7 +523,18 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
 		return -ENODEV;
 	}
 
-	ret = pinmux_map_to_setting(map, setting);
+	switch (map->type) {
+	case PIN_MAP_TYPE_MUX_GROUP:
+		ret = pinmux_map_to_setting(map, setting);
+		break;
+	case PIN_MAP_TYPE_CONFIGS_PIN:
+	case PIN_MAP_TYPE_CONFIGS_GROUP:
+		ret = pinconf_map_to_setting(map, setting);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
 	if (ret < 0) {
 		kfree(setting);
 		return ret;
@@ -626,9 +642,19 @@ static void pinctrl_put_locked(struct pinctrl *p, bool inlist)
 
 	list_for_each_entry_safe(state, n1, &p->states, node) {
 		list_for_each_entry_safe(setting, n2, &state->settings, node) {
-			if (state == p->state)
-				pinmux_disable_setting(setting);
-			pinmux_free_setting(setting);
+			switch (setting->type) {
+			case PIN_MAP_TYPE_MUX_GROUP:
+				if (state == p->state)
+					pinmux_disable_setting(setting);
+				pinmux_free_setting(setting);
+				break;
+			case PIN_MAP_TYPE_CONFIGS_PIN:
+			case PIN_MAP_TYPE_CONFIGS_GROUP:
+				pinconf_free_setting(setting);
+				break;
+			default:
+				break;
+			}
 			list_del(&setting->node);
 			kfree(setting);
 		}
@@ -703,9 +729,13 @@ static int pinctrl_select_state_locked(struct pinctrl *p,
 		 */
 		list_for_each_entry(setting, &p->state->settings, node) {
 			bool found = false;
+			if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
+				continue;
 			list_for_each_entry(setting2, &state->settings, node) {
-				if (setting2->group_selector ==
-						setting->group_selector) {
+				if (setting2->type != PIN_MAP_TYPE_MUX_GROUP)
+					continue;
+				if (setting2->data.mux.group ==
+						setting->data.mux.group) {
 					found = true;
 					break;
 				}
@@ -719,7 +749,18 @@ static int pinctrl_select_state_locked(struct pinctrl *p,
 
 	/* Apply all the settings for the new state */
 	list_for_each_entry(setting, &state->settings, node) {
-		ret = pinmux_enable_setting(setting);
+		switch (setting->type) {
+		case PIN_MAP_TYPE_MUX_GROUP:
+			ret = pinmux_enable_setting(setting);
+			break;
+		case PIN_MAP_TYPE_CONFIGS_PIN:
+		case PIN_MAP_TYPE_CONFIGS_GROUP:
+			ret = pinconf_apply_setting(setting);
+			break;
+		default:
+			ret = -EINVAL;
+			break;
+		}
 		if (ret < 0) {
 			/* FIXME: Difficult to return to prev state */
 			return ret;
@@ -756,33 +797,48 @@ EXPORT_SYMBOL_GPL(pinctrl_select_state);
 int pinctrl_register_mappings(struct pinctrl_map const *maps,
 			      unsigned num_maps)
 {
-	int i;
+	int i, ret;
 	struct pinctrl_maps *maps_node;
 
 	pr_debug("add %d pinmux maps\n", num_maps);
 
 	/* First sanity check the new mapping */
 	for (i = 0; i < num_maps; i++) {
+		if (!maps[i].dev_name) {
+			pr_err("failed to register map %s (%d): no device given\n",
+			       maps[i].name, i);
+			return -EINVAL;
+		}
+
 		if (!maps[i].name) {
 			pr_err("failed to register map %d: no map name given\n",
 			       i);
 			return -EINVAL;
 		}
 
-		if (!maps[i].ctrl_dev_name) {
+		if (maps[i].type != PIN_MAP_TYPE_DUMMY_STATE &&
+				!maps[i].ctrl_dev_name) {
 			pr_err("failed to register map %s (%d): no pin control device given\n",
 			       maps[i].name, i);
 			return -EINVAL;
 		}
 
-		if (!maps[i].function) {
-			pr_err("failed to register map %s (%d): no function ID given\n",
-			       maps[i].name, i);
-			return -EINVAL;
-		}
-
-		if (!maps[i].dev_name) {
-			pr_err("failed to register map %s (%d): no device given\n",
+		switch (maps[i].type) {
+		case PIN_MAP_TYPE_DUMMY_STATE:
+			break;
+		case PIN_MAP_TYPE_MUX_GROUP:
+			ret = pinmux_validate_map(&maps[i], i);
+			if (ret < 0)
+				return 0;
+			break;
+		case PIN_MAP_TYPE_CONFIGS_PIN:
+		case PIN_MAP_TYPE_CONFIGS_GROUP:
+			ret = pinconf_validate_map(&maps[i], i);
+			if (ret < 0)
+				return 0;
+			break;
+		default:
+			pr_err("failed to register map %s (%d): invalid type given\n",
 			       maps[i].name, i);
 			return -EINVAL;
 		}
@@ -934,6 +990,22 @@ static int pinctrl_devices_show(struct seq_file *s, void *what)
 	return 0;
 }
 
+static inline const char *map_type(enum pinctrl_map_type type)
+{
+	static const char * const names[] = {
+		"INVALID",
+		"DUMMY_STATE",
+		"MUX_GROUP",
+		"CONFIGS_PIN",
+		"CONFIGS_GROUP",
+	};
+
+	if (type >= ARRAY_SIZE(names))
+		return "UNKNOWN";
+
+	return names[type];
+}
+
 static int pinctrl_maps_show(struct seq_file *s, void *what)
 {
 	struct pinctrl_maps *maps_node;
@@ -945,12 +1017,27 @@ static int pinctrl_maps_show(struct seq_file *s, void *what)
 	mutex_lock(&pinctrl_mutex);
 
 	for_each_maps(maps_node, i, map) {
-		seq_printf(s, "%s:\n", map->name);
-		seq_printf(s, "  device: %s\n", map->dev_name);
-		seq_printf(s, "  controlling device %s\n", map->ctrl_dev_name);
-		seq_printf(s, "  function: %s\n", map->function);
-		seq_printf(s, "  group: %s\n", map->group ? map->group :
-			   "(default)");
+		seq_printf(s, "device %s\nstate %s\ntype %s (%d)\n",
+			   map->dev_name, map->name, map_type(map->type),
+			   map->type);
+
+		if (map->type != PIN_MAP_TYPE_DUMMY_STATE)
+			seq_printf(s, "controlling device %s\n",
+				   map->ctrl_dev_name);
+
+		switch (map->type) {
+		case PIN_MAP_TYPE_MUX_GROUP:
+			pinmux_show_map(s, map);
+			break;
+		case PIN_MAP_TYPE_CONFIGS_PIN:
+		case PIN_MAP_TYPE_CONFIGS_GROUP:
+			pinconf_show_map(s, map);
+			break;
+		default:
+			break;
+		}
+
+		seq_printf(s, "\n");
 	}
 
 	mutex_unlock(&pinctrl_mutex);
@@ -977,8 +1064,23 @@ static int pinctrl_show(struct seq_file *s, void *what)
 			seq_printf(s, "  state: %s\n", state->name);
 
 			list_for_each_entry(setting, &state->settings, node) {
-				seq_printf(s, "    ");
-				pinmux_dbg_show(s, setting);
+				struct pinctrl_dev *pctldev = setting->pctldev;
+
+				seq_printf(s, "    type: %s controller %s ",
+					   map_type(setting->type),
+					   pinctrl_dev_get_name(pctldev));
+
+				switch (setting->type) {
+				case PIN_MAP_TYPE_MUX_GROUP:
+					pinmux_show_setting(s, setting);
+					break;
+				case PIN_MAP_TYPE_CONFIGS_PIN:
+				case PIN_MAP_TYPE_CONFIGS_GROUP:
+					pinconf_show_setting(s, setting);
+					break;
+				default:
+					break;
+				}
 			}
 		}
 	}
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 5691d31..1cae372 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -72,17 +72,44 @@ struct pinctrl_state {
 };
 
 /**
+ * struct pinctrl_setting_mux - setting data for MAP_TYPE_MUX_GROUP
+ * @group: the group selector to program
+ * @func: the function selector to program
+ */
+struct pinctrl_setting_mux {
+	unsigned group;
+	unsigned func;
+};
+
+/**
+ * struct pinctrl_setting_configs - setting data for MAP_TYPE_CONFIGS_*
+ * @group_or_pin: the group selector or pin ID to program
+ * @configs: a pointer to an array of config parameters/values to program into
+ *	hardware. Each individual pin controller defines the format and meaning
+ *	of config parameters.
+ * @num_configs: the number of entries in array @configs
+ */
+struct pinctrl_setting_configs {
+	unsigned group_or_pin;
+	unsigned long *configs;
+	unsigned num_configs;
+};
+
+/**
  * struct pinctrl_setting - an individual mux setting
  * @node: list node for struct pinctrl_settings's @settings field
+ * @type: the type of setting
  * @pctldev: pin control device handling to be programmed
- * @group_selector: the group selector to program
- * @func_selector: the function selector to program
+ * @data: Data specific to the setting type
  */
 struct pinctrl_setting {
 	struct list_head node;
+	enum pinctrl_map_type type;
 	struct pinctrl_dev *pctldev;
-	unsigned group_selector;
-	unsigned func_selector;
+	union {
+		struct pinctrl_setting_mux mux;
+		struct pinctrl_setting_configs configs;
+	} data;
 };
 
 /**
diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index e0a4537..84869f2 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -36,6 +36,24 @@ int pinconf_check_ops(struct pinctrl_dev *pctldev)
 	return 0;
 }
 
+int pinconf_validate_map(struct pinctrl_map const *map, int i)
+{
+	if (!map->data.configs.group_or_pin) {
+		pr_err("failed to register map %s (%d): no group/pin given\n",
+		       map->name, i);
+		return -EINVAL;
+	}
+
+	if (map->data.configs.num_configs &&
+			!map->data.configs.configs) {
+		pr_err("failed to register map %s (%d): no configs ptr given\n",
+		       map->name, i);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int pin_config_get_for_pin(struct pinctrl_dev *pctldev, unsigned pin,
 			   unsigned long *config)
 {
@@ -260,8 +278,155 @@ unlock:
 }
 EXPORT_SYMBOL(pin_config_group_set);
 
+int pinconf_map_to_setting(struct pinctrl_map const *map,
+			  struct pinctrl_setting *setting)
+{
+	struct pinctrl_dev *pctldev = setting->pctldev;
+
+	switch (setting->type) {
+	case PIN_MAP_TYPE_CONFIGS_PIN:
+		setting->data.configs.group_or_pin =
+			pin_get_from_name(pctldev,
+					  map->data.configs.group_or_pin);
+		if (setting->data.configs.group_or_pin < 0)
+			return setting->data.configs.group_or_pin;
+		break;
+	case PIN_MAP_TYPE_CONFIGS_GROUP:
+		setting->data.configs.group_or_pin =
+			pinctrl_get_group_selector(pctldev,
+					map->data.configs.group_or_pin);
+		if (setting->data.configs.group_or_pin < 0)
+			return setting->data.configs.group_or_pin;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	setting->data.configs.num_configs = map->data.configs.num_configs;
+	setting->data.configs.configs = map->data.configs.configs;
+
+	return 0;
+}
+
+void pinconf_free_setting(struct pinctrl_setting const *setting)
+{
+}
+
+int pinconf_apply_setting(struct pinctrl_setting const *setting)
+{
+	struct pinctrl_dev *pctldev = setting->pctldev;
+	const struct pinconf_ops *ops = pctldev->desc->confops;
+	int i, ret;
+
+	if (!ops) {
+		dev_err(pctldev->dev, "missing confops\n");
+		return -EINVAL;
+	}
+
+	switch (setting->type) {
+	case PIN_MAP_TYPE_CONFIGS_PIN:
+		if (!ops->pin_config_set) {
+			dev_err(pctldev->dev, "missing pin_config_set op\n");
+			return -EINVAL;
+		}
+		for (i = 0; i < setting->data.configs.num_configs; i++) {
+			ret = ops->pin_config_set(pctldev,
+					setting->data.configs.group_or_pin,
+					setting->data.configs.configs[i]);
+			if (ret < 0) {
+				dev_err(pctldev->dev,
+					"pin_config_set op failed for pin %d config %08lx\n",
+					setting->data.configs.group_or_pin,
+					setting->data.configs.configs[i]);
+				return ret;
+			}
+		}
+		break;
+	case PIN_MAP_TYPE_CONFIGS_GROUP:
+		if (!ops->pin_config_group_set) {
+			dev_err(pctldev->dev,
+				"missing pin_config_group_set op\n");
+			return -EINVAL;
+		}
+		for (i = 0; i < setting->data.configs.num_configs; i++) {
+			ret = ops->pin_config_group_set(pctldev,
+					setting->data.configs.group_or_pin,
+					setting->data.configs.configs[i]);
+			if (ret < 0) {
+				dev_err(pctldev->dev,
+					"pin_config_group_set op failed for group %d config %08lx\n",
+					setting->data.configs.group_or_pin,
+					setting->data.configs.configs[i]);
+				return ret;
+			}
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_DEBUG_FS
 
+void pinconf_show_map(struct seq_file *s, struct pinctrl_map const *map)
+{
+	int i;
+
+	switch (map->type) {
+	case PIN_MAP_TYPE_CONFIGS_PIN:
+		seq_printf(s, "pin ");
+		break;
+	case PIN_MAP_TYPE_CONFIGS_GROUP:
+		seq_printf(s, "group ");
+		break;
+	default:
+		break;
+	}
+
+	seq_printf(s, "%s\n", map->data.configs.group_or_pin);
+
+	for (i = 0; i < map->data.configs.num_configs; i++)
+		seq_printf(s, "config %08lx\n", map->data.configs.configs[i]);
+}
+
+void pinconf_show_setting(struct seq_file *s,
+			  struct pinctrl_setting const *setting)
+{
+	struct pinctrl_dev *pctldev = setting->pctldev;
+	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
+	struct pin_desc *desc;
+	int i;
+
+	switch (setting->type) {
+	case PIN_MAP_TYPE_CONFIGS_PIN:
+		desc = pin_desc_get(setting->pctldev,
+				    setting->data.configs.group_or_pin);
+		seq_printf(s, "pin %s (%d)",
+			   desc->name ? desc->name : "unnamed",
+			   setting->data.configs.group_or_pin);
+		break;
+	case PIN_MAP_TYPE_CONFIGS_GROUP:
+		seq_printf(s, "group %s (%d)",
+			   pctlops->get_group_name(pctldev,
+					setting->data.configs.group_or_pin),
+			   setting->data.configs.group_or_pin);
+		break;
+	default:
+		break;
+	}
+
+	/*
+	 * FIXME: We should really get the pin controler to dump the config
+	 * values, so they can be decoded to something meaningful.
+	 */
+	for (i = 0; i < setting->data.configs.num_configs; i++)
+		seq_printf(s, " %08lx", setting->data.configs.configs[i]);
+
+	seq_printf(s, "\n");
+}
+
 static void pinconf_dump_pin(struct pinctrl_dev *pctldev,
 			     struct seq_file *s, int pin)
 {
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index 1d6ea9d..0ded227 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -15,6 +15,16 @@
 
 int pinconf_check_ops(struct pinctrl_dev *pctldev);
 
+int pinconf_validate_map(struct pinctrl_map const *map, int i);
+
+int pinconf_map_to_setting(struct pinctrl_map const *map,
+			  struct pinctrl_setting *setting);
+void pinconf_free_setting(struct pinctrl_setting const *setting);
+int pinconf_apply_setting(struct pinctrl_setting const *setting);
+
+void pinconf_show_map(struct seq_file *s, struct pinctrl_map const *map);
+void pinconf_show_setting(struct seq_file *s,
+			  struct pinctrl_setting const *setting);
 void pinconf_init_device_debugfs(struct dentry *devroot,
 				 struct pinctrl_dev *pctldev);
 
@@ -25,6 +35,36 @@ static inline int pinconf_check_ops(struct pinctrl_dev *pctldev)
 	return 0;
 }
 
+static inline int pinconf_validate_map(struct pinctrl_map const *map, int i)
+{
+	return 0;
+}
+
+static inline int pinconf_map_to_setting(struct pinctrl_map const *map,
+			  struct pinctrl_setting *setting)
+{
+	return 0;
+}
+
+static inline void pinconf_free_setting(struct pinctrl_setting const *setting)
+{
+}
+
+static inline int pinconf_apply_setting(struct pinctrl_setting const *setting)
+{
+	return 0;
+}
+
+static inline void pinconf_show_map(struct seq_file *s,
+				    struct pinctrl_map const *map)
+{
+}
+
+static inline void pinconf_show_setting(struct seq_file *s,
+			  struct pinctrl_setting const *setting)
+{
+}
+
 static inline void pinconf_init_device_debugfs(struct dentry *devroot,
 					       struct pinctrl_dev *pctldev)
 {
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 2d1aaf8..949512e 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -58,6 +58,17 @@ int pinmux_check_ops(struct pinctrl_dev *pctldev)
 	return 0;
 }
 
+int pinmux_validate_map(struct pinctrl_map const *map, int i)
+{
+	if (!map->data.mux.function) {
+		pr_err("failed to register map %s (%d): no function given\n",
+		       map->name, i);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * pin_request() - request a single pin to be muxed in, typically for GPIO
  * @pin: the pin number in the global pin space
@@ -284,21 +295,21 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
 	const unsigned *pins;
 	unsigned num_pins;
 
-	setting->func_selector =
-		pinmux_func_name_to_selector(pctldev, map->function);
-	if (setting->func_selector < 0)
-		return setting->func_selector;
+	setting->data.mux.func =
+		pinmux_func_name_to_selector(pctldev, map->data.mux.function);
+	if (setting->data.mux.func < 0)
+		return setting->data.mux.func;
 
-	ret = pmxops->get_function_groups(pctldev, setting->func_selector,
+	ret = pmxops->get_function_groups(pctldev, setting->data.mux.func,
 					  &groups, &num_groups);
 	if (ret < 0)
 		return ret;
 	if (!num_groups)
 		return -EINVAL;
 
-	if (map->group) {
+	if (map->data.mux.group) {
 		bool found = false;
-		group = map->group;
+		group = map->data.mux.group;
 		for (i = 0; i < num_groups; i++) {
 			if (!strcmp(group, groups[i])) {
 				found = true;
@@ -311,17 +322,16 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
 		group = groups[0];
 	}
 
-	setting->group_selector =
-		pinctrl_get_group_selector(pctldev, group);
-	if (setting->group_selector < 0)
-		return setting->group_selector;
+	setting->data.mux.group = pinctrl_get_group_selector(pctldev, group);
+	if (setting->data.mux.group < 0)
+		return setting->data.mux.group;
 
-	ret = pctlops->get_group_pins(pctldev, setting->group_selector,
-				      &pins, &num_pins);
+	ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, &pins,
+				      &num_pins);
 	if (ret) {
 		dev_err(pctldev->dev,
 			"could not get pins for device %s group selector %d\n",
-			pinctrl_dev_get_name(pctldev), setting->group_selector);
+			pinctrl_dev_get_name(pctldev), setting->data.mux.group);
 			return -ENODEV;
 	}
 
@@ -352,12 +362,12 @@ void pinmux_free_setting(struct pinctrl_setting const *setting)
 	int ret;
 	int i;
 
-	ret = pctlops->get_group_pins(pctldev, setting->group_selector,
+	ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
 				      &pins, &num_pins);
 	if (ret) {
 		dev_err(pctldev->dev,
 			"could not get pins for device %s group selector %d\n",
-			pinctrl_dev_get_name(pctldev), setting->group_selector);
+			pinctrl_dev_get_name(pctldev), setting->data.mux.group);
 		return;
 	}
 
@@ -370,8 +380,8 @@ int pinmux_enable_setting(struct pinctrl_setting const *setting)
 	struct pinctrl_dev *pctldev = setting->pctldev;
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
 
-	return ops->enable(pctldev, setting->func_selector,
-			   setting->group_selector);
+	return ops->enable(pctldev, setting->data.mux.func,
+			   setting->data.mux.group);
 }
 
 void pinmux_disable_setting(struct pinctrl_setting const *setting)
@@ -379,7 +389,7 @@ void pinmux_disable_setting(struct pinctrl_setting const *setting)
 	struct pinctrl_dev *pctldev = setting->pctldev;
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
 
-	ops->disable(pctldev, setting->func_selector, setting->group_selector);
+	ops->disable(pctldev, setting->data.mux.func, setting->data.mux.group);
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -458,18 +468,25 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 	return 0;
 }
 
-void pinmux_dbg_show(struct seq_file *s, struct pinctrl_setting const *setting)
+void pinmux_show_map(struct seq_file *s, struct pinctrl_map const *map)
+{
+	seq_printf(s, "group %s\nfunction %s\n",
+		map->data.mux.group ? map->data.mux.group : "(default)",
+		map->data.mux.function);
+}
+
+void pinmux_show_setting(struct seq_file *s,
+			 struct pinctrl_setting const *setting)
 {
 	struct pinctrl_dev *pctldev = setting->pctldev;
 	const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
 	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
 
-	seq_printf(s, "controller: %s group: %s (%u) function: %s (%u)\n",
-		   pinctrl_dev_get_name(pctldev),
-		   pctlops->get_group_name(pctldev, setting->group_selector),
-		   setting->group_selector,
-		   pmxops->get_function_name(pctldev, setting->func_selector),
-		   setting->func_selector);
+	seq_printf(s, "group: %s (%u) function: %s (%u)\n",
+		   pctlops->get_group_name(pctldev, setting->data.mux.group),
+		   setting->data.mux.group,
+		   pmxops->get_function_name(pctldev, setting->data.mux.func),
+		   setting->data.mux.func);
 }
 
 static int pinmux_functions_open(struct inode *inode, struct file *file)
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 1500ae8..6fc4700 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -14,6 +14,8 @@
 
 int pinmux_check_ops(struct pinctrl_dev *pctldev);
 
+int pinmux_validate_map(struct pinctrl_map const *map, int i);
+
 int pinmux_request_gpio(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range,
 			unsigned pin, unsigned gpio);
@@ -29,7 +31,9 @@ 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_show_map(struct seq_file *s, struct pinctrl_map const *map);
+void pinmux_show_setting(struct seq_file *s,
+			 struct pinctrl_setting const *setting);
 void pinmux_init_device_debugfs(struct dentry *devroot,
 				struct pinctrl_dev *pctldev);
 
@@ -40,6 +44,11 @@ static inline int pinmux_check_ops(struct pinctrl_dev *pctldev)
 	return 0;
 }
 
+static inline int pinmux_validate_map(struct pinctrl_map const *map, int i)
+{
+	return 0;
+}
+
 static inline int pinmux_request_gpio(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range,
 			unsigned pin, unsigned gpio)
@@ -80,12 +89,18 @@ static inline void pinmux_disable_setting(
 {
 }
 
-static inline void pinmux_init_device_debugfs(struct dentry *devroot,
-					      struct pinctrl_dev *pctldev)
+static inline void pinmux_show_map(struct seq_file *s,
+				   struct pinctrl_map const *map)
+{
+}
+
+static inline void pinmux_show_setting(struct seq_file *s,
+				       struct pinctrl_setting const *setting)
 {
 }
 
-static inline void pinmux_dbg_show(struct seq_file *s, struct pinctrl *p)
+static inline void pinmux_init_device_debugfs(struct dentry *devroot,
+					      struct pinctrl_dev *pctldev)
 {
 }
 
diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
index 05d25c8..b572153 100644
--- a/include/linux/pinctrl/machine.h
+++ b/include/linux/pinctrl/machine.h
@@ -14,6 +14,41 @@
 
 #include "pinctrl.h"
 
+enum pinctrl_map_type {
+	PIN_MAP_TYPE_INVALID,
+	PIN_MAP_TYPE_DUMMY_STATE,
+	PIN_MAP_TYPE_MUX_GROUP,
+	PIN_MAP_TYPE_CONFIGS_PIN,
+	PIN_MAP_TYPE_CONFIGS_GROUP,
+};
+
+/**
+ * struct pinctrl_map_mux - mapping table content for MAP_TYPE_MUX_GROUP
+ * @group: the name of the group whose mux function is to be configured. This
+ *	field may be left NULL, and the first applicable group for the function
+ *	will be used.
+ * @function: the mux function to select for the group
+ */
+struct pinctrl_map_mux {
+	const char *group;
+	const char *function;
+};
+
+/**
+ * struct pinctrl_map_configs - mapping table content for MAP_TYPE_CONFIGS_*
+ * @group_or_pin: the name of the pin or group whose configuration parameters
+ *	are to be configured.
+ * @configs: a pointer to an array of config parameters/values to program into
+ *	hardware. Each individual pin controller defines the format and meaning
+ *	of config parameters.
+ * @num_configs: the number of entries in array @configs
+ */
+struct pinctrl_map_configs {
+	const char *group_or_pin;
+	unsigned long *configs;
+	unsigned num_configs;
+};
+
 /**
  * struct pinctrl_map - boards/machines shall provide this map for devices
  * @dev_name: the name of the device using this specific mapping, the name
@@ -22,46 +57,96 @@
  *	hogged by the driver itself upon registration
  * @name: the name of this specific map entry for the particular machine.
  *	This is the parameter passed to pinmux_lookup_state()
+ * @type: the type of mapping table entry
  * @ctrl_dev_name: the name of the device controlling this specific mapping,
- *	the name must be the same as in your struct device*
- * @group: sometimes a function can map to different pin groups, so this
- *	selects a certain specific pin group to activate for the function, if
- *	left as NULL, the first applicable group will be used
- * @function: a function in the driver to use for this mapping, the driver
- *	will lookup the function referenced by this ID on the specified
- *	pin control device
+ *	the name must be the same as in your struct device*. This field is not
+ *	used for PIN_MAP_TYPE_DUMMY_STATE
+ * @data: Data specific to the mapping type
  */
 struct pinctrl_map {
 	const char *dev_name;
 	const char *name;
+	enum pinctrl_map_type type;
 	const char *ctrl_dev_name;
-	const char *group;
-	const char *function;
+	union {
+		struct pinctrl_map_mux mux;
+		struct pinctrl_map_configs configs;
+	} data;
 };
 
-/*
- * Convenience macro to set a simple map from a certain pin controller and a
- * certain function to a named device
- */
-#define PIN_MAP(a, b, c, d) \
-	{ .name = a, .ctrl_dev_name = b, .function = c, .dev_name = d }
+/* Convenience macros to create mapping table entries */
 
-/*
- * Convenience macro to map a system function onto a certain pinctrl device,
- * to be hogged by the pin control core until the system shuts down.
- */
-#define PIN_MAP_SYS_HOG(a, b) \
-	{ .name = PINCTRL_STATE_DEFAULT, .ctrl_dev_name = a, .dev_name = a, \
-	  .function = b, }
+#define PIN_MAP_DUMMY_STATE(dev, state) \
+	{								\
+		.dev_name = dev,					\
+		.name = state,						\
+		.type = PIN_MAP_TYPE_DUMMY_STATE,			\
+	}
 
-/*
- * Convenience macro to map a system function onto a certain pinctrl device
- * using a specified group, to be hogged by the pin control core until the
- * system shuts down.
- */
-#define PIN_MAP_SYS_HOG_GROUP(a, b, c) \
-	{ .name = PINCTRL_STATE_DEFAULT, .ctrl_dev_name = a, .dev_name = a, \
-	  .function = b, .group = c, }
+#define PIN_MAP_MUX_GROUP(dev, state, pinctrl, grp, func)		\
+	{								\
+		.dev_name = dev,					\
+		.name = state,						\
+		.type = PIN_MAP_TYPE_MUX_GROUP,				\
+		.ctrl_dev_name = pinctrl,				\
+		.data.mux = {						\
+			.group = grp,					\
+			.function = func,				\
+		},							\
+	}
+
+#define PIN_MAP_MUX_GROUP_DEFAULT(dev, pinctrl, grp, func)		\
+	PIN_MAP_MUX_GROUP(dev, PINCTRL_STATE_DEFAULT, pinctrl, grp, func)
+
+#define PIN_MAP_MUX_GROUP_HOG(dev, state, grp, func)			\
+	PIN_MAP_MUX_GROUP(dev, state, dev, grp, func)
+
+#define PIN_MAP_MUX_GROUP_HOG_DEFAULT(dev, grp, func)			\
+	PIN_MAP_MUX_GROUP(dev, PINCTRL_STATE_DEFAULT, dev, grp, func)
+
+#define PIN_MAP_MUX_CONFIGS_PIN(dev, state, pinctrl, pin, cfgs)		\
+	{								\
+		.dev_name = dev,					\
+		.name = state,						\
+		.type = PIN_MAP_TYPE_CONFIGS_PIN,			\
+		.ctrl_dev_name = pinctrl,				\
+		.data.configs = {					\
+			.group_or_pin = pin,				\
+			.configs = cfgs,				\
+			.num_configs = ARRAY_SIZE(cfgs),		\
+		},							\
+	}
+
+#define PIN_MAP_MUX_CONFIGS_PIN_DEFAULT(dev, pinctrl, pin, cfgs)	\
+	PIN_MAP_MUX_CONFIGS_PIN(dev, PINCTRL_STATE_DEFAULT, pinctrl, pin, cfgs)
+
+#define PIN_MAP_MUX_CONFIGS_PIN_HOG(dev, state, pin, cfgs)		\
+	PIN_MAP_MUX_CONFIGS_PIN(dev, state, dev, pin, cfgs)
+
+#define PIN_MAP_MUX_CONFIGS_PIN_HOG_DEFAULT(dev, pin, cfgs)		\
+	PIN_MAP_MUX_CONFIGS_PIN(dev, PINCTRL_STATE_DEFAULT, dev, pin, cfgs)
+
+#define PIN_MAP_MUX_CONFIGS_GROUP(dev, state, pinctrl, grp, cfgs)	\
+	{								\
+		.dev_name = dev,					\
+		.name = state,						\
+		.type = PIN_MAP_TYPE_CONFIGS_GROUP,			\
+		.ctrl_dev_name = pinctrl,				\
+		.data.configs = {					\
+			.group_or_pin = grp,				\
+			.configs = cfgs,				\
+			.num_configs = ARRAY_SIZE(cfgs),		\
+		},							\
+	}
+
+#define PIN_MAP_MUX_CONFIGS_GROUP_DEFAULT(dev, pinctrl, grp, cfgs)	\
+	PIN_MAP_MUX_CONFIGS_GROUP(dev, PINCTRL_STATE_DEFAULT, pinctrl, grp, cfgs)
+
+#define PIN_MAP_MUX_CONFIGS_GROUP_HOG(dev, state, grp, cfgs)		\
+	PIN_MAP_MUX_CONFIGS_GROUP(dev, state, dev, grp, cfgs)
+
+#define PIN_MAP_MUX_CONFIGS_GROUP_HOG_DEFAULT(dev, grp, cfgs)		\
+	PIN_MAP_MUX_CONFIGS_GROUP(dev, PINCTRL_STATE_DEFAULT, dev, grp, cfgs)
 
 #ifdef CONFIG_PINMUX
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V2 6/6] pinctrl: fix case of Tegra30's foo_groups[] arrays
  2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
                   ` (4 preceding siblings ...)
  2012-02-28 20:27 ` [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations Stephen Warren
@ 2012-02-28 20:27 ` Stephen Warren
  2012-02-29 16:46 ` [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Linus Walleij
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-02-28 20:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel, Stephen Warren

Tegra's pin groups all have lower-case names. Fix the foo_groups[] arrays
in the driver to be lower-case to match.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: New patch.

 drivers/pinctrl/pinctrl-tegra30.c | 1992 ++++++++++++++++++------------------
 1 files changed, 996 insertions(+), 996 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-tegra30.c b/drivers/pinctrl/pinctrl-tegra30.c
index 1ea1e6a..4d7571d 100644
--- a/drivers/pinctrl/pinctrl-tegra30.c
+++ b/drivers/pinctrl/pinctrl-tegra30.c
@@ -2014,1242 +2014,1242 @@ enum tegra_mux {
 	TEGRA_MUX_VI_ALT3,
 };
 static const char * const blink_groups[] = {
-	"CLK_32K_OUT_PA0",
+	"clk_32k_out_pa0",
 };
 
 static const char * const cec_groups[] = {
-	"HDMI_CEC_PEE3",
-	"OWR",
+	"hdmi_cec_pee3",
+	"owr",
 };
 
 static const char * const clk_12m_out_groups[] = {
-	"PV3",
+	"pv3",
 };
 
 static const char * const clk_32k_in_groups[] = {
-	"CLK_32K_IN",
+	"clk_32k_in",
 };
 
 static const char * const core_pwr_req_groups[] = {
-	"CORE_PWR_REQ",
+	"core_pwr_req",
 };
 
 static const char * const cpu_pwr_req_groups[] = {
-	"CPU_PWR_REQ",
+	"cpu_pwr_req",
 };
 
 static const char * const crt_groups[] = {
-	"CRT_HSYNC_PV6",
-	"CRT_VSYNC_PV7",
+	"crt_hsync_pv6",
+	"crt_vsync_pv7",
 };
 
 static const char * const dap_groups[] = {
-	"CLK1_REQ_PEE2",
-	"CLK2_REQ_PCC5",
+	"clk1_req_pee2",
+	"clk2_req_pcc5",
 };
 
 static const char * const ddr_groups[] = {
-	"VI_D0_PT4",
-	"VI_D1_PD5",
-	"VI_D10_PT2",
-	"VI_D11_PT3",
-	"VI_D2_PL0",
-	"VI_D3_PL1",
-	"VI_D4_PL2",
-	"VI_D5_PL3",
-	"VI_D6_PL4",
-	"VI_D7_PL5",
-	"VI_D8_PL6",
-	"VI_D9_PL7",
-	"VI_HSYNC_PD7",
-	"VI_VSYNC_PD6",
+	"vi_d0_pt4",
+	"vi_d1_pd5",
+	"vi_d10_pt2",
+	"vi_d11_pt3",
+	"vi_d2_pl0",
+	"vi_d3_pl1",
+	"vi_d4_pl2",
+	"vi_d5_pl3",
+	"vi_d6_pl4",
+	"vi_d7_pl5",
+	"vi_d8_pl6",
+	"vi_d9_pl7",
+	"vi_hsync_pd7",
+	"vi_vsync_pd6",
 };
 
 static const char * const dev3_groups[] = {
-	"CLK3_REQ_PEE1",
+	"clk3_req_pee1",
 };
 
 static const char * const displaya_groups[] = {
-	"DAP3_DIN_PP1",
-	"DAP3_DOUT_PP2",
-	"DAP3_FS_PP0",
-	"DAP3_SCLK_PP3",
-	"PBB3",
-	"PBB4",
-	"PBB5",
-	"PBB6",
-	"LCD_CS0_N_PN4",
-	"LCD_CS1_N_PW0",
-	"LCD_D0_PE0",
-	"LCD_D1_PE1",
-	"LCD_D10_PF2",
-	"LCD_D11_PF3",
-	"LCD_D12_PF4",
-	"LCD_D13_PF5",
-	"LCD_D14_PF6",
-	"LCD_D15_PF7",
-	"LCD_D16_PM0",
-	"LCD_D17_PM1",
-	"LCD_D18_PM2",
-	"LCD_D19_PM3",
-	"LCD_D2_PE2",
-	"LCD_D20_PM4",
-	"LCD_D21_PM5",
-	"LCD_D22_PM6",
-	"LCD_D23_PM7",
-	"LCD_D3_PE3",
-	"LCD_D4_PE4",
-	"LCD_D5_PE5",
-	"LCD_D6_PE6",
-	"LCD_D7_PE7",
-	"LCD_D8_PF0",
-	"LCD_D9_PF1",
-	"LCD_DC0_PN6",
-	"LCD_DC1_PD2",
-	"LCD_DE_PJ1",
-	"LCD_HSYNC_PJ3",
-	"LCD_M1_PW1",
-	"LCD_PCLK_PB3",
-	"LCD_PWR0_PB2",
-	"LCD_PWR1_PC1",
-	"LCD_PWR2_PC6",
-	"LCD_SCK_PZ4",
-	"LCD_SDIN_PZ2",
-	"LCD_SDOUT_PN5",
-	"LCD_VSYNC_PJ4",
-	"LCD_WR_N_PZ3",
+	"dap3_din_pp1",
+	"dap3_dout_pp2",
+	"dap3_fs_pp0",
+	"dap3_sclk_pp3",
+	"pbb3",
+	"pbb4",
+	"pbb5",
+	"pbb6",
+	"lcd_cs0_n_pn4",
+	"lcd_cs1_n_pw0",
+	"lcd_d0_pe0",
+	"lcd_d1_pe1",
+	"lcd_d10_pf2",
+	"lcd_d11_pf3",
+	"lcd_d12_pf4",
+	"lcd_d13_pf5",
+	"lcd_d14_pf6",
+	"lcd_d15_pf7",
+	"lcd_d16_pm0",
+	"lcd_d17_pm1",
+	"lcd_d18_pm2",
+	"lcd_d19_pm3",
+	"lcd_d2_pe2",
+	"lcd_d20_pm4",
+	"lcd_d21_pm5",
+	"lcd_d22_pm6",
+	"lcd_d23_pm7",
+	"lcd_d3_pe3",
+	"lcd_d4_pe4",
+	"lcd_d5_pe5",
+	"lcd_d6_pe6",
+	"lcd_d7_pe7",
+	"lcd_d8_pf0",
+	"lcd_d9_pf1",
+	"lcd_dc0_pn6",
+	"lcd_dc1_pd2",
+	"lcd_de_pj1",
+	"lcd_hsync_pj3",
+	"lcd_m1_pw1",
+	"lcd_pclk_pb3",
+	"lcd_pwr0_pb2",
+	"lcd_pwr1_pc1",
+	"lcd_pwr2_pc6",
+	"lcd_sck_pz4",
+	"lcd_sdin_pz2",
+	"lcd_sdout_pn5",
+	"lcd_vsync_pj4",
+	"lcd_wr_n_pz3",
 };
 
 static const char * const displayb_groups[] = {
-	"DAP3_DIN_PP1",
-	"DAP3_DOUT_PP2",
-	"DAP3_FS_PP0",
-	"DAP3_SCLK_PP3",
-	"PBB3",
-	"PBB4",
-	"PBB5",
-	"PBB6",
-	"LCD_CS0_N_PN4",
-	"LCD_CS1_N_PW0",
-	"LCD_D0_PE0",
-	"LCD_D1_PE1",
-	"LCD_D10_PF2",
-	"LCD_D11_PF3",
-	"LCD_D12_PF4",
-	"LCD_D13_PF5",
-	"LCD_D14_PF6",
-	"LCD_D15_PF7",
-	"LCD_D16_PM0",
-	"LCD_D17_PM1",
-	"LCD_D18_PM2",
-	"LCD_D19_PM3",
-	"LCD_D2_PE2",
-	"LCD_D20_PM4",
-	"LCD_D21_PM5",
-	"LCD_D22_PM6",
-	"LCD_D23_PM7",
-	"LCD_D3_PE3",
-	"LCD_D4_PE4",
-	"LCD_D5_PE5",
-	"LCD_D6_PE6",
-	"LCD_D7_PE7",
-	"LCD_D8_PF0",
-	"LCD_D9_PF1",
-	"LCD_DC0_PN6",
-	"LCD_DC1_PD2",
-	"LCD_DE_PJ1",
-	"LCD_HSYNC_PJ3",
-	"LCD_M1_PW1",
-	"LCD_PCLK_PB3",
-	"LCD_PWR0_PB2",
-	"LCD_PWR1_PC1",
-	"LCD_PWR2_PC6",
-	"LCD_SCK_PZ4",
-	"LCD_SDIN_PZ2",
-	"LCD_SDOUT_PN5",
-	"LCD_VSYNC_PJ4",
-	"LCD_WR_N_PZ3",
+	"dap3_din_pp1",
+	"dap3_dout_pp2",
+	"dap3_fs_pp0",
+	"dap3_sclk_pp3",
+	"pbb3",
+	"pbb4",
+	"pbb5",
+	"pbb6",
+	"lcd_cs0_n_pn4",
+	"lcd_cs1_n_pw0",
+	"lcd_d0_pe0",
+	"lcd_d1_pe1",
+	"lcd_d10_pf2",
+	"lcd_d11_pf3",
+	"lcd_d12_pf4",
+	"lcd_d13_pf5",
+	"lcd_d14_pf6",
+	"lcd_d15_pf7",
+	"lcd_d16_pm0",
+	"lcd_d17_pm1",
+	"lcd_d18_pm2",
+	"lcd_d19_pm3",
+	"lcd_d2_pe2",
+	"lcd_d20_pm4",
+	"lcd_d21_pm5",
+	"lcd_d22_pm6",
+	"lcd_d23_pm7",
+	"lcd_d3_pe3",
+	"lcd_d4_pe4",
+	"lcd_d5_pe5",
+	"lcd_d6_pe6",
+	"lcd_d7_pe7",
+	"lcd_d8_pf0",
+	"lcd_d9_pf1",
+	"lcd_dc0_pn6",
+	"lcd_dc1_pd2",
+	"lcd_de_pj1",
+	"lcd_hsync_pj3",
+	"lcd_m1_pw1",
+	"lcd_pclk_pb3",
+	"lcd_pwr0_pb2",
+	"lcd_pwr1_pc1",
+	"lcd_pwr2_pc6",
+	"lcd_sck_pz4",
+	"lcd_sdin_pz2",
+	"lcd_sdout_pn5",
+	"lcd_vsync_pj4",
+	"lcd_wr_n_pz3",
 };
 
 static const char * const dtv_groups[] = {
-	"GMI_A17_PB0",
-	"GMI_A18_PB1",
-	"GMI_CS0_N_PJ0",
-	"GMI_CS1_N_PJ2",
+	"gmi_a17_pb0",
+	"gmi_a18_pb1",
+	"gmi_cs0_n_pj0",
+	"gmi_cs1_n_pj2",
 };
 
 static const char * const extperiph1_groups[] = {
-	"CLK1_OUT_PW4",
+	"clk1_out_pw4",
 };
 
 static const char * const extperiph2_groups[] = {
-	"CLK2_OUT_PW5",
+	"clk2_out_pw5",
 };
 
 static const char * const extperiph3_groups[] = {
-	"CLK3_OUT_PEE0",
+	"clk3_out_pee0",
 };
 
 static const char * const gmi_groups[] = {
-	"DAP1_DIN_PN1",
-	"DAP1_DOUT_PN2",
-	"DAP1_FS_PN0",
-	"DAP1_SCLK_PN3",
-	"DAP2_DIN_PA4",
-	"DAP2_DOUT_PA5",
-	"DAP2_FS_PA2",
-	"DAP2_SCLK_PA3",
-	"DAP4_DIN_PP5",
-	"DAP4_DOUT_PP6",
-	"DAP4_FS_PP4",
-	"DAP4_SCLK_PP7",
-	"GEN2_I2C_SCL_PT5",
-	"GEN2_I2C_SDA_PT6",
-	"GMI_A16_PJ7",
-	"GMI_A17_PB0",
-	"GMI_A18_PB1",
-	"GMI_A19_PK7",
-	"GMI_AD0_PG0",
-	"GMI_AD1_PG1",
-	"GMI_AD10_PH2",
-	"GMI_AD11_PH3",
-	"GMI_AD12_PH4",
-	"GMI_AD13_PH5",
-	"GMI_AD14_PH6",
-	"GMI_AD15_PH7",
-	"GMI_AD2_PG2",
-	"GMI_AD3_PG3",
-	"GMI_AD4_PG4",
-	"GMI_AD5_PG5",
-	"GMI_AD6_PG6",
-	"GMI_AD7_PG7",
-	"GMI_AD8_PH0",
-	"GMI_AD9_PH1",
-	"GMI_ADV_N_PK0",
-	"GMI_CLK_PK1",
-	"GMI_CS0_N_PJ0",
-	"GMI_CS1_N_PJ2",
-	"GMI_CS2_N_PK3",
-	"GMI_CS3_N_PK4",
-	"GMI_CS4_N_PK2",
-	"GMI_CS6_N_PI3",
-	"GMI_CS7_N_PI6",
-	"GMI_DQS_PI2",
-	"GMI_IORDY_PI5",
-	"GMI_OE_N_PI1",
-	"GMI_RST_N_PI4",
-	"GMI_WAIT_PI7",
-	"GMI_WP_N_PC7",
-	"GMI_WR_N_PI0",
-	"PU0",
-	"PU1",
-	"PU2",
-	"PU3",
-	"PU4",
-	"PU5",
-	"PU6",
-	"SDMMC4_CLK_PCC4",
-	"SDMMC4_CMD_PT7",
-	"SDMMC4_DAT0_PAA0",
-	"SDMMC4_DAT1_PAA1",
-	"SDMMC4_DAT2_PAA2",
-	"SDMMC4_DAT3_PAA3",
-	"SDMMC4_DAT4_PAA4",
-	"SDMMC4_DAT5_PAA5",
-	"SDMMC4_DAT6_PAA6",
-	"SDMMC4_DAT7_PAA7",
-	"SPI1_CS0_N_PX6",
-	"SPI1_MOSI_PX4",
-	"SPI1_SCK_PX5",
-	"SPI2_CS0_N_PX3",
-	"SPI2_MISO_PX1",
-	"SPI2_MOSI_PX0",
-	"SPI2_SCK_PX2",
-	"UART2_CTS_N_PJ5",
-	"UART2_RTS_N_PJ6",
-	"UART3_CTS_N_PA1",
-	"UART3_RTS_N_PC0",
-	"UART3_RXD_PW7",
-	"UART3_TXD_PW6",
+	"dap1_din_pn1",
+	"dap1_dout_pn2",
+	"dap1_fs_pn0",
+	"dap1_sclk_pn3",
+	"dap2_din_pa4",
+	"dap2_dout_pa5",
+	"dap2_fs_pa2",
+	"dap2_sclk_pa3",
+	"dap4_din_pp5",
+	"dap4_dout_pp6",
+	"dap4_fs_pp4",
+	"dap4_sclk_pp7",
+	"gen2_i2c_scl_pt5",
+	"gen2_i2c_sda_pt6",
+	"gmi_a16_pj7",
+	"gmi_a17_pb0",
+	"gmi_a18_pb1",
+	"gmi_a19_pk7",
+	"gmi_ad0_pg0",
+	"gmi_ad1_pg1",
+	"gmi_ad10_ph2",
+	"gmi_ad11_ph3",
+	"gmi_ad12_ph4",
+	"gmi_ad13_ph5",
+	"gmi_ad14_ph6",
+	"gmi_ad15_ph7",
+	"gmi_ad2_pg2",
+	"gmi_ad3_pg3",
+	"gmi_ad4_pg4",
+	"gmi_ad5_pg5",
+	"gmi_ad6_pg6",
+	"gmi_ad7_pg7",
+	"gmi_ad8_ph0",
+	"gmi_ad9_ph1",
+	"gmi_adv_n_pk0",
+	"gmi_clk_pk1",
+	"gmi_cs0_n_pj0",
+	"gmi_cs1_n_pj2",
+	"gmi_cs2_n_pk3",
+	"gmi_cs3_n_pk4",
+	"gmi_cs4_n_pk2",
+	"gmi_cs6_n_pi3",
+	"gmi_cs7_n_pi6",
+	"gmi_dqs_pi2",
+	"gmi_iordy_pi5",
+	"gmi_oe_n_pi1",
+	"gmi_rst_n_pi4",
+	"gmi_wait_pi7",
+	"gmi_wp_n_pc7",
+	"gmi_wr_n_pi0",
+	"pu0",
+	"pu1",
+	"pu2",
+	"pu3",
+	"pu4",
+	"pu5",
+	"pu6",
+	"sdmmc4_clk_pcc4",
+	"sdmmc4_cmd_pt7",
+	"sdmmc4_dat0_paa0",
+	"sdmmc4_dat1_paa1",
+	"sdmmc4_dat2_paa2",
+	"sdmmc4_dat3_paa3",
+	"sdmmc4_dat4_paa4",
+	"sdmmc4_dat5_paa5",
+	"sdmmc4_dat6_paa6",
+	"sdmmc4_dat7_paa7",
+	"spi1_cs0_n_px6",
+	"spi1_mosi_px4",
+	"spi1_sck_px5",
+	"spi2_cs0_n_px3",
+	"spi2_miso_px1",
+	"spi2_mosi_px0",
+	"spi2_sck_px2",
+	"uart2_cts_n_pj5",
+	"uart2_rts_n_pj6",
+	"uart3_cts_n_pa1",
+	"uart3_rts_n_pc0",
+	"uart3_rxd_pw7",
+	"uart3_txd_pw6",
 };
 
 static const char * const gmi_alt_groups[] = {
-	"GMI_A16_PJ7",
-	"GMI_CS3_N_PK4",
-	"GMI_CS7_N_PI6",
-	"GMI_WP_N_PC7",
+	"gmi_a16_pj7",
+	"gmi_cs3_n_pk4",
+	"gmi_cs7_n_pi6",
+	"gmi_wp_n_pc7",
 };
 
 static const char * const hda_groups[] = {
-	"CLK1_REQ_PEE2",
-	"DAP1_DIN_PN1",
-	"DAP1_DOUT_PN2",
-	"DAP1_FS_PN0",
-	"DAP1_SCLK_PN3",
-	"DAP2_DIN_PA4",
-	"DAP2_DOUT_PA5",
-	"DAP2_FS_PA2",
-	"DAP2_SCLK_PA3",
-	"PEX_L0_CLKREQ_N_PDD2",
-	"PEX_L0_PRSNT_N_PDD0",
-	"PEX_L0_RST_N_PDD1",
-	"PEX_L1_CLKREQ_N_PDD6",
-	"PEX_L1_PRSNT_N_PDD4",
-	"PEX_L1_RST_N_PDD5",
-	"PEX_L2_CLKREQ_N_PCC7",
-	"PEX_L2_PRSNT_N_PDD7",
-	"PEX_L2_RST_N_PCC6",
-	"PEX_WAKE_N_PDD3",
-	"SPDIF_IN_PK6",
+	"clk1_req_pee2",
+	"dap1_din_pn1",
+	"dap1_dout_pn2",
+	"dap1_fs_pn0",
+	"dap1_sclk_pn3",
+	"dap2_din_pa4",
+	"dap2_dout_pa5",
+	"dap2_fs_pa2",
+	"dap2_sclk_pa3",
+	"pex_l0_clkreq_n_pdd2",
+	"pex_l0_prsnt_n_pdd0",
+	"pex_l0_rst_n_pdd1",
+	"pex_l1_clkreq_n_pdd6",
+	"pex_l1_prsnt_n_pdd4",
+	"pex_l1_rst_n_pdd5",
+	"pex_l2_clkreq_n_pcc7",
+	"pex_l2_prsnt_n_pdd7",
+	"pex_l2_rst_n_pcc6",
+	"pex_wake_n_pdd3",
+	"spdif_in_pk6",
 };
 
 static const char * const hdcp_groups[] = {
-	"GEN2_I2C_SCL_PT5",
-	"GEN2_I2C_SDA_PT6",
-	"LCD_PWR0_PB2",
-	"LCD_PWR2_PC6",
-	"LCD_SCK_PZ4",
-	"LCD_SDOUT_PN5",
-	"LCD_WR_N_PZ3",
+	"gen2_i2c_scl_pt5",
+	"gen2_i2c_sda_pt6",
+	"lcd_pwr0_pb2",
+	"lcd_pwr2_pc6",
+	"lcd_sck_pz4",
+	"lcd_sdout_pn5",
+	"lcd_wr_n_pz3",
 };
 
 static const char * const hdmi_groups[] = {
-	"HDMI_INT_PN7",
+	"hdmi_int_pn7",
 };
 
 static const char * const hsi_groups[] = {
-	"ULPI_DATA0_PO1",
-	"ULPI_DATA1_PO2",
-	"ULPI_DATA2_PO3",
-	"ULPI_DATA3_PO4",
-	"ULPI_DATA4_PO5",
-	"ULPI_DATA5_PO6",
-	"ULPI_DATA6_PO7",
-	"ULPI_DATA7_PO0",
+	"ulpi_data0_po1",
+	"ulpi_data1_po2",
+	"ulpi_data2_po3",
+	"ulpi_data3_po4",
+	"ulpi_data4_po5",
+	"ulpi_data5_po6",
+	"ulpi_data6_po7",
+	"ulpi_data7_po0",
 };
 
 static const char * const i2c1_groups[] = {
-	"GEN1_I2C_SCL_PC4",
-	"GEN1_I2C_SDA_PC5",
-	"SPDIF_IN_PK6",
-	"SPDIF_OUT_PK5",
-	"SPI2_CS1_N_PW2",
-	"SPI2_CS2_N_PW3",
+	"gen1_i2c_scl_pc4",
+	"gen1_i2c_sda_pc5",
+	"spdif_in_pk6",
+	"spdif_out_pk5",
+	"spi2_cs1_n_pw2",
+	"spi2_cs2_n_pw3",
 };
 
 static const char * const i2c2_groups[] = {
-	"GEN2_I2C_SCL_PT5",
-	"GEN2_I2C_SDA_PT6",
+	"gen2_i2c_scl_pt5",
+	"gen2_i2c_sda_pt6",
 };
 
 static const char * const i2c3_groups[] = {
-	"CAM_I2C_SCL_PBB1",
-	"CAM_I2C_SDA_PBB2",
-	"SDMMC4_CMD_PT7",
-	"SDMMC4_DAT4_PAA4",
+	"cam_i2c_scl_pbb1",
+	"cam_i2c_sda_pbb2",
+	"sdmmc4_cmd_pt7",
+	"sdmmc4_dat4_paa4",
 };
 
 static const char * const i2c4_groups[] = {
-	"DDC_SCL_PV4",
-	"DDC_SDA_PV5",
+	"ddc_scl_pv4",
+	"ddc_sda_pv5",
 };
 
 static const char * const i2cpwr_groups[] = {
-	"PWR_I2C_SCL_PZ6",
-	"PWR_I2C_SDA_PZ7",
+	"pwr_i2c_scl_pz6",
+	"pwr_i2c_sda_pz7",
 };
 
 static const char * const i2s0_groups[] = {
-	"DAP1_DIN_PN1",
-	"DAP1_DOUT_PN2",
-	"DAP1_FS_PN0",
-	"DAP1_SCLK_PN3",
+	"dap1_din_pn1",
+	"dap1_dout_pn2",
+	"dap1_fs_pn0",
+	"dap1_sclk_pn3",
 };
 
 static const char * const i2s1_groups[] = {
-	"DAP2_DIN_PA4",
-	"DAP2_DOUT_PA5",
-	"DAP2_FS_PA2",
-	"DAP2_SCLK_PA3",
+	"dap2_din_pa4",
+	"dap2_dout_pa5",
+	"dap2_fs_pa2",
+	"dap2_sclk_pa3",
 };
 
 static const char * const i2s2_groups[] = {
-	"DAP3_DIN_PP1",
-	"DAP3_DOUT_PP2",
-	"DAP3_FS_PP0",
-	"DAP3_SCLK_PP3",
+	"dap3_din_pp1",
+	"dap3_dout_pp2",
+	"dap3_fs_pp0",
+	"dap3_sclk_pp3",
 };
 
 static const char * const i2s3_groups[] = {
-	"DAP4_DIN_PP5",
-	"DAP4_DOUT_PP6",
-	"DAP4_FS_PP4",
-	"DAP4_SCLK_PP7",
+	"dap4_din_pp5",
+	"dap4_dout_pp6",
+	"dap4_fs_pp4",
+	"dap4_sclk_pp7",
 };
 
 static const char * const i2s4_groups[] = {
-	"PBB0",
-	"PBB7",
-	"PCC1",
-	"PCC2",
-	"SDMMC4_DAT4_PAA4",
-	"SDMMC4_DAT5_PAA5",
-	"SDMMC4_DAT6_PAA6",
-	"SDMMC4_DAT7_PAA7",
+	"pbb0",
+	"pbb7",
+	"pcc1",
+	"pcc2",
+	"sdmmc4_dat4_paa4",
+	"sdmmc4_dat5_paa5",
+	"sdmmc4_dat6_paa6",
+	"sdmmc4_dat7_paa7",
 };
 
 static const char * const invalid_groups[] = {
-	"KB_ROW3_PR3",
-	"SDMMC4_CLK_PCC4",
+	"kb_row3_pr3",
+	"sdmmc4_clk_pcc4",
 };
 
 static const char * const kbc_groups[] = {
-	"KB_COL0_PQ0",
-	"KB_COL1_PQ1",
-	"KB_COL2_PQ2",
-	"KB_COL3_PQ3",
-	"KB_COL4_PQ4",
-	"KB_COL5_PQ5",
-	"KB_COL6_PQ6",
-	"KB_COL7_PQ7",
-	"KB_ROW0_PR0",
-	"KB_ROW1_PR1",
-	"KB_ROW10_PS2",
-	"KB_ROW11_PS3",
-	"KB_ROW12_PS4",
-	"KB_ROW13_PS5",
-	"KB_ROW14_PS6",
-	"KB_ROW15_PS7",
-	"KB_ROW2_PR2",
-	"KB_ROW3_PR3",
-	"KB_ROW4_PR4",
-	"KB_ROW5_PR5",
-	"KB_ROW6_PR6",
-	"KB_ROW7_PR7",
-	"KB_ROW8_PS0",
-	"KB_ROW9_PS1",
+	"kb_col0_pq0",
+	"kb_col1_pq1",
+	"kb_col2_pq2",
+	"kb_col3_pq3",
+	"kb_col4_pq4",
+	"kb_col5_pq5",
+	"kb_col6_pq6",
+	"kb_col7_pq7",
+	"kb_row0_pr0",
+	"kb_row1_pr1",
+	"kb_row10_ps2",
+	"kb_row11_ps3",
+	"kb_row12_ps4",
+	"kb_row13_ps5",
+	"kb_row14_ps6",
+	"kb_row15_ps7",
+	"kb_row2_pr2",
+	"kb_row3_pr3",
+	"kb_row4_pr4",
+	"kb_row5_pr5",
+	"kb_row6_pr6",
+	"kb_row7_pr7",
+	"kb_row8_ps0",
+	"kb_row9_ps1",
 };
 
 static const char * const mio_groups[] = {
-	"KB_COL6_PQ6",
-	"KB_COL7_PQ7",
-	"KB_ROW10_PS2",
-	"KB_ROW11_PS3",
-	"KB_ROW12_PS4",
-	"KB_ROW13_PS5",
-	"KB_ROW14_PS6",
-	"KB_ROW15_PS7",
-	"KB_ROW6_PR6",
-	"KB_ROW7_PR7",
-	"KB_ROW8_PS0",
-	"KB_ROW9_PS1",
+	"kb_col6_pq6",
+	"kb_col7_pq7",
+	"kb_row10_ps2",
+	"kb_row11_ps3",
+	"kb_row12_ps4",
+	"kb_row13_ps5",
+	"kb_row14_ps6",
+	"kb_row15_ps7",
+	"kb_row6_pr6",
+	"kb_row7_pr7",
+	"kb_row8_ps0",
+	"kb_row9_ps1",
 };
 
 static const char * const nand_groups[] = {
-	"GMI_AD0_PG0",
-	"GMI_AD1_PG1",
-	"GMI_AD10_PH2",
-	"GMI_AD11_PH3",
-	"GMI_AD12_PH4",
-	"GMI_AD13_PH5",
-	"GMI_AD14_PH6",
-	"GMI_AD15_PH7",
-	"GMI_AD2_PG2",
-	"GMI_AD3_PG3",
-	"GMI_AD4_PG4",
-	"GMI_AD5_PG5",
-	"GMI_AD6_PG6",
-	"GMI_AD7_PG7",
-	"GMI_AD8_PH0",
-	"GMI_AD9_PH1",
-	"GMI_ADV_N_PK0",
-	"GMI_CLK_PK1",
-	"GMI_CS0_N_PJ0",
-	"GMI_CS1_N_PJ2",
-	"GMI_CS2_N_PK3",
-	"GMI_CS3_N_PK4",
-	"GMI_CS4_N_PK2",
-	"GMI_CS6_N_PI3",
-	"GMI_CS7_N_PI6",
-	"GMI_DQS_PI2",
-	"GMI_IORDY_PI5",
-	"GMI_OE_N_PI1",
-	"GMI_RST_N_PI4",
-	"GMI_WAIT_PI7",
-	"GMI_WP_N_PC7",
-	"GMI_WR_N_PI0",
-	"KB_COL0_PQ0",
-	"KB_COL1_PQ1",
-	"KB_COL2_PQ2",
-	"KB_COL3_PQ3",
-	"KB_COL4_PQ4",
-	"KB_COL5_PQ5",
-	"KB_COL6_PQ6",
-	"KB_COL7_PQ7",
-	"KB_ROW0_PR0",
-	"KB_ROW1_PR1",
-	"KB_ROW10_PS2",
-	"KB_ROW11_PS3",
-	"KB_ROW12_PS4",
-	"KB_ROW13_PS5",
-	"KB_ROW14_PS6",
-	"KB_ROW15_PS7",
-	"KB_ROW2_PR2",
-	"KB_ROW3_PR3",
-	"KB_ROW4_PR4",
-	"KB_ROW5_PR5",
-	"KB_ROW6_PR6",
-	"KB_ROW7_PR7",
-	"KB_ROW8_PS0",
-	"KB_ROW9_PS1",
-	"SDMMC4_CLK_PCC4",
-	"SDMMC4_CMD_PT7",
+	"gmi_ad0_pg0",
+	"gmi_ad1_pg1",
+	"gmi_ad10_ph2",
+	"gmi_ad11_ph3",
+	"gmi_ad12_ph4",
+	"gmi_ad13_ph5",
+	"gmi_ad14_ph6",
+	"gmi_ad15_ph7",
+	"gmi_ad2_pg2",
+	"gmi_ad3_pg3",
+	"gmi_ad4_pg4",
+	"gmi_ad5_pg5",
+	"gmi_ad6_pg6",
+	"gmi_ad7_pg7",
+	"gmi_ad8_ph0",
+	"gmi_ad9_ph1",
+	"gmi_adv_n_pk0",
+	"gmi_clk_pk1",
+	"gmi_cs0_n_pj0",
+	"gmi_cs1_n_pj2",
+	"gmi_cs2_n_pk3",
+	"gmi_cs3_n_pk4",
+	"gmi_cs4_n_pk2",
+	"gmi_cs6_n_pi3",
+	"gmi_cs7_n_pi6",
+	"gmi_dqs_pi2",
+	"gmi_iordy_pi5",
+	"gmi_oe_n_pi1",
+	"gmi_rst_n_pi4",
+	"gmi_wait_pi7",
+	"gmi_wp_n_pc7",
+	"gmi_wr_n_pi0",
+	"kb_col0_pq0",
+	"kb_col1_pq1",
+	"kb_col2_pq2",
+	"kb_col3_pq3",
+	"kb_col4_pq4",
+	"kb_col5_pq5",
+	"kb_col6_pq6",
+	"kb_col7_pq7",
+	"kb_row0_pr0",
+	"kb_row1_pr1",
+	"kb_row10_ps2",
+	"kb_row11_ps3",
+	"kb_row12_ps4",
+	"kb_row13_ps5",
+	"kb_row14_ps6",
+	"kb_row15_ps7",
+	"kb_row2_pr2",
+	"kb_row3_pr3",
+	"kb_row4_pr4",
+	"kb_row5_pr5",
+	"kb_row6_pr6",
+	"kb_row7_pr7",
+	"kb_row8_ps0",
+	"kb_row9_ps1",
+	"sdmmc4_clk_pcc4",
+	"sdmmc4_cmd_pt7",
 };
 
 static const char * const nand_alt_groups[] = {
-	"GMI_CS6_N_PI3",
-	"GMI_CS7_N_PI6",
-	"GMI_RST_N_PI4",
+	"gmi_cs6_n_pi3",
+	"gmi_cs7_n_pi6",
+	"gmi_rst_n_pi4",
 };
 
 static const char * const owr_groups[] = {
-	"PU0",
-	"PV2",
-	"KB_ROW5_PR5",
-	"OWR",
+	"pu0",
+	"pv2",
+	"kb_row5_pr5",
+	"owr",
 };
 
 static const char * const pcie_groups[] = {
-	"PEX_L0_CLKREQ_N_PDD2",
-	"PEX_L0_PRSNT_N_PDD0",
-	"PEX_L0_RST_N_PDD1",
-	"PEX_L1_CLKREQ_N_PDD6",
-	"PEX_L1_PRSNT_N_PDD4",
-	"PEX_L1_RST_N_PDD5",
-	"PEX_L2_CLKREQ_N_PCC7",
-	"PEX_L2_PRSNT_N_PDD7",
-	"PEX_L2_RST_N_PCC6",
-	"PEX_WAKE_N_PDD3",
+	"pex_l0_clkreq_n_pdd2",
+	"pex_l0_prsnt_n_pdd0",
+	"pex_l0_rst_n_pdd1",
+	"pex_l1_clkreq_n_pdd6",
+	"pex_l1_prsnt_n_pdd4",
+	"pex_l1_rst_n_pdd5",
+	"pex_l2_clkreq_n_pcc7",
+	"pex_l2_prsnt_n_pdd7",
+	"pex_l2_rst_n_pcc6",
+	"pex_wake_n_pdd3",
 };
 
 static const char * const pwm0_groups[] = {
-	"GMI_AD8_PH0",
-	"PU3",
-	"SDMMC3_DAT3_PB4",
-	"SDMMC3_DAT5_PD0",
-	"UART3_RTS_N_PC0",
+	"gmi_ad8_ph0",
+	"pu3",
+	"sdmmc3_dat3_pb4",
+	"sdmmc3_dat5_pd0",
+	"uart3_rts_n_pc0",
 };
 
 static const char * const pwm1_groups[] = {
-	"GMI_AD9_PH1",
-	"PU4",
-	"SDMMC3_DAT2_PB5",
-	"SDMMC3_DAT4_PD1",
+	"gmi_ad9_ph1",
+	"pu4",
+	"sdmmc3_dat2_pb5",
+	"sdmmc3_dat4_pd1",
 };
 
 static const char * const pwm2_groups[] = {
-	"GMI_AD10_PH2",
-	"PU5",
-	"SDMMC3_CLK_PA6",
+	"gmi_ad10_ph2",
+	"pu5",
+	"sdmmc3_clk_pa6",
 };
 
 static const char * const pwm3_groups[] = {
-	"GMI_AD11_PH3",
-	"PU6",
-	"SDMMC3_CMD_PA7",
+	"gmi_ad11_ph3",
+	"pu6",
+	"sdmmc3_cmd_pa7",
 };
 
 static const char * const pwr_int_n_groups[] = {
-	"PWR_INT_N",
+	"pwr_int_n",
 };
 
 static const char * const rsvd1_groups[] = {
-	"GMI_AD0_PG0",
-	"GMI_AD1_PG1",
-	"GMI_AD12_PH4",
-	"GMI_AD13_PH5",
-	"GMI_AD14_PH6",
-	"GMI_AD15_PH7",
-	"GMI_AD2_PG2",
-	"GMI_AD3_PG3",
-	"GMI_AD4_PG4",
-	"GMI_AD5_PG5",
-	"GMI_AD6_PG6",
-	"GMI_AD7_PG7",
-	"GMI_ADV_N_PK0",
-	"GMI_CLK_PK1",
-	"GMI_CS0_N_PJ0",
-	"GMI_CS1_N_PJ2",
-	"GMI_CS2_N_PK3",
-	"GMI_CS3_N_PK4",
-	"GMI_CS4_N_PK2",
-	"GMI_DQS_PI2",
-	"GMI_IORDY_PI5",
-	"GMI_OE_N_PI1",
-	"GMI_WAIT_PI7",
-	"GMI_WP_N_PC7",
-	"GMI_WR_N_PI0",
-	"PU1",
-	"PU2",
-	"PV0",
-	"PV1",
-	"SDMMC3_DAT0_PB7",
-	"SDMMC3_DAT1_PB6",
-	"SDMMC3_DAT2_PB5",
-	"SDMMC3_DAT3_PB4",
-	"VI_PCLK_PT0",
+	"gmi_ad0_pg0",
+	"gmi_ad1_pg1",
+	"gmi_ad12_ph4",
+	"gmi_ad13_ph5",
+	"gmi_ad14_ph6",
+	"gmi_ad15_ph7",
+	"gmi_ad2_pg2",
+	"gmi_ad3_pg3",
+	"gmi_ad4_pg4",
+	"gmi_ad5_pg5",
+	"gmi_ad6_pg6",
+	"gmi_ad7_pg7",
+	"gmi_adv_n_pk0",
+	"gmi_clk_pk1",
+	"gmi_cs0_n_pj0",
+	"gmi_cs1_n_pj2",
+	"gmi_cs2_n_pk3",
+	"gmi_cs3_n_pk4",
+	"gmi_cs4_n_pk2",
+	"gmi_dqs_pi2",
+	"gmi_iordy_pi5",
+	"gmi_oe_n_pi1",
+	"gmi_wait_pi7",
+	"gmi_wp_n_pc7",
+	"gmi_wr_n_pi0",
+	"pu1",
+	"pu2",
+	"pv0",
+	"pv1",
+	"sdmmc3_dat0_pb7",
+	"sdmmc3_dat1_pb6",
+	"sdmmc3_dat2_pb5",
+	"sdmmc3_dat3_pb4",
+	"vi_pclk_pt0",
 };
 
 static const char * const rsvd2_groups[] = {
-	"CLK1_OUT_PW4",
-	"CLK2_OUT_PW5",
-	"CLK2_REQ_PCC5",
-	"CLK3_OUT_PEE0",
-	"CLK3_REQ_PEE1",
-	"CLK_32K_IN",
-	"CLK_32K_OUT_PA0",
-	"CORE_PWR_REQ",
-	"CPU_PWR_REQ",
-	"CRT_HSYNC_PV6",
-	"CRT_VSYNC_PV7",
-	"DAP3_DIN_PP1",
-	"DAP3_DOUT_PP2",
-	"DAP3_FS_PP0",
-	"DAP3_SCLK_PP3",
-	"DAP4_DIN_PP5",
-	"DAP4_DOUT_PP6",
-	"DAP4_FS_PP4",
-	"DAP4_SCLK_PP7",
-	"DDC_SCL_PV4",
-	"DDC_SDA_PV5",
-	"GEN1_I2C_SCL_PC4",
-	"GEN1_I2C_SDA_PC5",
-	"PBB0",
-	"PBB7",
-	"PCC1",
-	"PCC2",
-	"PV0",
-	"PV1",
-	"PV2",
-	"PV3",
-	"HDMI_CEC_PEE3",
-	"HDMI_INT_PN7",
-	"JTAG_RTCK_PU7",
-	"PWR_I2C_SCL_PZ6",
-	"PWR_I2C_SDA_PZ7",
-	"PWR_INT_N",
-	"SDMMC1_CLK_PZ0",
-	"SDMMC1_CMD_PZ1",
-	"SDMMC1_DAT0_PY7",
-	"SDMMC1_DAT1_PY6",
-	"SDMMC1_DAT2_PY5",
-	"SDMMC1_DAT3_PY4",
-	"SDMMC3_DAT0_PB7",
-	"SDMMC3_DAT1_PB6",
-	"SDMMC4_RST_N_PCC3",
-	"SPDIF_OUT_PK5",
-	"SYS_CLK_REQ_PZ5",
-	"UART3_CTS_N_PA1",
-	"UART3_RXD_PW7",
-	"UART3_TXD_PW6",
-	"ULPI_CLK_PY0",
-	"ULPI_DIR_PY1",
-	"ULPI_NXT_PY2",
-	"ULPI_STP_PY3",
-	"VI_D0_PT4",
-	"VI_D10_PT2",
-	"VI_D11_PT3",
-	"VI_HSYNC_PD7",
-	"VI_VSYNC_PD6",
+	"clk1_out_pw4",
+	"clk2_out_pw5",
+	"clk2_req_pcc5",
+	"clk3_out_pee0",
+	"clk3_req_pee1",
+	"clk_32k_in",
+	"clk_32k_out_pa0",
+	"core_pwr_req",
+	"cpu_pwr_req",
+	"crt_hsync_pv6",
+	"crt_vsync_pv7",
+	"dap3_din_pp1",
+	"dap3_dout_pp2",
+	"dap3_fs_pp0",
+	"dap3_sclk_pp3",
+	"dap4_din_pp5",
+	"dap4_dout_pp6",
+	"dap4_fs_pp4",
+	"dap4_sclk_pp7",
+	"ddc_scl_pv4",
+	"ddc_sda_pv5",
+	"gen1_i2c_scl_pc4",
+	"gen1_i2c_sda_pc5",
+	"pbb0",
+	"pbb7",
+	"pcc1",
+	"pcc2",
+	"pv0",
+	"pv1",
+	"pv2",
+	"pv3",
+	"hdmi_cec_pee3",
+	"hdmi_int_pn7",
+	"jtag_rtck_pu7",
+	"pwr_i2c_scl_pz6",
+	"pwr_i2c_sda_pz7",
+	"pwr_int_n",
+	"sdmmc1_clk_pz0",
+	"sdmmc1_cmd_pz1",
+	"sdmmc1_dat0_py7",
+	"sdmmc1_dat1_py6",
+	"sdmmc1_dat2_py5",
+	"sdmmc1_dat3_py4",
+	"sdmmc3_dat0_pb7",
+	"sdmmc3_dat1_pb6",
+	"sdmmc4_rst_n_pcc3",
+	"spdif_out_pk5",
+	"sys_clk_req_pz5",
+	"uart3_cts_n_pa1",
+	"uart3_rxd_pw7",
+	"uart3_txd_pw6",
+	"ulpi_clk_py0",
+	"ulpi_dir_py1",
+	"ulpi_nxt_py2",
+	"ulpi_stp_py3",
+	"vi_d0_pt4",
+	"vi_d10_pt2",
+	"vi_d11_pt3",
+	"vi_hsync_pd7",
+	"vi_vsync_pd6",
 };
 
 static const char * const rsvd3_groups[] = {
-	"CAM_I2C_SCL_PBB1",
-	"CAM_I2C_SDA_PBB2",
-	"CLK1_OUT_PW4",
-	"CLK1_REQ_PEE2",
-	"CLK2_OUT_PW5",
-	"CLK2_REQ_PCC5",
-	"CLK3_OUT_PEE0",
-	"CLK3_REQ_PEE1",
-	"CLK_32K_IN",
-	"CLK_32K_OUT_PA0",
-	"CORE_PWR_REQ",
-	"CPU_PWR_REQ",
-	"CRT_HSYNC_PV6",
-	"CRT_VSYNC_PV7",
-	"DAP2_DIN_PA4",
-	"DAP2_DOUT_PA5",
-	"DAP2_FS_PA2",
-	"DAP2_SCLK_PA3",
-	"DDC_SCL_PV4",
-	"DDC_SDA_PV5",
-	"GEN1_I2C_SCL_PC4",
-	"GEN1_I2C_SDA_PC5",
-	"PBB0",
-	"PBB7",
-	"PCC1",
-	"PCC2",
-	"PV0",
-	"PV1",
-	"PV2",
-	"PV3",
-	"HDMI_CEC_PEE3",
-	"HDMI_INT_PN7",
-	"JTAG_RTCK_PU7",
-	"KB_ROW0_PR0",
-	"KB_ROW1_PR1",
-	"KB_ROW2_PR2",
-	"KB_ROW3_PR3",
-	"LCD_D0_PE0",
-	"LCD_D1_PE1",
-	"LCD_D10_PF2",
-	"LCD_D11_PF3",
-	"LCD_D12_PF4",
-	"LCD_D13_PF5",
-	"LCD_D14_PF6",
-	"LCD_D15_PF7",
-	"LCD_D16_PM0",
-	"LCD_D17_PM1",
-	"LCD_D18_PM2",
-	"LCD_D19_PM3",
-	"LCD_D2_PE2",
-	"LCD_D20_PM4",
-	"LCD_D21_PM5",
-	"LCD_D22_PM6",
-	"LCD_D23_PM7",
-	"LCD_D3_PE3",
-	"LCD_D4_PE4",
-	"LCD_D5_PE5",
-	"LCD_D6_PE6",
-	"LCD_D7_PE7",
-	"LCD_D8_PF0",
-	"LCD_D9_PF1",
-	"LCD_DC0_PN6",
-	"LCD_DC1_PD2",
-	"LCD_DE_PJ1",
-	"LCD_HSYNC_PJ3",
-	"LCD_M1_PW1",
-	"LCD_PCLK_PB3",
-	"LCD_PWR1_PC1",
-	"LCD_VSYNC_PJ4",
-	"OWR",
-	"PEX_L0_CLKREQ_N_PDD2",
-	"PEX_L0_PRSNT_N_PDD0",
-	"PEX_L0_RST_N_PDD1",
-	"PEX_L1_CLKREQ_N_PDD6",
-	"PEX_L1_PRSNT_N_PDD4",
-	"PEX_L1_RST_N_PDD5",
-	"PEX_L2_CLKREQ_N_PCC7",
-	"PEX_L2_PRSNT_N_PDD7",
-	"PEX_L2_RST_N_PCC6",
-	"PEX_WAKE_N_PDD3",
-	"PWR_I2C_SCL_PZ6",
-	"PWR_I2C_SDA_PZ7",
-	"PWR_INT_N",
-	"SDMMC1_CLK_PZ0",
-	"SDMMC1_CMD_PZ1",
-	"SDMMC4_RST_N_PCC3",
-	"SYS_CLK_REQ_PZ5",
+	"cam_i2c_scl_pbb1",
+	"cam_i2c_sda_pbb2",
+	"clk1_out_pw4",
+	"clk1_req_pee2",
+	"clk2_out_pw5",
+	"clk2_req_pcc5",
+	"clk3_out_pee0",
+	"clk3_req_pee1",
+	"clk_32k_in",
+	"clk_32k_out_pa0",
+	"core_pwr_req",
+	"cpu_pwr_req",
+	"crt_hsync_pv6",
+	"crt_vsync_pv7",
+	"dap2_din_pa4",
+	"dap2_dout_pa5",
+	"dap2_fs_pa2",
+	"dap2_sclk_pa3",
+	"ddc_scl_pv4",
+	"ddc_sda_pv5",
+	"gen1_i2c_scl_pc4",
+	"gen1_i2c_sda_pc5",
+	"pbb0",
+	"pbb7",
+	"pcc1",
+	"pcc2",
+	"pv0",
+	"pv1",
+	"pv2",
+	"pv3",
+	"hdmi_cec_pee3",
+	"hdmi_int_pn7",
+	"jtag_rtck_pu7",
+	"kb_row0_pr0",
+	"kb_row1_pr1",
+	"kb_row2_pr2",
+	"kb_row3_pr3",
+	"lcd_d0_pe0",
+	"lcd_d1_pe1",
+	"lcd_d10_pf2",
+	"lcd_d11_pf3",
+	"lcd_d12_pf4",
+	"lcd_d13_pf5",
+	"lcd_d14_pf6",
+	"lcd_d15_pf7",
+	"lcd_d16_pm0",
+	"lcd_d17_pm1",
+	"lcd_d18_pm2",
+	"lcd_d19_pm3",
+	"lcd_d2_pe2",
+	"lcd_d20_pm4",
+	"lcd_d21_pm5",
+	"lcd_d22_pm6",
+	"lcd_d23_pm7",
+	"lcd_d3_pe3",
+	"lcd_d4_pe4",
+	"lcd_d5_pe5",
+	"lcd_d6_pe6",
+	"lcd_d7_pe7",
+	"lcd_d8_pf0",
+	"lcd_d9_pf1",
+	"lcd_dc0_pn6",
+	"lcd_dc1_pd2",
+	"lcd_de_pj1",
+	"lcd_hsync_pj3",
+	"lcd_m1_pw1",
+	"lcd_pclk_pb3",
+	"lcd_pwr1_pc1",
+	"lcd_vsync_pj4",
+	"owr",
+	"pex_l0_clkreq_n_pdd2",
+	"pex_l0_prsnt_n_pdd0",
+	"pex_l0_rst_n_pdd1",
+	"pex_l1_clkreq_n_pdd6",
+	"pex_l1_prsnt_n_pdd4",
+	"pex_l1_rst_n_pdd5",
+	"pex_l2_clkreq_n_pcc7",
+	"pex_l2_prsnt_n_pdd7",
+	"pex_l2_rst_n_pcc6",
+	"pex_wake_n_pdd3",
+	"pwr_i2c_scl_pz6",
+	"pwr_i2c_sda_pz7",
+	"pwr_int_n",
+	"sdmmc1_clk_pz0",
+	"sdmmc1_cmd_pz1",
+	"sdmmc4_rst_n_pcc3",
+	"sys_clk_req_pz5",
 };
 
 static const char * const rsvd4_groups[] = {
-	"CLK1_OUT_PW4",
-	"CLK1_REQ_PEE2",
-	"CLK2_OUT_PW5",
-	"CLK2_REQ_PCC5",
-	"CLK3_OUT_PEE0",
-	"CLK3_REQ_PEE1",
-	"CLK_32K_IN",
-	"CLK_32K_OUT_PA0",
-	"CORE_PWR_REQ",
-	"CPU_PWR_REQ",
-	"CRT_HSYNC_PV6",
-	"CRT_VSYNC_PV7",
-	"DAP4_DIN_PP5",
-	"DAP4_DOUT_PP6",
-	"DAP4_FS_PP4",
-	"DAP4_SCLK_PP7",
-	"DDC_SCL_PV4",
-	"DDC_SDA_PV5",
-	"GEN1_I2C_SCL_PC4",
-	"GEN1_I2C_SDA_PC5",
-	"GEN2_I2C_SCL_PT5",
-	"GEN2_I2C_SDA_PT6",
-	"GMI_A19_PK7",
-	"GMI_AD0_PG0",
-	"GMI_AD1_PG1",
-	"GMI_AD10_PH2",
-	"GMI_AD11_PH3",
-	"GMI_AD12_PH4",
-	"GMI_AD13_PH5",
-	"GMI_AD14_PH6",
-	"GMI_AD15_PH7",
-	"GMI_AD2_PG2",
-	"GMI_AD3_PG3",
-	"GMI_AD4_PG4",
-	"GMI_AD5_PG5",
-	"GMI_AD6_PG6",
-	"GMI_AD7_PG7",
-	"GMI_AD8_PH0",
-	"GMI_AD9_PH1",
-	"GMI_ADV_N_PK0",
-	"GMI_CLK_PK1",
-	"GMI_CS2_N_PK3",
-	"GMI_CS4_N_PK2",
-	"GMI_DQS_PI2",
-	"GMI_IORDY_PI5",
-	"GMI_OE_N_PI1",
-	"GMI_RST_N_PI4",
-	"GMI_WAIT_PI7",
-	"GMI_WR_N_PI0",
-	"PCC2",
-	"PU0",
-	"PU1",
-	"PU2",
-	"PU3",
-	"PU4",
-	"PU5",
-	"PU6",
-	"PV0",
-	"PV1",
-	"PV2",
-	"PV3",
-	"HDMI_CEC_PEE3",
-	"HDMI_INT_PN7",
-	"JTAG_RTCK_PU7",
-	"KB_COL2_PQ2",
-	"KB_COL3_PQ3",
-	"KB_COL4_PQ4",
-	"KB_COL5_PQ5",
-	"KB_ROW0_PR0",
-	"KB_ROW1_PR1",
-	"KB_ROW2_PR2",
-	"KB_ROW4_PR4",
-	"LCD_CS0_N_PN4",
-	"LCD_CS1_N_PW0",
-	"LCD_D0_PE0",
-	"LCD_D1_PE1",
-	"LCD_D10_PF2",
-	"LCD_D11_PF3",
-	"LCD_D12_PF4",
-	"LCD_D13_PF5",
-	"LCD_D14_PF6",
-	"LCD_D15_PF7",
-	"LCD_D16_PM0",
-	"LCD_D17_PM1",
-	"LCD_D18_PM2",
-	"LCD_D19_PM3",
-	"LCD_D2_PE2",
-	"LCD_D20_PM4",
-	"LCD_D21_PM5",
-	"LCD_D22_PM6",
-	"LCD_D23_PM7",
-	"LCD_D3_PE3",
-	"LCD_D4_PE4",
-	"LCD_D5_PE5",
-	"LCD_D6_PE6",
-	"LCD_D7_PE7",
-	"LCD_D8_PF0",
-	"LCD_D9_PF1",
-	"LCD_DC0_PN6",
-	"LCD_DC1_PD2",
-	"LCD_DE_PJ1",
-	"LCD_HSYNC_PJ3",
-	"LCD_M1_PW1",
-	"LCD_PCLK_PB3",
-	"LCD_PWR1_PC1",
-	"LCD_SDIN_PZ2",
-	"LCD_VSYNC_PJ4",
-	"OWR",
-	"PEX_L0_CLKREQ_N_PDD2",
-	"PEX_L0_PRSNT_N_PDD0",
-	"PEX_L0_RST_N_PDD1",
-	"PEX_L1_CLKREQ_N_PDD6",
-	"PEX_L1_PRSNT_N_PDD4",
-	"PEX_L1_RST_N_PDD5",
-	"PEX_L2_CLKREQ_N_PCC7",
-	"PEX_L2_PRSNT_N_PDD7",
-	"PEX_L2_RST_N_PCC6",
-	"PEX_WAKE_N_PDD3",
-	"PWR_I2C_SCL_PZ6",
-	"PWR_I2C_SDA_PZ7",
-	"PWR_INT_N",
-	"SPI1_MISO_PX7",
-	"SYS_CLK_REQ_PZ5",
-	"UART3_CTS_N_PA1",
-	"UART3_RTS_N_PC0",
-	"UART3_RXD_PW7",
-	"UART3_TXD_PW6",
-	"VI_D0_PT4",
-	"VI_D1_PD5",
-	"VI_D10_PT2",
-	"VI_D11_PT3",
-	"VI_D2_PL0",
-	"VI_D3_PL1",
-	"VI_D4_PL2",
-	"VI_D5_PL3",
-	"VI_D6_PL4",
-	"VI_D7_PL5",
-	"VI_D8_PL6",
-	"VI_D9_PL7",
-	"VI_HSYNC_PD7",
-	"VI_PCLK_PT0",
-	"VI_VSYNC_PD6",
+	"clk1_out_pw4",
+	"clk1_req_pee2",
+	"clk2_out_pw5",
+	"clk2_req_pcc5",
+	"clk3_out_pee0",
+	"clk3_req_pee1",
+	"clk_32k_in",
+	"clk_32k_out_pa0",
+	"core_pwr_req",
+	"cpu_pwr_req",
+	"crt_hsync_pv6",
+	"crt_vsync_pv7",
+	"dap4_din_pp5",
+	"dap4_dout_pp6",
+	"dap4_fs_pp4",
+	"dap4_sclk_pp7",
+	"ddc_scl_pv4",
+	"ddc_sda_pv5",
+	"gen1_i2c_scl_pc4",
+	"gen1_i2c_sda_pc5",
+	"gen2_i2c_scl_pt5",
+	"gen2_i2c_sda_pt6",
+	"gmi_a19_pk7",
+	"gmi_ad0_pg0",
+	"gmi_ad1_pg1",
+	"gmi_ad10_ph2",
+	"gmi_ad11_ph3",
+	"gmi_ad12_ph4",
+	"gmi_ad13_ph5",
+	"gmi_ad14_ph6",
+	"gmi_ad15_ph7",
+	"gmi_ad2_pg2",
+	"gmi_ad3_pg3",
+	"gmi_ad4_pg4",
+	"gmi_ad5_pg5",
+	"gmi_ad6_pg6",
+	"gmi_ad7_pg7",
+	"gmi_ad8_ph0",
+	"gmi_ad9_ph1",
+	"gmi_adv_n_pk0",
+	"gmi_clk_pk1",
+	"gmi_cs2_n_pk3",
+	"gmi_cs4_n_pk2",
+	"gmi_dqs_pi2",
+	"gmi_iordy_pi5",
+	"gmi_oe_n_pi1",
+	"gmi_rst_n_pi4",
+	"gmi_wait_pi7",
+	"gmi_wr_n_pi0",
+	"pcc2",
+	"pu0",
+	"pu1",
+	"pu2",
+	"pu3",
+	"pu4",
+	"pu5",
+	"pu6",
+	"pv0",
+	"pv1",
+	"pv2",
+	"pv3",
+	"hdmi_cec_pee3",
+	"hdmi_int_pn7",
+	"jtag_rtck_pu7",
+	"kb_col2_pq2",
+	"kb_col3_pq3",
+	"kb_col4_pq4",
+	"kb_col5_pq5",
+	"kb_row0_pr0",
+	"kb_row1_pr1",
+	"kb_row2_pr2",
+	"kb_row4_pr4",
+	"lcd_cs0_n_pn4",
+	"lcd_cs1_n_pw0",
+	"lcd_d0_pe0",
+	"lcd_d1_pe1",
+	"lcd_d10_pf2",
+	"lcd_d11_pf3",
+	"lcd_d12_pf4",
+	"lcd_d13_pf5",
+	"lcd_d14_pf6",
+	"lcd_d15_pf7",
+	"lcd_d16_pm0",
+	"lcd_d17_pm1",
+	"lcd_d18_pm2",
+	"lcd_d19_pm3",
+	"lcd_d2_pe2",
+	"lcd_d20_pm4",
+	"lcd_d21_pm5",
+	"lcd_d22_pm6",
+	"lcd_d23_pm7",
+	"lcd_d3_pe3",
+	"lcd_d4_pe4",
+	"lcd_d5_pe5",
+	"lcd_d6_pe6",
+	"lcd_d7_pe7",
+	"lcd_d8_pf0",
+	"lcd_d9_pf1",
+	"lcd_dc0_pn6",
+	"lcd_dc1_pd2",
+	"lcd_de_pj1",
+	"lcd_hsync_pj3",
+	"lcd_m1_pw1",
+	"lcd_pclk_pb3",
+	"lcd_pwr1_pc1",
+	"lcd_sdin_pz2",
+	"lcd_vsync_pj4",
+	"owr",
+	"pex_l0_clkreq_n_pdd2",
+	"pex_l0_prsnt_n_pdd0",
+	"pex_l0_rst_n_pdd1",
+	"pex_l1_clkreq_n_pdd6",
+	"pex_l1_prsnt_n_pdd4",
+	"pex_l1_rst_n_pdd5",
+	"pex_l2_clkreq_n_pcc7",
+	"pex_l2_prsnt_n_pdd7",
+	"pex_l2_rst_n_pcc6",
+	"pex_wake_n_pdd3",
+	"pwr_i2c_scl_pz6",
+	"pwr_i2c_sda_pz7",
+	"pwr_int_n",
+	"spi1_miso_px7",
+	"sys_clk_req_pz5",
+	"uart3_cts_n_pa1",
+	"uart3_rts_n_pc0",
+	"uart3_rxd_pw7",
+	"uart3_txd_pw6",
+	"vi_d0_pt4",
+	"vi_d1_pd5",
+	"vi_d10_pt2",
+	"vi_d11_pt3",
+	"vi_d2_pl0",
+	"vi_d3_pl1",
+	"vi_d4_pl2",
+	"vi_d5_pl3",
+	"vi_d6_pl4",
+	"vi_d7_pl5",
+	"vi_d8_pl6",
+	"vi_d9_pl7",
+	"vi_hsync_pd7",
+	"vi_pclk_pt0",
+	"vi_vsync_pd6",
 };
 
 static const char * const rtck_groups[] = {
-	"JTAG_RTCK_PU7",
+	"jtag_rtck_pu7",
 };
 
 static const char * const sata_groups[] = {
-	"GMI_CS6_N_PI3",
+	"gmi_cs6_n_pi3",
 };
 
 static const char * const sdmmc1_groups[] = {
-	"SDMMC1_CLK_PZ0",
-	"SDMMC1_CMD_PZ1",
-	"SDMMC1_DAT0_PY7",
-	"SDMMC1_DAT1_PY6",
-	"SDMMC1_DAT2_PY5",
-	"SDMMC1_DAT3_PY4",
+	"sdmmc1_clk_pz0",
+	"sdmmc1_cmd_pz1",
+	"sdmmc1_dat0_py7",
+	"sdmmc1_dat1_py6",
+	"sdmmc1_dat2_py5",
+	"sdmmc1_dat3_py4",
 };
 
 static const char * const sdmmc2_groups[] = {
-	"DAP1_DIN_PN1",
-	"DAP1_DOUT_PN2",
-	"DAP1_FS_PN0",
-	"DAP1_SCLK_PN3",
-	"KB_ROW10_PS2",
-	"KB_ROW11_PS3",
-	"KB_ROW12_PS4",
-	"KB_ROW13_PS5",
-	"KB_ROW14_PS6",
-	"KB_ROW15_PS7",
-	"KB_ROW6_PR6",
-	"KB_ROW7_PR7",
-	"KB_ROW8_PS0",
-	"KB_ROW9_PS1",
-	"SPDIF_IN_PK6",
-	"SPDIF_OUT_PK5",
-	"VI_D1_PD5",
-	"VI_D2_PL0",
-	"VI_D3_PL1",
-	"VI_D4_PL2",
-	"VI_D5_PL3",
-	"VI_D6_PL4",
-	"VI_D7_PL5",
-	"VI_D8_PL6",
-	"VI_D9_PL7",
-	"VI_PCLK_PT0",
+	"dap1_din_pn1",
+	"dap1_dout_pn2",
+	"dap1_fs_pn0",
+	"dap1_sclk_pn3",
+	"kb_row10_ps2",
+	"kb_row11_ps3",
+	"kb_row12_ps4",
+	"kb_row13_ps5",
+	"kb_row14_ps6",
+	"kb_row15_ps7",
+	"kb_row6_pr6",
+	"kb_row7_pr7",
+	"kb_row8_ps0",
+	"kb_row9_ps1",
+	"spdif_in_pk6",
+	"spdif_out_pk5",
+	"vi_d1_pd5",
+	"vi_d2_pl0",
+	"vi_d3_pl1",
+	"vi_d4_pl2",
+	"vi_d5_pl3",
+	"vi_d6_pl4",
+	"vi_d7_pl5",
+	"vi_d8_pl6",
+	"vi_d9_pl7",
+	"vi_pclk_pt0",
 };
 
 static const char * const sdmmc3_groups[] = {
-	"SDMMC3_CLK_PA6",
-	"SDMMC3_CMD_PA7",
-	"SDMMC3_DAT0_PB7",
-	"SDMMC3_DAT1_PB6",
-	"SDMMC3_DAT2_PB5",
-	"SDMMC3_DAT3_PB4",
-	"SDMMC3_DAT4_PD1",
-	"SDMMC3_DAT5_PD0",
-	"SDMMC3_DAT6_PD3",
-	"SDMMC3_DAT7_PD4",
+	"sdmmc3_clk_pa6",
+	"sdmmc3_cmd_pa7",
+	"sdmmc3_dat0_pb7",
+	"sdmmc3_dat1_pb6",
+	"sdmmc3_dat2_pb5",
+	"sdmmc3_dat3_pb4",
+	"sdmmc3_dat4_pd1",
+	"sdmmc3_dat5_pd0",
+	"sdmmc3_dat6_pd3",
+	"sdmmc3_dat7_pd4",
 };
 
 static const char * const sdmmc4_groups[] = {
-	"CAM_I2C_SCL_PBB1",
-	"CAM_I2C_SDA_PBB2",
-	"CAM_MCLK_PCC0",
-	"PBB0",
-	"PBB3",
-	"PBB4",
-	"PBB5",
-	"PBB6",
-	"PBB7",
-	"PCC1",
-	"SDMMC4_CLK_PCC4",
-	"SDMMC4_CMD_PT7",
-	"SDMMC4_DAT0_PAA0",
-	"SDMMC4_DAT1_PAA1",
-	"SDMMC4_DAT2_PAA2",
-	"SDMMC4_DAT3_PAA3",
-	"SDMMC4_DAT4_PAA4",
-	"SDMMC4_DAT5_PAA5",
-	"SDMMC4_DAT6_PAA6",
-	"SDMMC4_DAT7_PAA7",
-	"SDMMC4_RST_N_PCC3",
+	"cam_i2c_scl_pbb1",
+	"cam_i2c_sda_pbb2",
+	"cam_mclk_pcc0",
+	"pbb0",
+	"pbb3",
+	"pbb4",
+	"pbb5",
+	"pbb6",
+	"pbb7",
+	"pcc1",
+	"sdmmc4_clk_pcc4",
+	"sdmmc4_cmd_pt7",
+	"sdmmc4_dat0_paa0",
+	"sdmmc4_dat1_paa1",
+	"sdmmc4_dat2_paa2",
+	"sdmmc4_dat3_paa3",
+	"sdmmc4_dat4_paa4",
+	"sdmmc4_dat5_paa5",
+	"sdmmc4_dat6_paa6",
+	"sdmmc4_dat7_paa7",
+	"sdmmc4_rst_n_pcc3",
 };
 
 static const char * const spdif_groups[] = {
-	"SDMMC3_DAT6_PD3",
-	"SDMMC3_DAT7_PD4",
-	"SPDIF_IN_PK6",
-	"SPDIF_OUT_PK5",
-	"UART2_RXD_PC3",
-	"UART2_TXD_PC2",
+	"sdmmc3_dat6_pd3",
+	"sdmmc3_dat7_pd4",
+	"spdif_in_pk6",
+	"spdif_out_pk5",
+	"uart2_rxd_pc3",
+	"uart2_txd_pc2",
 };
 
 static const char * const spi1_groups[] = {
-	"SPI1_CS0_N_PX6",
-	"SPI1_MISO_PX7",
-	"SPI1_MOSI_PX4",
-	"SPI1_SCK_PX5",
-	"ULPI_CLK_PY0",
-	"ULPI_DIR_PY1",
-	"ULPI_NXT_PY2",
-	"ULPI_STP_PY3",
+	"spi1_cs0_n_px6",
+	"spi1_miso_px7",
+	"spi1_mosi_px4",
+	"spi1_sck_px5",
+	"ulpi_clk_py0",
+	"ulpi_dir_py1",
+	"ulpi_nxt_py2",
+	"ulpi_stp_py3",
 };
 
 static const char * const spi2_groups[] = {
-	"SDMMC3_CMD_PA7",
-	"SDMMC3_DAT4_PD1",
-	"SDMMC3_DAT5_PD0",
-	"SDMMC3_DAT6_PD3",
-	"SDMMC3_DAT7_PD4",
-	"SPI1_CS0_N_PX6",
-	"SPI1_MOSI_PX4",
-	"SPI1_SCK_PX5",
-	"SPI2_CS0_N_PX3",
-	"SPI2_CS1_N_PW2",
-	"SPI2_CS2_N_PW3",
-	"SPI2_MISO_PX1",
-	"SPI2_MOSI_PX0",
-	"SPI2_SCK_PX2",
-	"ULPI_DATA4_PO5",
-	"ULPI_DATA5_PO6",
-	"ULPI_DATA6_PO7",
-	"ULPI_DATA7_PO0",
+	"sdmmc3_cmd_pa7",
+	"sdmmc3_dat4_pd1",
+	"sdmmc3_dat5_pd0",
+	"sdmmc3_dat6_pd3",
+	"sdmmc3_dat7_pd4",
+	"spi1_cs0_n_px6",
+	"spi1_mosi_px4",
+	"spi1_sck_px5",
+	"spi2_cs0_n_px3",
+	"spi2_cs1_n_pw2",
+	"spi2_cs2_n_pw3",
+	"spi2_miso_px1",
+	"spi2_mosi_px0",
+	"spi2_sck_px2",
+	"ulpi_data4_po5",
+	"ulpi_data5_po6",
+	"ulpi_data6_po7",
+	"ulpi_data7_po0",
 };
 
 static const char * const spi2_alt_groups[] = {
-	"SPI1_CS0_N_PX6",
-	"SPI1_MISO_PX7",
-	"SPI1_MOSI_PX4",
-	"SPI1_SCK_PX5",
-	"SPI2_CS1_N_PW2",
-	"SPI2_CS2_N_PW3",
+	"spi1_cs0_n_px6",
+	"spi1_miso_px7",
+	"spi1_mosi_px4",
+	"spi1_sck_px5",
+	"spi2_cs1_n_pw2",
+	"spi2_cs2_n_pw3",
 };
 
 static const char * const spi3_groups[] = {
-	"SDMMC3_CLK_PA6",
-	"SDMMC3_DAT0_PB7",
-	"SDMMC3_DAT1_PB6",
-	"SDMMC3_DAT2_PB5",
-	"SDMMC3_DAT3_PB4",
-	"SDMMC4_DAT0_PAA0",
-	"SDMMC4_DAT1_PAA1",
-	"SDMMC4_DAT2_PAA2",
-	"SDMMC4_DAT3_PAA3",
-	"SPI1_MISO_PX7",
-	"SPI2_CS0_N_PX3",
-	"SPI2_CS1_N_PW2",
-	"SPI2_CS2_N_PW3",
-	"SPI2_MISO_PX1",
-	"SPI2_MOSI_PX0",
-	"SPI2_SCK_PX2",
-	"ULPI_DATA0_PO1",
-	"ULPI_DATA1_PO2",
-	"ULPI_DATA2_PO3",
-	"ULPI_DATA3_PO4",
+	"sdmmc3_clk_pa6",
+	"sdmmc3_dat0_pb7",
+	"sdmmc3_dat1_pb6",
+	"sdmmc3_dat2_pb5",
+	"sdmmc3_dat3_pb4",
+	"sdmmc4_dat0_paa0",
+	"sdmmc4_dat1_paa1",
+	"sdmmc4_dat2_paa2",
+	"sdmmc4_dat3_paa3",
+	"spi1_miso_px7",
+	"spi2_cs0_n_px3",
+	"spi2_cs1_n_pw2",
+	"spi2_cs2_n_pw3",
+	"spi2_miso_px1",
+	"spi2_mosi_px0",
+	"spi2_sck_px2",
+	"ulpi_data0_po1",
+	"ulpi_data1_po2",
+	"ulpi_data2_po3",
+	"ulpi_data3_po4",
 };
 
 static const char * const spi4_groups[] = {
-	"GMI_A16_PJ7",
-	"GMI_A17_PB0",
-	"GMI_A18_PB1",
-	"GMI_A19_PK7",
-	"SDMMC3_DAT4_PD1",
-	"SDMMC3_DAT5_PD0",
-	"SDMMC3_DAT6_PD3",
-	"SDMMC3_DAT7_PD4",
-	"UART2_CTS_N_PJ5",
-	"UART2_RTS_N_PJ6",
-	"UART2_RXD_PC3",
-	"UART2_TXD_PC2",
+	"gmi_a16_pj7",
+	"gmi_a17_pb0",
+	"gmi_a18_pb1",
+	"gmi_a19_pk7",
+	"sdmmc3_dat4_pd1",
+	"sdmmc3_dat5_pd0",
+	"sdmmc3_dat6_pd3",
+	"sdmmc3_dat7_pd4",
+	"uart2_cts_n_pj5",
+	"uart2_rts_n_pj6",
+	"uart2_rxd_pc3",
+	"uart2_txd_pc2",
 };
 
 static const char * const spi5_groups[] = {
-	"LCD_CS0_N_PN4",
-	"LCD_CS1_N_PW0",
-	"LCD_PWR0_PB2",
-	"LCD_PWR2_PC6",
-	"LCD_SCK_PZ4",
-	"LCD_SDIN_PZ2",
-	"LCD_SDOUT_PN5",
-	"LCD_WR_N_PZ3",
+	"lcd_cs0_n_pn4",
+	"lcd_cs1_n_pw0",
+	"lcd_pwr0_pb2",
+	"lcd_pwr2_pc6",
+	"lcd_sck_pz4",
+	"lcd_sdin_pz2",
+	"lcd_sdout_pn5",
+	"lcd_wr_n_pz3",
 };
 
 static const char * const spi6_groups[] = {
-	"SPI2_CS0_N_PX3",
-	"SPI2_MISO_PX1",
-	"SPI2_MOSI_PX0",
-	"SPI2_SCK_PX2",
+	"spi2_cs0_n_px3",
+	"spi2_miso_px1",
+	"spi2_mosi_px0",
+	"spi2_sck_px2",
 };
 
 static const char * const sysclk_groups[] = {
-	"SYS_CLK_REQ_PZ5",
+	"sys_clk_req_pz5",
 };
 
 static const char * const test_groups[] = {
-	"KB_COL0_PQ0",
-	"KB_COL1_PQ1",
+	"kb_col0_pq0",
+	"kb_col1_pq1",
 };
 
 static const char * const trace_groups[] = {
-	"KB_COL0_PQ0",
-	"KB_COL1_PQ1",
-	"KB_COL2_PQ2",
-	"KB_COL3_PQ3",
-	"KB_COL4_PQ4",
-	"KB_COL5_PQ5",
-	"KB_COL6_PQ6",
-	"KB_COL7_PQ7",
-	"KB_ROW4_PR4",
-	"KB_ROW5_PR5",
+	"kb_col0_pq0",
+	"kb_col1_pq1",
+	"kb_col2_pq2",
+	"kb_col3_pq3",
+	"kb_col4_pq4",
+	"kb_col5_pq5",
+	"kb_col6_pq6",
+	"kb_col7_pq7",
+	"kb_row4_pr4",
+	"kb_row5_pr5",
 };
 
 static const char * const uarta_groups[] = {
-	"PU0",
-	"PU1",
-	"PU2",
-	"PU3",
-	"PU4",
-	"PU5",
-	"PU6",
-	"SDMMC1_CLK_PZ0",
-	"SDMMC1_CMD_PZ1",
-	"SDMMC1_DAT0_PY7",
-	"SDMMC1_DAT1_PY6",
-	"SDMMC1_DAT2_PY5",
-	"SDMMC1_DAT3_PY4",
-	"SDMMC3_CLK_PA6",
-	"SDMMC3_CMD_PA7",
-	"UART2_CTS_N_PJ5",
-	"UART2_RTS_N_PJ6",
-	"UART2_RXD_PC3",
-	"UART2_TXD_PC2",
-	"ULPI_DATA0_PO1",
-	"ULPI_DATA1_PO2",
-	"ULPI_DATA2_PO3",
-	"ULPI_DATA3_PO4",
-	"ULPI_DATA4_PO5",
-	"ULPI_DATA5_PO6",
-	"ULPI_DATA6_PO7",
-	"ULPI_DATA7_PO0",
+	"pu0",
+	"pu1",
+	"pu2",
+	"pu3",
+	"pu4",
+	"pu5",
+	"pu6",
+	"sdmmc1_clk_pz0",
+	"sdmmc1_cmd_pz1",
+	"sdmmc1_dat0_py7",
+	"sdmmc1_dat1_py6",
+	"sdmmc1_dat2_py5",
+	"sdmmc1_dat3_py4",
+	"sdmmc3_clk_pa6",
+	"sdmmc3_cmd_pa7",
+	"uart2_cts_n_pj5",
+	"uart2_rts_n_pj6",
+	"uart2_rxd_pc3",
+	"uart2_txd_pc2",
+	"ulpi_data0_po1",
+	"ulpi_data1_po2",
+	"ulpi_data2_po3",
+	"ulpi_data3_po4",
+	"ulpi_data4_po5",
+	"ulpi_data5_po6",
+	"ulpi_data6_po7",
+	"ulpi_data7_po0",
 };
 
 static const char * const uartb_groups[] = {
-	"UART2_CTS_N_PJ5",
-	"UART2_RTS_N_PJ6",
-	"UART2_RXD_PC3",
-	"UART2_TXD_PC2",
+	"uart2_cts_n_pj5",
+	"uart2_rts_n_pj6",
+	"uart2_rxd_pc3",
+	"uart2_txd_pc2",
 };
 
 static const char * const uartc_groups[] = {
-	"UART3_CTS_N_PA1",
-	"UART3_RTS_N_PC0",
-	"UART3_RXD_PW7",
-	"UART3_TXD_PW6",
+	"uart3_cts_n_pa1",
+	"uart3_rts_n_pc0",
+	"uart3_rxd_pw7",
+	"uart3_txd_pw6",
 };
 
 static const char * const uartd_groups[] = {
-	"GMI_A16_PJ7",
-	"GMI_A17_PB0",
-	"GMI_A18_PB1",
-	"GMI_A19_PK7",
-	"ULPI_CLK_PY0",
-	"ULPI_DIR_PY1",
-	"ULPI_NXT_PY2",
-	"ULPI_STP_PY3",
+	"gmi_a16_pj7",
+	"gmi_a17_pb0",
+	"gmi_a18_pb1",
+	"gmi_a19_pk7",
+	"ulpi_clk_py0",
+	"ulpi_dir_py1",
+	"ulpi_nxt_py2",
+	"ulpi_stp_py3",
 };
 
 static const char * const uarte_groups[] = {
-	"SDMMC1_DAT0_PY7",
-	"SDMMC1_DAT1_PY6",
-	"SDMMC1_DAT2_PY5",
-	"SDMMC1_DAT3_PY4",
-	"SDMMC4_DAT0_PAA0",
-	"SDMMC4_DAT1_PAA1",
-	"SDMMC4_DAT2_PAA2",
-	"SDMMC4_DAT3_PAA3",
+	"sdmmc1_dat0_py7",
+	"sdmmc1_dat1_py6",
+	"sdmmc1_dat2_py5",
+	"sdmmc1_dat3_py4",
+	"sdmmc4_dat0_paa0",
+	"sdmmc4_dat1_paa1",
+	"sdmmc4_dat2_paa2",
+	"sdmmc4_dat3_paa3",
 };
 
 static const char * const ulpi_groups[] = {
-	"ULPI_CLK_PY0",
-	"ULPI_DATA0_PO1",
-	"ULPI_DATA1_PO2",
-	"ULPI_DATA2_PO3",
-	"ULPI_DATA3_PO4",
-	"ULPI_DATA4_PO5",
-	"ULPI_DATA5_PO6",
-	"ULPI_DATA6_PO7",
-	"ULPI_DATA7_PO0",
-	"ULPI_DIR_PY1",
-	"ULPI_NXT_PY2",
-	"ULPI_STP_PY3",
+	"ulpi_clk_py0",
+	"ulpi_data0_po1",
+	"ulpi_data1_po2",
+	"ulpi_data2_po3",
+	"ulpi_data3_po4",
+	"ulpi_data4_po5",
+	"ulpi_data5_po6",
+	"ulpi_data6_po7",
+	"ulpi_data7_po0",
+	"ulpi_dir_py1",
+	"ulpi_nxt_py2",
+	"ulpi_stp_py3",
 };
 
 static const char * const vgp1_groups[] = {
-	"CAM_I2C_SCL_PBB1",
+	"cam_i2c_scl_pbb1",
 };
 
 static const char * const vgp2_groups[] = {
-	"CAM_I2C_SDA_PBB2",
+	"cam_i2c_sda_pbb2",
 };
 
 static const char * const vgp3_groups[] = {
-	"PBB3",
-	"SDMMC4_DAT5_PAA5",
+	"pbb3",
+	"sdmmc4_dat5_paa5",
 };
 
 static const char * const vgp4_groups[] = {
-	"PBB4",
-	"SDMMC4_DAT6_PAA6",
+	"pbb4",
+	"sdmmc4_dat6_paa6",
 };
 
 static const char * const vgp5_groups[] = {
-	"PBB5",
-	"SDMMC4_DAT7_PAA7",
+	"pbb5",
+	"sdmmc4_dat7_paa7",
 };
 
 static const char * const vgp6_groups[] = {
-	"PBB6",
-	"SDMMC4_RST_N_PCC3",
+	"pbb6",
+	"sdmmc4_rst_n_pcc3",
 };
 
 static const char * const vi_groups[] = {
-	"CAM_MCLK_PCC0",
-	"VI_D0_PT4",
-	"VI_D1_PD5",
-	"VI_D10_PT2",
-	"VI_D11_PT3",
-	"VI_D2_PL0",
-	"VI_D3_PL1",
-	"VI_D4_PL2",
-	"VI_D5_PL3",
-	"VI_D6_PL4",
-	"VI_D7_PL5",
-	"VI_D8_PL6",
-	"VI_D9_PL7",
-	"VI_HSYNC_PD7",
-	"VI_MCLK_PT1",
-	"VI_PCLK_PT0",
-	"VI_VSYNC_PD6",
+	"cam_mclk_pcc0",
+	"vi_d0_pt4",
+	"vi_d1_pd5",
+	"vi_d10_pt2",
+	"vi_d11_pt3",
+	"vi_d2_pl0",
+	"vi_d3_pl1",
+	"vi_d4_pl2",
+	"vi_d5_pl3",
+	"vi_d6_pl4",
+	"vi_d7_pl5",
+	"vi_d8_pl6",
+	"vi_d9_pl7",
+	"vi_hsync_pd7",
+	"vi_mclk_pt1",
+	"vi_pclk_pt0",
+	"vi_vsync_pd6",
 };
 
 static const char * const vi_alt1_groups[] = {
-	"CAM_MCLK_PCC0",
-	"VI_MCLK_PT1",
+	"cam_mclk_pcc0",
+	"vi_mclk_pt1",
 };
 
 static const char * const vi_alt2_groups[] = {
-	"VI_MCLK_PT1",
+	"vi_mclk_pt1",
 };
 
 static const char * const vi_alt3_groups[] = {
-	"CAM_MCLK_PCC0",
-	"VI_MCLK_PT1",
+	"cam_mclk_pcc0",
+	"vi_mclk_pt1",
 };
 
 #define FUNCTION(fname)					\
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 4/6] pinctrl: API changes to support multiple states per device
  2012-02-28 20:27 ` [PATCH V2 4/6] pinctrl: API changes to support multiple states per device Stephen Warren
@ 2012-02-29  6:46   ` Dong Aisheng
  2012-02-29 17:22     ` Stephen Warren
  0 siblings, 1 reply; 18+ messages in thread
From: Dong Aisheng @ 2012-02-29  6:46 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Linus Walleij, Linus Walleij, B29396, s.hauer, dongas86,
	shawn.guo, thomas.abraham, tony, linux-kernel

On Tue, Feb 28, 2012 at 01:27:30PM -0700, Stephen Warren wrote:
> The API model is changed from:
> 
> p = pinctrl_get(dev, "state1");
> pinctrl_enable(p);
> ...
> pinctrl_disable(p);
> pinctrl_put(p);
> p = pinctrl_get(dev, "state2");
> pinctrl_enable(p);
> ...
> pinctrl_disable(p);
> pinctrl_put(p);
> 
> to this:
> 
> p = pinctrl_get(dev);
> s1 = pinctrl_lookup_state(p, "state1");
> s2 = pinctrl_lookup_state(p, "state2");
> pinctrl_select_state(p, s1);
> ...
> pinctrl_select_state(p, s2);
> ...
> pinctrl_put(p);
> 
> This allows devices to directly transition between states without
> disabling the pin controller programming and put()/get()ing the
> configuration data each time. This model will also better suit pinconf
> programming, which doesn't have a concept of "disable".
> 
> The special-case hogging feature of pin controllers is re-written to use
> the regular APIs instead of special-case code. Hence, the pinmux-hogs
> debugfs file is removed; see the top-level pinctrl-handles files for
> equivalent data.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v2: Make use of PINCTRL_STATE_DEFAULT, split out some documentation
> cleanup into an earlier patch. Various minor fixes. Fixes due to
> rebasing on updated earlier patches. Remove usecount field from struct
> pinctrl; allow only one concurrent pinctrl_get() per device.
> 
Glad to hear this.

The whole patch looks pretty good to me.
A few trivial comments, else
Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

> +static struct pinctrl *find_pinctrl(struct device *dev)
> +{
> +	struct pinctrl *p;
> +
> +	list_for_each_entry(p, &pinctrldev_list, node)
s/pinctrldev_list/pinctrl_list ?

> -static void pinctrl_disable_locked(struct pinctrl *p)
> +static int pinctrl_select_state_locked(struct pinctrl *p,
> +				       struct pinctrl_state *state)
>  {
> -	struct pinctrl_setting *setting;
> +	struct pinctrl_setting *setting, *setting2;
> +	int ret;
>  
> -	if (p == NULL)
> -		return;
> +	if (p->state == state)
> +		return 0;
>  
> -	if (--p->usecount == 0) {
> -		list_for_each_entry(setting, &p->settings, node)
> -			pinmux_disable_setting(setting);
> +	if (p->state) {
> +		/*
> +		 * The set of groups with a mux configuration in the old state
> +		 * may not be identical to the set of groups with a mux setting
> +		 * in the new state. While this might be unusual, it's entirely
> +		 * possible for the "user"-supplied mapping table to be written
> +		 * that way. For each group that was configured in the old state
> +		 * but not in the new state, this code puts that group into a
> +		 * safe/disabled state.
It means the old state function of some groups may not have been disabled before
enabling the new function mode.
May it be a little error-prone?
Maybe we can not depend on user to disable a function before enable another for
a group when doing pinmux_enable_setting considering they may be different
registers.

> +		 */
> +		list_for_each_entry(setting, &p->state->settings, node) {
> +			bool found = false;
> +			list_for_each_entry(setting2, &state->settings, node) {
> +				if (setting2->group_selector ==
> +						setting->group_selector) {
> +					found = true;
> +					break;
> +				}
> +			}
> +			if (!found)
> +				pinmux_disable_setting(setting);
> +		}
> +	}
> +
> +	p->state = state;
> +
> +	/* Apply all the settings for the new state */
> +	list_for_each_entry(setting, &state->settings, node) {
> +		ret = pinmux_enable_setting(setting);
> +		if (ret < 0) {
> +			/* FIXME: Difficult to return to prev state */
> +			return ret;
> +		}
>  	}
> +
> +	return 0;
>  }
>  
>  /**

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, ...
  2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
                   ` (5 preceding siblings ...)
  2012-02-28 20:27 ` [PATCH V2 6/6] pinctrl: fix case of Tegra30's foo_groups[] arrays Stephen Warren
@ 2012-02-29 16:46 ` Linus Walleij
  2012-02-29 17:42   ` Stephen Warren
  6 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2012-02-29 16:46 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel

On Tue, Feb 28, 2012 at 9:27 PM, Stephen Warren <swarren@nvidia.com> wrote:

> This is the current set of patches I have outstanding for pinctrl.

Thanks, now the problem is that I think this is based on top of
the other two patches, which happen to break U300 so I cannot
test it :-(

It'd be great if you could resolve the hogging issue in the other
patch set and put them as patch 1 & 2 of this patch set
so I have the whole picture and can proceed from there.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 4/6] pinctrl: API changes to support multiple states per device
  2012-02-29  6:46   ` Dong Aisheng
@ 2012-02-29 17:22     ` Stephen Warren
  2012-03-01  3:09       ` Dong Aisheng
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Warren @ 2012-02-29 17:22 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Linus Walleij, Linus Walleij, B29396, s.hauer, dongas86,
	shawn.guo, thomas.abraham, tony, linux-kernel

Dong Aisheng wrote at Tuesday, February 28, 2012 11:47 PM:
> On Tue, Feb 28, 2012 at 01:27:30PM -0700, Stephen Warren wrote:
...
> > +static struct pinctrl *find_pinctrl(struct device *dev)
> > +{
> > +	struct pinctrl *p;
> > +
> > +	list_for_each_entry(p, &pinctrldev_list, node)
>
> s/pinctrldev_list/pinctrl_list ?

Uggh. I could have sworn I fixed that before. Perhaps I had the same
silly error somewhere else too. Thanks for catching it.
> 
> > +static int pinctrl_select_state_locked(struct pinctrl *p,
> > +				       struct pinctrl_state *state)
...
> > +	if (p->state) {
> > +		/*
> > +		 * The set of groups with a mux configuration in the old state
> > +		 * may not be identical to the set of groups with a mux setting
> > +		 * in the new state. While this might be unusual, it's entirely
> > +		 * possible for the "user"-supplied mapping table to be written
> > +		 * that way. For each group that was configured in the old state
> > +		 * but not in the new state, this code puts that group into a
> > +		 * safe/disabled state.
>
> It means the old state function of some groups may not have been disabled before
> enabling the new function mode.
> May it be a little error-prone?
> Maybe we can not depend on user to disable a function before enable another for
> a group when doing pinmux_enable_setting considering they may be different
> registers.

The idea here is that there are two cases:

1) (hopefully the typical case)

Group X HAS a mux function set in OLD state
Group X HAS a mux function set in NEW state

In this case, we simply call pinmux_enable_setting(new state).

We do this so that we can transition directly from the old state to the
new state without going through any artificial intermediate states, to
make the mux switching as glitchless as possible.

If the HW has some restriction where it has to operate like:

Old state -> idle -> new state

Rather than:

Old state -> new state

Then I think that should be taken care of within the individual driver;
given most mux selection is just a register field that feeds into a simple
signal mux in HW, such a requirement seems pretty unlikely to me, but you
never know.

2)

Group X HAS a mux function set in OLD state
Group X DOES NOT have a mux function set in NEW state

In this case, we call pinmux_disable_setting(old state) to ensure that
the pin is put into some idle state that can't interfere with any other
pin.

-- 
nvpublic


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, ...
  2012-02-29 16:46 ` [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Linus Walleij
@ 2012-02-29 17:42   ` Stephen Warren
  2012-02-29 18:09     ` Linus Walleij
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Warren @ 2012-02-29 17:42 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel

Linus Walleij wrote at Wednesday, February 29, 2012 9:46 AM:
> On Tue, Feb 28, 2012 at 9:27 PM, Stephen Warren <swarren@nvidia.com> wrote:
> 
> > This is the current set of patches I have outstanding for pinctrl.
> 
> Thanks, now the problem is that I think this is based on top of
> the other two patches, which happen to break U300 so I cannot
> test it :-(

I think this series will unbreak U300, i.e. if you apply everything
it'll all work fine? Of course, git bisect isn't maintained, so I still
need to rework the series as I mentioned in my previous email.

> It'd be great if you could resolve the hogging issue in the other
> patch set and put them as patch 1 & 2 of this patch set
> so I have the whole picture and can proceed from there.

OK.

-- 
nvpublic


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, ...
  2012-02-29 17:42   ` Stephen Warren
@ 2012-02-29 18:09     ` Linus Walleij
  0 siblings, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2012-02-29 18:09 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Linus Walleij, B29396, s.hauer, dongas86, shawn.guo,
	thomas.abraham, tony, linux-kernel

On Wed, Feb 29, 2012 at 6:42 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Linus Walleij wrote at Wednesday, February 29, 2012 9:46 AM:
>> On Tue, Feb 28, 2012 at 9:27 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>
>> > This is the current set of patches I have outstanding for pinctrl.
>>
>> Thanks, now the problem is that I think this is based on top of
>> the other two patches, which happen to break U300 so I cannot
>> test it :-(
>
> I think this series will unbreak U300, i.e. if you apply everything
> it'll all work fine?

Hm I wanted to try this but it seems patch 4/6 isn't on the list?

Strange ... oh well I guess I'll test the next respin instead.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 4/6] pinctrl: API changes to support multiple states per device
  2012-02-29 17:22     ` Stephen Warren
@ 2012-03-01  3:09       ` Dong Aisheng
  0 siblings, 0 replies; 18+ messages in thread
From: Dong Aisheng @ 2012-03-01  3:09 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Linus Walleij, Linus Walleij, B29396, s.hauer, dongas86,
	shawn.guo, thomas.abraham, tony, linux-kernel

On Wed, Feb 29, 2012 at 09:22:42AM -0800, Stephen Warren wrote:
> Dong Aisheng wrote at Tuesday, February 28, 2012 11:47 PM:
.....
> > > +	if (p->state) {
> > > +		/*
> > > +		 * The set of groups with a mux configuration in the old state
> > > +		 * may not be identical to the set of groups with a mux setting
> > > +		 * in the new state. While this might be unusual, it's entirely
> > > +		 * possible for the "user"-supplied mapping table to be written
> > > +		 * that way. For each group that was configured in the old state
> > > +		 * but not in the new state, this code puts that group into a
> > > +		 * safe/disabled state.
> >
> > It means the old state function of some groups may not have been disabled before
> > enabling the new function mode.
> > May it be a little error-prone?
> > Maybe we can not depend on user to disable a function before enable another for
> > a group when doing pinmux_enable_setting considering they may be different
> > registers.
> 
> The idea here is that there are two cases:
> 
> 1) (hopefully the typical case)
> 
> Group X HAS a mux function set in OLD state
> Group X HAS a mux function set in NEW state
> 
> In this case, we simply call pinmux_enable_setting(new state).
> 
> We do this so that we can transition directly from the old state to the
> new state without going through any artificial intermediate states, to
> make the mux switching as glitchless as possible.
> 
> If the HW has some restriction where it has to operate like:
> 
> Old state -> idle -> new state
> 
> Rather than:
> 
> Old state -> new state
> 
> Then I think that should be taken care of within the individual driver;
> given most mux selection is just a register field that feeds into a simple
> signal mux in HW, such a requirement seems pretty unlikely to me, but you
> never know.
> 
I thought before that the mux function for GROUP X in OLD state may be set
in register A while mux function in NEW state may be set in register B.
So before setting new state function, user should disable old state function
(clear register A)first. (Maybe i thought too far)

But now i realize that you're correct, this can be handled in individual driver
for different HW requirement.

> 2)
> 
> Group X HAS a mux function set in OLD state
> Group X DOES NOT have a mux function set in NEW state
> 
> In this case, we call pinmux_disable_setting(old state) to ensure that
> the pin is put into some idle state that can't interfere with any other
> pin.
> 
Thanks for the detailed clarification.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations
  2012-02-28 20:27 ` [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations Stephen Warren
@ 2012-03-01  8:46   ` Dong Aisheng
  2012-03-01 16:52     ` Stephen Warren
  0 siblings, 1 reply; 18+ messages in thread
From: Dong Aisheng @ 2012-03-01  8:46 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Linus Walleij, Linus Walleij, B29396, s.hauer, dongas86,
	shawn.guo, thomas.abraham, tony, linux-kernel

On Tue, Feb 28, 2012 at 01:27:31PM -0700, Stephen Warren wrote:
> The pinctrl mapping table can now contain entries to:
> * Set the mux function of a pin group
> * Apply a set of pin config options to a pin or a group
> 
> This allows pinctrl_select_state() to apply pin configs settings as well
> as mux settings.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
....
> +Finally, some devices expect the mapping table to contain certain specific
> +named states. When running on hardware that doesn't need any pin controller
> +configuration, the mapping table must still contain those named states, in
> +order to explicitly indicate that the states were provided and intended to
> +be empty. Table entry macro PIN_MAP_DUMMY_STATE serves the purpose of defining
> +a named state without causing any pin controller to be programmed:
> +
> +static struct pinctrl_map __initdata mapping[] = {
> +	PIN_MAP_DUMMY_STATE("foo-i2c.0", PINCTRL_STATE_DEFAULT),
>  };
Is this used for shared devices between different platforms which may need pinmux
setting while another not?

>  
> -PIN_MAP_SYS_HOG("pinctrl-foo", "power_func")
> +PIN_MAP_MUX_GROUP_HOG("pinctrl-foo", NULL /* group */, "power_func")
Missed state name or should be PIN_MAP_MUX_GROUP_HOG_DEFAULT?

>  
>  This gives the exact same result as the above construction.
>  
> diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
> index 18973ca..c965bb5 100644
> --- a/arch/arm/mach-u300/core.c
> +++ b/arch/arm/mach-u300/core.c
> @@ -1569,13 +1569,13 @@ static struct platform_device dma_device = {
>  /* Pinmux settings */
>  static struct pinctrl_map __initdata u300_pinmux_map[] = {
>  	/* anonymous maps for chip power and EMIFs */
> -	PIN_MAP_SYS_HOG("pinctrl-u300", "power"),
> -	PIN_MAP_SYS_HOG("pinctrl-u300", "emif0"),
> -	PIN_MAP_SYS_HOG("pinctrl-u300", "emif1"),
> +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "power"),
> +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif0"),
> +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif1"),
>  	/* per-device maps for MMC/SD, SPI and UART */
> -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "mmc0", "mmci"),
> -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "spi0", "pl022"),
> -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "uart0", "uart0"),
> +	PIN_MAP_MUX_GROUP_DEFAULT("mmci",  "pinctrl-u300", NULL, "mmc0"),
> +	PIN_MAP_MUX_GROUP_DEFAULT("pl022", "pinctrl-u300", NULL, "spi0"),
> +	PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0"),
Although not big issue, but i'm a bit more intended to the original way that
for the NULL group name using:
PIN_MAP_MUX_DEFAULT("mmci",  "pinctrl-u300", "mmc0"),
Or using PIN_MAP_MUX_GROUP_DEFAULT, but just totally remove the optional NULL
group name support to keep consistence and avoid confusing.

> diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
> index 05d25c8..b572153 100644
> --- a/include/linux/pinctrl/machine.h
> +++ b/include/linux/pinctrl/machine.h
> @@ -14,6 +14,41 @@
>  
>  #include "pinctrl.h"
>  
> +enum pinctrl_map_type {
> +	PIN_MAP_TYPE_INVALID,
> +	PIN_MAP_TYPE_DUMMY_STATE,
> +	PIN_MAP_TYPE_MUX_GROUP,
> +	PIN_MAP_TYPE_CONFIGS_PIN,
> +	PIN_MAP_TYPE_CONFIGS_GROUP,

Basically it causes a bit confusing to me that we have per pin config
but we do not have per pin mux.
However, i still have not got a better idea for this issue.

> +#define PIN_MAP_MUX_CONFIGS_GROUP(dev, state, pinctrl, grp, cfgs)	\
Actually this macro is only for pin config setting,
maybe PIN_MAP_CONFIGS_GROUP is better, right?

> +	{								\
> +		.dev_name = dev,					\
> +		.name = state,						\
> +		.type = PIN_MAP_TYPE_CONFIGS_GROUP,			\
> +		.ctrl_dev_name = pinctrl,				\
We have three entries above duplicated with MUX macro.

> +		.data.configs = {					\
> +			.group_or_pin = grp,				\
> +			.configs = cfgs,				\
> +			.num_configs = ARRAY_SIZE(cfgs),		\
> +		},							\
> +	}
It seems you separate the pin mux setting and pin config setting in
different maps.
Can we merge them into one map?
That will save a lot of map entries and improve searching performance.

Another question is that:
the current IMX pinmux setting for non-dt is like:
#define MX51_I2C_PAD_CTRL       (PAD_CTL_SRE_FAST | PAD_CTL_ODE | \
                                PAD_CTL_DSE_HIGH | PAD_CTL_PUS_100K_UP | \
#define MX51_PAD_KEY_COL4__I2C2_SCL	\
	IOMUX_PAD(0x65c, 0x26c, 0x13, 0x9b8, 1, MX51_I2C_PAD_CTRL)
This macro includes both mux and config setting.
It's very easy and conveniently to use.
So i'd really like to see a similar using after migrate to pinctrl subsystem
that using a macro to set both.

One possible working method i thought is extended macro based on your code:
#define PIN_MAP_MUX_CONFIG_GROUP(dev, state, pinctrl, grp, func, cfgs)	\
	PIN_MAP_MUX_GROUP(dev, state, pinctrl, grp, func),		\
	PIN_MAP_CONFIG_GROUP(dev, state, pinctrl, grp, cfgs)

It works for group.
However, we can not also do this for per PIN config since PIN_MAP_MUX_*
is only for group.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations
  2012-03-01  8:46   ` Dong Aisheng
@ 2012-03-01 16:52     ` Stephen Warren
  2012-03-02  3:46       ` Dong Aisheng
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Warren @ 2012-03-01 16:52 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Linus Walleij, Linus Walleij, B29396, s.hauer, dongas86,
	shawn.guo, thomas.abraham, tony, linux-kernel

Dong Aisheng wrote at Thursday, March 01, 2012 1:46 AM:
> On Tue, Feb 28, 2012 at 01:27:31PM -0700, Stephen Warren wrote:
> > The pinctrl mapping table can now contain entries to:
> > * Set the mux function of a pin group
> > * Apply a set of pin config options to a pin or a group
> >
> > This allows pinctrl_select_state() to apply pin configs settings as well
> > as mux settings.
> >
> > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > ---
> ....
> > +Finally, some devices expect the mapping table to contain certain specific
> > +named states. When running on hardware that doesn't need any pin controller
> > +configuration, the mapping table must still contain those named states, in
> > +order to explicitly indicate that the states were provided and intended to
> > +be empty. Table entry macro PIN_MAP_DUMMY_STATE serves the purpose of defining
> > +a named state without causing any pin controller to be programmed:
> > +
> > +static struct pinctrl_map __initdata mapping[] = {
> > +	PIN_MAP_DUMMY_STATE("foo-i2c.0", PINCTRL_STATE_DEFAULT),
> >  };
>
> Is this used for shared devices between different platforms which may need pinmux
> setting while another not?

Exactly yes.

> > -PIN_MAP_SYS_HOG("pinctrl-foo", "power_func")
> > +PIN_MAP_MUX_GROUP_HOG("pinctrl-foo", NULL /* group */, "power_func")
>
> Missed state name or should be PIN_MAP_MUX_GROUP_HOG_DEFAULT?

Thanks, fixed.

> >  This gives the exact same result as the above construction.
> >
> > diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
> > index 18973ca..c965bb5 100644
> > --- a/arch/arm/mach-u300/core.c
> > +++ b/arch/arm/mach-u300/core.c
> > @@ -1569,13 +1569,13 @@ static struct platform_device dma_device = {
> >  /* Pinmux settings */
> >  static struct pinctrl_map __initdata u300_pinmux_map[] = {
> >  	/* anonymous maps for chip power and EMIFs */
> > -	PIN_MAP_SYS_HOG("pinctrl-u300", "power"),
> > -	PIN_MAP_SYS_HOG("pinctrl-u300", "emif0"),
> > -	PIN_MAP_SYS_HOG("pinctrl-u300", "emif1"),
> > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "power"),
> > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif0"),
> > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif1"),
> >  	/* per-device maps for MMC/SD, SPI and UART */
> > -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "mmc0", "mmci"),
> > -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "spi0", "pl022"),
> > -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "uart0", "uart0"),
> > +	PIN_MAP_MUX_GROUP_DEFAULT("mmci",  "pinctrl-u300", NULL, "mmc0"),
> > +	PIN_MAP_MUX_GROUP_DEFAULT("pl022", "pinctrl-u300", NULL, "spi0"),
> > +	PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0"),
>
> Although not big issue, but i'm a bit more intended to the original way that
> for the NULL group name using:
> PIN_MAP_MUX_DEFAULT("mmci",  "pinctrl-u300", "mmc0"),
> Or using PIN_MAP_MUX_GROUP_DEFAULT, but just totally remove the optional NULL
> group name support to keep consistence and avoid confusing.

The group parameter can't be removed, since in general, the mapping table
needs some way to indicate which group to activate the function on. For
example, consider an I2C controller that can be muxed onto two different
sets of pins based on the board layout - you need to specify which to use.

I suppose there could be yet more and more macros for muxing; one set
that hard-codes group to NULL internally and hence doesn't take a parameter,
and another that takes the group from a parameter. The set of macros and
their names would start getting extremely unwieldy then though.

> > diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
> > index 05d25c8..b572153 100644
> > --- a/include/linux/pinctrl/machine.h
> > +++ b/include/linux/pinctrl/machine.h
> > @@ -14,6 +14,41 @@
> >
> >  #include "pinctrl.h"
> >
> > +enum pinctrl_map_type {
> > +	PIN_MAP_TYPE_INVALID,
> > +	PIN_MAP_TYPE_DUMMY_STATE,
> > +	PIN_MAP_TYPE_MUX_GROUP,
> > +	PIN_MAP_TYPE_CONFIGS_PIN,
> > +	PIN_MAP_TYPE_CONFIGS_GROUP,
> 
> Basically it causes a bit confusing to me that we have per pin config
> but we do not have per pin mux.
> However, i still have not got a better idea for this issue.

Well, that's a limitation of how the pinctrl core works right now. It'd
be entirely possible to fix this, I think, and we can easily expand
that enum if that happens.

> > +#define PIN_MAP_MUX_CONFIGS_GROUP(dev, state, pinctrl, grp, cfgs)	\
>
> Actually this macro is only for pin config setting,
> maybe PIN_MAP_CONFIGS_GROUP is better, right?

Oh yes, the macro names should either have "MUX" /or/ "CONFIG", not both.
I'll fix that.

> > +	{								\
> > +		.dev_name = dev,					\
> > +		.name = state,						\
> > +		.type = PIN_MAP_TYPE_CONFIGS_GROUP,			\
> > +		.ctrl_dev_name = pinctrl,				\
>
> We have three entries above duplicated with MUX macro.

Well yes that's true, but is it really worth fixing? If we fix this,
Then note that PIN_MUX_DUMMY_STATE also shares the first 3 of those 4
lines, so we'll end up with a crazy maze of macros by removing all
possible duplication

> > +		.data.configs = {					\
> > +			.group_or_pin = grp,				\
> > +			.configs = cfgs,				\
> > +			.num_configs = ARRAY_SIZE(cfgs),		\
> > +		},							\
> > +	}
>
> It seems you separate the pin mux setting and pin config setting in
> different maps.
> Can we merge them into one map?
> That will save a lot of map entries and improve searching performance.

Well, it's technically possible.

But then, you'll need macros that just define a mux setting, just configs
for groups, or both mux setting and config settings. Then, cross-product
that with whether the state name is default or not, and cross-product that
with whether the macro should hard-code group to NULL or not etc. etc...
It seems like somewhat of a nightmare. Parsing this table is in the slow-
path of pinctrl_get() anyway, so I don't think it's an enormous deal.

One alternative might be to represent mux settings as just another config
on the group though. But that'd require the pin config namespace to have
some standardized values and some driver-specific values.

> Another question is that:
> the current IMX pinmux setting for non-dt is like:
> #define MX51_I2C_PAD_CTRL       (PAD_CTL_SRE_FAST | PAD_CTL_ODE | \
>                                 PAD_CTL_DSE_HIGH | PAD_CTL_PUS_100K_UP | \
> #define MX51_PAD_KEY_COL4__I2C2_SCL	\
> 	IOMUX_PAD(0x65c, 0x26c, 0x13, 0x9b8, 1, MX51_I2C_PAD_CTRL)
>
> This macro includes both mux and config setting.
> It's very easy and conveniently to use.
> So i'd really like to see a similar using after migrate to pinctrl subsystem
> that using a macro to set both.
>
> One possible working method i thought is extended macro based on your code:
>
> #define PIN_MAP_MUX_CONFIG_GROUP(dev, state, pinctrl, grp, func, cfgs)	\
> 	PIN_MAP_MUX_GROUP(dev, state, pinctrl, grp, func),		\
> 	PIN_MAP_CONFIG_GROUP(dev, state, pinctrl, grp, cfgs)

That's exactly what I've done for the Tegra:

#define TEGRA_MAP_MUX(_group_, _function_) \
	PIN_MAP_MUX_GROUP_HOG_DEFAULT(PINMUX_DEV, _group_, _function_)

#define TEGRA_MAP_CONF(_group_, _pull_, _drive_) \
	PIN_MAP_CONFIGS_GROUP_HOG_DEFAULT(PINMUX_DEV, _group_, tegra_pincfg_pull##_pull_##_##_drive_)

#define TEGRA_MAP_MUXCONF(_group_, _function_, _pull_, _drive_) \
	TEGRA_MAP_MUX(_group_, _function_), \
	TEGRA_MAP_CONF(_group_, _pull_, _drive_)

And the mapping table arrays in the board files use TEGRA_MAP_MUXCONF().

I'm not convinced it's a good idea to try and come up with common macros
for this in <pinctrl/machine.h> due to the massive number of macros it'd
take to represent all possible combinations. And even with such a common
combo macro, I'd still need to define a custom TEGRA_MAP_MUXCONF in order
to select which one of the common macros to use (in order to use a shorter
name in board files than the long common macro names), hard-code the pin
controller driver name, hard-code the configs array name, etc.

> It works for group.
> However, we can not also do this for per PIN config since PIN_MAP_MUX_*
> is only for group.

If you know that your chip muxes per pin, then presumably the pin and
group names are the same, so your combination macro can simply "call"
PIN_MAP_CONFIGS_PIN_*() instead of PIN_MAP_CONFIGS_GROUP_*().

Another solution may be to extend the pinctrl core to allow muxing per
pin where the HW does, so you could use pin-based macros for both mux
and config. But, lets tackle that after this current round of patches
is merged.

-- 
nvpublic


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations
  2012-03-01 16:52     ` Stephen Warren
@ 2012-03-02  3:46       ` Dong Aisheng
  2012-03-02 22:49         ` Stephen Warren
  0 siblings, 1 reply; 18+ messages in thread
From: Dong Aisheng @ 2012-03-02  3:46 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Linus Walleij, Linus Walleij, B29396, s.hauer, dongas86,
	shawn.guo, thomas.abraham, tony, linux-kernel

On Thu, Mar 01, 2012 at 08:52:12AM -0800, Stephen Warren wrote:
> Dong Aisheng wrote at Thursday, March 01, 2012 1:46 AM:
> > On Tue, Feb 28, 2012 at 01:27:31PM -0700, Stephen Warren wrote:
...
> > >  This gives the exact same result as the above construction.
> > >
> > > diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
> > > index 18973ca..c965bb5 100644
> > > --- a/arch/arm/mach-u300/core.c
> > > +++ b/arch/arm/mach-u300/core.c
> > > @@ -1569,13 +1569,13 @@ static struct platform_device dma_device = {
> > >  /* Pinmux settings */
> > >  static struct pinctrl_map __initdata u300_pinmux_map[] = {
> > >  	/* anonymous maps for chip power and EMIFs */
> > > -	PIN_MAP_SYS_HOG("pinctrl-u300", "power"),
> > > -	PIN_MAP_SYS_HOG("pinctrl-u300", "emif0"),
> > > -	PIN_MAP_SYS_HOG("pinctrl-u300", "emif1"),
> > > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "power"),
> > > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif0"),
> > > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif1"),
> > >  	/* per-device maps for MMC/SD, SPI and UART */
> > > -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "mmc0", "mmci"),
> > > -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "spi0", "pl022"),
> > > -	PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "uart0", "uart0"),
> > > +	PIN_MAP_MUX_GROUP_DEFAULT("mmci",  "pinctrl-u300", NULL, "mmc0"),
> > > +	PIN_MAP_MUX_GROUP_DEFAULT("pl022", "pinctrl-u300", NULL, "spi0"),
> > > +	PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0"),
> >
> > Although not big issue, but i'm a bit more intended to the original way that
> > for the NULL group name using:
> > PIN_MAP_MUX_DEFAULT("mmci",  "pinctrl-u300", "mmc0"),
> > Or using PIN_MAP_MUX_GROUP_DEFAULT, but just totally remove the optional NULL
> > group name support to keep consistence and avoid confusing.
> 
> The group parameter can't be removed, since in general, the mapping table
> needs some way to indicate which group to activate the function on. For
> example, consider an I2C controller that can be muxed onto two different
> sets of pins based on the board layout - you need to specify which to use.
> 

I did not mean remove the group parameter support, i meant remove the optinal NULL
group name support to avoid using the macro like:
PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0")
User should be better to specify a group name for this macro to avoid misleading.

However if want to support NULL group name, i'd rather like the macro:
PIN_MAP_MUX_DEFAULT("mmci",  "pinctrl-u300", "mmc0"),

> I suppose there could be yet more and more macros for muxing; one set
> that hard-codes group to NULL internally and hence doesn't take a parameter,
> and another that takes the group from a parameter. The set of macros and
> their names would start getting extremely unwieldy then though.
> 
Yes, that's what i mean.

> > > diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
> > > index 05d25c8..b572153 100644
> > > --- a/include/linux/pinctrl/machine.h
> > > +++ b/include/linux/pinctrl/machine.h
> > > @@ -14,6 +14,41 @@
> > >
> > >  #include "pinctrl.h"
> > >
> > > +enum pinctrl_map_type {
> > > +	PIN_MAP_TYPE_INVALID,
> > > +	PIN_MAP_TYPE_DUMMY_STATE,
> > > +	PIN_MAP_TYPE_MUX_GROUP,
> > > +	PIN_MAP_TYPE_CONFIGS_PIN,
> > > +	PIN_MAP_TYPE_CONFIGS_GROUP,
> > 
> > Basically it causes a bit confusing to me that we have per pin config
> > but we do not have per pin mux.
> > However, i still have not got a better idea for this issue.
> 
> Well, that's a limitation of how the pinctrl core works right now. It'd
Correct.

> be entirely possible to fix this, I think, and we can easily expand
> that enum if that happens.
> 
Sounds good.
I was not asking you to fix it in this patch.
we can go for it letter since it's another issue and may need some changes
due to core limitation.

> > > +#define PIN_MAP_MUX_CONFIGS_GROUP(dev, state, pinctrl, grp, cfgs)	\
> >
> > Actually this macro is only for pin config setting,
> > maybe PIN_MAP_CONFIGS_GROUP is better, right?
> 
> Oh yes, the macro names should either have "MUX" /or/ "CONFIG", not both.
> I'll fix that.
> 
Sounds good.

> > > +	{								\
> > > +		.dev_name = dev,					\
> > > +		.name = state,						\
> > > +		.type = PIN_MAP_TYPE_CONFIGS_GROUP,			\
> > > +		.ctrl_dev_name = pinctrl,				\
> >
> > We have three entries above duplicated with MUX macro.
> 
> Well yes that's true, but is it really worth fixing? If we fix this,
> Then note that PIN_MUX_DUMMY_STATE also shares the first 3 of those 4
> lines, so we'll end up with a crazy maze of macros by removing all
> possible duplication
> 
> > > +		.data.configs = {					\
> > > +			.group_or_pin = grp,				\
> > > +			.configs = cfgs,				\
> > > +			.num_configs = ARRAY_SIZE(cfgs),		\
> > > +		},							\
> > > +	}
> >
> > It seems you separate the pin mux setting and pin config setting in
> > different maps.
> > Can we merge them into one map?
> > That will save a lot of map entries and improve searching performance.
> 
> Well, it's technically possible.
> 
> But then, you'll need macros that just define a mux setting, just configs
> for groups, or both mux setting and config settings. Then, cross-product
> that with whether the state name is default or not, and cross-product that
> with whether the macro should hard-code group to NULL or not etc. etc...
Well, they're indeed a lot.
But it seems they're not related to whether merging config and mux into one
map, right?
Because we already have and need these macros but just separated into two
parts for mux and config and that does not save too much.
Considering user may add a lot of per pin config entries in map(like tegra),
that will really increase the map table a lot and surely will affect the map
search performance a lot for mux setting.

And if we want to save macros, i'd rather prefer to eliminate the NULL group
name support since it really does not help too much and we already eliminate
the optional NULL state name support, so why not NULL group name?

> It seems like somewhat of a nightmare. Parsing this table is in the slow-
> path of pinctrl_get() anyway, so I don't think it's an enormous deal.
> 
> One alternative might be to represent mux settings as just another config
> on the group though. But that'd require the pin config namespace to have
> some standardized values and some driver-specific values.
> 
Do you mean treat them all(mux and config) as a config setting?
Then what benefits can we get?
Does it have any difference from separate them but still in one map?

> > Another question is that:
> > the current IMX pinmux setting for non-dt is like:
> > #define MX51_I2C_PAD_CTRL       (PAD_CTL_SRE_FAST | PAD_CTL_ODE | \
> >                                 PAD_CTL_DSE_HIGH | PAD_CTL_PUS_100K_UP | \
> > #define MX51_PAD_KEY_COL4__I2C2_SCL	\
> > 	IOMUX_PAD(0x65c, 0x26c, 0x13, 0x9b8, 1, MX51_I2C_PAD_CTRL)
> >
> > This macro includes both mux and config setting.
> > It's very easy and conveniently to use.
> > So i'd really like to see a similar using after migrate to pinctrl subsystem
> > that using a macro to set both.
> >
> > One possible working method i thought is extended macro based on your code:
> >
> > #define PIN_MAP_MUX_CONFIG_GROUP(dev, state, pinctrl, grp, func, cfgs)	\
> > 	PIN_MAP_MUX_GROUP(dev, state, pinctrl, grp, func),		\
> > 	PIN_MAP_CONFIG_GROUP(dev, state, pinctrl, grp, cfgs)
> 
> That's exactly what I've done for the Tegra:
> 
> #define TEGRA_MAP_MUX(_group_, _function_) \
> 	PIN_MAP_MUX_GROUP_HOG_DEFAULT(PINMUX_DEV, _group_, _function_)
> 
> #define TEGRA_MAP_CONF(_group_, _pull_, _drive_) \
> 	PIN_MAP_CONFIGS_GROUP_HOG_DEFAULT(PINMUX_DEV, _group_, tegra_pincfg_pull##_pull_##_##_drive_)
> 
> #define TEGRA_MAP_MUXCONF(_group_, _function_, _pull_, _drive_) \
> 	TEGRA_MAP_MUX(_group_, _function_), \
> 	TEGRA_MAP_CONF(_group_, _pull_, _drive_)
> 
> And the mapping table arrays in the board files use TEGRA_MAP_MUXCONF().
> 
> I'm not convinced it's a good idea to try and come up with common macros
> for this in <pinctrl/machine.h> due to the massive number of macros it'd
> take to represent all possible combinations. And even with such a common
> combo macro, I'd still need to define a custom TEGRA_MAP_MUXCONF in order
> to select which one of the common macros to use (in order to use a shorter
> name in board files than the long common macro names), hard-code the pin
> controller driver name, hard-code the configs array name, etc.
> 
Well, yes that it will increase more macros.
I can accept to let individual driver to define it according to their needs
first.

> > It works for group.
> > However, we can not also do this for per PIN config since PIN_MAP_MUX_*
> > is only for group.
> 
> If you know that your chip muxes per pin, then presumably the pin and
> group names are the same, so your combination macro can simply "call"
> PIN_MAP_CONFIGS_PIN_*() instead of PIN_MAP_CONFIGS_GROUP_*().
Well, the problem is that for virtual groups, the pin and group
name are not the same.
One solution i can see is that user may need to encode all pin configs
of a group in one config array and still use PIN_MAP_CONFIGS_GROUP_*(),
then each pinctrl driver has to decode that config array and do correct
setting.

> 
> Another solution may be to extend the pinctrl core to allow muxing per
> pin where the HW does, so you could use pin-based macros for both mux
> and config. But, lets tackle that after this current round of patches
> is merged.
> 
Yes, agree.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations
  2012-03-02  3:46       ` Dong Aisheng
@ 2012-03-02 22:49         ` Stephen Warren
  2012-03-05  7:58           ` Dong Aisheng
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Warren @ 2012-03-02 22:49 UTC (permalink / raw)
  To: Dong Aisheng, Linus Walleij, Linus Walleij
  Cc: B29396, s.hauer, dongas86, shawn.guo, thomas.abraham, tony, linux-kernel

Dong Aisheng wrote at Thursday, March 01, 2012 8:47 PM:
> On Thu, Mar 01, 2012 at 08:52:12AM -0800, Stephen Warren wrote:
> > Dong Aisheng wrote at Thursday, March 01, 2012 1:46 AM:
> > > On Tue, Feb 28, 2012 at 01:27:31PM -0700, Stephen Warren wrote:
...
> > > > diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
...
> > > > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "power"),
> > > > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif0"),
> > > > +	PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif1"),
...
> > > > +	PIN_MAP_MUX_GROUP_DEFAULT("mmci",  "pinctrl-u300", NULL, "mmc0"),
> > > > +	PIN_MAP_MUX_GROUP_DEFAULT("pl022", "pinctrl-u300", NULL, "spi0"),
> > > > +	PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0"),
> > >
> > > Although not big issue, but i'm a bit more intended to the original way that
> > > for the NULL group name using:
> > > PIN_MAP_MUX_DEFAULT("mmci",  "pinctrl-u300", "mmc0"),
> > > Or using PIN_MAP_MUX_GROUP_DEFAULT, but just totally remove the optional NULL
> > > group name support to keep consistence and avoid confusing.
> >
> > The group parameter can't be removed, since in general, the mapping table
> > needs some way to indicate which group to activate the function on. For
> > example, consider an I2C controller that can be muxed onto two different
> > sets of pins based on the board layout - you need to specify which to use.
> >
> 
> I did not mean remove the group parameter support, i meant remove the optinal NULL
> group name support to avoid using the macro like:
> PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0")
> User should be better to specify a group name for this macro to avoid misleading.

Ah right.

Yes, I'd be perfectly happy removing support for not specifying group
as NULL, and requiring a specific group name to be provided in all cases.
I like being specific:-)

Linus,

What are your thoughts here? IIRC, you were the one who wanted to be
able to specify group==NULL, and have the pinctrl core use the "first"
supported group for the function.

Either way, I think that if we did require non-NULL group, this should
be a separate patch we can introduce after merging my series, to avoid
feature creep.

...
> > > > +		.data.configs = {					\
> > > > +			.group_or_pin = grp,				\
> > > > +			.configs = cfgs,				\
> > > > +			.num_configs = ARRAY_SIZE(cfgs),		\
> > > > +		},							\
> > > > +	}
> > >
> > > It seems you separate the pin mux setting and pin config setting in
> > > different maps.
> > > Can we merge them into one map?
> > > That will save a lot of map entries and improve searching performance.
> >
> > Well, it's technically possible.
> >
> > But then, you'll need macros that just define a mux setting, just configs
> > for groups, or both mux setting and config settings. Then, cross-product
> > that with whether the state name is default or not, and cross-product that
> > with whether the macro should hard-code group to NULL or not etc. etc...
>
> Well, they're indeed a lot.
> But it seems they're not related to whether merging config and mux into one
> map, right?

There's certainly some relationship; see my explanation above re: why
you'd end up with a lot more macros. Of course, if we didn't create all
the special-case macros, this problem wouldn't exist, so I don't think
there would be a problem...

> > It seems like somewhat of a nightmare. Parsing this table is in the slow-
> > path of pinctrl_get() anyway, so I don't think it's an enormous deal.
> >
> > One alternative might be to represent mux settings as just another config
> > on the group though. But that'd require the pin config namespace to have
> > some standardized values and some driver-specific values.
>
> Do you mean treat them all(mux and config) as a config setting?
> Then what benefits can we get?
> Does it have any difference from separate them but still in one map?

I was thinking that we could put the mux values into the config array,
so instead of the mapping table for a combined mux+config entry having
specific fields for:

* pin group to configure
* mux function to set (or set to NULL/-1/... if not used)
* list of config options to set (or NULL if not used)

We could put the mux function into the config array. That way, instead
of having two optional mux and config fields, we'd have 1 mandatory
config field, and the config array may-or-may-not have a mux entry in
it, e.g.:

unsigned long tegra_config_ata[] = {
      TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_MUX, TEGRA_MUX_FUNCTION_SPI1),
	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_PULL, TEGRA_PINCONFIG_PULL_NONE),
	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_TRISTATE, TEGRA_PINCONFIG_DRIVEN),
};

However, upon further reflection, that might be a bad idea, because then
there would be no way to share common sets of pin config entries between
multiple mapping table entries, since every pin/group would differ in
their mux setting, i.e. the following couldn't share anything:

unsigned long tegra_config_atb[] = {
      TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_MUX, TEGRA_MUX_FUNCTION_SPI2),
	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_PULL, TEGRA_PINCONFIG_PULL_NONE),
	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_TRISTATE, TEGRA_PINCONFIG_DRIVEN),
};

But with the current scheme the mux setting isn't there, so the two arrays
would be the same and could be shared.

-- 
nvpublic


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations
  2012-03-02 22:49         ` Stephen Warren
@ 2012-03-05  7:58           ` Dong Aisheng
  0 siblings, 0 replies; 18+ messages in thread
From: Dong Aisheng @ 2012-03-05  7:58 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Linus Walleij, Linus Walleij, B29396, s.hauer, dongas86,
	shawn.guo, thomas.abraham, tony, linux-kernel

On Fri, Mar 02, 2012 at 02:49:35PM -0800, Stephen Warren wrote:
> Dong Aisheng wrote at Thursday, March 01, 2012 8:47 PM:
> > On Thu, Mar 01, 2012 at 08:52:12AM -0800, Stephen Warren wrote:
> > > Dong Aisheng wrote at Thursday, March 01, 2012 1:46 AM:
> > > > On Tue, Feb 28, 2012 at 01:27:31PM -0700, Stephen Warren wrote:
.......
> > > > > +		.data.configs = {					\
> > > > > +			.group_or_pin = grp,				\
> > > > > +			.configs = cfgs,				\
> > > > > +			.num_configs = ARRAY_SIZE(cfgs),		\
> > > > > +		},							\
> > > > > +	}
> > > >
> > > > It seems you separate the pin mux setting and pin config setting in
> > > > different maps.
> > > > Can we merge them into one map?
> > > > That will save a lot of map entries and improve searching performance.
> > >
> > > Well, it's technically possible.
> > >
> > > But then, you'll need macros that just define a mux setting, just configs
> > > for groups, or both mux setting and config settings. Then, cross-product
> > > that with whether the state name is default or not, and cross-product that
> > > with whether the macro should hard-code group to NULL or not etc. etc...
> >
> > Well, they're indeed a lot.
> > But it seems they're not related to whether merging config and mux into one
> > map, right?
> 
> There's certainly some relationship; see my explanation above re: why
> you'd end up with a lot more macros. Of course, if we didn't create all
> the special-case macros, this problem wouldn't exist, so I don't think
> there would be a problem...
> 
Hmm, you missed some other words of mine in last mail.
It does not save too much.

And yes, the problem wouldn't exist if do not create all macros.
One way is that we can eliminate NULL group name support to reduce macros
(it seemed you agreed with it).

> > > It seems like somewhat of a nightmare. Parsing this table is in the slow-
> > > path of pinctrl_get() anyway, so I don't think it's an enormous deal.
> > >
> > > One alternative might be to represent mux settings as just another config
> > > on the group though. But that'd require the pin config namespace to have
> > > some standardized values and some driver-specific values.
> >
> > Do you mean treat them all(mux and config) as a config setting?
> > Then what benefits can we get?
> > Does it have any difference from separate them but still in one map?
> 
> I was thinking that we could put the mux values into the config array,
> so instead of the mapping table for a combined mux+config entry having
> specific fields for:
Yes, it is similar as my thinking.
The purpose is the same:
provide a more convenient way for user to use pinctrl, that says, users can use
one macro to define both mux and config.

> 
> * pin group to configure
> * mux function to set (or set to NULL/-1/... if not used)
> * list of config options to set (or NULL if not used)
> 
> We could put the mux function into the config array. That way, instead
> of having two optional mux and config fields, we'd have 1 mandatory
> config field, and the config array may-or-may-not have a mux entry in
> it, e.g.:
> 
> unsigned long tegra_config_ata[] = {
>       TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_MUX, TEGRA_MUX_FUNCTION_SPI1),
> 	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_PULL, TEGRA_PINCONFIG_PULL_NONE),
> 	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_TRISTATE, TEGRA_PINCONFIG_DRIVEN),
> };
> 
> However, upon further reflection, that might be a bad idea, because then
> there would be no way to share common sets of pin config entries between
> multiple mapping table entries, since every pin/group would differ in
> their mux setting, i.e. the following couldn't share anything:
> 
> unsigned long tegra_config_atb[] = {
>       TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_MUX, TEGRA_MUX_FUNCTION_SPI2),
> 	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_PULL, TEGRA_PINCONFIG_PULL_NONE),
> 	TEGRA_PINCONF_PACK(TEGRA_PINCONF_PARAM_TRISTATE, TEGRA_PINCONFIG_DRIVEN),
> };
> 
> But with the current scheme the mux setting isn't there, so the two arrays
> would be the same and could be shared.
> 
Yes, so i also prefer to separate them.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-03-05  7:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-28 20:27 [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Stephen Warren
2012-02-28 20:27 ` [PATCH V2 1/6] pinctrl: Fix and simplify locking Stephen Warren
2012-02-28 20:27 ` [PATCH V2 2/6] pinctrl: Refactor struct pinctrl handling in core.c vs pinmux.c Stephen Warren
2012-02-28 20:27 ` [PATCH V2 3/6] pinctrl: Add usecount to pins for muxing Stephen Warren
2012-02-28 20:27 ` [PATCH V2 4/6] pinctrl: API changes to support multiple states per device Stephen Warren
2012-02-29  6:46   ` Dong Aisheng
2012-02-29 17:22     ` Stephen Warren
2012-03-01  3:09       ` Dong Aisheng
2012-02-28 20:27 ` [PATCH V2 5/6] pinctrl: Enhance mapping table to support pin config operations Stephen Warren
2012-03-01  8:46   ` Dong Aisheng
2012-03-01 16:52     ` Stephen Warren
2012-03-02  3:46       ` Dong Aisheng
2012-03-02 22:49         ` Stephen Warren
2012-03-05  7:58           ` Dong Aisheng
2012-02-28 20:27 ` [PATCH V2 6/6] pinctrl: fix case of Tegra30's foo_groups[] arrays Stephen Warren
2012-02-29 16:46 ` [PATCH V2 0/6] pinctrl: API rework, pinconfig in mapping table, Linus Walleij
2012-02-29 17:42   ` Stephen Warren
2012-02-29 18:09     ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).