All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 00/12] support pinconf in pinctrl single
@ 2013-02-02 17:25 Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property Haojian Zhuang
                   ` (11 more replies)
  0 siblings, 12 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Changelog:
v8:
1. Always to check return value of pinctrl_request_gpio().
2. Add pinctrl_overlapped_gpio_range() to handle the case that special
GPIO pins don't have back-end pinctrl interface.
3. Add document of pinctrl single.
4. Replace power source property by drive strength property in pinctrl
single driver since current should be drive strength property.
5. Rebase all the patches.

v7:
1. Discard the method of adding gpio range from pinctrl-single driver.
Use gpiolib driver to support gpio range from DTS instead.
2. Add gpio request function to claim pin in gpio pl061 driver.
3. Adjust the initcall level in gpio pl061 driver.
4. Allocate gpio number from lowest gpio number to highest. The original
implementation is inverted. It's hard to use since it inverted the sequence
of gpio number.
5. Remove the support of pxa910 temporarily since gpio pxa driver need to
be updated for supporting this solution.

v6:
1. Two configuration array will be created for each pin group.
This first array is stored in pcs_function structure. The 32-bit
configruation argument is stored in this array. Driver stores
data while parsing DTS file, and loads these config array if
function selector is indicated.
The second array is stored in pinctrl_map structure. Driver won't
use it directly. So we could avoid to append lookup pinctrl map
method that is introduced in v5.

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.

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-05  0:23   ` Tony Lindgren
                     ` (2 more replies)
  2013-02-02 17:25 ` [PATCH v8 02/12] gpio: fix wrong checking condition for gpio range Haojian Zhuang
                   ` (10 subsequent siblings)
  11 siblings, 3 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Add gpio offset into "gpio-range-cells" property. It's used to support
sparse pinctrl range in gpio chip.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Documentation/devicetree/bindings/gpio/gpio.txt |    6 +++---
 arch/arm/boot/dts/spear1310.dtsi                |    4 ++--
 arch/arm/boot/dts/spear1340.dtsi                |    4 ++--
 arch/arm/boot/dts/spear310.dtsi                 |    4 ++--
 arch/arm/boot/dts/spear320.dtsi                 |    4 ++--
 drivers/gpio/gpiolib-of.c                       |   15 ++-------------
 6 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index a336287..d933af3 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -98,7 +98,7 @@ announce the pinrange to the pin ctrl subsystem. For example,
 		compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank";
 		reg = <0x1460 0x18>;
 		gpio-controller;
-		gpio-ranges = <&pinctrl1 20 10>, <&pinctrl2 50 20>;
+		gpio-ranges = <&pinctrl1 0 20 10>, <&pinctrl2 10 50 20>;
 
     }
 
@@ -107,8 +107,8 @@ where,
 
    Next values specify the base pin and number of pins for the range
    handled by 'qe_pio_e' gpio. In the given example from base pin 20 to
-   pin 29 under pinctrl1 and pin 50 to pin 69 under pinctrl2 is handled
-   by this gpio controller.
+   pin 29 under pinctrl1 with gpio offset 0 and pin 50 to pin 69 under
+   pinctrl2 with gpio offset 10 is handled by this gpio controller.
 
 The pinctrl node must have "#gpio-range-cells" property to show number of
 arguments to pass with phandle from gpio controllers node.
diff --git a/arch/arm/boot/dts/spear1310.dtsi b/arch/arm/boot/dts/spear1310.dtsi
index 1513c19..122ae94 100644
--- a/arch/arm/boot/dts/spear1310.dtsi
+++ b/arch/arm/boot/dts/spear1310.dtsi
@@ -89,7 +89,7 @@
 		pinmux: pinmux at e0700000 {
 			compatible = "st,spear1310-pinmux";
 			reg = <0xe0700000 0x1000>;
-			#gpio-range-cells = <2>;
+			#gpio-range-cells = <3>;
 		};
 
 		apb {
@@ -212,7 +212,7 @@
 				interrupt-controller;
 				gpio-controller;
 				#gpio-cells = <2>;
-				gpio-ranges = <&pinmux 0 246>;
+				gpio-ranges = <&pinmux 0 0 246>;
 				status = "disabled";
 
 				st-plgpio,ngpio = <246>;
diff --git a/arch/arm/boot/dts/spear1340.dtsi b/arch/arm/boot/dts/spear1340.dtsi
index b2d41b7..7ec1eb8 100644
--- a/arch/arm/boot/dts/spear1340.dtsi
+++ b/arch/arm/boot/dts/spear1340.dtsi
@@ -63,7 +63,7 @@
 		pinmux: pinmux at e0700000 {
 			compatible = "st,spear1340-pinmux";
 			reg = <0xe0700000 0x1000>;
-			#gpio-range-cells = <2>;
+			#gpio-range-cells = <3>;
 		};
 
 		pwm: pwm at e0180000 {
@@ -146,7 +146,7 @@
 				interrupt-controller;
 				gpio-controller;
 				#gpio-cells = <2>;
-				gpio-ranges = <&pinmux 0 252>;
+				gpio-ranges = <&pinmux 0 0 252>;
 				status = "disabled";
 
 				st-plgpio,ngpio = <250>;
diff --git a/arch/arm/boot/dts/spear310.dtsi b/arch/arm/boot/dts/spear310.dtsi
index ab45b8c..9537208 100644
--- a/arch/arm/boot/dts/spear310.dtsi
+++ b/arch/arm/boot/dts/spear310.dtsi
@@ -25,7 +25,7 @@
 		pinmux: pinmux at b4000000 {
 			compatible = "st,spear310-pinmux";
 			reg = <0xb4000000 0x1000>;
-			#gpio-range-cells = <2>;
+			#gpio-range-cells = <3>;
 		};
 
 		fsmc: flash at 44000000 {
@@ -102,7 +102,7 @@
 				interrupt-controller;
 				gpio-controller;
 				#gpio-cells = <2>;
-				gpio-ranges = <&pinmux 0 102>;
+				gpio-ranges = <&pinmux 0 0 102>;
 				status = "disabled";
 
 				st-plgpio,ngpio = <102>;
diff --git a/arch/arm/boot/dts/spear320.dtsi b/arch/arm/boot/dts/spear320.dtsi
index caa5520..ffea342 100644
--- a/arch/arm/boot/dts/spear320.dtsi
+++ b/arch/arm/boot/dts/spear320.dtsi
@@ -24,7 +24,7 @@
 		pinmux: pinmux at b3000000 {
 			compatible = "st,spear320-pinmux";
 			reg = <0xb3000000 0x1000>;
-			#gpio-range-cells = <2>;
+			#gpio-range-cells = <3>;
 		};
 
 		clcd at 90000000 {
@@ -130,7 +130,7 @@
 				interrupt-controller;
 				gpio-controller;
 				#gpio-cells = <2>;
-				gpio-ranges = <&pinmux 0 102>;
+				gpio-ranges = <&pinmux 0 0 102>;
 				status = "disabled";
 
 				st-plgpio,ngpio = <102>;
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 25b1dbe..380f84e 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -238,22 +238,11 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 		if (!pctldev)
 			break;
 
-		/*
-		 * This assumes that the n GPIO pins are consecutive in the
-		 * GPIO number space, and that the pins are also consecutive
-		 * in their local number space. Currently it is not possible
-		 * to add different ranges for one and the same GPIO chip,
-		 * as the code assumes that we have one consecutive range
-		 * on both, mapping 1-to-1.
-		 *
-		 * TODO: make the OF bindings handle multiple sparse ranges
-		 * on the same GPIO chip.
-		 */
 		ret = gpiochip_add_pin_range(chip,
 					     pinctrl_dev_get_devname(pctldev),
-					     0, /* offset in gpiochip */
 					     pinspec.args[0],
-					     pinspec.args[1]);
+					     pinspec.args[1],
+					     pinspec.args[2]);
 
 		if (ret)
 			break;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 02/12] gpio: fix wrong checking condition for gpio range
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-05 17:02   ` Linus Walleij
  2013-02-02 17:25 ` [PATCH v8 03/12] gpio: find gpio base by ascend order Haojian Zhuang
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

If index++ calculates from 0, the checking condition of "while
(index++)" fails & it doesn't check any more. It doesn't follow
the loop that used at here.

Replace it by endless loop at here. Then it keeps parsing
"gpio-ranges" property until it ends.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/gpio/gpiolib-of.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 380f84e..dae24c0 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -228,7 +228,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 	if (!np)
 		return;
 
-	do {
+	for (;; index++) {
 		ret = of_parse_phandle_with_args(np, "gpio-ranges",
 				"#gpio-range-cells", index, &pinspec);
 		if (ret)
@@ -246,8 +246,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 
 		if (ret)
 			break;
-
-	} while (index++);
+	}
 }
 
 #else
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 03/12] gpio: find gpio base by ascend order
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 02/12] gpio: fix wrong checking condition for gpio range Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-05 17:14   ` Linus Walleij
  2013-02-02 17:25 ` [PATCH v8 04/12] gpio: pl061: allocate irq dynamically Haojian Zhuang
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

gpiochip_find_base() always tries to find valid gpio with descend order.
It's inconvient if gpio information is passing from DTS. Now try to find
valid gpio with ascend order.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/gpio/gpiolib.c |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 199fca1..8af57e7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -128,20 +128,21 @@ static int gpiochip_find_base(int ngpio)
 	int spare = 0;
 	int base = -ENOSPC;
 
-	for (i = ARCH_NR_GPIOS - 1; i >= 0 ; i--) {
+	for (i = 0, base = 0; i < ARCH_NR_GPIOS; i++) {
 		struct gpio_desc *desc = &gpio_desc[i];
 		struct gpio_chip *chip = desc->chip;
 
-		if (!chip && !test_bit(FLAG_RESERVED, &desc->flags)) {
+		if (chip) {
+			spare = 0;
+			i += chip->ngpio - 1;
+			base = i + 1;
+		} else if (test_bit(FLAG_RESERVED, &desc->flags)) {
+			spare = 0;
+			base = i + 1;
+		} else {
 			spare++;
-			if (spare == ngpio) {
-				base = i;
+			if (spare == ngpio)
 				break;
-			}
-		} else {
-			spare = 0;
-			if (chip)
-				i -= chip->ngpio - 1;
 		}
 	}
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 04/12] gpio: pl061: allocate irq dynamically
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (2 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 03/12] gpio: find gpio base by ascend order Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 05/12] pinctrl: verify whether gpio chip overlapps range Haojian Zhuang
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

In original implementation, irq base is always specified in platform
data. If it's not specified, pl061 gpio driver can't pass the probe()
function since irq base is missing. While moving to device tree, everything
should be parsed from DTS file. So allocate irq dynamically for irq
base.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/gpio/gpio-pl061.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index b820869..fd26f63 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -15,6 +15,7 @@
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/bitops.h>
 #include <linux/workqueue.h>
 #include <linux/gpio.h>
@@ -211,6 +212,10 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
 			       IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
 }
 
+static const struct irq_domain_ops pl061_domain_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+};
+
 static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
 {
 	struct device *dev = &adev->dev;
@@ -225,10 +230,14 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
 	if (pdata) {
 		chip->gc.base = pdata->gpio_base;
 		chip->irq_base = pdata->irq_base;
-	} else if (adev->dev.of_node) {
+	} else {
 		chip->gc.base = -1;
-		chip->irq_base = 0;
-	} else
+		chip->irq_base = irq_alloc_descs(-1, 0, PL061_GPIO_NR, 0);
+		if (chip->irq_base < 0)
+			return chip->irq_base;
+	}
+	if (!irq_domain_add_legacy(adev->dev.of_node, PL061_GPIO_NR,
+				   chip->irq_base, 0, &pl061_domain_ops, chip))
 		return -ENODEV;
 
 	if (!devm_request_mem_region(dev, adev->res.start,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 05/12] pinctrl: verify whether gpio chip overlapps range
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (3 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 04/12] gpio: pl061: allocate irq dynamically Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 06/12] gpio: pl061: bind pinctrl by gpio request Haojian Zhuang
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

pinctrl_get_device_gpio_range() only checks whether a certain GPIO pin
is in gpio range. But maybe some GPIO pins don't have back-end pinctrl
interface, it means that these pins are always configured as GPIO
function.

Append pinctrl_overlapped_gpio_range() that is used to check whether
the pins of GPIO chip are overlapped with pins in the GPIO range. This
function will be called after pinctrl_get_device_gpio_range() fails.

If overlapped GPIO pins are found, it means that pinctrl device is already
launched and a certain GPIO pin don't have back-end pinctrl interface.
Then pinctrl_request_gpio() shouldn't return -EPROBE_DEFER in this case.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/pinctrl/core.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 5a2fe9a..f5814cf 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -27,6 +27,7 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/machine.h>
+#include <asm-generic/gpio.h>
 #include "core.h"
 #include "devicetree.h"
 #include "pinmux.h"
@@ -293,6 +294,39 @@ pinctrl_match_gpio_range(struct pinctrl_dev *pctldev, unsigned gpio)
 }
 
 /**
+ * pinctrl_overlapped_gpio_range() - check if the GPIO chip of a certain GPIO
+ * pin is overlapped with gpio range.
+ * @gpio: gpio pin to check taken from the global GPIO pin space
+ *
+ * This function is complement of pinctrl_match_gpio_range(). If the return
+ * value of pinctrl_match_gpio_range() is NULL, this function could be used
+ * to check whether pinctrl device is ready or not. Maybe some GPIO pins
+ * don't have back-end pinctrl interface.
+ * If the return value is true, it means that pinctrl device is ready & the
+ * certain GPIO pin doesn't have back-end pinctrl device. If the return value
+ * is false, it means that pinctrl device may not be ready.
+ */
+static bool pinctrl_overlapped_gpio_range(unsigned gpio)
+{
+	struct pinctrl_dev *pctldev;
+	struct pinctrl_gpio_range *range = NULL;
+	struct gpio_chip *chip = gpio_to_chip(gpio);
+
+	/* Loop over the pin controllers */
+	list_for_each_entry(pctldev, &pinctrldev_list, node) {
+		/* Loop over the ranges */
+		list_for_each_entry(range, &pctldev->gpio_ranges, node) {
+			/* Check if any gpio range overlapped with gpio chip */
+			if (range->base + range->npins - 1 < chip->base ||
+			    range->base > chip->base + chip->ngpio - 1)
+				continue;
+			return true;
+		}
+	}
+	return false;
+}
+
+/**
  * pinctrl_get_device_gpio_range() - find device for GPIO range
  * @gpio: the pin to locate the pin controller for
  * @outdev: the pin control device if found
@@ -459,6 +493,8 @@ int pinctrl_request_gpio(unsigned gpio)
 
 	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
 	if (ret) {
+		if (pinctrl_overlapped_gpio_range(gpio))
+			ret = 0;
 		mutex_unlock(&pinctrl_mutex);
 		return ret;
 	}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 06/12] gpio: pl061: bind pinctrl by gpio request
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (4 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 05/12] pinctrl: verify whether gpio chip overlapps range Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 07/12] pinctrl: single: create new gpio function range Haojian Zhuang
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Add the pl061_gpio_request() to request pinctrl. Create the logic
between pl061 gpio driver and pinctrl (pinctrl-single) driver.

While a gpio pin is requested, it will request pinctrl driver to
set that pin with gpio function mode. So pinctrl driver should
append .gpio_request_enable() in pinmux_ops.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/gpio/gpio-pl061.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index fd26f63..292620c 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -23,6 +23,7 @@
 #include <linux/amba/bus.h>
 #include <linux/amba/pl061.h>
 #include <linux/slab.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/pm.h>
 #include <asm/mach/irq.h>
 
@@ -61,6 +62,17 @@ struct pl061_gpio {
 #endif
 };
 
+static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	/*
+	 * Map back to global GPIO space and request muxing, the direction
+	 * parameter does not matter for this controller.
+	 */
+	int gpio = chip->base + offset;
+
+	return pinctrl_request_gpio(gpio);
+}
+
 static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
 {
 	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
@@ -251,6 +263,7 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
 
 	spin_lock_init(&chip->lock);
 
+	chip->gc.request = pl061_gpio_request;
 	chip->gc.direction_input = pl061_direction_input;
 	chip->gc.direction_output = pl061_direction_output;
 	chip->gc.get = pl061_get_value;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 07/12] pinctrl: single: create new gpio function range
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (5 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 06/12] gpio: pl061: bind pinctrl by gpio request Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 08/12] pinctrl: generic: dump pin configuration Haojian Zhuang
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Since gpio driver could create gpio range in DTS, it could invokes
pinctrl_request_gpio(). In the pinctrl-single driver, it needs to
configure pins with gpio function mode.

A new gpio function range should be created in DTS file in below.

pinctrl-single,gpio-range = <phandle pin_offset nr_pins gpio_func>;

range: gpio-range {
	#pinctrl-single,gpio-range-cells = <3>;
};

The difference between gpio-ranges property in gpio driver and
pinctrl-single,gpio-range property in pinctrl-single driver.

1. gpio-ranges = <phandle gpio_offset_in_chip pin_offset nr_pins>
	gpio-ranges = < &pmx0 0 89 1 &pmx0 1 89 1 &pmx0 2 90 1
			&pmx0 3 90 1 &pmx0 4 91 1 &pmx0 5 92 1>;

2. gpio driver could get pin offset from gpio-ranges property.
   pinctrl-single driver could get gpio function mode from gpio_func
   that is stored in @gpiofuncs list in struct pcs_device.
   This new pinctrl-single,gpio-range is used as complement for
   gpio-ranges property in gpio driver.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/pinctrl/pinctrl-single.c |   73 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 5c32e88..8b9dd95 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -77,6 +77,20 @@ struct pcs_function {
 };
 
 /**
+ * struct pcs_gpiofunc_range - pin ranges with same mux value of gpio function
+ * @offset:	offset base of pins
+ * @npins:	number pins with the same mux value of gpio function
+ * @gpiofunc:	mux value of gpio function
+ * @node:	list node
+ */
+struct pcs_gpiofunc_range {
+	unsigned offset;
+	unsigned npins;
+	unsigned gpiofunc;
+	struct list_head node;
+};
+
+/**
  * struct pcs_data - wrapper for data needed by pinctrl framework
  * @pa:		pindesc array
  * @cur:	index to current element
@@ -123,6 +137,7 @@ struct pcs_name {
  * @ftree:	function index radix tree
  * @pingroups:	list of pingroups
  * @functions:	list of functions
+ * @gpiofuncs:	list of gpio functions
  * @ngroups:	number of pingroups
  * @nfuncs:	number of functions
  * @desc:	pin controller descriptor
@@ -148,6 +163,7 @@ struct pcs_device {
 	struct radix_tree_root ftree;
 	struct list_head pingroups;
 	struct list_head functions;
+	struct list_head gpiofuncs;
 	unsigned ngroups;
 	unsigned nfuncs;
 	struct pinctrl_desc desc;
@@ -403,9 +419,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_gpiofunc_range *frange = NULL;
+	struct list_head *pos, *tmp;
+	int mux_bytes = 0;
+	unsigned data;
+
+	list_for_each_safe(pos, tmp, &pcs->gpiofuncs) {
+		frange = list_entry(pos, struct pcs_gpiofunc_range, node);
+		if (pin >= frange->offset + frange->npins
+			|| pin < frange->offset)
+			continue;
+		mux_bytes = pcs->width / BITS_PER_BYTE;
+		data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
+		data |= frange->gpiofunc;
+		pcs->write(data, pcs->base + pin * mux_bytes);
+		break;
+	}
+	return 0;
 }
 
 static struct pinmux_ops pcs_pinmux_ops = {
@@ -879,6 +912,37 @@ static void pcs_free_resources(struct pcs_device *pcs)
 
 static struct of_device_id pcs_of_match[];
 
+static int pcs_add_gpio_func(struct device_node *node, struct pcs_device *pcs)
+{
+	const char *propname = "pinctrl-single,gpio-range";
+	const char *cellname = "#pinctrl-single,gpio-range-cells";
+	struct of_phandle_args gpiospec;
+	struct pcs_gpiofunc_range *range;
+	int ret, i;
+
+	for (i = 0; ; i++) {
+		ret = of_parse_phandle_with_args(node, propname, cellname,
+						 i, &gpiospec);
+		/* Do not treat it as error. Only treat it as end condition. */
+		if (ret) {
+			ret = 0;
+			break;
+		}
+		range = devm_kzalloc(pcs->dev, sizeof(*range), GFP_KERNEL);
+		if (!range) {
+			ret = -ENOMEM;
+			break;
+		}
+		range->offset = gpiospec.args[0];
+		range->npins = gpiospec.args[1];
+		range->gpiofunc = gpiospec.args[2];
+		mutex_lock(&pcs->mutex);
+		list_add_tail(&range->node, &pcs->gpiofuncs);
+		mutex_unlock(&pcs->mutex);
+	}
+	return ret;
+}
+
 static int pcs_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -900,6 +964,7 @@ static int pcs_probe(struct platform_device *pdev)
 	mutex_init(&pcs->mutex);
 	INIT_LIST_HEAD(&pcs->pingroups);
 	INIT_LIST_HEAD(&pcs->functions);
+	INIT_LIST_HEAD(&pcs->gpiofuncs);
 
 	PCS_GET_PROP_U32("pinctrl-single,register-width", &pcs->width,
 			 "register width not specified\n");
@@ -975,6 +1040,10 @@ static int pcs_probe(struct platform_device *pdev)
 		goto free;
 	}
 
+	ret = pcs_add_gpio_func(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

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 08/12] pinctrl: generic: dump pin configuration
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (6 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 07/12] pinctrl: single: create new gpio function range Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-05  0:35   ` Tony Lindgren
  2013-02-02 17:25 ` [PATCH v8 09/12] pinctrl: single: set function mask as optional Haojian Zhuang
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Add the support of dumping pin configuration.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/pinctrl/pinconf-generic.c |   12 ++++++++++++
 drivers/pinctrl/pinconf.h         |    8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index e5948f8..66e4ae5 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -120,4 +120,16 @@ void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
 	}
 }
 
+void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
+				 struct seq_file *s, unsigned long config)
+{
+	int i;
+
+	for(i = 0; i < ARRAY_SIZE(conf_items); i++) {
+		if (pinconf_to_config_param(config) != conf_items[i].param)
+			continue;
+		seq_printf(s, "%s: 0x%x", conf_items[i].display,
+			   pinconf_to_config_argument(config));
+	}
+}
 #endif
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index e3ed8cb..8023421 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -98,6 +98,8 @@ void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
 void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
 			      struct seq_file *s, const char *gname);
 
+void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
+				 struct seq_file *s, unsigned long config);
 #else
 
 static inline void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
@@ -114,4 +116,10 @@ static inline void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
 	return;
 }
 
+static void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
+					struct seq_file *s,
+					unsigned long config)
+{
+	return;
+}
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 09/12] pinctrl: single: set function mask as optional
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (7 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 08/12] pinctrl: generic: dump pin configuration Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 10/12] pinctrl: single: support generic pinconf Haojian Zhuang
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Since Hisilicon's pin controller is divided into two parts. One is the
function mux, and the other is pin configuration. These two parts are
in the different memory regions. So make pinctrl-single,function-mask
as optional property. Then we can define pingroups without valid
function mux that is only used for pin configuration.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/pinctrl/pinctrl-single.c |   26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 8b9dd95..fe8f321 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -350,6 +350,9 @@ static int pcs_enable(struct pinctrl_dev *pctldev, unsigned fselector,
 	int i;
 
 	pcs = pinctrl_dev_get_drvdata(pctldev);
+	/* If function mask is null, needn't enable it. */
+	if (!pcs->fmask)
+		return 0;
 	func = radix_tree_lookup(&pcs->ftree, fselector);
 	if (!func)
 		return -EINVAL;
@@ -384,6 +387,10 @@ static void pcs_disable(struct pinctrl_dev *pctldev, unsigned fselector,
 	int i;
 
 	pcs = pinctrl_dev_get_drvdata(pctldev);
+	/* If function mask is null, needn't disable it. */
+	if (!pcs->fmask)
+		return;
+
 	func = radix_tree_lookup(&pcs->ftree, fselector);
 	if (!func) {
 		dev_err(pcs->dev, "%s could not find function%i\n",
@@ -427,6 +434,10 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
 	int mux_bytes = 0;
 	unsigned data;
 
+	/* If function mask is null, return directly. */
+	if (!pcs->fmask)
+		return -ENOTSUPP;
+
 	list_for_each_safe(pos, tmp, &pcs->gpiofuncs) {
 		frange = list_entry(pos, struct pcs_gpiofunc_range, node);
 		if (pin >= frange->offset + frange->npins
@@ -969,10 +980,17 @@ static int pcs_probe(struct platform_device *pdev)
 	PCS_GET_PROP_U32("pinctrl-single,register-width", &pcs->width,
 			 "register width not specified\n");
 
-	PCS_GET_PROP_U32("pinctrl-single,function-mask", &pcs->fmask,
-			 "function register mask not specified\n");
-	pcs->fshift = ffs(pcs->fmask) - 1;
-	pcs->fmax = pcs->fmask >> pcs->fshift;
+	ret = of_property_read_u32(np, "pinctrl-single,function-mask",
+				   &pcs->fmask);
+	if (!ret) {
+		pcs->fshift = ffs(pcs->fmask) - 1;
+		pcs->fmax = pcs->fmask >> pcs->fshift;
+	} else {
+		/* If mask property doesn't exist, function mux is invalid. */
+		pcs->fmask = 0;
+		pcs->fshift = 0;
+		pcs->fmax = 0;
+	}
 
 	ret = of_property_read_u32(np, "pinctrl-single,function-off",
 					&pcs->foff);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 10/12] pinctrl: single: support generic pinconf
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (8 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 09/12] pinctrl: single: set function mask as optional Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-05  0:46   ` Tony Lindgren
  2013-02-02 17:25 ` [PATCH v8 11/12] ARM: hs: enable hi4511 with device tree Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 12/12] document: devicetree: bind pinconf with pin single Haojian Zhuang
  11 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Support the operation of generic pinconf. The supported config arguments
are INPUT_SCHMITT, INPUT_SCHMITT_DISABLE, POWER_SOURCE, BIAS_DISABLE,
BIAS_PULLUP, BIAS_PULLDOWN, SLEW_RATE.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/pinctrl/Kconfig          |    1 +
 drivers/pinctrl/pinctrl-single.c |  339 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 333 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 9db3497..066581c 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -154,6 +154,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 fe8f321..1d9f3cc 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -22,8 +22,10 @@
 
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
 
 #include "core.h"
+#include "pinconf.h"
 
 #define DRIVER_NAME			"pinctrl-single"
 #define PCS_MUX_PINS_NAME		"pinctrl-single,pins"
@@ -59,6 +61,31 @@ struct pcs_func_vals {
 };
 
 /**
+ * struct pcs_conf_vals - pinconf parameter, pinconf register offset
+ * and value, match, mask
+ * @param:	config parameter
+ * @val:	register value
+ * @match:	register match value
+ * @mask:	mask of register value
+ */
+struct pcs_conf_vals {
+	enum pin_config_param param;
+	unsigned val;
+	unsigned match;
+	unsigned mask;
+};
+
+/**
+ * struct pcs_conf_type - pinconf property name, pinconf param pair
+ * @name:	property name in DTS file
+ * @param:	config parameter
+ */
+struct pcs_conf_type {
+	const char *name;
+	enum pin_config_param param;
+};
+
+/**
  * struct pcs_function - pinctrl function
  * @name:	pinctrl function name
  * @vals:	register and vals array
@@ -73,6 +100,8 @@ struct pcs_function {
 	unsigned nvals;
 	const char **pgnames;
 	int npgnames;
+	struct pcs_conf_vals *conf;
+	int nconfs;
 	struct list_head node;
 };
 
@@ -131,6 +160,7 @@ struct pcs_name {
  * @fshift:	function register shift
  * @foff:	value to turn mux off
  * @fmax:	max number of functions in fmask
+ * @is_pinconf:	whether supports pinconf
  * @names:	array of register names for pins
  * @pins:	physical pins on the SoC
  * @pgtree:	pingroup index radix tree
@@ -157,6 +187,7 @@ struct pcs_device {
 	unsigned foff;
 	unsigned fmax;
 	bool bits_per_mux;
+	bool is_pinconf;
 	struct pcs_name *names;
 	struct pcs_data pins;
 	struct radix_tree_root pgtree;
@@ -342,6 +373,28 @@ static int pcs_get_function_groups(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
+static int pcs_get_function(struct pinctrl_dev *pctldev, unsigned pin,
+			    struct pcs_function **func)
+{
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	struct pin_desc *pdesc = pin_desc_get(pctldev, pin);
+	const struct pinctrl_setting_mux *setting;
+	unsigned fselector;
+
+	/* If pin is not described in DTS & enabled, mux_setting is NULL. */
+	setting = pdesc->mux_setting;
+	if (!setting)
+		return -ENOTSUPP;
+	fselector = setting->func;
+	*func = radix_tree_lookup(&pcs->ftree, fselector);
+	if (!(*func)) {
+		dev_err(pcs->dev, "%s could not find function%i\n",
+			__func__, fselector);
+		return -ENOTSUPP;
+	}
+	return 0;
+}
+
 static int pcs_enable(struct pinctrl_dev *pctldev, unsigned fselector,
 	unsigned group)
 {
@@ -464,29 +517,137 @@ static struct pinmux_ops pcs_pinmux_ops = {
 static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
 				unsigned pin, unsigned long *config)
 {
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	struct pcs_function *func;
+	unsigned offset = 0, data = 0, i, j, ret;
+
+	ret = pcs_get_function(pctldev, pin, &func);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < func->nconfs; i++) {
+		if (pinconf_to_config_param(*config) != func->conf[i].param)
+			continue;
+		offset = pin * (pcs->width / BITS_PER_BYTE);
+		data = pcs->read(pcs->base + offset) & func->conf[i].mask;
+		switch (func->conf[i].param) {
+		/* 3 parameters */
+		case PIN_CONFIG_BIAS_DISABLE:
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+		case PIN_CONFIG_BIAS_PULL_UP:
+		case PIN_CONFIG_INPUT_SCHMITT_DISABLE:
+			if (data != func->conf[i].match)
+				return -ENOTSUPP;
+			*config = data;
+			break;
+		case PIN_CONFIG_INPUT_SCHMITT:
+			/* either INPUT_SCHMITT or DISABLE */
+			for (j = 0; j < func->nconfs; j++) {
+				switch (func->conf[j].param) {
+				case PIN_CONFIG_INPUT_SCHMITT_DISABLE:
+					if (data == func->conf[j].match)
+						return -ENOTSUPP;
+					break;
+				default:
+					break;
+				}
+			}
+			*config = data;
+			break;
+		case PIN_CONFIG_DRIVE_STRENGTH:
+		case PIN_CONFIG_SLEW_RATE:
+		default:
+			*config = data;
+			break;
+		}
+		return 0;
+	}
 	return -ENOTSUPP;
 }
 
 static int pcs_pinconf_set(struct pinctrl_dev *pctldev,
 				unsigned pin, unsigned long config)
 {
+	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
+	struct pcs_function *func;
+	unsigned offset = 0, shift = 0, arg = 0, i, data, ret;
+
+	ret = pcs_get_function(pctldev, pin, &func);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < func->nconfs; i++) {
+		if (pinconf_to_config_param(config) == func->conf[i].param) {
+			offset = pin * (pcs->width / BITS_PER_BYTE);
+			data = pcs->read(pcs->base + offset);
+			switch (func->conf[i].param) {
+			/* 2 parameters */
+			case PIN_CONFIG_INPUT_SCHMITT:
+			case PIN_CONFIG_DRIVE_STRENGTH:
+			case PIN_CONFIG_SLEW_RATE:
+				shift = ffs(func->conf[i].mask) - 1;
+				arg = pinconf_to_config_argument(config);
+				data &= ~func->conf[i].mask;
+				data |= (arg << shift) & func->conf[i].mask;
+				break;
+			/* 3 parameters */
+			case PIN_CONFIG_BIAS_DISABLE:
+			case PIN_CONFIG_BIAS_PULL_DOWN:
+			case PIN_CONFIG_BIAS_PULL_UP:
+			case PIN_CONFIG_INPUT_SCHMITT_DISABLE:
+				data &= ~func->conf[i].mask;
+				data |= func->conf[i].val;
+				break;
+			default:
+				return -ENOTSUPP;
+			}
+			pcs->write(data, pcs->base + offset);
+			return 0;
+		}
+	}
 	return -ENOTSUPP;
 }
 
 static int pcs_pinconf_group_get(struct pinctrl_dev *pctldev,
 				unsigned group, unsigned long *config)
 {
-	return -ENOTSUPP;
+	const unsigned *pins;
+	unsigned npins, old = 0;
+	int i, ret;
+
+	ret = pcs_get_group_pins(pctldev, group, &pins, &npins);
+	if (ret)
+		return ret;
+	for (i = 0; i < npins; i++) {
+		if (pcs_pinconf_get(pctldev, pins[i], config))
+			return -ENOTSUPP;
+		/* configs do not match between two pins */
+		if (i && (old != *config))
+			return -ENOTSUPP;
+		old = *config;
+	}
+	return 0;
 }
 
 static int pcs_pinconf_group_set(struct pinctrl_dev *pctldev,
 				unsigned group, unsigned long config)
 {
-	return -ENOTSUPP;
+	const unsigned *pins;
+	unsigned npins;
+	int i, ret;
+
+	ret = pcs_get_group_pins(pctldev, group, &pins, &npins);
+	if (ret)
+		return ret;
+	for (i = 0; i < npins; i++) {
+		if (pcs_pinconf_set(pctldev, pins[i], config))
+			return -ENOTSUPP;
+	}
+	return 0;
 }
 
 static void pcs_pinconf_dbg_show(struct pinctrl_dev *pctldev,
-				struct seq_file *s, unsigned offset)
+				struct seq_file *s, unsigned pin)
 {
 }
 
@@ -495,6 +656,13 @@ static void pcs_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
 {
 }
 
+static void pcs_pinconf_config_dbg_show(struct pinctrl_dev *pctldev,
+					struct seq_file *s,
+					unsigned long config)
+{
+	pinconf_generic_dump_config(pctldev, s, config);
+}
+
 static struct pinconf_ops pcs_pinconf_ops = {
 	.pin_config_get = pcs_pinconf_get,
 	.pin_config_set = pcs_pinconf_set,
@@ -502,6 +670,7 @@ static struct pinconf_ops pcs_pinconf_ops = {
 	.pin_config_group_set = pcs_pinconf_group_set,
 	.pin_config_dbg_show = pcs_pinconf_dbg_show,
 	.pin_config_group_dbg_show = pcs_pinconf_group_dbg_show,
+	.pin_config_config_dbg_show = pcs_pinconf_config_dbg_show,
 };
 
 /**
@@ -692,11 +861,151 @@ 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;
+}
+
+static void add_config(struct pcs_conf_vals **conf, enum pin_config_param param,
+		       unsigned value, unsigned match, unsigned mask)
+{
+	(*conf)->param = param;
+	(*conf)->val = value;
+	(*conf)->match = match;
+	(*conf)->mask = mask;
+	(*conf)++;
+}
+
+static void add_setting(unsigned long **setting, enum pin_config_param param,
+			unsigned arg)
+{
+	**setting = pinconf_to_config_packed(param, arg);
+	(*setting)++;
+}
+
+/* add pinconf setting with 2 parameters */
+static void pcs_add_conf2(struct device_node *np, const char *name,
+			  enum pin_config_param param,
+			  struct pcs_conf_vals **conf,
+			  unsigned long **settings)
+{
+	unsigned value[2];
+	int ret;
+
+	ret = of_property_read_u32_array(np, name, value, 2);
+	if (ret)
+		return;
+	/* value to set,  mask */
+	value[0] &= value[1];
+	add_config(conf, param, value[0], 0, value[1]);
+	add_setting(settings, param, value[0]);
+}
+
+/* add pinconf setting with 3 parameters */
+static void pcs_add_conf3(struct device_node *np, const char *name,
+			  enum pin_config_param param,
+			  struct pcs_conf_vals **conf,
+			  unsigned long **settings)
+{
+	unsigned value[3];
+	int ret;
+
+	ret = of_property_read_u32_array(np, name, value, 3);
+	if (ret)
+		return;
+	/* value to set, match, mask */
+	value[0] &= value[2];
+	value[1] &= value[2];
+	add_config(conf, param, value[0], value[1], value[2]);
+	if (pcs_config_match(value[0], value[1]))
+		ret = 1;
+	else
+		ret = 0;
+	add_setting(settings, param, ret);
+}
+
+static int pcs_parse_pinconf(struct pcs_device *pcs, struct device_node *np,
+			     struct pcs_function *func,
+			     struct pinctrl_map **map)
+
+{
+	struct pinctrl_map *m = *map;
+	int i = 0, nconfs = 0;
+	unsigned long *settings = NULL, *s = NULL;
+	struct pcs_conf_vals *conf = NULL;
+	struct pcs_conf_type prop2[] = {
+		{ "pinctrl-single,drive-strength", PIN_CONFIG_DRIVE_STRENGTH, },
+		{ "pinctrl-single,slew-rate", PIN_CONFIG_SLEW_RATE, },
+		{ "pinctrl-single,input-schmitt", PIN_CONFIG_INPUT_SCHMITT, },
+	};
+	struct pcs_conf_type prop3[] = {
+		{ "pinctrl-single,bias-disable", PIN_CONFIG_BIAS_DISABLE, },
+		{ "pinctrl-single,bias-pullup", PIN_CONFIG_BIAS_PULL_UP, },
+		{ "pinctrl-single,bias-pulldown", PIN_CONFIG_BIAS_PULL_DOWN, },
+		{ "pinctrl-single,input-schmitt-disable",
+			PIN_CONFIG_INPUT_SCHMITT_DISABLE, },
+	};
+
+	/* If pinconf isn't supported, don't parse properties in below. */
+	if (!pcs->is_pinconf)
+		return 0;
+
+	/* cacluate how much properties are supported in current node */
+	for (i = 0; i < ARRAY_SIZE(prop2); i++) {
+		if (of_find_property(np, prop2[i].name, NULL))
+			nconfs++;
+	}
+	for (i = 0; i < ARRAY_SIZE(prop3); i++) {
+		if (of_find_property(np, prop3[i].name, NULL))
+			nconfs++;
+	}
+	if (!nconfs)
+		return 0;
+
+	func->conf = devm_kzalloc(pcs->dev,
+				  sizeof(struct pcs_conf_vals) * nconfs,
+				  GFP_KERNEL);
+	if (!func->conf)
+		return -ENOMEM;
+	func->nconfs = nconfs;
+	conf = &(func->conf[0]);
+	m++;
+	settings = devm_kzalloc(pcs->dev, sizeof(unsigned long) * nconfs,
+				GFP_KERNEL);
+	if (!settings)
+		return -ENOMEM;
+	s = &settings[0];
+
+	for (i = 0; i < ARRAY_SIZE(prop2); i++) {
+		pcs_add_conf2(np, prop2[i].name, prop2[i].param, &conf, &s);
+	}
+	for (i = 0; i < ARRAY_SIZE(prop3); i++) {
+		pcs_add_conf3(np, prop3[i].name, prop3[i].param, &conf, &s);
+	}
+	m->type = PIN_MAP_TYPE_CONFIGS_GROUP;
+	m->data.configs.group_or_pin = np->name;
+	m->data.configs.configs = settings;
+	m->data.configs.num_configs = nconfs;
+	return 0;
+}
+
+static void pcs_free_pingroups(struct pcs_device *pcs);
+
 /**
  * smux_parse_one_pinctrl_entry() - parses a device tree mux entry
  * @pcs: pinctrl driver instance
  * @np: device node of the mux entry
  * @map: map entry
+ * @num_maps: number of map
  * @pgnames: pingroup names
  *
  * Note that this binding currently supports only sets of one register + value.
@@ -713,6 +1022,7 @@ static int pcs_get_pin_by_offset(struct pcs_device *pcs, unsigned offset)
 static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 						struct device_node *np,
 						struct pinctrl_map **map,
+						unsigned *num_maps,
 						const char **pgnames)
 {
 	struct pcs_func_vals *vals;
@@ -785,8 +1095,18 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 	(*map)->data.mux.group = np->name;
 	(*map)->data.mux.function = np->name;
 
+	if (pcs->is_pinconf) {
+		if (pcs_parse_pinconf(pcs, np, function, map))
+			goto free_pingroups;
+		*num_maps = 2;
+	} else {
+		*num_maps = 1;
+	}
 	return 0;
 
+free_pingroups:
+	pcs_free_pingroups(pcs);
+	*num_maps = 1;
 free_function:
 	pcs_remove_function(pcs, function);
 
@@ -815,7 +1135,8 @@ static int pcs_dt_node_to_map(struct pinctrl_dev *pctldev,
 
 	pcs = pinctrl_dev_get_drvdata(pctldev);
 
-	*map = devm_kzalloc(pcs->dev, sizeof(**map), GFP_KERNEL);
+	/* create 2 maps. One is for pinmux, and the other is for pinconf. */
+	*map = devm_kzalloc(pcs->dev, sizeof(**map) * 2, GFP_KERNEL);
 	if (!*map)
 		return -ENOMEM;
 
@@ -827,13 +1148,13 @@ static int pcs_dt_node_to_map(struct pinctrl_dev *pctldev,
 		goto free_map;
 	}
 
-	ret = pcs_parse_one_pinctrl_entry(pcs, np_config, map, pgnames);
+	ret = pcs_parse_one_pinctrl_entry(pcs, np_config, map, num_maps,
+					  pgnames);
 	if (ret < 0) {
 		dev_err(pcs->dev, "no pins entries for %s\n",
 			np_config->name);
 		goto free_pgnames;
 	}
-	*num_maps = 1;
 
 	return 0;
 
@@ -976,6 +1297,7 @@ static int pcs_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&pcs->pingroups);
 	INIT_LIST_HEAD(&pcs->functions);
 	INIT_LIST_HEAD(&pcs->gpiofuncs);
+	pcs->is_pinconf = match->data;
 
 	PCS_GET_PROP_U32("pinctrl-single,register-width", &pcs->width,
 			 "register width not specified\n");
@@ -1046,6 +1368,8 @@ static int pcs_probe(struct platform_device *pdev)
 	pcs->desc.pmxops = &pcs_pinmux_ops;
 	pcs->desc.confops = &pcs_pinconf_ops;
 	pcs->desc.owner = THIS_MODULE;
+	if (match->data)
+		pcs_pinconf_ops.is_generic = true;
 
 	ret = pcs_allocate_pin_table(pcs);
 	if (ret < 0)
@@ -1086,7 +1410,8 @@ static int pcs_remove(struct platform_device *pdev)
 }
 
 static struct of_device_id pcs_of_match[] = {
-	{ .compatible = DRIVER_NAME, },
+	{ .compatible = "pinctrl-single", .data = (void *)false },
+	{ .compatible = "pinconf-single", .data = (void *)true },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, pcs_of_match);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 11/12] ARM: hs: enable hi4511 with device tree
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (9 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 10/12] pinctrl: single: support generic pinconf Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-02 17:25 ` [PATCH v8 12/12] document: devicetree: bind pinconf with pin single Haojian Zhuang
  11 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Enable Hisilicon Hi4511 development platform with device tree support.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/boot/dts/Makefile    |    1 +
 arch/arm/boot/dts/hi3620.dtsi |  556 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/hi4511.dts  |  772 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1329 insertions(+)
 create mode 100644 arch/arm/boot/dts/hi3620.dtsi
 create mode 100644 arch/arm/boot/dts/hi4511.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index ea2c272..2a22336 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -50,6 +50,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
 	exynos5440-ssdk5440.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += highbank.dtb \
 	ecx-2000.dtb
+dtb-$(CONFIG_ARCH_HS) += hi4511.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += integratorap.dtb \
 	integratorcp.dtb
 dtb-$(CONFIG_ARCH_LPC32XX) += ea3250.dtb phy3250.dtb
diff --git a/arch/arm/boot/dts/hi3620.dtsi b/arch/arm/boot/dts/hi3620.dtsi
new file mode 100644
index 0000000..547a93b
--- /dev/null
+++ b/arch/arm/boot/dts/hi3620.dtsi
@@ -0,0 +1,556 @@
+/*
+ * Hisilicon Ltd. Hi3620 SoC
+ *
+ * Copyright (C) 2012-2013 Linaro Ltd.
+ * Author: Haojian Zhuang <haojian.zhuang@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.
+ */
+
+/include/ "skeleton.dtsi"
+
+/ {
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+	};
+
+	amba {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "arm,amba-bus";
+		interrupt-parent = <&intc>;
+		ranges;
+
+		osc32k: osc at 0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <32768>;
+			clock-output-names = "osc32khz";
+		};
+
+		osc26m: osc at 1 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <26000000>;
+			clock-output-names = "osc26mhz";
+		};
+
+		pclk: clk at 0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <26000000>;
+			clock-output-names = "apb_pclk";
+		};
+
+		timclk0: clk at 1 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <60000000>;
+			clock-output-names = "timer0";
+		};
+
+		timclk1: clk at 2 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <60000000>;
+			clock-output-names = "timer1";
+		};
+
+		l2: l2-cache {
+			compatible = "arm,pl310-cache";
+			reg = <0xfc10000 0x100000>;
+			interrupts = <0 15 4>;
+			cache-unified;
+			cache-level = <2>;
+		};
+
+		intc: interrupt-controller at fc001000 {
+			compatible = "arm,cortex-a9-gic";
+			#interrupt-cells = <3>;
+			#address-cells = <0>;
+			interrupt-controller;
+			/* gic dist base, gic cpu base */
+			reg = <0xfc001000 0x1000>, <0xfc000100 0x100>;
+		};
+
+		timer0: timer at fc800000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0xfc800000 0x1000>;
+			/* timer00 & timer01 */
+			interrupts = <0 0 4>, <0 1 4>;
+			clocks = <&timclk0 &timclk1 &pclk>;
+			clock-names = "timer0", "timer1", "apb_pclk";
+			status = "disabled";
+		};
+
+		timer1: timer at fc801000 {
+			/*
+			 * Only used in NORMAL state, not available ins
+			 * SLOW or DOZE state.
+			 * The rate is fixed in 24MHz.
+			 */
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0xfc801000 0x1000>;
+			/* timer10 & timer11 */
+			interrupts = <0 2 4>, <0 3 4>;
+			clocks = <&timclk0 &timclk1 &pclk>;
+			clock-names = "timer0", "timer1", "apb_pclk";
+			status = "disabled";
+		};
+
+		timer2: timer at fca01000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0xfca01000 0x1000>;
+			/* timer20 & timer21 */
+			interrupts = <0 4 4>, <0 5 4>;
+			clocks = <&timclk0 &timclk1 &pclk>;
+			clock-names = "timer0", "timer1", "apb_pclk";
+			status = "disabled";
+		};
+
+		timer3: timer at fca02000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0xfca02000 0x1000>;
+			/* timer30 & timer31 */
+			interrupts = <0 6 4>, <0 7 4>;
+			clocks = <&timclk0 &timclk1 &pclk>;
+			clock-names = "timer0", "timer1", "apb_pclk";
+			status = "disabled";
+		};
+
+		timer4: timer at fca03000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0xfca03000 0x1000>;
+			/* timer40 & timer41 */
+			interrupts = <0 96 4>, <0 97 4>;
+			clocks = <&timclk0 &timclk1 &pclk>;
+			clock-names = "timer0", "timer1", "apb_pclk";
+			status = "disabled";
+		};
+
+		uart0: uart at fcb00000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0xfcb00000 0x1000>;
+			interrupts = <0 20 4>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart1: uart at fcb01000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0xfcb01000 0x1000>;
+			interrupts = <0 21 4>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart2: uart at fcb02000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0xfcb02000 0x1000>;
+			interrupts = <0 22 4>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart3: uart at fcb03000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0xfcb03000 0x1000>;
+			interrupts = <0 23 4>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart4: uart at fcb04000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0xfcb04000 0x1000>;
+			interrupts = <0 24 4>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio0: gpio at fc806000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc806000 0x1000>;
+			interrupts = <0 64 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 2 0 1 &pmx0 3 0 1 &pmx0 4 0 1
+					&pmx0 5 0 1 &pmx0 6 1 1 &pmx0 7 2 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio1: gpio at fc807000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc807000 0x1000>;
+			interrupts = <0 65 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 3 1 &pmx0 1 3 1 &pmx0 2 3 1
+					&pmx0 3 3 1 &pmx0 4 3 1 &pmx0 5 4 1
+					&pmx0 6 5 1 &pmx0 7 6 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio2: gpio at fc808000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc808000 0x1000>;
+			interrupts = <0 66 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 7 1 &pmx0 1 8 1 &pmx0 2 9 1
+					&pmx0 3 10 1 &pmx0 4 3 1 &pmx0 5 3 1
+					&pmx0 6 3 1 &pmx0 7 3 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio3: gpio at fc809000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc809000 0x1000>;
+			interrupts = <0 67 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 3 1 &pmx0 1 3 1 &pmx0 2 3 1
+					&pmx0 3 3 1 &pmx0 4 11 1 &pmx0 5 11 1
+					&pmx0 6 11 1 &pmx0 7 11 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio4: gpio at fc80a000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc80a000 0x1000>;
+			interrupts = <0 68 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 11 1 &pmx0 1 11 1 &pmx0 2 11 1
+					&pmx0 3 11 1 &pmx0 4 12 1 &pmx0 5 12 1
+					&pmx0 6 13 1 &pmx0 7 13 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio5: gpio at fc80b000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc80b000 0x1000>;
+			interrupts = <0 69 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 14 1 &pmx0 1 15 1 &pmx0 2 16 1
+					&pmx0 3 16 1 &pmx0 4 16 1 &pmx0 5 16 1
+					&pmx0 6 16 1 &pmx0 7 16 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio6: gpio at fc80c000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc80c000 0x1000>;
+			interrupts = <0 70 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 16 1 &pmx0 1 16 1 &pmx0 2 17 1
+					&pmx0 3 17 1 &pmx0 4 18 1 &pmx0 5 18 1
+					&pmx0 6 18 1 &pmx0 7 19 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio7: gpio at fc80d000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc80d000 0x1000>;
+			interrupts = <0 71 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 19 1 &pmx0 1 20 1 &pmx0 2 21 1
+					&pmx0 3 22 1 &pmx0 4 23 1 &pmx0 5 24 1
+					&pmx0 6 25 1 &pmx0 7 26 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio8: gpio at fc80e000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc80e000 0x1000>;
+			interrupts = <0 72 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 27 1 &pmx0 1 28 1 &pmx0 2 29 1
+					&pmx0 3 30 1 &pmx0 4 31 1 &pmx0 5 32 1
+					&pmx0 6 33 1 &pmx0 7 34 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio9: gpio at fc80f000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc80f000 0x1000>;
+			interrupts = <0 73 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 35 1 &pmx0 1 36 1 &pmx0 2 37 1
+					&pmx0 3 38 1 &pmx0 4 39 1 &pmx0 5 40 1
+					&pmx0 6 41 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio10: gpio at fc810000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc810000 0x1000>;
+			interrupts = <0 74 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 2 43 1 &pmx0 3 44 1 &pmx0 4 45 1
+					&pmx0 5 45 1 &pmx0 6 46 1 &pmx0 7 46 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio11: gpio at fc811000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc811000 0x1000>;
+			interrupts = <0 75 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 47 1 &pmx0 1 47 1 &pmx0 2 47 1
+					&pmx0 3 47 1 &pmx0 4 47 1 &pmx0 5 48 1
+					&pmx0 6 49 1 &pmx0 7 49 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio12: gpio at fc812000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc812000 0x1000>;
+			interrupts = <0 76 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 49 1 &pmx0 1 50 1 &pmx0 2 49 1
+					&pmx0 3 49 1 &pmx0 4 51 1 &pmx0 5 51 1
+					&pmx0 6 51 1 &pmx0 7 52 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio13: gpio at fc813000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc813000 0x1000>;
+			interrupts = <0 77 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 51 1 &pmx0 1 51 1 &pmx0 2 53 1
+					&pmx0 3 53 1 &pmx0 4 53 1 &pmx0 5 54 1
+					&pmx0 6 55 1 &pmx0 7 56 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio14: gpio at fc814000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc814000 0x1000>;
+			interrupts = <0 78 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 57 1 &pmx0 1 97 1 &pmx0 2 97 1
+					&pmx0 3 58 1 &pmx0 4 59 1 &pmx0 5 60 1
+					&pmx0 6 60 1 &pmx0 7 61 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio15: gpio at fc815000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc815000 0x1000>;
+			interrupts = <0 79 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 61 1 &pmx0 1 62 1 &pmx0 2 62 1
+					&pmx0 3 63 1 &pmx0 4 63 1 &pmx0 5 64 1
+					&pmx0 6 64 1 &pmx0 7 65 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio16: gpio at fc816000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc816000 0x1000>;
+			interrupts = <0 80 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 66 1 &pmx0 1 67 1 &pmx0 2 68 1
+					&pmx0 3 69 1 &pmx0 4 70 1 &pmx0 5 71 1
+					&pmx0 6 72 1 &pmx0 7 73 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio17: gpio at fc817000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc817000 0x1000>;
+			interrupts = <0 81 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 74 1 &pmx0 1 75 1 &pmx0 2 76 1
+					&pmx0 3 77 1 &pmx0 4 78 1 &pmx0 5 79 1
+					&pmx0 6 80 1 &pmx0 7 81 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio18: gpio at fc818000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc818000 0x1000>;
+			interrupts = <0 82 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 82 1 &pmx0 1 83 1 &pmx0 2 83 1
+					&pmx0 3 84 1 &pmx0 4 84 1 &pmx0 5 85 1
+					&pmx0 6 86 1 &pmx0 7 87 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio19: gpio at fc819000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc819000 0x1000>;
+			interrupts = <0 83 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 87 1 &pmx0 1 87 1 &pmx0 2 88 1
+					&pmx0 3 88 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio20: gpio at fc81a000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc81a000 0x1000>;
+			interrupts = <0 84 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 0 89 1 &pmx0 1 89 1 &pmx0 2 90 1
+					&pmx0 3 90 1 &pmx0 4 91 1 &pmx0 5 92 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		gpio21: gpio at fc81b000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0xfc81b000 0x1000>;
+			interrupts = <0 85 0x4>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <	&pmx0 3 94 1 &pmx0 7 96 1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+			status = "disable";
+		};
+
+		pmx0: pinmux at fc803000 {
+			compatible = "pinctrl-single";
+			reg = <0xfc803000 0x188>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#gpio-range-cells = <3>;
+			ranges;
+
+			pinctrl-single,register-width = <32>;
+			pinctrl-single,function-mask = <7>;
+			/* pin base, nr pins & gpio function */
+			pinctrl-single,gpio-range = <&range 0 3 0 &range 3 9 1
+						&range 12 1 0 &range 13 29 1
+						&range 43 1 0 &range 44 49 1
+						&range 94 1 1 &range 96 2 1>;
+
+			range: gpio-range {
+				#pinctrl-single,gpio-range-cells = <3>;
+			};
+		};
+
+		pmx1: pinmux at fc803800 {
+			compatible = "pinconf-single";
+			reg = <0xfc803800 0x2dc>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			pinctrl-single,register-width = <32>;
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/hi4511.dts b/arch/arm/boot/dts/hi4511.dts
new file mode 100644
index 0000000..f10b28f
--- /dev/null
+++ b/arch/arm/boot/dts/hi4511.dts
@@ -0,0 +1,772 @@
+/*
+ *  Copyright (C) 2012-2013 Linaro Ltd.
+ *  Author: Haojian Zhuang <haojian.zhuang@linaro.org>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+/include/ "hi3620.dtsi"
+
+/ {
+	model = "Hisilicon Hi4511 Development Board";
+	compatible = "hisilicon,hi3620-hi4511";
+
+	chosen {
+		bootargs = "console=ttyAMA0,115200 root=/dev/nfs nfsroot=192.168.1.100:/nfsroot/ ip=192.168.1.101:192.168.1.100::255.255.255.0::eth0:on mem=512m earlyprintk";
+	};
+
+	memory {
+		reg = <0x00000000 0x20000000>;
+	};
+
+	amba {
+		timer0: timer at fc800000 {
+			status = "ok";
+		};
+
+		uart0: uart at fcb00000 {	/* console */
+			pinctrl-names = "default", "idle";
+			pinctrl-0 = <&uart0_pmx_func &uart0_cfg_func>;
+			pinctrl-1 = <&uart0_pmx_idle &uart0_cfg_idle>;
+			status = "ok";
+		};
+
+		uart1: uart at fcb01000 { /* modem */
+			pinctrl-names = "default", "idle";
+			pinctrl-0 = <&uart1_pmx_func &uart1_cfg_func>;
+			pinctrl-1 = <&uart1_pmx_idle &uart1_cfg_idle>;
+			status = "ok";
+		};
+
+		uart2: uart at fcb02000 { /* audience */
+			pinctrl-names = "default", "idle";
+			pinctrl-0 = <&uart2_pmx_func &uart2_cfg_func>;
+			pinctrl-1 = <&uart2_pmx_idle &uart2_cfg_idle>;
+			status = "ok";
+		};
+
+		uart3: uart at fcb03000 {
+			pinctrl-names = "default", "idle";
+			pinctrl-0 = <&uart3_pmx_func &uart3_cfg_func>;
+			pinctrl-1 = <&uart3_pmx_idle &uart3_cfg_idle>;
+			status = "ok";
+		};
+
+		uart4: uart at fcb04000 {
+			pinctrl-names = "default", "idle";
+			pinctrl-0 = <&uart4_pmx_func &uart4_cfg_func>;
+			pinctrl-1 = <&uart4_pmx_idle &uart4_cfg_func>;
+			status = "ok";
+		};
+
+		gpio0: gpio at fc806000 {
+			status = "ok";
+		};
+
+		gpio1: gpio at fc807000 {
+			status = "ok";
+		};
+
+		gpio2: gpio at fc808000 {
+			status = "ok";
+		};
+
+		gpio3: gpio at fc809000 {
+			status = "ok";
+		};
+
+		gpio4: gpio at fc80a000 {
+			status = "ok";
+		};
+
+		gpio5: gpio at fc80b000 {
+			status = "ok";
+		};
+
+		gpio6: gpio at fc80c000 {
+			status = "ok";
+		};
+
+		gpio7: gpio at fc80d000 {
+			status = "ok";
+		};
+
+		gpio8: gpio at fc80e000 {
+			status = "ok";
+		};
+
+		gpio9: gpio at fc80f000 {
+			status = "ok";
+		};
+
+		gpio10: gpio at fc810000 {
+			status = "ok";
+		};
+
+		gpio11: gpio at fc811000 {
+			status = "ok";
+		};
+
+		gpio12: gpio at fc812000 {
+			status = "ok";
+		};
+
+		gpio13: gpio at fc813000 {
+			status = "ok";
+		};
+
+		gpio14: gpio at fc814000 {
+			status = "ok";
+		};
+
+		gpio15: gpio at fc815000 {
+			status = "ok";
+		};
+
+		gpio16: gpio at fc816000 {
+			status = "ok";
+		};
+
+		gpio17: gpio at fc817000 {
+			status = "ok";
+		};
+
+		gpio18: gpio at fc818000 {
+			status = "ok";
+		};
+
+		gpio19: gpio at fc819000 {
+			status = "ok";
+		};
+
+		gpio20: gpio at fc81a000 {
+			status = "ok";
+		};
+
+		gpio21: gpio at fc81b000 {
+			status = "ok";
+		};
+
+		gpio-keys {
+			compatible = "gpio-keys";
+
+			call {
+				label = "call";
+				gpios = <&gpio17 2 0>;
+				linux,code = <169>;	/* KEY_PHONE */
+			};
+		};
+
+		pmx0: pinmux at fc803000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c0_pmx_func>;
+
+			board_pmx_pins: pinmux_board_pmx_pins {
+				pinctrl-single,pins = <
+					0x008 0x0	/* GPIO -- eFUSE_DOUT */
+					0x100 0x0	/* USIM_CLK & USIM_DATA (IOMG63) */
+					0x104 0x0	/* USIM_RST (IOMG96) */
+				>;
+			};
+			sd_pmx_pins: pinmux_sd_pins {
+				pinctrl-single,pins = <
+					0x0bc 0x0	/* SD_CLK, SD_CMD, SD_DATA[0:2] */
+				>;
+			};
+			uart0_pmx_func: pinmux_uart0_pins at 0  {
+				pinctrl-single,pins = <
+					0x0f0 0x0
+					0x0f4 0x0	/* UART0_RX & UART0_TX */
+				>;
+			};
+			uart0_pmx_idle: pinmux_uart0_pins at 1 {
+				pinctrl-single,pins = <
+					/*0x0f0 0x1*/	/* UART0_CTS & UART0_RTS */
+					0x0f4 0x1	/* UART0_RX & UART0_TX */
+				>;
+			};
+			uart1_pmx_func: pinmux_uart1_pins at 0  {
+				pinctrl-single,pins = <
+					0x0f8 0x0	/* UART1_CTS & UART1_RTS (IOMG61) */
+					0x0fc 0x0	/* UART1_RX & UART1_TX (IOMG62) */
+				>;
+			};
+			uart1_pmx_idle: pinmux_uart1_pins at 1  {
+				pinctrl-single,pins = <
+					0x0f8 0x1	/* GPIO (IOMG61) */
+					0x0fc 0x1	/* GPIO (IOMG62) */
+				>;
+			};
+			uart2_pmx_func: pinmux_uart2_pins at 0 {
+				pinctrl-single,pins = <
+					0x104 0x2	/* UART2_RXD (IOMG96) */
+					0x108 0x2	/* UART2_TXD (IOMG64) */
+				>;
+			};
+			uart2_pmx_idle: pinmux_uart2_pins at 1 {
+				pinctrl-single,pins = <
+					0x104 0x1	/* GPIO (IOMG96) */
+					0x108 0x1	/* GPIO (IOMG64) */
+				>;
+			};
+			uart3_pmx_func: pinmux_uart3_pins at 0 {
+				pinctrl-single,pins = <
+					0x160 0x2	/* UART3_CTS & UART3_RTS (IOMG85) */
+					0x164 0x2	/* UART3_RXD & UART3_TXD (IOMG86) */
+				>;
+			};
+			uart3_pmx_idle: pinmux_uart3_pins at 1 {
+				pinctrl-single,pins = <
+					0x160 0x1	/* GPIO (IOMG85) */
+					0x164 0x1	/* GPIO (IOMG86) */
+				>;
+			};
+			uart4_pmx_func: pinmux_uart4_pins at 0 {
+				pinctrl-single,pins = <
+					0x168 0x0	/* UART4_CTS & UART4_RTS (IOMG87) */
+					0x16c 0x0	/* UART4_RXD (IOMG88) */
+					0x170 0x0	/* UART4_TXD (IOMG93) */
+				>;
+			};
+			uart4_pmx_idle: pinmux_uart4_pins at 1 {
+				pinctrl-single,pins = <
+					0x168 0x1	/* GPIO (IOMG87) */
+					0x16c 0x1	/* GPIO (IOMG88) */
+					0x170 0x1	/* GPIO (IOMG93) */
+				>;
+			};
+			i2c0_pmx_func: pinmux_i2c0_pins at 0 {
+				pinctrl-single,pins = <
+					0x0b4 0x0	/* I2C0_SCL & I2C0_SDA (IOMG45) */
+				>;
+			};
+			i2c0_pmx_idle: pinmux_i2c0_pins at 1 {
+				pinctrl-single,pins = <
+					0x0b4 0x1	/* GPIO (IOMG45) */
+				>;
+			};
+			i2c1_pmx_func: pinmux_i2c1_pins at 0 {
+				pinctrl-single,pins = <
+					0x0b8 0x0	/* I2C1_SCL & I2C1_SDA (IOMG46) */
+				>;
+			};
+			i2c1_pmx_idle: pinmux_i2c1_pins at 1 {
+				pinctrl-single,pins = <
+					0x0b8 0x1	/* GPIO (IOMG46) */
+				>;
+			};
+			i2c2_pmx_func: pinmux_i2c2_pins at 0 {
+				pinctrl-single,pins = <
+					0x068 0x0	/* I2C2_SCL (IOMG26) */
+					0x06c 0x0	/* I2C2_SDA (IOMG27) */
+				>;
+			};
+			i2c2_pmx_idle: pinmux_i2c2_pins at 0 {
+				pinctrl-single,pins = <
+					0x068 0x1	/* GPIO (IOMG26) */
+					0x06c 0x1	/* GPIO (IOMG27) */
+				>;
+			};
+			i2c3_pmx_func: pinmux_i2c3_pins at 0 {
+				pinctrl-single,pins = <
+					0x050 0x2	/* I2C3_SCL (IOMG20) */
+					0x054 0x2	/* I2C3_SDA (IOMG21) */
+				>;
+			};
+			i2c3_pmx_idle: pinmux_i2c3_pins at 0 {
+				pinctrl-single,pins = <
+					0x050 0x1	/* GPIO (IOMG20) */
+					0x054 0x1	/* GPIO (IOMG21) */
+				>;
+			};
+			spi0_pmx_func: pinmux_spi0_pins at 0 {
+				pinctrl-single,pins = <
+					0x0d4 0x0	/* SPI0_CLK/SPI0_DI/SPI0_DO (IOMG53) */
+					0x0d8 0x0	/* SPI0_CS0 (IOMG54) */
+					0x0dc 0x0	/* SPI0_CS1 (IOMG55) */
+					0x0e0 0x0	/* SPI0_CS2 (IOMG56) */
+					0x0e4 0x0	/* SPI0_CS3 (IOMG57) */
+				>;
+			};
+			spi0_pmx_idle: pinmux_spi0_pins at 1 {
+				pinctrl-single,pins = <
+					0x0d4 0x1	/* GPIO (IOMG53) */
+					0x0d8 0x1	/* GPIO (IOMG54) */
+					0x0dc 0x1	/* GPIO (IOMG55) */
+					0x0e0 0x1	/* GPIO (IOMG56) */
+					0x0e4 0x1	/* GPIO (IOMG57) */
+				>;
+			};
+			spi1_pmx_func: pinmux_spi1_pins at 0 {
+				pinctrl-single,pins = <
+					0x184 0x0	/* SPI1_CLK/SPI1_DI (IOMG98) */
+					0x0e8 0x0	/* SPI1_DO (IOMG58) */
+					0x0ec 0x0	/* SPI1_CS (IOMG95) */
+				>;
+			};
+			spi1_pmx_idle: pinmux_spi1_pins at 1 {
+				pinctrl-single,pins = <
+					0x184 0x1	/* GPIO (IOMG98) */
+					0x0e8 0x1	/* GPIO (IOMG58) */
+					0x0ec 0x1	/* GPIO (IOMG95) */
+				>;
+			};
+			kpc_pmx_func: pinmux_kpc_pins at 0 {
+				pinctrl-single,pins = <
+					0x12c 0x0	/* KEY_IN0 (IOMG73) */
+					0x130 0x0	/* KEY_IN1 (IOMG74) */
+					0x134 0x0	/* KEY_IN2 (IOMG75) */
+					0x10c 0x0	/* KEY_OUT0 (IOMG65) */
+					0x110 0x0	/* KEY_OUT1 (IOMG66) */
+					0x114 0x0	/* KEY_OUT2 (IOMG67) */
+				>;
+			};
+			kpc_pmx_idle: pinmux_kpc_pins at 1 {
+				pinctrl-single,pins = <
+					0x12c 0x1	/* GPIO (IOMG73) */
+					0x130 0x1	/* GPIO (IOMG74) */
+					0x134 0x1	/* GPIO (IOMG75) */
+					0x10c 0x1	/* GPIO (IOMG65) */
+					0x110 0x1	/* GPIO (IOMG66) */
+					0x114 0x1	/* GPIO (IOMG67) */
+				>;
+			};
+			gpio_key_func: pinmux_gpiokey_pins {
+				pinctrl-single,pins = <
+					0x10c 0x1	/* KEY_OUT0/GPIO (IOMG65) */
+					0x130 0x1	/* KEY_IN1/GPIO (IOMG74) */
+				>;
+			};
+			emmc_pmx_func: pinmux_emmc_pins at 0 {
+				pinctrl-single,pins = <
+					0x030 0x2	/* eMMC_CMD/eMMC_CLK (IOMG12) */
+					0x018 0x0	/* NAND_CS3_N (IOMG6) */
+					0x024 0x0	/* NAND_BUSY2_N (IOMG8) */
+					0x028 0x0	/* NAND_BUSY3_N (IOMG9) */
+					0x02c 0x2	/* eMMC_DATA[0:7] (IOMG10) */
+				>;
+			};
+			emmc_pmx_idle: pinmux_emmc_pins at 1 {
+				pinctrl-single,pins = <
+					0x030 0x0	/* GPIO (IOMG12) */
+					0x018 0x1	/* GPIO (IOMG6) */
+					0x024 0x1	/* GPIO (IOMG8) */
+					0x028 0x1	/* GPIO (IOMG9) */
+					0x02c 0x1	/* GPIO (IOMG10) */
+				>;
+			};
+			sd_pmx_func: pinmux_sd_pins at 0 {
+				pinctrl-single,pins = <
+					0x0bc 0x0	/* SD_CLK/SD_CMD/SD_DATA0/SD_DATA1/SD_DATA2 (IOMG47) */
+					0x0c0 0x0	/* SD_DATA3 (IOMG48) */
+				>;
+			};
+			sd_pmx_idle: pinmux_sd_pins at 1 {
+				pinctrl-single,pins = <
+					0x0bc 0x1	/* GPIO (IOMG47) */
+					0x0c0 0x1	/* GPIO (IOMG48) */
+				>;
+			};
+			nand_pmx_func: pinmux_nand_pins at 0 {
+				pinctrl-single,pins = <
+					0x00c 0x0	/* NAND_ALE/NAND_CLE/.../NAND_DATA[0:7] (IOMG3) */
+					0x010 0x0	/* NAND_CS1_N (IOMG4) */
+					0x014 0x0	/* NAND_CS2_N (IOMG5) */
+					0x018 0x0	/* NAND_CS3_N (IOMG6) */
+					0x01c 0x0	/* NAND_BUSY0_N (IOMG94) */
+					0x020 0x0	/* NAND_BUSY1_N (IOMG7) */
+					0x024 0x0	/* NAND_BUSY2_N (IOMG8) */
+					0x028 0x0	/* NAND_BUSY3_N (IOMG9) */
+					0x02c 0x0	/* NAND_DATA[8:15] (IOMG10) */
+				>;
+			};
+			nand_pmx_idle: pinmux_nand_pins at 1 {
+				pinctrl-single,pins = <
+					0x00c 0x1	/* GPIO (IOMG3) */
+					0x010 0x1	/* GPIO (IOMG4) */
+					0x014 0x1	/* GPIO (IOMG5) */
+					0x018 0x1	/* GPIO (IOMG6) */
+					0x01c 0x1	/* GPIO (IOMG94) */
+					0x020 0x1	/* GPIO (IOMG7) */
+					0x024 0x1	/* GPIO (IOMG8) */
+					0x028 0x1	/* GPIO (IOMG9) */
+					0x02c 0x1	/* GPIO (IOMG10) */
+				>;
+			};
+			sdio_pmx_func: pinmux_sdio_pins at 0 {
+				pinctrl-single,pins = <
+					0x0c4 0x0	/* SDIO_CLK/SDIO_CMD/SDIO_DATA[0:3] (IOMG49) */
+				>;
+			};
+			sdio_pmx_idle: pinmux_sdio_pins at 1 {
+				pinctrl-single,pins = <
+					0x0c4 0x1	/* GPIO (IOMG49) */
+				>;
+			};
+			audio_out_pmx_func: pinmux_audio_pins at 0 {
+				pinctrl-single,pins = <
+					0x0f0 0x1	/* GPIO (IOMG59), audio spk & earphone */
+				>;
+			};
+		};
+
+		pmx1: pinmux at fc803800 {
+			pinctrl-names = "default";
+			pinctrl-0 = <	&board_pu_pins &board_pd_pins &board_pd_ps_pins
+					&board_np_pins &board_ps_pins &kpc_cfg_func
+					&audio_out_cfg_func>;
+			board_pu_pins: pinmux_board_pu_pins {
+				pinctrl-single,pins = <
+					0x014 0		/* GPIO_158 (IOCFG2) */
+					0x018 0		/* GPIO_159 (IOCFG3) */
+					0x01c 0		/* BOOT_MODE0 (IOCFG4) */
+					0x020 0		/* BOOT_MODE1 (IOCFG5) */
+				>;
+				pinctrl-single,bias-pullup = <1 1 1>;
+				pinctrl-single,bias-disable = <1 0 1>;
+			};
+			board_pd_pins: pinmux_board_pd_pins {
+				pinctrl-single,pins = <
+					0x038 0		/* eFUSE_DOUT (IOCFG11) */
+					0x150 0		/* ISP_GPIO8 (IOCFG93) */
+					0x154 0		/* ISP_GPIO9 (IOCFG94) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+			};
+			board_pd_ps_pins: pinmux_board_pd_ps_pins {
+				pinctrl-single,pins = <
+					0x220 0		/* USIM_CLK (IOCFG144) */
+					0x224 0		/* USIM_DATA (IOCFG145) */
+					0x228 0		/* USIM_RST (IOCFG146) */
+					0x2d8 0		/* CLK_OUT0 (IOCFG190) */
+					0x004 0		/* PMU_SPI_DATA (IOCFG192) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			board_np_pins: pinmux_board_np_pins {
+				pinctrl-single,pins = <
+					0x24c 0		/* KEYPAD_OUT7 (IOCFG155) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+			};
+			board_ps_pins: pinmux_board_ps_pins {
+				pinctrl-single,pins = <
+					0x000 0		/* PMU_SPI_CLK (IOCFG191) */
+					0x008 0		/* PMU_SPI_CS_N (IOCFG193) */
+				>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			uart0_cfg_func: pinmux_uart0_pins at 2 {
+				pinctrl-single,pins = <
+					0x208 0		/* UART0_RXD (IOCFG138) */
+					0x20c 0		/* UART0_TXD (IOCFG139) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+			};
+			uart0_cfg_idle: pinmux_uart0_pins at 3 {
+				pinctrl-single,pins = <
+					0x208 0		/* UART0_RXD (IOCFG138) */
+					0x20c 0		/* UART0_TXD (IOCFG139) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+			};
+			uart1_cfg_func: pinmux_uart1_pins at 2 {
+				pinctrl-single,pins = <
+					0x210 0		/* UART1_CTS (IOCFG140) */
+					0x214 0		/* UART1_RTS (IOCFG141) */
+					0x218 0		/* UART1_RXD (IOCFG142) */
+					0x21c 0		/* UART1_TXD (IOCFG143) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+			};
+			uart1_cfg_idle: pinmux_uart1_pins at 3 {
+				pinctrl-single,pins = <
+					0x210 0		/* UART1_CTS (IOCFG140) */
+					0x214 0		/* UART1_RTS (IOCFG141) */
+					0x218 0		/* UART1_RXD (IOCFG142) */
+					0x21c 0		/* UART1_TXD (IOCFG143) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+			};
+			uart2_cfg_func: pinmux_uart2_pins at 2 {
+				pinctrl-single,pins = <
+					0x210 0		/* UART1_CTS (IOCFG140) */
+					0x214 0		/* UART1_RTS (IOCFG141) */
+					0x218 0		/* UART1_RXD (IOCFG142) */
+					0x21c 0		/* UART1_TXD (IOCFG143) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+			};
+			uart2_cfg_idle: pinmux_uart2_pins at 3 {
+				pinctrl-single,pins = <
+					0x210 0		/* GPIO (IOCFG140) */
+					0x214 0		/* GPIO (IOCFG141) */
+					0x218 0		/* GPIO (IOCFG142) */
+					0x21c 0		/* GPIO (IOCFG143) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+			};
+			uart3_cfg_func: pinmux_uart3_pins at 2 {
+				pinctrl-single,pins = <
+					0x294 0		/* UART3_CTS (IOCFG173) */
+					0x298 0		/* UART3_RTS (IOCFG174) */
+					0x29c 0		/* UART3_RXD (IOCFG175) */
+					0x2a0 0		/* UART3_TXD (IOCFG176) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+			};
+			uart3_cfg_idle: pinmux_uart3_pins at 3 {
+				pinctrl-single,pins = <
+					0x294 0		/* UART3_CTS (IOCFG173) */
+					0x298 0		/* UART3_RTS (IOCFG174) */
+					0x29c 0		/* UART3_RXD (IOCFG175) */
+					0x2a0 0		/* UART3_TXD (IOCFG176) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+			};
+			uart4_cfg_func: pinmux_uart4_pins at 2 {
+				pinctrl-single,pins = <
+					0x2a4 0		/* UART4_CTS (IOCFG177) */
+					0x2a8 0		/* UART4_RTS (IOCFG178) */
+					0x2ac 0		/* UART4_RXD (IOCFG179) */
+					0x2b0 0		/* UART4_TXD (IOCFG180) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+			};
+			i2c0_cfg_func: pinmux_i2c0_pins at 2 {
+				pinctrl-single,pins = <
+					0x17c 0		/* I2C0_SCL (IOCFG103) */
+					0x180 0		/* I2C0_SDA (IOCFG104) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			i2c1_cfg_func: pinmux_i2c1_pins at 2 {
+				pinctrl-single,pins = <
+					0x184 0		/* I2C1_SCL (IOCFG105) */
+					0x188 0		/* I2C1_SDA (IOCFG106) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			i2c2_cfg_func: pinmux_i2c2_pins at 2 {
+				pinctrl-single,pins = <
+					0x118 0		/* I2C2_SCL (IOCFG79) */
+					0x11c 0		/* I2C2_SDA (IOCFG80) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			i2c3_cfg_func: pinmux_i2c3_pins at 2 {
+				pinctrl-single,pins = <
+					0x100 0		/* I2C3_SCL (IOCFG73) */
+					0x104 0		/* I2C3_SDA (IOCFG74) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <0 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			spi0_cfg_func1: pinmux_spi0_pins at 2 {
+				pinctrl-single,pins = <
+					0x1d4 0		/* SPI0_CLK (IOCFG125) */
+					0x1d8 0		/* SPI0_DI (IOCFG126) */
+					0x1dc 0		/* SPI0_DO (IOCFG127) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			spi0_cfg_func2: pinmux_spi0_pins at 3 {
+				pinctrl-single,pins = <
+					0x1e0 0		/* SPI0_CS0 (IOCFG128) */
+					0x1e4 0		/* SPI0_CS1 (IOCFG129) */
+					0x1e8 0		/* SPI0_CS2 (IOCFG130 */
+					0x1ec 0		/* SPI0_CS3 (IOCFG131) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <1 1 1>;
+				pinctrl-single,bias-disable = <1 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			spi1_cfg_func1: pinmux_spi1_pins at 2 {
+				pinctrl-single,pins = <
+					0x1f0 0		/* SPI1_CLK (IOCFG132) */
+					0x1f4 0		/* SPI1_DI (IOCFG133) */
+					0x1f8 0		/* SPI1_DO (IOCFG134) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			spi1_cfg_func2: pinmux_spi1_pins at 3 {
+				pinctrl-single,pins = <
+					0x1fc 0		/* SPI1_CS (IOCFG135) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <1 1 1>;
+				pinctrl-single,bias-disable = <1 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			kpc_cfg_func: pinmux_kpc_pins at 2 {
+				pinctrl-single,pins = <
+					0x250 0		/* KEY_IN0 (IOCFG156) */
+					0x254 0		/* KEY_IN1 (IOCFG157) */
+					0x258 0		/* KEY_IN2 (IOCFG158) */
+					0x230 0		/* KEY_OUT0 (IOCFG148) */
+					0x234 0		/* KEY_OUT1 (IOCFG149) */
+					0x238 0		/* KEY_OUT2 (IOCFG150) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+			};
+			emmc_cfg_func: pinmux_emmc_pins at 2 {
+				pinctrl-single,pins = <
+					0x0ac 0		/* eMMC_CMD (IOCFG40) */
+					0x0b0 0		/* eMMC_CLK (IOCFG41) */
+					0x058 0		/* NAND_CS3_N (IOCFG19) */
+					0x064 0		/* NAND_BUSY2_N (IOCFG22) */
+					0x068 0		/* NAND_BUSY3_N (IOCFG23) */
+					0x08c 0		/* NAND_DATA8 (IOCFG32) */
+					0x090 0		/* NAND_DATA9 (IOCFG33) */
+					0x094 0		/* NAND_DATA10 (IOCFG34) */
+					0x098 0		/* NAND_DATA11 (IOCFG35) */
+					0x09c 0		/* NAND_DATA12 (IOCFG36) */
+					0x0a0 0		/* NAND_DATA13 (IOCFG37) */
+					0x0a4 0		/* NAND_DATA14 (IOCFG38) */
+					0x0a8 0		/* NAND_DATA15 (IOCFG39) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <1 1 1>;
+				pinctrl-single,bias-disable = <1 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			sd_cfg_func1: pinmux_sd_pins at 2 {
+				pinctrl-single,pins = <
+					0x18c 0		/* SD_CLK (IOCFG107) */
+					0x190 0		/* SD_CMD (IOCFG108) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			sd_cfg_func2: pinmux_sd_pins at 3 {
+				pinctrl-single,pins = <
+					0x194 0		/* SD_DATA0 (IOCFG109) */
+					0x198 0		/* SD_DATA1 (IOCFG110) */
+					0x19c 0		/* SD_DATA2 (IOCFG111) */
+					0x1a0 0		/* SD_DATA3 (IOCFG112) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+				pinctrl-single,drive-strength = <0x70 0xf0>;
+			};
+			nand_cfg_func1: pinmux_nand_pins at 2 {
+				pinctrl-single,pins = <
+					0x03c 0		/* NAND_ALE (IOCFG12) */
+					0x040 0		/* NAND_CLE (IOCFG13) */
+					0x06c 0		/* NAND_DATA0 (IOCFG24) */
+					0x070 0		/* NAND_DATA1 (IOCFG25) */
+					0x074 0		/* NAND_DATA2 (IOCFG26) */
+					0x078 0		/* NAND_DATA3 (IOCFG27) */
+					0x07c 0		/* NAND_DATA4 (IOCFG28) */
+					0x080 0		/* NAND_DATA5 (IOCFG29) */
+					0x084 0		/* NAND_DATA6 (IOCFG30) */
+					0x088 0		/* NAND_DATA7 (IOCFG31) */
+					0x08c 0		/* NAND_DATA8 (IOCFG32) */
+					0x090 0		/* NAND_DATA9 (IOCFG33) */
+					0x094 0		/* NAND_DATA10 (IOCFG34) */
+					0x098 0		/* NAND_DATA11 (IOCFG35) */
+					0x09c 0		/* NAND_DATA12 (IOCFG36) */
+					0x0a0 0		/* NAND_DATA13 (IOCFG37) */
+					0x0a4 0		/* NAND_DATA14 (IOCFG38) */
+					0x0a8 0		/* NAND_DATA15 (IOCFG39) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			nand_cfg_func2: pinmux_nand_pins at 3 {
+				pinctrl-single,pins = <
+					0x044 0		/* NAND_RE_N (IOCFG14) */
+					0x048 0		/* NAND_WE_N (IOCFG15) */
+					0x04c 0		/* NAND_CS0_N (IOCFG16) */
+					0x050 0		/* NAND_CS1_N (IOCFG17) */
+					0x054 0		/* NAND_CS2_N (IOCFG18) */
+					0x058 0		/* NAND_CS3_N (IOCFG19) */
+					0x05c 0		/* NAND_BUSY0_N (IOCFG20) */
+					0x060 0		/* NAND_BUSY1_N (IOCFG21) */
+					0x064 0		/* NAND_BUSY2_N (IOCFG22) */
+					0x068 0		/* NAND_BUSY3_N (IOCFG23) */
+				>;
+				pinctrl-single,bias-pulldown = <0 2 2>;
+				pinctrl-single,bias-pullup = <1 1 1>;
+				pinctrl-single,bias-disable = <1 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			sdio_cfg_func: pinmux_sdio_pins at 2 {
+				pinctrl-single,pins = <
+					0x1a4 0		/* SDIO0_CLK (IOCG113) */
+					0x1a8 0		/* SDIO0_CMD (IOCG114) */
+					0x1ac 0		/* SDIO0_DATA0 (IOCG115) */
+					0x1b0 0		/* SDIO0_DATA1 (IOCG116) */
+					0x1b4 0		/* SDIO0_DATA2 (IOCG117) */
+					0x1b8 0		/* SDIO0_DATA3 (IOCG118) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+				pinctrl-single,drive-strength = <0x30 0xf0>;
+			};
+			audio_out_cfg_func: pinmux_audio_pins at 1 {
+				pinctrl-single,pins = <
+					0x200 0		/* GPIO (IOCFG136) */
+					0x204 0		/* GPIO (IOCFG137) */
+				>;
+				pinctrl-single,bias-pulldown = <2 2 2>;
+				pinctrl-single,bias-pullup = <0 1 1>;
+				pinctrl-single,bias-disable = <2 0 3>;
+			};
+		};
+	};
+};
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
                   ` (10 preceding siblings ...)
  2013-02-02 17:25 ` [PATCH v8 11/12] ARM: hs: enable hi4511 with device tree Haojian Zhuang
@ 2013-02-02 17:25 ` Haojian Zhuang
  2013-02-05  4:07   ` Tony Lindgren
  11 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-02 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

From: Haojian Zhuang <haojian.zhuang@gmail.com>

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 |  108 +++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
index 2c81e45..eeb2e93 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,61 @@ 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,drive-strength : array of value that are used to configure
+  drive strength in the pinmux register. They're value of drive strength
+  current and drive strength mask.
+
+		/* drive strength current, mask */
+		pinctrl-single,power-source = <0x30 0xf0>;
+
+- pinctrl-single,bias-disable : array of value that are used to configure the
+  input bias disabled in the pinmux register. They're value of bias value,
+  match bias disabled value and bias disabled mask.
+
+		/* bias value, match bias disabled value, mask */
+		pinctrl-single,bias-disable = <2 0 3>;
+
+- pinctrl-single,bias-pullup : array of value that are used to configure the
+  input bias pullup in the pinmux register. They're value of bias value,
+  match bias pullup value and bias pullup mask.
+
+		/* bias value, match bias pullup value, mask */
+		pinctrl-single,bias-pullup = <0 1 1>;
+
+- pinctrl-single,bias-pulldown : array of value that are used to configure the
+  input bias pulldown in the pinmux register. They're value of bias value,
+  match bias pulldown value and bias pulldown mask.
+
+		/* bias value, match bias pulldown value, mask */
+		pinctrl-single,bias-pulldown = <2 2 2>;
+
+- pinctrl-single,input-schmitt : array of value that are used to configure
+  input schmitt in the pinmux register. In some silicons, there're two input
+  schmitt value (rising-edge & falling-edge) in the pinmux register.
+
+		/* input schmitt value, mask */
+		pinctrl-single,input-schmitt = <0x30 0x70>;
+
+- pinctrl-single,input-schmitt-disable : array of value that are used to
+  configure input schmitt disabled in the pinmux register. They're value of
+  input schmitt field, match disable value & mask.
+
+		/* input schmitt value, match disable value, mask */
+		pinctrl-single,input-schmitt-disable = <0x30 0x40 0x70>;
+
+- pinctrl-single,gpio-range : list of value that are used to configure a GPIO
+  range. They're value of subnode phandle, pin base in pinctrl device, pin
+  number in this range, GPIO function value of this GPIO range.
+  The number of parameters is depend on #pinctrl-single,gpio-range-cells
+  property.
+
+		/* pin base, nr pins & gpio function */
+		pinctrl-single,gpio-range = <&range 0 3 0 &range 3 9 1>;
+
 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 +96,20 @@ 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. This sub-node is required
+by pinctrl-single,gpio-range property.
+
+Required properties in sub-node:
+- #pinctrl-single,gpio-range-cells : the number of parameters after phandle in
+  pinctrl-single,gpio-range property.
+
+	range: gpio-range {
+		#pinctrl-single,gpio-range-cells = <3>;
+	};
+
+
 Example:
 
 /* SoC common file */
@@ -76,6 +144,29 @@ 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>;
+
+	/* sparse GPIO range could be supported */
+	pinctrl-single,gpio-range = <&range 0 3 0 &range 3 9 1
+				&range 12 1 0 &range 13 29 1
+				&range 43 1 0 &range 44 49 1
+				&range 94 1 1 &range 96 2 1>;
+
+	range: gpio-range {
+		#pinctrl-single,gpio-range-cells = <3>;
+	};
+};
+
+
 /* board specific .dts file */
 
 &pmx_core {
@@ -96,6 +187,16 @@ control_devconf0: pinmux at 48002274 {
 		>;
 	};
 
+	uart0_pins: pinmux_uart0_pins {
+		pinctrl-single,pins = <
+			0x208 0		/* UART0_RXD (IOCFG138) */
+			0x20c 0		/* UART0_TXD (IOCFG139) */
+		>;
+		pinctrl-single,bias-pulldown = <0 2 2>;
+		pinctrl-single,bias-pullup = <0 1 1>;
+		pinctrl-single,bias-disable = <0 0 3>;
+	};
+
 	/* map uart2 pins */
 	uart2_pins: pinmux_uart2_pins {
 		pinctrl-single,pins = <
@@ -122,6 +223,11 @@ control_devconf0: pinmux@48002274 {
 
 };
 
+&uart1 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart0_pins>;
+};
+
 &uart2 {
        pinctrl-names = "default";
        pinctrl-0 = <&uart2_pins>;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property
  2013-02-02 17:25 ` [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property Haojian Zhuang
@ 2013-02-05  0:23   ` Tony Lindgren
  2013-02-05  1:06     ` Haojian Zhuang
  2013-02-10 19:03   ` Linus Walleij
  2013-04-29 16:00   ` [v8,01/12] " James Hogan
  2 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2013-02-05  0:23 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:29]:
> Add gpio offset into "gpio-range-cells" property. It's used to support
> sparse pinctrl range in gpio chip.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  Documentation/devicetree/bindings/gpio/gpio.txt |    6 +++---
>  arch/arm/boot/dts/spear1310.dtsi                |    4 ++--
>  arch/arm/boot/dts/spear1340.dtsi                |    4 ++--
>  arch/arm/boot/dts/spear310.dtsi                 |    4 ++--
>  arch/arm/boot/dts/spear320.dtsi                 |    4 ++--
>  drivers/gpio/gpiolib-of.c                       |   15 ++-------------
>  6 files changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> index a336287..d933af3 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -98,7 +98,7 @@ announce the pinrange to the pin ctrl subsystem. For example,
>  		compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank";
>  		reg = <0x1460 0x18>;
>  		gpio-controller;
> -		gpio-ranges = <&pinctrl1 20 10>, <&pinctrl2 50 20>;
> +		gpio-ranges = <&pinctrl1 0 20 10>, <&pinctrl2 10 50 20>;
>  
>      }
>  
> @@ -107,8 +107,8 @@ where,
>  
>     Next values specify the base pin and number of pins for the range
>     handled by 'qe_pio_e' gpio. In the given example from base pin 20 to
> -   pin 29 under pinctrl1 and pin 50 to pin 69 under pinctrl2 is handled
> -   by this gpio controller.
> +   pin 29 under pinctrl1 with gpio offset 0 and pin 50 to pin 69 under
> +   pinctrl2 with gpio offset 10 is handled by this gpio controller.
>  
>  The pinctrl node must have "#gpio-range-cells" property to show number of
>  arguments to pass with phandle from gpio controllers node.
> diff --git a/arch/arm/boot/dts/spear1310.dtsi b/arch/arm/boot/dts/spear1310.dtsi
> index 1513c19..122ae94 100644
> --- a/arch/arm/boot/dts/spear1310.dtsi
> +++ b/arch/arm/boot/dts/spear1310.dtsi
> @@ -89,7 +89,7 @@
>  		pinmux: pinmux at e0700000 {
>  			compatible = "st,spear1310-pinmux";
>  			reg = <0xe0700000 0x1000>;
> -			#gpio-range-cells = <2>;
> +			#gpio-range-cells = <3>;
>  		};
>  
>  		apb {

Hmm is this safe to do if there are bootloaders using the old binding?
Maybe we should support both #gpio-range-cells = <2> and <3> bindings
instead?

Regards,

Tony

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 08/12] pinctrl: generic: dump pin configuration
  2013-02-02 17:25 ` [PATCH v8 08/12] pinctrl: generic: dump pin configuration Haojian Zhuang
@ 2013-02-05  0:35   ` Tony Lindgren
  2013-02-05  0:57     ` Tony Lindgren
  2013-02-05  1:08     ` Haojian Zhuang
  0 siblings, 2 replies; 42+ messages in thread
From: Tony Lindgren @ 2013-02-05  0:35 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:30]:
> @@ -114,4 +116,10 @@ static inline void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
>  	return;
>  }
>  
> +static void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
> +					struct seq_file *s,
> +					unsigned long config)
> +{
> +	return;
> +}
>  #endif

This needs to be static inline void to avoid the following warning:

drivers/pinctrl/pinconf.h:122: warning: ?pinconf_generic_dump_config? defined but not used

Regards,

Tony

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 10/12] pinctrl: single: support generic pinconf
  2013-02-02 17:25 ` [PATCH v8 10/12] pinctrl: single: support generic pinconf Haojian Zhuang
@ 2013-02-05  0:46   ` Tony Lindgren
  2013-02-05  1:07     ` Haojian Zhuang
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2013-02-05  0:46 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:32]:
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -22,8 +22,10 @@
>  
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/pinctrl/pinmux.h>
> +#include <linux/pinctrl/pinconf-generic.h>
>  
>  #include "core.h"
> +#include "pinconf.h"
>  
>  #define DRIVER_NAME			"pinctrl-single"
>  #define PCS_MUX_PINS_NAME		"pinctrl-single,pins"

I'm getting the following with this patch applied:

"pinconf_generic_dump_config" [drivers/pinctrl/pinctrl-single.ko] undefined!

This is with the following options:

CONFIG_PINCTRL=y
CONFIG_PINMUX=y
CONFIG_PINCONF=y
CONFIG_GENERIC_PINCONF=y
CONFIG_DEBUG_PINCTRL=y
CONFIG_PINCTRL_SINGLE=m

The includes look right, probably you're just missing
EXPORT_SYMBOL(pinconf_generic_dump_config) in the patch
adding pinconf_generic_dump_config?

Regards,

Tony

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 08/12] pinctrl: generic: dump pin configuration
  2013-02-05  0:35   ` Tony Lindgren
@ 2013-02-05  0:57     ` Tony Lindgren
  2013-02-05  1:09       ` Haojian Zhuang
  2013-02-05  1:08     ` Haojian Zhuang
  1 sibling, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2013-02-05  0:57 UTC (permalink / raw)
  To: linux-arm-kernel

* Tony Lindgren <tony@atomide.com> [130204 16:42]:
> * Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:30]:
> > @@ -114,4 +116,10 @@ static inline void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
> >  	return;
> >  }
> >  
> > +static void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
> > +					struct seq_file *s,
> > +					unsigned long config)
> > +{
> > +	return;
> > +}
> >  #endif
> 
> This needs to be static inline void to avoid the following warning:
> 
> drivers/pinctrl/pinconf.h:122: warning: ?pinconf_generic_dump_config? defined but not used

Actually this patch needs few other changes to export it and compile,
here's the version that worked for me with pinctlr-single as a module:

--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -16,6 +16,7 @@
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/debugfs.h>
+#include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinconf.h>
@@ -120,4 +121,19 @@ void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
 	}
 }
 
+int pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
+				 struct seq_file *s, unsigned long config)
+{
+	int i;
+
+	for(i = 0; i < ARRAY_SIZE(conf_items); i++) {
+		if (pinconf_to_config_param(config) != conf_items[i].param)
+			continue;
+		seq_printf(s, "%s: 0x%x", conf_items[i].display,
+			   pinconf_to_config_argument(config));
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(pinconf_generic_dump_config);
 #endif
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index e3ed8cb..c91ea3e 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -98,6 +98,8 @@ void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
 void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
 			      struct seq_file *s, const char *gname);
 
+int pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
+				 struct seq_file *s, unsigned long config);
 #else
 
 static inline void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
@@ -114,4 +116,10 @@ static inline void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
 	return;
 }
 
+static inline int pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
+					struct seq_file *s,
+					unsigned long config)
+{
+	return;
+}
 #endif

^ permalink raw reply related	[flat|nested] 42+ messages in thread
* [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property
  2013-02-05  0:23   ` Tony Lindgren
@ 2013-02-05  1:06     ` Haojian Zhuang
  0 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-05  1:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 5 February 2013 08:23, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:29]:
>> Add gpio offset into "gpio-range-cells" property. It's used to support
>> sparse pinctrl range in gpio chip.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/gpio/gpio.txt |    6 +++---
>>  arch/arm/boot/dts/spear1310.dtsi                |    4 ++--
>>  arch/arm/boot/dts/spear1340.dtsi                |    4 ++--
>>  arch/arm/boot/dts/spear310.dtsi                 |    4 ++--
>>  arch/arm/boot/dts/spear320.dtsi                 |    4 ++--
>>  drivers/gpio/gpiolib-of.c                       |   15 ++-------------
>>  6 files changed, 13 insertions(+), 24 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
>> index a336287..d933af3 100644
>> --- a/Documentation/devicetree/bindings/gpio/gpio.txt
>> +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
>> @@ -98,7 +98,7 @@ announce the pinrange to the pin ctrl subsystem. For example,
>>               compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank";
>>               reg = <0x1460 0x18>;
>>               gpio-controller;
>> -             gpio-ranges = <&pinctrl1 20 10>, <&pinctrl2 50 20>;
>> +             gpio-ranges = <&pinctrl1 0 20 10>, <&pinctrl2 10 50 20>;
>>
>>      }
>>
>> @@ -107,8 +107,8 @@ where,
>>
>>     Next values specify the base pin and number of pins for the range
>>     handled by 'qe_pio_e' gpio. In the given example from base pin 20 to
>> -   pin 29 under pinctrl1 and pin 50 to pin 69 under pinctrl2 is handled
>> -   by this gpio controller.
>> +   pin 29 under pinctrl1 with gpio offset 0 and pin 50 to pin 69 under
>> +   pinctrl2 with gpio offset 10 is handled by this gpio controller.
>>
>>  The pinctrl node must have "#gpio-range-cells" property to show number of
>>  arguments to pass with phandle from gpio controllers node.
>> diff --git a/arch/arm/boot/dts/spear1310.dtsi b/arch/arm/boot/dts/spear1310.dtsi
>> index 1513c19..122ae94 100644
>> --- a/arch/arm/boot/dts/spear1310.dtsi
>> +++ b/arch/arm/boot/dts/spear1310.dtsi
>> @@ -89,7 +89,7 @@
>>               pinmux: pinmux at e0700000 {
>>                       compatible = "st,spear1310-pinmux";
>>                       reg = <0xe0700000 0x1000>;
>> -                     #gpio-range-cells = <2>;
>> +                     #gpio-range-cells = <3>;
>>               };
>>
>>               apb {
>
> Hmm is this safe to do if there are bootloaders using the old binding?
> Maybe we should support both #gpio-range-cells = <2> and <3> bindings
> instead?
>
> Regards,
>
> Tony

I think that bootloader aren't using the old binding. I always Cc Shiraz.
If it can't work, he would speak loudly.

And I don't want to add complexity in this function. So I force the number of
parameters is three.

Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 10/12] pinctrl: single: support generic pinconf
  2013-02-05  0:46   ` Tony Lindgren
@ 2013-02-05  1:07     ` Haojian Zhuang
  0 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-05  1:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 5 February 2013 08:46, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:32]:
>> --- a/drivers/pinctrl/pinctrl-single.c
>> +++ b/drivers/pinctrl/pinctrl-single.c
>> @@ -22,8 +22,10 @@
>>
>>  #include <linux/pinctrl/pinctrl.h>
>>  #include <linux/pinctrl/pinmux.h>
>> +#include <linux/pinctrl/pinconf-generic.h>
>>
>>  #include "core.h"
>> +#include "pinconf.h"
>>
>>  #define DRIVER_NAME                  "pinctrl-single"
>>  #define PCS_MUX_PINS_NAME            "pinctrl-single,pins"
>
> I'm getting the following with this patch applied:
>
> "pinconf_generic_dump_config" [drivers/pinctrl/pinctrl-single.ko] undefined!
>
> This is with the following options:
>
> CONFIG_PINCTRL=y
> CONFIG_PINMUX=y
> CONFIG_PINCONF=y
> CONFIG_GENERIC_PINCONF=y
> CONFIG_DEBUG_PINCTRL=y
> CONFIG_PINCTRL_SINGLE=m
>
> The includes look right, probably you're just missing
> EXPORT_SYMBOL(pinconf_generic_dump_config) in the patch
> adding pinconf_generic_dump_config?
>
> Regards,
>
> Tony

Thanks for your catch. I'll fix it right now.

Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 08/12] pinctrl: generic: dump pin configuration
  2013-02-05  0:35   ` Tony Lindgren
  2013-02-05  0:57     ` Tony Lindgren
@ 2013-02-05  1:08     ` Haojian Zhuang
  1 sibling, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-05  1:08 UTC (permalink / raw)
  To: linux-arm-kernel

On 5 February 2013 08:35, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:30]:
>> @@ -114,4 +116,10 @@ static inline void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
>>       return;
>>  }
>>
>> +static void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
>> +                                     struct seq_file *s,
>> +                                     unsigned long config)
>> +{
>> +     return;
>> +}
>>  #endif
>
> This needs to be static inline void to avoid the following warning:
>
> drivers/pinctrl/pinconf.h:122: warning: ?pinconf_generic_dump_config? defined but not used
>
> Regards,
>
> Tony

Thanks. I'll update it.

Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 08/12] pinctrl: generic: dump pin configuration
  2013-02-05  0:57     ` Tony Lindgren
@ 2013-02-05  1:09       ` Haojian Zhuang
  0 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-05  1:09 UTC (permalink / raw)
  To: linux-arm-kernel

On 5 February 2013 08:57, Tony Lindgren <tony@atomide.com> wrote:
> * Tony Lindgren <tony@atomide.com> [130204 16:42]:
>> * Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:30]:
>> > @@ -114,4 +116,10 @@ static inline void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
>> >     return;
>> >  }
>> >
>> > +static void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
>> > +                                   struct seq_file *s,
>> > +                                   unsigned long config)
>> > +{
>> > +   return;
>> > +}
>> >  #endif
>>
>> This needs to be static inline void to avoid the following warning:
>>
>> drivers/pinctrl/pinconf.h:122: warning: ?pinconf_generic_dump_config? defined but not used
>
> Actually this patch needs few other changes to export it and compile,
> here's the version that worked for me with pinctlr-single as a module:
>

Thanks. I'll follow your changes.

Regards
Haojian

> --- a/drivers/pinctrl/pinconf-generic.c
> +++ b/drivers/pinctrl/pinconf-generic.c
> @@ -16,6 +16,7 @@
>  #include <linux/device.h>
>  #include <linux/slab.h>
>  #include <linux/debugfs.h>
> +#include <linux/module.h>
>  #include <linux/seq_file.h>
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/pinctrl/pinconf.h>
> @@ -120,4 +121,19 @@ void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
>         }
>  }
>
> +int pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
> +                                struct seq_file *s, unsigned long config)
> +{
> +       int i;
> +
> +       for(i = 0; i < ARRAY_SIZE(conf_items); i++) {
> +               if (pinconf_to_config_param(config) != conf_items[i].param)
> +                       continue;
> +               seq_printf(s, "%s: 0x%x", conf_items[i].display,
> +                          pinconf_to_config_argument(config));
> +       }
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(pinconf_generic_dump_config);
>  #endif
> diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
> index e3ed8cb..c91ea3e 100644
> --- a/drivers/pinctrl/pinconf.h
> +++ b/drivers/pinctrl/pinconf.h
> @@ -98,6 +98,8 @@ void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
>  void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
>                               struct seq_file *s, const char *gname);
>
> +int pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
> +                                struct seq_file *s, unsigned long config);
>  #else
>
>  static inline void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
> @@ -114,4 +116,10 @@ static inline void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
>         return;
>  }
>
> +static inline int pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
> +                                       struct seq_file *s,
> +                                       unsigned long config)
> +{
> +       return;
> +}
>  #endif

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-02 17:25 ` [PATCH v8 12/12] document: devicetree: bind pinconf with pin single Haojian Zhuang
@ 2013-02-05  4:07   ` Tony Lindgren
  2013-02-05  8:06     ` Haojian Zhuang
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2013-02-05  4:07 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:32]:
> +- pinctrl-single,drive-strength : array of value that are used to configure
> +  drive strength in the pinmux register. They're value of drive strength
> +  current and drive strength mask.
> +
> +		/* drive strength current, mask */
> +		pinctrl-single,power-source = <0x30 0xf0>;
> +

Looks like a typo, this should probably say pinctrl-single,drive-strength
here instead of power-source?

For the cases where there's a value to be set I'm still wondering how does
a client driver change the value if that needs to be changed dynamically?
Can the value be passed as value + PIN_CONFIG_END?

> +- pinctrl-single,bias-disable : array of value that are used to configure the
> +  input bias disabled in the pinmux register. They're value of bias value,
> +  match bias disabled value and bias disabled mask.
> +
> +		/* bias value, match bias disabled value, mask */
> +		pinctrl-single,bias-disable = <2 0 3>;
> +

I still suggest we use "enable" naming instead of "disable" naming.
So pinctrl-single,bias-enable instead of pinctrl-single,bias-disable.

Then let's change the binding slightly to make it more readable:

		/* enable value, disable value, mask */
		pinctrl-single,bias-enable = <2 0 3>;

For example, I have bit 22 cleared to enable MMC PBIAS, or bit 22
set to disable PBIAS:

		                             /* enable disable  mask */
		pinctrl-single,bias-enable = <0x000000 0x400000 0x400000>;

Then I think pretty much the only change to pinconf_get() would be:

	data = pcs->read(pcs->base + offset) & func->conf[i].mask;

	switch (func->conf[i].param) {
	/* 3 parameters */
	case PIN_CONFIG_BIAS_ENABLE:
	case PIN_CONFIG_BIAS_PULL_DOWN:
	case PIN_CONFIG_BIAS_PULL_UP:
	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
	*config = data;
	if (data == func->conf[i].enable)
		res = 0;
	else
		res = -ENOTSUPP;
	break;
	...

And then the client driver can always assume that if the return value
is 0, the ENABLE value in question is set. And if -ENOTSUPP (or how about
-ENOTSET :) is returned, the ENABLE value is not set?

Then regarding pinconf_set(), don't we also need pinconf_clear() in
addition to pinconf_set()?

Otherwise we need both ENABLE and DISABLE enumeration, which seems
unnecessary to me. So rather than have both PIN_CONFIG_BIAS_ENABLE and
PIN_CONFIG_BIAS_DISABLE, we just need PIN_CONFIG_BIAS_ENABLE.

> +- pinctrl-single,bias-pullup : array of value that are used to configure the
> +  input bias pullup in the pinmux register. They're value of bias value,
> +  match bias pullup value and bias pullup mask.
> +
> +		/* bias value, match bias pullup value, mask */
> +		pinctrl-single,bias-pullup = <0 1 1>;

It seems that you could then use same generic comment in all places
then to make it a bit easier to read:

		/* enable value, disable value, mask */
		pinctrl-single,bias-pullup = <0 1 1>;
...

Regards,

Tony

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-05  4:07   ` Tony Lindgren
@ 2013-02-05  8:06     ` Haojian Zhuang
  2013-02-05 13:51       ` Haojian Zhuang
  2013-02-05 23:21       ` Tony Lindgren
  0 siblings, 2 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-05  8:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 5, 2013 at 12:07 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:32]:
>> +- pinctrl-single,drive-strength : array of value that are used to configure
>> +  drive strength in the pinmux register. They're value of drive strength
>> +  current and drive strength mask.
>> +
>> +             /* drive strength current, mask */
>> +             pinctrl-single,power-source = <0x30 0xf0>;
>> +
>
> Looks like a typo, this should probably say pinctrl-single,drive-strength
> here instead of power-source?
>
Yes, I should update it to drive strength.

> For the cases where there's a value to be set I'm still wondering how does
> a client driver change the value if that needs to be changed dynamically?
> Can the value be passed as value + PIN_CONFIG_END?
>
pinconf_to_config_packed() can help to bind param & argument into config
variable.

>> +- pinctrl-single,bias-disable : array of value that are used to configure the
>> +  input bias disabled in the pinmux register. They're value of bias value,
>> +  match bias disabled value and bias disabled mask.
>> +
>> +             /* bias value, match bias disabled value, mask */
>> +             pinctrl-single,bias-disable = <2 0 3>;
>> +
>
> I still suggest we use "enable" naming instead of "disable" naming.
> So pinctrl-single,bias-enable instead of pinctrl-single,bias-disable.
>
> Then let's change the binding slightly to make it more readable:
>
>                 /* enable value, disable value, mask */
>                 pinctrl-single,bias-enable = <2 0 3>;
>
> For example, I have bit 22 cleared to enable MMC PBIAS, or bit 22
> set to disable PBIAS:
>
>                                              /* enable disable  mask */
>                 pinctrl-single,bias-enable = <0x000000 0x400000 0x400000>;
>
In Hi3620 SoC, the bias could only be configured pull up or pull down. There's
no any bit for controlling bias enable or bias disable. But user may need to
remove both pullup and pulldown at runtime. In this case, I set bias disable
if there's no pullup & pulldown.

* Without bias enable/disable bit.
            bias-disable = <user input value, expected disable value, mask>;
            bias-pullup = <user input value, expected pullup value, mask>;
      "expected disable value" is the set of not pullup bit and not
pull down bit.
      "expected pullup value" is the set of pullup bit.

* With bias enable/disable bit.
      "expected disable value" is the set of disable bit, NOT pullup
bit and NOT pulldown bit.
      "expected pullup value" is the set of NOT disable bit & pullup bit.

PULLUP --> DISABLE: only need to set the expected disable value in
bias-disable property.
DISABLE --> PULLUP: only need to set the expected pullup value in
bias-pullup property.
PULLUP --> PULLDOWN: set the expected pulldown value. So both pullup
and pulldown
bits are enabled unless user call DISABLE first.. Maybe it's not
right. I can fix it and keep
either pullup or pulldown enabled. It's ok to add pinconf_clear() for
bias as you mentioned.

And I think that we need four parameters with your format.
      <user setting on bias enable/disable, expected enable value,
expected disable value, mask */
Otherwise, how could we know user's setting? Especially, we need to
configure pullup.
In your case, expected pullup enable & expected pullup disable are set
in bias-pullup property.
>                 /* enable value, disable value, mask */
>                 pinctrl-single,bias-pullup = <0 1 1>;

Actually, I'm OK to use enable value. And it needs 4 parameters since
disable value may not be zero.

By the way, bias property would be the format in below with 4 parameters.
* Without bias enable/disable bit.
          bias-enable = <0, 0, 0, set of both pullup and pulldown mask>;

* With bias enable/disable bit.
          bias-enable = <user setting, expected enable, expected
disable, mask of only enable/disable bit>;

Behavior of pinconf_clear():
     - call bias-disable
     - call the expected bias setting.

DISABLE --> PULLUP, user call config BIAS_PULLUP without additional argument.
PULLUP --> PULLDOWN, user call with config BIAS_PULLDOWN without
additional argument.
PULLUP --> DISABLE, user call with config BIAS_ENABLE and 0 for argument.

The switch from PULLUP to DISABLE is a little strange since we have to
call BIAS_ENABLE.
Is it good for you?

> Then I think pretty much the only change to pinconf_get() would be:
>
>         data = pcs->read(pcs->base + offset) & func->conf[i].mask;
>
>         switch (func->conf[i].param) {
>         /* 3 parameters */
>         case PIN_CONFIG_BIAS_ENABLE:
>         case PIN_CONFIG_BIAS_PULL_DOWN:
>         case PIN_CONFIG_BIAS_PULL_UP:
>         case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
>         *config = data;
>         if (data == func->conf[i].enable)
>                 res = 0;
>         else
>                 res = -ENOTSUPP;
>         break;
>         ...
>
> And then the client driver can always assume that if the return value
> is 0, the ENABLE value in question is set. And if -ENOTSUPP (or how about
> -ENOTSET :) is returned, the ENABLE value is not set?
>
Return constant value 0 is better. I'll update.

> Then regarding pinconf_set(), don't we also need pinconf_clear() in
> addition to pinconf_set()?
>
> Otherwise we need both ENABLE and DISABLE enumeration, which seems
> unnecessary to me. So rather than have both PIN_CONFIG_BIAS_ENABLE and
> PIN_CONFIG_BIAS_DISABLE, we just need PIN_CONFIG_BIAS_ENABLE.
I can append pinconf_clear() to handle it.

>
>> +- pinctrl-single,bias-pullup : array of value that are used to configure the
>> +  input bias pullup in the pinmux register. They're value of bias value,
>> +  match bias pullup value and bias pullup mask.
>> +
>> +             /* bias value, match bias pullup value, mask */
>> +             pinctrl-single,bias-pullup = <0 1 1>;
>
> It seems that you could then use same generic comment in all places
> then to make it a bit easier to read:
>
>                 /* enable value, disable value, mask */
>                 pinctrl-single,bias-pullup = <0 1 1>;
Discussed above.

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-05  8:06     ` Haojian Zhuang
@ 2013-02-05 13:51       ` Haojian Zhuang
  2013-02-05 23:30         ` Tony Lindgren
  2013-02-05 23:21       ` Tony Lindgren
  1 sibling, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-05 13:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 5, 2013 at 4:06 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> On Tue, Feb 5, 2013 at 12:07 PM, Tony Lindgren <tony@atomide.com> wrote:
>> * Haojian Zhuang <haojian.zhuang@linaro.org> [130202 09:32]:
>>> +- pinctrl-single,drive-strength : array of value that are used to configure
>>> +  drive strength in the pinmux register. They're value of drive strength
>>> +  current and drive strength mask.
>>> +
>>> +             /* drive strength current, mask */
>>> +             pinctrl-single,power-source = <0x30 0xf0>;
>>> +
>>
>> Looks like a typo, this should probably say pinctrl-single,drive-strength
>> here instead of power-source?
>>
> Yes, I should update it to drive strength.
>
>> For the cases where there's a value to be set I'm still wondering how does
>> a client driver change the value if that needs to be changed dynamically?
>> Can the value be passed as value + PIN_CONFIG_END?
>>
> pinconf_to_config_packed() can help to bind param & argument into config
> variable.
>
>>> +- pinctrl-single,bias-disable : array of value that are used to configure the
>>> +  input bias disabled in the pinmux register. They're value of bias value,
>>> +  match bias disabled value and bias disabled mask.
>>> +
>>> +             /* bias value, match bias disabled value, mask */
>>> +             pinctrl-single,bias-disable = <2 0 3>;
>>> +
>>
>> I still suggest we use "enable" naming instead of "disable" naming.
>> So pinctrl-single,bias-enable instead of pinctrl-single,bias-disable.
>>
>> Then let's change the binding slightly to make it more readable:
>>
>>                 /* enable value, disable value, mask */
>>                 pinctrl-single,bias-enable = <2 0 3>;
>>
>> For example, I have bit 22 cleared to enable MMC PBIAS, or bit 22
>> set to disable PBIAS:
>>
>>                                              /* enable disable  mask */
>>                 pinctrl-single,bias-enable = <0x000000 0x400000 0x400000>;
>>
> In Hi3620 SoC, the bias could only be configured pull up or pull down. There's
> no any bit for controlling bias enable or bias disable. But user may need to
> remove both pullup and pulldown at runtime. In this case, I set bias disable
> if there's no pullup & pulldown.
>
> * Without bias enable/disable bit.
>             bias-disable = <user input value, expected disable value, mask>;
>             bias-pullup = <user input value, expected pullup value, mask>;
>       "expected disable value" is the set of not pullup bit and not
> pull down bit.
>       "expected pullup value" is the set of pullup bit.
>
> * With bias enable/disable bit.
>       "expected disable value" is the set of disable bit, NOT pullup
> bit and NOT pulldown bit.
>       "expected pullup value" is the set of NOT disable bit & pullup bit.
>
> PULLUP --> DISABLE: only need to set the expected disable value in
> bias-disable property.
> DISABLE --> PULLUP: only need to set the expected pullup value in
> bias-pullup property.
> PULLUP --> PULLDOWN: set the expected pulldown value. So both pullup
> and pulldown
> bits are enabled unless user call DISABLE first.. Maybe it's not
> right. I can fix it and keep
> either pullup or pulldown enabled. It's ok to add pinconf_clear() for
> bias as you mentioned.
>
> And I think that we need four parameters with your format.
>       <user setting on bias enable/disable, expected enable value,
> expected disable value, mask */
> Otherwise, how could we know user's setting? Especially, we need to
> configure pullup.
> In your case, expected pullup enable & expected pullup disable are set
> in bias-pullup property.
>>                 /* enable value, disable value, mask */
>>                 pinctrl-single,bias-pullup = <0 1 1>;
>
> Actually, I'm OK to use enable value. And it needs 4 parameters since
> disable value may not be zero.
>
> By the way, bias property would be the format in below with 4 parameters.
> * Without bias enable/disable bit.
>           bias-enable = <0, 0, 0, set of both pullup and pulldown mask>;
>
> * With bias enable/disable bit.
>           bias-enable = <user setting, expected enable, expected
> disable, mask of only enable/disable bit>;
>
> Behavior of pinconf_clear():
>      - call bias-disable
>      - call the expected bias setting.
>
> DISABLE --> PULLUP, user call config BIAS_PULLUP without additional argument.
> PULLUP --> PULLDOWN, user call with config BIAS_PULLDOWN without
> additional argument.
> PULLUP --> DISABLE, user call with config BIAS_ENABLE and 0 for argument.
>
> The switch from PULLUP to DISABLE is a little strange since we have to
> call BIAS_ENABLE.
> Is it good for you?
>

Maybe it didn't cover your case. It seems that there's only one bit
for bias in your case.
You can control MMC BIAS enable or disable. And there's neither pullup
nor pulldown.
Is it right?

If so, how about this?

* Without pullup & pulldown bias. With bias enable & disable bit.
(Your MMC bias)
     1) 3 params
           bias-disable = <user setting of bias value, expected bias
disable value, mask>;
           bias-enable = <user setting of bias value, expected bias
enable value, mask>;
     or 2) 4 params
           bias-enable = <user setting of bias value, expected bias
enable value, expected bias disable value, mask>;

     So I will need both config BIAS_ENABLE & config BIAS_DISABLE.
Driver switches between BIAS_ENABLE & BIAS_DISABLE.

* Without bias enable & disable bit. With bias pullup & pulldown bit.
(Hi3620 bias)
     1) 3 params
           bias-disable = <user setting of bias value, expected
disable value (w/o pullup & pulldown), mask (pullup & pulldown)>;
           bias-pullup = <user setting of bias value, expected bias
pullup value, mask of pullup>;
           bias-pulldown ...
     or 2) 4 params
           bias-enable = <user setting of bias value, expected enable
value (w pullup & pulldown), expected disable value (w/o pullup &
pulldown), mask (pullup & pulldown)>;
           bias-pullup = <user setting of bias value, expected bias
pullup value, expected bias pullup disable value, mask of pullup>;
           bias-pulldown ...
     Driver switches among BIAS_DISABLE, BIAS_PULLDOWN, BIAS_PULLUP.
It seems that BIAS_ENABLE is redundant operation at here.

* With bias enable & disable bit. With bias pullup & pulldown bit.
(MMP/PXA bias)
     1) 3 params
          bias-disable = <user setting of bias value, expected disable
value (single bit), mask (enable/disable)>;
          bias-pullup = <user setting of bias value, expected pullup
value, mask (pullup)>;
          bias-pulldown ...
     or 2) 4 params
          bias-enable = <user setting of bias value, expected enable
value, expected disable value, mask>;
          bias-pullup = <user setting of bias value, expected bias
pullup value, expected bias pullup disable value, mask of pullup>;
          bias-pulldown ...
     Driver switches among BIAS_DISABLE, BIAS_PULLDOWN, BIAS_PULLUP.
It seems that BIAS_ENABLE is redundant operation at here. Because user
won't only enable bias without pullup or pulldown.

There're two usage for all three cases. Which one do you like?

Best Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 02/12] gpio: fix wrong checking condition for gpio range
  2013-02-02 17:25 ` [PATCH v8 02/12] gpio: fix wrong checking condition for gpio range Haojian Zhuang
@ 2013-02-05 17:02   ` Linus Walleij
  0 siblings, 0 replies; 42+ messages in thread
From: Linus Walleij @ 2013-02-05 17:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 2, 2013 at 6:25 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> If index++ calculates from 0, the checking condition of "while
> (index++)" fails & it doesn't check any more. It doesn't follow
> the loop that used at here.
>
> Replace it by endless loop at here. Then it keeps parsing
> "gpio-ranges" property until it ends.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 03/12] gpio: find gpio base by ascend order
  2013-02-02 17:25 ` [PATCH v8 03/12] gpio: find gpio base by ascend order Haojian Zhuang
@ 2013-02-05 17:14   ` Linus Walleij
  2013-02-06  1:59     ` Haojian Zhuang
  0 siblings, 1 reply; 42+ messages in thread
From: Linus Walleij @ 2013-02-05 17:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 2, 2013 at 6:25 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> gpiochip_find_base() always tries to find valid gpio with descend order.
> It's inconvient if gpio information is passing from DTS. Now try to find
> valid gpio with ascend order.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

This is more scary stuff.

As you know GPIO numbers are exposed to userspace.

Systems with this change risk having their dynamically added
GPIO controller enumerated in a different fashion. And
userspace clients may be relying on these numbers.

And we do not break userspace.

I know this is not elegant but I'm afraid the descending search
needs to be kept for compatibibility reasons.

BTW: please CC Grant likely on all patches.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-05  8:06     ` Haojian Zhuang
  2013-02-05 13:51       ` Haojian Zhuang
@ 2013-02-05 23:21       ` Tony Lindgren
  1 sibling, 0 replies; 42+ messages in thread
From: Tony Lindgren @ 2013-02-05 23:21 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@gmail.com> [130205 00:09]:
> On Tue, Feb 5, 2013 at 12:07 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > For the cases where there's a value to be set I'm still wondering how does
> > a client driver change the value if that needs to be changed dynamically?
> > Can the value be passed as value + PIN_CONFIG_END?
> >
> pinconf_to_config_packed() can help to bind param & argument into config
> variable.

OK good to hear you've looked into that.
...
 
> And I think that we need four parameters with your format.
>       <user setting on bias enable/disable, expected enable value,
> expected disable value, mask */
> Otherwise, how could we know user's setting? Especially, we need to
> configure pullup.
> In your case, expected pullup enable & expected pullup disable are set
> in bias-pullup property.
> >                 /* enable value, disable value, mask */
> >                 pinctrl-single,bias-pullup = <0 1 1>;
> 
> Actually, I'm OK to use enable value. And it needs 4 parameters since
> disable value may not be zero.

OK. Four values makes sense to me so we can configure the default
pinconf value like we already do for pinmux.
 
> By the way, bias property would be the format in below with 4 parameters.
> * Without bias enable/disable bit.
>           bias-enable = <0, 0, 0, set of both pullup and pulldown mask>;
> 
> * With bias enable/disable bit.
>           bias-enable = <user setting, expected enable, expected
> disable, mask of only enable/disable bit>;
> 
> Behavior of pinconf_clear():
>      - call bias-disable
>      - call the expected bias setting.

Hmm I was thinking pinconf_clear() would get passed the same parameters
as pinconf_set() except it just does the opposite. But maybe I'm not
following you here.
 
> DISABLE --> PULLUP, user call config BIAS_PULLUP without additional argument.
> PULLUP --> PULLDOWN, user call with config BIAS_PULLDOWN without
> additional argument.
> PULLUP --> DISABLE, user call with config BIAS_ENABLE and 0 for argument.
> 
> The switch from PULLUP to DISABLE is a little strange since we have to
> call BIAS_ENABLE.
> Is it good for you?

I'm not sure I follow your example here, but sounds like the PULLUP to
DISABLE case can be maybe handled by covering both bits in the mask?
 
> > Then regarding pinconf_set(), don't we also need pinconf_clear() in
> > addition to pinconf_set()?
> >
> > Otherwise we need both ENABLE and DISABLE enumeration, which seems
> > unnecessary to me. So rather than have both PIN_CONFIG_BIAS_ENABLE and
> > PIN_CONFIG_BIAS_DISABLE, we just need PIN_CONFIG_BIAS_ENABLE.
> I can append pinconf_clear() to handle it.
 
OK great, I guess then we should have everything in place to make it
usable from client drivers also.

Regards,

Tony

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-05 13:51       ` Haojian Zhuang
@ 2013-02-05 23:30         ` Tony Lindgren
  2013-02-06 15:07           ` Haojian Zhuang
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2013-02-05 23:30 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@gmail.com> [130205 05:55]:
> 
> Maybe it didn't cover your case. It seems that there's only one bit
> for bias in your case.
> You can control MMC BIAS enable or disable. And there's neither pullup
> nor pulldown.
> Is it right?

Yes for this register there are no pulls. But I there are others that
have pull bits.

>      So I will need both config BIAS_ENABLE & config BIAS_DISABLE.
> Driver switches between BIAS_ENABLE & BIAS_DISABLE.

I'd like to avoid mapping both enable and disable for everything
which seems bloated..
 
>      or 2) 4 params
>            bias-enable = <user setting of bias value, expected enable
> value (w pullup & pulldown), expected disable value (w/o pullup &
> pulldown), mask (pullup & pulldown)>;
>            bias-pullup = <user setting of bias value, expected bias
> pullup value, expected bias pullup disable value, mask of pullup>;
>            bias-pulldown ...
>      Driver switches among BIAS_DISABLE, BIAS_PULLDOWN, BIAS_PULLUP.
> It seems that BIAS_ENABLE is redundant operation at here.

..so this 4 params option makes sense to me assuming it works
for your case.
 
> There're two usage for all three cases. Which one do you like?

It seems that the 4 params option is most generic to me.

Regards,

Tony

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 03/12] gpio: find gpio base by ascend order
  2013-02-05 17:14   ` Linus Walleij
@ 2013-02-06  1:59     ` Haojian Zhuang
  2013-02-06  4:33       ` Alex Courbot
  2013-02-06  8:44       ` Linus Walleij
  0 siblings, 2 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-06  1:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 February 2013 01:14, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Feb 2, 2013 at 6:25 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>
>> gpiochip_find_base() always tries to find valid gpio with descend order.
>> It's inconvient if gpio information is passing from DTS. Now try to find
>> valid gpio with ascend order.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>
> This is more scary stuff.
>
> As you know GPIO numbers are exposed to userspace.
>
> Systems with this change risk having their dynamically added
> GPIO controller enumerated in a different fashion. And
> userspace clients may be relying on these numbers.
>
> And we do not break userspace.
>
> I know this is not elegant but I'm afraid the descending search
> needs to be kept for compatibibility reasons.
>
> BTW: please CC Grant likely on all patches.
>
> Yours,
> Linus Walleij

But descending search isn't good for reading.

I try to allocate all gpio numbers in Hi3620 from gpiochip_find_base().
If it's descending search, GPIO0~7 is mapped to gpio248~255;
GPIO8~GPIO15 is mapped to gpio240~gpio247. It's not easy to read,
and it breaks the knowledge of gpio number on schematic & datasheet.

Unless we don't use allocating gpio numbers dynamically and add
a common property to parse gpio base of each chip in DTS file.
It's also OK to me add a common property.

Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 03/12] gpio: find gpio base by ascend order
  2013-02-06  1:59     ` Haojian Zhuang
@ 2013-02-06  4:33       ` Alex Courbot
  2013-02-06  5:20         ` Haojian Zhuang
  2013-02-06  8:44       ` Linus Walleij
  1 sibling, 1 reply; 42+ messages in thread
From: Alex Courbot @ 2013-02-06  4:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/06/2013 10:59 AM, Haojian Zhuang wrote:
> On 6 February 2013 01:14, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sat, Feb 2, 2013 at 6:25 PM, Haojian Zhuang
>> <haojian.zhuang@linaro.org> wrote:
>>
>>> gpiochip_find_base() always tries to find valid gpio with descend order.
>>> It's inconvient if gpio information is passing from DTS. Now try to find
>>> valid gpio with ascend order.

GPIOs in the device tree are typically referenced by a phandle which 
include the controller and relative HW number. Are there cases where we 
must use absolute GPIO numbers in the DT?

>>>
>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>
>> This is more scary stuff.
>>
>> As you know GPIO numbers are exposed to userspace.
>>
>> Systems with this change risk having their dynamically added
>> GPIO controller enumerated in a different fashion. And
>> userspace clients may be relying on these numbers.
>>
>> And we do not break userspace.
>>
>> I know this is not elegant but I'm afraid the descending search
>> needs to be kept for compatibibility reasons.
>>
>> BTW: please CC Grant likely on all patches.
>>
>> Yours,
>> Linus Walleij
>
> But descending search isn't good for reading.
>
> I try to allocate all gpio numbers in Hi3620 from gpiochip_find_base().
> If it's descending search, GPIO0~7 is mapped to gpio248~255;
> GPIO8~GPIO15 is mapped to gpio240~gpio247. It's not easy to read,
> and it breaks the knowledge of gpio number on schematic & datasheet.
>
> Unless we don't use allocating gpio numbers dynamically and add
> a common property to parse gpio base of each chip in DTS file.
> It's also OK to me add a common property.

There is also one more problem with this reordering: if a GPIO chip with 
a base GPIO set gets probed *after* a bunch of chips without a proper 
base, its range in the number space is likely to have been stolen by one 
of the "dynamic" chips.

Ideally all chips would come with a base GPIO, but we cannot rule out 
hotplugable interfaces. Even more ideally the integer numberspace would 
go away altogether with the new gpiod interface and the sysfs interface 
would be replaced with one where exported GPIOs would be under their 
chip node, and referenced by their hw number. But that would break 
userspace even more. Or maybe there could be a config option to choose 
between the "legacy" integer-space user interface and this new scheme. 
Eventually, the number space could be deprecated.

Alex.

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 03/12] gpio: find gpio base by ascend order
  2013-02-06  4:33       ` Alex Courbot
@ 2013-02-06  5:20         ` Haojian Zhuang
  0 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-06  5:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 February 2013 12:33, Alex Courbot <acourbot@nvidia.com> wrote:
> On 02/06/2013 10:59 AM, Haojian Zhuang wrote:
>>
>> On 6 February 2013 01:14, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>
>>> On Sat, Feb 2, 2013 at 6:25 PM, Haojian Zhuang
>>> <haojian.zhuang@linaro.org> wrote:
>>>
>>>> gpiochip_find_base() always tries to find valid gpio with descend order.
>>>> It's inconvient if gpio information is passing from DTS. Now try to find
>>>> valid gpio with ascend order.
>
>
> GPIOs in the device tree are typically referenced by a phandle which include
> the controller and relative HW number. Are there cases where we must use
> absolute GPIO numbers in the DT?
>

At first, you only consider SoC GPIO pins are in the same GPIO controllers.
ARM pl061 is a gpio controller with only 8 GPIO pins. So there're a lot of
pl061 controllers in one SoC.

In the second, all GPIO pins in one SoC are counter by ascending
order. It's also
same in the schematic. And there's sysfs interface for GPIO debugging in kernel.
With the descending order, gpio248 is real GPIO0 in SoC. It's crazy for develop
debugging.

>
>>>>
>>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>>
>>>
>>> This is more scary stuff.
>>>
>>> As you know GPIO numbers are exposed to userspace.
>>>
>>> Systems with this change risk having their dynamically added
>>> GPIO controller enumerated in a different fashion. And
>>> userspace clients may be relying on these numbers.
>>>
>>> And we do not break userspace.
>>>
>>> I know this is not elegant but I'm afraid the descending search
>>> needs to be kept for compatibibility reasons.
>>>
>>> BTW: please CC Grant likely on all patches.
>>>
>>> Yours,
>>> Linus Walleij
>>
>>
>> But descending search isn't good for reading.
>>
>> I try to allocate all gpio numbers in Hi3620 from gpiochip_find_base().
>> If it's descending search, GPIO0~7 is mapped to gpio248~255;
>> GPIO8~GPIO15 is mapped to gpio240~gpio247. It's not easy to read,
>> and it breaks the knowledge of gpio number on schematic & datasheet.
>>
>> Unless we don't use allocating gpio numbers dynamically and add
>> a common property to parse gpio base of each chip in DTS file.
>> It's also OK to me add a common property.
>
>
> There is also one more problem with this reordering: if a GPIO chip with a
> base GPIO set gets probed *after* a bunch of chips without a proper base,
> its range in the number space is likely to have been stolen by one of the
> "dynamic" chips.
>
> Ideally all chips would come with a base GPIO, but we cannot rule out
> hotplugable interfaces. Even more ideally the integer numberspace would go
> away altogether with the new gpiod interface and the sysfs interface would
> be replaced with one where exported GPIOs would be under their chip node,
> and referenced by their hw number. But that would break userspace even more.
> Or maybe there could be a config option to choose between the "legacy"
> integer-space user interface and this new scheme. Eventually, the number
> space could be deprecated.
>
> Alex.
>

How about adding one property on declaring that the GPIO number space is in
ascending order? Then it won't break the "legacy" decreasing order.

Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 03/12] gpio: find gpio base by ascend order
  2013-02-06  1:59     ` Haojian Zhuang
  2013-02-06  4:33       ` Alex Courbot
@ 2013-02-06  8:44       ` Linus Walleij
  2013-02-06  9:15         ` Haojian Zhuang
  1 sibling, 1 reply; 42+ messages in thread
From: Linus Walleij @ 2013-02-06  8:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 6, 2013 at 2:59 AM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:
> On 6 February 2013 01:14, Linus Walleij <linus.walleij@linaro.org> wrote:

>> This is more scary stuff.
>>
>> As you know GPIO numbers are exposed to userspace.
>>
>> Systems with this change risk having their dynamically added
>> GPIO controller enumerated in a different fashion. And
>> userspace clients may be relying on these numbers.
>>
>> And we do not break userspace.
>>
>> I know this is not elegant but I'm afraid the descending search
>> needs to be kept for compatibibility reasons.
>>
>> BTW: please CC Grant likely on all patches.
>>
>> Yours,
>> Linus Walleij
>
> But descending search isn't good for reading.

But you may be breaking userspace.

When I, as a subsystem maintainer merge a patch that break
userspace interfaces, things like this happen:

https://lkml.org/lkml/2012/12/23/75
http://developers.slashdot.org/story/12/12/29/018234/linus-chews-up-kernel-maintainer-for-introducing-userspace-bug

You can argue all you want about wanting to change things
that affect userspace for internal kernel refactoring or fit
with device tree or whatever, it's just not going to happen,
because the Big Penguin has installed a culture of fear
around breaking userspace.

If you want the policy changed you can talk to Torvalds.

> I try to allocate all gpio numbers in Hi3620 from gpiochip_find_base().
> If it's descending search, GPIO0~7 is mapped to gpio248~255;
> GPIO8~GPIO15 is mapped to gpio240~gpio247. It's not easy to read,
> and it breaks the knowledge of gpio number on schematic & datasheet.

It may make things elegant and nice on your (new) system but
break everyone else's, and they were first in the kernel, they may
have userspace clients and so, we cannot change this.

> Unless we don't use allocating gpio numbers dynamically and add
> a common property to parse gpio base of each chip in DTS file.
> It's also OK to me add a common property.

As explained elsewhere, global GPIO numbers don't belong
in the device tree, as it is a Linux-specific pecularity.
If this approach was chosen anyway, it would be named
something like linux,gpio-base-offset

One compromise would be to add global setting like
gpio_add_dynamic_gpios_ascendingly() that will change
the behaviour on a *specific* system, or maybe on all
device tree systems, and keep both code paths.
Yes, it is ugly and unelegant, but with the userspace
contract, what can we do? We do all sort of ugliness
for userspace.

After reading this you may be on the clear why I am so
hesitant about Roland Stigge's blocked GPIOs as well,
that will become one more userspace ABI set in stone
FOREVER.

I'd like Grant's input on this... he has the big view on
GPIO plus device tree.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 03/12] gpio: find gpio base by ascend order
  2013-02-06  8:44       ` Linus Walleij
@ 2013-02-06  9:15         ` Haojian Zhuang
  0 siblings, 0 replies; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-06  9:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 February 2013 16:44, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Feb 6, 2013 at 2:59 AM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>> On 6 February 2013 01:14, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>>> This is more scary stuff.
>>>
>>> As you know GPIO numbers are exposed to userspace.
>>>
>>> Systems with this change risk having their dynamically added
>>> GPIO controller enumerated in a different fashion. And
>>> userspace clients may be relying on these numbers.
>>>
>>> And we do not break userspace.
>>>
>>> I know this is not elegant but I'm afraid the descending search
>>> needs to be kept for compatibibility reasons.
>>>
>>> BTW: please CC Grant likely on all patches.
>>>
>>> Yours,
>>> Linus Walleij
>>
>> But descending search isn't good for reading.
>
> But you may be breaking userspace.
>
> When I, as a subsystem maintainer merge a patch that break
> userspace interfaces, things like this happen:
>
> https://lkml.org/lkml/2012/12/23/75
> http://developers.slashdot.org/story/12/12/29/018234/linus-chews-up-kernel-maintainer-for-introducing-userspace-bug
>
> You can argue all you want about wanting to change things
> that affect userspace for internal kernel refactoring or fit
> with device tree or whatever, it's just not going to happen,
> because the Big Penguin has installed a culture of fear
> around breaking userspace.
>
> If you want the policy changed you can talk to Torvalds.
>
>> I try to allocate all gpio numbers in Hi3620 from gpiochip_find_base().
>> If it's descending search, GPIO0~7 is mapped to gpio248~255;
>> GPIO8~GPIO15 is mapped to gpio240~gpio247. It's not easy to read,
>> and it breaks the knowledge of gpio number on schematic & datasheet.
>
> It may make things elegant and nice on your (new) system but
> break everyone else's, and they were first in the kernel, they may
> have userspace clients and so, we cannot change this.
>
>> Unless we don't use allocating gpio numbers dynamically and add
>> a common property to parse gpio base of each chip in DTS file.
>> It's also OK to me add a common property.
>
> As explained elsewhere, global GPIO numbers don't belong
> in the device tree, as it is a Linux-specific pecularity.
> If this approach was chosen anyway, it would be named
> something like linux,gpio-base-offset
>
> One compromise would be to add global setting like
> gpio_add_dynamic_gpios_ascendingly() that will change
> the behaviour on a *specific* system, or maybe on all
> device tree systems, and keep both code paths.
> Yes, it is ugly and unelegant, but with the userspace
> contract, what can we do? We do all sort of ugliness
> for userspace.
>
> After reading this you may be on the clear why I am so
> hesitant about Roland Stigge's blocked GPIOs as well,
> that will become one more userspace ABI set in stone
> FOREVER.
>
> I'd like Grant's input on this... he has the big view on
> GPIO plus device tree.
>
> Yours,
> Linus Walleij

Since it may break the userspace ABI, I agree that we shouldn't
change current solution. Thanks for your kindly illustration.

In this patch series, I'll initialize pdata->gpio_base first and use
aux structure in machine driver.

Then I'll try to something like "linux,gpio-base-offset" in GPIO
system, and drop aux structure from machine driver. Since I'm
expecting GPIO/PINCTRL could be similar as IRQ that everything
could be parsed from DT, it could make driver simpler.

Best Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-05 23:30         ` Tony Lindgren
@ 2013-02-06 15:07           ` Haojian Zhuang
  2013-02-06 17:16             ` Tony Lindgren
  0 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-06 15:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 February 2013 07:30, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@gmail.com> [130205 05:55]:
>>
>> Maybe it didn't cover your case. It seems that there's only one bit
>> for bias in your case.
>> You can control MMC BIAS enable or disable. And there's neither pullup
>> nor pulldown.
>> Is it right?
>
> Yes for this register there are no pulls. But I there are others that
> have pull bits.
>
>>      So I will need both config BIAS_ENABLE & config BIAS_DISABLE.
>> Driver switches between BIAS_ENABLE & BIAS_DISABLE.
>
> I'd like to avoid mapping both enable and disable for everything
> which seems bloated..
>

pinctrl-coh901 driver already uses PIN_CONFIG_BIAS_DISABLE without argument.

I think that it's better to use both BIAS_ENABLE & BIAS_DISABLE. Otherwise,
I need to update pinctrl-coh901 driver with updating the interface.
The work is a little
large.

Maybe we can use another name, PIN_CONFIG_BIAS_AUTOPULL. Then we can
distinguish it from PULL_UP & PULL_DOWN without any confusion. What's your
opinion?

Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-06 15:07           ` Haojian Zhuang
@ 2013-02-06 17:16             ` Tony Lindgren
  0 siblings, 0 replies; 42+ messages in thread
From: Tony Lindgren @ 2013-02-06 17:16 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@linaro.org> [130206 07:11]:
> On 6 February 2013 07:30, Tony Lindgren <tony@atomide.com> wrote:
> > * Haojian Zhuang <haojian.zhuang@gmail.com> [130205 05:55]:
> >>
> >> Maybe it didn't cover your case. It seems that there's only one bit
> >> for bias in your case.
> >> You can control MMC BIAS enable or disable. And there's neither pullup
> >> nor pulldown.
> >> Is it right?
> >
> > Yes for this register there are no pulls. But I there are others that
> > have pull bits.
> >
> >>      So I will need both config BIAS_ENABLE & config BIAS_DISABLE.
> >> Driver switches between BIAS_ENABLE & BIAS_DISABLE.
> >
> > I'd like to avoid mapping both enable and disable for everything
> > which seems bloated..
> >
> 
> pinctrl-coh901 driver already uses PIN_CONFIG_BIAS_DISABLE without argument.
> 
> I think that it's better to use both BIAS_ENABLE & BIAS_DISABLE. Otherwise,
> I need to update pinctrl-coh901 driver with updating the interface.
> The work is a little
> large.

Well how about just let's keep the existing PIN_CONFIG_BIAS_DISABLE
and PIN_CONFIG_BIAS_ENABLE for compability, but let's still standardize
on PIN_CONFIG_XXX_ENABLE for all the new ones to avoid bloat?

> Maybe we can use another name, PIN_CONFIG_BIAS_AUTOPULL. Then we can
> distinguish it from PULL_UP & PULL_DOWN without any confusion. What's your
> opinion?

Sounds OK to me if you need that.

Regards,

Tony

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property
  2013-02-02 17:25 ` [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property Haojian Zhuang
  2013-02-05  0:23   ` Tony Lindgren
@ 2013-02-10 19:03   ` Linus Walleij
  2013-02-11  4:25     ` Viresh Kumar
  2013-04-29 16:00   ` [v8,01/12] " James Hogan
  2 siblings, 1 reply; 42+ messages in thread
From: Linus Walleij @ 2013-02-10 19:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 2, 2013 at 6:25 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> Add gpio offset into "gpio-range-cells" property. It's used to support
> sparse pinctrl range in gpio chip.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

Can't decide on this thing, no reply from Shiraz.

Viresh, what is your opinion?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property
  2013-02-10 19:03   ` Linus Walleij
@ 2013-02-11  4:25     ` Viresh Kumar
  0 siblings, 0 replies; 42+ messages in thread
From: Viresh Kumar @ 2013-02-11  4:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 11, 2013 at 12:33 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Sat, Feb 2, 2013 at 6:25 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>
>> Add gpio offset into "gpio-range-cells" property. It's used to support
>> sparse pinctrl range in gpio chip.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>
> Can't decide on this thing, no reply from Shiraz.
>
> Viresh, what is your opinion?

Sorry for missing this thread earlier. Looks fine to me:

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [v8,01/12] gpio: add gpio offset in gpio range cells property
  2013-02-02 17:25 ` [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property Haojian Zhuang
  2013-02-05  0:23   ` Tony Lindgren
  2013-02-10 19:03   ` Linus Walleij
@ 2013-04-29 16:00   ` James Hogan
  2013-04-29 16:49     ` Haojian Zhuang
  2 siblings, 1 reply; 42+ messages in thread
From: James Hogan @ 2013-04-29 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/02/13 17:25, Haojian Zhuang wrote:
> Add gpio offset into "gpio-range-cells" property. It's used to support
> sparse pinctrl range in gpio chip.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

This is an ABI breakage. I've been using the #gpio-range-cells = <2>
since around October. Please can we try and maintain backward
compatibility in future, even if it's only temporary.

Thanks
James

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [v8,01/12] gpio: add gpio offset in gpio range cells property
  2013-04-29 16:00   ` [v8,01/12] " James Hogan
@ 2013-04-29 16:49     ` Haojian Zhuang
  2013-04-29 20:16       ` James Hogan
  0 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-04-29 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 30 April 2013 00:00, James Hogan <james.hogan@imgtec.com> wrote:
> On 02/02/13 17:25, Haojian Zhuang wrote:
>> Add gpio offset into "gpio-range-cells" property. It's used to support
>> sparse pinctrl range in gpio chip.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> This is an ABI breakage. I've been using the #gpio-range-cells = <2>
> since around October. Please can we try and maintain backward
> compatibility in future, even if it's only temporary.
>
> Thanks
> James
>

I've updated all code with #gpio-range-cells = <3> in kernel. If you change this
back to <2>, you'll break current pinctrl-single driver. I appended this because
there may be not 1-to-1 mapping between gpio pins & pinmux pins in some
SoC. So the new parameter is used to specify the gpio pin offset.

So it's not temporary.

Regards
Haojian

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [v8,01/12] gpio: add gpio offset in gpio range cells property
  2013-04-29 16:49     ` Haojian Zhuang
@ 2013-04-29 20:16       ` James Hogan
  0 siblings, 0 replies; 42+ messages in thread
From: James Hogan @ 2013-04-29 20:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 29/04/13 17:49, Haojian Zhuang wrote:
> On 30 April 2013 00:00, James Hogan <james.hogan@imgtec.com> wrote:
>> On 02/02/13 17:25, Haojian Zhuang wrote:
>>> Add gpio offset into "gpio-range-cells" property. It's used to support
>>> sparse pinctrl range in gpio chip.
>>>
>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>
>> This is an ABI breakage. I've been using the #gpio-range-cells = <2>
>> since around October. Please can we try and maintain backward
>> compatibility in future, even if it's only temporary.
>>
>> Thanks
>> James
>>
> 
> I've updated all code with #gpio-range-cells = <3> in kernel. If you change this
> back to <2>, you'll break current pinctrl-single driver. I appended this because
> there may be not 1-to-1 mapping between gpio pins & pinmux pins in some
> SoC. So the new parameter is used to specify the gpio pin offset.

Yes, I agree having 3 cells is useful, and I wasn't suggesting reverting
your patch, but it's an ABI now so the change should really be done in a
backwards compatible way so that device tree files that still have
#gpio-range-cells = <2> continue to work.

At the moment in -next, if you use an old device tree then the gpio
ranges are muddled up, with npins set apparently to a random (in my case
very large) number, I'm guessing this is just uninitialised data at the
end of the array. Even if you didn't have to maintain backwards
compatibility, it should at least check the number of cells matches what
it expects before reading all 3 entries.

> 
> So it's not temporary.

Sorry, what I meant was "even if maintaining the backwards compatibility
(supporting both <2> and <3>) is only temporary".

Cheers
James

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-11 17:10 ` [PATCH v8 12/12] document: devicetree: bind pinconf with pin single Haojian Zhuang
@ 2013-02-13 23:51   ` Tony Lindgren
  0 siblings, 0 replies; 42+ messages in thread
From: Tony Lindgren @ 2013-02-13 23:51 UTC (permalink / raw)
  To: linux-arm-kernel

* Haojian Zhuang <haojian.zhuang@linaro.org> [130211 09:16]:
> From: Haojian Zhuang <haojian.zhuang@gmail.com>
> 
> Add comments with pinconf & gpio range in the document of
> pinctrl-single.

Other than leaving out BIAS_AUTO_PULL, looks good to me:

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

^ permalink raw reply	[flat|nested] 42+ messages in thread
* [PATCH v8 12/12] document: devicetree: bind pinconf with pin single
  2013-02-11 17:10 [PATCH v8 00/12] bind pinconf with pinctrl single Haojian Zhuang
@ 2013-02-11 17:10 ` Haojian Zhuang
  2013-02-13 23:51   ` Tony Lindgren
  0 siblings, 1 reply; 42+ messages in thread
From: Haojian Zhuang @ 2013-02-11 17:10 UTC (permalink / raw)
  To: linux-arm-kernel

From: Haojian Zhuang <haojian.zhuang@gmail.com>

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 |  118 +++++++++++++++++++-
 1 file changed, 117 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
index 2c81e45..ac63d87 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,72 @@ 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,drive-strength : array of value that are used to configure
+  drive strength in the pinmux register. They're value of drive strength
+  current and drive strength mask.
+
+		/* drive strength current, mask */
+		pinctrl-single,power-source = <0x30 0xf0>;
+
+- pinctrl-single,bias-pullup : array of value that are used to configure the
+  input bias pullup in the pinmux register.
+
+		/* input, enabled pullup bits, disabled pullup bits, mask */
+		pinctrl-single,bias-pullup = <0 1 0 1>;
+
+- pinctrl-single,bias-pulldown : array of value that are used to configure the
+  input bias pulldown in the pinmux register.
+
+		/* input, enabled pulldown bits, disabled pulldown bits, mask */
+		pinctrl-single,bias-pulldown = <2 2 0 2>;
+
+- pinctrl-single,bias-autopull : array of value that are used to configure the
+  input bias autopull.
+
+		/* input, enabled autopull bits, disabled autopull bits, mask */
+		pinctrl-single,bias-autopull = <1 1 0 1>;
+
+  bias-pullup & bias-pulldown should be used together without bias-autoupll,
+  vice visa.
+
+  * Only one bit to control input bias enable or disable: User should use
+    pinctrl-single,bias-autoupll property at here.
+  * Two bits to control input bias pullup and pulldown: User should use
+    pinctrl-single,bias-pullup & pinctrl-single,bias-pulldown. One bit means
+    pullup, and the other one bit means pulldown.
+  * Three bits to control input bias enable, pullup and pulldown. User should
+    use pinctrl-single,bias-pullup & pinctrl-single,bias-pulldown. Input bias
+    enable bit should be included in pullup or pulldown bits.
+  * Although driver could set PIN_CONFIG_BIAS_DISABLE, there's no property as
+    pinctrl-single,bias-disable. Because pinctrl single driver could implement
+    it by calling pulldown, pullup, autopull disabled.
+
+- pinctrl-single,input-schmitt : array of value that are used to configure
+  input schmitt in the pinmux register. In some silicons, there're two input
+  schmitt value (rising-edge & falling-edge) in the pinmux register.
+
+		/* input schmitt value, mask */
+		pinctrl-single,input-schmitt = <0x30 0x70>;
+
+- pinctrl-single,input-schmitt-enable : array of value that are used to
+  configure input schmitt enable or disable in the pinmux register.
+
+		/* input, enable bits, disable bits, mask */
+		pinctrl-single,input-schmitt-enable = <0x30 0x40 0 0x70>;
+
+- pinctrl-single,gpio-range : list of value that are used to configure a GPIO
+  range. They're value of subnode phandle, pin base in pinctrl device, pin
+  number in this range, GPIO function value of this GPIO range.
+  The number of parameters is depend on #pinctrl-single,gpio-range-cells
+  property.
+
+		/* pin base, nr pins & gpio function */
+		pinctrl-single,gpio-range = <&range 0 3 0 &range 3 9 1>;
+
 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 +107,20 @@ 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. This sub-node is required
+by pinctrl-single,gpio-range property.
+
+Required properties in sub-node:
+- #pinctrl-single,gpio-range-cells : the number of parameters after phandle in
+  pinctrl-single,gpio-range property.
+
+	range: gpio-range {
+		#pinctrl-single,gpio-range-cells = <3>;
+	};
+
+
 Example:
 
 /* SoC common file */
@@ -76,6 +155,29 @@ 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>;
+
+	/* sparse GPIO range could be supported */
+	pinctrl-single,gpio-range = <&range 0 3 0 &range 3 9 1
+				&range 12 1 0 &range 13 29 1
+				&range 43 1 0 &range 44 49 1
+				&range 94 1 1 &range 96 2 1>;
+
+	range: gpio-range {
+		#pinctrl-single,gpio-range-cells = <3>;
+	};
+};
+
+
 /* board specific .dts file */
 
 &pmx_core {
@@ -96,6 +198,15 @@ control_devconf0: pinmux at 48002274 {
 		>;
 	};
 
+	uart0_pins: pinmux_uart0_pins {
+		pinctrl-single,pins = <
+			0x208 0		/* UART0_RXD (IOCFG138) */
+			0x20c 0		/* UART0_TXD (IOCFG139) */
+		>;
+		pinctrl-single,bias-pulldown = <0 2 2>;
+		pinctrl-single,bias-pullup = <0 1 1>;
+	};
+
 	/* map uart2 pins */
 	uart2_pins: pinmux_uart2_pins {
 		pinctrl-single,pins = <
@@ -122,6 +233,11 @@ control_devconf0: pinmux@48002274 {
 
 };
 
+&uart1 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart0_pins>;
+};
+
 &uart2 {
        pinctrl-names = "default";
        pinctrl-0 = <&uart2_pins>;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread
end of thread, other threads:[~2013-04-29 20:16 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-02 17:25 [PATCH v8 00/12] support pinconf in pinctrl single Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 01/12] gpio: add gpio offset in gpio range cells property Haojian Zhuang
2013-02-05  0:23   ` Tony Lindgren
2013-02-05  1:06     ` Haojian Zhuang
2013-02-10 19:03   ` Linus Walleij
2013-02-11  4:25     ` Viresh Kumar
2013-04-29 16:00   ` [v8,01/12] " James Hogan
2013-04-29 16:49     ` Haojian Zhuang
2013-04-29 20:16       ` James Hogan
2013-02-02 17:25 ` [PATCH v8 02/12] gpio: fix wrong checking condition for gpio range Haojian Zhuang
2013-02-05 17:02   ` Linus Walleij
2013-02-02 17:25 ` [PATCH v8 03/12] gpio: find gpio base by ascend order Haojian Zhuang
2013-02-05 17:14   ` Linus Walleij
2013-02-06  1:59     ` Haojian Zhuang
2013-02-06  4:33       ` Alex Courbot
2013-02-06  5:20         ` Haojian Zhuang
2013-02-06  8:44       ` Linus Walleij
2013-02-06  9:15         ` Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 04/12] gpio: pl061: allocate irq dynamically Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 05/12] pinctrl: verify whether gpio chip overlapps range Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 06/12] gpio: pl061: bind pinctrl by gpio request Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 07/12] pinctrl: single: create new gpio function range Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 08/12] pinctrl: generic: dump pin configuration Haojian Zhuang
2013-02-05  0:35   ` Tony Lindgren
2013-02-05  0:57     ` Tony Lindgren
2013-02-05  1:09       ` Haojian Zhuang
2013-02-05  1:08     ` Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 09/12] pinctrl: single: set function mask as optional Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 10/12] pinctrl: single: support generic pinconf Haojian Zhuang
2013-02-05  0:46   ` Tony Lindgren
2013-02-05  1:07     ` Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 11/12] ARM: hs: enable hi4511 with device tree Haojian Zhuang
2013-02-02 17:25 ` [PATCH v8 12/12] document: devicetree: bind pinconf with pin single Haojian Zhuang
2013-02-05  4:07   ` Tony Lindgren
2013-02-05  8:06     ` Haojian Zhuang
2013-02-05 13:51       ` Haojian Zhuang
2013-02-05 23:30         ` Tony Lindgren
2013-02-06 15:07           ` Haojian Zhuang
2013-02-06 17:16             ` Tony Lindgren
2013-02-05 23:21       ` Tony Lindgren
2013-02-11 17:10 [PATCH v8 00/12] bind pinconf with pinctrl single Haojian Zhuang
2013-02-11 17:10 ` [PATCH v8 12/12] document: devicetree: bind pinconf with pin single Haojian Zhuang
2013-02-13 23:51   ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.