[PATCH v3 0/2] ARM: hip04: add GPIO support

[PATCH v3 0/2] 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.

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

Zhou Wang (2):
  ARM: hip04: set ARCH_NR_GPIO to 128
  ARM: dts: hip04: add GPIO pieces

 arch/arm/Kconfig                |    1 +
 arch/arm/boot/dts/hip04.dtsi    |   75 +++++++++++++++++++++++++++++++++++++++
 arch/arm/configs/hisi_defconfig |    3 ++
 3 files changed, 79 insertions(+)

-- 
1.7.9.5

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Zhou Wang]

Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
controllers with 32 GPIOs each.

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

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..26aae1e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
 	default 352 if ARCH_VT8500
 	default 288 if ARCH_ROCKCHIP
 	default 264 if MACH_H4700
+	default 128 if ARCH_HIP04
 	default 0
 	help
 	  Maximum number of GPIOs in the system.
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 v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Arnd Bergmann]

On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
> Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
> controllers with 32 GPIOs each.
> 
> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
> ---
>  arch/arm/Kconfig                |    1 +
>  arch/arm/configs/hisi_defconfig |    3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 89c4b5c..26aae1e 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
>         default 352 if ARCH_VT8500
>         default 288 if ARCH_ROCKCHIP
>         default 264 if MACH_H4700
> +       default 128 if ARCH_HIP04
>         default 0
>         help
>           Maximum number of GPIOs in the system.
> 

If I remember correctly, you don't actually need to set this if all gpio
clients are using the new gpio descriptor interfaces instead of gpio
numbers. Would that work for you? You'd have to know which devices
can be connected to the gpio controller for all hip04 based machines,
but I think this is a short list of boards.

Linus, one question for you: Does the ARCH_NR_GPIO actually work as designed?
I see that e.g. OMAP4 defines 192 GPIO lines but does not have an ARCH_NR_GPIO
setting. In an OMAP4-only kernel you would get the default value (512)
from include/asm-generic/gpio.h, and in a multiplatform kernel you get at
least 512 if any of the (ARCH_EXYNOS || ARCH_KEYSTONE || SOC_OMAP5 || \
SOC_DRA7XX || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 || ARCH_SHMOBILE
|| ARCH_TEGRA) are set, so that is still good.

However if one builds a kernel that just enables OMAP4 and HIP04, I suspect
it can't work on OMAP4 for any gpio line above 128, which seems to be
a fundamental multiplatform problem.

Do we neet to increase the default to 512 for all ARCH_MULTIPLATFORM
configurations and just leave ARCH_SHMOBILE, ARCH_TEGRA and MACH_H4700
here as special cases?

	Arnd

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Linus Walleij]

On Fri, Nov 28, 2014 at 10:33 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 November 2014 14:29:47 Zhou Wang wrote:

>> Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
>> controllers with 32 GPIOs each.
>>
>> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>> ---
>>  arch/arm/Kconfig                |    1 +
>>  arch/arm/configs/hisi_defconfig |    3 +++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 89c4b5c..26aae1e 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
>>         default 352 if ARCH_VT8500
>>         default 288 if ARCH_ROCKCHIP
>>         default 264 if MACH_H4700
>> +       default 128 if ARCH_HIP04
>>         default 0
>>         help
>>           Maximum number of GPIOs in the system.
>>
>
> If I remember correctly, you don't actually need to set this if all gpio
> clients are using the new gpio descriptor interfaces instead of gpio
> numbers.

Unfortunately you still have to. We are working on removing the
dependency on ARCH_NR_GPIO, old habits die hard. But I just
merged this patch:
http://marc.info/?l=linux-gpio&m=141638350328535&w=2

Which makes the situation better.

There is however some other use of this define, so there is
some work required still to get rid of it.

And that patch may blow up in testing too...

> Linus, one question for you: Does the ARCH_NR_GPIO actually work as designed?
> I see that e.g. OMAP4 defines 192 GPIO lines but does not have an ARCH_NR_GPIO
> setting. In an OMAP4-only kernel you would get the default value (512)
> from include/asm-generic/gpio.h, and in a multiplatform kernel you get at
> least 512 if any of the (ARCH_EXYNOS || ARCH_KEYSTONE || SOC_OMAP5 || \
> SOC_DRA7XX || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 || ARCH_SHMOBILE
> || ARCH_TEGRA) are set, so that is still good.

Yeah that is how it kind of works...

> However if one builds a kernel that just enables OMAP4 and HIP04, I suspect
> it can't work on OMAP4 for any gpio line above 128, which seems to be
> a fundamental multiplatform problem.

Yes I guess you are right :(

It's probably just so that so many platforms converge on 512.

> Do we neet to increase the default to 512 for all ARCH_MULTIPLATFORM
> configurations and just leave ARCH_SHMOBILE, ARCH_TEGRA and MACH_H4700
> here as special cases?

That'd be good while we are working to kill off
ARCH_NR_GPIO for good.

I guess there could be arch-specific problems with trying
to get rid of ARCH_NR_GPIO for good, do you have some
input on this? (Arch maintainer hat on...)

Yours,
Linus Walleij

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Arnd Bergmann]

On Friday 28 November 2014 16:54:44 Linus Walleij wrote:
> On Fri, Nov 28, 2014 at 10:33 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
> 
> >> Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
> >> controllers with 32 GPIOs each.
> >>
> >> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
> >> ---
> >>  arch/arm/Kconfig                |    1 +
> >>  arch/arm/configs/hisi_defconfig |    3 +++
> >>  2 files changed, 4 insertions(+)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 89c4b5c..26aae1e 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
> >>         default 352 if ARCH_VT8500
> >>         default 288 if ARCH_ROCKCHIP
> >>         default 264 if MACH_H4700
> >> +       default 128 if ARCH_HIP04
> >>         default 0
> >>         help
> >>           Maximum number of GPIOs in the system.
> >>
> >
> > If I remember correctly, you don't actually need to set this if all gpio
> > clients are using the new gpio descriptor interfaces instead of gpio
> > numbers.
> 
> Unfortunately you still have to. We are working on removing the
> dependency on ARCH_NR_GPIO, old habits die hard. 

Ok, I see.

> But I just
> merged this patch:
> http://marc.info/?l=linux-gpio&m=141638350328535&w=2
> 
> Which makes the situation better.

Ah, cool!

> There is however some other use of this define, so there is
> some work required still to get rid of it.

These are the only ones I could find:

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 9f0682534e2f..fe05d8b375ca 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -224,9 +224,6 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	if (WARN_ON(ARCH_NR_GPIOS < ngpio))
-		ngpio = ARCH_NR_GPIOS;
-
 	chips = devm_kzalloc(dev,
 			     ngpio * sizeof(struct davinci_gpio_controller),
 			     GFP_KERNEL);
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 746db6acf648..4f70024c2f9a 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -281,12 +281,12 @@ struct nmk_pinctrl {
 	void __iomem *prcm_base;
 };
 
-static struct nmk_gpio_chip *
-nmk_gpio_chips[DIV_ROUND_UP(ARCH_NR_GPIOS, NMK_GPIO_PER_CHIP)];
+#define NUM_BANKS 9 /* increase if necessary */
+
+static struct nmk_gpio_chip *nmk_gpio_chips[NUM_BANKS];
 
 static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
 
-#define NUM_BANKS ARRAY_SIZE(nmk_gpio_chips)
 
 static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
 				unsigned offset, int gpio_mode)
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 383ade1a211b..b53bcf5c58e7 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -13,23 +13,6 @@
 #include <linux/gpio/driver.h>
 #include <linux/gpio/consumer.h>
 
-/* Platforms may implement their GPIO interface with library code,
- * at a small performance cost for non-inlined operations and some
- * extra memory (for code and for per-GPIO table entries).
- *
- * While the GPIO programming interface defines valid GPIO numbers
- * to be in the range 0..MAX_INT, this library restricts them to the
- * smaller range 0..ARCH_NR_GPIOS-1.
- *
- * ARCH_NR_GPIOS is somewhat arbitrary; it usually reflects the sum of
- * builtin/SoC GPIOs plus a number of GPIOs on expanders; the latter is
- * actually an estimate of a board-specific value.
- */
-
-#ifndef ARCH_NR_GPIOS
-#define ARCH_NR_GPIOS		512
-#endif
-
 /*
  * "valid" GPIO numbers are nonnegative and may be passed to
  * setup routines like gpio_request().  only some valid numbers
@@ -41,7 +24,11 @@
 
 static inline bool gpio_is_valid(int number)
 {
-	return number >= 0 && number < ARCH_NR_GPIOS;
+#ifdef ARCH_NR_GPIOS
+	if (number > ARCH_NR_GPIOS)
+		return 0;
+#endif
+	return number >= 0;
 }
 
 struct device;

> > However if one builds a kernel that just enables OMAP4 and HIP04, I suspect
> > it can't work on OMAP4 for any gpio line above 128, which seems to be
> > a fundamental multiplatform problem.
> 
> Yes I guess you are right :(
> 
> It's probably just so that so many platforms converge on 512.
> 
> > Do we neet to increase the default to 512 for all ARCH_MULTIPLATFORM
> > configurations and just leave ARCH_SHMOBILE, ARCH_TEGRA and MACH_H4700
> > here as special cases?
> 
> That'd be good while we are working to kill off
> ARCH_NR_GPIO for good.
> 
> I guess there could be arch-specific problems with trying
> to get rid of ARCH_NR_GPIO for good, do you have some
> input on this? (Arch maintainer hat on...)

I think killing off the hardcoded number as soon as possible is
best, but it doesn't seem appropriate for stable backports or 3.18,
so let's do the 512 default on arm32-multiplatform with stable
backports.

	Arnd

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Alexandre Courbot]

On Sat, Nov 29, 2014 at 6:16 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 November 2014 16:54:44 Linus Walleij wrote:
>> On Fri, Nov 28, 2014 at 10:33 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
>>
>> >> Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
>> >> controllers with 32 GPIOs each.
>> >>
>> >> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>> >> ---
>> >>  arch/arm/Kconfig                |    1 +
>> >>  arch/arm/configs/hisi_defconfig |    3 +++
>> >>  2 files changed, 4 insertions(+)
>> >>
>> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> >> index 89c4b5c..26aae1e 100644
>> >> --- a/arch/arm/Kconfig
>> >> +++ b/arch/arm/Kconfig
>> >> @@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
>> >>         default 352 if ARCH_VT8500
>> >>         default 288 if ARCH_ROCKCHIP
>> >>         default 264 if MACH_H4700
>> >> +       default 128 if ARCH_HIP04
>> >>         default 0
>> >>         help
>> >>           Maximum number of GPIOs in the system.
>> >>
>> >
>> > If I remember correctly, you don't actually need to set this if all gpio
>> > clients are using the new gpio descriptor interfaces instead of gpio
>> > numbers.
>>
>> Unfortunately you still have to. We are working on removing the
>> dependency on ARCH_NR_GPIO, old habits die hard.
>
> Ok, I see.
>
>> But I just
>> merged this patch:
>> http://marc.info/?l=linux-gpio&m=141638350328535&w=2
>>
>> Which makes the situation better.
>
> Ah, cool!
>
>> There is however some other use of this define, so there is
>> some work required still to get rid of it.
>
> These are the only ones I could find:
>
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 9f0682534e2f..fe05d8b375ca 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -224,9 +224,6 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> -       if (WARN_ON(ARCH_NR_GPIOS < ngpio))
> -               ngpio = ARCH_NR_GPIOS;
> -
>         chips = devm_kzalloc(dev,
>                              ngpio * sizeof(struct davinci_gpio_controller),
>                              GFP_KERNEL);
> diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> index 746db6acf648..4f70024c2f9a 100644
> --- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> @@ -281,12 +281,12 @@ struct nmk_pinctrl {
>         void __iomem *prcm_base;
>  };
>
> -static struct nmk_gpio_chip *
> -nmk_gpio_chips[DIV_ROUND_UP(ARCH_NR_GPIOS, NMK_GPIO_PER_CHIP)];
> +#define NUM_BANKS 9 /* increase if necessary */
> +
> +static struct nmk_gpio_chip *nmk_gpio_chips[NUM_BANKS];
>
>  static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
>
> -#define NUM_BANKS ARRAY_SIZE(nmk_gpio_chips)
>
>  static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
>                                 unsigned offset, int gpio_mode)
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 383ade1a211b..b53bcf5c58e7 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -13,23 +13,6 @@
>  #include <linux/gpio/driver.h>
>  #include <linux/gpio/consumer.h>
>
> -/* Platforms may implement their GPIO interface with library code,
> - * at a small performance cost for non-inlined operations and some
> - * extra memory (for code and for per-GPIO table entries).
> - *
> - * While the GPIO programming interface defines valid GPIO numbers
> - * to be in the range 0..MAX_INT, this library restricts them to the
> - * smaller range 0..ARCH_NR_GPIOS-1.
> - *
> - * ARCH_NR_GPIOS is somewhat arbitrary; it usually reflects the sum of
> - * builtin/SoC GPIOs plus a number of GPIOs on expanders; the latter is
> - * actually an estimate of a board-specific value.
> - */
> -
> -#ifndef ARCH_NR_GPIOS
> -#define ARCH_NR_GPIOS          512
> -#endif
> -
>  /*
>   * "valid" GPIO numbers are nonnegative and may be passed to
>   * setup routines like gpio_request().  only some valid numbers
> @@ -41,7 +24,11 @@
>
>  static inline bool gpio_is_valid(int number)
>  {
> -       return number >= 0 && number < ARCH_NR_GPIOS;
> +#ifdef ARCH_NR_GPIOS
> +       if (number > ARCH_NR_GPIOS)
> +               return 0;
> +#endif
> +       return number >= 0;
>  }
>
>  struct device;
>
>> > However if one builds a kernel that just enables OMAP4 and HIP04, I suspect
>> > it can't work on OMAP4 for any gpio line above 128, which seems to be
>> > a fundamental multiplatform problem.
>>
>> Yes I guess you are right :(
>>
>> It's probably just so that so many platforms converge on 512.
>>
>> > Do we neet to increase the default to 512 for all ARCH_MULTIPLATFORM
>> > configurations and just leave ARCH_SHMOBILE, ARCH_TEGRA and MACH_H4700
>> > here as special cases?
>>
>> That'd be good while we are working to kill off
>> ARCH_NR_GPIO for good.
>>
>> I guess there could be arch-specific problems with trying
>> to get rid of ARCH_NR_GPIO for good, do you have some
>> input on this? (Arch maintainer hat on...)
>
> I think killing off the hardcoded number as soon as possible is
> best, but it doesn't seem appropriate for stable backports or 3.18,
> so let's do the 512 default on arm32-multiplatform with stable
> backports.

The main problem I see with killing ARCH_NR_GPIO is that it is used to
find a free GPIO range for GPIO chips which base GPIO is not
pre-decided (i.e. base < 0). gpiochip_find_base() returns the highest
free range matching the chip's needs. Removing it, we will have two
issues:

1) how do we decide the GPIO base for chips which base member is < 0?
We could take an arbitrarily high number, but this leads to...
2) ... are we ok with GPIO base number changing for such chips,
especially since this number is potentially exported by the sysfs
interface?

This talks in favor of named GPIO exports, but the problem of
user-space compatibility will remain. This problem apart, I'm all for
removing ARCH_NR_GPIOs and actually already have patches that do so,
provided the static GPIO array removal patch survives a few weeks of
testing in -next.

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Zhou Wang]

On 2014?11?28? 23:54, Linus Walleij wrote:
> On Fri, Nov 28, 2014 at 10:33 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
>
>>> Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
>>> controllers with 32 GPIOs each.
>>>
>>> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>>> ---
>>>   arch/arm/Kconfig                |    1 +
>>>   arch/arm/configs/hisi_defconfig |    3 +++
>>>   2 files changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index 89c4b5c..26aae1e 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
>>>          default 352 if ARCH_VT8500
>>>          default 288 if ARCH_ROCKCHIP
>>>          default 264 if MACH_H4700
>>> +       default 128 if ARCH_HIP04
>>>          default 0
>>>          help
>>>            Maximum number of GPIOs in the system.
>>>
>>
>> If I remember correctly, you don't actually need to set this if all gpio
>> clients are using the new gpio descriptor interfaces instead of gpio
>> numbers.
>
> Unfortunately you still have to. We are working on removing the
> dependency on ARCH_NR_GPIO, old habits die hard. But I just
> merged this patch:
> http://marc.info/?l=linux-gpio&m=141638350328535&w=2
>
> Which makes the situation better.
>
> There is however some other use of this define, so there is
> some work required still to get rid of it.
>
> And that patch may blow up in testing too...
>
>> Linus, one question for you: Does the ARCH_NR_GPIO actually work as designed?
>> I see that e.g. OMAP4 defines 192 GPIO lines but does not have an ARCH_NR_GPIO
>> setting. In an OMAP4-only kernel you would get the default value (512)
>> from include/asm-generic/gpio.h, and in a multiplatform kernel you get at
>> least 512 if any of the (ARCH_EXYNOS || ARCH_KEYSTONE || SOC_OMAP5 || \
>> SOC_DRA7XX || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 || ARCH_SHMOBILE
>> || ARCH_TEGRA) are set, so that is still good.
>
> Yeah that is how it kind of works...
>
>> However if one builds a kernel that just enables OMAP4 and HIP04, I suspect
>> it can't work on OMAP4 for any gpio line above 128, which seems to be
>> a fundamental multiplatform problem.
>
> Yes I guess you are right :(

Hi Linus,

If so, what is your opinion about this patch about Hip04 GPIO?
If we use default ARCH_NR_GPIO, it will be strange to user:
for GPIO 0~127, their GPIO numbers are 384~511 :(

Best Regards,
Zhou Wang

>
> It's probably just so that so many platforms converge on 512.
>
>> Do we neet to increase the default to 512 for all ARCH_MULTIPLATFORM
>> configurations and just leave ARCH_SHMOBILE, ARCH_TEGRA and MACH_H4700
>> here as special cases?
>
> That'd be good while we are working to kill off
> ARCH_NR_GPIO for good.
>
> I guess there could be arch-specific problems with trying
> to get rid of ARCH_NR_GPIO for good, do you have some
> input on this? (Arch maintainer hat on...)
>
> Yours,
> Linus Walleij
>

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Zhou Wang]

On 2014?11?28? 17:33, Arnd Bergmann wrote:
> On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
>> Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
>> controllers with 32 GPIOs each.
>>
>> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>> ---
>>   arch/arm/Kconfig                |    1 +
>>   arch/arm/configs/hisi_defconfig |    3 +++
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 89c4b5c..26aae1e 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
>>          default 352 if ARCH_VT8500
>>          default 288 if ARCH_ROCKCHIP
>>          default 264 if MACH_H4700
>> +       default 128 if ARCH_HIP04
>>          default 0
>>          help
>>            Maximum number of GPIOs in the system.
>>
>
> If I remember correctly, you don't actually need to set this if all gpio
> clients are using the new gpio descriptor interfaces instead of gpio
> numbers. Would that work for you? You'd have to know which devices

Hi Arnd,

If I don't set this, it will use the default ARCH_NR_GPIO(512), then
the range of GPIO number will be 384~511 which is very strange to users,
because Hip04 based machines can only support 128 GPIOs.

Best Regards,
Zhou Wang

> can be connected to the gpio controller for all hip04 based machines,
> but I think this is a short list of boards.
>
> Linus, one question for you: Does the ARCH_NR_GPIO actually work as designed?
> I see that e.g. OMAP4 defines 192 GPIO lines but does not have an ARCH_NR_GPIO
> setting. In an OMAP4-only kernel you would get the default value (512)
> from include/asm-generic/gpio.h, and in a multiplatform kernel you get at
> least 512 if any of the (ARCH_EXYNOS || ARCH_KEYSTONE || SOC_OMAP5 || \
> SOC_DRA7XX || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 || ARCH_SHMOBILE
> || ARCH_TEGRA) are set, so that is still good.
>
> However if one builds a kernel that just enables OMAP4 and HIP04, I suspect
> it can't work on OMAP4 for any gpio line above 128, which seems to be
> a fundamental multiplatform problem.
>
> Do we neet to increase the default to 512 for all ARCH_MULTIPLATFORM
> configurations and just leave ARCH_SHMOBILE, ARCH_TEGRA and MACH_H4700
> here as special cases?
>
> 	Arnd
>

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Linus Walleij]

On Sat, Nov 29, 2014 at 8:11 AM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
> On 2014?11?28? 17:33, Arnd Bergmann wrote:
>> On Friday 28 November 2014 14:29:47 Zhou Wang wrote:

>>>          default 264 if MACH_H4700
>>> +       default 128 if ARCH_HIP04
>>>          default 0
>>>          help
>>>            Maximum number of GPIOs in the system.
>>>
>>
>> If I remember correctly, you don't actually need to set this if all gpio
>> clients are using the new gpio descriptor interfaces instead of gpio
>> numbers. Would that work for you? You'd have to know which devices
>
> If I don't set this, it will use the default ARCH_NR_GPIO(512), then
> the range of GPIO number will be 384~511 which is very strange to users,
> because Hip04 based machines can only support 128 GPIOs.

That is a totally different problem.

Not that these numbers have ever been stable...

Think of a patch to gpiochip_find_base() in
drivers/gpio/gpiolib.c to fix this so that numbers are
assigned a better way rather than doing
random hacks with numbers like this.

Yours,
Linus Walleij

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Arnd Bergmann]

On Monday 01 December 2014 15:04:49 Linus Walleij wrote:
> On Sat, Nov 29, 2014 at 8:11 AM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
> > On 2014?11?28? 17:33, Arnd Bergmann wrote:
> >> On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
> 
> >>>          default 264 if MACH_H4700
> >>> +       default 128 if ARCH_HIP04
> >>>          default 0
> >>>          help
> >>>            Maximum number of GPIOs in the system.
> >>>
> >>
> >> If I remember correctly, you don't actually need to set this if all gpio
> >> clients are using the new gpio descriptor interfaces instead of gpio
> >> numbers. Would that work for you? You'd have to know which devices
> >
> > If I don't set this, it will use the default ARCH_NR_GPIO(512), then
> > the range of GPIO number will be 384~511 which is very strange to users,
> > because Hip04 based machines can only support 128 GPIOs.
> 
> That is a totally different problem.
> 
> Not that these numbers have ever been stable...

Right, in particular, it would be different between hisi_defconfig
and multi_v7_defconfig.

> Think of a patch to gpiochip_find_base() in
> drivers/gpio/gpiolib.c to fix this so that numbers are
> assigned a better way rather than doing
> random hacks with numbers like this.

Maybe we can find the highest number that is ever allocated
statically by any driver (since it's all from source code), and do
the dynamic allocations forward from that number instead of allocating
backward?

Or could we try to find the drivers that use DT /and/ allocate a static
number and then see if we can avoid the need for static numbers for
those?

	Arnd

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Zhou Wang]

On 2014?12?01? 22:04, Linus Walleij wrote:
> On Sat, Nov 29, 2014 at 8:11 AM, Zhou Wang <wangzhou.bry@gmail.com> wrote:
>> On 2014?11?28? 17:33, Arnd Bergmann wrote:
>>> On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
>
>>>>           default 264 if MACH_H4700
>>>> +       default 128 if ARCH_HIP04
>>>>           default 0
>>>>           help
>>>>             Maximum number of GPIOs in the system.
>>>>
>>>
>>> If I remember correctly, you don't actually need to set this if all gpio
>>> clients are using the new gpio descriptor interfaces instead of gpio
>>> numbers. Would that work for you? You'd have to know which devices
>>
>> If I don't set this, it will use the default ARCH_NR_GPIO(512), then
>> the range of GPIO number will be 384~511 which is very strange to users,
>> because Hip04 based machines can only support 128 GPIOs.
>
> That is a totally different problem.
>
> Not that these numbers have ever been stable...
>
> Think of a patch to gpiochip_find_base() in
> drivers/gpio/gpiolib.c to fix this so that numbers are
> assigned a better way rather than doing
> random hacks with numbers like this.
>
> Yours,
> Linus Walleij
>
Hi Linus,

how about a patch like this, we read the base from the dts here.

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8e98ca..0c40f53 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -107,11 +107,16 @@ 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)
  {
         struct gpio_chip *chip;
+       int ngpio = gpio_chip->ngpio;
         int base = ARCH_NR_GPIOS - ngpio;

+       /* just prototype */
+       if (!of_property_read_u32(gpio_chip->dev->of_node, "base", &base))
+               return base;
+
         list_for_each_entry_reverse(chip, &gpio_chips, list) {
                 /* found a free space? */
                 if (chip->base + chip->ngpio <= base)
@@ -236,7 +241,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;

Best regards,
Zhou Wang

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Arnd Bergmann]

On Tuesday 02 December 2014 14:43:51 Zhou Wang wrote:
> 
> how about a patch like this, we read the base from the dts here.
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e8e98ca..0c40f53 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -107,11 +107,16 @@ 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)
>   {
>          struct gpio_chip *chip;
> +       int ngpio = gpio_chip->ngpio;
>          int base = ARCH_NR_GPIOS - ngpio;
> 
> +       /* just prototype */
> +       if (!of_property_read_u32(gpio_chip->dev->of_node, "base", &base))
> +               return base;
> +

I don't think that would be appropriate. The concept of a gpio number
base is implementation specific to current Linux versions and we want
to get rid of that in the future, so it should not be part of an
OS-independent spec.

	Arnd

[PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

[Zhou Wang]

On 2014?12?02? 16:42, Arnd Bergmann wrote:
> On Tuesday 02 December 2014 14:43:51 Zhou Wang wrote:
>>
>> how about a patch like this, we read the base from the dts here.
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index e8e98ca..0c40f53 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -107,11 +107,16 @@ 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)
>>    {
>>           struct gpio_chip *chip;
>> +       int ngpio = gpio_chip->ngpio;
>>           int base = ARCH_NR_GPIOS - ngpio;
>>
>> +       /* just prototype */
>> +       if (!of_property_read_u32(gpio_chip->dev->of_node, "base", &base))
>> +               return base;
>> +
>
> I don't think that would be appropriate. The concept of a gpio number
> base is implementation specific to current Linux versions and we want
> to get rid of that in the future, so it should not be part of an
> OS-independent spec.
>
> 	Arnd
>
Got it. I will make a patch in which it finds base number of a GPIO
controller in a increasing order. Maybe it can get rid of ARCH_NR_GPIOS
in the future.

Thanks,
Zhou Wang

[PATCH v3 2/2] 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.

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

diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
index 93b6c90..c0f76b8 100644
--- a/arch/arm/boot/dts/hip04.dtsi
+++ b/arch/arm/boot/dts/hip04.dtsi
@@ -263,5 +263,80 @@
 			interrupts = <0 372 4>;
 		};
 
+		gpio at 4003000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4003000 0x1000>;
+
+			gpio3: gpio-controller at 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>;
+			};
+		};
+
+		gpio at 4002000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4002000 0x1000>;
+
+			gpio2: gpio-controller at 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 at 4001000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4001000 0x1000>;
+
+			gpio1: gpio-controller at 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 at 4000000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x4000000 0x1000>;
+
+			gpio0: gpio-controller at 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>;
+			};
+		};
 	};
 };
-- 
1.7.9.5