* [PATCH 0/2] GPIO support for Socionext Synquacer
@ 2017-10-27 20:21 Ard Biesheuvel
2017-10-27 20:21 ` [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module Ard Biesheuvel
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2017-10-27 20:21 UTC (permalink / raw)
To: linux-gpio
Cc: linus.walleij, daniel.thompson, leif.lindholm, jaswinder.singh,
masami.hiramatsu, 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module 2017-10-27 20:21 [PATCH 0/2] GPIO support for Socionext Synquacer Ard Biesheuvel @ 2017-10-27 20:21 ` Ard Biesheuvel 2017-10-30 17:15 ` Ard Biesheuvel 2017-10-31 12:12 ` Linus Walleij 2017-10-27 20:21 ` [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" Ard Biesheuvel 2017-10-31 12:20 ` [PATCH 0/2] GPIO support for Socionext Synquacer Linus Walleij 2 siblings, 2 replies; 14+ messages in thread From: Ard Biesheuvel @ 2017-10-27 20:21 UTC (permalink / raw) To: linux-gpio Cc: linus.walleij, daniel.thompson, leif.lindholm, jaswinder.singh, masami.hiramatsu, Ard Biesheuvel, Geliang Tang, Paul Gortmaker 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module 2017-10-27 20:21 ` [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module Ard Biesheuvel @ 2017-10-30 17:15 ` Ard Biesheuvel 2017-10-31 12:12 ` Linus Walleij 1 sibling, 0 replies; 14+ messages in thread From: Ard Biesheuvel @ 2017-10-30 17:15 UTC (permalink / raw) To: linux-gpio, Linus Walleij Cc: Daniel Thompson, Leif Lindholm, Jaswinder Singh, Masami Hiramatsu, Ard Biesheuvel, Geliang Tang, Paul Gortmaker 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. ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module 2017-10-27 20:21 ` [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module Ard Biesheuvel 2017-10-30 17:15 ` Ard Biesheuvel @ 2017-10-31 12:12 ` Linus Walleij 1 sibling, 0 replies; 14+ messages in thread From: Linus Walleij @ 2017-10-31 12:12 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu, Geliang Tang, Paul Gortmaker 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" 2017-10-27 20:21 [PATCH 0/2] GPIO support for Socionext Synquacer Ard Biesheuvel 2017-10-27 20:21 ` [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module Ard Biesheuvel @ 2017-10-27 20:21 ` Ard Biesheuvel 2017-10-28 1:53 ` Axel Lin 2017-10-31 12:14 ` Linus Walleij 2017-10-31 12:20 ` [PATCH 0/2] GPIO support for Socionext Synquacer Linus Walleij 2 siblings, 2 replies; 14+ messages in thread From: Ard Biesheuvel @ 2017-10-27 20:21 UTC (permalink / raw) To: linux-gpio Cc: linus.walleij, daniel.thompson, leif.lindholm, jaswinder.singh, masami.hiramatsu, Ard Biesheuvel, Axel Lin 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" 2017-10-27 20:21 ` [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" Ard Biesheuvel @ 2017-10-28 1:53 ` Axel Lin 2017-10-31 12:14 ` Linus Walleij 1 sibling, 0 replies; 14+ messages in thread From: Axel Lin @ 2017-10-28 1:53 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-gpio, Linus Walleij, Daniel Thompson, leif.lindholm, Jassi Brar, masami.hiramatsu 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" 2017-10-27 20:21 ` [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" Ard Biesheuvel 2017-10-28 1:53 ` Axel Lin @ 2017-10-31 12:14 ` Linus Walleij 1 sibling, 0 replies; 14+ messages in thread From: Linus Walleij @ 2017-10-31 12:14 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu, Axel Lin 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] GPIO support for Socionext Synquacer 2017-10-27 20:21 [PATCH 0/2] GPIO support for Socionext Synquacer Ard Biesheuvel 2017-10-27 20:21 ` [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module Ard Biesheuvel 2017-10-27 20:21 ` [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" Ard Biesheuvel @ 2017-10-31 12:20 ` Linus Walleij 2017-10-31 12:27 ` Ard Biesheuvel 2 siblings, 1 reply; 14+ messages in thread From: Linus Walleij @ 2017-10-31 12:20 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] GPIO support for Socionext Synquacer 2017-10-31 12:20 ` [PATCH 0/2] GPIO support for Socionext Synquacer Linus Walleij @ 2017-10-31 12:27 ` Ard Biesheuvel 2017-10-31 12:59 ` Ard Biesheuvel ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Ard Biesheuvel @ 2017-10-31 12:27 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu 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? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] GPIO support for Socionext Synquacer 2017-10-31 12:27 ` Ard Biesheuvel @ 2017-10-31 12:59 ` Ard Biesheuvel 2017-10-31 13:10 ` Linus Walleij 2017-10-31 13:13 ` Linus Walleij 2017-10-31 13:19 ` Linus Walleij 2 siblings, 1 reply; 14+ messages in thread From: Ard Biesheuvel @ 2017-10-31 12:59 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] GPIO support for Socionext Synquacer 2017-10-31 12:59 ` Ard Biesheuvel @ 2017-10-31 13:10 ` Linus Walleij 0 siblings, 0 replies; 14+ messages in thread From: Linus Walleij @ 2017-10-31 13:10 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] GPIO support for Socionext Synquacer 2017-10-31 12:27 ` Ard Biesheuvel 2017-10-31 12:59 ` Ard Biesheuvel @ 2017-10-31 13:13 ` Linus Walleij 2017-10-31 13:19 ` Ard Biesheuvel 2017-10-31 13:19 ` Linus Walleij 2 siblings, 1 reply; 14+ messages in thread From: Linus Walleij @ 2017-10-31 13:13 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] GPIO support for Socionext Synquacer 2017-10-31 13:13 ` Linus Walleij @ 2017-10-31 13:19 ` Ard Biesheuvel 0 siblings, 0 replies; 14+ messages in thread From: Ard Biesheuvel @ 2017-10-31 13:19 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] GPIO support for Socionext Synquacer 2017-10-31 12:27 ` Ard Biesheuvel 2017-10-31 12:59 ` Ard Biesheuvel 2017-10-31 13:13 ` Linus Walleij @ 2017-10-31 13:19 ` Linus Walleij 2 siblings, 0 replies; 14+ messages in thread From: Linus Walleij @ 2017-10-31 13:19 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-gpio, Daniel Thompson, Leif Lindholm, Jassi Brar, Masami Hiramatsu 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-10-31 13:19 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-27 20:21 [PATCH 0/2] GPIO support for Socionext Synquacer Ard Biesheuvel 2017-10-27 20:21 ` [PATCH 1/2] gpio: mb86s7x: share with other SoCs as module Ard Biesheuvel 2017-10-30 17:15 ` Ard Biesheuvel 2017-10-31 12:12 ` Linus Walleij 2017-10-27 20:21 ` [PATCH 2/2] gpio: mb86s70: Revert "Return error if requesting an already assigned gpio" Ard Biesheuvel 2017-10-28 1:53 ` Axel Lin 2017-10-31 12:14 ` Linus Walleij 2017-10-31 12:20 ` [PATCH 0/2] GPIO support for Socionext Synquacer Linus Walleij 2017-10-31 12:27 ` Ard Biesheuvel 2017-10-31 12:59 ` Ard Biesheuvel 2017-10-31 13:10 ` Linus Walleij 2017-10-31 13:13 ` Linus Walleij 2017-10-31 13:19 ` Ard Biesheuvel 2017-10-31 13:19 ` Linus Walleij
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.