[PATCH v5 0/10] use pinctrl-single in arch mmp

[PATCH v5 0/10] use pinctrl-single in arch mmp

[Haojian Zhuang]

Changelog:
v5:
1. Move the properties of pinconf into pin group. So those mask
properties could be merged with other pinconf properties. 
2. Append lookup pinctrl map method.
3. Append input schmitt disable config parameter.
4. Clean code.

v4:
1. Define gpio range as sub-node, not label. And remove
pinctrl-single,gpio-ranges property.
2. Use new two properties in sub-node, reg &
pinctrl-single,gpio. GPIO number & GPIO function are listed in
the pinctrl-single,gpio property.
3. Reference the names like pinctrl-single,bias.
4. Add compatible name "pinconf-single". If the compatible name is
"pinctrl-single", there's no pinconf. If the compatible name is
"pinconf-single", there's the generic pinconf in pinctrl-single.
5. Update documents.

v3:
1. Add more comments in document.
2. Replace pcs_readl() & pcs_writel() by pcs->read() & pcs->write().
3. Clean code.

v2:
1. Remove "pinctrl-single,gpio-mask". Since GPIO function is one of the
mux function in the pinmux register of both OMAP and PXA/MMP silicons.
Use "pinctrl-single,function-mask" instead.
2. Remove "pinctrl-single,gpio-enable" & "pinctrl-single,gpio-disable".
Use "pinctrl-single,gpio-func" instead. Because GPIO mode is only one
of the mux functions in the pinmux register. Defining "gpio-enable" &
"gpio-disable" are redundant.
3. Define register with __iomem, not u32 type.
4. Remove "pinctrl-single,input-schmit-shift",
"pinctrl-single,power-source-shift", "pinctrl-single,bias-shift". All
these properties could be calculated by mask fields.
5. Return -EPROBE_DEFER if pinmux could be got in device driver. And
the device driver would be probed again deferred.

[PATCH v5 01/10] ARM: mmp: select pinctrl driver

[Haojian Zhuang]

Pinctrl driver is necessary for MMP DT & MMP2 DT platforms.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-mmp/Kconfig |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-mmp/Kconfig b/arch/arm/mach-mmp/Kconfig
index 178d4da..ebdda83 100644
--- a/arch/arm/mach-mmp/Kconfig
+++ b/arch/arm/mach-mmp/Kconfig
@@ -89,6 +89,8 @@ config MACH_MMP_DT
 	select CPU_PXA168
 	select CPU_PXA910
 	select USE_OF
+	select PINCTRL
+	select PINCTRL_SINGLE
 	help
 	  Include support for Marvell MMP2 based platforms using
 	  the device tree. Needn't select any other machine while
@@ -99,6 +101,8 @@ config MACH_MMP2_DT
 	depends on !CPU_MOHAWK
 	select CPU_MMP2
 	select USE_OF
+	select PINCTRL
+	select PINCTRL_SINGLE
 	help
 	  Include support for Marvell MMP2 based platforms using
 	  the device tree.
-- 
1.7.10.4

[PATCH v5 02/10] pinctrl: single: support gpio request and free

[Haojian Zhuang]

Marvell's PXA/MMP silicon also match the behavior of pinctrl-single.
Each pin binds to one register. A lot of pins could be configured
as gpio.

GPIO range is defined as a child node of pinmux in .dtsi file. If those
pins are with the same gpio function configuration in the pinmux
register, they could be defined in the same GPIO range. For this new
child node, two properties are used.

reg = <the start of pinmux register in range, size of range>

pinctrl-single,gpio: <gpio base in range, the gpio function of the range
		in the pinmux register>

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/pinctrl/pinctrl-single.c |   80 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 726a729..2476a68 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -30,6 +30,7 @@
 #define PCS_MUX_BITS_NAME		"pinctrl-single,bits"
 #define PCS_REG_NAME_LEN		((sizeof(unsigned long) * 2) + 1)
 #define PCS_OFF_DISABLED		~0U
+#define PCS_MAX_GPIO_VALUES		2
 
 /**
  * struct pcs_pingroup - pingroups for a function
@@ -77,6 +78,16 @@ struct pcs_function {
 };
 
 /**
+ * struct pcs_gpio_range - pinctrl gpio range
+ * @range:	subrange of the GPIO number space
+ * @gpio_func:	gpio function value in the pinmux register
+ */
+struct pcs_gpio_range {
+	struct pinctrl_gpio_range range;
+	int gpio_func;
+};
+
+/**
  * struct pcs_data - wrapper for data needed by pinctrl framework
  * @pa:		pindesc array
  * @cur:	index to current element
@@ -403,9 +414,26 @@ static void pcs_disable(struct pinctrl_dev *pctldev, unsigned fselector,
 }
 
 static int pcs_request_gpio(struct pinctrl_dev *pctldev,
-			struct pinctrl_gpio_range *range, unsigned offset)
+			    struct pinctrl_gpio_range *range, unsigned pin)
 {
-	return -ENOTSUPP;
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	struct pcs_gpio_range *gpio = NULL;
+	int end, mux_bytes;
+	unsigned data;
+
+	gpio = container_of(range, struct pcs_gpio_range, range);
+	end = range->pin_base + range->npins - 1;
+	if (pin < range->pin_base || pin > end) {
+		dev_err(pctldev->dev,
+			"pin %d isn't in the range of %d to %d\n",
+			pin, range->pin_base, end);
+		return -EINVAL;
+	}
+	mux_bytes = pcs->width / BITS_PER_BYTE;
+	data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
+	data |= gpio->gpio_func;
+	pcs->write(data, pcs->base + pin * mux_bytes);
+	return 0;
 }
 
 static struct pinmux_ops pcs_pinmux_ops = {
@@ -879,6 +907,50 @@ static void pcs_free_resources(struct pcs_device *pcs)
 
 static struct of_device_id pcs_of_match[];
 
+static int __devinit pcs_add_gpio_range(struct device_node *node,
+					struct pcs_device *pcs)
+{
+	struct pcs_gpio_range *gpio;
+	struct device_node *child;
+	struct resource r;
+	const char name[] = "pinctrl-single";
+	u32 gpiores[PCS_MAX_GPIO_VALUES];
+	int ret, i = 0, mux_bytes = 0;
+
+	for_each_child_of_node(node, child) {
+		ret = of_address_to_resource(child, 0, &r);
+		if (ret < 0)
+			continue;
+		memset(gpiores, 0, sizeof(u32) * PCS_MAX_GPIO_VALUES);
+		ret = of_property_read_u32_array(child, "pinctrl-single,gpio",
+						 gpiores, PCS_MAX_GPIO_VALUES);
+		if (ret < 0)
+			continue;
+		gpio = devm_kzalloc(pcs->dev, sizeof(*gpio), GFP_KERNEL);
+		if (!gpio) {
+			dev_err(pcs->dev, "failed to allocate pcs gpio\n");
+			return -ENOMEM;
+		}
+		gpio->range.name = devm_kzalloc(pcs->dev, sizeof(name),
+						GFP_KERNEL);
+		if (!gpio->range.name) {
+			dev_err(pcs->dev, "failed to allocate range name\n");
+			return -ENOMEM;
+		}
+		memcpy((char *)gpio->range.name, name, sizeof(name));
+
+		gpio->range.id = i++;
+		gpio->range.base = gpiores[0];
+		gpio->gpio_func = gpiores[1];
+		mux_bytes = pcs->width / BITS_PER_BYTE;
+		gpio->range.pin_base = (r.start - pcs->res->start) / mux_bytes;
+		gpio->range.npins = (r.end - r.start) / mux_bytes + 1;
+
+		pinctrl_add_gpio_range(pcs->pctl, &gpio->range);
+	}
+	return 0;
+}
+
 static int __devinit pcs_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -975,6 +1047,10 @@ static int __devinit pcs_probe(struct platform_device *pdev)
 		goto free;
 	}
 
+	ret = pcs_add_gpio_range(np, pcs);
+	if (ret < 0)
+		goto free;
+
 	dev_info(pcs->dev, "%i pins at pa %p size %u\n",
 		 pcs->desc.npins, pcs->base, pcs->size);
 
-- 
1.7.10.4

[PATCH v5 02/10] pinctrl: single: support gpio request and free

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@gmail.com> [121115 00:39]:
> Marvell's PXA/MMP silicon also match the behavior of pinctrl-single.
> Each pin binds to one register. A lot of pins could be configured
> as gpio.
> 
> GPIO range is defined as a child node of pinmux in .dtsi file. If those
> pins are with the same gpio function configuration in the pinmux
> register, they could be defined in the same GPIO range. For this new
> child node, two properties are used.
> 
> reg = <the start of pinmux register in range, size of range>
> 
> pinctrl-single,gpio: <gpio base in range, the gpio function of the range
> 		in the pinmux register>
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Acked-by: Tony Lindgren <tony@atomide.com>

[PATCH v5 02/10] pinctrl: single: support gpio request and free

[Linus Walleij]

On Thu, Nov 15, 2012 at 9:36 AM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:

> Marvell's PXA/MMP silicon also match the behavior of pinctrl-single.
> Each pin binds to one register. A lot of pins could be configured
> as gpio.
>
> GPIO range is defined as a child node of pinmux in .dtsi file. If those
> pins are with the same gpio function configuration in the pinmux
> register, they could be defined in the same GPIO range. For this new
> child node, two properties are used.
>
> reg = <the start of pinmux register in range, size of range>
>
> pinctrl-single,gpio: <gpio base in range, the gpio function of the range
>                 in the pinmux register>
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Patch applied with Tony's ACK.

Yours,
Linus Walleij

[PATCH v5 03/10] pinctrl: append method of lookup pinctrl map

[Haojian Zhuang]

pinctrl driver gets pinctrl map if both group and map type is specified.
In pinctrl-single driver, each pin group is defined in DT file. The pinconf
information is also defined in each pin group.

pinctrl-single driver could store those mask/shift/value information
into pinctrl map. pinconf_get()/pinconf_set() and
pinconf_group_get()/pinconf_group_set() could get those information from
pinctrl map.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/pinctrl/core.c           |   24 ++++++++++++++++++++++++
 include/linux/pinctrl/consumer.h |    4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 2e39c04..0d36270 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -839,6 +839,30 @@ int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
 }
 EXPORT_SYMBOL_GPL(pinctrl_select_state);
 
+const struct pinctrl_map *pinctrl_lookup_map(struct pinctrl *p,
+					     const char *name, unsigned type)
+{
+	struct pinctrl_maps *maps_node;
+	struct pinctrl_map const *map;
+	int i;
+
+	for_each_maps(maps_node, i, map) {
+		/* Map must be for this device */
+		if (strcmp(map->ctrl_dev_name, dev_name(p->dev)))
+			continue;
+		if (map->type != type)
+			continue;
+		if (map->type == PIN_MAP_TYPE_MUX_GROUP)
+			if (!strcmp(map->data.mux.group, name))
+				return map;
+		if (map->type == PIN_MAP_TYPE_CONFIGS_GROUP)
+			if (!strcmp(map->data.configs.group_or_pin, name))
+				return map;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(pinctrl_lookup_map);
+
 static void devm_pinctrl_release(struct device *dev, void *res)
 {
 	pinctrl_put(*(struct pinctrl **)res);
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 4aad3ce..2687515 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -36,6 +36,10 @@ 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);
+extern const struct pinctrl_map * __must_check pinctrl_lookup_map(
+						struct pinctrl *p,
+						const char *name,
+						unsigned type);
 
 extern struct pinctrl * __must_check devm_pinctrl_get(struct device *dev);
 extern void devm_pinctrl_put(struct pinctrl *p);
-- 
1.7.10.4

[PATCH v5 03/10] pinctrl: append method of lookup pinctrl map

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@gmail.com> [121115 00:39]:
> --- a/include/linux/pinctrl/consumer.h
> +++ b/include/linux/pinctrl/consumer.h
> @@ -36,6 +36,10 @@ 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);
> +extern const struct pinctrl_map * __must_check pinctrl_lookup_map(
> +						struct pinctrl *p,
> +						const char *name,
> +						unsigned type);
>  
>  extern struct pinctrl * __must_check devm_pinctrl_get(struct device *dev);
>  extern void devm_pinctrl_put(struct pinctrl *p);

I think this should be in drivers/pinctrl/core.h, or is there
a need for the consumer drivers to use this?

Regards,

Tony

[PATCH v5 03/10] pinctrl: append method of lookup pinctrl map

[Haojian Zhuang]

On Sat, Nov 17, 2012 at 8:46 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@gmail.com> [121115 00:39]:
>> --- a/include/linux/pinctrl/consumer.h
>> +++ b/include/linux/pinctrl/consumer.h
>> @@ -36,6 +36,10 @@ 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);
>> +extern const struct pinctrl_map * __must_check pinctrl_lookup_map(
>> +                                             struct pinctrl *p,
>> +                                             const char *name,
>> +                                             unsigned type);
>>
>>  extern struct pinctrl * __must_check devm_pinctrl_get(struct device *dev);
>>  extern void devm_pinctrl_put(struct pinctrl *p);
>
> I think this should be in drivers/pinctrl/core.h, or is there
> a need for the consumer drivers to use this?
>
Seems reasonable. I'll move into into core.h.

[PATCH v5 04/10] pinctrl: generic: add input schmitt disable parameter

[Haojian Zhuang]

In Marvell PXA/MMP silicons, input schmitt disable value is 0x40, not 0.
So append new config parameter -- input schmitt disable.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/pinctrl/pinconf-generic.c       |    1 +
 include/linux/pinctrl/pinconf-generic.h |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 33fbaea..833a364 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -41,6 +41,7 @@ struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_DRIVE_PUSH_PULL, "output drive push pull", NULL),
 	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_DRAIN, "output drive open drain", NULL),
 	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_SOURCE, "output drive open source", NULL),
+	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_DISABLE, "input schmitt disabled", NULL),
 	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT, "input schmitt trigger", NULL),
 	PCONFDUMP(PIN_CONFIG_INPUT_DEBOUNCE, "input debounce", "time units"),
 	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector"),
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index 4f0abb9..47a1bdd 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -46,11 +46,11 @@
  * @PIN_CONFIG_DRIVE_OPEN_SOURCE: the pin will be driven with open source
  *	(open emitter). Sending this config will enabale open drain mode, the
  *	argument is ignored.
+ * @PIN_CONFIG_INPUT_SCHMITT_DISABLE: disable schmitt-trigger mode on the pin.
  * @PIN_CONFIG_INPUT_SCHMITT: this will configure an input pin to run in
  *	schmitt-trigger mode. If the schmitt-trigger has adjustable hysteresis,
  *	the threshold value is given on a custom format as argument when
- *	setting pins to this mode. The argument zero turns the schmitt trigger
- *	off.
+ *	setting pins to this mode.
  * @PIN_CONFIG_INPUT_DEBOUNCE: this will configure the pin to debounce mode,
  *	which means it will wait for signals to settle when reading inputs. The
  *	argument gives the debounce time on a custom format. Setting the
@@ -74,6 +74,7 @@ enum pin_config_param {
 	PIN_CONFIG_DRIVE_PUSH_PULL,
 	PIN_CONFIG_DRIVE_OPEN_DRAIN,
 	PIN_CONFIG_DRIVE_OPEN_SOURCE,
+	PIN_CONFIG_INPUT_SCHMITT_DISABLE,
 	PIN_CONFIG_INPUT_SCHMITT,
 	PIN_CONFIG_INPUT_DEBOUNCE,
 	PIN_CONFIG_POWER_SOURCE,
-- 
1.7.10.4

[PATCH v5 04/10] pinctrl: generic: add input schmitt disable parameter

[Linus Walleij]

On Thu, Nov 15, 2012 at 9:36 AM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:

> In Marvell PXA/MMP silicons, input schmitt disable value is 0x40, not 0.
> So append new config parameter -- input schmitt disable.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>

I've applied this to the pinctrl tree, doesn't hurt anyone...

Yours,
Linus Walleij

[PATCH v5 05/10] pinctrl: single: support pinconf generic

[Haojian Zhuang]

Add pinconf generic support with POWER SOURCE, BIAS PULL.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/pinctrl/Kconfig          |    1 +
 drivers/pinctrl/pinctrl-single.c |  322 +++++++++++++++++++++++++++++++++++---
 2 files changed, 305 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 7bf914d..e9f2d2d 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -139,6 +139,7 @@ config PINCTRL_SINGLE
 	depends on OF
 	select PINMUX
 	select PINCONF
+	select GENERIC_PINCONF
 	help
 	  This selects the device tree based generic pinctrl driver.
 
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 2476a68..be12aca 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -20,6 +20,8 @@
 #include <linux/of_device.h>
 #include <linux/of_address.h>
 
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 
@@ -33,6 +35,26 @@
 #define PCS_MAX_GPIO_VALUES		2
 
 /**
+ * union pcs_pinconf_argument_t -- upper 16-bit in the pinconf data
+ *
+ * The 32-bit pinmux register value could be divided into some different
+ * config arguments. Each config parameter binds to one config argument.
+ * And each config argument can't exceed 5-bit value.
+ * @data:	argument of config in pinconf generic
+ * @value:	5-bit value
+ * @mask:	5-bit mask
+ * @shift:	shift of value/mask field in pinmux register
+ */
+union pcs_pinconf_argument_t {
+	u16 data;
+	struct {
+		unsigned value:5;
+		unsigned mask:5;
+		unsigned shift:5;
+	} bits;
+};
+
+/**
  * struct pcs_pingroup - pingroups for a function
  * @np:		pingroup device node pointer
  * @name:	pingroup name
@@ -88,6 +110,16 @@ struct pcs_gpio_range {
 };
 
 /**
+ * struct pcs_conf - configuration of pinctrl device
+ * @nconf:	number of configuration that is defined for pinconf
+ * @is_generic:	for pin controller that want to use the generic interface
+ */
+struct pcs_conf {
+	unsigned nconfs;
+	bool is_generic;
+};
+
+/**
  * struct pcs_data - wrapper for data needed by pinctrl framework
  * @pa:		pindesc array
  * @cur:	index to current element
@@ -130,6 +162,7 @@ struct pcs_name {
  * @fmax:	max number of functions in fmask
  * @names:	array of register names for pins
  * @pins:	physical pins on the SoC
+ * @conf:	value of pinconf
  * @pgtree:	pingroup index radix tree
  * @ftree:	function index radix tree
  * @pingroups:	list of pingroups
@@ -155,6 +188,7 @@ struct pcs_device {
 	bool bits_per_mux;
 	struct pcs_name *names;
 	struct pcs_data pins;
+	struct pcs_conf conf;
 	struct radix_tree_root pgtree;
 	struct radix_tree_root ftree;
 	struct list_head pingroups;
@@ -445,28 +479,168 @@ static struct pinmux_ops pcs_pinmux_ops = {
 	.gpio_request_enable = pcs_request_gpio,
 };
 
+static void pcs_free_pingroups(struct pcs_device *pcs);
+
+static int pcs_pinconf_get_config(struct pinctrl_dev *pctldev, unsigned pin,
+				  enum pin_config_param config_param)
+{
+	const struct pinctrl_map *map;
+	enum pin_config_param param;
+	const unsigned *pins = NULL;
+	const char *name;
+	unsigned long *conf;
+	unsigned count, group, i, npins;
+	int found = 0;
+
+	count = pcs_get_groups_count(pctldev);
+	for (group = 0; group < count; group++) {
+		if (pcs_get_group_pins(pctldev, group, &pins, &npins))
+			return -EINVAL;
+		for (i = 0; i < npins; i++) {
+			if (pins[i] == pin) {
+				found = 1;
+				break;
+			}
+		}
+		if (found)
+			break;
+	}
+	if (!found)
+		return -ENOTSUPP;
+
+	name = pcs_get_group_name(pctldev, group);
+	if (!name)
+		return -EINVAL;
+	map = pinctrl_lookup_map(pctldev->p, name, PIN_MAP_TYPE_CONFIGS_GROUP);
+	if (!map)
+		return -ENOTSUPP;
+
+	conf = map->data.configs.configs;
+	for (i = 0; i < map->data.configs.num_configs; i++) {
+		param = pinconf_to_config_param(conf[i]);
+		if (config_param == param)
+			return conf[i];
+	}
+	return -ENOTSUPP;
+}
+
 static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
 				unsigned pin, unsigned long *config)
 {
-	return -ENOTSUPP;
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param config_param = pinconf_to_config_param(*config);
+	union pcs_pinconf_argument_t arg;
+	u32 offset;
+	unsigned data;
+	int ret;
+
+	if (!pcs->conf.nconfs)
+		return -ENOTSUPP;
+
+	ret = pcs_pinconf_get_config(pctldev, pin, config_param);
+	if (ret < 0)
+		return ret;
+
+	offset = pin * (pcs->width / BITS_PER_BYTE);
+	data = pcs->read(pcs->base + offset);
+
+	arg.data = pinconf_to_config_argument(ret);
+	data = (data >> arg.bits.shift) & arg.bits.mask;
+	arg.bits.value = data;
+	*config = pinconf_to_config_packed(config_param, arg.data);
+	return 0;
 }
 
 static int pcs_pinconf_set(struct pinctrl_dev *pctldev,
 				unsigned pin, unsigned long config)
 {
-	return -ENOTSUPP;
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param config_param = pinconf_to_config_param(config);
+	union pcs_pinconf_argument_t arg;
+	u32 offset;
+	unsigned data;
+	int ret;
+
+	if (!pcs->conf.nconfs)
+		return -ENOTSUPP;
+
+	ret = pcs_pinconf_get_config(pctldev, pin, config_param);
+	if (ret < 0)
+		return ret;
+
+	offset = pin * (pcs->width / BITS_PER_BYTE);
+	data = pcs->read(pcs->base + offset);
+
+	arg.data = pinconf_to_config_argument(config);
+	data = (data >> arg.bits.shift) & arg.bits.mask;
+	arg.bits.value = data;
+	data = pinconf_to_config_packed(config_param, arg.data);
+	pcs->write(data, pcs->base + offset);
+	return 0;
 }
 
 static int pcs_pinconf_group_get(struct pinctrl_dev *pctldev,
 				unsigned group, unsigned long *config)
 {
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	const struct pinctrl_map *map;
+	enum pin_config_param param, config_param;
+	union pcs_pinconf_argument_t arg;
+	const char *name;
+	unsigned long *conf;
+	int i;
+
+	if (!pcs->conf.nconfs)
+		return -ENOTSUPP;
+
+	name = pcs_get_group_name(pctldev, group);
+	if (!name)
+		return -EINVAL;
+	map = pinctrl_lookup_map(pctldev->p, name, PIN_MAP_TYPE_CONFIGS_GROUP);
+	if (!map)
+		return -ENOTSUPP;
+	config_param = pinconf_to_config_param(*config);
+	conf = map->data.configs.configs;
+	for (i = 0; i < map->data.configs.num_configs; i++) {
+		param = pinconf_to_config_param(conf[i]);
+		if (config_param == param) {
+			arg.data = pinconf_to_config_argument(conf[i]);
+			*config = pinconf_to_config_packed(param, arg.data);
+			return 0;
+		}
+	}
 	return -ENOTSUPP;
 }
 
 static int pcs_pinconf_group_set(struct pinctrl_dev *pctldev,
 				unsigned group, unsigned long config)
 {
-	return -ENOTSUPP;
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	struct pcs_pingroup *pins;
+	union pcs_pinconf_argument_t arg;
+	u32 offset, data;
+	unsigned ret, mask;
+	int i, mux_bytes;
+
+	if (!pcs->conf.nconfs)
+		return -ENOTSUPP;
+
+	pins = radix_tree_lookup(&pcs->pgtree, group);
+	if (!pins) {
+		dev_err(pcs->dev, "%s could not find pingroup%i\n",
+			__func__, group);
+		return -EINVAL;
+	}
+	mux_bytes = pcs->width / BITS_PER_BYTE;
+	arg.data = pinconf_to_config_argument(config);
+	mask = arg.bits.mask << arg.bits.shift;
+	data = arg.bits.value << arg.bits.shift;
+	for (i = 0; i < pins->ngpins; i++) {
+		offset = pins->gpins[i] * mux_bytes;
+		ret = pcs->read(pcs->base + offset) & ~mask;
+		pcs->write(ret | data, pcs->base + offset);
+	}
+	return 0;
 }
 
 static void pcs_pinconf_dbg_show(struct pinctrl_dev *pctldev,
@@ -676,8 +850,22 @@ static int pcs_get_pin_by_offset(struct pcs_device *pcs, unsigned offset)
 	return index;
 }
 
+static int pcs_config_match(unsigned data, unsigned match)
+{
+	int ret = 0;
+
+	if (!match) {
+		if (!data)
+			ret = 1;
+	} else {
+		if ((data & match) == match)
+			ret = 1;
+	}
+	return ret;
+}
+
 /**
- * smux_parse_one_pinctrl_entry() - parses a device tree mux entry
+ * pcs_parse_one_pinctrl_entry() - parses a device tree mux entry
  * @pcs: pinctrl driver instance
  * @np: device node of the mux entry
  * @map: map entry
@@ -700,9 +888,13 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 						const char **pgnames)
 {
 	struct pcs_func_vals *vals;
+	struct pinctrl_map *m = *map;
 	const __be32 *mux;
-	int size, params, rows, *pins, index = 0, found = 0, res = -ENOMEM;
+	int size, params, rows, *pins, i = 0, found = 0, res = -ENOMEM;
 	struct pcs_function *function;
+	unsigned long *conf;
+	union pcs_pinconf_argument_t arg;
+	u32 value[8], data, nconfs;
 
 	if (pcs->bits_per_mux) {
 		params = 3;
@@ -733,16 +925,16 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 	if (!pins)
 		goto free_vals;
 
-	while (index < size) {
+	while (i < size) {
 		unsigned offset, val;
 		int pin;
 
-		offset = be32_to_cpup(mux + index++);
-		val = be32_to_cpup(mux + index++);
+		offset = be32_to_cpup(mux + i++);
+		val = be32_to_cpup(mux + i++);
 		vals[found].reg = pcs->base + offset;
 		vals[found].val = val;
 		if (params == 3) {
-			val = be32_to_cpup(mux + index++);
+			val = be32_to_cpup(mux + i++);
 			vals[found].mask = val;
 		}
 
@@ -756,6 +948,7 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 		pins[found++] = pin;
 	}
 
+	nconfs = pcs->conf.nconfs;
 	pgnames[0] = np->name;
 	function = pcs_add_function(pcs, np, np->name, vals, found, pgnames, 1);
 	if (!function)
@@ -765,12 +958,82 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 	if (res < 0)
 		goto free_function;
 
-	(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)->data.mux.group = np->name;
-	(*map)->data.mux.function = np->name;
+	m->type = PIN_MAP_TYPE_MUX_GROUP;
+	m->data.mux.group = np->name;
+	m->data.mux.function = np->name;
+
+	if (!nconfs)
+		return 0;
+
+	conf = devm_kzalloc(pcs->dev, sizeof(*conf) * nconfs, GFP_KERNEL);
+	if (!conf) {
+		res = -ENOMEM;
+		goto free_pingroup;
+	}
+	i = 0;
+	if (!of_property_read_u32_array(np, "pinctrl-single,bias",
+					value, 5)) {
+		/* array: bias value, mask, disable, pull down, pull up */
+		data = value[0] & value[1];
+		arg.bits.shift = ffs(value[1]) - 1;
+		arg.bits.mask = value[1] >> arg.bits.shift;
+		if (pcs_config_match(data, value[2])) {
+			arg.bits.value = value[2] >> arg.bits.shift;
+			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_DISABLE,
+						    arg.data);
+		}
+		if (pcs_config_match(data, value[3])) {
+			arg.bits.value = value[3] >> arg.bits.shift;
+			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_DOWN,
+						    arg.data);
+		}
+		if (pcs_config_match(data, value[4])) {
+			arg.bits.value = value[4] >> arg.bits.shift;
+			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_UP,
+						    arg.data);
+		}
+	}
+	if (!of_property_read_u32_array(np, "pinctrl-single,power-source",
+					value, 2)) {
+		/* array: power source value, mask */
+		data = value[0] & value[1];
+		arg.bits.shift = ffs(value[1]) - 1;
+		arg.bits.mask = value[1] >> arg.bits.shift;
+		arg.bits.value = data >> arg.bits.shift;
+		conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_POWER_SOURCE, arg.data);
+	}
+	if (!of_property_read_u32_array(np, "pinctrl-single,input-schmitt",
+					value, 3)) {
+		/* array: input schmitt value, mask, disable */
+		data = value[0] & value[1];
+		arg.bits.shift = ffs(value[1]) - 1;
+		arg.bits.mask = value[1] >> arg.bits.shift;
+		if (pcs_config_match(data, value[2])) {
+			arg.bits.value = value[2] >> arg.bits.shift;
+			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_DISABLE,
+						    arg.data);
+		} else {
+			arg.bits.value = data >> arg.bits.shift;
+			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT,
+						    arg.data);
+		}
+	}
+	m++;
+	m->type = PIN_MAP_TYPE_CONFIGS_GROUP;
+	m->data.configs.group_or_pin = np->name;
+	m->data.configs.configs = conf;
+	if (i > nconfs) {
+		dev_err(pcs->dev, "pinconf items (%d) more than nconfs (%d)\n",
+			i, nconfs);
+		i = nconfs;
+	}
+	m->data.configs.num_configs = i;
 
 	return 0;
 
+free_pingroup:
+	pcs_free_pingroups(pcs);
+
 free_function:
 	pcs_remove_function(pcs, function);
 
@@ -782,6 +1045,7 @@ free_vals:
 
 	return res;
 }
+
 /**
  * pcs_dt_node_to_map() - allocates and parses pinctrl maps
  * @pctldev: pinctrl instance
@@ -797,13 +1061,17 @@ static int pcs_dt_node_to_map(struct pinctrl_dev *pctldev,
 	const char **pgnames;
 	int ret;
 
+
 	pcs = pinctrl_dev_get_drvdata(pctldev);
 
-	*map = devm_kzalloc(pcs->dev, sizeof(**map), GFP_KERNEL);
-	if (!map)
-		return -ENOMEM;
+	if (!pcs->conf.nconfs)
+		*num_maps = 1;
+	else
+		*num_maps = 2;
 
-	*num_maps = 0;
+	*map = devm_kzalloc(pcs->dev, sizeof(**map) * (*num_maps), GFP_KERNEL);
+	if (!*map)
+		return -ENOMEM;
 
 	pgnames = devm_kzalloc(pcs->dev, sizeof(*pgnames), GFP_KERNEL);
 	if (!pgnames) {
@@ -817,7 +1085,6 @@ static int pcs_dt_node_to_map(struct pinctrl_dev *pctldev,
 			np_config->name);
 		goto free_pgnames;
 	}
-	*num_maps = 1;
 
 	return 0;
 
@@ -957,11 +1224,13 @@ static int __devinit pcs_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 	struct resource *res;
 	struct pcs_device *pcs;
+	const struct pcs_conf *conf;
 	int ret;
 
 	match = of_match_device(pcs_of_match, &pdev->dev);
 	if (!match)
 		return -EINVAL;
+	conf = match->data;
 
 	pcs = devm_kzalloc(&pdev->dev, sizeof(*pcs), GFP_KERNEL);
 	if (!pcs) {
@@ -969,6 +1238,7 @@ static int __devinit pcs_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 	pcs->dev = &pdev->dev;
+	memcpy(&pcs->conf, conf, sizeof(*conf));
 	mutex_init(&pcs->mutex);
 	INIT_LIST_HEAD(&pcs->pingroups);
 	INIT_LIST_HEAD(&pcs->functions);
@@ -989,6 +1259,9 @@ static int __devinit pcs_probe(struct platform_device *pdev)
 	pcs->bits_per_mux = of_property_read_bool(np,
 						  "pinctrl-single,bit-per-mux");
 
+	if (conf->nconfs)
+		pcs_pinconf_ops.is_generic = true;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(pcs->dev, "could not get resource\n");
@@ -1074,8 +1347,21 @@ static int __devexit pcs_remove(struct platform_device *pdev)
 	return 0;
 }
 
+/* PINCONF isn't supported */
+static struct pcs_conf pinctrl_conf __devinitdata = {
+	.nconfs = 0,
+	.is_generic = false,
+};
+
+/* Generic PINCONF is supported. */
+static struct pcs_conf pinconf_conf __devinitdata = {
+	.nconfs = 7,
+	.is_generic = true,
+};
+
 static struct of_device_id pcs_of_match[] __devinitdata = {
-	{ .compatible = DRIVER_NAME, },
+	{ .compatible = "pinctrl-single", .data = &pinctrl_conf },
+	{ .compatible = "pinconf-single", .data = &pinconf_conf },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, pcs_of_match);
-- 
1.7.10.4

[PATCH v5 05/10] pinctrl: single: support pinconf generic

[Tony Lindgren]

Hi,

>  static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
>  				unsigned pin, unsigned long *config)
>  {
> -	return -ENOTSUPP;
> +	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param config_param = pinconf_to_config_param(*config);
> +	union pcs_pinconf_argument_t arg;
> +	u32 offset;
> +	unsigned data;
> +	int ret;
> +
> +	if (!pcs->conf.nconfs)
> +		return -ENOTSUPP;
> +
> +	ret = pcs_pinconf_get_config(pctldev, pin, config_param);
> +	if (ret < 0)
> +		return ret;
> +
> +	offset = pin * (pcs->width / BITS_PER_BYTE);
> +	data = pcs->read(pcs->base + offset);
> +
> +	arg.data = pinconf_to_config_argument(ret);
> +	data = (data >> arg.bits.shift) & arg.bits.mask;

Shouldn't you just return data here?

	*config = data;

	return 0;

> +	arg.bits.value = data;
> +	*config = pinconf_to_config_packed(config_param, arg.data);
> +	return 0;

Instead of these? Or how else is the consumer driver supposed
to use the value?

>  }
>  
>  static int pcs_pinconf_set(struct pinctrl_dev *pctldev,
>  				unsigned pin, unsigned long config)
>  {
> -	return -ENOTSUPP;
> +	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param config_param = pinconf_to_config_param(config);
> +	union pcs_pinconf_argument_t arg;
> +	u32 offset;
> +	unsigned data;
> +	int ret;
> +
> +	if (!pcs->conf.nconfs)
> +		return -ENOTSUPP;
> +
> +	ret = pcs_pinconf_get_config(pctldev, pin, config_param);
> +	if (ret < 0)
> +		return ret;
> +
> +	offset = pin * (pcs->width / BITS_PER_BYTE);
> +	data = pcs->read(pcs->base + offset);
> +
> +	arg.data = pinconf_to_config_argument(config);
> +	data = (data >> arg.bits.shift) & arg.bits.mask;
> +	arg.bits.value = data;
> +	data = pinconf_to_config_packed(config_param, arg.data);
> +	pcs->write(data, pcs->base + offset);
> +	return 0;
>  }

I need to look at this more to understand how this actually relates
to what gets written to the register with my test case :)
  
> @@ -765,12 +958,82 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
>  	if (res < 0)
>  		goto free_function;
>  
> -	(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
> -	(*map)->data.mux.group = np->name;
> -	(*map)->data.mux.function = np->name;
> +	m->type = PIN_MAP_TYPE_MUX_GROUP;
> +	m->data.mux.group = np->name;
> +	m->data.mux.function = np->name;
> +
> +	if (!nconfs)
> +		return 0;
> +
> +	conf = devm_kzalloc(pcs->dev, sizeof(*conf) * nconfs, GFP_KERNEL);
> +	if (!conf) {
> +		res = -ENOMEM;
> +		goto free_pingroup;
> +	}
> +	i = 0;
> +	if (!of_property_read_u32_array(np, "pinctrl-single,bias",
> +					value, 5)) {
> +		/* array: bias value, mask, disable, pull down, pull up */
> +		data = value[0] & value[1];
> +		arg.bits.shift = ffs(value[1]) - 1;
> +		arg.bits.mask = value[1] >> arg.bits.shift;
> +		if (pcs_config_match(data, value[2])) {
> +			arg.bits.value = value[2] >> arg.bits.shift;
> +			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_DISABLE,
> +						    arg.data);
> +		}
> +		if (pcs_config_match(data, value[3])) {
> +			arg.bits.value = value[3] >> arg.bits.shift;
> +			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_DOWN,
> +						    arg.data);
> +		}
> +		if (pcs_config_match(data, value[4])) {
> +			arg.bits.value = value[4] >> arg.bits.shift;
> +			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_UP,
> +						    arg.data);
> +		}
> +	}

Doesn't this mean you only get one of the above working? What happens
if you initially need to set BIAS_DISABLE, but then the consumer driver
wants to set BIAS_PULL_DOWN or something?

Maybe we need separate pinctrl-single,bias-disable, pinctrl-single,bias-pull-down
and pinctrl-single,bias-pull-up?

> +	if (!of_property_read_u32_array(np, "pinctrl-single,power-source",
> +					value, 2)) {
> +		/* array: power source value, mask */
> +		data = value[0] & value[1];
> +		arg.bits.shift = ffs(value[1]) - 1;
> +		arg.bits.mask = value[1] >> arg.bits.shift;
> +		arg.bits.value = data >> arg.bits.shift;
> +		conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_POWER_SOURCE, arg.data);
> +	}
> +	if (!of_property_read_u32_array(np, "pinctrl-single,input-schmitt",
> +					value, 3)) {
> +		/* array: input schmitt value, mask, disable */
> +		data = value[0] & value[1];
> +		arg.bits.shift = ffs(value[1]) - 1;
> +		arg.bits.mask = value[1] >> arg.bits.shift;
> +		if (pcs_config_match(data, value[2])) {
> +			arg.bits.value = value[2] >> arg.bits.shift;
> +			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_DISABLE,
> +						    arg.data);
> +		} else {
> +			arg.bits.value = data >> arg.bits.shift;
> +			conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT,
> +						    arg.data);
> +		}
> +	}

And I think this has a similar problem? What if the consumer driver wants
to set INPUT_SCHMITT with pin_config_set() and the initial value is
SCHMITT_DISABLE? Doesn't the consumer driver get -ENOTSUPP in that case?

I'll look at this more early next week and try to get my testcase working
with it.

Regards,

Tony

[PATCH v5 05/10] pinctrl: single: support pinconf generic

[Haojian Zhuang]

On Sat, Nov 17, 2012 at 8:43 AM, Tony Lindgren <tony@atomide.com> wrote:
> Hi,
>
>>  static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
>>                               unsigned pin, unsigned long *config)
>>  {
>> -     return -ENOTSUPP;
>> +     struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>> +     enum pin_config_param config_param = pinconf_to_config_param(*config);
>> +     union pcs_pinconf_argument_t arg;
>> +     u32 offset;
>> +     unsigned data;
>> +     int ret;
>> +
>> +     if (!pcs->conf.nconfs)
>> +             return -ENOTSUPP;
>> +
>> +     ret = pcs_pinconf_get_config(pctldev, pin, config_param);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     offset = pin * (pcs->width / BITS_PER_BYTE);
>> +     data = pcs->read(pcs->base + offset);
>> +
>> +     arg.data = pinconf_to_config_argument(ret);
>> +     data = (data >> arg.bits.shift) & arg.bits.mask;
>
> Shouldn't you just return data here?
>
>         *config = data;
>
>         return 0;
>

Since we're using pinconf-generic, let's review how
pinconf_generic_dump_pin() works.

pinconf_generic_dump_pin():
        config = pinconf_to_config_packed(conf_items[i].param, 0);
        The lower 16-bit values are parameters. and the upper 16-bit
values are pre-set with 0 for argument.

        ret = pin_config_get_for_pin(pctldev, pin, &config);
        It'll invoke pcs_pinconf_get() that we're taking care. I want
to return the value with both argument
and parameters. Since the argument may be any location in 32-bit
register, I organize them into packed
structure. If I return the value field directly, it may conflict with
the lower 16-bit config parameter.

        if (conf_items[i].format && pinconf_to_config_argument(config) != 0)
            seq_printf(s, " (%u %s)",
pinconf_to_config_argument(config), conf_items[i].format);
        At here, it loads the upper 16-bit config argument. It also
means that returning value field directly
will break this.

        So how about change it in below.
        if (conf_items[i].format)
            seq_printf(s, " (%u %s)", config, conf_items[i].format);

>> +     arg.bits.value = data;
>> +     *config = pinconf_to_config_packed(config_param, arg.data);
>> +     return 0;
>
> Instead of these? Or how else is the consumer driver supposed
> to use the value?
>

I agree that returning data directly would be easy for the usage in
device driver. Let's define it as "data >> shift".
Is it OK?

>>  }
>>
>>  static int pcs_pinconf_set(struct pinctrl_dev *pctldev,
>>                               unsigned pin, unsigned long config)
>>  {
>> -     return -ENOTSUPP;
>> +     struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>> +     enum pin_config_param config_param = pinconf_to_config_param(config);
>> +     union pcs_pinconf_argument_t arg;
>> +     u32 offset;
>> +     unsigned data;
>> +     int ret;
>> +
>> +     if (!pcs->conf.nconfs)
>> +             return -ENOTSUPP;
>> +
>> +     ret = pcs_pinconf_get_config(pctldev, pin, config_param);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     offset = pin * (pcs->width / BITS_PER_BYTE);
>> +     data = pcs->read(pcs->base + offset);
>> +
>> +     arg.data = pinconf_to_config_argument(config);
>> +     data = (data >> arg.bits.shift) & arg.bits.mask;
>> +     arg.bits.value = data;
>> +     data = pinconf_to_config_packed(config_param, arg.data);
>> +     pcs->write(data, pcs->base + offset);
>> +     return 0;
>>  }
>
> I need to look at this more to understand how this actually relates
> to what gets written to the register with my test case :)

No problem.
>
>> @@ -765,12 +958,82 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
>>       if (res < 0)
>>               goto free_function;
>>
>> -     (*map)->type = PIN_MAP_TYPE_MUX_GROUP;
>> -     (*map)->data.mux.group = np->name;
>> -     (*map)->data.mux.function = np->name;
>> +     m->type = PIN_MAP_TYPE_MUX_GROUP;
>> +     m->data.mux.group = np->name;
>> +     m->data.mux.function = np->name;
>> +
>> +     if (!nconfs)
>> +             return 0;
>> +
>> +     conf = devm_kzalloc(pcs->dev, sizeof(*conf) * nconfs, GFP_KERNEL);
>> +     if (!conf) {
>> +             res = -ENOMEM;
>> +             goto free_pingroup;
>> +     }
>> +     i = 0;
>> +     if (!of_property_read_u32_array(np, "pinctrl-single,bias",
>> +                                     value, 5)) {
>> +             /* array: bias value, mask, disable, pull down, pull up */
>> +             data = value[0] & value[1];
>> +             arg.bits.shift = ffs(value[1]) - 1;
>> +             arg.bits.mask = value[1] >> arg.bits.shift;
>> +             if (pcs_config_match(data, value[2])) {
>> +                     arg.bits.value = value[2] >> arg.bits.shift;
>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_DISABLE,
>> +                                                 arg.data);
>> +             }
>> +             if (pcs_config_match(data, value[3])) {
>> +                     arg.bits.value = value[3] >> arg.bits.shift;
>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_DOWN,
>> +                                                 arg.data);
>> +             }
>> +             if (pcs_config_match(data, value[4])) {
>> +                     arg.bits.value = value[4] >> arg.bits.shift;
>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_UP,
>> +                                                 arg.data);
>> +             }
>> +     }
>
> Doesn't this mean you only get one of the above working? What happens
> if you initially need to set BIAS_DISABLE, but then the consumer driver
> wants to set BIAS_PULL_DOWN or something?
>
> Maybe we need separate pinctrl-single,bias-disable, pinctrl-single,bias-pull-down
> and pinctrl-single,bias-pull-up?
>
Good point. I'll separate them.

>> +     if (!of_property_read_u32_array(np, "pinctrl-single,power-source",
>> +                                     value, 2)) {
>> +             /* array: power source value, mask */
>> +             data = value[0] & value[1];
>> +             arg.bits.shift = ffs(value[1]) - 1;
>> +             arg.bits.mask = value[1] >> arg.bits.shift;
>> +             arg.bits.value = data >> arg.bits.shift;
>> +             conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_POWER_SOURCE, arg.data);
>> +     }
>> +     if (!of_property_read_u32_array(np, "pinctrl-single,input-schmitt",
>> +                                     value, 3)) {
>> +             /* array: input schmitt value, mask, disable */
>> +             data = value[0] & value[1];
>> +             arg.bits.shift = ffs(value[1]) - 1;
>> +             arg.bits.mask = value[1] >> arg.bits.shift;
>> +             if (pcs_config_match(data, value[2])) {
>> +                     arg.bits.value = value[2] >> arg.bits.shift;
>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_DISABLE,
>> +                                                 arg.data);
>> +             } else {
>> +                     arg.bits.value = data >> arg.bits.shift;
>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT,
>> +                                                 arg.data);
>> +             }
>> +     }
>
> And I think this has a similar problem? What if the consumer driver wants
> to set INPUT_SCHMITT with pin_config_set() and the initial value is
> SCHMITT_DISABLE? Doesn't the consumer driver get -ENOTSUPP in that case?
>
OK. I'll update them.

> I'll look at this more early next week and try to get my testcase working
> with it.
>
> Regards,
>
> Tony

[PATCH v5 05/10] pinctrl: single: support pinconf generic

[Haojian Zhuang]

On Sun, Nov 18, 2012 at 12:51 PM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:
> On Sat, Nov 17, 2012 at 8:43 AM, Tony Lindgren <tony@atomide.com> wrote:
>> Hi,
>>
>>>  static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
>>>                               unsigned pin, unsigned long *config)
>>>  {
>>> -     return -ENOTSUPP;
>>> +     struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>>> +     enum pin_config_param config_param = pinconf_to_config_param(*config);
>>> +     union pcs_pinconf_argument_t arg;
>>> +     u32 offset;
>>> +     unsigned data;
>>> +     int ret;
>>> +
>>> +     if (!pcs->conf.nconfs)
>>> +             return -ENOTSUPP;
>>> +
>>> +     ret = pcs_pinconf_get_config(pctldev, pin, config_param);
>>> +     if (ret < 0)
>>> +             return ret;
>>> +
>>> +     offset = pin * (pcs->width / BITS_PER_BYTE);
>>> +     data = pcs->read(pcs->base + offset);
>>> +
>>> +     arg.data = pinconf_to_config_argument(ret);
>>> +     data = (data >> arg.bits.shift) & arg.bits.mask;
>>
>> Shouldn't you just return data here?
>>
>>         *config = data;
>>
>>         return 0;
>>
>
> Since we're using pinconf-generic, let's review how
> pinconf_generic_dump_pin() works.
>
> pinconf_generic_dump_pin():
>         config = pinconf_to_config_packed(conf_items[i].param, 0);
>         The lower 16-bit values are parameters. and the upper 16-bit
> values are pre-set with 0 for argument.
>
>         ret = pin_config_get_for_pin(pctldev, pin, &config);
>         It'll invoke pcs_pinconf_get() that we're taking care. I want
> to return the value with both argument
> and parameters. Since the argument may be any location in 32-bit
> register, I organize them into packed
> structure. If I return the value field directly, it may conflict with
> the lower 16-bit config parameter.
>
>         if (conf_items[i].format && pinconf_to_config_argument(config) != 0)
>             seq_printf(s, " (%u %s)",
> pinconf_to_config_argument(config), conf_items[i].format);
>         At here, it loads the upper 16-bit config argument. It also
> means that returning value field directly
> will break this.
>
>         So how about change it in below.
>         if (conf_items[i].format)
>             seq_printf(s, " (%u %s)", config, conf_items[i].format);
>

Oh. I messed with something. Let's the usage case again.

pin_config_set() sets the config value into pinmux register. The upper 16-bit
is config argument, and the lower 16-bit is config parameter. The
pinmux register
is 32-bit in Marvell's silicon. I think it's 32-bit value in most silicons.

And real field value could be lower 16-bit. So I have to pack mask,
shift, config value
& config parameter into 32-bit data. The limitation is mask, shift,
value are all 5-bit
value. And this is also the reason that I define pinctrl_lookup_map()
into consumer.h.
How could consumer driver know mask & shift? It has to lookup map to
get the real
configuration.

We also need to keep pin_config_get() compatible with pin_config_set(). I define
the config data as packed mask, shift, config value & config parameter.

The conclusion is that we have two choices.
1. Use packed way. It's a little complex.
2. Change the interface of pin_config_set()/pin_config_get(). We need
to use the new
interface in below.
    pin_config_set(const char *dev_name, const char *name, unsigned
long parameter,
                          unsigned long config);
    pin_config_get(const char *dev_name, const char *name, unsigned
long parameter,
                          unsigned long *config);
    And the config should be "value >> shift".

What's your opinion? I prefer the second method since it's easy to
understand and maintain.
Then I could hide pinctrl_lookup_map() in core.h.

>>> +     arg.bits.value = data;
>>> +     *config = pinconf_to_config_packed(config_param, arg.data);
>>> +     return 0;
>>
>> Instead of these? Or how else is the consumer driver supposed
>> to use the value?
>>
>
> I agree that returning data directly would be easy for the usage in
> device driver. Let's define it as "data >> shift".
> Is it OK?
>
>>>  }
>>>
>>>  static int pcs_pinconf_set(struct pinctrl_dev *pctldev,
>>>                               unsigned pin, unsigned long config)
>>>  {
>>> -     return -ENOTSUPP;
>>> +     struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>>> +     enum pin_config_param config_param = pinconf_to_config_param(config);
>>> +     union pcs_pinconf_argument_t arg;
>>> +     u32 offset;
>>> +     unsigned data;
>>> +     int ret;
>>> +
>>> +     if (!pcs->conf.nconfs)
>>> +             return -ENOTSUPP;
>>> +
>>> +     ret = pcs_pinconf_get_config(pctldev, pin, config_param);
>>> +     if (ret < 0)
>>> +             return ret;
>>> +
>>> +     offset = pin * (pcs->width / BITS_PER_BYTE);
>>> +     data = pcs->read(pcs->base + offset);
>>> +
>>> +     arg.data = pinconf_to_config_argument(config);
>>> +     data = (data >> arg.bits.shift) & arg.bits.mask;
>>> +     arg.bits.value = data;
>>> +     data = pinconf_to_config_packed(config_param, arg.data);
>>> +     pcs->write(data, pcs->base + offset);
>>> +     return 0;
>>>  }
>>
>> I need to look at this more to understand how this actually relates
>> to what gets written to the register with my test case :)
>
> No problem.
>>
>>> @@ -765,12 +958,82 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
>>>       if (res < 0)
>>>               goto free_function;
>>>
>>> -     (*map)->type = PIN_MAP_TYPE_MUX_GROUP;
>>> -     (*map)->data.mux.group = np->name;
>>> -     (*map)->data.mux.function = np->name;
>>> +     m->type = PIN_MAP_TYPE_MUX_GROUP;
>>> +     m->data.mux.group = np->name;
>>> +     m->data.mux.function = np->name;
>>> +
>>> +     if (!nconfs)
>>> +             return 0;
>>> +
>>> +     conf = devm_kzalloc(pcs->dev, sizeof(*conf) * nconfs, GFP_KERNEL);
>>> +     if (!conf) {
>>> +             res = -ENOMEM;
>>> +             goto free_pingroup;
>>> +     }
>>> +     i = 0;
>>> +     if (!of_property_read_u32_array(np, "pinctrl-single,bias",
>>> +                                     value, 5)) {
>>> +             /* array: bias value, mask, disable, pull down, pull up */
>>> +             data = value[0] & value[1];
>>> +             arg.bits.shift = ffs(value[1]) - 1;
>>> +             arg.bits.mask = value[1] >> arg.bits.shift;
>>> +             if (pcs_config_match(data, value[2])) {
>>> +                     arg.bits.value = value[2] >> arg.bits.shift;
>>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_DISABLE,
>>> +                                                 arg.data);
>>> +             }
>>> +             if (pcs_config_match(data, value[3])) {
>>> +                     arg.bits.value = value[3] >> arg.bits.shift;
>>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_DOWN,
>>> +                                                 arg.data);
>>> +             }
>>> +             if (pcs_config_match(data, value[4])) {
>>> +                     arg.bits.value = value[4] >> arg.bits.shift;
>>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_UP,
>>> +                                                 arg.data);
>>> +             }
>>> +     }
>>
>> Doesn't this mean you only get one of the above working? What happens
>> if you initially need to set BIAS_DISABLE, but then the consumer driver
>> wants to set BIAS_PULL_DOWN or something?
>>
>> Maybe we need separate pinctrl-single,bias-disable, pinctrl-single,bias-pull-down
>> and pinctrl-single,bias-pull-up?
>>
> Good point. I'll separate them.
>
>>> +     if (!of_property_read_u32_array(np, "pinctrl-single,power-source",
>>> +                                     value, 2)) {
>>> +             /* array: power source value, mask */
>>> +             data = value[0] & value[1];
>>> +             arg.bits.shift = ffs(value[1]) - 1;
>>> +             arg.bits.mask = value[1] >> arg.bits.shift;
>>> +             arg.bits.value = data >> arg.bits.shift;
>>> +             conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_POWER_SOURCE, arg.data);
>>> +     }
>>> +     if (!of_property_read_u32_array(np, "pinctrl-single,input-schmitt",
>>> +                                     value, 3)) {
>>> +             /* array: input schmitt value, mask, disable */
>>> +             data = value[0] & value[1];
>>> +             arg.bits.shift = ffs(value[1]) - 1;
>>> +             arg.bits.mask = value[1] >> arg.bits.shift;
>>> +             if (pcs_config_match(data, value[2])) {
>>> +                     arg.bits.value = value[2] >> arg.bits.shift;
>>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_DISABLE,
>>> +                                                 arg.data);
>>> +             } else {
>>> +                     arg.bits.value = data >> arg.bits.shift;
>>> +                     conf[i++] = PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT,
>>> +                                                 arg.data);
>>> +             }
>>> +     }
>>
>> And I think this has a similar problem? What if the consumer driver wants
>> to set INPUT_SCHMITT with pin_config_set() and the initial value is
>> SCHMITT_DISABLE? Doesn't the consumer driver get -ENOTSUPP in that case?
>>
> OK. I'll update them.
>
>> I'll look at this more early next week and try to get my testcase working
>> with it.
>>
>> Regards,
>>
>> Tony

[PATCH v5 05/10] pinctrl: single: support pinconf generic

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@gmail.com> [121118 06:25]:
> 
> pin_config_set() sets the config value into pinmux register. The upper 16-bit
> is config argument, and the lower 16-bit is config parameter. The
> pinmux register
> is 32-bit in Marvell's silicon. I think it's 32-bit value in most silicons.

Well what if we soon have a 64-bit mux register? We should probably
reserve more than 5 bits for the shift to handle those cases as
well.
 
> And real field value could be lower 16-bit. So I have to pack mask,
> shift, config value
> & config parameter into 32-bit data. The limitation is mask, shift,
> value are all 5-bit
> value. And this is also the reason that I define pinctrl_lookup_map()
> into consumer.h.

Yes this might be problematic for 64-bit systems..

> How could consumer driver know mask & shift? It has to lookup map to
> get the real
> configuration.

I think the consumer driver should not know anything about the
mask & shift.
 
> We also need to keep pin_config_get() compatible with pin_config_set(). I define
> the config data as packed mask, shift, config value & config parameter.
> 
> The conclusion is that we have two choices.
> 1. Use packed way. It's a little complex.

I think the packed way won't work well for consumer drivers.

> 2. Change the interface of pin_config_set()/pin_config_get(). We need
> to use the new
> interface in below.
>     pin_config_set(const char *dev_name, const char *name, unsigned
> long parameter,
>                           unsigned long config);
>     pin_config_get(const char *dev_name, const char *name, unsigned
> long parameter,
>                           unsigned long *config);
>     And the config should be "value >> shift".
> 
> What's your opinion? I prefer the second method since it's easy to
> understand and maintain.
> Then I could hide pinctrl_lookup_map() in core.h.

Yes something like that makes sense to me. Except IMHO we should
use some cookie for a pin instead of dev_name + name, and have
some struct for the config. Something like this perhaps:

pin_config_get(struct pin *pin, struct pin_config *config,
		unsigned long parameter);

Regards,

Tony 

[PATCH v5 05/10] pinctrl: single: support pinconf generic

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@gmail.com> [121117 20:53]:
> On Sat, Nov 17, 2012 at 8:43 AM, Tony Lindgren <tony@atomide.com> wrote:
> 
> I agree that returning data directly would be easy for the usage in
> device driver. Let's define it as "data >> shift".
> Is it OK?

Yes returning data >> shift makes sense to me from consumer driver
point of view, so I guess then it's just what I too suggested:

data = (data >> arg.bits.shift) & arg.bits.mask;
*config = data;

Or did I miss something? 

Regards,

Tony

[PATCH v5 06/10] ARM: dts: support pinctrl single in pxa910

[Haojian Zhuang]

Add pinctrl-single support with device tree in pxa910 dkb platform.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 arch/arm/boot/dts/pxa910-dkb.dts |  204 +++++++++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/pxa910.dtsi    |   68 +++++++++++++
 2 files changed, 271 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/pxa910-dkb.dts b/arch/arm/boot/dts/pxa910-dkb.dts
index 595492a..76e9c8d 100644
--- a/arch/arm/boot/dts/pxa910-dkb.dts
+++ b/arch/arm/boot/dts/pxa910-dkb.dts
@@ -24,10 +24,212 @@
 
 	soc {
 		apb at d4000000 {
-			uart1: uart at d4017000 {
+			pmx: pinmux at d401e000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&board_pins>;
+
+				board_pins: pinmux_board_pins {
+					/* pins not owned by device driver */
+					/* w1 */
+					pinctrl-single,pins = <
+						0x0cc 0x2	/* CLK_REQ_W1 */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				uart1_pins: pinmux_uart1_pins {
+					pinctrl-single,pins = <
+						0x198 0x6	/* GPIO47_UART1_RXD */
+						0x19c 0x6	/* GPIO48_UART1_TXD */
+					>;
+					/* power source, mask */
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					/* bias, mask, disable, pull down, pull up */
+					pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>;
+					/* input schmitt, mask, disable */
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				uart2_pins: pinmux_uart2_pins {
+					pinctrl-single,pins = <
+						0x150 0x4	/* GPIO29_UART2_CTS */
+						0x154 0x4	/* GPIO30_UART2_RTS */
+						0x158 0x4	/* GPIO31_UART2_TXD */
+						0x15c 0x4	/* GPIO32_UART2_RXD */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				uart3_pins: pinmux_uart3_pins {
+					pinctrl-single,pins = <
+						0x188 0x7	/* GPIO43_UART3_RXD */
+						0x18c 0x7	/* GPIO44_UART3_TXD */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				twsi1_pins: pinmux_twsi1_pins {
+					pinctrl-single,pins = <
+						0x1b0 0x2	/* GPIO53_TWSI_SCL */
+						0x1b4 0x2	/* GPIO54_TWSI_SDA */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				nand_pins: pinmux_nand_pins {
+					pinctrl-single,pins = <
+						0x040 0x0	/* ND_IO0 */
+						0x03c 0x0	/* ND_IO1 */
+						0x038 0x0	/* ND_IO2 */
+						0x034 0x0	/* ND_IO3 */
+						0x030 0x0	/* ND_IO4 */
+						0x02c 0x0	/* ND_IO5 */
+						0x028 0x0	/* ND_IO6 */
+						0x024 0x0	/* ND_IO7 */
+						0x020 0x0	/* ND_IO8 */
+						0x01c 0x0	/* ND_IO9 */
+						0x018 0x0	/* ND_IO10 */
+						0x014 0x0	/* ND_IO11 */
+						0x010 0x0	/* ND_IO12 */
+						0x00c 0x0	/* ND_IO13 */
+						0x008 0x0	/* ND_IO14 */
+						0x004 0x0	/* ND_IO15 */
+						0x044 0x0	/* ND_nCS0 */
+						0x060 0x1	/* ND_ALE */
+						0x05c 0x0	/* ND_CLE */
+						0x054 0x1	/* ND_nWE */
+						0x058 0x1	/* ND_nRE */
+						0x068 0x0	/* ND_RDY0 */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				mmc1_ldata_pins: pinmux_mmc1_ldata_pins {
+					pinctrl-single,pins = <
+						0x0a0 0x0	/* MMC1_DATA0 */
+						0x09c 0x0	/* MMC1_DATA1 */
+						0x098 0x0	/* MMC1_DATA2 */
+						0x094 0x0	/* MMC1_DATA3 */
+					>;
+					pinctrl-single,power-source = <0x1800 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				mmc1_hdata_pins: pinmux_mmc1_hdata_pins {
+					pinctrl-single,pins = <
+						0x090 0x0	/* MMC1_DATA4 */
+						0x08c 0x0	/* MMC1_DATA5 */
+						0x088 0x0	/* MMC1_DATA6 */
+						0x084 0x0	/* MMC1_DATA7 */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				mmc1_clk_pins: pinmux_mmc1_clk_pins {
+					pinctrl-single,pins = <
+						0x0a4 0x0	/* MMC1_CMD */
+						0x0a8 0x0	/* MMC1_CLK */
+					>;
+					pinctrl-single,power-source = <0x1800 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				mmc1_cd_pins: pinmux_mmc1_cd_pins {
+					pinctrl-single,pins = <
+						0x0ac 0x0	/* MMC1_CD */
+						0x0b0 0x0	/* MMC1_WP */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				mmc2_pins: pinmux_mmc2_pins {
+					pinctrl-single,pins = <
+						0x180 0x1	/* MMC2_CMD */
+						0x184 0x1	/* MMC2_CLK */
+						0x17c 0x1	/* MMC2_DATA0 */
+						0x178 0x1	/* MMC2_DATA1 */
+						0x174 0x1	/* MMC2_DATA2 */
+						0x170 0x1	/* MMC2_DATA3 */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				ssp1_pins: pinmux_ssp1_pins {
+					pinctrl-single,pins = <
+						0x130 0x1	/* GPIO21_SSP1_SCLK */
+						0x134 0x1	/* GPIO22_SSP1_FRM */
+						0x138 0x1	/* GPIO23_SSP1_TXD */
+						0x13c 0x1	/* GPIO24_SSP1_RXD */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				keypad_pins: pinmux_keypad_pins {
+					pinctrl-single,pins = <
+						0x0dc 0x1	/* GPIO0_MKIN0 */
+						0x0e0 0x1	/* GPIO1_MKOUT0 */
+						0x0e4 0x1	/* GPIO2_MKIN1 */
+						0x0e8 0x1	/* GPIO3_MKOUT1 */
+						0x0ec 0x1	/* GPIO4_MKIN2 */
+						0x0f0 0x1	/* GPIO5_MKOUT2 */
+						0x0f4 0x1	/* GPIO6_MKIN3 */
+						0x0f8 0x1	/* GPIO7_MKOUT3 */
+						0x0fc 0x1	/* GPIO8_MKIN4 */
+						0x100 0x1	/* GPIO9_MKOUT4 */
+						0x10c 0x1	/* GPIO12_MKIN6 */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				nfc_pins: pinmux_nfc_pins {
+					pinctrl-single,pins = <
+						0x120 0x0	/* GPIO17 */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+				wlan_pins: pinmux_wlan_pins {
+					pinctrl-single,pins = <
+						0x114 0x0	/* GPIO14 */
+						0x12c 0x0	/* GPIO20 */
+						0x160 0x0	/* GPIO33 */
+						0x164 0x0	/* GPIO34 */
+						0x168 0x0	/* GPIO35 */
+						0x16c 0x0	/* GPIO36 */
+					>;
+					pinctrl-single,power-source = <0x1000 0x1800>;
+					pinctrl-single,bias = <0 0xe000 0 0xa000 0xc000>;
+					pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+				};
+			};
+			uart1: uart at d4017000 {	/* FFUART */
+				pinctrl-names = "default";
+				pinctrl-0 = <&uart1_pins>;
+				status = "okay";
+			};
+			uart2: uart at d4018000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&uart2_pins>;
+				status = "okay";
+			};
+			uart3: uart at d4036000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&uart3_pins>;
 				status = "okay";
 			};
 			twsi1: i2c at d4011000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&twsi1_pins>;
 				status = "okay";
 
 				pmic: 88pm860x at 34 {
diff --git a/arch/arm/boot/dts/pxa910.dtsi b/arch/arm/boot/dts/pxa910.dtsi
index 825aaca..c2f8b21 100644
--- a/arch/arm/boot/dts/pxa910.dtsi
+++ b/arch/arm/boot/dts/pxa910.dtsi
@@ -54,6 +54,74 @@
 			reg = <0xd4000000 0x00200000>;
 			ranges;
 
+			pmx: pinmux at d401e000 {
+				compatible = "pinconf-single";
+				reg = <0xd401e000 0x0330>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges;
+
+				pinctrl-single,register-width = <32>;
+				pinctrl-single,function-mask = <7>;
+
+				range0: range at d401e0dc {
+					/* GPIO0 ~ GPIO54 */
+					reg = <0xd401e0dc 0xdc>;
+					/* gpio base & gpio func */
+					pinctrl-single,gpio = <0 0>;
+				};
+				range1: range at d401e2f0 {
+					/* GPIO55 ~ GPIO59 */
+					reg = <0xd401e2f0 0x14>;
+					pinctrl-single,gpio = <55 1>;
+				};
+				range2: range at d401e304 {
+					/* GPIO60 ~ GPIO66 */
+					reg = <0xd401e304 0x1c>;
+					pinctrl-single,gpio = <60 0>;
+				};
+				range3: range at d401e1b8 {
+					/* GPIO67 ~ GPIO109 */
+					reg = <0xd401e1b8 0xac>;
+					pinctrl-single,gpio = <67 0>;
+				};
+				range4: range at d401e298 {
+					/* GPIO110 ~ GPIO116 */
+					reg = <0xd401e298 0x1c>;
+					pinctrl-single,gpio = <110 0>;
+				};
+				range5: range at d401e0b4 {
+					/* GPIO117 ~ GPIO120 */
+					reg = <0xd401e0b4 0x10>;
+					pinctrl-single,gpio = <117 1>;
+				};
+				range6: range at d401e32c {
+					/* GPIO121 */
+					reg = <0xd401e32c 0x04>;
+					pinctrl-single,gpio = <121 0>;
+				};
+				range7: range at d401e0c8 {
+					/* GPIO122 ~ GPIO123 */
+					reg = <0xd401e0c8 0x08>;
+					pinctrl-single,gpio = <122 1>;
+				};
+				range8: range at d401e0d0 {
+					/* GPIO124 */
+					reg = <0xd401e0d0 0x04>;
+					pinctrl-single,gpio = <124 0>;
+				};
+				range9: range at d401e0d4 {
+					/* GPIO125 */
+					reg = <0xd401e0d4 0x04>;
+					pinctrl-single,gpio = <125 1>;
+				};
+				range10: range at d401e06c {
+					/* GPIO126 ~ GPIO127 */
+					reg = <0xd401e06c 0x08>;
+					pinctrl-single,gpio = <126 0>;
+				};
+			};
+
 			timer0: timer at d4014000 {
 				compatible = "mrvl,mmp-timer";
 				reg = <0xd4014000 0x100>;
-- 
1.7.10.4

[PATCH v5 07/10] document: devicetree: bind pinconf with pin single

[Haojian Zhuang]

Add comments with pinconf & gpio range in the document of
pinctrl-single.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 .../devicetree/bindings/pinctrl/pinctrl-single.txt |   82 +++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
index 2c81e45..8f0c0dd 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
@@ -1,7 +1,9 @@
 One-register-per-pin type device tree based pinctrl driver
 
 Required properties:
-- compatible : "pinctrl-single"
+- compatible : "pinctrl-single" or "pinconf-single".
+  "pinctrl-single" means that pinconf isn't supported.
+  "pinconf-single" means that generic pinconf is supported.
 
 - reg : offset and length of the register set for the mux registers
 
@@ -14,9 +16,31 @@ Optional properties:
 - pinctrl-single,function-off : function off mode for disabled state if
   available and same for all registers; if not specified, disabling of
   pin functions is ignored
+
 - pinctrl-single,bit-per-mux : boolean to indicate that one register controls
   more than one pin
 
+- pinctrl-single,power-source : array of value that are used to configure
+  power source in the pinmux register. They're value of power source field
+  and power source mask.
+
+		/* power source, mask */
+		pinctrl-single,power-source = <0x1000 0x1800>;
+
+- pinctrl-single,bias : array of value that are used to configure the input
+  bias in the pinmux register.  They're value of bias field, bias mask,
+  bias disable value, bias pull down value & bias pull up value.
+
+		/* bias, mask, disable, pull down, pull up */
+		pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>;
+
+- pinctrl-single,input-schmitt : array of value that are used to configure
+  input schmitt in the pinmux register. They're value of input schmitt field,
+  mask, & disable value.
+
+		/* input schmitt value, mask, disable */
+		pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+
 This driver assumes that there is only one register for each pin (unless the
 pinctrl-single,bit-per-mux is set), and uses the common pinctrl bindings as
 specified in the pinctrl-bindings.txt document in this directory.
@@ -42,6 +66,24 @@ Where 0xdc is the offset from the pinctrl register base address for the
 device pinctrl register, 0x18 is the desired value, and 0xff is the sub mask to
 be used when applying this change to the register.
 
+
+Optional sub-node: In case some pins could be configured as GPIO in the pinmux
+register, those pins could be defined as a GPIO range. The sub-node should
+be defined in .dtsi files of those silicons.
+
+Required properties in sub-node:
+- reg : offset and length of the GPIO range sub-node.
+
+- pinctrl-single,gpio : array of GPIO base number in the range and the GPIO
+  function in the pinmux register.
+
+	range0: {
+		/* GPIO0 ~ GPIO54 */
+		reg = <0xd401e0dc 55>;
+		pinctrl-single,gpio = <0 0>;
+	};
+
+
 Example:
 
 /* SoC common file */
@@ -76,6 +118,26 @@ control_devconf0: pinmux at 48002274 {
 	pinctrl-single,function-mask = <0x5F>;
 };
 
+/* third controller instance for pins in gpio domain */
+pmx_gpio: pinmux at d401e000 {
+	compatible = "pinconf-single";
+	reg = <0xd401e000 0x0330>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	pinctrl-single,register-width = <32>;
+	pinctrl-single,function-mask = <7>;
+
+	range0: range at d401e0dc {
+		/* GPIO0 ~ GPIO54 */
+		reg = <0xd401e0dc 0xdc>;
+		/* gpio base & gpio func */
+		pinctrl-single,gpio = <0 0>;
+	};     
+};
+
+
 /* board specific .dts file */
 
 &pmx_core {
@@ -96,6 +158,19 @@ control_devconf0: pinmux at 48002274 {
 		>;
 	};
 
+	uart1_pins: pinmux_uart1_pins {
+		pinctrl-single,pins = <
+			0x198 0x6	/* GPIO47_UART1_RXD */
+			0x19c 0x6	/* GPIO48_UART1_TXD */
+		>;
+		/* power source, mask */
+		pinctrl-single,power-source = <0x1000 0x1800>;
+		/* bias, mask, disable, pull down, pull up */
+		pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>;
+		/* input schmitt, mask, disable */
+		pinctrl-single,input-schmitt = <0x40 0x70 0x40>;
+	};
+
 	/* map uart2 pins */
 	uart2_pins: pinmux_uart2_pins {
 		pinctrl-single,pins = <
@@ -122,6 +197,11 @@ control_devconf0: pinmux@48002274 {
 
 };
 
+&uart1 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart1_pins>;
+};
+
 &uart2 {
        pinctrl-names = "default";
        pinctrl-0 = <&uart2_pins>;
-- 
1.7.10.4

[PATCH v5 07/10] document: devicetree: bind pinconf with pin single

[Stephen Warren]

On 11/15/2012 01:36 AM, Haojian Zhuang wrote:
> Add comments with pinconf & gpio range in the document of
> pinctrl-single.

> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt

> +- pinctrl-single,power-source : array of value that are used to configure
> +  power source in the pinmux register. They're value of power source field
> +  and power source mask.
> +
> +		/* power source, mask */
> +		pinctrl-single,power-source = <0x1000 0x1800>;
> +
> +- pinctrl-single,bias : array of value that are used to configure the input
> +  bias in the pinmux register.  They're value of bias field, bias mask,
> +  bias disable value, bias pull down value & bias pull up value.
> +
> +		/* bias, mask, disable, pull down, pull up */
> +		pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>;
> +
> +- pinctrl-single,input-schmitt : array of value that are used to configure
> +  input schmitt in the pinmux register. They're value of input schmitt field,
> +  mask, & disable value.
> +
> +		/* input schmitt value, mask, disable */
> +		pinctrl-single,input-schmitt = <0x40 0x70 0x40>;

For all 3 of those, the first cell in the property is "value". What does
"value" represent? Surely the properties in the pinmux controller node
itself just specify how to write to the field (so should specify field
mask, and perhaps values for specific enums). However, any "value" to be
actually written to the field should be specified by the pin
configuration (sub) nodes, not the global top-level properties.

[PATCH v5 08/10] tty: pxa: configure pin

[Haojian Zhuang]

Configure pins by pinctrl driver.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/tty/serial/pxa.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 9033fc6..02dc771 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -37,6 +37,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
@@ -809,6 +810,7 @@ static int serial_pxa_probe_dt(struct platform_device *pdev,
 			       struct uart_pxa_port *sport)
 {
 	struct device_node *np = pdev->dev.of_node;
+	struct pinctrl *pinctrl;
 	int ret;
 
 	if (!np)
@@ -819,6 +821,10 @@ static int serial_pxa_probe_dt(struct platform_device *pdev,
 		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
 		return ret;
 	}
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		return -EPROBE_DEFER;
+
 	sport->port.line = ret;
 	return 0;
 }
@@ -857,7 +863,7 @@ static int serial_pxa_probe(struct platform_device *dev)
 	ret = serial_pxa_probe_dt(dev, sport);
 	if (ret > 0)
 		sport->port.line = dev->id;
-	else if (ret < 0)
+	if (ret < 0)
 		goto err_clk;
 	snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);
 
-- 
1.7.10.4

[PATCH v5 09/10] i2c: pxa: use devm_kzalloc

[Haojian Zhuang]

Use devm_kzalloc & add checking in probe() function.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Ben Dooks <ben-linux@fluff.org>
---
 drivers/i2c/busses/i2c-pxa.c |   26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..7c8b5d0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1078,6 +1078,8 @@ static int i2c_pxa_probe_pdata(struct platform_device *pdev,
 	struct i2c_pxa_platform_data *plat = pdev->dev.platform_data;
 	const struct platform_device_id *id = platform_get_device_id(pdev);
 
+	if (!id)
+		return -EINVAL;
 	*i2c_types = id->driver_data;
 	if (plat) {
 		i2c->use_pio = plat->use_pio;
@@ -1094,29 +1096,23 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	struct resource *res = NULL;
 	int ret, irq;
 
-	i2c = kzalloc(sizeof(struct pxa_i2c), GFP_KERNEL);
-	if (!i2c) {
-		ret = -ENOMEM;
-		goto emalloc;
-	}
+	i2c = devm_kzalloc(&dev->dev, sizeof(struct pxa_i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
 
 	ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type);
 	if (ret > 0)
 		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
 	if (ret < 0)
-		goto eclk;
+		return ret;
 
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(dev, 0);
-	if (res == NULL || irq < 0) {
-		ret = -ENODEV;
-		goto eclk;
-	}
+	if (res == NULL || irq < 0)
+		return -ENODEV;
 
-	if (!request_mem_region(res->start, resource_size(res), res->name)) {
-		ret = -ENOMEM;
-		goto eclk;
-	}
+	if (!request_mem_region(res->start, resource_size(res), res->name))
+		return -ENOMEM;
 
 	i2c->adap.owner   = THIS_MODULE;
 	i2c->adap.retries = 5;
@@ -1209,8 +1205,6 @@ ereqirq:
 eremap:
 	clk_put(i2c->clk);
 eclk:
-	kfree(i2c);
-emalloc:
 	release_mem_region(res->start, resource_size(res));
 	return ret;
 }
-- 
1.7.10.4

[PATCH v5 10/10] i2c: pxa: configure pinmux

[Haojian Zhuang]

Configure pins by pinctrl driver.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Ben Dooks <ben-linux@fluff.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/i2c/busses/i2c-pxa.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 7c8b5d0..11e4a30 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -32,6 +32,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_i2c.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
 #include <linux/clk.h>
@@ -1051,6 +1052,7 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 			    enum pxa_i2c_types *i2c_types)
 {
 	struct device_node *np = pdev->dev.of_node;
+	struct pinctrl *pinctrl;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
 	int ret;
@@ -1063,6 +1065,9 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 		return ret;
 	}
 	pdev->id = ret;
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		return -EPROBE_DEFER;
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
-- 
1.7.10.4