[PATCH 1/3] MAINTAINERS: Add an entry for the ATH79 GPIO driver

[PATCH 1/3] MAINTAINERS: Add an entry for the ATH79 GPIO driver

[Alban Bedel]

Add an entry for the ATH79 GPIO driver with myself as maintainer.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9de185d..957b148 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1780,6 +1780,14 @@ S:	Supported
 F:	Documentation/aoe/
 F:	drivers/block/aoe/
 
+ATHEROS 71XX/9XXX GPIO DRIVER
+M:	"Alban Bedel <albeu@free.fr>"
+W:	https://github.com/AlbanBedel/linux
+T:	git git://github.com/AlbanBedel/linux
+S:	Maintained
+F:	drivers/gpio/gpio-ath79.c
+F:	Documentation/devicetree/bindings/gpio/gpio-ath79.txt
+
 ATHEROS ATH GENERIC UTILITIES
 M:	"Luis R. Rodriguez" <mcgrof@do-not-panic.com>
 L:	linux-wireless@vger.kernel.org
-- 
2.0.0

[PATCH 2/3] gpio: ath79: Move to the generic GPIO driver

[Alban Bedel]

Turn the ath79 driver into a true driver supporting multiple
instances while dropping most of the code in favor of the generic
MMIO GPIO driver.

As the driver now depend on CONFIG_GPIO_GENERIC also add a Kconfig
entry to make the driver optional.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 drivers/gpio/Kconfig      |   9 +++
 drivers/gpio/Makefile     |   2 +-
 drivers/gpio/gpio-ath79.c | 152 +++++++++-------------------------------------
 3 files changed, 38 insertions(+), 125 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8949b3f..a979f1a 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -119,6 +119,15 @@ config GPIO_ALTERA
 
 	  If driver is built as a module it will be called gpio-altera.
 
+config GPIO_ATH79
+	tristate "Atheros AR71XX/AR724X/AR913X GPIO support"
+	default y if ATH79
+	depends on ATH79 || COMPILE_TEST
+	select GPIO_GENERIC
+	help
+	  Select this option to enable GPIO driver for
+	  Atheros AR71XX/AR724X/AR913X SoC devices.
+
 config GPIO_BCM_KONA
 	bool "Broadcom Kona GPIO"
 	depends on OF_GPIO && (ARCH_BCM_MOBILE || COMPILE_TEST)
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index f79a7c4..24c7371 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -20,7 +20,7 @@ obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
 obj-$(CONFIG_GPIO_ALTERA)  	+= gpio-altera.o
 obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
-obj-$(CONFIG_ATH79)		+= gpio-ath79.o
+obj-$(CONFIG_GPIO_ATH79)	+= gpio-ath79.o
 obj-$(CONFIG_GPIO_BCM_KONA)	+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BRCMSTB)	+= gpio-brcmstb.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
diff --git a/drivers/gpio/gpio-ath79.c b/drivers/gpio/gpio-ath79.c
index 03b9953..3e71ced 100644
--- a/drivers/gpio/gpio-ath79.c
+++ b/drivers/gpio/gpio-ath79.c
@@ -12,129 +12,15 @@
  *  by the Free Software Foundation.
  */
 
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/types.h>
-#include <linux/spinlock.h>
-#include <linux/io.h>
-#include <linux/ioport.h>
-#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
 #include <linux/platform_data/gpio-ath79.h>
 #include <linux/of_device.h>
+#include <linux/basic_mmio_gpio.h>
 
 #include <asm/mach-ath79/ar71xx_regs.h>
 
-static void __iomem *ath79_gpio_base;
-static u32 ath79_gpio_count;
-static DEFINE_SPINLOCK(ath79_gpio_lock);
-
-static void __ath79_gpio_set_value(unsigned gpio, int value)
-{
-	void __iomem *base = ath79_gpio_base;
-
-	if (value)
-		__raw_writel(1 << gpio, base + AR71XX_GPIO_REG_SET);
-	else
-		__raw_writel(1 << gpio, base + AR71XX_GPIO_REG_CLEAR);
-}
-
-static int __ath79_gpio_get_value(unsigned gpio)
-{
-	return (__raw_readl(ath79_gpio_base + AR71XX_GPIO_REG_IN) >> gpio) & 1;
-}
-
-static int ath79_gpio_get_value(struct gpio_chip *chip, unsigned offset)
-{
-	return __ath79_gpio_get_value(offset);
-}
-
-static void ath79_gpio_set_value(struct gpio_chip *chip,
-				  unsigned offset, int value)
-{
-	__ath79_gpio_set_value(offset, value);
-}
-
-static int ath79_gpio_direction_input(struct gpio_chip *chip,
-				       unsigned offset)
-{
-	void __iomem *base = ath79_gpio_base;
-	unsigned long flags;
-
-	spin_lock_irqsave(&ath79_gpio_lock, flags);
-
-	__raw_writel(__raw_readl(base + AR71XX_GPIO_REG_OE) & ~(1 << offset),
-		     base + AR71XX_GPIO_REG_OE);
-
-	spin_unlock_irqrestore(&ath79_gpio_lock, flags);
-
-	return 0;
-}
-
-static int ath79_gpio_direction_output(struct gpio_chip *chip,
-					unsigned offset, int value)
-{
-	void __iomem *base = ath79_gpio_base;
-	unsigned long flags;
-
-	spin_lock_irqsave(&ath79_gpio_lock, flags);
-
-	if (value)
-		__raw_writel(1 << offset, base + AR71XX_GPIO_REG_SET);
-	else
-		__raw_writel(1 << offset, base + AR71XX_GPIO_REG_CLEAR);
-
-	__raw_writel(__raw_readl(base + AR71XX_GPIO_REG_OE) | (1 << offset),
-		     base + AR71XX_GPIO_REG_OE);
-
-	spin_unlock_irqrestore(&ath79_gpio_lock, flags);
-
-	return 0;
-}
-
-static int ar934x_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
-{
-	void __iomem *base = ath79_gpio_base;
-	unsigned long flags;
-
-	spin_lock_irqsave(&ath79_gpio_lock, flags);
-
-	__raw_writel(__raw_readl(base + AR71XX_GPIO_REG_OE) | (1 << offset),
-		     base + AR71XX_GPIO_REG_OE);
-
-	spin_unlock_irqrestore(&ath79_gpio_lock, flags);
-
-	return 0;
-}
-
-static int ar934x_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
-					int value)
-{
-	void __iomem *base = ath79_gpio_base;
-	unsigned long flags;
-
-	spin_lock_irqsave(&ath79_gpio_lock, flags);
-
-	if (value)
-		__raw_writel(1 << offset, base + AR71XX_GPIO_REG_SET);
-	else
-		__raw_writel(1 << offset, base + AR71XX_GPIO_REG_CLEAR);
-
-	__raw_writel(__raw_readl(base + AR71XX_GPIO_REG_OE) & ~(1 << offset),
-		     base + AR71XX_GPIO_REG_OE);
-
-	spin_unlock_irqrestore(&ath79_gpio_lock, flags);
-
-	return 0;
-}
-
-static struct gpio_chip ath79_gpio_chip = {
-	.label			= "ath79",
-	.get			= ath79_gpio_get_value,
-	.set			= ath79_gpio_set_value,
-	.direction_input	= ath79_gpio_direction_input,
-	.direction_output	= ath79_gpio_direction_output,
-	.base			= 0,
+struct ath79_gpio {
+	struct bgpio_chip bgc;
 };
 
 static const struct of_device_id ath79_gpio_of_match[] = {
@@ -147,10 +33,17 @@ static int ath79_gpio_probe(struct platform_device *pdev)
 {
 	struct ath79_gpio_platform_data *pdata = pdev->dev.platform_data;
 	struct device_node *np = pdev->dev.of_node;
+	void __iomem *ath79_gpio_base;
+	struct ath79_gpio *ctrl;
 	struct resource *res;
+	u32 ath79_gpio_count;
 	bool oe_inverted;
 	int err;
 
+	ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
 	if (np) {
 		err = of_property_read_u32(np, "ngpios", &ath79_gpio_count);
 		if (err) {
@@ -176,14 +69,25 @@ static int ath79_gpio_probe(struct platform_device *pdev)
 	if (!ath79_gpio_base)
 		return -ENOMEM;
 
-	ath79_gpio_chip.dev = &pdev->dev;
-	ath79_gpio_chip.ngpio = ath79_gpio_count;
-	if (oe_inverted) {
-		ath79_gpio_chip.direction_input = ar934x_gpio_direction_input;
-		ath79_gpio_chip.direction_output = ar934x_gpio_direction_output;
+	err = bgpio_init(&ctrl->bgc, &pdev->dev, 4,
+			ath79_gpio_base + AR71XX_GPIO_REG_IN,
+			ath79_gpio_base + AR71XX_GPIO_REG_SET,
+			ath79_gpio_base + AR71XX_GPIO_REG_CLEAR,
+			oe_inverted ?
+				NULL : ath79_gpio_base + AR71XX_GPIO_REG_OE,
+			oe_inverted ?
+				ath79_gpio_base + AR71XX_GPIO_REG_OE : NULL,
+			0);
+	if (err) {
+		dev_err(&pdev->dev, "bgpio_init failed\n");
+		return err;
 	}
 
-	err = gpiochip_add(&ath79_gpio_chip);
+	ctrl->bgc.gc.label = "ath79";
+	ctrl->bgc.gc.base = 0;
+	ctrl->bgc.gc.ngpio = ath79_gpio_count;
+
+	err = gpiochip_add(&ctrl->bgc.gc);
 	if (err) {
 		dev_err(&pdev->dev,
 			"cannot add AR71xx GPIO chip, error=%d", err);
-- 
2.0.0

Re: [PATCH 2/3] gpio: ath79: Move to the generic GPIO driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]

Hi Alban,

[auto build test ERROR on v4.3-rc7 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Alban-Bedel/MAINTAINERS-Add-an-entry-for-the-ATH79-GPIO-driver/20151030-183832
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/gpio/gpio-ath79.c:20:40: fatal error: asm/mach-ath79/ar71xx_regs.h: No such file or directory
   compilation terminated.

vim +20 drivers/gpio/gpio-ath79.c

6eae43c5 arch/mips/ath79/gpio.c    Gabor Juhos 2011-01-04  14  
7351d84b drivers/gpio/gpio-ath79.c Alban Bedel 2015-10-30  15  #include <linux/gpio/driver.h>
2ddf3a79 arch/mips/ath79/gpio.c    Alban Bedel 2015-05-31  16  #include <linux/platform_data/gpio-ath79.h>
2ddf3a79 arch/mips/ath79/gpio.c    Alban Bedel 2015-05-31  17  #include <linux/of_device.h>
7351d84b drivers/gpio/gpio-ath79.c Alban Bedel 2015-10-30  18  #include <linux/basic_mmio_gpio.h>
6eae43c5 arch/mips/ath79/gpio.c    Gabor Juhos 2011-01-04  19  
6eae43c5 arch/mips/ath79/gpio.c    Gabor Juhos 2011-01-04 @20  #include <asm/mach-ath79/ar71xx_regs.h>
6eae43c5 arch/mips/ath79/gpio.c    Gabor Juhos 2011-01-04  21  
7351d84b drivers/gpio/gpio-ath79.c Alban Bedel 2015-10-30  22  struct ath79_gpio {
7351d84b drivers/gpio/gpio-ath79.c Alban Bedel 2015-10-30  23  	struct bgpio_chip bgc;

:::::: The code at line 20 was first introduced by commit
:::::: 6eae43c57ee92de91f6cc7c391cea97c43295da0 MIPS: ath79: add GPIOLIB support

:::::: TO: Gabor Juhos <juhosg@openwrt.org>
:::::: CC: Ralf Baechle <ralf@linux-mips.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 51596 bytes --]

Re: [PATCH 2/3] gpio: ath79: Move to the generic GPIO driver

[Linus Walleij]

On Fri, Oct 30, 2015 at 11:36 AM, Alban Bedel <albeu@free.fr> wrote:

> Turn the ath79 driver into a true driver supporting multiple
> instances while dropping most of the code in favor of the generic
> MMIO GPIO driver.
>
> As the driver now depend on CONFIG_GPIO_GENERIC also add a Kconfig
> entry to make the driver optional.
>
> Signed-off-by: Alban Bedel <albeu@free.fr>

This patch looks nice but sadly does not apply to my "devel" branch in
the GPIO tree:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=devel

Please rebase it on this branch or wait until after the merge window.

> +config GPIO_ATH79
> +       tristate "Atheros AR71XX/AR724X/AR913X GPIO support"
> +       default y if ATH79
> +       depends on ATH79 || COMPILE_TEST

I think the build robot is complaining because of COMPILE_TEST. This
driver doesn't really compile on anything else than ATH79 does it?
Noone else has asm/mach-ath79/ar71xx_regs.h

So I suggest dropping the compile test until this is fixed (e.g. by moving
the GPIO register offsets into the driver, what do I know).

> +       select GPIO_GENERIC

This is very very nice.

And works out nice, as we can see.

Yours,
Linus Walleij

Re: [PATCH 2/3] gpio: ath79: Move to the generic GPIO driver

[Alban]

On Sat, 31 Oct 2015 22:02:53 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Fri, Oct 30, 2015 at 11:36 AM, Alban Bedel <albeu@free.fr> wrote:
> 
> > Turn the ath79 driver into a true driver supporting multiple
> > instances while dropping most of the code in favor of the generic
> > MMIO GPIO driver.
> >
> > As the driver now depend on CONFIG_GPIO_GENERIC also add a Kconfig
> > entry to make the driver optional.
> >
> > Signed-off-by: Alban Bedel <albeu@free.fr>
> 
> This patch looks nice but sadly does not apply to my "devel" branch in
> the GPIO tree:
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=devel
> 
> Please rebase it on this branch or wait until after the merge window.

I see that you applied my previous patch "gpio: ath79: Convert to the
state container design pattern", this series was supposed to replace
this patch. I'm sorry I forgot to mention this. That old patch doesn't
make much sense any more as most of the code it changed just get
dropped in the move to GPIO_GENERIC.

> > +config GPIO_ATH79
> > +       tristate "Atheros AR71XX/AR724X/AR913X GPIO support"
> > +       default y if ATH79
> > +       depends on ATH79 || COMPILE_TEST
> 
> I think the build robot is complaining because of COMPILE_TEST. This
> driver doesn't really compile on anything else than ATH79 does it?
> Noone else has asm/mach-ath79/ar71xx_regs.h
>
> So I suggest dropping the compile test until this is fixed (e.g. by
> moving the GPIO register offsets into the driver, what do I know).

I'm going to split the compile test support to a separate patch.
 
Alban

Re: [PATCH 2/3] gpio: ath79: Move to the generic GPIO driver

[Linus Walleij]

On Sun, Nov 1, 2015 at 6:46 PM, Alban <albeu@free.fr> wrote:
> On Sat, 31 Oct 2015 22:02:53 +0100
> Linus Walleij <linus.walleij@linaro.org> wrote:

>> Please rebase it on this branch or wait until after the merge window.
>
> I see that you applied my previous patch "gpio: ath79: Convert to the
> state container design pattern", this series was supposed to replace
> this patch. I'm sorry I forgot to mention this. That old patch doesn't
> make much sense any more as most of the code it changed just get
> dropped in the move to GPIO_GENERIC.

Hm you can tell me exactly which commits I need to revert
or something ... I cannot rebase the tree now.

It would be best if you cook a GPIO_GENERIC patch based on
devel.

Yours,
Linus Walleij

[PATCH 3/3] gpio: ath79: Make the driver removable

[Alban Bedel]

As we now allow the driver to be built as a module it should be
removable.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 drivers/gpio/gpio-ath79.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpio/gpio-ath79.c b/drivers/gpio/gpio-ath79.c
index 3e71ced..6108dff 100644
--- a/drivers/gpio/gpio-ath79.c
+++ b/drivers/gpio/gpio-ath79.c
@@ -43,6 +43,7 @@ static int ath79_gpio_probe(struct platform_device *pdev)
 	ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
 	if (!ctrl)
 		return -ENOMEM;
+	platform_set_drvdata(pdev, ctrl);
 
 	if (np) {
 		err = of_property_read_u32(np, "ngpios", &ath79_gpio_count);
@@ -97,12 +98,20 @@ static int ath79_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int ath79_gpio_remove(struct platform_device *pdev)
+{
+	struct ath79_gpio *ctrl = platform_get_drvdata(pdev);
+
+	return bgpio_remove(&ctrl->bgc);
+}
+
 static struct platform_driver ath79_gpio_driver = {
 	.driver = {
 		.name = "ath79-gpio",
 		.of_match_table	= ath79_gpio_of_match,
 	},
 	.probe = ath79_gpio_probe,
+	.remove = ath79_gpio_remove,
 };
 
 module_platform_driver(ath79_gpio_driver);
-- 
2.0.0

Re: [PATCH 3/3] gpio: ath79: Make the driver removable

[Linus Walleij]

On Fri, Oct 30, 2015 at 11:36 AM, Alban Bedel <albeu@free.fr> wrote:

> As we now allow the driver to be built as a module it should be
> removable.
>
> Signed-off-by: Alban Bedel <albeu@free.fr>

Looks good but needs rebasing on top of the rebased 2/3.

Yours,
Linus Walleij

Re: [PATCH 1/3] MAINTAINERS: Add an entry for the ATH79 GPIO driver

[Linus Walleij]

On Fri, Oct 30, 2015 at 11:36 AM, Alban Bedel <albeu@free.fr> wrote:

> Add an entry for the ATH79 GPIO driver with myself as maintainer.
>
> Signed-off-by: Alban Bedel <albeu@free.fr>

Applied, but:

> +M:     "Alban Bedel <albeu@free.fr>"

I unquoted that. I didn't see anyone quote all including the mail
address before.

Yours,
Linus Walleij