support pinconf in pinctrl single driver

The above error happens in builds including pinctrl-single - the reason
is this, where resource_size_t may be 64-bit.

                gpio->range.pin_base = (r.start - pcs->res->start) /
                mux_bytes;
                gpio->range.npins = (r.end - r.start) / mux_bytes + 1;

This reverts commit 2e8b2eab94c35d83bb7da71c63b4695f32ddca88.

Conflicts:
drivers/pinctrl/pinctrl-single.c

ERROR: "__aeabi_uldivmod" [drivers/pinctrl/pinctrl-single.ko] undefined!]

Quoted 4 lines · Russell King

The above error happens in builds including pinctrl-single - the reason
is this, where resource_size_t may be 64-bit.

Quoted 3 lines

                gpio->range.pin_base = (r.start - pcs->res->start) /
                mux_bytes;
                gpio->range.npins = (r.end - r.start) / mux_bytes + 1;

The reason of not fixing this issue and reverting the patch instead is this patch can't handle another case. It's not easy to handle multiple gpios sharing one pin register. So this gpio range feature will be implemented by other patches.

---
 drivers/pinctrl/pinctrl-single.c |   79 +-------------------------------------
 1 file changed, 2 insertions(+), 77 deletions(-)

drivers/pinctrl/pinctrl-single.c +2 -77

index f6a360b..5c32e88 100644
@@ -30,7 +30,6 @@
 #define PCS_MUX_BITS_NAME		"pinctrl-single,bits"
 #define PCS_REG_NAME_LEN		((sizeof(unsigned long) * 2) + 1)
 #define PCS_OFF_DISABLED		~0U
-#define PCS_MAX_GPIO_VALUES		2
 
 /**
  * struct pcs_pingroup - pingroups for a function

@@ -78,16 +77,6 @@ struct pcs_function {
 };
 
 /**
- * struct pcs_gpio_range - pinctrl gpio range
- * @range:	subrange of the GPIO number space
- * @gpio_func:	gpio function value in the pinmux register
- */
-struct pcs_gpio_range {
-	struct pinctrl_gpio_range range;
-	int gpio_func;
-};
-
-/**
  * struct pcs_data - wrapper for data needed by pinctrl framework
  * @pa:		pindesc array
  * @cur:	index to current element

@@ -414,26 +403,9 @@ 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 pin)
+			struct pinctrl_gpio_range *range, unsigned offset)
 {
-	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
-	struct pcs_gpio_range *gpio = NULL;
-	int end, mux_bytes;
-	unsigned data;
-
-	gpio = container_of(range, struct pcs_gpio_range, range);
-	end = range->pin_base + range->npins - 1;
-	if (pin < range->pin_base || pin > end) {
-		dev_err(pctldev->dev,
-			"pin %d isn't in the range of %d to %d\n",
-			pin, range->pin_base, end);
-		return -EINVAL;
-	}
-	mux_bytes = pcs->width / BITS_PER_BYTE;
-	data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
-	data |= gpio->gpio_func;
-	pcs->write(data, pcs->base + pin * mux_bytes);
-	return 0;
+	return -ENOTSUPP;
 }
 
 static struct pinmux_ops pcs_pinmux_ops = {

@@ -907,49 +879,6 @@ static void pcs_free_resources(struct pcs_device *pcs)
 
 static struct of_device_id pcs_of_match[];
 
-static int pcs_add_gpio_range(struct device_node *node, struct pcs_device *pcs)
-{
-	struct pcs_gpio_range *gpio;
-	struct device_node *child;
-	struct resource r;
-	const char name[] = "pinctrl-single";
-	u32 gpiores[PCS_MAX_GPIO_VALUES];
-	int ret, i = 0, mux_bytes = 0;
-
-	for_each_child_of_node(node, child) {
-		ret = of_address_to_resource(child, 0, &r);
-		if (ret < 0)
-			continue;
-		memset(gpiores, 0, sizeof(u32) * PCS_MAX_GPIO_VALUES);
-		ret = of_property_read_u32_array(child, "pinctrl-single,gpio",
-						 gpiores, PCS_MAX_GPIO_VALUES);
-		if (ret < 0)
-			continue;
-		gpio = devm_kzalloc(pcs->dev, sizeof(*gpio), GFP_KERNEL);
-		if (!gpio) {
-			dev_err(pcs->dev, "failed to allocate pcs gpio\n");
-			return -ENOMEM;
-		}
-		gpio->range.name = devm_kzalloc(pcs->dev, sizeof(name),
-						GFP_KERNEL);
-		if (!gpio->range.name) {
-			dev_err(pcs->dev, "failed to allocate range name\n");
-			return -ENOMEM;
-		}
-		memcpy((char *)gpio->range.name, name, sizeof(name));
-
-		gpio->range.id = i++;
-		gpio->range.base = gpiores[0];
-		gpio->gpio_func = gpiores[1];
-		mux_bytes = pcs->width / BITS_PER_BYTE;
-		gpio->range.pin_base = (r.start - pcs->res->start) / mux_bytes;
-		gpio->range.npins = (r.end - r.start) / mux_bytes + 1;
-
-		pinctrl_add_gpio_range(pcs->pctl, &gpio->range);
-	}
-	return 0;
-}
-
 static int pcs_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;

@@ -1046,10 +975,6 @@ static int pcs_probe(struct platform_device *pdev)
 		goto free;
 	}
 
-	ret = pcs_add_gpio_range(np, pcs);
-	if (ret < 0)
-		goto free;
-
 	dev_info(pcs->dev, "%i pins at pa %p size %u\n",
 		 pcs->desc.npins, pcs->base, pcs->size);
 

Signature · 1 line

1.7.10.4

This reverts commit 2e8b2eab94c35d83bb7da71c63b4695f32ddca88.

Conflicts:
drivers/pinctrl/pinctrl-single.c

ERROR: "__aeabi_uldivmod" [drivers/pinctrl/pinctrl-single.ko] undefined!]

Add new function to get devname from pinctrl_dev. pinctrl_dev_get_name() can only get pinctrl description name. If we want to use gpio driver to find pinctrl device node, we need to fetch the pinctrl device name.

Signed-off-by Haojian Zhuang <[email protected]>

gpiochip_add_pin_range() needs pinctrl device name as parameter. Currently the parameter is pinctrl description name. So fix it.

Signed-off-by Haojian Zhuang <[email protected]>

---
 drivers/gpio/gpiolib-of.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpiolib-of.c +0 -0

index d542a14..25b1dbe 100644

Quoted 13 lines · Haojian Zhuang

gpiochip_add_pin_range() needs pinctrl device name as parameter. Currently the parameter is pinctrl description name. So fix it.

Signed-off-by Haojian Zhuang <[email protected]>

---
 drivers/gpio/gpiolib-of.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpiolib-of.c +0 -0

index d542a14..25b1dbe 100644

drivers/gpio/gpiolib-of.c static void of_gpiochip_add_pin_range(struct gpio_chip *chip) · line 256

Quoted 8 lines

drivers/gpio/gpiolib-of.c +1 -1

@@ -250,7 +250,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
                 * on the same GPIO chip.
                 */
                ret = gpiochip_add_pin_range(chip,
-                                            pinctrl_dev_get_name(pctldev),
+                                            pinctrl_dev_get_devname(pctldev),
                                             0, /* offset in gpiochip */
                                             pinspec.args[0],
                                             pinspec.args[1]);

Hm looks like the right thing to do!

Patch applied.

It must be such that everyone using this so far has just set the name to be identical to the device name.

Maybe we should just drop the name in the descriptor if it's causing problems like this?

Yours,
Linus Walleij

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

Signed-off-by Haojian Zhuang <[email protected]>

Since index++ calculates from 0, the checking condition of "while (index++)" is always fake. So replace it by unconditional loop.

Signed-off-by Haojian Zhuang <[email protected]>

---
  drivers/gpio/gpiolib-of.c |    6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

drivers/gpio/gpiolib-of.c +0 -0

index b1f0682..011e1e98 100644

Hello.

Quoted 19 lines · Haojian Zhuang

Since index++ calculates from 0, the checking condition of "while (index++)" is always fake. So replace it by unconditional loop.

Signed-off-by Haojian Zhuang <[email protected]>

---
  drivers/gpio/gpiolib-of.c |    6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

drivers/gpio/gpiolib-of.c +0 -0

index b1f0682..011e1e98 100644
@@ -228,7 +228,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip
 *chip)
        if (!np)
                return;
 

drivers/gpio/gpiolib-of.c static void of_gpiochip_add_pin_range(struct gpio_chip *chip) · line 231

Quoted 2 lines

drivers/gpio/gpiolib-of.c +1 -1

-       do {
+       for (;;) {

   Why not:

        for (;; index++) {

Quoted 5 lines

drivers/gpio/gpiolib-of.c +0 -0

                ret = of_parse_phandle_with_args(np, "gpio-ranges",
                                "#gpio-range-cells", index, &pinspec);

if (ret)

drivers/gpio/gpiolib-of.c static void of_gpiochip_add_pin_range(struct gpio_chip · line 260

Quoted 9 lines

drivers/gpio/gpiolib-of.c +2 -2

@@ -254,8 +254,8 @@ static void of_gpiochip_add_pin_range(struct gpio_chip
 *chip)
 
                if (ret)
                        break;
-
-       } while (index++);
+               index++;
+       }
  }

WBR, Sergei

Since index++ calculates from 0, the checking condition of "while (index++)" is always fake.

So replace it by unconditional loop.

Signed-off-by Haojian Zhuang <[email protected]>

---
  drivers/gpio/gpiolib-of.c |    6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

drivers/gpio/gpiolib-of.c +1 -1

index b1f0682..011e1e98 100644
@@ -228,7 +228,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
  	if (!np)
  		return;
 
-	do {
+	for (;;) {
  		ret = of_parse_phandle_with_args(np, "gpio-ranges",
  				"#gpio-range-cells", index, &pinspec);
  		if (ret)

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

Signed-off-by Haojian Zhuang <[email protected]>

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

Quoted 3 lines · Haojian Zhuang

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

Why is it more convenient? Just to make the numbers smaller? The reason it uses a descending search is to mimimize the possibility of collision with fixed GPIO numbers.

g.

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 <[email protected]>

drivers/gpio/gpio-pl061.c +4 -0

+static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+       /*
+        * Map back to global GPIO space and request muxing, the direction

drivers/gpio/gpio-pl061.c +3 -0

+       return 0;
+}
+
 static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
 {
        struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);

Quoted 8 lines · 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 <[email protected]>

(...)

Quoted 4 lines

drivers/gpio/gpio-pl061.c +4 -0

+static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+       /*
+        * Map back to global GPIO space and request muxing, the direction

drivers/gpio/gpio-pl061.c struct pl061_gpio { · line 78

Quoted 10 lines

drivers/gpio/gpio-pl061.c +10 -0

+        * parameter does not matter for this controller.
+        */
+       int gpio = chip->base + offset;
+
+       /*
+        * Do NOT check the return value at here. Since sometimes the gpio
+        * pin needn't to be configured in pinmux controller. So it's
+        * impossible to find the matched gpio range.
+        */
+       pinctrl_request_gpio(gpio);

Handling of error code?

(Maybe I should add a __must_check on this function.)

Quoted 7 lines

drivers/gpio/gpio-pl061.c +3 -0

+       return 0;
+}
+
 static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
 {
        struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);

drivers/gpio/gpio-pl061.c static int pl061_probe(struct amba_device *adev, const struct amba_id *id) · line 275

Quoted 7 lines

drivers/gpio/gpio-pl061.c +1 -0

@@ -251,6 +269,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;

What happens on a platform that has a PL061 GPIO block but no pinctrl related to it?

But still has some other pinctrl driver in the platform ....

Right, it'll return -EPROBE_DEFER from pinctrl_request_gpio().

This may happen on for example a combined SPEAr kernel where some platforms have PL061 and others us a pin controller, so both will be enabled.

I think, add a field like this to struct pl061_gpio:

bool has_pinctrl_backend;

The only call that from pl061_gpio_request() if this is set:

if (pl061->has_pinctrl_backend)
ret = pinctrl_request_gpio(gpio);

Then assign it in some clever way. For DT I think the proper way would be so add a cross-binding to the pin controller, like:

gpio2: gpio at d8100000 {
    #gpio-cells = <2>;
    compatible = "arm,pl061", "arm,primecell";
(...)
    pinctrl = <&mr_pincontrol>;
};

Then just check if you have this pinctrl binding set to figure out if has_pinctrl_backend should be true.

Yours,
Linus Walleij

Quoted 6 lines

Quoted 1 line

+ pinctrl_request_gpio(gpio);

Handling of error code?

(Maybe I should add a __must_check on this function.)

My case is a little special. I don't want to check return value because some gpio pins don't have pinmux registers in Hisilicon SoC.
So pinctrl_request_gpio() will always return error for these special pins in Hisilicon SoC.

If we must check the return value, maybe we need append a dummy pinctrl driver for those special gpio pins. How do you think about it? Of course, I need to evaluate
whether it's possible to implement.

Quoted 11 lines · Haojian Zhuang

Quoted 6 lines

Quoted 1 line

+ pinctrl_request_gpio(gpio);

Handling of error code?

(Maybe I should add a __must_check on this function.)

My case is a little special. I don't want to check return value because some gpio pins don't have pinmux registers in Hisilicon SoC.
So pinctrl_request_gpio() will always return error for these special pins in Hisilicon SoC.

Quoted 4 lines

If we must check the return value, maybe we need append a dummy pinctrl driver for those special gpio pins. How do you think about it? Of course, I need to evaluate
whether it's possible to implement.

Hm. A dummy pinctrl back-end is not very elegant.

It's better if the GPIO driver (gpio-pl061) keep track of the ranges that are connected to the pinctrl.

If the ranges are encoded in the device tree (as I guess you want to do in this case) then the GPIO driver need to check these ranges to see if it uses a pinctrl backend. Magic behind-the-scenes is very hard to understand for people reading this code later.

What about this:

In gpiolib.c, function gpiochip_add_pin_range(), we save a copy of each range in &chip->pin_ranges.

Add a function to gpiolib.c to check if a certain gpio is in the range of the current chip.

Then use that:

if (gpio_in_pinrange(gpio))

    pinctrl_request_gpio(gpio);

Yours,
Linus Walleij

Quoted 5 lines

Add a function to gpiolib.c to check if a certain gpio is in the range of the current chip.

Then use that:

Quoted 5 lines

if (gpio_in_pinrange(gpio))

    pinctrl_request_gpio(gpio);

Yours,
Linus Walleij

It seems better. I'll update it.

Replace subsys initcall by module initcall level. Since pinctrl driver is already launched before gpio driver. It's unnecessary to set gpio driver in subsys init call level.

Signed-off-by Haojian Zhuang <[email protected]>

Quoted 5 lines · Haojian Zhuang

Replace subsys initcall by module initcall level. Since pinctrl driver is already launched before gpio driver. It's unnecessary to set gpio driver in subsys init call level.

Signed-off-by Haojian Zhuang <[email protected]>

On you platform maybe it works, but have you made sure that nobody else will be affected?

SPEAr of course, then these:

arch/arm/mach-realview/core.c: * GPIO on PL061 is active,
which is the proper
arch/arm/mach-socfpga/Kconfig: select GPIO_PL061 if GPIOLIB

Pawel, Dinh: are you OK with this change?

Quoted 12 lines · Linus Walleij

Quoted 5 lines · Haojian Zhuang

Replace subsys initcall by module initcall level. Since pinctrl driver is already launched before gpio driver. It's unnecessary to set gpio driver in subsys init call level.

Signed-off-by Haojian Zhuang <[email protected]>

On you platform maybe it works, but have you made sure that nobody else will be affected?

Quoted 7 lines

SPEAr of course, then these:

arch/arm/mach-realview/core.c: * GPIO on PL061 is active,
which is the proper
arch/arm/mach-socfpga/Kconfig: select GPIO_PL061 if GPIOLIB

Pawel, Dinh: are you OK with this change?

Hm. Doesn't this make the MMCI probing depending on the module_init order? As in: wouldn't it make the mmci probe completely fail (not even defer it) if it was called before the pl061? In that case even your, Linus, hack with inverting the CD status wouldn't work...

Pawel

Quoted 1 line

Quoted 1 line

Pawel, Dinh: are you OK with this change?

Still works ok on mach-socfpga.

Quoted 3 lines · Dinh Nguyen

Quoted 1 line

Quoted 1 line

Pawel, Dinh: are you OK with this change?

Still works ok on mach-socfpga.

Thanks! Adding your Tested-by tag, OK?

Yours,
Linus Walleij

Quoted 18 lines · Linus Walleij

Quoted 5 lines · Haojian Zhuang

Replace subsys initcall by module initcall level. Since pinctrl driver is already launched before gpio driver. It's unnecessary to set gpio driver in subsys init call level.

Signed-off-by Haojian Zhuang <[email protected]>

On you platform maybe it works, but have you made sure that nobody else will be affected?

SPEAr of course, then these:

arch/arm/mach-realview/core.c: * GPIO on PL061 is active,
which is the proper
arch/arm/mach-socfpga/Kconfig: select GPIO_PL061 if GPIOLIB

Quoted 1 line

Pawel, Dinh: are you OK with this change?

Hm. Doesn't this make the MMCI probing depending on the module_init order? As in: wouldn't it make the mmci probe completely fail (not even defer it) if it was called before the pl061? In that case even your, Linus, hack with inverting the CD status wouldn't work...

Pawel

The sequence of module probe is pinctrl --> gpio --> mmc. So the dependance of mmc on gpio isn't broken.

Hm. Doesn't this make the MMCI probing depending on the module_init order? As in: wouldn't it make the mmci probe completely fail (not even defer it) if it was called before the pl061? In that case even your, Linus, hack with inverting the CD status wouldn't work...

AFAICT gpio_request() will return -EPROBE_DEFER if the GPIO cannot be located.

Quoted 4 lines · Pawel Moll

Hm. Doesn't this make the MMCI probing depending on the module_init order? As in: wouldn't it make the mmci probe completely fail (not even defer it) if it was called before the pl061? In that case even your, Linus, hack with inverting the CD status wouldn't work...

According to Haojian it works, but for sure the MMCI driver *should* (on the eternal list of SHOULDDO) bail out and return any

Quoted 7 lines · Linus Walleij

Quoted 4 lines · Pawel Moll

Hm. Doesn't this make the MMCI probing depending on the module_init order? As in: wouldn't it make the mmci probe completely fail (not even defer it) if it was called before the pl061? In that case even your, Linus, hack with inverting the CD status wouldn't work...

Quoted 2 lines

According to Haojian it works, but for sure the MMCI driver *should* (on the eternal list of SHOULDDO) bail out and return any

Rather than talking about what should and should not, why not look at the code - it only takes a few moments to check what the behaviour would be. And it is correct - errors are correctly propagated out of the probe function.

That's hardly surprising given who's supposed to be the maintainer of that driver.

Replace subsys initcall by module initcall level. Since pinctrl driver is already launched before gpio driver. It's unnecessary to set gpio driver in subsys init call level.

Signed-off-by Haojian Zhuang <[email protected]>

Add PIN_CONFIG_SLEW_RATE parameter into pinconf-generic driver.

Signed-off-by Haojian Zhuang <[email protected]>

drivers/pinctrl/pinctrl-single.c +4 -0

+static int pcs_parse_pinconf(struct pcs_device *pcs, struct device_node *np,
+			     struct pcs_function *func,
+			     struct pinctrl_map **map)
+

Hi,

Quoted 11 lines · Haojian Zhuang

drivers/pinctrl/pinctrl-single.c +11 -0

+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,power-source", PIN_CONFIG_POWER_SOURCE, },

drivers/pinctrl/pinctrl-single.c static int pcs_get_pin_by_offset(struct pcs_device *pcs, unsigned offset) · line 949

Quoted 3 lines

drivers/pinctrl/pinctrl-single.c +3 -0

+             { "pinctrl-single,slew-rate", PIN_CONFIG_SLEW_RATE, },
+             { "pinctrl-single,input-schmitt", PIN_CONFIG_INPUT_SCHMITT, },
+     };

The PIN_CONFIG_POWER_SOURCE should be split to PIN_CONFIG_POWER_ENABLE and PIN_CONFIG_POWER_SOURCE. If you only have one bit for it, I guess then you can just use PIN_CONFIG_POWER_ENABLE and make PIN_CONFIG_POWER_SOURCE a NOP.

I also suggest we standardize on PIN_CONFIG_*_ENABLE on the naming instead of PIN_CONFIG_*_DISABLE. Many of these features enable some input logic, and by default they should be off to save power.

drivers/pinctrl/pinctrl-single.c static int pcs_get_pin_by_offset(struct pcs_device *pcs, unsigned offset) · line 956

Quoted 7 lines

drivers/pinctrl/pinctrl-single.c +7 -0

+     struct pcs_conf_type prop3[] = {
+             { "pinctrl-single,bias-disable", PIN_CONFIG_BIAS_DISABLE, },
+             { "pinctrl-single,bias-pullup", PIN_CONFIG_BIAS_PULL_UP, },
+             { "pinctrl-single,bias-pulldown", PIN_CONFIG_BIAS_PULL_DOWN, },
+             { "pinctrl-single,input-schmitt-disable",
+                     PIN_CONFIG_INPUT_SCHMITT_DISABLE, },
+     };

I'm not aware of cases where we need both INPUT_SCHMITT and INPUT_SCHMITT_DISABLE, so we may just want to have INPUT_SCHMITT_ENABLE if that works for you.

Other than that, looks good to me. I'll update my patches here and do some tests against my pinctrl-single,bits testcase.

Regards,

Tony

Quoted 14 lines · Tony Lindgren

Hi,

Quoted 11 lines · Haojian Zhuang

drivers/pinctrl/pinctrl-single.c +11 -0

+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,power-source", PIN_CONFIG_POWER_SOURCE, },

Quoted 9 lines

drivers/pinctrl/pinctrl-single.c static int pcs_get_pin_by_offset(struct pcs_device *pcs, unsigned offset) · line 949

Quoted 3 lines

drivers/pinctrl/pinctrl-single.c +3 -0

+             { "pinctrl-single,slew-rate", PIN_CONFIG_SLEW_RATE, },
+             { "pinctrl-single,input-schmitt", PIN_CONFIG_INPUT_SCHMITT, },
+     };

The PIN_CONFIG_POWER_SOURCE should be split to PIN_CONFIG_POWER_ENABLE and PIN_CONFIG_POWER_SOURCE. If you only have one bit for it, I guess then you can just use PIN_CONFIG_POWER_ENABLE and make PIN_CONFIG_POWER_SOURCE a NOP.

In Hisilicon Hi3620 SoC, 4bits are used for power source. b0000 is 2mA, b0001 is 4mA, .... b1111 is 8mA.
There's no power enable bit. In Marvell silicon, it's also same.

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

Quoted 11 lines · Haojian Zhuang

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

Linus, I'll let you make decisions on this patch series since it is mostly pinctrl related.

g.