diff mbox

[U-Boot,v5,3/8] GPIO: add Dove support to Kirkwood GPIO driver

Message ID 1372195668-25496-4-git-send-email-t-uboot@infra-silbe.de
State Deferred
Delegated to: Stefan Roese
Headers show

Commit Message

Sascha Silbe June 25, 2013, 9:27 p.m. UTC 
The GPIO support of Dove is very similar to that on Kirkwood (and
possibly orion5x as well). Instead of duplicating the code, we tweak
the Kirkwood driver so it works for Dove, too.

Signed-off-by: Sascha Silbe <t-uboot@infra-silbe.de>
---
 v4->v5: Modify Kirkwood driver rather than duplicating it.

 The patch description might do with slightly more detail, but I'd
 like some feedback on the approach first. The patch itself should be
 pretty self-explanatory.

 arch/arm/include/asm/arch-kirkwood/gpio.h          | 42 ++-------------------
 drivers/gpio/kw_gpio.c                             | 43 +++++++++++-----------
 .../asm/arch-kirkwood/gpio.h => include/kw_gpio.h  | 43 +++++++++++-----------
 3 files changed, 47 insertions(+), 81 deletions(-)

Comments

Sebastian Hesselbarth June 25, 2013, 10:39 p.m. UTC | #1 
On 06/25/2013 11:27 PM, Sascha Silbe wrote:
> The GPIO support of Dove is very similar to that on Kirkwood (and
> possibly orion5x as well). Instead of duplicating the code, we tweak
> the Kirkwood driver so it works for Dove, too.
>
> Signed-off-by: Sascha Silbe<t-uboot@infra-silbe.de>
> ---
[...]
> diff --git a/drivers/gpio/kw_gpio.c b/drivers/gpio/kw_gpio.c
> index 51a826d..d6fdb69 100644
> --- a/drivers/gpio/kw_gpio.c
> +++ b/drivers/gpio/kw_gpio.c
> @@ -1,7 +1,11 @@
>   /*
> - * arch/arm/plat-orion/gpio.c
> + * Marvell Dove and Kirkwood SoC GPIO handling
>    *
> - * Marvell Orion SoC GPIO handling.
> + * Sebastian Hesselbarth<sebastian.hesselbarth@gmail.com>
> + *
> + * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver.
> + * Removed orion_gpiochip struct and kernel level irq handling.
> + * Dieter Kiermaier dk-arm-linux@gmx.de
>    *
[...]
>   int kw_gpio_is_valid(unsigned pin, int mode)
> @@ -88,7 +88,7 @@ int kw_gpio_is_valid(unsigned pin, int mode)
>   	}
>
>   err_out:
> -		printf("%s: invalid GPIO %d\n", __func__, pin);
> +        printf("%s: invalid GPIO %d/%d\n", __func__, pin, GPIO_MAX);

nit: indent with TAB

Sebastian
Wolfgang Denk June 26, 2013, 2:09 p.m. UTC | #2 
Dear Sascha Silbe,

In message <1372195668-25496-4-git-send-email-t-uboot@infra-silbe.de> you wrote:
> The GPIO support of Dove is very similar to that on Kirkwood (and
> possibly orion5x as well). Instead of duplicating the code, we tweak
> the Kirkwood driver so it works for Dove, too.

This throws a number of checkpatch warnings and errors (line over 80
characters, code indent should use tabs where possible, please, no
spaces at the start of a line). Please fix.

> -/*
> - * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver.
> - * Removed kernel level irq handling. Took some macros from kernel to
> - * allow build.
> - *
> - * Dieter Kiermaier dk-arm-linux@gmx.de
> - */

Please never, never ever remove existing Copyright information or
information about the origin of the code.  This is a strong NAK.

> index cd1bc00..7e35833 100644
> --- a/arch/arm/include/asm/arch-kirkwood/gpio.h
> +++ b/include/kw_gpio.h
> @@ -1,5 +1,7 @@
>  /*
> - * arch/asm-arm/mach-kirkwood/include/mach/gpio.h
> + * Marvell Dove and Kirkwood SoCs common gpio
> + *
> + * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Which purpose has the mentioning of this name here?  If this is
supposed to be a copyright claim, it should say so.  

> - * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver.
> - * Removed kernel level irq handling. Took some macros from kernel to
> - * allow build.
> - *
> - * Dieter Kiermaier dk-arm-linux@gmx.de

Again, this gets dropped.  All these changes look pretty much fishy to
me.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-kirkwood/gpio.h b/arch/arm/include/asm/arch-kirkwood/gpio.h
index cd1bc00..8c8f239 100644
--- a/arch/arm/include/asm/arch-kirkwood/gpio.h
+++ b/arch/arm/include/asm/arch-kirkwood/gpio.h
@@ -20,46 +20,12 @@ 
  * MA 02110-1301 USA
  */
 
-/*
- * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver.
- * Removed kernel level irq handling. Took some macros from kernel to
- * allow build.
- *
- * Dieter Kiermaier dk-arm-linux@gmx.de
- */
+#ifndef __ARCH_KIRKWOOD_GPIO_H
+#define __ARCH_KIRKWOOD_GPIO_H
 
-#ifndef __KIRKWOOD_GPIO_H
-#define __KIRKWOOD_GPIO_H
-
-/* got from kernel include/linux/bitops.h */
-#define BITS_PER_BYTE 8
-#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
+#include <kw_gpio.h>
 
 #define GPIO_MAX		50
-#define GPIO_OFF(pin)		(((pin) >> 5) ? 0x0040 : 0x0000)
-#define GPIO_OUT(pin)		(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x00)
-#define GPIO_IO_CONF(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x04)
-#define GPIO_BLINK_EN(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x08)
-#define GPIO_IN_POL(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x0c)
-#define GPIO_DATA_IN(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x10)
-#define GPIO_EDGE_CAUSE(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x14)
-#define GPIO_EDGE_MASK(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x18)
-#define GPIO_LEVEL_MASK(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x1c)
-
-/*
- * Kirkwood-specific GPIO API
- */
-
-void kw_gpio_set_valid(unsigned pin, int mode);
-int kw_gpio_is_valid(unsigned pin, int mode);
-int kw_gpio_direction_input(unsigned pin);
-int kw_gpio_direction_output(unsigned pin, int value);
-int kw_gpio_get_value(unsigned pin);
-void kw_gpio_set_value(unsigned pin, int value);
-void kw_gpio_set_blink(unsigned pin, int blink);
-void kw_gpio_set_unused(unsigned pin);
-
-#define GPIO_INPUT_OK		(1 << 0)
-#define GPIO_OUTPUT_OK		(1 << 1)
+#define GPIO_BASE(pin)		(((pin) >> 5) ? (KW_GPIO0_BASE + 0x0040) : KW_GPIO0_BASE)
 
 #endif
diff --git a/drivers/gpio/kw_gpio.c b/drivers/gpio/kw_gpio.c
index 51a826d..d6fdb69 100644
--- a/drivers/gpio/kw_gpio.c
+++ b/drivers/gpio/kw_gpio.c
@@ -1,7 +1,11 @@ 
 /*
- * arch/arm/plat-orion/gpio.c
+ * Marvell Dove and Kirkwood SoC GPIO handling
  *
- * Marvell Orion SoC GPIO handling.
+ * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ *
+ * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver.
+ * Removed orion_gpiochip struct and kernel level irq handling.
+ * Dieter Kiermaier dk-arm-linux@gmx.de
  *
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -22,58 +26,54 @@ 
  * MA 02110-1301 USA
  */
 
-/*
- * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver.
- * Removed orion_gpiochip struct and kernel level irq handling.
- *
- * Dieter Kiermaier dk-arm-linux@gmx.de
- */
-
 #include <common.h>
 #include <asm/bitops.h>
 #include <asm/io.h>
-#include <asm/arch/kirkwood.h>
 #include <asm/arch/gpio.h>
+#include <kw_gpio.h>
 
 static unsigned long gpio_valid_input[BITS_TO_LONGS(GPIO_MAX)];
 static unsigned long gpio_valid_output[BITS_TO_LONGS(GPIO_MAX)];
 
 void __set_direction(unsigned pin, int input)
 {
+	u32 base = GPIO_BASE(pin);
 	u32 u;
 
-	u = readl(GPIO_IO_CONF(pin));
+	u = readl(GPIO_IO_CONF(base));
 	if (input)
 		u |= 1 << (pin & 31);
 	else
 		u &= ~(1 << (pin & 31));
-	writel(u, GPIO_IO_CONF(pin));
+	writel(u, GPIO_IO_CONF(base));
 
-	u = readl(GPIO_IO_CONF(pin));
+	u = readl(GPIO_IO_CONF(base));
 }
 
 void __set_level(unsigned pin, int high)
 {
+	u32 base = GPIO_BASE(pin);
 	u32 u;
 
-	u = readl(GPIO_OUT(pin));
+	u = readl(GPIO_OUT(base));
 	if (high)
 		u |= 1 << (pin & 31);
 	else
 		u &= ~(1 << (pin & 31));
-	writel(u, GPIO_OUT(pin));
+	writel(u, GPIO_OUT(base));
 }
 
 void __set_blinking(unsigned pin, int blink)
 {
+	u32 base = GPIO_BASE(pin);
 	u32 u;
 
-	u = readl(GPIO_BLINK_EN(pin));
+	u = readl(GPIO_BLINK_EN(base));
 	if (blink)
 		u |= 1 << (pin & 31);
 	else
 		u &= ~(1 << (pin & 31));
-	writel(u, GPIO_BLINK_EN(pin));
+	writel(u, GPIO_BLINK_EN(base));
 }
 
 int kw_gpio_is_valid(unsigned pin, int mode)
@@ -88,7 +88,7 @@  int kw_gpio_is_valid(unsigned pin, int mode)
 	}
 
 err_out:
-		printf("%s: invalid GPIO %d\n", __func__, pin);
+        printf("%s: invalid GPIO %d/%d\n", __func__, pin, GPIO_MAX);
 	return 1;
 }
 
@@ -140,12 +140,13 @@  int kw_gpio_direction_output(unsigned pin, int value)
 
 int kw_gpio_get_value(unsigned pin)
 {
+	u32 base = GPIO_BASE(pin);
 	int val;
 
-	if (readl(GPIO_IO_CONF(pin)) & (1 << (pin & 31)))
-		val = readl(GPIO_DATA_IN(pin)) ^ readl(GPIO_IN_POL(pin));
+	if (readl(GPIO_IO_CONF(base)) & (1 << (pin & 31)))
+		val = readl(GPIO_DATA_IN(base)) ^ readl(GPIO_IN_POL(base));
 	else
-		val = readl(GPIO_OUT(pin));
+		val = readl(GPIO_OUT(base));
 
 	return (val >> (pin & 31)) & 1;
 }
diff --git a/arch/arm/include/asm/arch-kirkwood/gpio.h b/include/kw_gpio.h
similarity index 60%
copy from arch/arm/include/asm/arch-kirkwood/gpio.h
copy to include/kw_gpio.h
index cd1bc00..7e35833 100644
--- a/arch/arm/include/asm/arch-kirkwood/gpio.h
+++ b/include/kw_gpio.h
@@ -1,5 +1,7 @@ 
 /*
- * arch/asm-arm/mach-kirkwood/include/mach/gpio.h
+ * Marvell Dove and Kirkwood SoCs common gpio
+ *
+ * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
  *
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -20,34 +22,34 @@ 
  * MA 02110-1301 USA
  */
 
+#ifndef __KW_GPIO_H
+#define __KW_GPIO_H
+
 /*
- * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver.
- * Removed kernel level irq handling. Took some macros from kernel to
- * allow build.
- *
- * Dieter Kiermaier dk-arm-linux@gmx.de
+ * SoC-specific gpio.h defines
+ * GPIO_MAX and GPIO_BASE(pin) macro
  */
 
-#ifndef __KIRKWOOD_GPIO_H
-#define __KIRKWOOD_GPIO_H
+#define GPIO_INPUT_OK		(1 << 0)
+#define GPIO_OUTPUT_OK		(1 << 1)
+#define GPIO_LOW		0
+#define GPIO_HIGH		1
 
 /* got from kernel include/linux/bitops.h */
 #define BITS_PER_BYTE 8
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 
-#define GPIO_MAX		50
-#define GPIO_OFF(pin)		(((pin) >> 5) ? 0x0040 : 0x0000)
-#define GPIO_OUT(pin)		(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x00)
-#define GPIO_IO_CONF(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x04)
-#define GPIO_BLINK_EN(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x08)
-#define GPIO_IN_POL(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x0c)
-#define GPIO_DATA_IN(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x10)
-#define GPIO_EDGE_CAUSE(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x14)
-#define GPIO_EDGE_MASK(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x18)
-#define GPIO_LEVEL_MASK(pin)	(KW_GPIO0_BASE + GPIO_OFF(pin) + 0x1c)
+#define GPIO_OUT(base)		((base) + 0x00)
+#define GPIO_IO_CONF(base)	((base) + 0x04)
+#define GPIO_BLINK_EN(base)	((base) + 0x08)
+#define GPIO_IN_POL(base)	((base) + 0x0c)
+#define GPIO_DATA_IN(base)	((base) + 0x10)
+#define GPIO_EDGE_CAUSE(base)	((base) + 0x14)
+#define GPIO_EDGE_MASK(base)	((base) + 0x18)
+#define GPIO_LEVEL_MASK(base)	((base) + 0x1c)
 
 /*
- * Kirkwood-specific GPIO API
+ * Dove/Kirkwood-specific GPIO API
  */
 
 void kw_gpio_set_valid(unsigned pin, int mode);
@@ -59,7 +61,4 @@  void kw_gpio_set_value(unsigned pin, int value);
 void kw_gpio_set_blink(unsigned pin, int blink);
 void kw_gpio_set_unused(unsigned pin);
 
-#define GPIO_INPUT_OK		(1 << 0)
-#define GPIO_OUTPUT_OK		(1 << 1)
-
 #endif