[PATCH v4 0/4] ARM: hip04: add GPIO support

[PATCH v4 0/4] ARM: hip04: add GPIO support

[Zhou Wang]

This series add the support for the GPIOs of Hisilicon Soc hip04. Hip04 uses
synopsis' GPIO IP, and we use the dwapb GPIO driver here. This series add the
corresponding dts.

The 3/4 patch supports to find GPIO base number in increasing order,
if gpio-number-forward is defined in related dts nodes. If there are some
GPIO controllers in one Soc, all related GPIO dts node should contain
gpio-number-forward property. If not, error will come out.

Change in v4:
- Delete "default 128 if ARCH_HIP04" in arch/arm/Kconfig
- add a new patch to support find GPIO base number in increasing order
- add gpio-number-forward property in snps gpio binding doc
Change in v3:
- Move CONFIG_GPIO_DWAPB to hisi_defconfig
- Add CONFIG_GPIOLIB, CONFIG_GPIO_SYSFS in hisi_defconfig
Change in v2:
- Add select GPIO_DWAPB in arch/arm/mach-hisi/Kconfig

Link for v3:
- http://www.spinics.net/lists/devicetree/msg60324.html

Zhou Wang (4):
  ARM: hip04: add GPIO configure in hisi_defconfig
  ARM: dts: hip04: add GPIO pieces
  gpio: Add find base in a increasing order
  Documentation: dt: gpio: Add gpio-number-forward property in snps
    gpio binding doc

 .../devicetree/bindings/gpio/snps-dwapb-gpio.txt   |    2 +
 arch/arm/boot/dts/hip04.dtsi                       |   79 ++++++++++++++++++++
 arch/arm/configs/hisi_defconfig                    |    3 +
 drivers/gpio/gpiolib.c                             |   63 +++++++++++++---
 4 files changed, 138 insertions(+), 9 deletions(-)

-- 
1.7.9.5

[PATCH v4 1/4] ARM: hip04: add GPIO configure in hisi_defconfig

[Zhou Wang]

The GPIO controller of Hisilicon hip04 Soc uses the IP from Sysnopsis,
gpio-dwapb.c can be reused to support this GPIO controller. This patch
adds related configure items in hisi_defconfig.

Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
---
 arch/arm/configs/hisi_defconfig |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/hisi_defconfig b/arch/arm/configs/hisi_defconfig
index 1772505..130292b 100644
--- a/arch/arm/configs/hisi_defconfig
+++ b/arch/arm/configs/hisi_defconfig
@@ -41,6 +41,9 @@ CONFIG_SPI_PL022=y
 CONFIG_PINCTRL_SINGLE=y
 CONFIG_GPIO_GENERIC_PLATFORM=y
 CONFIG_REGULATOR_GPIO=y
+CONFIG_GPIOLIB=y
+CONFIG_GPIO_DWAPB=y
+CONFIG_GPIO_SYSFS=y
 CONFIG_DRM=y
 CONFIG_FB_SIMPLE=y
 CONFIG_USB=y
-- 
1.7.9.5

[PATCH v4 2/4] ARM: dts: hip04: add GPIO pieces

[Zhou Wang]

Hisilicon Soc hip04 has four GPIO controllers, each one has 32
GPIOs and can be configured to be an interrupt controller.The GPIO
controllers are compatible with the snps,dw-apb-gpio driver.
This patch add the corresponding device tree nodes.

A new property caller "gpio-number-forward" has been added in
dw-apb-gpio dts node to support to get base number of a GPIO
controller in increasing order.

Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
---
 arch/arm/boot/dts/hip04.dtsi |   79 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
index 93b6c90..811ca7b 100644
--- a/arch/arm/boot/dts/hip04.dtsi
+++ b/arch/arm/boot/dts/hip04.dtsi
@@ -263,5 +263,84 @@
 			interrupts = <0 372 4>;
 		};
 
+		gpio@4000000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4000000 0x1000>;
+			gpio-number-forward;
+
+			gpio0: gpio-controller@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				gpio-controller;
+				#gpio-cells = <2>;
+				snps,nr-gpios = <32>;
+				reg = <0>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 389 4>;
+			};
+		};
+
+		gpio@4001000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4001000 0x1000>;
+			gpio-number-forward;
+
+			gpio1: gpio-controller@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				gpio-controller;
+				#gpio-cells = <2>;
+				snps,nr-gpios = <32>;
+				reg = <0>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 390 4>;
+			};
+		};
+
+		gpio@4002000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4002000 0x1000>;
+			gpio-number-forward;
+
+			gpio2: gpio-controller@0 {
+			compatible = "snps,dw-apb-gpio-port";
+				gpio-controller;
+				#gpio-cells = <2>;
+				snps,nr-gpios = <32>;
+				reg = <0>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 391 4>;
+			};
+		};
+
+		gpio@4003000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4003000 0x1000>;
+			gpio-number-forward;
+
+			gpio3: gpio-controller@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				gpio-controller;
+				#gpio-cells = <2>;
+				snps,nr-gpios = <32>;
+				reg = <0>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 392 4>;
+			};
+		};
 	};
 };
-- 
1.7.9.5

[PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Zhou Wang]

In function gpiochip_find_base, base number of a GPIO controller
is found in decreasing order. ARCH_NR_GPIOS is used to define from
which number we begin to search for base number of a GPIO controller.

In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
http://www.spinics.net/lists/devicetree/msg60433.html

This patch adds the support to find base number of a GPIO controller
in increasing order. It will assign base number from 0.
A new dts property called gpio-number-forward must be add to the related
GPIO dts nodes if you want it works well.

Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
---
 drivers/gpio/gpiolib.c |   63 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8e98ca..d5e8e13 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -55,6 +55,15 @@ static DEFINE_MUTEX(gpio_lookup_lock);
 static LIST_HEAD(gpio_lookup_list);
 LIST_HEAD(gpio_chips);
 
+/* check find_base_order to find if prior GPIO controller and current GPIO
+ * controller find base number in different orders. Backward is default.
+ */
+static enum {
+	gpio_number_backward,
+	gpio_number_forward,
+	gpio_order_different,
+} find_base_order;
+
 static inline void desc_set_label(struct gpio_desc *d, const char *label)
 {
 	d->label = label;
@@ -107,18 +116,54 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
 EXPORT_SYMBOL_GPL(gpiod_to_chip);
 
 /* dynamic allocation of GPIOs, e.g. on a hotplugged device */
-static int gpiochip_find_base(int ngpio)
+static int gpiochip_find_base(struct gpio_chip *gpio_chip)
 {
+	int base;
 	struct gpio_chip *chip;
-	int base = ARCH_NR_GPIOS - ngpio;
+	int ngpio = gpio_chip->ngpio;
 
-	list_for_each_entry_reverse(chip, &gpio_chips, list) {
-		/* found a free space? */
-		if (chip->base + chip->ngpio <= base)
-			break;
-		else
+	if (find_base_order == gpio_order_different) {
+		pr_err("%s: stop adding gpio chip\n", __func__);
+		return -EINVAL;
+	}
+
+	if (of_property_read_bool(gpio_chip->dev->of_node,
+	    "gpio-number-forward")) {
+		/* find base in increasing order */
+		base = 0;
+
+		if (!list_empty(&gpio_chips)) {
+			if (find_base_order == gpio_number_backward) {
+				find_base_order = gpio_order_different;
+				pr_err("%s: find base in different order\n",
+					__func__);
+				return -EINVAL;
+			}
+			chip = list_last_entry(&gpio_chips, struct gpio_chip,
+						list);
+			base = chip->base + chip->ngpio;
+		}
+
+		find_base_order = gpio_number_forward;
+	} else {
+		if (find_base_order == gpio_number_forward) {
+			find_base_order = gpio_order_different;
+			pr_err("%s: find base in different order\n", __func__);
+			return -EINVAL;
+		}
+
+		base = ARCH_NR_GPIOS - ngpio;
+
+		list_for_each_entry_reverse(chip, &gpio_chips, list) {
+			/* found a free space? */
+			if (chip->base + chip->ngpio <= base)
+				break;
+			else
 			/* nope, check the space right before the chip */
-			base = chip->base - ngpio;
+				base = chip->base - ngpio;
+		}
+
+		find_base_order = gpio_number_backward;
 	}
 
 	if (gpio_is_valid(base)) {
@@ -236,7 +281,7 @@ int gpiochip_add(struct gpio_chip *chip)
 	spin_lock_irqsave(&gpio_lock, flags);
 
 	if (base < 0) {
-		base = gpiochip_find_base(chip->ngpio);
+		base = gpiochip_find_base(chip);
 		if (base < 0) {
 			status = base;
 			goto unlock;
-- 
1.7.9.5

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Alexandre Courbot]

On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
> In function gpiochip_find_base, base number of a GPIO controller
> is found in decreasing order. ARCH_NR_GPIOS is used to define from
> which number we begin to search for base number of a GPIO controller.
>
> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
> http://www.spinics.net/lists/devicetree/msg60433.html
>
> This patch adds the support to find base number of a GPIO controller
> in increasing order. It will assign base number from 0.
> A new dts property called gpio-number-forward must be add to the related
> GPIO dts nodes if you want it works well.

Global GPIO numbers are a Linux-only concept, so your property should
be named linux,gpio-number-forward.

But even that way I don't think I like this idea. This just adds some
more mess to how the GPIO number space is constructed, and it is
already quite messy as it is. You have no guarantee over the probe
order of your GPIO controllers, so they may very well be probed in a
different order and end up with different base numbers anytime.

Not that this is your fault - the number namespace is broken by design
and I don't think there is a way to fix it. The long-term solution is
to stop using it by switching to the gpiod interface.

First a few questions to understand why you need your GPIOs numbered
this way, and if you need it at all:
- Can't you use the gpiod interface instead so you don't need to rely
on GPIO numbers?
- Do you plan to use the sysfs interface? If so then we are screwed,
because there is no way to use it without using global GPIO numbers.
This is something we should/will fix with named exported GPIOs, but we
are not here yet.

As to how we can solve your problem: if you must use GPIO integers
(because you need to use the sysfs interface for instance), and need
them affected consistently, the only way I can think of is to
introduce a "linux,gpio-base" property that will set gpiochip->base to
a fixed number. It still kind of sucks, but at least it will enforce
that different controllers get the same base number reliably, and the
firmware is the best placed to know how many GPIOs each controller has
and decide an appropriate layout.

I know we pushed back against this kind of solution in the past, but
it is still more reliable than having a property that affects how the
kernel's base finding function works, and will offer us a way to
eventually put ARCH_NR_GPIOS and gpiochip_find_base() to rest (at the
cost of backwards-compatibility, but we just cannot do without it).
Linus, do you agree?

In the medium term, we need to offer a solution for user-space to
*not* rely on GPIO numbers, and that will necessarily go through a new
sysfs interface, since at the moment even GPIO chips use their base
number in their exported name.

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Zhou Wang]

On 2014年12月10日 16:51, Alexandre Courbot wrote:
> On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
>> In function gpiochip_find_base, base number of a GPIO controller
>> is found in decreasing order. ARCH_NR_GPIOS is used to define from
>> which number we begin to search for base number of a GPIO controller.
>>
>> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
>> http://www.spinics.net/lists/devicetree/msg60433.html
>>
>> This patch adds the support to find base number of a GPIO controller
>> in increasing order. It will assign base number from 0.
>> A new dts property called gpio-number-forward must be add to the related
>> GPIO dts nodes if you want it works well.
>
> Global GPIO numbers are a Linux-only concept, so your property should
> be named linux,gpio-number-forward.
>
> But even that way I don't think I like this idea. This just adds some
> more mess to how the GPIO number space is constructed, and it is
> already quite messy as it is. You have no guarantee over the probe
> order of your GPIO controllers, so they may very well be probed in a
> different order and end up with different base numbers anytime.
>
> Not that this is your fault - the number namespace is broken by design
> and I don't think there is a way to fix it. The long-term solution is
> to stop using it by switching to the gpiod interface.
>
> First a few questions to understand why you need your GPIOs numbered
> this way, and if you need it at all:
> - Can't you use the gpiod interface instead so you don't need to rely
> on GPIO numbers?

Hi Alexandre,

Sorry for late. Could you give me more clue about the gpiod interface? 
Don't we also call gpio_request() which uses GPIO number to request a
GPIO?

> - Do you plan to use the sysfs interface? If so then we are screwed,
> because there is no way to use it without using global GPIO numbers.

I am now enabling GPIO in Hip04-d01. Maybe, I can just use
the default ARCH_NR_GPIOS, then users can manage GPIO through sysfs.
However if so, GPIO 0~127 will be mapped to GPIO 384~511.

> This is something we should/will fix with named exported GPIOs, but we
> are not here yet.
>
> As to how we can solve your problem: if you must use GPIO integers
> (because you need to use the sysfs interface for instance), and need
> them affected consistently, the only way I can think of is to
> introduce a "linux,gpio-base" property that will set gpiochip->base to
> a fixed number. It still kind of sucks, but at least it will enforce

Thanks for your suggestion. But setting "linux,gpio-base" will bring
gpio base implementation specific, and in face there is no place to gain
this gpio base info, not appropriate both in gpio arch code and dwapb
code.

Thanks again for your comments,
Zhou Wang

> that different controllers get the same base number reliably, and the
> firmware is the best placed to know how many GPIOs each controller has
> and decide an appropriate layout.
>
> I know we pushed back against this kind of solution in the past, but
> it is still more reliable than having a property that affects how the
> kernel's base finding function works, and will offer us a way to
> eventually put ARCH_NR_GPIOS and gpiochip_find_base() to rest (at the
> cost of backwards-compatibility, but we just cannot do without it).
> Linus, do you agree?
>
> In the medium term, we need to offer a solution for user-space to
> *not* rely on GPIO numbers, and that will necessarily go through a new
> sysfs interface, since at the moment even GPIO chips use their base
> number in their exported name.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Alexandre Courbot]

On Mon, Dec 15, 2014 at 1:01 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
> On 2014年12月10日 16:51, Alexandre Courbot wrote:
>>
>> On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
>>>
>>> In function gpiochip_find_base, base number of a GPIO controller
>>> is found in decreasing order. ARCH_NR_GPIOS is used to define from
>>> which number we begin to search for base number of a GPIO controller.
>>>
>>> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
>>> http://www.spinics.net/lists/devicetree/msg60433.html
>>>
>>> This patch adds the support to find base number of a GPIO controller
>>> in increasing order. It will assign base number from 0.
>>> A new dts property called gpio-number-forward must be add to the related
>>> GPIO dts nodes if you want it works well.
>>
>>
>> Global GPIO numbers are a Linux-only concept, so your property should
>> be named linux,gpio-number-forward.
>>
>> But even that way I don't think I like this idea. This just adds some
>> more mess to how the GPIO number space is constructed, and it is
>> already quite messy as it is. You have no guarantee over the probe
>> order of your GPIO controllers, so they may very well be probed in a
>> different order and end up with different base numbers anytime.
>>
>> Not that this is your fault - the number namespace is broken by design
>> and I don't think there is a way to fix it. The long-term solution is
>> to stop using it by switching to the gpiod interface.
>>
>> First a few questions to understand why you need your GPIOs numbered
>> this way, and if you need it at all:
>> - Can't you use the gpiod interface instead so you don't need to rely
>> on GPIO numbers?
>
>
> Hi Alexandre,
>
> Sorry for late. Could you give me more clue about the gpiod interface? Don't
> we also call gpio_request() which uses GPIO number to request a
> GPIO?

See Documentation/gpio/consumer.txt and Documentation/gpio/board.txt.
You can obtain a GPIO descriptor without using a number by calling
gpiod_get(). Prior to that, individual GPIOs need to be bound to
devices and functions using either DT (preferred) or the platform
interface. The old integer-based GPIO interface is considered
deprecated, although still widely used. But new code should rely
exclusively on gpiod, and we encourage people to convert existing code
to it too.

>
>> - Do you plan to use the sysfs interface? If so then we are screwed,
>> because there is no way to use it without using global GPIO numbers.
>
>
> I am now enabling GPIO in Hip04-d01. Maybe, I can just use
> the default ARCH_NR_GPIOS, then users can manage GPIO through sysfs.
> However if so, GPIO 0~127 will be mapped to GPIO 384~511.

Yeah, I know that's not ideal. As a workaround, users can maybe
identify the right gpiochip by parsing /sys/class/gpio/gpiochip* and
comparing the "label" node. Once the right chip is found, its "base"
entry gives the base GPIO number which can be used to export the
desired GPIO.

We are also taking steps to come with a better sysfs interface. I will
keep you in the loop.

>
>> This is something we should/will fix with named exported GPIOs, but we
>> are not here yet.
>>
>> As to how we can solve your problem: if you must use GPIO integers
>> (because you need to use the sysfs interface for instance), and need
>> them affected consistently, the only way I can think of is to
>> introduce a "linux,gpio-base" property that will set gpiochip->base to
>> a fixed number. It still kind of sucks, but at least it will enforce
>
>
> Thanks for your suggestion. But setting "linux,gpio-base" will bring
> gpio base implementation specific, and in face there is no place to gain
> this gpio base info, not appropriate both in gpio arch code and dwapb
> code.

Yeah, besides this property did not receive a warm reception. So my
suggestion for now is to workaround the issue using the technique
described above, until we come with a better sysfs interface that does
not rely on GPIO numbers. Sorry for that inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Zhou Wang]

On 2014年12月17日 11:09, Alexandre Courbot wrote:
> On Mon, Dec 15, 2014 at 1:01 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
>> On 2014年12月10日 16:51, Alexandre Courbot wrote:
>>>
>>> On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
>>>>
>>>> In function gpiochip_find_base, base number of a GPIO controller
>>>> is found in decreasing order. ARCH_NR_GPIOS is used to define from
>>>> which number we begin to search for base number of a GPIO controller.
>>>>
>>>> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
>>>> http://www.spinics.net/lists/devicetree/msg60433.html
>>>>
>>>> This patch adds the support to find base number of a GPIO controller
>>>> in increasing order. It will assign base number from 0.
>>>> A new dts property called gpio-number-forward must be add to the related
>>>> GPIO dts nodes if you want it works well.
>>>
>>>
>>> Global GPIO numbers are a Linux-only concept, so your property should
>>> be named linux,gpio-number-forward.
>>>
>>> But even that way I don't think I like this idea. This just adds some
>>> more mess to how the GPIO number space is constructed, and it is
>>> already quite messy as it is. You have no guarantee over the probe
>>> order of your GPIO controllers, so they may very well be probed in a
>>> different order and end up with different base numbers anytime.
>>>
>>> Not that this is your fault - the number namespace is broken by design
>>> and I don't think there is a way to fix it. The long-term solution is
>>> to stop using it by switching to the gpiod interface.
>>>
>>> First a few questions to understand why you need your GPIOs numbered
>>> this way, and if you need it at all:
>>> - Can't you use the gpiod interface instead so you don't need to rely
>>> on GPIO numbers?
>>
>>
>> Hi Alexandre,
>>
>> Sorry for late. Could you give me more clue about the gpiod interface? Don't
>> we also call gpio_request() which uses GPIO number to request a
>> GPIO?
>
> See Documentation/gpio/consumer.txt and Documentation/gpio/board.txt.
> You can obtain a GPIO descriptor without using a number by calling
> gpiod_get(). Prior to that, individual GPIOs need to be bound to
> devices and functions using either DT (preferred) or the platform
> interface. The old integer-based GPIO interface is considered
> deprecated, although still widely used. But new code should rely
> exclusively on gpiod, and we encourage people to convert existing code
> to it too.
>

Hi Alexandre,

Thanks a lot for your explanation!!

>>
>>> - Do you plan to use the sysfs interface? If so then we are screwed,
>>> because there is no way to use it without using global GPIO numbers.
>>
>>
>> I am now enabling GPIO in Hip04-d01. Maybe, I can just use
>> the default ARCH_NR_GPIOS, then users can manage GPIO through sysfs.
>> However if so, GPIO 0~127 will be mapped to GPIO 384~511.
>
> Yeah, I know that's not ideal. As a workaround, users can maybe
> identify the right gpiochip by parsing /sys/class/gpio/gpiochip* and
> comparing the "label" node. Once the right chip is found, its "base"
> entry gives the base GPIO number which can be used to export the
> desired GPIO.
>
> We are also taking steps to come with a better sysfs interface. I will
> keep you in the loop.
>

Thanks. I'd like to keep an eye on this.

Regard,
Zhou

>>
>>> This is something we should/will fix with named exported GPIOs, but we
>>> are not here yet.
>>>
>>> As to how we can solve your problem: if you must use GPIO integers
>>> (because you need to use the sysfs interface for instance), and need
>>> them affected consistently, the only way I can think of is to
>>> introduce a "linux,gpio-base" property that will set gpiochip->base to
>>> a fixed number. It still kind of sucks, but at least it will enforce
>>
>>
>> Thanks for your suggestion. But setting "linux,gpio-base" will bring
>> gpio base implementation specific, and in face there is no place to gain
>> this gpio base info, not appropriate both in gpio arch code and dwapb
>> code.
>
> Yeah, besides this property did not receive a warm reception. So my
> suggestion for now is to workaround the issue using the technique
> described above, until we come with a better sysfs interface that does
> not rely on GPIO numbers. Sorry for that inconvenience.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Linus Walleij]

On Wed, Dec 10, 2014 at 9:51 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
>> In function gpiochip_find_base, base number of a GPIO controller
>> is found in decreasing order. ARCH_NR_GPIOS is used to define from
>> which number we begin to search for base number of a GPIO controller.
>>
>> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
>> http://www.spinics.net/lists/devicetree/msg60433.html
>>
>> This patch adds the support to find base number of a GPIO controller
>> in increasing order. It will assign base number from 0.
>> A new dts property called gpio-number-forward must be add to the related
>> GPIO dts nodes if you want it works well.
>
> Global GPIO numbers are a Linux-only concept, so your property should
> be named linux,gpio-number-forward.
>
> But even that way I don't think I like this idea. This just adds some
> more mess to how the GPIO number space is constructed, and it is
> already quite messy as it is. You have no guarantee over the probe
> order of your GPIO controllers, so they may very well be probed in a
> different order and end up with different base numbers anytime.

Yup.

The way stuff is usually forced ordered in device tree is to use
aliases, e.g.:

        aliases {
                serial0 = &pb1176_serial0;
                serial1 = &pb1176_serial1;
                serial2 = &pb1176_serial2;
                serial3 = &pb1176_serial3;
                serial4 = &fpga_serial;
        };

By getting the alias ID with of_alias_get_id(np, "serial")
a certain serial port is assigned to be
0, 1, 2 ...

I think I could accept a tweak of this patch that will register
GPIOs beginning from 0 if and only if the alias mechanism is
used.

        aliases {
                gpio0 = &foo_gpio0;
                gpio1 = &expander;
        };


The actual Linux-specific integer base will *NOT* be in the
device tree, but if you know how many GPIOs are on each
controller the number space is still stable and predictable
beginning from 0. If one want to keep track of the Linux
number space one can do so with comments in the device
tree or something.

> I know we pushed back against this kind of solution in the past, but
> it is still more reliable than having a property that affects how the
> kernel's base finding function works, and will offer us a way to
> eventually put ARCH_NR_GPIOS and gpiochip_find_base() to rest (at the
> cost of backwards-compatibility, but we just cannot do without it).
> Linus, do you agree?

What do you think about the above?

Yours,
Linus Walleij

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Alexandre Courbot]

On Wed, Jan 14, 2015 at 5:13 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Dec 10, 2014 at 9:51 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
>>> In function gpiochip_find_base, base number of a GPIO controller
>>> is found in decreasing order. ARCH_NR_GPIOS is used to define from
>>> which number we begin to search for base number of a GPIO controller.
>>>
>>> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
>>> http://www.spinics.net/lists/devicetree/msg60433.html
>>>
>>> This patch adds the support to find base number of a GPIO controller
>>> in increasing order. It will assign base number from 0.
>>> A new dts property called gpio-number-forward must be add to the related
>>> GPIO dts nodes if you want it works well.
>>
>> Global GPIO numbers are a Linux-only concept, so your property should
>> be named linux,gpio-number-forward.
>>
>> But even that way I don't think I like this idea. This just adds some
>> more mess to how the GPIO number space is constructed, and it is
>> already quite messy as it is. You have no guarantee over the probe
>> order of your GPIO controllers, so they may very well be probed in a
>> different order and end up with different base numbers anytime.
>
> Yup.
>
> The way stuff is usually forced ordered in device tree is to use
> aliases, e.g.:
>
>         aliases {
>                 serial0 = &pb1176_serial0;
>                 serial1 = &pb1176_serial1;
>                 serial2 = &pb1176_serial2;
>                 serial3 = &pb1176_serial3;
>                 serial4 = &fpga_serial;
>         };
>
> By getting the alias ID with of_alias_get_id(np, "serial")
> a certain serial port is assigned to be
> 0, 1, 2 ...
>
> I think I could accept a tweak of this patch that will register
> GPIOs beginning from 0 if and only if the alias mechanism is
> used.
>
>         aliases {
>                 gpio0 = &foo_gpio0;
>                 gpio1 = &expander;
>         };
>
>
> The actual Linux-specific integer base will *NOT* be in the
> device tree, but if you know how many GPIOs are on each
> controller the number space is still stable and predictable
> beginning from 0. If one want to keep track of the Linux
> number space one can do so with comments in the device
> tree or something.
>
>> I know we pushed back against this kind of solution in the past, but
>> it is still more reliable than having a property that affects how the
>> kernel's base finding function works, and will offer us a way to
>> eventually put ARCH_NR_GPIOS and gpiochip_find_base() to rest (at the
>> cost of backwards-compatibility, but we just cannot do without it).
>> Linus, do you agree?
>
> What do you think about the above?

It would definitely help with the probe order, but unfortunately not
with the fact that GPIO bases are allocated in decreasing order
starting from the potentially varying ARCH_NR_GPIOS. So that alone
will not be able to ensure stable GPIO numbers.

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Linus Walleij]

On Wed, Jan 14, 2015 at 9:17 AM, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Jan 14, 2015 at 5:13 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Dec 10, 2014 at 9:51 AM, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> In function gpiochip_find_base, base number of a GPIO controller
>>>> is found in decreasing order. ARCH_NR_GPIOS is used to define from
>>>> which number we begin to search for base number of a GPIO controller.
>>>>
>>>> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
>>>> http://www.spinics.net/lists/devicetree/msg60433.html
>>>>
>>>> This patch adds the support to find base number of a GPIO controller
>>>> in increasing order. It will assign base number from 0.
>>>> A new dts property called gpio-number-forward must be add to the related
>>>> GPIO dts nodes if you want it works well.
>>>
>>> Global GPIO numbers are a Linux-only concept, so your property should
>>> be named linux,gpio-number-forward.
>>>
>>> But even that way I don't think I like this idea. This just adds some
>>> more mess to how the GPIO number space is constructed, and it is
>>> already quite messy as it is. You have no guarantee over the probe
>>> order of your GPIO controllers, so they may very well be probed in a
>>> different order and end up with different base numbers anytime.
>>
>> Yup.
>>
>> The way stuff is usually forced ordered in device tree is to use
>> aliases, e.g.:
>>
>>         aliases {
>>                 serial0 = &pb1176_serial0;
>>                 serial1 = &pb1176_serial1;
>>                 serial2 = &pb1176_serial2;
>>                 serial3 = &pb1176_serial3;
>>                 serial4 = &fpga_serial;
>>         };
>>
>> By getting the alias ID with of_alias_get_id(np, "serial")
>> a certain serial port is assigned to be
>> 0, 1, 2 ...
>>
>> I think I could accept a tweak of this patch that will register
>> GPIOs beginning from 0 if and only if the alias mechanism is
>> used.
>>
>>         aliases {
>>                 gpio0 = &foo_gpio0;
>>                 gpio1 = &expander;
>>         };
>>
>>
>> The actual Linux-specific integer base will *NOT* be in the
>> device tree, but if you know how many GPIOs are on each
>> controller the number space is still stable and predictable
>> beginning from 0. If one want to keep track of the Linux
>> number space one can do so with comments in the device
>> tree or something.
>>
>>> I know we pushed back against this kind of solution in the past, but
>>> it is still more reliable than having a property that affects how the
>>> kernel's base finding function works, and will offer us a way to
>>> eventually put ARCH_NR_GPIOS and gpiochip_find_base() to rest (at the
>>> cost of backwards-compatibility, but we just cannot do without it).
>>> Linus, do you agree?
>>
>> What do you think about the above?
>
> It would definitely help with the probe order, but unfortunately not
> with the fact that GPIO bases are allocated in decreasing order
> starting from the potentially varying ARCH_NR_GPIOS. So that alone
> will not be able to ensure stable GPIO numbers.

What I mean is that if and only if aliases are used, we
allocate GPIOs in ascending order starting at 0.

Let us remember why this is done: dynamic GPIO numbers are
allocated in descending order because it was assumed that some
static GPIOs are already allocated starting from 0, like on-chip
GPIOs.

We can also make it a Kconfig option, because for a large chunk
of systems only using device tree we can
actually allocate from zero. Notably any ARCH_MULTIPLATFORM
should just do this if we can make a quick survey of such systems
and see that they don't fool around setting .base in their GPIO
chips.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] gpio: Add find GPIO base in increasing order

[Alexandre Courbot]

On Sat, Jan 17, 2015 at 12:24 AM, Linus Walleij
<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Jan 14, 2015 at 9:17 AM, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Wed, Jan 14, 2015 at 5:13 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Dec 10, 2014 at 9:51 AM, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On Fri, Dec 5, 2014 at 12:38 PM, Zhou Wang <wangzhou.bry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> In function gpiochip_find_base, base number of a GPIO controller
>>>>> is found in decreasing order. ARCH_NR_GPIOS is used to define from
>>>>> which number we begin to search for base number of a GPIO controller.
>>>>>
>>>>> In fact, ARCH_NR_GPIOS brings us some multiplatform problems, like:
>>>>> http://www.spinics.net/lists/devicetree/msg60433.html
>>>>>
>>>>> This patch adds the support to find base number of a GPIO controller
>>>>> in increasing order. It will assign base number from 0.
>>>>> A new dts property called gpio-number-forward must be add to the related
>>>>> GPIO dts nodes if you want it works well.
>>>>
>>>> Global GPIO numbers are a Linux-only concept, so your property should
>>>> be named linux,gpio-number-forward.
>>>>
>>>> But even that way I don't think I like this idea. This just adds some
>>>> more mess to how the GPIO number space is constructed, and it is
>>>> already quite messy as it is. You have no guarantee over the probe
>>>> order of your GPIO controllers, so they may very well be probed in a
>>>> different order and end up with different base numbers anytime.
>>>
>>> Yup.
>>>
>>> The way stuff is usually forced ordered in device tree is to use
>>> aliases, e.g.:
>>>
>>>         aliases {
>>>                 serial0 = &pb1176_serial0;
>>>                 serial1 = &pb1176_serial1;
>>>                 serial2 = &pb1176_serial2;
>>>                 serial3 = &pb1176_serial3;
>>>                 serial4 = &fpga_serial;
>>>         };
>>>
>>> By getting the alias ID with of_alias_get_id(np, "serial")
>>> a certain serial port is assigned to be
>>> 0, 1, 2 ...
>>>
>>> I think I could accept a tweak of this patch that will register
>>> GPIOs beginning from 0 if and only if the alias mechanism is
>>> used.
>>>
>>>         aliases {
>>>                 gpio0 = &foo_gpio0;
>>>                 gpio1 = &expander;
>>>         };
>>>
>>>
>>> The actual Linux-specific integer base will *NOT* be in the
>>> device tree, but if you know how many GPIOs are on each
>>> controller the number space is still stable and predictable
>>> beginning from 0. If one want to keep track of the Linux
>>> number space one can do so with comments in the device
>>> tree or something.
>>>
>>>> I know we pushed back against this kind of solution in the past, but
>>>> it is still more reliable than having a property that affects how the
>>>> kernel's base finding function works, and will offer us a way to
>>>> eventually put ARCH_NR_GPIOS and gpiochip_find_base() to rest (at the
>>>> cost of backwards-compatibility, but we just cannot do without it).
>>>> Linus, do you agree?
>>>
>>> What do you think about the above?
>>
>> It would definitely help with the probe order, but unfortunately not
>> with the fact that GPIO bases are allocated in decreasing order
>> starting from the potentially varying ARCH_NR_GPIOS. So that alone
>> will not be able to ensure stable GPIO numbers.
>
> What I mean is that if and only if aliases are used, we
> allocate GPIOs in ascending order starting at 0.
>
> Let us remember why this is done: dynamic GPIO numbers are
> allocated in descending order because it was assumed that some
> static GPIOs are already allocated starting from 0, like on-chip
> GPIOs.
>
> We can also make it a Kconfig option, because for a large chunk
> of systems only using device tree we can
> actually allocate from zero. Notably any ARCH_MULTIPLATFORM
> should just do this if we can make a quick survey of such systems
> and see that they don't fool around setting .base in their GPIO
> chips.

Ah, in that case yes, I agree it would probably work fine.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 4/4] Documentation: dt: gpio: Add gpio-number-forward property in snps gpio binding doc

[Zhou Wang]

This property is used to indicate gpiochip_find_base() to find GPIO
base number in increasing order.

Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
---
 .../devicetree/bindings/gpio/snps-dwapb-gpio.txt   |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
index dd5d2c0..b7f01c8 100644
--- a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
@@ -29,6 +29,7 @@ controller.
 - interrupts : The interrupt to the parent controller raised when GPIOs
   generate the interrupts.
 - snps,nr-gpios : The number of pins in the port, a single cell.
+- gpio-number-forward : Will find GPIO base number in increasing order.
 
 Example:
 
@@ -37,6 +38,7 @@ gpio: gpio@20000 {
 	reg = <0x20000 0x1000>;
 	#address-cells = <1>;
 	#size-cells = <0>;
+	gpio-number-forward;
 
 	porta: gpio-controller@0 {
 		compatible = "snps,dw-apb-gpio-port";
-- 
1.7.9.5

Re: [PATCH v4 4/4] Documentation: dt: gpio: Add gpio-number-forward property in snps gpio binding doc

[Linus Walleij]

On Fri, Dec 5, 2014 at 4:38 AM, Zhou Wang <wangzhou.bry@gmail.com> wrote:

> This property is used to indicate gpiochip_find_base() to find GPIO
> base number in increasing order.
>
> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>

NAK. GPIO numbers is a Linux-internal thing, not a OS independent
property.

Yours,
Linus Walleij