[PATCH 0/2] MIPS: ath79: Move the GPIO driver to drivers/gpio

[PATCH 0/2] MIPS: ath79: Move the GPIO driver to drivers/gpio

[Alban Bedel]

Hi all,

as requested when I posted the ATH79 OF support serie, here is the move of
the GPIO driver to the GPIO drivers directory. While at it I also removed
the custom pinmux API as it not used by any board.

Alban

Alban Bedel (2):
  MIPS: ath79: Remove the unused GPIO function API
  MIPS: ath79: Move the GPIO driver to drivers/gpio

 arch/mips/ath79/Makefile  |   2 +-
 arch/mips/ath79/common.h  |   3 -
 arch/mips/ath79/gpio.c    | 279 ----------------------------------------------
 drivers/gpio/Makefile     |   1 +
 drivers/gpio/gpio-ath79.c | 236 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 238 insertions(+), 283 deletions(-)
 delete mode 100644 arch/mips/ath79/gpio.c
 create mode 100644 drivers/gpio/gpio-ath79.c

-- 
2.0.0

[PATCH 1/2] MIPS: ath79: Remove the unused GPIO function API

[Alban Bedel]

To prepare moving the GPIO driver to drivers/gpio remove the
platform specific pinmux API. As it is not used by any board,
and such functionality should better be implemented using the
pinmux subsystem just removing it seems to be the best option.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 arch/mips/ath79/common.h |  3 ---
 arch/mips/ath79/gpio.c   | 43 -------------------------------------------
 2 files changed, 46 deletions(-)

diff --git a/arch/mips/ath79/common.h b/arch/mips/ath79/common.h
index e5ea712..ca7cc19 100644
--- a/arch/mips/ath79/common.h
+++ b/arch/mips/ath79/common.h
@@ -25,9 +25,6 @@ unsigned long ath79_get_sys_clk_rate(const char *id);
 void ath79_ddr_ctrl_init(void);
 void ath79_ddr_wb_flush(unsigned int reg);
 
-void ath79_gpio_function_enable(u32 mask);
-void ath79_gpio_function_disable(u32 mask);
-void ath79_gpio_function_setup(u32 set, u32 clear);
 void ath79_gpio_init(void);
 
 #endif /* __ATH79_COMMON_H */
diff --git a/arch/mips/ath79/gpio.c b/arch/mips/ath79/gpio.c
index f59ccb2..c3c92eb 100644
--- a/arch/mips/ath79/gpio.c
+++ b/arch/mips/ath79/gpio.c
@@ -24,8 +24,6 @@
 #include <linux/of_device.h>
 
 #include <asm/mach-ath79/ar71xx_regs.h>
-#include <asm/mach-ath79/ath79.h>
-#include "common.h"
 
 static void __iomem *ath79_gpio_base;
 static u32 ath79_gpio_count;
@@ -139,47 +137,6 @@ static struct gpio_chip ath79_gpio_chip = {
 	.base			= 0,
 };
 
-static void __iomem *ath79_gpio_get_function_reg(void)
-{
-	u32 reg = 0;
-
-	if (soc_is_ar71xx() ||
-	    soc_is_ar724x() ||
-	    soc_is_ar913x() ||
-	    soc_is_ar933x())
-		reg = AR71XX_GPIO_REG_FUNC;
-	else if (soc_is_ar934x())
-		reg = AR934X_GPIO_REG_FUNC;
-	else
-		BUG();
-
-	return ath79_gpio_base + reg;
-}
-
-void ath79_gpio_function_setup(u32 set, u32 clear)
-{
-	void __iomem *reg = ath79_gpio_get_function_reg();
-	unsigned long flags;
-
-	spin_lock_irqsave(&ath79_gpio_lock, flags);
-
-	__raw_writel((__raw_readl(reg) & ~clear) | set, reg);
-	/* flush write */
-	__raw_readl(reg);
-
-	spin_unlock_irqrestore(&ath79_gpio_lock, flags);
-}
-
-void ath79_gpio_function_enable(u32 mask)
-{
-	ath79_gpio_function_setup(mask, 0);
-}
-
-void ath79_gpio_function_disable(u32 mask)
-{
-	ath79_gpio_function_setup(0, mask);
-}
-
 static const struct of_device_id ath79_gpio_of_match[] = {
 	{ .compatible = "qca,ar7100-gpio" },
 	{ .compatible = "qca,ar9340-gpio" },
-- 
2.0.0

[PATCH 2/2] MIPS: ath79: Move the GPIO driver to drivers/gpio

[Alban Bedel]

GPIO drivers should be in drivers/gpio

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 arch/mips/ath79/Makefile  |   2 +-
 arch/mips/ath79/gpio.c    | 236 ----------------------------------------------
 drivers/gpio/Makefile     |   1 +
 drivers/gpio/gpio-ath79.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 238 insertions(+), 237 deletions(-)
 delete mode 100644 arch/mips/ath79/gpio.c
 create mode 100644 drivers/gpio/gpio-ath79.c

diff --git a/arch/mips/ath79/Makefile b/arch/mips/ath79/Makefile
index 5c9ff69..fcc382c 100644
--- a/arch/mips/ath79/Makefile
+++ b/arch/mips/ath79/Makefile
@@ -8,7 +8,7 @@
 # under the terms of the GNU General Public License version 2 as published
 # by the Free Software Foundation.
 
-obj-y	:= prom.o setup.o irq.o common.o clock.o gpio.o
+obj-y	:= prom.o setup.o irq.o common.o clock.o
 
 obj-$(CONFIG_EARLY_PRINTK)		+= early_printk.o
 obj-$(CONFIG_PCI)			+= pci.o
diff --git a/arch/mips/ath79/gpio.c b/arch/mips/ath79/gpio.c
deleted file mode 100644
index c3c92eb..0000000
--- a/arch/mips/ath79/gpio.c
+++ /dev/null
@@ -1,236 +0,0 @@
-/*
- *  Atheros AR71XX/AR724X/AR913X GPIO API support
- *
- *  Copyright (C) 2010-2011 Jaiganesh Narayanan <jnarayanan@atheros.com>
- *  Copyright (C) 2008-2011 Gabor Juhos <juhosg@openwrt.org>
- *  Copyright (C) 2008 Imre Kaloz <kaloz@openwrt.org>
- *
- *  Parts of this file are based on Atheros' 2.6.15/2.6.31 BSP
- *
- *  This program is free software; you can redistribute it and/or modify it
- *  under the terms of the GNU General Public License version 2 as published
- *  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/platform_data/gpio-ath79.h>
-#include <linux/of_device.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,
-};
-
-static const struct of_device_id ath79_gpio_of_match[] = {
-	{ .compatible = "qca,ar7100-gpio" },
-	{ .compatible = "qca,ar9340-gpio" },
-	{},
-};
-
-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;
-	struct resource *res;
-	bool oe_inverted;
-	int err;
-
-	if (np) {
-		err = of_property_read_u32(np, "ngpios", &ath79_gpio_count);
-		if (err) {
-			dev_err(&pdev->dev, "ngpios property is not valid\n");
-			return err;
-		}
-		if (ath79_gpio_count >= 32) {
-			dev_err(&pdev->dev, "ngpios must be less than 32\n");
-			return -EINVAL;
-		}
-		oe_inverted = of_device_is_compatible(np, "qca,ar9340-gpio");
-	} else if (pdata) {
-		ath79_gpio_count = pdata->ngpios;
-		oe_inverted = pdata->oe_inverted;
-	} else {
-		dev_err(&pdev->dev, "No DT node or platform data found\n");
-		return -EINVAL;
-	}
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ath79_gpio_base = devm_ioremap_nocache(
-		&pdev->dev, res->start, resource_size(res));
-	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 = gpiochip_add(&ath79_gpio_chip);
-	if (err) {
-		dev_err(&pdev->dev,
-			"cannot add AR71xx GPIO chip, error=%d", err);
-		return err;
-	}
-
-	return 0;
-}
-
-static struct platform_driver ath79_gpio_driver = {
-	.driver = {
-		.name = "ath79-gpio",
-		.of_match_table	= ath79_gpio_of_match,
-	},
-	.probe = ath79_gpio_probe,
-};
-
-module_platform_driver(ath79_gpio_driver);
-
-int gpio_get_value(unsigned gpio)
-{
-	if (gpio < ath79_gpio_count)
-		return __ath79_gpio_get_value(gpio);
-
-	return __gpio_get_value(gpio);
-}
-EXPORT_SYMBOL(gpio_get_value);
-
-void gpio_set_value(unsigned gpio, int value)
-{
-	if (gpio < ath79_gpio_count)
-		__ath79_gpio_set_value(gpio, value);
-	else
-		__gpio_set_value(gpio, value);
-}
-EXPORT_SYMBOL(gpio_set_value);
-
-int gpio_to_irq(unsigned gpio)
-{
-	/* FIXME */
-	return -EINVAL;
-}
-EXPORT_SYMBOL(gpio_to_irq);
-
-int irq_to_gpio(unsigned irq)
-{
-	/* FIXME */
-	return -EINVAL;
-}
-EXPORT_SYMBOL(irq_to_gpio);
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index f82cd67..2b64f61 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -20,6 +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_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
new file mode 100644
index 0000000..c3c92eb
--- /dev/null
+++ b/drivers/gpio/gpio-ath79.c
@@ -0,0 +1,236 @@
+/*
+ *  Atheros AR71XX/AR724X/AR913X GPIO API support
+ *
+ *  Copyright (C) 2010-2011 Jaiganesh Narayanan <jnarayanan@atheros.com>
+ *  Copyright (C) 2008-2011 Gabor Juhos <juhosg@openwrt.org>
+ *  Copyright (C) 2008 Imre Kaloz <kaloz@openwrt.org>
+ *
+ *  Parts of this file are based on Atheros' 2.6.15/2.6.31 BSP
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License version 2 as published
+ *  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/platform_data/gpio-ath79.h>
+#include <linux/of_device.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,
+};
+
+static const struct of_device_id ath79_gpio_of_match[] = {
+	{ .compatible = "qca,ar7100-gpio" },
+	{ .compatible = "qca,ar9340-gpio" },
+	{},
+};
+
+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;
+	struct resource *res;
+	bool oe_inverted;
+	int err;
+
+	if (np) {
+		err = of_property_read_u32(np, "ngpios", &ath79_gpio_count);
+		if (err) {
+			dev_err(&pdev->dev, "ngpios property is not valid\n");
+			return err;
+		}
+		if (ath79_gpio_count >= 32) {
+			dev_err(&pdev->dev, "ngpios must be less than 32\n");
+			return -EINVAL;
+		}
+		oe_inverted = of_device_is_compatible(np, "qca,ar9340-gpio");
+	} else if (pdata) {
+		ath79_gpio_count = pdata->ngpios;
+		oe_inverted = pdata->oe_inverted;
+	} else {
+		dev_err(&pdev->dev, "No DT node or platform data found\n");
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ath79_gpio_base = devm_ioremap_nocache(
+		&pdev->dev, res->start, resource_size(res));
+	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 = gpiochip_add(&ath79_gpio_chip);
+	if (err) {
+		dev_err(&pdev->dev,
+			"cannot add AR71xx GPIO chip, error=%d", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static struct platform_driver ath79_gpio_driver = {
+	.driver = {
+		.name = "ath79-gpio",
+		.of_match_table	= ath79_gpio_of_match,
+	},
+	.probe = ath79_gpio_probe,
+};
+
+module_platform_driver(ath79_gpio_driver);
+
+int gpio_get_value(unsigned gpio)
+{
+	if (gpio < ath79_gpio_count)
+		return __ath79_gpio_get_value(gpio);
+
+	return __gpio_get_value(gpio);
+}
+EXPORT_SYMBOL(gpio_get_value);
+
+void gpio_set_value(unsigned gpio, int value)
+{
+	if (gpio < ath79_gpio_count)
+		__ath79_gpio_set_value(gpio, value);
+	else
+		__gpio_set_value(gpio, value);
+}
+EXPORT_SYMBOL(gpio_set_value);
+
+int gpio_to_irq(unsigned gpio)
+{
+	/* FIXME */
+	return -EINVAL;
+}
+EXPORT_SYMBOL(gpio_to_irq);
+
+int irq_to_gpio(unsigned irq)
+{
+	/* FIXME */
+	return -EINVAL;
+}
+EXPORT_SYMBOL(irq_to_gpio);
-- 
2.0.0

Re: [PATCH 1/2] MIPS: ath79: Remove the unused GPIO function API

[Sergey Ryazanov]

2015-07-03 12:11 GMT+03:00 Alban Bedel <albeu@free.fr>:
> To prepare moving the GPIO driver to drivers/gpio remove the
> platform specific pinmux API. As it is not used by any board,
> and such functionality should better be implemented using the
> pinmux subsystem just removing it seems to be the best option.
>
For reference: OpenWRT uses this functions to activate UART.

-- 
Sergey

Re: [PATCH 0/2] MIPS: ath79: Move the GPIO driver to drivers/gpio

[Linus Walleij]

On Fri, Jul 3, 2015 at 11:11 AM, Alban Bedel <albeu@free.fr> wrote:

> as requested when I posted the ATH79 OF support serie, here is the move of
> the GPIO driver to the GPIO drivers directory. While at it I also removed
> the custom pinmux API as it not used by any board.

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

Please apply to the MIPS tree, because it is better to have all
GPIO mess collected in one place than spread out over the kernel.

Admittedly not having it in "my" folder makes me sloppy and not try to
push for modernizing the GPIO driver(s)...

This driver could use some modernization and platform cleanup though.
I'll comment on the move patch for reference, don't see it as any blocker,
more as a TODO list.

Yours,
Linus Walleij

Re: [PATCH 2/2] MIPS: ath79: Move the GPIO driver to drivers/gpio

[Linus Walleij]

On Fri, Jul 3, 2015 at 11:11 AM, Alban Bedel <albeu@free.fr> wrote:

> +++ b/drivers/gpio/gpio-ath79.c
> @@ -0,0 +1,236 @@
> +/*
> + *  Atheros AR71XX/AR724X/AR913X GPIO API support
> + *
> + *  Copyright (C) 2010-2011 Jaiganesh Narayanan <jnarayanan@atheros.com>
> + *  Copyright (C) 2008-2011 Gabor Juhos <juhosg@openwrt.org>
> + *  Copyright (C) 2008 Imre Kaloz <kaloz@openwrt.org>
> + *
> + *  Parts of this file are based on Atheros' 2.6.15/2.6.31 BSP
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under the terms of the GNU General Public License version 2 as published
> + *  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>

Nominally only <linux/gpio/driver.h> should be enough for a driver.

> +#include <linux/platform_data/gpio-ath79.h>
> +#include <linux/of_device.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);

Would prefer to use the state container design pattern from
Documentation/driver-model/design-patterns.txt

> +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);

I have a vague memory of some semantics being strange in the MIPS
camp, but doesn't writel_relaxed() do what you want? (Benji brought
something related up for discussion on the ksummit list...)

> +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);
> +}

Strange double functions that can be refactored out I think.
All internal uses should be able to reference the gpio_chip.

> +static void ath79_gpio_set_value(struct gpio_chip *chip,
> +                                 unsigned offset, int value)
> +{
> +       __ath79_gpio_set_value(offset, value);
> +}

Same.

> +       __raw_writel(__raw_readl(base + AR71XX_GPIO_REG_OE) | (1 << offset),
> +                    base + AR71XX_GPIO_REG_OE);

I tend to #include <linux/bitops.h>

And just | BIT(offset) there in the end.

Some prefer it like this, I just think the explit bitops look neat.

> +int gpio_get_value(unsigned gpio)
> +EXPORT_SYMBOL(gpio_get_value);
> +void gpio_set_value(unsigned gpio, int value)
> +EXPORT_SYMBOL(gpio_set_value);
> +int gpio_to_irq(unsigned gpio)
> +EXPORT_SYMBOL(gpio_to_irq);

These are some quite horrific overrides of the gpiolib functions.
I would like it rooted out and replaced with the gpiolib implementations.
I think we managed to exorcise this "generic GPIO" from ARM but
I'm honestly not certain.

> +int irq_to_gpio(unsigned irq)
> +{
> +       /* FIXME */
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL(irq_to_gpio);

Can't you just delete this? It has been removed from generic GPIO and
gpiolib because of ambiguity.

Yours,
Linus Walleij

Re: [PATCH 1/2] MIPS: ath79: Remove the unused GPIO function API

[Alban]

On Sat, 4 Jul 2015 19:58:32 +0300
Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:

> 2015-07-03 12:11 GMT+03:00 Alban Bedel <albeu@free.fr>:
> > To prepare moving the GPIO driver to drivers/gpio remove the
> > platform specific pinmux API. As it is not used by any board,
> > and such functionality should better be implemented using the
> > pinmux subsystem just removing it seems to be the best option.
> >
> For reference: OpenWRT uses this functions to activate UART.

The pinctrl-single driver should be usable for all SoC where this code
was used. I haven't tried it yet, but it should only be a matter of
writing the DTS down.

Alban

Re: [PATCH 1/2] MIPS: ath79: Remove the unused GPIO function API

[Linus Walleij]

On Fri, Jul 3, 2015 at 11:11 AM, Alban Bedel <albeu@free.fr> wrote:

> To prepare moving the GPIO driver to drivers/gpio remove the
> platform specific pinmux API. As it is not used by any board,
> and such functionality should better be implemented using the
> pinmux subsystem just removing it seems to be the best option.
>
> Signed-off-by: Alban Bedel <albeu@free.fr>

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

Yours,
Linus Walleij

Re: [PATCH 1/2] MIPS: ath79: Remove the unused GPIO function API

[Linus Walleij]

On Sat, Jul 4, 2015 at 6:58 PM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
> 2015-07-03 12:11 GMT+03:00 Alban Bedel <albeu@free.fr>:
>> To prepare moving the GPIO driver to drivers/gpio remove the
>> platform specific pinmux API. As it is not used by any board,
>> and such functionality should better be implemented using the
>> pinmux subsystem just removing it seems to be the best option.
>>
> For reference: OpenWRT uses this functions to activate UART.

OpenWRT is wholeheartedly and warmly welcome to work upstream,
I will help whenever I can.

I have seen their patch stack and it makes me sad :(

Yours,
Linus Walleij