[PATCH v8 00/12] bind pinconf with pinctrl single

[PATCH v8 00/12] bind pinconf with pinctrl single

[Haojian Zhuang]

Changelog:
v8:
1. Update allocate irq dynamically in gpio-pl061.
2. Update bias-pullup/bias-pulldown as 4 parameters.
3. Remove bias-disable.
4. Add bias-autopull.
5. Rename input-schmitt-disable to input-schmitt-enable.
6. Discard allocating global gpio number in ascending order.

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.

[PATCH v8 01/12] gpio: add gpio offset in gpio range cells property

[Haojian Zhuang]

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

[PATCH v8 01/12] gpio: add gpio offset in gpio range cells property

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 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>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Seems reasonable to me:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

However I want Grant to have the final say on this patch.

Yours,
Linus Walleij

[PATCH v8 02/12] gpio: fix wrong checking condition for gpio range

[Haojian Zhuang]

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

[PATCH v8 02/12] gpio: fix wrong checking condition for gpio range

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 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>

Grant can you pick this patch (and the other one, 01/12)?

I would stack them in my tree unless there was such deep-core
OF business involved, which makes me insecure.

Yours,
Linus Walleij

[PATCH v8 03/12] gpio: pl061: allocate irq dynamically

[Haojian Zhuang]

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 |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index b820869..1a6d05c 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,15 @@ 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 = -1;
+	}
+	chip->irq_base = irq_alloc_descs(chip->irq_base, 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

[PATCH v8 03/12] gpio: pl061: allocate irq dynamically

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

We don't convert drivers to irqdomain without actually *using*
the irqdomain. I don't know where this pattern comes from,
but I'm suspicious about patch sets that just focus on enabling
DT functionality without considering irqdomain stuff as a concept
of its own.

The biggest problem with this patch is what is missing from it:

- irq_base shall be *deleted* from struct pl061_gpio
- instead a struct irqdomain  * shall be stored for offsetting IRQs
- everywhere chip->irq_base is referenced, use irq_find_mapping()
- In pl061_to_irq, irq_create_mapping() shall be used, so it will
  allocate a descriptor even if we're using the driver with
  SPARSE_IRQ and a linear domain.

Look at e.g. gpio-langwell.c for guidance.

> +       chip->irq_base = irq_alloc_descs(chip->irq_base, 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;

Instead of the above, please use:

chip->irqdomain = irq_domain_add_simple(adev->dev.of_node,
                                         PL061_GPIO_NR,
                                         chip->irq_base,
                                         &pl061_domain_ops,
                                         chip);

Notice that I don't throw the domain away after creation either...

This call will allocate descriptors for you as long as the
irq_base > 0, which it should be, since 0 is NO_IRQ.

It make things easier the day you start using a purely
dynamic approach.

Yours,
Linus Walleij

[PATCH v8 03/12] gpio: pl061: allocate irq dynamically

[Haojian Zhuang]

On 14 February 2013 22:04, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>
> We don't convert drivers to irqdomain without actually *using*
> the irqdomain. I don't know where this pattern comes from,
> but I'm suspicious about patch sets that just focus on enabling
> DT functionality without considering irqdomain stuff as a concept
> of its own.
>
> The biggest problem with this patch is what is missing from it:
>
> - irq_base shall be *deleted* from struct pl061_gpio
> - instead a struct irqdomain  * shall be stored for offsetting IRQs
> - everywhere chip->irq_base is referenced, use irq_find_mapping()
> - In pl061_to_irq, irq_create_mapping() shall be used, so it will
>   allocate a descriptor even if we're using the driver with
>   SPARSE_IRQ and a linear domain.
>
> Look at e.g. gpio-langwell.c for guidance.
>
>> +       chip->irq_base = irq_alloc_descs(chip->irq_base, 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;
>
> Instead of the above, please use:
>
> chip->irqdomain = irq_domain_add_simple(adev->dev.of_node,
>                                          PL061_GPIO_NR,
>                                          chip->irq_base,
>                                          &pl061_domain_ops,
>                                          chip);
>
> Notice that I don't throw the domain away after creation either...
>
> This call will allocate descriptors for you as long as the
> irq_base > 0, which it should be, since 0 is NO_IRQ.
>
> It make things easier the day you start using a purely
> dynamic approach.
>
> Yours,
> Linus Walleij

Thank you. I'll update it.

Regards
Haojian

[PATCH v8 04/12] pinctrl: verify whether gpio chip overlapps range

[Haojian Zhuang]

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 b0a8e9a..140d6cb 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

[PATCH v8 04/12] pinctrl: verify whether gpio chip overlapps range

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

>  /**
> + * 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)

The name of this function confuses me, can we name it something
like:

pinctrl_gpio_is_in_some_chip_but_no_range()

Which is actually what you test, right?

> +{
> +       struct pinctrl_dev *pctldev;
> +       struct pinctrl_gpio_range *range = NULL;
> +       struct gpio_chip *chip = gpio_to_chip(gpio);

Handle if chip happens to become NULL here.

> +       /* 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;

If you change the name of the function like above this call can be understood,
but maybe also add a comment above this call explaining the situation.

Yours,
Linus Walleij

[PATCH v8 04/12] pinctrl: verify whether gpio chip overlapps range

[Haojian Zhuang]

On 14 February 2013 23:23, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>
>>  /**
>> + * 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)
>
> The name of this function confuses me, can we name it something
> like:
>
> pinctrl_gpio_is_in_some_chip_but_no_range()
>
> Which is actually what you test, right?
>
How about pinctrl_match_gpio_range_exclude_some_pins()?

>> +{
>> +       struct pinctrl_dev *pctldev;
>> +       struct pinctrl_gpio_range *range = NULL;
>> +       struct gpio_chip *chip = gpio_to_chip(gpio);
>
> Handle if chip happens to become NULL here.
>
>> +       /* 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;
>
> If you change the name of the function like above this call can be understood,
> but maybe also add a comment above this call explaining the situation.
>
> Yours,
> Linus Walleij

[PATCH v8 04/12] pinctrl: verify whether gpio chip overlapps range

[Linus Walleij]

On Thu, Feb 14, 2013 at 6:01 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:
> On 14 February 2013 23:23, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
>> <haojian.zhuang@linaro.org> wrote:
>>
>>>  /**
>>> + * 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)
>>
>> The name of this function confuses me, can we name it something
>> like:
>>
>> pinctrl_gpio_is_in_some_chip_but_no_range()
>>
>> Which is actually what you test, right?
>>
> How about pinctrl_match_gpio_range_exclude_some_pins()?

Actually I get ever more confused by this patch, I just don't understand
it. :-(

Something will need to be expanded on here or I will be unable to
maintaine the code, sorry for being a slow learner...

By the way:
Nowaday the recommended practice is to register a GPIO chip,
then add ranges using gpiochip_add_pin_range() in the non-DT
case. Doesn't that create a race window when the above code will
not behave as expected, e.g. between registration of the gpio
chip, the pin controller and the ranges? I guess the code assumes
that *all* initialization is complete?

Yours,
Linus Walleij

[PATCH v8 04/12] pinctrl: verify whether gpio chip overlapps range

[Haojian Zhuang]

On 15 February 2013 17:06, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Feb 14, 2013 at 6:01 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>> On 14 February 2013 23:23, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
>>> <haojian.zhuang@linaro.org> wrote:
>>>
>>>>  /**
>>>> + * 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)
>>>
>>> The name of this function confuses me, can we name it something
>>> like:
>>>
>>> pinctrl_gpio_is_in_some_chip_but_no_range()
>>>
>>> Which is actually what you test, right?
>>>
>> How about pinctrl_match_gpio_range_exclude_some_pins()?
>
> Actually I get ever more confused by this patch, I just don't understand
> it. :-(
>
> Something will need to be expanded on here or I will be unable to
> maintaine the code, sorry for being a slow learner...
>
> By the way:
> Nowaday the recommended practice is to register a GPIO chip,
> then add ranges using gpiochip_add_pin_range() in the non-DT
> case. Doesn't that create a race window when the above code will
> not behave as expected, e.g. between registration of the gpio
> chip, the pin controller and the ranges? I guess the code assumes
> that *all* initialization is complete?
>
> Yours,
> Linus Walleij

The implementation is a little tricky at here. Let me explain the case.

We have a back-end pinctrl-single device & pl061 gpio devices. So we
need to define gpio range in DT. But there's no related pinmux registers
on some special pins, like gpio159. Although we initialized pinctrl-single
driver, we still get failure from pinctrl_match_gpio_range() because
it doesn't exist in DT.

I implement it to double check for the failure case. If match_gpio_range()
return failure, we need to check whether the pinctrl device isn't ready or
the special pins like gpio159 don't exist in gpio range. So if the
pinctrl device
isn't ready, it return EPROBE_DEFERED. If the special pins doesn't exist
in gpio range, it's not the error case & return 0 (sucessful).

Regards
Haojian

[PATCH v8 05/12] gpio: pl061: bind pinctrl by gpio request

[Haojian Zhuang]

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 1a6d05c..bc9b4b2 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);
@@ -252,6 +264,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

[PATCH v8 05/12] gpio: pl061: bind pinctrl by gpio request

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> 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>
(...)
> +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);
> +}

So this will work find if the platform supports pinctrl, and either the
pin controller can do something with this, or (after the other patch)
if the GPIO is in some range, but not handled by some
pin controller.

But what about the case where there is a pin controller on the
system, but no range matching this pin?

What will happen then? Eternal deferral?

Yours,
Linus Walleij

[PATCH v8 05/12] gpio: pl061: bind pinctrl by gpio request

[Haojian Zhuang]

On 14 February 2013 23:29, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>
>> 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>
> (...)
>> +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);
>> +}
>
> So this will work find if the platform supports pinctrl, and either the
> pin controller can do something with this, or (after the other patch)
> if the GPIO is in some range, but not handled by some
> pin controller.
>
> But what about the case where there is a pin controller on the
> system, but no range matching this pin?
>
> What will happen then? Eternal deferral?
>
> Yours,
> Linus Walleij

If there's a back-end pin controller ready, but no range matching this pin,
it will return 0, not error. This is the purpose that I want to add
pinctrl_overlapped_gpio_range().

Yes, the name is so confusing. Let's figure out a clear name.

Regards
Haojian

[PATCH v8 06/12] pinctrl: single: create new gpio function range

[Haojian Zhuang]

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 gpio-ranges property is used in gpio driver and the
pinctrl-single,gpio-range property is used 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

[PATCH v8 06/12] pinctrl: single: create new gpio function range

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@linaro.org> [130211 09:15]:
> 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.

Minor typo above: s/invokes/invoke/
 
> 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 gpio-ranges property is used in gpio driver and the
> pinctrl-single,gpio-range property is used 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>;

I think the second gpio-ranges above should be really
pinctr-single,gpio-range instead of gpio-ranges?
 
> 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.

Other than that looks OK to me. Assuming the other related GPIO patches
are fine and don't cause changes to this:

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

[PATCH v8 06/12] pinctrl: single: create new gpio function range

[Haojian Zhuang]

On 14 February 2013 02:39, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@linaro.org> [130211 09:15]:
>> 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.
>
> Minor typo above: s/invokes/invoke/
>
I'll fix it.

>> 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 gpio-ranges property is used in gpio driver and the
>> pinctrl-single,gpio-range property is used 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>;
>
> I think the second gpio-ranges above should be really
> pinctr-single,gpio-range instead of gpio-ranges?
>

No, it's not pinctrl-single,gpio-range property. I list both two properties are
here, since I need to explain the difference between gpio-ranges &
pinctrl-single,gpio-range.

Thanks
Haojian

>> 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.
>
> Other than that looks OK to me. Assuming the other related GPIO patches
> are fine and don't cause changes to this:
>
> Acked-by: Tony Lindgren <tony@atomide.com>

[PATCH v8 06/12] pinctrl: single: create new gpio function range

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> 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.

Is it OK if I wait with patches 6-12 while we sort out the first
5?

Yours,
Linus Walleij

[PATCH v8 06/12] pinctrl: single: create new gpio function range

[Haojian Zhuang]

On 14 February 2013 23:24, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>
>> 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.
>
> Is it OK if I wait with patches 6-12 while we sort out the first
> 5?
>
> Yours,
> Linus Walleij

Since Tony has some comments on dropping BIAS_AUTO_PULL,
I'll send the v9 including your comments.

Regards
Haojian

[PATCH v8 07/12] pinctrl: generic: dump pin configuration

[Haojian Zhuang]

Add the support of dumping pin configuration.

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

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index e5948f8..ef24230 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -12,6 +12,7 @@
 #define pr_fmt(fmt) "generic pinconfig core: " fmt
 
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/slab.h>
@@ -120,4 +121,17 @@ 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));
+	}
+}
+EXPORT_SYMBOL_GPL(pinconf_generic_dump_config);
 #endif
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index e3ed8cb..1f7113e 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 inline void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
+					       struct seq_file *s,
+					       unsigned long config)
+{
+	return;
+}
 #endif
-- 
1.7.10.4

[PATCH v8 08/12] pinctrl: single: set function mask as optional

[Haojian Zhuang]

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

[PATCH v8 08/12] pinctrl: single: set function mask as optional

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@linaro.org> [130211 09:15]:
> 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.

Looks OK to me:

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

[PATCH v8 09/12] pinctrl: generic: add auto pull config parameter

[Haojian Zhuang]

There's only one bit to control pin bias as enabled or disabled for some
pins in OMAP SoC. So append PIN_CONFIG_BIAS_AUTO_PULL for this case.
User shouldn't switch pin state between AUTO_PULL and PULL_UP/PULL_DOWN,
since they're similiar concepts.

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

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index ef24230..4a67848 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -37,6 +37,7 @@ struct pin_config_item {
 struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_BIAS_DISABLE, "input bias disabled", NULL),
 	PCONFDUMP(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, "input bias high impedance", NULL),
+	PCONFDUMP(PIN_CONFIG_BIAS_AUTO_PULL, "input bias auto pull", NULL),
 	PCONFDUMP(PIN_CONFIG_BIAS_PULL_UP, "input bias pull up", NULL),
 	PCONFDUMP(PIN_CONFIG_BIAS_PULL_DOWN, "input bias pull down", NULL),
 	PCONFDUMP(PIN_CONFIG_DRIVE_PUSH_PULL, "output drive push pull", NULL),
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index 3e7909a..f2daff4 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -29,6 +29,8 @@
  *	if for example some other pin is going to drive the signal connected
  *	to it for a while. Pins used for input are usually always high
  *	impedance.
+ * @PIN_CONFIG_BIAS_AUTO_PULL: the pin will be pulled without specifying
+ *      pull-up or pull-down.
  * @PIN_CONFIG_BIAS_PULL_UP: the pin will be pulled up (usually with high
  *	impedance to VDD). If the argument is != 0 pull-up is enabled,
  *	if it is 0, pull-up is disabled.
@@ -76,6 +78,7 @@
 enum pin_config_param {
 	PIN_CONFIG_BIAS_DISABLE,
 	PIN_CONFIG_BIAS_HIGH_IMPEDANCE,
+	PIN_CONFIG_BIAS_AUTO_PULL,
 	PIN_CONFIG_BIAS_PULL_UP,
 	PIN_CONFIG_BIAS_PULL_DOWN,
 	PIN_CONFIG_DRIVE_PUSH_PULL,
-- 
1.7.10.4

[PATCH v8 09/12] pinctrl: generic: add auto pull config parameter

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@linaro.org> [130211 09:15]:
> There's only one bit to control pin bias as enabled or disabled for some
> pins in OMAP SoC. So append PIN_CONFIG_BIAS_AUTO_PULL for this case.
> User shouldn't switch pin state between AUTO_PULL and PULL_UP/PULL_DOWN,
> since they're similiar concepts.

Please just drop patch from the series for now. This needs further research
on what the hardware is actually doing. I suspect we can replace this with
just either PIN_CONFIG_BIAS_PULL_UP or PIN_CONFIG_BIAS_PULL_DOWN.

Regards,

Tony
 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  drivers/pinctrl/pinconf-generic.c       |    1 +
>  include/linux/pinctrl/pinconf-generic.h |    3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
> index ef24230..4a67848 100644
> --- a/drivers/pinctrl/pinconf-generic.c
> +++ b/drivers/pinctrl/pinconf-generic.c
> @@ -37,6 +37,7 @@ struct pin_config_item {
>  struct pin_config_item conf_items[] = {
>  	PCONFDUMP(PIN_CONFIG_BIAS_DISABLE, "input bias disabled", NULL),
>  	PCONFDUMP(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, "input bias high impedance", NULL),
> +	PCONFDUMP(PIN_CONFIG_BIAS_AUTO_PULL, "input bias auto pull", NULL),
>  	PCONFDUMP(PIN_CONFIG_BIAS_PULL_UP, "input bias pull up", NULL),
>  	PCONFDUMP(PIN_CONFIG_BIAS_PULL_DOWN, "input bias pull down", NULL),
>  	PCONFDUMP(PIN_CONFIG_DRIVE_PUSH_PULL, "output drive push pull", NULL),
> diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
> index 3e7909a..f2daff4 100644
> --- a/include/linux/pinctrl/pinconf-generic.h
> +++ b/include/linux/pinctrl/pinconf-generic.h
> @@ -29,6 +29,8 @@
>   *	if for example some other pin is going to drive the signal connected
>   *	to it for a while. Pins used for input are usually always high
>   *	impedance.
> + * @PIN_CONFIG_BIAS_AUTO_PULL: the pin will be pulled without specifying
> + *      pull-up or pull-down.
>   * @PIN_CONFIG_BIAS_PULL_UP: the pin will be pulled up (usually with high
>   *	impedance to VDD). If the argument is != 0 pull-up is enabled,
>   *	if it is 0, pull-up is disabled.
> @@ -76,6 +78,7 @@
>  enum pin_config_param {
>  	PIN_CONFIG_BIAS_DISABLE,
>  	PIN_CONFIG_BIAS_HIGH_IMPEDANCE,
> +	PIN_CONFIG_BIAS_AUTO_PULL,
>  	PIN_CONFIG_BIAS_PULL_UP,
>  	PIN_CONFIG_BIAS_PULL_DOWN,
>  	PIN_CONFIG_DRIVE_PUSH_PULL,
> -- 
> 1.7.10.4
> 

[PATCH v8 09/12] pinctrl: generic: add auto pull config parameter

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> There's only one bit to control pin bias as enabled or disabled for some
> pins in OMAP SoC. So append PIN_CONFIG_BIAS_AUTO_PULL for this case.
> User shouldn't switch pin state between AUTO_PULL and PULL_UP/PULL_DOWN,
> since they're similiar concepts.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

Makes perfect sense.
Patch applied with Tony's ACK!

Yours,
Linus Walleij

[PATCH v8 09/12] pinctrl: generic: add auto pull config parameter

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [130215 00:58]:
> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
> 
> > There's only one bit to control pin bias as enabled or disabled for some
> > pins in OMAP SoC. So append PIN_CONFIG_BIAS_AUTO_PULL for this case.
> > User shouldn't switch pin state between AUTO_PULL and PULL_UP/PULL_DOWN,
> > since they're similiar concepts.
> >
> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> 
> Makes perfect sense.
> Patch applied with Tony's ACK!

Sorry the conclusion was that this probably won't be needed and can
probably be done with just PULL_UP/PULL_DOWN.

So can you please drop or revert this AUTO_PULL one?

Regards,

Tony

[PATCH v8 09/12] pinctrl: generic: add auto pull config parameter

[Linus Walleij]

On Fri, Feb 15, 2013 at 5:37 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Linus Walleij <linus.walleij@linaro.org> [130215 00:58]:
>> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
>> <haojian.zhuang@linaro.org> wrote:
>>
>> > There's only one bit to control pin bias as enabled or disabled for some
>> > pins in OMAP SoC. So append PIN_CONFIG_BIAS_AUTO_PULL for this case.
>> > User shouldn't switch pin state between AUTO_PULL and PULL_UP/PULL_DOWN,
>> > since they're similiar concepts.
>> >
>> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>
>> Makes perfect sense.
>> Patch applied with Tony's ACK!
>
> Sorry the conclusion was that this probably won't be needed and can
> probably be done with just PULL_UP/PULL_DOWN.
>
> So can you please drop or revert this AUTO_PULL one?

Sorry I was drunk or something.

I never applied this patch, I applied the other one,
with the title "pinctrl: generic: rename input schmitt disable".

This one stays out.

Yours,
Linus Walleij

[PATCH v8 09/12] pinctrl: generic: add auto pull config parameter

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [130215 12:58]:
> On Fri, Feb 15, 2013 at 5:37 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Linus Walleij <linus.walleij@linaro.org> [130215 00:58]:
> >> On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
> >> <haojian.zhuang@linaro.org> wrote:
> >>
> >> > There's only one bit to control pin bias as enabled or disabled for some
> >> > pins in OMAP SoC. So append PIN_CONFIG_BIAS_AUTO_PULL for this case.
> >> > User shouldn't switch pin state between AUTO_PULL and PULL_UP/PULL_DOWN,
> >> > since they're similiar concepts.
> >> >
> >> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> >>
> >> Makes perfect sense.
> >> Patch applied with Tony's ACK!
> >
> > Sorry the conclusion was that this probably won't be needed and can
> > probably be done with just PULL_UP/PULL_DOWN.
> >
> > So can you please drop or revert this AUTO_PULL one?
> 
> Sorry I was drunk or something.
> 
> I never applied this patch, I applied the other one,
> with the title "pinctrl: generic: rename input schmitt disable".
> 
> This one stays out.

OK thanks for clarifying it :)

Tony

[PATCH v8 10/12] pinctrl: generic: rename input schmitt disable

[Haojian Zhuang]

Rename PIN_CONFIG_INPUT_SCHMITT_DISABLE to
PIN_CONFIG_INPUT_SCHMITT_ENABLE. It's used to make it more generialize.

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

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 4a67848..e43d2e0 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -43,7 +43,7 @@ struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_DRIVE_PUSH_PULL, "output drive push pull", NULL),
 	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_DRAIN, "output drive open drain", NULL),
 	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_SOURCE, "output drive open source", NULL),
-	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_DISABLE, "input schmitt disabled", NULL),
+	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_ENABLE, "input schmitt enabled", NULL),
 	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT, "input schmitt trigger", NULL),
 	PCONFDUMP(PIN_CONFIG_INPUT_DEBOUNCE, "input debounce", "time units"),
 	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector"),
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index f2daff4..93314cb 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -50,7 +50,9 @@
  *	argument is ignored.
  * @PIN_CONFIG_DRIVE_STRENGTH: the pin will output the current passed as
  * 	argument. The argument is in mA.
- * @PIN_CONFIG_INPUT_SCHMITT_DISABLE: disable schmitt-trigger mode on the pin.
+ * @PIN_CONFIG_INPUT_SCHMITT_ENABLE: control schmitt-trigger mode on the pin.
+ *      If the argument != 0, schmitt-trigger mode is enabled. If it's 0,
+ *      schmitt-trigger mode is disabled.
  * @PIN_CONFIG_INPUT_SCHMITT: this will configure an input pin to run in
  *	schmitt-trigger mode. If the schmitt-trigger has adjustable hysteresis,
  *	the threshold value is given on a custom format as argument when
@@ -85,7 +87,7 @@ enum pin_config_param {
 	PIN_CONFIG_DRIVE_OPEN_DRAIN,
 	PIN_CONFIG_DRIVE_OPEN_SOURCE,
 	PIN_CONFIG_DRIVE_STRENGTH,
-	PIN_CONFIG_INPUT_SCHMITT_DISABLE,
+	PIN_CONFIG_INPUT_SCHMITT_ENABLE,
 	PIN_CONFIG_INPUT_SCHMITT,
 	PIN_CONFIG_INPUT_DEBOUNCE,
 	PIN_CONFIG_POWER_SOURCE,
-- 
1.7.10.4

[PATCH v8 10/12] pinctrl: generic: rename input schmitt disable

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@linaro.org> [130211 09:15]:
> Rename PIN_CONFIG_INPUT_SCHMITT_DISABLE to
> PIN_CONFIG_INPUT_SCHMITT_ENABLE. It's used to make it more generialize.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

Makes sense to me for future naming of things:

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

> ---
>  drivers/pinctrl/pinconf-generic.c       |    2 +-
>  include/linux/pinctrl/pinconf-generic.h |    6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
> index 4a67848..e43d2e0 100644
> --- a/drivers/pinctrl/pinconf-generic.c
> +++ b/drivers/pinctrl/pinconf-generic.c
> @@ -43,7 +43,7 @@ struct pin_config_item conf_items[] = {
>  	PCONFDUMP(PIN_CONFIG_DRIVE_PUSH_PULL, "output drive push pull", NULL),
>  	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_DRAIN, "output drive open drain", NULL),
>  	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_SOURCE, "output drive open source", NULL),
> -	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_DISABLE, "input schmitt disabled", NULL),
> +	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_ENABLE, "input schmitt enabled", NULL),
>  	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT, "input schmitt trigger", NULL),
>  	PCONFDUMP(PIN_CONFIG_INPUT_DEBOUNCE, "input debounce", "time units"),
>  	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector"),
> diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
> index f2daff4..93314cb 100644
> --- a/include/linux/pinctrl/pinconf-generic.h
> +++ b/include/linux/pinctrl/pinconf-generic.h
> @@ -50,7 +50,9 @@
>   *	argument is ignored.
>   * @PIN_CONFIG_DRIVE_STRENGTH: the pin will output the current passed as
>   * 	argument. The argument is in mA.
> - * @PIN_CONFIG_INPUT_SCHMITT_DISABLE: disable schmitt-trigger mode on the pin.
> + * @PIN_CONFIG_INPUT_SCHMITT_ENABLE: control schmitt-trigger mode on the pin.
> + *      If the argument != 0, schmitt-trigger mode is enabled. If it's 0,
> + *      schmitt-trigger mode is disabled.
>   * @PIN_CONFIG_INPUT_SCHMITT: this will configure an input pin to run in
>   *	schmitt-trigger mode. If the schmitt-trigger has adjustable hysteresis,
>   *	the threshold value is given on a custom format as argument when
> @@ -85,7 +87,7 @@ enum pin_config_param {
>  	PIN_CONFIG_DRIVE_OPEN_DRAIN,
>  	PIN_CONFIG_DRIVE_OPEN_SOURCE,
>  	PIN_CONFIG_DRIVE_STRENGTH,
> -	PIN_CONFIG_INPUT_SCHMITT_DISABLE,
> +	PIN_CONFIG_INPUT_SCHMITT_ENABLE,
>  	PIN_CONFIG_INPUT_SCHMITT,
>  	PIN_CONFIG_INPUT_DEBOUNCE,
>  	PIN_CONFIG_POWER_SOURCE,
> -- 
> 1.7.10.4
> 

[PATCH v8 11/12] pinctrl: single: support generic pinconf

[Haojian Zhuang]

Support the operation of generic pinconf. The supported config arguments
are INPUT_SCHMITT, INPUT_SCHMITT_ENABLE, DRIVE_STRENGHT, BIAS_DISABLE,
BIAS_AUTOPULL, BIAS_PULLUP, BIAS_PULLDOWN, SLEW_RATE.

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

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 34f51d2..5a690ce 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -166,6 +166,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..eda8a38 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,33 @@ struct pcs_func_vals {
 };
 
 /**
+ * struct pcs_conf_vals - pinconf parameter, pinconf register offset
+ * and value, enable, disable, mask
+ * @param:	config parameter
+ * @val:	user input bits in the pinconf register
+ * @enable:	enable bits in the pinconf register
+ * @disable:	disable bits in the pinconf register
+ * @mask:	mask bits in the register value
+ */
+struct pcs_conf_vals {
+	enum pin_config_param param;
+	unsigned val;
+	unsigned enable;
+	unsigned disable;
+	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 +102,8 @@ struct pcs_function {
 	unsigned nvals;
 	const char **pgnames;
 	int npgnames;
+	struct pcs_conf_vals *conf;
+	int nconfs;
 	struct list_head node;
 };
 
@@ -131,6 +162,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 +189,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;
@@ -171,6 +204,17 @@ struct pcs_device {
 	void (*write)(unsigned val, void __iomem *reg);
 };
 
+static int pcs_pinconf_get(struct pinctrl_dev *pctldev, unsigned pin,
+			   unsigned long *config);
+static int pcs_pinconf_set(struct pinctrl_dev *pctldev, unsigned pin,
+			   unsigned long config);
+
+static enum pin_config_param pcs_bias[] = {
+	PIN_CONFIG_BIAS_AUTO_PULL,
+	PIN_CONFIG_BIAS_PULL_DOWN,
+	PIN_CONFIG_BIAS_PULL_UP,
+};
+
 /*
  * REVISIT: Reads and writes could eventually use regmap or something
  * generic. But at least on omaps, some mux registers are performance
@@ -342,6 +386,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)
 {
@@ -461,32 +527,192 @@ static struct pinmux_ops pcs_pinmux_ops = {
 	.gpio_request_enable = pcs_request_gpio,
 };
 
+/* Clear BIAS value */
+static void pcs_pinconf_clear_bias(struct pinctrl_dev *pctldev, unsigned pin)
+{
+	unsigned long config;
+	int i;
+	for (i = 0; i < ARRAY_SIZE(pcs_bias); i++) {
+		config = pinconf_to_config_packed(pcs_bias[i], 0);
+		pcs_pinconf_set(pctldev, pin, config);
+	}
+}
+
+/*
+ * Check whether PIN_CONFIG_BIAS_DISABLE is valid.
+ * It's depend on that AUTO_PULL, PULL_DOWN & PULL_UP configs are all invalid.
+ */
+static bool pcs_pinconf_bias_disable(struct pinctrl_dev *pctldev, unsigned pin)
+{
+	unsigned long config;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pcs_bias); i++) {
+		config = pinconf_to_config_packed(pcs_bias[i], 0);
+		if (!pcs_pinconf_get(pctldev, pin, &config))
+			goto out;
+	}
+	return true;
+out:
+	return false;
+}
+
 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;
+	enum pin_config_param param;
+	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++) {
+		param = pinconf_to_config_param(*config);
+		if (param == PIN_CONFIG_BIAS_DISABLE) {
+			if (pcs_pinconf_bias_disable(pctldev, pin)) {
+				*config = 0;
+				return 0;
+			} else {
+				return -ENOTSUPP;
+			}
+		} else if (param != 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) {
+		/* 4 parameters */
+		case PIN_CONFIG_BIAS_AUTO_PULL:
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+		case PIN_CONFIG_BIAS_PULL_UP:
+		case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+			if ((data != func->conf[i].enable) ||
+			    (data == func->conf[i].disable))
+				return -ENOTSUPP;
+			*config = 0;
+			break;
+		/* 2 parameters */
+		case PIN_CONFIG_INPUT_SCHMITT:
+			for (j = 0; j < func->nconfs; j++) {
+				switch (func->conf[j].param) {
+				case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+					if (data != func->conf[j].enable)
+						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;
+	u16 argument;
+
+	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);
+			argument = pinconf_to_config_argument(config);
+			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;
+			/* 4 parameters */
+			case PIN_CONFIG_BIAS_DISABLE:
+				pcs_pinconf_clear_bias(pctldev, pin);
+				break;
+			case PIN_CONFIG_BIAS_PULL_DOWN:
+			case PIN_CONFIG_BIAS_PULL_UP:
+				if (argument)
+					pcs_pinconf_clear_bias(pctldev, pin);
+				/* fall through */
+			case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+				data &= ~func->conf[i].mask;
+				if (argument)
+					data |= func->conf[i].enable;
+				else
+					data |= func->conf[i].disable;
+				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 +721,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 +735,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 +926,158 @@ static int pcs_get_pin_by_offset(struct pcs_device *pcs, unsigned offset)
 	return index;
 }
 
+/*
+ * check whether data matches enable bits or disable bits
+ * Return value: 1 for matching enable bits, 0 for matching disable bits,
+ *               and negative value for matching failure.
+ */
+static int pcs_config_match(unsigned data, unsigned enable, unsigned disable)
+{
+	int ret = -EINVAL;
+
+	if (data == enable)
+		ret = 1;
+	else if (data == disable)
+		ret = 0;
+	return ret;
+}
+
+static void add_config(struct pcs_conf_vals **conf, enum pin_config_param param,
+		       unsigned value, unsigned enable, unsigned disable,
+		       unsigned mask)
+{
+	(*conf)->param = param;
+	(*conf)->val = value;
+	(*conf)->enable = enable;
+	(*conf)->disable = disable;
+	(*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 pcs_device *pcs, 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;
+	/* set value & mask */
+	value[0] &= value[1];
+	/* skip enable & disable */
+	add_config(conf, param, value[0], 0, 0, value[1]);
+	add_setting(settings, param, value[0]);
+}
+
+/* add pinconf setting with 4 parameters */
+static void pcs_add_conf4(struct pcs_device *pcs, struct device_node *np,
+			  const char *name, enum pin_config_param param,
+			  struct pcs_conf_vals **conf, unsigned long **settings)
+{
+	unsigned value[4];
+	int ret;
+
+	/* value to set, enable, disable, mask */
+	ret = of_property_read_u32_array(np, name, value, 4);
+	if (ret)
+		return;
+	if (!value[3]) {
+		dev_err(pcs->dev, "mask field of the property can't be 0\n");
+		return;
+	}
+	value[0] &= value[3];
+	value[1] &= value[3];
+	value[2] &= value[3];
+	ret = pcs_config_match(value[0], value[1], value[2]);
+	if (ret < 0)
+		dev_dbg(pcs->dev, "failed to match enable or disable bits\n");
+	add_config(conf, param, value[0], value[1], value[2], value[3]);
+	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 prop4[] = {
+		{ "pinctrl-single,bias-autopull", PIN_CONFIG_BIAS_AUTO_PULL, },
+		{ "pinctrl-single,bias-pullup", PIN_CONFIG_BIAS_PULL_UP, },
+		{ "pinctrl-single,bias-pulldown", PIN_CONFIG_BIAS_PULL_DOWN, },
+		{ "pinctrl-single,input-schmitt-enable",
+			PIN_CONFIG_INPUT_SCHMITT_ENABLE, },
+	};
+
+	/* 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(prop4); i++) {
+		if (of_find_property(np, prop4[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(pcs, np, prop2[i].name, prop2[i].param,
+			      &conf, &s);
+	for (i = 0; i < ARRAY_SIZE(prop4); i++)
+		pcs_add_conf4(pcs, np, prop4[i].name, prop4[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 +1094,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 +1167,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 +1207,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 +1220,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 +1369,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 +1440,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 +1482,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

[PATCH v8 11/12] pinctrl: single: support generic pinconf

[Tony Lindgren]

* Haojian Zhuang <haojian.zhuang@linaro.org> [130211 09:15]:
> Support the operation of generic pinconf. The supported config arguments
> are INPUT_SCHMITT, INPUT_SCHMITT_ENABLE, DRIVE_STRENGHT, BIAS_DISABLE,
> BIAS_AUTOPULL, BIAS_PULLUP, BIAS_PULLDOWN, SLEW_RATE.

I suggest you leave out BIAS_AUTOPULL here too. And then maybe check
the naming above for better search results :) It should BIAS_PULL_UP
etc I guess?

Other than that this works good for me, thanks for all the effort!

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

[PATCH v8 12/12] document: devicetree: bind pinconf with pin single

[Haojian Zhuang]

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

[PATCH v8 12/12] document: devicetree: bind pinconf with pin single

[Tony Lindgren]

* 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>

[PATCH v8 00/12] bind pinconf with pinctrl single

[Linus Walleij]

On Mon, Feb 11, 2013 at 6:10 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> Changelog:
> v8:

I just want to take the opportunity to also state that I am *very* *impressed*
with the patch set and your perseverance on making this pin controller
reusable for HiSilicon.

Even if I make grumpy comments on things I want to stress that the patch
set is in general very good.

Yours,
Linus Walleij

[PATCH v8 00/12] support pinconf in pinctrl single

[Haojian Zhuang]

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.

[PATCH v8 01/12] gpio: add gpio offset in gpio range cells property

[Haojian Zhuang]

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

[PATCH v8 01/12] gpio: add gpio offset in gpio range cells property

[Tony Lindgren]

* 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

[PATCH v8 01/12] gpio: add gpio offset in gpio range cells property

[Haojian Zhuang]

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

[PATCH v8 01/12] gpio: add gpio offset in gpio range cells property

[Linus Walleij]

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

[PATCH v8 01/12] gpio: add gpio offset in gpio range cells property

[Viresh Kumar]

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>