[PATCH 0/2] GPIO support for Socionext Synquacer

[PATCH 0/2] GPIO support for Socionext Synquacer

[Ard Biesheuvel]

The Socionext Synquacer SC2A11, which is used in the arm64 Developer Box,
shares its GPIO IP with a Fujitsu SoC for which we already have support
in the tree. So let's tweak it so that we can reuse it.

Cc: Linus Walleij <linus.walleij@linaro.org>

Ard Biesheuvel (2):
  gpio: mb86s7x: share with other SoCs as module
  gpio: mb86s70: Revert "Return error if requesting an already assigned
    gpio"

 drivers/gpio/Kconfig        |  3 +--
 drivers/gpio/gpio-mb86s7x.c | 17 +++++++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)

-- 
2.11.0

[PATCH 1/2] gpio: mb86s7x: share with other SoCs as module

[Ard Biesheuvel]

In order to reuse this driver for the Socionext Synquacer SC2A11 SoC,
which inherited this IP from Fujitsu, remove the ARCH_MB86S7X Kconfig
dependency, and revert the changes that prevent it from being built as
a module.

This reverts commits d65aa4b67b4f47f303bdeaef1e4d42ef18e6b293 and
d5610e514e92144d19bd5e39e5cf3804bbf85f3e.

Cc: Geliang Tang <geliangtang@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/gpio/Kconfig        |  3 +--
 drivers/gpio/gpio-mb86s7x.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3f80f167ed56..bf40a948e4cc 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -286,8 +286,7 @@ config GPIO_LYNXPOINT
 	  Requires ACPI device enumeration code to set up a platform device.
 
 config GPIO_MB86S7X
-	bool "GPIO support for Fujitsu MB86S7x Platforms"
-	depends on ARCH_MB86S7X || COMPILE_TEST
+	tristate "GPIO support for Fujitsu MB86S7x Platforms"
 	help
 	  Say yes here to support the GPIO controller in Fujitsu MB86S70 SoCs.
 
diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c
index 94d772677ed6..6e1598471733 100644
--- a/drivers/gpio/gpio-mb86s7x.c
+++ b/drivers/gpio/gpio-mb86s7x.c
@@ -17,6 +17,7 @@
 #include <linux/io.h>
 #include <linux/init.h>
 #include <linux/clk.h>
+#include <linux/module.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/ioport.h>
@@ -209,6 +210,7 @@ static const struct of_device_id mb86s70_gpio_dt_ids[] = {
 	{ .compatible = "fujitsu,mb86s70-gpio" },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, mb86s70_gpio_dt_ids);
 
 static struct platform_driver mb86s70_gpio_driver = {
 	.driver = {
@@ -219,4 +221,12 @@ static struct platform_driver mb86s70_gpio_driver = {
 	.remove = mb86s70_gpio_remove,
 };
 
-builtin_platform_driver(mb86s70_gpio_driver);
+static int __init mb86s70_gpio_init(void)
+{
+	return platform_driver_register(&mb86s70_gpio_driver);
+}
+module_init(mb86s70_gpio_init);
+
+MODULE_DESCRIPTION("MB86S7x GPIO Driver");
+MODULE_ALIAS("platform:mb86s70-gpio");
+MODULE_LICENSE("GPL");
-- 
2.11.0

[PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio"

[Ard Biesheuvel]

Commit fd9c963c5661 ("gpio: mb86s70: Return error if requesting an
already assigned gpio") adds code that infers from the state of the
GPIO Pin Function Register (PFR) whether a GPIO has been assigned
already. This assumes that the pin functions are set to 'peripheral'
when the driver is loaded, which is not guaranteed. Also, the GPIO
layer is perfectly capable of keeping track of which GPIOs have been
assigned already, so we shouldn't need this check in the first place.

This reverts commit fd9c963c5661af3403e77e312c0d9941773b6c1b.

Cc: Axel Lin <axel.lin@ingics.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/gpio/gpio-mb86s7x.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c
index 6e1598471733..5cd77dcdbb16 100644
--- a/drivers/gpio/gpio-mb86s7x.c
+++ b/drivers/gpio/gpio-mb86s7x.c
@@ -53,11 +53,6 @@ static int mb86s70_gpio_request(struct gpio_chip *gc, unsigned gpio)
 	spin_lock_irqsave(&gchip->lock, flags);
 
 	val = readl(gchip->base + PFR(gpio));
-	if (!(val & OFFSET(gpio))) {
-		spin_unlock_irqrestore(&gchip->lock, flags);
-		return -EINVAL;
-	}
-
 	val &= ~OFFSET(gpio);
 	writel(val, gchip->base + PFR(gpio));
 
-- 
2.11.0

Re: [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio"

[Axel Lin]

2017-10-28 4:21 GMT+08:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> Commit fd9c963c5661 ("gpio: mb86s70: Return error if requesting an
> already assigned gpio") adds code that infers from the state of the
> GPIO Pin Function Register (PFR) whether a GPIO has been assigned
> already. This assumes that the pin functions are set to 'peripheral'
> when the driver is loaded, which is not guaranteed. Also, the GPIO
> layer is perfectly capable of keeping track of which GPIOs have been
> assigned already, so we shouldn't need this check in the first place.
Yes, agree.
Acked-by: Axel Lin <axel.lin@ingics.com>

>
> This reverts commit fd9c963c5661af3403e77e312c0d9941773b6c1b.
>
> Cc: Axel Lin <axel.lin@ingics.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/gpio/gpio-mb86s7x.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c
> index 6e1598471733..5cd77dcdbb16 100644
> --- a/drivers/gpio/gpio-mb86s7x.c
> +++ b/drivers/gpio/gpio-mb86s7x.c
> @@ -53,11 +53,6 @@ static int mb86s70_gpio_request(struct gpio_chip *gc, unsigned gpio)
>         spin_lock_irqsave(&gchip->lock, flags);
>
>         val = readl(gchip->base + PFR(gpio));
> -       if (!(val & OFFSET(gpio))) {
> -               spin_unlock_irqrestore(&gchip->lock, flags);
> -               return -EINVAL;
> -       }
> -
>         val &= ~OFFSET(gpio);
>         writel(val, gchip->base + PFR(gpio));
>
> --
> 2.11.0
>

Re: [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module

[Ard Biesheuvel]

On 27 October 2017 at 21:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> In order to reuse this driver for the Socionext Synquacer SC2A11 SoC,
> which inherited this IP from Fujitsu, remove the ARCH_MB86S7X Kconfig
> dependency, and revert the changes that prevent it from being built as
> a module.
>
> This reverts commits d65aa4b67b4f47f303bdeaef1e4d42ef18e6b293 and
> d5610e514e92144d19bd5e39e5cf3804bbf85f3e.
>
> Cc: Geliang Tang <geliangtang@gmail.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/gpio/Kconfig        |  3 +--
>  drivers/gpio/gpio-mb86s7x.c | 12 +++++++++++-
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 3f80f167ed56..bf40a948e4cc 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -286,8 +286,7 @@ config GPIO_LYNXPOINT
>           Requires ACPI device enumeration code to set up a platform device.
>
>  config GPIO_MB86S7X
> -       bool "GPIO support for Fujitsu MB86S7x Platforms"
> -       depends on ARCH_MB86S7X || COMPILE_TEST
> +       tristate "GPIO support for Fujitsu MB86S7x Platforms"
>         help
>           Say yes here to support the GPIO controller in Fujitsu MB86S70 SoCs.
>
> diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c
> index 94d772677ed6..6e1598471733 100644
> --- a/drivers/gpio/gpio-mb86s7x.c
> +++ b/drivers/gpio/gpio-mb86s7x.c
> @@ -17,6 +17,7 @@
>  #include <linux/io.h>
>  #include <linux/init.h>
>  #include <linux/clk.h>
> +#include <linux/module.h>
>  #include <linux/err.h>
>  #include <linux/errno.h>
>  #include <linux/ioport.h>
> @@ -209,6 +210,7 @@ static const struct of_device_id mb86s70_gpio_dt_ids[] = {
>         { .compatible = "fujitsu,mb86s70-gpio" },
>         { /* sentinel */ }
>  };
> +MODULE_DEVICE_TABLE(of, mb86s70_gpio_dt_ids);
>
>  static struct platform_driver mb86s70_gpio_driver = {
>         .driver = {
> @@ -219,4 +221,12 @@ static struct platform_driver mb86s70_gpio_driver = {
>         .remove = mb86s70_gpio_remove,
>  };
>
> -builtin_platform_driver(mb86s70_gpio_driver);
> +static int __init mb86s70_gpio_init(void)
> +{
> +       return platform_driver_register(&mb86s70_gpio_driver);
> +}
> +module_init(mb86s70_gpio_init);
> +
> +MODULE_DESCRIPTION("MB86S7x GPIO Driver");
> +MODULE_ALIAS("platform:mb86s70-gpio");
> +MODULE_LICENSE("GPL");
> --
> 2.11.0
>

The .c changes are a straight revert of the above patches, but perhaps
we should fold this in on top:

diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c
index 5cd77dcdbb16..3134c0d2bfe4 100644
--- a/drivers/gpio/gpio-mb86s7x.c
+++ b/drivers/gpio/gpio-mb86s7x.c
@@ -215,12 +215,7 @@ static struct platform_driver mb86s70_gpio_driver = {
        .probe = mb86s70_gpio_probe,
        .remove = mb86s70_gpio_remove,
 };
-
-static int __init mb86s70_gpio_init(void)
-{
-       return platform_driver_register(&mb86s70_gpio_driver);
-}
-module_init(mb86s70_gpio_init);
+module_platform_driver(mb86s70_gpio_driver);

 MODULE_DESCRIPTION("MB86S7x GPIO Driver");
 MODULE_ALIAS("platform:mb86s70-gpio");


so that the module can actually be unloaded again.

Re: [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module

[Linus Walleij]

On Fri, Oct 27, 2017 at 10:21 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> In order to reuse this driver for the Socionext Synquacer SC2A11 SoC,
> which inherited this IP from Fujitsu, remove the ARCH_MB86S7X Kconfig
> dependency, and revert the changes that prevent it from being built as
> a module.
>
> This reverts commits d65aa4b67b4f47f303bdeaef1e4d42ef18e6b293 and
> d5610e514e92144d19bd5e39e5cf3804bbf85f3e.
>
> Cc: Geliang Tang <geliangtang@gmail.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Patch applied, I folded in your patch snippet
using module_platform_driver() on top.

Yours,
Linus Walleij

Re: [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio"

[Linus Walleij]

On Fri, Oct 27, 2017 at 10:21 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> Commit fd9c963c5661 ("gpio: mb86s70: Return error if requesting an
> already assigned gpio") adds code that infers from the state of the
> GPIO Pin Function Register (PFR) whether a GPIO has been assigned
> already. This assumes that the pin functions are set to 'peripheral'
> when the driver is loaded, which is not guaranteed. Also, the GPIO
> layer is perfectly capable of keeping track of which GPIOs have been
> assigned already, so we shouldn't need this check in the first place.
>
> This reverts commit fd9c963c5661af3403e77e312c0d9941773b6c1b.
>
> Cc: Axel Lin <axel.lin@ingics.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Patch applied with Axel's ACK.

Yours,
Linus Walleij

Re: [PATCH 0/2] GPIO support for Socionext Synquacer

[Linus Walleij]

On Fri, Oct 27, 2017 at 10:21 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> The Socionext Synquacer SC2A11, which is used in the arm64 Developer Box,
> shares its GPIO IP with a Fujitsu SoC for which we already have support
> in the tree. So let's tweak it so that we can reuse it.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
>
> Ard Biesheuvel (2):
>   gpio: mb86s7x: share with other SoCs as module
>   gpio: mb86s70: Revert "Return error if requesting an already assigned
>     gpio"

Nice. We might need to look into the following wrt this driver:

- Using generic MMIO GPIO, i.e. select GPIO_GENERIC in Kconfig
  and a patch such as commit 6d125412fc16802012a17665638f49b0b0c81f18
  "gpio: iop: Use generic GPIO MMIO functions for driver"
  apart from reduced code size this brings the .get_multiple() and
  .set_multiple() callbacks for FREE.
  The fact that the driver is so simple that it should have been using
  MMIO/GENERIC GPIO is a plain oversight during review.

- When submitting the DTS for that developer box, make sure that
  the 96boards header has proper GPIO line names from day 1,
  see e.g.
  commit bbaf867e2d3796bca465d07ffcd800a3bd570861
  "arm64: dts: hikey: name the GPIO lines"

Ard: if you have this machine on your desk help with the above would
be much appreciated (plus it's fun!) thanks a bunch :)

Yours,
Linus Walleij

Re: [PATCH 0/2] GPIO support for Socionext Synquacer

[Ard Biesheuvel]

On 31 October 2017 at 12:20, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Oct 27, 2017 at 10:21 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>
>> The Socionext Synquacer SC2A11, which is used in the arm64 Developer Box,
>> shares its GPIO IP with a Fujitsu SoC for which we already have support
>> in the tree. So let's tweak it so that we can reuse it.
>>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>
>> Ard Biesheuvel (2):
>>   gpio: mb86s7x: share with other SoCs as module
>>   gpio: mb86s70: Revert "Return error if requesting an already assigned
>>     gpio"
>
> Nice. We might need to look into the following wrt this driver:
>
> - Using generic MMIO GPIO, i.e. select GPIO_GENERIC in Kconfig
>   and a patch such as commit 6d125412fc16802012a17665638f49b0b0c81f18
>   "gpio: iop: Use generic GPIO MMIO functions for driver"
>   apart from reduced code size this brings the .get_multiple() and
>   .set_multiple() callbacks for FREE.
>   The fact that the driver is so simple that it should have been using
>   MMIO/GENERIC GPIO is a plain oversight during review.
>

Does this work with the layout if this chip? It has 32 GPIO lines,
whose controls are mapped onto the lowest 8 bits of 4 adjacent 32-bit
registers.

> - When submitting the DTS for that developer box, make sure that
>   the 96boards header has proper GPIO line names from day 1,
>   see e.g.
>   commit bbaf867e2d3796bca465d07ffcd800a3bd570861
>   "arm64: dts: hikey: name the GPIO lines"
>

I currently have this in my DTS:

&gpio {
    dsw3_1 {
        gpios = <0 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

    dsw3_2 {
        gpios = <1 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

    dsw3_3 {
        gpios = <2 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

    dsw3_4 {
        gpios = <3 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

    dsw3_5 {
        gpios = <4 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

    dsw3_6 {
        gpios = <5 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

    dsw3_7 {
        gpios = <6 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

    dsw3_8 {
        gpios = <7 GPIO_ACTIVE_HIGH>;
        gpio-hog;
        input;
    };

for the 8 DIP switches that are connected to GPIO lines. There are
more assigned, to various function, and 8 of them are routed to the
96boards low speed connector as well.

> Ard: if you have this machine on your desk help with the above would
> be much appreciated (plus it's fun!) thanks a bunch :)
>

Of course, if you help me understand it :-)

So I can add the names for all the lines that have a purpose, but is
that orthogonal to hogging?

Re: [PATCH 0/2] GPIO support for Socionext Synquacer

[Ard Biesheuvel]

On 31 October 2017 at 12:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 31 October 2017 at 12:20, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Fri, Oct 27, 2017 at 10:21 PM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>
>>> The Socionext Synquacer SC2A11, which is used in the arm64 Developer Box,
>>> shares its GPIO IP with a Fujitsu SoC for which we already have support
>>> in the tree. So let's tweak it so that we can reuse it.
>>>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Ard Biesheuvel (2):
>>>   gpio: mb86s7x: share with other SoCs as module
>>>   gpio: mb86s70: Revert "Return error if requesting an already assigned
>>>     gpio"
>>
>> Nice. We might need to look into the following wrt this driver:
>>
>> - Using generic MMIO GPIO, i.e. select GPIO_GENERIC in Kconfig
>>   and a patch such as commit 6d125412fc16802012a17665638f49b0b0c81f18
>>   "gpio: iop: Use generic GPIO MMIO functions for driver"
>>   apart from reduced code size this brings the .get_multiple() and
>>   .set_multiple() callbacks for FREE.
>>   The fact that the driver is so simple that it should have been using
>>   MMIO/GENERIC GPIO is a plain oversight during review.
>>
>
> Does this work with the layout if this chip? It has 32 GPIO lines,
> whose controls are mapped onto the lowest 8 bits of 4 adjacent 32-bit
> registers.
>
>> - When submitting the DTS for that developer box, make sure that
>>   the 96boards header has proper GPIO line names from day 1,
>>   see e.g.
>>   commit bbaf867e2d3796bca465d07ffcd800a3bd570861
>>   "arm64: dts: hikey: name the GPIO lines"
>>
>
> I currently have this in my DTS:
>
> &gpio {
>     dsw3_1 {
>         gpios = <0 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_2 {
>         gpios = <1 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_3 {
>         gpios = <2 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_4 {
>         gpios = <3 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_5 {
>         gpios = <4 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_6 {
>         gpios = <5 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_7 {
>         gpios = <6 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_8 {
>         gpios = <7 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
> for the 8 DIP switches that are connected to GPIO lines. There are
> more assigned, to various function, and 8 of them are routed to the
> 96boards low speed connector as well.
>
>> Ard: if you have this machine on your desk help with the above would
>> be much appreciated (plus it's fun!) thanks a bunch :)
>>
>
> Of course, if you help me understand it :-)
>
> So I can add the names for all the lines that have a purpose, but is
> that orthogonal to hogging?

OK, so i now have

# cat /sys/kernel/debug/gpio
gpiochip0: GPIOs 480-511, parent: platform/51000000.gpio, 51000000.gpio:
 gpio-480 (DSW3-PIN1           |dsw3_1              ) in  lo
 gpio-481 (DSW3-PIN2           |dsw3_2              ) in  lo
 gpio-482 (DSW3-PIN3           |dsw3_3              ) in  lo
 gpio-483 (DSW3-PIN4           |dsw3_4              ) in  hi
 gpio-484 (DSW3-PIN5           |dsw3_5              ) in  hi
 gpio-485 (DSW3-PIN6           |dsw3_6              ) in  lo
 gpio-486 (DSW3-PIN7           |dsw3_7              ) in  lo
 gpio-487 (DSW3-PIN8           |dsw3_8              ) in  lo
 gpio-488 (NC                  )
 gpio-489 (PWROFF#             )
 gpio-490 (GPIO-A              )
 gpio-491 (GPIO-B              )
 gpio-492 (GPIO-C              )
 gpio-493 (GPIO-D              )
 gpio-494 (PCIE1EXTINT         )
 gpio-495 (PCIE0EXTINT         )
 gpio-496 (PHY2-INT#           )
 gpio-497 (PHY1-INT#           )
 gpio-498 (GPIO-E              )
 gpio-499 (GPIO-F              )
 gpio-500 (GPIO-G              )
 gpio-501 (GPIO-H              )
 gpio-502 (GPIO-I              )
 gpio-503 (GPIO-J              )
 gpio-504 (GPIO-K              )
 gpio-505 (GPIO-L              )
 gpio-506 (PEC-PD26            )
 gpio-507 (PEC-PD27            )
 gpio-508 (PEC-PD28            )
 gpio-509 (PEC-PD29            )
 gpio-510 (PEC-PD30            )
 gpio-511 (PEC-PD31            )

after adding this

    gpio-line-names = "DSW3-PIN1", "DSW3-PIN2", "DSW3-PIN3", "DSW3-PIN4",
                      "DSW3-PIN5", "DSW3-PIN6", "DSW3-PIN7", "DSW3-PIN8",
                      "NC", "PWROFF#",
                      "GPIO-A", "GPIO-B", "GPIO-C", "GPIO-D",
                      "PCIE1EXTINT", "PCIE0EXTINT",
                      "PHY2-INT#", "PHY1-INT#",
                      "GPIO-E", "GPIO-F", "GPIO-G", "GPIO-H",
                      "GPIO-I", "GPIO-J", "GPIO-K", "GPIO-L",
                      "PEC-PD26", "PEC-PD27", "PEC-PD28",
                      "PEC-PD29", "PEC-PD30", "PEC-PD31";

to the DT node of the GPIO controller.

Re: [PATCH 0/2] GPIO support for Socionext Synquacer

[Linus Walleij]

On Tue, Oct 31, 2017 at 1:59 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> OK, so i now have
>
> # cat /sys/kernel/debug/gpio

"lsgpio" from tools/gpio/lsgpio.c is even nicer, but OK :)

> gpiochip0: GPIOs 480-511, parent: platform/51000000.gpio, 51000000.gpio:
>  gpio-480 (DSW3-PIN1           |dsw3_1              ) in  lo
>  gpio-481 (DSW3-PIN2           |dsw3_2              ) in  lo
>  gpio-482 (DSW3-PIN3           |dsw3_3              ) in  lo
>  gpio-483 (DSW3-PIN4           |dsw3_4              ) in  hi
>  gpio-484 (DSW3-PIN5           |dsw3_5              ) in  hi
>  gpio-485 (DSW3-PIN6           |dsw3_6              ) in  lo
>  gpio-486 (DSW3-PIN7           |dsw3_7              ) in  lo
>  gpio-487 (DSW3-PIN8           |dsw3_8              ) in  lo
>  gpio-488 (NC                  )
>  gpio-489 (PWROFF#             )
>  gpio-490 (GPIO-A              )
>  gpio-491 (GPIO-B              )
>  gpio-492 (GPIO-C              )
>  gpio-493 (GPIO-D              )
>  gpio-494 (PCIE1EXTINT         )
>  gpio-495 (PCIE0EXTINT         )
>  gpio-496 (PHY2-INT#           )
>  gpio-497 (PHY1-INT#           )
>  gpio-498 (GPIO-E              )
>  gpio-499 (GPIO-F              )
>  gpio-500 (GPIO-G              )
>  gpio-501 (GPIO-H              )
>  gpio-502 (GPIO-I              )
>  gpio-503 (GPIO-J              )
>  gpio-504 (GPIO-K              )
>  gpio-505 (GPIO-L              )
>  gpio-506 (PEC-PD26            )
>  gpio-507 (PEC-PD27            )
>  gpio-508 (PEC-PD28            )
>  gpio-509 (PEC-PD29            )
>  gpio-510 (PEC-PD30            )
>  gpio-511 (PEC-PD31            )
>
> after adding this
>
>     gpio-line-names = "DSW3-PIN1", "DSW3-PIN2", "DSW3-PIN3", "DSW3-PIN4",
>                       "DSW3-PIN5", "DSW3-PIN6", "DSW3-PIN7", "DSW3-PIN8",
>                       "NC", "PWROFF#",
>                       "GPIO-A", "GPIO-B", "GPIO-C", "GPIO-D",
>                       "PCIE1EXTINT", "PCIE0EXTINT",
>                       "PHY2-INT#", "PHY1-INT#",
>                       "GPIO-E", "GPIO-F", "GPIO-G", "GPIO-H",
>                       "GPIO-I", "GPIO-J", "GPIO-K", "GPIO-L",
>                       "PEC-PD26", "PEC-PD27", "PEC-PD28",
>                       "PEC-PD29", "PEC-PD30", "PEC-PD31";
>
> to the DT node of the GPIO controller.

Perfect. Proper naming of GPIO-A thru L gives userspace an
easy time to get a grip on the right GPIO line on the Low Speed
Connector.

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

Yours,
Linus Walleij

Re: [PATCH 0/2] GPIO support for Socionext Synquacer

[Linus Walleij]

On Tue, Oct 31, 2017 at 1:27 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> I currently have this in my DTS:
>
> &gpio {
>     dsw3_1 {
>         gpios = <0 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_2 {
>         gpios = <1 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_3 {
>         gpios = <2 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_4 {
>         gpios = <3 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_5 {
>         gpios = <4 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_6 {
>         gpios = <5 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_7 {
>         gpios = <6 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
>     dsw3_8 {
>         gpios = <7 GPIO_ACTIVE_HIGH>;
>         gpio-hog;
>         input;
>     };
>
> for the 8 DIP switches that are connected to GPIO lines.

I have no idea how to make proper use of DIP switches really.
We *could* route them as inputs using GPIO keys, but it would
maybe give people the idea that it is a good idea to start
prying them at runtime.

If they don't have a usecase I would just leave them as this.

I guess/hope they will not be used by userspace either.

> So I can add the names for all the lines that have a purpose, but is
> that orthogonal to hogging?

Naming is orthogonal to hogging and should be a functional name
for the line, preferably header name, else rail name or something
else reasonable.

Yours,
Linus Walleij

Re: [PATCH 0/2] GPIO support for Socionext Synquacer

[Ard Biesheuvel]

On 31 October 2017 at 13:13, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Oct 31, 2017 at 1:27 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>
>> I currently have this in my DTS:
>>
>> &gpio {
>>     dsw3_1 {
>>         gpios = <0 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>>     dsw3_2 {
>>         gpios = <1 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>>     dsw3_3 {
>>         gpios = <2 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>>     dsw3_4 {
>>         gpios = <3 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>>     dsw3_5 {
>>         gpios = <4 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>>     dsw3_6 {
>>         gpios = <5 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>>     dsw3_7 {
>>         gpios = <6 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>>     dsw3_8 {
>>         gpios = <7 GPIO_ACTIVE_HIGH>;
>>         gpio-hog;
>>         input;
>>     };
>>
>> for the 8 DIP switches that are connected to GPIO lines.
>
> I have no idea how to make proper use of DIP switches really.
> We *could* route them as inputs using GPIO keys, but it would
> maybe give people the idea that it is a good idea to start
> prying them at runtime.
>
> If they don't have a usecase I would just leave them as this.
>
> I guess/hope they will not be used by userspace either.
>

Not a clue. I guess I can remove the hogs, and simply describe them
using the names.

They are probably more useful at boot time, i.e., to clear the NVRAM
and to en/disable secure boot etc.

>> So I can add the names for all the lines that have a purpose, but is
>> that orthogonal to hogging?
>
> Naming is orthogonal to hogging and should be a functional name
> for the line, preferably header name, else rail name or something
> else reasonable.

Re: [PATCH 0/2] GPIO support for Socionext Synquacer

[Linus Walleij]

On Tue, Oct 31, 2017 at 1:27 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 31 October 2017 at 12:20, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Fri, Oct 27, 2017 at 10:21 PM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>
>>> The Socionext Synquacer SC2A11, which is used in the arm64 Developer Box,
>>> shares its GPIO IP with a Fujitsu SoC for which we already have support
>>> in the tree. So let's tweak it so that we can reuse it.
>>>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Ard Biesheuvel (2):
>>>   gpio: mb86s7x: share with other SoCs as module
>>>   gpio: mb86s70: Revert "Return error if requesting an already assigned
>>>     gpio"
>>
>> Nice. We might need to look into the following wrt this driver:
>>
>> - Using generic MMIO GPIO, i.e. select GPIO_GENERIC in Kconfig
>>   and a patch such as commit 6d125412fc16802012a17665638f49b0b0c81f18
>>   "gpio: iop: Use generic GPIO MMIO functions for driver"
>>   apart from reduced code size this brings the .get_multiple() and
>>   .set_multiple() callbacks for FREE.
>>   The fact that the driver is so simple that it should have been using
>>   MMIO/GENERIC GPIO is a plain oversight during review.
>
> Does this work with the layout if this chip? It has 32 GPIO lines,
> whose controls are mapped onto the lowest 8 bits of 4 adjacent 32-bit
> registers.

I didn't look close enough. No GPIO MMIO is just for simple
1:1 mapping of say 32 bits, so it won't work.

There is opportunity to exploit [get|set]_multiple() callbacks
for any bits that end up in the same group of 8 though,
but it requires some elaborate code for just this driver.
It does make for a nice 8-line oscilloscope to sample
all 8 lines simultaneously with .get_multiple() for example.

But that is just optimization.

If you have the datasheet, also check if the block is really this
simple and whether it maybe actually supports some pin config
like open drain or debounce etc, we have support for that
as well these days using .set_config() in the drivers.

Yours,
Linus Walleij