[PATCH 00/12] Blackfin arch GPIO updating

[PATCH 00/12] Blackfin arch GPIO updating

[Bryan Wu]

As David mentioned, I send out these series patch to LKML for review.
These patches are related Blackfin arch GPIO updating, not big change at all.
I think it is OK for git-pull in -RC2 or later.

Thanks
- Bryan Wu

[PATCH 01/12] Blackfin arch: add peripheral resource allocation support

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/kernel/bfin_gpio.c                  |  272 ++++++++++++++++++---
 include/asm-blackfin/mach-bf533/bfin_serial_5xx.h |   11 +-
 include/asm-blackfin/mach-bf537/bfin_serial_5xx.h |   23 +-
 include/asm-blackfin/mach-bf537/portmux.h         |    2 +-
 include/asm-blackfin/mach-bf561/bfin_serial_5xx.h |   11 +-
 5 files changed, 274 insertions(+), 45 deletions(-)

diff --git a/arch/blackfin/kernel/bfin_gpio.c b/arch/blackfin/kernel/bfin_gpio.c
index bafcfa5..9f30948 100644
--- a/arch/blackfin/kernel/bfin_gpio.c
+++ b/arch/blackfin/kernel/bfin_gpio.c
@@ -84,6 +84,7 @@
 #include <linux/err.h>
 #include <asm/blackfin.h>
 #include <asm/gpio.h>
+#include <asm/portmux.h>
 #include <linux/irq.h>
 
 #ifdef BF533_FAMILY
@@ -115,7 +116,11 @@ static struct gpio_port_t *gpio_bankb[gpio_bank(MAX_BLACKFIN_GPIOS)] = {
 };
 #endif
 
-static unsigned short reserved_map[gpio_bank(MAX_BLACKFIN_GPIOS)];
+static unsigned short reserved_gpio_map[gpio_bank(MAX_BLACKFIN_GPIOS)];
+static unsigned short reserved_peri_map[gpio_bank(MAX_BLACKFIN_GPIOS + 16)];
+char *str_ident = NULL;
+
+#define RESOURCE_LABEL_SIZE 16
 
 #ifdef CONFIG_PM
 static unsigned short wakeup_map[gpio_bank(MAX_BLACKFIN_GPIOS)];
@@ -143,13 +148,39 @@ inline int check_gpio(unsigned short gpio)
 	return 0;
 }
 
+static void set_label(unsigned short ident, const char *label)
+{
+
+	if (label && str_ident) {
+		strncpy(str_ident + ident * RESOURCE_LABEL_SIZE, label,
+			 RESOURCE_LABEL_SIZE);
+		str_ident[ident * RESOURCE_LABEL_SIZE +
+			 RESOURCE_LABEL_SIZE - 1] = 0;
+	}
+}
+
+static char *get_label(unsigned short ident)
+{
+	if (!str_ident)
+		return "UNKNOWN";
+
+	return (str_ident[ident * RESOURCE_LABEL_SIZE] ?
+		(str_ident + ident * RESOURCE_LABEL_SIZE) : "UNKNOWN");
+}
+
+static int cmp_label(unsigned short ident, const char *label)
+{
+	if (label && str_ident)
+		return strncmp(str_ident + ident * RESOURCE_LABEL_SIZE,
+				 label, strlen(label));
+	else
+		return -EINVAL;
+}
+
 #ifdef BF537_FAMILY
 static void port_setup(unsigned short gpio, unsigned short usage)
 {
 	if (usage == GPIO_USAGE) {
-		if (*port_fer[gpio_bank(gpio)] & gpio_bit(gpio))
-			printk(KERN_WARNING "bfin-gpio: Possible Conflict with Peripheral "
-			       "usage and GPIO %d detected!\n", gpio);
 		*port_fer[gpio_bank(gpio)] &= ~gpio_bit(gpio);
 	} else
 		*port_fer[gpio_bank(gpio)] |= gpio_bit(gpio);
@@ -159,6 +190,56 @@ static void port_setup(unsigned short gpio, unsigned short usage)
 # define port_setup(...)  do { } while (0)
 #endif
 
+#ifdef BF537_FAMILY
+
+#define PMUX_LUT_RES		0
+#define PMUX_LUT_OFFSET		1
+#define PMUX_LUT_ENTRIES	41
+#define PMUX_LUT_SIZE		2
+
+static unsigned short port_mux_lut[PMUX_LUT_ENTRIES][PMUX_LUT_SIZE] = {
+	{P_PPI0_D13, 11}, {P_PPI0_D14, 11}, {P_PPI0_D15, 11},
+	{P_SPORT1_TFS, 11}, {P_SPORT1_TSCLK, 11}, {P_SPORT1_DTPRI, 11},
+	{P_PPI0_D10, 10}, {P_PPI0_D11, 10}, {P_PPI0_D12, 10},
+	{P_SPORT1_RSCLK, 10}, {P_SPORT1_RFS, 10}, {P_SPORT1_DRPRI, 10},
+	{P_PPI0_D8, 9}, {P_PPI0_D9, 9}, {P_SPORT1_DRSEC, 9},
+	{P_SPORT1_DTSEC, 9}, {P_TMR2, 8}, {P_PPI0_FS3, 8}, {P_TMR3, 7},
+	{P_SPI0_SSEL4, 7}, {P_TMR4, 6}, {P_SPI0_SSEL5, 6}, {P_TMR5, 5},
+	{P_SPI0_SSEL6, 5}, {P_UART1_RX, 4}, {P_UART1_TX, 4}, {P_TMR6, 4},
+	{P_TMR7, 4}, {P_UART0_RX, 3}, {P_UART0_TX, 3}, {P_DMAR0, 3},
+	{P_DMAR1, 3}, {P_SPORT0_DTSEC, 1}, {P_SPORT0_DRSEC, 1},
+	{P_CAN0_RX, 1}, {P_CAN0_TX, 1}, {P_SPI0_SSEL7, 1},
+	{P_SPORT0_TFS, 0}, {P_SPORT0_DTPRI, 0}, {P_SPI0_SSEL2, 0},
+	{P_SPI0_SSEL3, 0}
+};
+
+static void portmux_setup(unsigned short per, unsigned short function)
+{
+	u16 y, muxreg, offset;
+
+	for (y = 0; y < PMUX_LUT_ENTRIES; y++) {
+		if (port_mux_lut[y][PMUX_LUT_RES] == per) {
+
+			/* SET PORTMUX REG */
+
+			offset = port_mux_lut[y][PMUX_LUT_OFFSET];
+			muxreg = bfin_read_PORT_MUX();
+
+			if (offset != 1) {
+				muxreg &= ~(1 << offset);
+			} else {
+				muxreg &= ~(3 << 1);
+			}
+
+			muxreg |= (function << offset);
+			bfin_write_PORT_MUX(muxreg);
+		}
+	}
+}
+
+#else
+# define portmux_setup(...)  do { } while (0)
+#endif
 
 static void default_gpio(unsigned short gpio)
 {
@@ -179,22 +260,15 @@ static void default_gpio(unsigned short gpio)
 
 static int __init bfin_gpio_init(void)
 {
-	int i;
-
-	printk(KERN_INFO "Blackfin GPIO Controller\n");
 
-	for (i = 0; i < MAX_BLACKFIN_GPIOS; i += GPIO_BANKSIZE)
-		reserved_map[gpio_bank(i)] = 0;
+	str_ident = kzalloc(RESOURCE_LABEL_SIZE * 256, GFP_KERNEL);
+	if (!str_ident)
+		return -ENOMEM;
 
-#if defined(BF537_FAMILY) && (defined(CONFIG_BFIN_MAC) || defined(CONFIG_BFIN_MAC_MODULE))
-# if defined(CONFIG_BFIN_MAC_RMII)
-	reserved_map[gpio_bank(PORT_H)] = 0xC373;
-# else
-	reserved_map[gpio_bank(PORT_H)] = 0xFFFF;
-# endif
-#endif
+	printk(KERN_INFO "Blackfin GPIO Controller\n");
 
 	return 0;
+
 }
 
 arch_initcall(bfin_gpio_init);
@@ -223,7 +297,7 @@ arch_initcall(bfin_gpio_init);
 void set_gpio_ ## name(unsigned short gpio, unsigned short arg) \
 { \
 	unsigned long flags; \
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio))); \
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio))); \
 	local_irq_save(flags); \
 	if (arg) \
 		gpio_bankb[gpio_bank(gpio)]->name |= gpio_bit(gpio); \
@@ -243,7 +317,7 @@ SET_GPIO(both)
 #define SET_GPIO_SC(name) \
 void set_gpio_ ## name(unsigned short gpio, unsigned short arg) \
 { \
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio))); \
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio))); \
 	if (arg) \
 		gpio_bankb[gpio_bank(gpio)]->name ## _set = gpio_bit(gpio); \
 	else \
@@ -258,7 +332,7 @@ SET_GPIO_SC(maskb)
 void set_gpio_data(unsigned short gpio, unsigned short arg)
 {
 	unsigned long flags;
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio)));
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio)));
 	local_irq_save(flags);
 	if (arg)
 		gpio_bankb[gpio_bank(gpio)]->data_set = gpio_bit(gpio);
@@ -277,7 +351,7 @@ SET_GPIO_SC(data)
 void set_gpio_toggle(unsigned short gpio)
 {
 	unsigned long flags;
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio)));
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio)));
 	local_irq_save(flags);
 	gpio_bankb[gpio_bank(gpio)]->toggle = gpio_bit(gpio);
 	bfin_read_CHIPID();
@@ -286,7 +360,7 @@ void set_gpio_toggle(unsigned short gpio)
 #else
 void set_gpio_toggle(unsigned short gpio)
 {
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio)));
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio)));
 	gpio_bankb[gpio_bank(gpio)]->toggle = gpio_bit(gpio);
 }
 #endif
@@ -350,7 +424,7 @@ unsigned short get_gpio_data(unsigned short gpio)
 {
 	unsigned long flags;
 	unsigned short ret;
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio)));
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio)));
 	local_irq_save(flags);
 	ret = 0x01 & (gpio_bankb[gpio_bank(gpio)]->data >> gpio_sub_n(gpio));
 	bfin_read_CHIPID();
@@ -494,13 +568,14 @@ u32 gpio_pm_setup(void)
 			gpio_bank_saved[bank].dir   = gpio_bankb[bank]->dir;
 			gpio_bank_saved[bank].edge  = gpio_bankb[bank]->edge;
 			gpio_bank_saved[bank].both  = gpio_bankb[bank]->both;
-			gpio_bank_saved[bank].reserved = reserved_map[bank];
+			gpio_bank_saved[bank].reserved =
+						reserved_gpio_map[bank];
 
 			gpio = i;
 
 			while (mask) {
 				if (mask & 1) {
-					reserved_map[gpio_bank(gpio)] |=
+					reserved_gpio_map[gpio_bank(gpio)] |=
 							gpio_bit(gpio);
 					bfin_gpio_wakeup_type(gpio,
 						wakeup_flags_map[gpio]);
@@ -540,7 +615,8 @@ void gpio_pm_restore(void)
 			gpio_bankb[bank]->edge  = gpio_bank_saved[bank].edge;
 			gpio_bankb[bank]->both  = gpio_bank_saved[bank].both;
 
-			reserved_map[bank] = gpio_bank_saved[bank].reserved;
+			reserved_gpio_map[bank] =
+					gpio_bank_saved[bank].reserved;
 
 		}
 
@@ -550,6 +626,140 @@ void gpio_pm_restore(void)
 
 #endif
 
+
+
+
+int peripheral_request(unsigned short per, const char *label)
+{
+	unsigned long flags;
+	unsigned short ident = P_IDENT(per);
+
+	/*
+	 * Don't cares are pins with only one dedicated function
+	 */
+
+	if (per & P_DONTCARE)
+		return 0;
+
+	if (!(per & P_DEFINED))
+		return -ENODEV;
+
+	if (check_gpio(ident) < 0)
+		return -EINVAL;
+
+	local_irq_save(flags);
+
+	if (unlikely(reserved_gpio_map[gpio_bank(ident)] & gpio_bit(ident))) {
+		printk(KERN_ERR
+		       "%s: Peripheral %d is already reserved as GPIO by %s !\n",
+		       __FUNCTION__, ident, get_label(ident));
+		dump_stack();
+		local_irq_restore(flags);
+		return -EBUSY;
+	}
+
+	if (unlikely(reserved_peri_map[gpio_bank(ident)] & gpio_bit(ident))) {
+
+	/*
+	 * Pin functions like AMC address strobes my
+	 * be requested and used by several drivers
+	 */
+
+	if (!(per & P_MAYSHARE)) {
+
+	/*
+	 * Allow that the identical pin function can
+	 * be requested from the same driver twice
+	 */
+
+		if (cmp_label(ident, label) == 0)
+			goto anyway;
+
+			printk(KERN_ERR
+			       "%s: Peripheral %d function %d is already"
+			       "reserved by %s !\n",
+			       __FUNCTION__, ident, P_FUNCT2MUX(per),
+				get_label(ident));
+			dump_stack();
+			local_irq_restore(flags);
+			return -EBUSY;
+		}
+
+	}
+
+anyway:
+
+
+	portmux_setup(per, P_FUNCT2MUX(per));
+
+	port_setup(ident, PERIPHERAL_USAGE);
+
+	reserved_peri_map[gpio_bank(ident)] |= gpio_bit(ident);
+	local_irq_restore(flags);
+	set_label(ident, label);
+
+	return 0;
+}
+EXPORT_SYMBOL(peripheral_request);
+
+int peripheral_request_list(unsigned short per[], const char *label)
+{
+	u16 cnt;
+	int ret;
+
+	for (cnt = 0; per[cnt] != 0; cnt++) {
+		ret = peripheral_request(per[cnt], label);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(peripheral_request_list);
+
+void peripheral_free(unsigned short per)
+{
+	unsigned long flags;
+	unsigned short ident = P_IDENT(per);
+
+	if (per & P_DONTCARE)
+		return;
+
+	if (!(per & P_DEFINED))
+		return;
+
+	if (check_gpio(ident) < 0)
+		return;
+
+	local_irq_save(flags);
+
+	if (unlikely(!(reserved_peri_map[gpio_bank(ident)]
+			 & gpio_bit(ident)))) {
+		local_irq_restore(flags);
+		return;
+	}
+
+	if (!(per & P_MAYSHARE)) {
+		port_setup(ident, GPIO_USAGE);
+	}
+
+	reserved_peri_map[gpio_bank(ident)] &= ~gpio_bit(ident);
+
+	local_irq_restore(flags);
+}
+EXPORT_SYMBOL(peripheral_free);
+
+void peripheral_free_list(unsigned short per[])
+{
+	u16 cnt;
+
+	for (cnt = 0; per[cnt] != 0; cnt++) {
+		peripheral_free(per[cnt]);
+	}
+
+}
+EXPORT_SYMBOL(peripheral_free_list);
+
 /***********************************************************
 *
 * FUNCTIONS: Blackfin GPIO Driver
@@ -574,13 +784,13 @@ int gpio_request(unsigned short gpio, const char *label)
 
 	local_irq_save(flags);
 
-	if (unlikely(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio))) {
+	if (unlikely(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio))) {
 		printk(KERN_ERR "bfin-gpio: GPIO %d is already reserved!\n", gpio);
 		dump_stack();
 		local_irq_restore(flags);
 		return -EBUSY;
 	}
-	reserved_map[gpio_bank(gpio)] |= gpio_bit(gpio);
+	reserved_gpio_map[gpio_bank(gpio)] |= gpio_bit(gpio);
 
 	local_irq_restore(flags);
 
@@ -599,7 +809,7 @@ void gpio_free(unsigned short gpio)
 
 	local_irq_save(flags);
 
-	if (unlikely(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio)))) {
+	if (unlikely(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio)))) {
 		printk(KERN_ERR "bfin-gpio: GPIO %d wasn't reserved!\n", gpio);
 		dump_stack();
 		local_irq_restore(flags);
@@ -608,7 +818,7 @@ void gpio_free(unsigned short gpio)
 
 	default_gpio(gpio);
 
-	reserved_map[gpio_bank(gpio)] &= ~gpio_bit(gpio);
+	reserved_gpio_map[gpio_bank(gpio)] &= ~gpio_bit(gpio);
 
 	local_irq_restore(flags);
 }
@@ -618,7 +828,7 @@ void gpio_direction_input(unsigned short gpio)
 {
 	unsigned long flags;
 
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio)));
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio)));
 
 	local_irq_save(flags);
 	gpio_bankb[gpio_bank(gpio)]->dir &= ~gpio_bit(gpio);
@@ -631,7 +841,7 @@ void gpio_direction_output(unsigned short gpio)
 {
 	unsigned long flags;
 
-	BUG_ON(!(reserved_map[gpio_bank(gpio)] & gpio_bit(gpio)));
+	BUG_ON(!(reserved_gpio_map[gpio_bank(gpio)] & gpio_bit(gpio)));
 
 	local_irq_save(flags);
 	gpio_bankb[gpio_bank(gpio)]->inen &= ~gpio_bit(gpio);
diff --git a/include/asm-blackfin/mach-bf533/bfin_serial_5xx.h b/include/asm-blackfin/mach-bf533/bfin_serial_5xx.h
index e043caf..69b9f8e 100644
--- a/include/asm-blackfin/mach-bf533/bfin_serial_5xx.h
+++ b/include/asm-blackfin/mach-bf533/bfin_serial_5xx.h
@@ -1,5 +1,6 @@
 #include <linux/serial.h>
 #include <asm/dma.h>
+#include <asm/portmux.h>
 
 #define NR_PORTS                1
 
@@ -92,18 +93,24 @@ struct bfin_serial_res bfin_serial_resource[] = {
 	}
 };
 
+#define DRIVER_NAME "bfin-uart"
 
 int nr_ports = NR_PORTS;
 static void bfin_serial_hw_init(struct bfin_serial_port *uart)
 {
 
+#ifdef CONFIG_SERIAL_BFIN_UART0
+	peripheral_request(P_UART0_TX, DRIVER_NAME);
+	peripheral_request(P_UART0_RX, DRIVER_NAME);
+#endif
+
 #ifdef CONFIG_SERIAL_BFIN_CTSRTS
 	if (uart->cts_pin >= 0) {
-		gpio_request(uart->cts_pin, NULL);
+		gpio_request(uart->cts_pin, DRIVER_NAME);
 		gpio_direction_input(uart->cts_pin);
 	}
 	if (uart->rts_pin >= 0) {
-		gpio_request(uart->rts_pin, NULL);
+		gpio_request(uart->rts_pin, DRIVER_NAME);
 		gpio_direction_input(uart->rts_pin);
 	}
 #endif
diff --git a/include/asm-blackfin/mach-bf537/bfin_serial_5xx.h b/include/asm-blackfin/mach-bf537/bfin_serial_5xx.h
index 8f5d9c4..6fb328f 100644
--- a/include/asm-blackfin/mach-bf537/bfin_serial_5xx.h
+++ b/include/asm-blackfin/mach-bf537/bfin_serial_5xx.h
@@ -1,5 +1,6 @@
 #include <linux/serial.h>
 #include <asm/dma.h>
+#include <asm/portmux.h>
 
 #define NR_PORTS		2
 
@@ -122,25 +123,29 @@ struct bfin_serial_res bfin_serial_resource[] = {
 
 int nr_ports = ARRAY_SIZE(bfin_serial_resource);
 
+#define DRIVER_NAME "bfin-uart"
+
 static void bfin_serial_hw_init(struct bfin_serial_port *uart)
 {
-	unsigned short val;
-	val = bfin_read16(BFIN_PORT_MUX);
-	val &= ~(PFDE | PFTE);
-	bfin_write16(BFIN_PORT_MUX, val);
 
-	val = bfin_read16(PORTF_FER);
-	val |= 0xF;
-	bfin_write16(PORTF_FER, val);
+#ifdef CONFIG_SERIAL_BFIN_UART0
+	peripheral_request(P_UART0_TX, DRIVER_NAME);
+	peripheral_request(P_UART0_RX, DRIVER_NAME);
+#endif
+
+#ifdef CONFIG_SERIAL_BFIN_UART1
+	peripheral_request(P_UART1_TX, DRIVER_NAME);
+	peripheral_request(P_UART1_RX, DRIVER_NAME);
+#endif
 
 #ifdef CONFIG_SERIAL_BFIN_CTSRTS
 	if (uart->cts_pin >= 0) {
-		gpio_request(uart->cts_pin, NULL);
+		gpio_request(uart->cts_pin, DRIVER_NAME);
 		gpio_direction_input(uart->cts_pin);
 	}
 
 	if (uart->rts_pin >= 0) {
-		gpio_request(uart->rts_pin, NULL);
+		gpio_request(uart->rts_pin, DRIVER_NAME);
 		gpio_direction_output(uart->rts_pin);
 	}
 #endif
diff --git a/include/asm-blackfin/mach-bf537/portmux.h b/include/asm-blackfin/mach-bf537/portmux.h
index 23e13c5..7daa247 100644
--- a/include/asm-blackfin/mach-bf537/portmux.h
+++ b/include/asm-blackfin/mach-bf537/portmux.h
@@ -106,4 +106,4 @@
 #define P_SPI0_SSEL2	(P_DEFINED | P_IDENT(PORT_PJ11) | P_FUNCT(1))
 #define P_SPI0_SSEL7	(P_DEFINED | P_IDENT(PORT_PJ5) | P_FUNCT(2))
 
-#endif /* _MACH_PORTMUX_H_ */
+#endif				/* _MACH_PORTMUX_H_ */
diff --git a/include/asm-blackfin/mach-bf561/bfin_serial_5xx.h b/include/asm-blackfin/mach-bf561/bfin_serial_5xx.h
index e043caf..69b9f8e 100644
--- a/include/asm-blackfin/mach-bf561/bfin_serial_5xx.h
+++ b/include/asm-blackfin/mach-bf561/bfin_serial_5xx.h
@@ -1,5 +1,6 @@
 #include <linux/serial.h>
 #include <asm/dma.h>
+#include <asm/portmux.h>
 
 #define NR_PORTS                1
 
@@ -92,18 +93,24 @@ struct bfin_serial_res bfin_serial_resource[] = {
 	}
 };
 
+#define DRIVER_NAME "bfin-uart"
 
 int nr_ports = NR_PORTS;
 static void bfin_serial_hw_init(struct bfin_serial_port *uart)
 {
 
+#ifdef CONFIG_SERIAL_BFIN_UART0
+	peripheral_request(P_UART0_TX, DRIVER_NAME);
+	peripheral_request(P_UART0_RX, DRIVER_NAME);
+#endif
+
 #ifdef CONFIG_SERIAL_BFIN_CTSRTS
 	if (uart->cts_pin >= 0) {
-		gpio_request(uart->cts_pin, NULL);
+		gpio_request(uart->cts_pin, DRIVER_NAME);
 		gpio_direction_input(uart->cts_pin);
 	}
 	if (uart->rts_pin >= 0) {
-		gpio_request(uart->rts_pin, NULL);
+		gpio_request(uart->rts_pin, DRIVER_NAME);
 		gpio_direction_input(uart->rts_pin);
 	}
 #endif
-- 
1.5.2

[PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/mach-common/ints-priority-dc.c |    4 ++--
 arch/blackfin/mach-common/ints-priority-sc.c |    8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/blackfin/mach-common/ints-priority-dc.c b/arch/blackfin/mach-common/ints-priority-dc.c
index 660f881..d5d9e57 100644
--- a/arch/blackfin/mach-common/ints-priority-dc.c
+++ b/arch/blackfin/mach-common/ints-priority-dc.c
@@ -221,7 +221,7 @@ static unsigned int bf561_gpio_irq_startup(unsigned int irq)
 
 	if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
 
-		ret = gpio_request(gpionr, NULL);
+		ret = gpio_request(gpionr, "IRQ");
 		if (ret)
 			return ret;
 
@@ -261,7 +261,7 @@ static int bf561_gpio_irq_type(unsigned int irq, unsigned int type)
 
 		if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
 
-			ret = gpio_request(gpionr, NULL);
+			ret = gpio_request(gpionr, "IRQ");
 			if (ret)
 				return ret;
 
diff --git a/arch/blackfin/mach-common/ints-priority-sc.c b/arch/blackfin/mach-common/ints-priority-sc.c
index 4708023..505b948 100644
--- a/arch/blackfin/mach-common/ints-priority-sc.c
+++ b/arch/blackfin/mach-common/ints-priority-sc.c
@@ -343,7 +343,7 @@ static unsigned int bfin_gpio_irq_startup(unsigned int irq)
 	u16 gpionr = irq - IRQ_PF0;
 
 	if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
-		ret = gpio_request(gpionr, NULL);
+		ret = gpio_request(gpionr, "IRQ");
 		if (ret)
 			return ret;
 	}
@@ -377,7 +377,7 @@ static int bfin_gpio_irq_type(unsigned int irq, unsigned int type)
 	if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
 		    IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) {
 		if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
-			ret = gpio_request(gpionr, NULL);
+			ret = gpio_request(gpionr, "IRQ");
 			if (ret)
 				return ret;
 		}
@@ -587,7 +587,7 @@ static unsigned int bfin_gpio_irq_startup(unsigned int irq)
 	}
 
 	if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
-		ret = gpio_request(gpionr, NULL);
+		ret = gpio_request(gpionr, "IRQ");
 		if (ret)
 			return ret;
 	}
@@ -627,7 +627,7 @@ static int bfin_gpio_irq_type(unsigned int irq, unsigned int type)
 	if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
 		    IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) {
 		if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
-			ret = gpio_request(gpionr, NULL);
+			ret = gpio_request(gpionr, "IRQ");
 			if (ret)
 				return ret;
 		}
-- 
1.5.2

[PATCH 03/12] Blackfin arch: fix PORT_J BUG for BF537/6 EMAC driver

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/kernel/bfin_gpio.c          |   19 +++++++++------
 include/asm-blackfin/mach-bf537/portmux.h |   35 ++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/arch/blackfin/kernel/bfin_gpio.c b/arch/blackfin/kernel/bfin_gpio.c
index 9f30948..5d488ef 100644
--- a/arch/blackfin/kernel/bfin_gpio.c
+++ b/arch/blackfin/kernel/bfin_gpio.c
@@ -180,11 +180,13 @@ static int cmp_label(unsigned short ident, const char *label)
 #ifdef BF537_FAMILY
 static void port_setup(unsigned short gpio, unsigned short usage)
 {
-	if (usage == GPIO_USAGE) {
-		*port_fer[gpio_bank(gpio)] &= ~gpio_bit(gpio);
-	} else
-		*port_fer[gpio_bank(gpio)] |= gpio_bit(gpio);
-	SSYNC();
+	if (!check_gpio(gpio)) {
+		if (usage == GPIO_USAGE) {
+			*port_fer[gpio_bank(gpio)] &= ~gpio_bit(gpio);
+		} else
+			*port_fer[gpio_bank(gpio)] |= gpio_bit(gpio);
+		SSYNC();
+	}
 }
 #else
 # define port_setup(...)  do { } while (0)
@@ -644,11 +646,10 @@ int peripheral_request(unsigned short per, const char *label)
 	if (!(per & P_DEFINED))
 		return -ENODEV;
 
-	if (check_gpio(ident) < 0)
-		return -EINVAL;
-
 	local_irq_save(flags);
 
+	if (!check_gpio(ident)) {
+
 	if (unlikely(reserved_gpio_map[gpio_bank(ident)] & gpio_bit(ident))) {
 		printk(KERN_ERR
 		       "%s: Peripheral %d is already reserved as GPIO by %s !\n",
@@ -658,6 +659,8 @@ int peripheral_request(unsigned short per, const char *label)
 		return -EBUSY;
 	}
 
+	}
+
 	if (unlikely(reserved_peri_map[gpio_bank(ident)] & gpio_bit(ident))) {
 
 	/*
diff --git a/include/asm-blackfin/mach-bf537/portmux.h b/include/asm-blackfin/mach-bf537/portmux.h
index 7daa247..ae6c53b 100644
--- a/include/asm-blackfin/mach-bf537/portmux.h
+++ b/include/asm-blackfin/mach-bf537/portmux.h
@@ -106,4 +106,37 @@
 #define P_SPI0_SSEL2	(P_DEFINED | P_IDENT(PORT_PJ11) | P_FUNCT(1))
 #define P_SPI0_SSEL7	(P_DEFINED | P_IDENT(PORT_PJ5) | P_FUNCT(2))
 
-#endif				/* _MACH_PORTMUX_H_ */
+#define P_MII0 {\
+	P_MII0_ETxD0, \
+	P_MII0_ETxD1, \
+	P_MII0_ETxD2, \
+	P_MII0_ETxD3, \
+	P_MII0_ETxEN, \
+	P_MII0_TxCLK, \
+	P_MII0_PHYINT, \
+	P_MII0_COL, \
+	P_MII0_ERxD0, \
+	P_MII0_ERxD1, \
+	P_MII0_ERxD2, \
+	P_MII0_ERxD3, \
+	P_MII0_ERxDV, \
+	P_MII0_ERxCLK, \
+	P_MII0_ERxER, \
+	P_MII0_CRS, \
+	P_MDC, \
+	P_MDIO, 0}
+
+
+#define P_RMII0 {\
+	P_MII0_ETxD0, \
+	P_MII0_ETxD1, \
+	P_MII0_ETxEN, \
+	P_MII0_ERxD0, \
+	P_MII0_ERxD1, \
+	P_MII0_ERxER, \
+	P_RMII0_REF_CLK, \
+	P_RMII0_MDINT, \
+	P_RMII0_CRS_DV, \
+	P_MDC, \
+	P_MDIO, 0}
+#endif			        	/* _MACH_PORTMUX_H_ */
-- 
1.5.2

[PATCH 04/12] Blackfin arch: Finalize the generic gpio support - add gpio_to_irq and irq_to_gpio

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 include/asm-blackfin/gpio.h           |   13 +++++++++++++
 include/asm-blackfin/mach-bf533/irq.h |    2 ++
 include/asm-blackfin/mach-bf537/irq.h |    2 ++
 include/asm-blackfin/mach-bf548/irq.h |    2 ++
 include/asm-blackfin/mach-bf561/irq.h |    2 ++
 5 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/include/asm-blackfin/gpio.h b/include/asm-blackfin/gpio.h
index 7480cfa..e714363 100644
--- a/include/asm-blackfin/gpio.h
+++ b/include/asm-blackfin/gpio.h
@@ -421,6 +421,19 @@ unsigned short gpio_get_value(unsigned short gpio);
 void gpio_direction_input(unsigned short gpio);
 void gpio_direction_output(unsigned short gpio);
 
+#include <asm-generic/gpio.h>		/* cansleep wrappers */
+#include <asm/irq.h>
+
+static inline int gpio_to_irq(unsigned gpio)
+{
+	return (gpio + GPIO_IRQ_BASE);
+}
+
+static inline int irq_to_gpio(unsigned irq)
+{
+	return (irq - GPIO_IRQ_BASE);
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ARCH_BLACKFIN_GPIO_H__ */
diff --git a/include/asm-blackfin/mach-bf533/irq.h b/include/asm-blackfin/mach-bf533/irq.h
index 9879e68..452fb82 100644
--- a/include/asm-blackfin/mach-bf533/irq.h
+++ b/include/asm-blackfin/mach-bf533/irq.h
@@ -128,6 +128,8 @@ Core        Emulation               **
 #define IRQ_PF14		47
 #define IRQ_PF15		48
 
+#define GPIO_IRQ_BASE		IRQ_PF0
+
 #ifdef CONFIG_IRQCHIP_DEMUX_GPIO
 #define	NR_IRQS		(IRQ_PF15+1)
 #else
diff --git a/include/asm-blackfin/mach-bf537/irq.h b/include/asm-blackfin/mach-bf537/irq.h
index 8af2a83..36c44bc 100644
--- a/include/asm-blackfin/mach-bf537/irq.h
+++ b/include/asm-blackfin/mach-bf537/irq.h
@@ -160,6 +160,8 @@ Core        Emulation               **
 #define IRQ_PH14        96
 #define IRQ_PH15        97
 
+#define GPIO_IRQ_BASE	IRQ_PF0
+
 #ifdef CONFIG_IRQCHIP_DEMUX_GPIO
 #define NR_IRQS     (IRQ_PH15+1)
 #else
diff --git a/include/asm-blackfin/mach-bf548/irq.h b/include/asm-blackfin/mach-bf548/irq.h
index e548d3c..21f06f7 100644
--- a/include/asm-blackfin/mach-bf548/irq.h
+++ b/include/asm-blackfin/mach-bf548/irq.h
@@ -337,6 +337,8 @@ Events         (highest priority)  EMU         0
 #define IRQ_PJ14	BFIN_PJ_IRQ(14)		/* N/A */
 #define IRQ_PJ15	BFIN_PJ_IRQ(15)		/* N/A */
 
+#define GPIO_IRQ_BASE	IRQ_PA0
+
 #ifdef CONFIG_IRQCHIP_DEMUX_GPIO
 #define NR_IRQS     (IRQ_PJ15+1)
 #else
diff --git a/include/asm-blackfin/mach-bf561/irq.h b/include/asm-blackfin/mach-bf561/irq.h
index a753ce7..1278992 100644
--- a/include/asm-blackfin/mach-bf561/irq.h
+++ b/include/asm-blackfin/mach-bf561/irq.h
@@ -289,6 +289,8 @@
 #define IRQ_PF46		119
 #define IRQ_PF47		120
 
+#define GPIO_IRQ_BASE		IRQ_PF0
+
 #ifdef CONFIG_IRQCHIP_DEMUX_GPIO
 #define NR_IRQS			(IRQ_PF47 + 1)
 #else
-- 
1.5.2

[PATCH 05/12] Blackfin arch: Advertise GENERIC_GPIO and remove duplicated GENERIC_CALIBRATE_DELAY

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
index 017defa..5c1e215 100644
--- a/arch/blackfin/Kconfig
+++ b/arch/blackfin/Kconfig
@@ -57,7 +57,7 @@ config GENERIC_TIME
 	bool
 	default n
 
-config GENERIC_CALIBRATE_DELAY
+config GENERIC_GPIO
 	bool
 	default y
 
-- 
1.5.2

[PATCH 06/12] Blackfin arch: Add PORT_J.High (needed for BF548-EZkit Touchscreen interrupts) - remove PORT_C.H

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/mach-bf548/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/blackfin/mach-bf548/Kconfig b/arch/blackfin/mach-bf548/Kconfig
index e78b03d..3976b7f 100644
--- a/arch/blackfin/mach-bf548/Kconfig
+++ b/arch/blackfin/mach-bf548/Kconfig
@@ -282,7 +282,7 @@ menu "Assignment"
 
 config PINTx_REASSIGN
 	bool "Reprogram PINT Assignment"
-	default n
+	default y
 	help
 	  The interrupt assignment registers controls the pin-to-interrupt
 	  assignment in a byte-wide manner. Each option allows you to select
@@ -303,7 +303,7 @@ config PINT1_ASSIGN
 config PINT2_ASSIGN
 	hex "PINT2_ASSIGN"
 	depends on PINTx_REASSIGN
-	default 0x00000101
+	default 0x07000101
 config PINT3_ASSIGN
 	hex "PINT3_ASSIGN"
 	depends on PINTx_REASSIGN
-- 
1.5.2

[PATCH 07/12] Blackfin arch: bug fixing, add missing BF533_FAMILY GPIO_PFx definition

[Bryan Wu]

Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 include/asm-blackfin/gpio.h |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/include/asm-blackfin/gpio.h b/include/asm-blackfin/gpio.h
index e714363..dd203cd 100644
--- a/include/asm-blackfin/gpio.h
+++ b/include/asm-blackfin/gpio.h
@@ -144,6 +144,24 @@
 
 #ifdef BF533_FAMILY
 #define MAX_BLACKFIN_GPIOS 16
+
+#define	GPIO_PF0	0
+#define	GPIO_PF1	1
+#define	GPIO_PF2	2
+#define	GPIO_PF3	3
+#define	GPIO_PF4	4
+#define	GPIO_PF5	5
+#define	GPIO_PF6	6
+#define	GPIO_PF7	7
+#define	GPIO_PF8	8
+#define	GPIO_PF9	9
+#define	GPIO_PF10	10
+#define	GPIO_PF11	11
+#define	GPIO_PF12	12
+#define	GPIO_PF13	13
+#define	GPIO_PF14	14
+#define	GPIO_PF15	15
+
 #endif
 
 #ifdef BF537_FAMILY
-- 
1.5.2

[PATCH 08/12] Blackfin arch: add missing gpio error handling to make sure we roll back requests in case one fails

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/kernel/bfin_gpio.c |   10 ++++++++--
 arch/blackfin/mach-bf548/gpio.c  |   11 +++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/blackfin/kernel/bfin_gpio.c b/arch/blackfin/kernel/bfin_gpio.c
index 5d488ef..f712772 100644
--- a/arch/blackfin/kernel/bfin_gpio.c
+++ b/arch/blackfin/kernel/bfin_gpio.c
@@ -711,9 +711,15 @@ int peripheral_request_list(unsigned short per[], const char *label)
 	int ret;
 
 	for (cnt = 0; per[cnt] != 0; cnt++) {
+
 		ret = peripheral_request(per[cnt], label);
-		if (ret < 0)
-			return ret;
+
+		if (ret < 0) {
+			for ( ; cnt > 0; cnt--) {
+				peripheral_free(per[cnt - 1]);
+			}
+		return ret;
+		}
 	}
 
 	return 0;
diff --git a/arch/blackfin/mach-bf548/gpio.c b/arch/blackfin/mach-bf548/gpio.c
index c073ab3..f3b9dea 100644
--- a/arch/blackfin/mach-bf548/gpio.c
+++ b/arch/blackfin/mach-bf548/gpio.c
@@ -212,11 +212,18 @@ int peripheral_request_list(unsigned short per[], const char *label)
 	int ret;
 
 	for (cnt = 0; per[cnt] != 0; cnt++) {
+
 		ret = peripheral_request(per[cnt], label);
-		if (ret < 0)
-			return ret;
+
+		if (ret < 0) {
+			for ( ; cnt > 0; cnt--) {
+				peripheral_free(per[cnt - 1]);
+			}
+		return ret;
+		}
 	}
 
+
 	return 0;
 }
 EXPORT_SYMBOL(peripheral_request_list);
-- 
1.5.2

[PATCH 09/12] Blackfin arch: scrub remaining ASSEMBLY usage since the switch to __ASSEMBLY__

[Bryan Wu]

From: Mike Frysinger <michael.frysinger@analog.com>

Signed-off-by: Mike Frysinger <michael.frysinger@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 include/asm-blackfin/mach-bf533/blackfin.h |    2 +-
 include/asm-blackfin/mach-bf537/blackfin.h |    2 +-
 include/asm-blackfin/mach-bf548/blackfin.h |    2 +-
 include/asm-blackfin/mach-bf561/blackfin.h |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/asm-blackfin/mach-bf533/blackfin.h b/include/asm-blackfin/mach-bf533/blackfin.h
index e438449..f3b240a 100644
--- a/include/asm-blackfin/mach-bf533/blackfin.h
+++ b/include/asm-blackfin/mach-bf533/blackfin.h
@@ -38,7 +38,7 @@
 #include "defBF532.h"
 #include "anomaly.h"
 
-#if !(defined(__ASSEMBLY__) || defined(ASSEMBLY))
+#if !defined(__ASSEMBLY__)
 #include "cdefBF532.h"
 #endif
 
diff --git a/include/asm-blackfin/mach-bf537/blackfin.h b/include/asm-blackfin/mach-bf537/blackfin.h
index bbd9705..f196588 100644
--- a/include/asm-blackfin/mach-bf537/blackfin.h
+++ b/include/asm-blackfin/mach-bf537/blackfin.h
@@ -43,7 +43,7 @@
 #include "defBF537.h"
 #endif
 
-#if !(defined(__ASSEMBLY__) || defined(ASSEMBLY))
+#if !defined(__ASSEMBLY__)
 #include "cdefBF534.h"
 
 /* UART 0*/
diff --git a/include/asm-blackfin/mach-bf548/blackfin.h b/include/asm-blackfin/mach-bf548/blackfin.h
index 791218f..19e84dd 100644
--- a/include/asm-blackfin/mach-bf548/blackfin.h
+++ b/include/asm-blackfin/mach-bf548/blackfin.h
@@ -54,7 +54,7 @@
 #include "defBF549.h"
 #endif
 
-#if !(defined(__ASSEMBLY__) || defined(ASSEMBLY))
+#if !defined(__ASSEMBLY__)
 #ifdef CONFIG_BF542
 #include "cdefBF542.h"
 #endif
diff --git a/include/asm-blackfin/mach-bf561/blackfin.h b/include/asm-blackfin/mach-bf561/blackfin.h
index 2537c84..562aee3 100644
--- a/include/asm-blackfin/mach-bf561/blackfin.h
+++ b/include/asm-blackfin/mach-bf561/blackfin.h
@@ -38,7 +38,7 @@
 #include "defBF561.h"
 #include "anomaly.h"
 
-#if !(defined(__ASSEMBLY__) || defined(ASSEMBLY))
+#if !defined(__ASSEMBLY__)
 #include "cdefBF561.h"
 #endif
 
-- 
1.5.2

[PATCH 10/12] Blackfin arch: update platform driver resource information to the ezkitBF548 board file

[Bryan Wu]

Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/mach-bf548/boards/ezkit.c |  367 ++++++++++++++++++++++++++++++-
 1 files changed, 366 insertions(+), 1 deletions(-)

diff --git a/arch/blackfin/mach-bf548/boards/ezkit.c b/arch/blackfin/mach-bf548/boards/ezkit.c
index 96ad95f..59e64c5 100644
--- a/arch/blackfin/mach-bf548/boards/ezkit.c
+++ b/arch/blackfin/mach-bf548/boards/ezkit.c
@@ -35,9 +35,13 @@
 #include <linux/spi/spi.h>
 #include <linux/spi/flash.h>
 #include <linux/irq.h>
-#include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <asm/bfin5xx_spi.h>
+#include <asm/dma.h>
+#include <asm/gpio.h>
+#include <asm/mach/nand.h>
+#include <asm/mach/bf54x_keys.h>
+#include <linux/spi/ad7877.h>
 
 /*
  * Name the Board for the /proc/cpuinfo
@@ -48,6 +52,87 @@ char *bfin_board_name = "ADSP-BF548-EZKIT";
  *  Driver needs to know address, irq and flag pin.
  */
 
+#if defined(CONFIG_FB_BF54X_LQ043) || defined(CONFIG_FB_BF54X_LQ043_MODULE)
+
+#include <asm/mach/bf54x-lq043.h>
+
+static struct bfin_bf54xfb_mach_info bf54x_lq043_data = {
+	.width =	480,
+	.height =	272,
+	.xres =		{480, 480, 480},
+	.yres =		{272, 272, 272},
+	.bpp =		{24, 24, 24},
+	.disp =		GPIO_PE3,
+};
+
+static struct resource bf54x_lq043_resources[] = {
+	{
+		.start = IRQ_EPPI0_ERR,
+		.end = IRQ_EPPI0_ERR,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device bf54x_lq043_device = {
+	.name		= "bf54x-lq043",
+	.id		= -1,
+	.num_resources 	= ARRAY_SIZE(bf54x_lq043_resources),
+	.resource 	= bf54x_lq043_resources,
+	.dev		= {
+		.platform_data = &bf54x_lq043_data,
+	},
+};
+#endif
+
+#if defined(CONFIG_KEYBOARD_BFIN) || defined(CONFIG_KEYBOARD_BFIN_MODULE)
+static int bf548_keymap[] = {
+	KEYVAL(0, 0, KEY_ENTER),
+	KEYVAL(0, 1, KEY_HELP),
+	KEYVAL(0, 2, KEY_0),
+	KEYVAL(0, 3, KEY_BACKSPACE),
+	KEYVAL(1, 0, KEY_TAB),
+	KEYVAL(1, 1, KEY_9),
+	KEYVAL(1, 2, KEY_8),
+	KEYVAL(1, 3, KEY_7),
+	KEYVAL(2, 0, KEY_DOWN),
+	KEYVAL(2, 1, KEY_6),
+	KEYVAL(2, 2, KEY_5),
+	KEYVAL(2, 3, KEY_4),
+	KEYVAL(3, 0, KEY_UP),
+	KEYVAL(3, 1, KEY_3),
+	KEYVAL(3, 2, KEY_2),
+	KEYVAL(3, 3, KEY_1),
+};
+
+static struct bfin_kpad_platform_data bf54x_kpad_data = {
+	.rows			= 4,
+	.cols			= 4,
+	.keymap 		= bf548_keymap,
+	.keymapsize 		= ARRAY_SIZE(bf548_keymap),
+	.debounce_time		= 5000,	/* ns (5ms) */
+	.coldrive_time		= 1000, /* ns (1ms) */
+	.keyup_test_interval	= 50, /* ms (50ms) */
+};
+
+static struct resource bf54x_kpad_resources[] = {
+	{
+		.start = IRQ_KEY,
+		.end = IRQ_KEY,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device bf54x_kpad_device = {
+	.name		= "bf54x-keys",
+	.id		= -1,
+	.num_resources 	= ARRAY_SIZE(bf54x_kpad_resources),
+	.resource 	= bf54x_kpad_resources,
+	.dev		= {
+		.platform_data = &bf54x_kpad_data,
+	},
+};
+#endif
+
 #if defined(CONFIG_RTC_DRV_BFIN) || defined(CONFIG_RTC_DRV_BFIN_MODULE)
 static struct platform_device rtc_device = {
 	.name = "rtc-bfin",
@@ -94,6 +179,248 @@ static struct platform_device bfin_uart_device = {
 };
 #endif
 
+#if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE)
+static struct resource smsc911x_resources[] = {
+	{
+		.name = "smsc911x-memory",
+		.start = 0x24000000,
+		.end = 0x24000000 + 0xFF,
+		.flags = IORESOURCE_MEM,
+	},
+	{
+		.start = IRQ_PE8,
+		.end = IRQ_PE8,
+		.flags = IORESOURCE_IRQ | IORESOURCE_IRQ_LOWLEVEL,
+	},
+};
+static struct platform_device smsc911x_device = {
+	.name = "smsc911x",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(smsc911x_resources),
+	.resource = smsc911x_resources,
+};
+#endif
+
+#if defined(CONFIG_USB_BF54x_HCD) || defined(CONFIG_USB_BF54x_HCD_MODULE)
+static struct resource bf54x_hcd_resources[] = {
+	{
+		.start = 0xFFC03C00,
+		.end = 0xFFC040FF,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device bf54x_hcd = {
+	.name = "bf54x-hcd",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(bf54x_hcd_resources),
+	.resource = bf54x_hcd_resources,
+};
+#endif
+
+#if defined(CONFIG_PATA_BF54X) || defined(CONFIG_PATA_BF54X_MODULE)
+static struct resource bfin_atapi_resources[] = {
+	{
+		.start = 0xFFC03800,
+		.end = 0xFFC0386F,
+		.flags = IORESOURCE_MEM,
+	},
+	{
+		.start = IRQ_ATAPI_ERR,
+		.end = IRQ_ATAPI_ERR,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device bfin_atapi_device = {
+	.name = "bf54x-atapi",
+	.id = -1,
+	.num_resources = ARRAY_SIZE(bfin_atapi_resources),
+	.resource = bfin_atapi_resources,
+};
+#endif
+
+#if defined(CONFIG_MTD_NAND_BF54X) || defined(CONFIG_MTD_NAND_BF54X_MODULE)
+static struct mtd_partition partition_info[] = {
+	{
+		.name = "Linux Kernel",
+		.offset = 0,
+		.size = 4 * SIZE_1M,
+	},
+	{
+		.name = "File System",
+		.offset = 4 * SIZE_1M,
+		.size = (256 - 4) * SIZE_1M,
+	},
+};
+
+static struct bf54x_nand_platform bf54x_nand_platform = {
+	.page_size = NFC_PG_SIZE_256,
+	.data_width = NFC_NWIDTH_8,
+	.partitions = partition_info,
+	.nr_partitions = ARRAY_SIZE(partition_info),
+	.rd_dly = 3,
+	.wr_dly = 3,
+};
+
+static struct resource bf54x_nand_resources[] = {
+	{
+		.start = 0xFFC03B00,
+		.end = 0xFFC03B4F,
+		.flags = IORESOURCE_MEM,
+	},
+	{
+		.start = CH_NFC,
+		.end = CH_NFC,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device bf54x_nand_device = {
+	.name = "bf54x-nand",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(bf54x_nand_resources),
+	.resource = bf54x_nand_resources,
+	.dev = {
+		.platform_data = &bf54x_nand_platform,
+	},
+};
+#endif
+
+#if defined(CONFIG_SPI_BFIN) || defined(CONFIG_SPI_BFIN_MODULE)
+/* all SPI peripherals info goes here */
+#if defined(CONFIG_MTD_M25P80) \
+	|| defined(CONFIG_MTD_M25P80_MODULE)
+/* SPI flash chip (m25p16) */
+static struct mtd_partition bfin_spi_flash_partitions[] = {
+	{
+		.name = "bootloader",
+		.size = 0x00030000,
+		.offset = 0,
+		.mask_flags = MTD_CAP_ROM
+	}, {
+		.name = "linux kernel",
+		.size = 0x1d0000,
+		.offset = 0x30000
+	}
+};
+
+static struct flash_platform_data bfin_spi_flash_data = {
+	.name = "m25p80",
+	.parts = bfin_spi_flash_partitions,
+	.nr_parts = ARRAY_SIZE(bfin_spi_flash_partitions),
+	.type = "m25p16",
+};
+
+static struct bfin5xx_spi_chip spi_flash_chip_info = {
+	.enable_dma = 0,         /* use dma transfer with this chip*/
+	.bits_per_word = 8,
+	.cs_change_per_word = 0,
+};
+#endif
+
+#if defined(CONFIG_TOUCHSCREEN_AD7877) || defined(CONFIG_TOUCHSCREEN_AD7877_MODULE)
+static struct bfin5xx_spi_chip spi_ad7877_chip_info = {
+	.cs_change_per_word = 1,
+	.enable_dma = 0,
+	.bits_per_word = 16,
+};
+
+static const struct ad7877_platform_data bfin_ad7877_ts_info = {
+	.model			= 7877,
+	.vref_delay_usecs	= 50,	/* internal, no capacitor */
+	.x_plate_ohms		= 419,
+	.y_plate_ohms		= 486,
+	.pressure_max		= 1000,
+	.pressure_min		= 0,
+	.stopacq_polarity 	= 1,
+	.first_conversion_delay = 3,
+	.acquisition_time 	= 1,
+	.averaging 		= 1,
+	.pen_down_acc_interval 	= 1,
+};
+#endif
+
+static struct spi_board_info bf54x_spi_board_info[] __initdata = {
+#if defined(CONFIG_MTD_M25P80) \
+	|| defined(CONFIG_MTD_M25P80_MODULE)
+	{
+		/* the modalias must be the same as spi device driver name */
+		.modalias = "m25p80", /* Name of spi_driver for this device */
+		.max_speed_hz = 25000000,     /* max spi clock (SCK) speed in HZ */
+		.bus_num = 0, /* Framework bus number */
+		.chip_select = 1, /* SPI_SSEL1*/
+		.platform_data = &bfin_spi_flash_data,
+		.controller_data = &spi_flash_chip_info,
+		.mode = SPI_MODE_3,
+	},
+#endif
+#if defined(CONFIG_TOUCHSCREEN_AD7877) || defined(CONFIG_TOUCHSCREEN_AD7877_MODULE)
+{
+	.modalias		= "ad7877",
+	.platform_data		= &bfin_ad7877_ts_info,
+	.irq			= IRQ_PJ11,
+	.max_speed_hz		= 12500000,     /* max spi clock (SCK) speed in HZ */
+	.bus_num		= 1,
+	.chip_select  		= 2,
+	.controller_data = &spi_ad7877_chip_info,
+},
+#endif
+};
+
+/* SPI (0) */
+static struct resource bfin_spi0_resource[] = {
+	[0] = {
+		.start = SPI0_REGBASE,
+		.end   = SPI0_REGBASE + 0xFF,
+		.flags = IORESOURCE_MEM,
+	},
+	[1] = {
+		.start = CH_SPI0,
+		.end   = CH_SPI0,
+		.flags = IORESOURCE_IRQ,
+	}
+};
+
+/* SPI controller data */
+static struct bfin5xx_spi_master bf54x_spi_master_info = {
+	.num_chipselect = 8,
+	.enable_dma = 1,  /* master has the ability to do dma transfer */
+};
+
+static struct platform_device bf54x_spi_master_device = {
+	.name = "bfin-spi",
+	.id = 0, /* Bus number */
+	.num_resources = ARRAY_SIZE(bfin_spi0_resource),
+	.resource = bfin_spi0_resource,
+	.dev = {
+		.platform_data = &bf54x_spi_master_info, /* Passed to driver */
+		},
+};
+#endif  /* spi master and devices */
+
+#if defined(CONFIG_I2C_BLACKFIN_TWI) || defined(CONFIG_I2C_BLACKFIN_TWI_MODULE)
+static struct resource bfin_twi0_resource[] = {
+	[0] = {
+		.start = 0xFFC01400,
+		.end   = 0xFFC014FF,
+		.flags = IORESOURCE_MEM,
+	},
+	[1] = {
+		.start = IRQ_TWI0,
+		.end   = IRQ_TWI0,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device i2c_bfin_twi0_device = {
+	.name = "i2c-bfin-twi",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(bfin_twi0_resource),
+	.resource = bfin_twi0_resource,
+};
+#endif
+
 static struct platform_device *ezkit_devices[] __initdata = {
 #if defined(CONFIG_RTC_DRV_BFIN) || defined(CONFIG_RTC_DRV_BFIN_MODULE)
 	&rtc_device,
@@ -102,12 +429,50 @@ static struct platform_device *ezkit_devices[] __initdata = {
 #if defined(CONFIG_SERIAL_BFIN) || defined(CONFIG_SERIAL_BFIN_MODULE)
 	&bfin_uart_device,
 #endif
+
+#if defined(CONFIG_FB_BF54X_LQ043) || defined(CONFIG_FB_BF54X_LQ043_MODULE)
+	&bf54x_lq043_device,
+#endif
+
+#if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE)
+	&smsc911x_device,
+#endif
+
+#if defined(CONFIG_USB_BF54x_HCD) || defined(CONFIG_USB_BF54x_HCD_MODULE)
+	&bf54x_hcd,
+#endif
+
+#if defined(CONFIG_PATA_BF54X) || defined(CONFIG_PATA_BF54X_MODULE)
+	&bfin_atapi_device,
+#endif
+
+#if defined(CONFIG_MTD_NAND_BF54X) || defined(CONFIG_MTD_NAND_BF54X_MODULE)
+	&bf54x_nand_device,
+#endif
+
+#if defined(CONFIG_SPI_BFIN) || defined(CONFIG_SPI_BFIN_MODULE)
+	&bf54x_spi_master_device,
+#endif
+
+#if defined(CONFIG_KEYBOARD_BFIN) || defined(CONFIG_KEYBOARD_BFIN_MODULE)
+	&bf54x_kpad_device,
+#endif
+
+#if defined(CONFIG_I2C_BLACKFIN_TWI) || defined(CONFIG_I2C_BLACKFIN_TWI_MODULE)
+	&i2c_bfin_twi0_device,
+#endif
 };
 
 static int __init stamp_init(void)
 {
 	printk(KERN_INFO "%s(): registering device resources\n", __FUNCTION__);
 	platform_add_devices(ezkit_devices, ARRAY_SIZE(ezkit_devices));
+
+#if defined(CONFIG_SPI_BFIN) || defined(CONFIG_SPI_BFIN_MODULE)
+	spi_register_board_info(bf54x_spi_board_info,
+			ARRAY_SIZE(bf54x_spi_board_info));
+#endif
+
 	return 0;
 }
 
-- 
1.5.2

[PATCH 11/12] Blackfin arch: after removing fs.h from mm.h, fix the broken on Blackfin arch.

[Bryan Wu]

Cc: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 arch/blackfin/kernel/init_task.c |    1 +
 arch/blackfin/kernel/process.c   |    2 ++
 arch/blackfin/kernel/sys_bfin.c  |    1 +
 arch/blackfin/kernel/traps.c     |    1 +
 4 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/blackfin/kernel/init_task.c b/arch/blackfin/kernel/init_task.c
index b45188f..673c860 100644
--- a/arch/blackfin/kernel/init_task.c
+++ b/arch/blackfin/kernel/init_task.c
@@ -31,6 +31,7 @@
 #include <linux/module.h>
 #include <linux/init_task.h>
 #include <linux/mqueue.h>
+#include <linux/fs.h>
 
 static struct fs_struct init_fs = INIT_FS;
 static struct files_struct init_files = INIT_FILES;
diff --git a/arch/blackfin/kernel/process.c b/arch/blackfin/kernel/process.c
index 5a51dd6..6a7aefe 100644
--- a/arch/blackfin/kernel/process.c
+++ b/arch/blackfin/kernel/process.c
@@ -33,6 +33,8 @@
 #include <linux/user.h>
 #include <linux/a.out.h>
 #include <linux/uaccess.h>
+#include <linux/fs.h>
+#include <linux/err.h>
 
 #include <asm/blackfin.h>
 #include <asm/fixed_code.h>
diff --git a/arch/blackfin/kernel/sys_bfin.c b/arch/blackfin/kernel/sys_bfin.c
index f5e1ae3..abcd148 100644
--- a/arch/blackfin/kernel/sys_bfin.c
+++ b/arch/blackfin/kernel/sys_bfin.c
@@ -37,6 +37,7 @@
 #include <linux/syscalls.h>
 #include <linux/mman.h>
 #include <linux/file.h>
+#include <linux/fs.h>
 #include <linux/uaccess.h>
 #include <linux/ipc.h>
 #include <linux/unistd.h>
diff --git a/arch/blackfin/kernel/traps.c b/arch/blackfin/kernel/traps.c
index 8766bd6..792a841 100644
--- a/arch/blackfin/kernel/traps.c
+++ b/arch/blackfin/kernel/traps.c
@@ -31,6 +31,7 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/kallsyms.h>
+#include <linux/fs.h>
 #include <asm/traps.h>
 #include <asm/cacheflush.h>
 #include <asm/blackfin.h>
-- 
1.5.2

[PATCH 12/12] Blackfin serial driver: use new GPIO API

[Bryan Wu]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
 include/asm-blackfin/mach-bf548/bfin_serial_5xx.h |   39 ++++++++++-----------
 1 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/include/asm-blackfin/mach-bf548/bfin_serial_5xx.h b/include/asm-blackfin/mach-bf548/bfin_serial_5xx.h
index 2f4afc9..f21a162 100644
--- a/include/asm-blackfin/mach-bf548/bfin_serial_5xx.h
+++ b/include/asm-blackfin/mach-bf548/bfin_serial_5xx.h
@@ -1,5 +1,6 @@
 #include <linux/serial.h>
 #include <asm/dma.h>
+#include <asm/portmux.h>
 
 #define NR_PORTS		4
 
@@ -143,50 +144,48 @@ struct bfin_serial_res bfin_serial_resource[] = {
 
 int nr_ports = ARRAY_SIZE(bfin_serial_resource);
 
+#define DRIVER_NAME "bfin-uart"
+
 static void bfin_serial_hw_init(struct bfin_serial_port *uart)
 {
 #ifdef CONFIG_SERIAL_BFIN_UART0
-	/* Enable UART0 RX and TX on pin 7 & 8 of PORT E */
-	bfin_write_PORTE_FER(0x180 | bfin_read_PORTE_FER());
-	bfin_write_PORTE_MUX(0x3C000 | bfin_read_PORTE_MUX());
+	peripheral_request(P_UART0_TX, DRIVER_NAME);
+	peripheral_request(P_UART0_RX, DRIVER_NAME);
 #endif
 
 #ifdef CONFIG_SERIAL_BFIN_UART1
-	/* Enable UART1 RX and TX on pin 0 & 1 of PORT H */
-	bfin_write_PORTH_FER(0x3 | bfin_read_PORTH_FER());
-	bfin_write_PORTH_MUX(~0xF & bfin_read_PORTH_MUX());
+	peripheral_request(P_UART1_TX, DRIVER_NAME);
+	peripheral_request(P_UART1_RX, DRIVER_NAME);
+
 #ifdef CONFIG_BFIN_UART1_CTSRTS
-	/* Enable UART1 RTS and CTS on pin 9 & 10 of PORT E */
-	bfin_write_PORTE_FER(0x600 | bfin_read_PORTE_FER());
-	bfin_write_PORTE_MUX(~0x3C0000 & bfin_read_PORTE_MUX());
+	peripheral_request(P_UART1_RTS, DRIVER_NAME);
+	peripheral_request(P_UART1_CTS DRIVER_NAME);
 #endif
 #endif
 
 #ifdef CONFIG_SERIAL_BFIN_UART2
-	/* Enable UART2 RX and TX on pin 4 & 5 of PORT B */
-	bfin_write_PORTB_FER(0x30 | bfin_read_PORTB_FER());
-	bfin_write_PORTB_MUX(~0xF00 & bfin_read_PORTB_MUX());
+	peripheral_request(P_UART2_TX, DRIVER_NAME);
+	peripheral_request(P_UART2_RX, DRIVER_NAME);
 #endif
 
 #ifdef CONFIG_SERIAL_BFIN_UART3
-	/* Enable UART3 RX and TX on pin 6 & 7 of PORT B */
-	bfin_write_PORTB_FER(0xC0 | bfin_read_PORTB_FER());
-	bfin_write_PORTB_MUX(~0xF000 | bfin_read_PORTB_MUX());
+	peripheral_request(P_UART3_TX, DRIVER_NAME);
+	peripheral_request(P_UART3_RX, DRIVER_NAME);
+
 #ifdef CONFIG_BFIN_UART3_CTSRTS
-	/* Enable UART3 RTS and CTS on pin 2 & 3 of PORT B */
-	bfin_write_PORTB_FER(0xC | bfin_read_PORTB_FER());
-	bfin_write_PORTB_MUX(~0xF0 | bfin_read_PORTB_MUX());
+	peripheral_request(P_UART3_RTS, DRIVER_NAME);
+	peripheral_request(P_UART3_CTS DRIVER_NAME);
 #endif
 #endif
 	SSYNC();
 #ifdef CONFIG_SERIAL_BFIN_CTSRTS
 	if (uart->cts_pin >= 0) {
-		gpio_request(uart->cts_pin, NULL);
+		gpio_request(uart->cts_pin, DRIVER_NAME);
 		gpio_direction_input(uart->cts_pin);
 	}
 
 	if (uart->rts_pin >= 0) {
-		gpio_request(uart->rts_pin, NULL);
+		gpio_request(uart->rts_pin, DRIVER_NAME);
 		gpio_direction_output(uart->rts_pin);
 	}
 #endif
-- 
1.5.2

Re: [PATCH 12/12] Blackfin serial driver: use new GPIO API

[David Brownell]

On Tuesday 07 August 2007, Bryan Wu wrote:
> -       /* Enable UART0 RX and TX on pin 7 & 8 of PORT E */
> -       bfin_write_PORTE_FER(0x180 | bfin_read_PORTE_FER());
> -       bfin_write_PORTE_MUX(0x3C000 | bfin_read_PORTE_MUX());
> +       peripheral_request(P_UART0_TX, DRIVER_NAME);
> +       peripheral_request(P_UART0_RX, DRIVER_NAME);


This is not a GPIO API, so the patch summary and description
are wrong.  That's pin muxing.  You didn't change any of
the GPIO related calls.

Re: [PATCH 01/12] Blackfin arch: add peripheral resource allocation support

[David Brownell]

On Tuesday 07 August 2007, Bryan Wu wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>

The patch description here is IMO misleading, and is clearly
weak-to-nonexistent ...  what this patch does is

 * Start tracking the label strings provided by gpio_request()
 * Provide a new portmux mechanisms
 * Start using those in the serial support code

When I read "resource allocation" I think of "struct resource"
from <linux/ioport.h>, allocate_resource(), and so on.  So while
it's true there are other kinds of driver resource, it's rather
unnatural for me to think about pin mux and gpio issues in any
terms other than chip and board setup.


> +static int cmp_label(unsigned short ident, const char *label)
> +{
> +	if (label && str_ident)
> +		return strncmp(str_ident + ident * RESOURCE_LABEL_SIZE,
> +				 label, strlen(label));
> +	else
> +		return -EINVAL;
> +}

GRPIO labels are purely for diagnostics.  There's no reason to
compare one to another.  You seem to be using these for purposes
in addition to GPIOs though ... probably worth commenting on that
unusual scheme.


> +int peripheral_request(unsigned short per, const char *label)
> +{
> +	...
> +
> +	if (unlikely(reserved_peri_map[gpio_bank(ident)] & gpio_bit(ident))) {
> +
> +	/*
> +	 * Pin functions like AMC address strobes my
> +	 * be requested and used by several drivers
> +	 */
> +
> +	if (!(per & P_MAYSHARE)) {

Goofy indentation.  And as a rule, drivers have been kept out of
the business of configuring pin usage.  It's simpler that way;
they don't need to try coping with configuration errors like two
drivers wanting conflicting usage ... or as you say above, needing
some explicit sharing mechanism ...


> +
> +	/*
> +	 * Allow that the identical pin function can
> +	 * be requested from the same driver twice
> +	 */

... or as you say here, needing to structure themselves so they
don't configure the same usage more than once ...


That said, how you handle pinmux on Blackfin is your business.

But you should know that this approach seems idiosyncratic and
more complex than needed:  when pin config is done early and as
part of board setup, drivers don't need to care about it or to
handle any pinmux errors.  And heck, products can sometimes be
shipped with the bootloader having done all pinmux setup, so
Linux won't need to worry about it at all.  That can help ship
multiple board revisions using the same kernel.

- Dave

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[David Brownell]

Again, the patch descriptions need work.  This changes the
IRQ code (to add those labels).  $SUBJECT doesn't mention IRQs,
neither does the description ...


On Tuesday 07 August 2007, Bryan Wu wrote:
> --- a/arch/blackfin/mach-common/ints-priority-dc.c
> +++ b/arch/blackfin/mach-common/ints-priority-dc.c
> @@ -221,7 +221,7 @@ static unsigned int bf561_gpio_irq_startup(unsigned int irq)
>  
>         if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
>  
> -               ret = gpio_request(gpionr, NULL);
> +               ret = gpio_request(gpionr, "IRQ");
>                 if (ret)
>                         return ret;
>  
> @@ -261,7 +261,7 @@ static int bf561_gpio_irq_type(unsigned int irq, unsigned int type)
>  
>                 if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
>  
> -                       ret = gpio_request(gpionr, NULL);
> +                       ret = gpio_request(gpionr, "IRQ");
>                         if (ret)
>                                 return ret;
>  

Just for the record, this is an unusual way to use these calls.

Other platforms completely decouple these issues from the
IRQ infrastructure ... doing the pinmux and gpio claiming
separately from the request_irq()/free_irq() paths, mostly
as part of board setup.  Doing all of that "early":

 - keeps those error returns from causing hard-to-track-down
   runtime bugs;

 - works always, even on platforms where a given IRQ may
   appear on any of several pins/balls;

 - makes it easier to cross-check against board schematics,
   by keeping most board-specific setup in one source file;

 - shrinks the kernel's runtime footprint;

 - allows the label to be more descriptive ... describeing
   exactly *which* IRQ, so that using the labels for better
   diagnostics actually gives better diagnostics.

Again, not "wrong"; but probably sub-optimal.  You might
want to move towards earlier binding now, while Linux is
still young on Blackfin and you don't have legacy code to
worry about.

- Dave

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[Mike Frysinger]

On 8/17/07, David Brownell <david-b@pacbell.net> wrote:
> Again, the patch descriptions need work.  This changes the
> IRQ code (to add those labels).  $SUBJECT doesn't mention IRQs,
> neither does the description ...
>
>
> On Tuesday 07 August 2007, Bryan Wu wrote:
> > --- a/arch/blackfin/mach-common/ints-priority-dc.c
> > +++ b/arch/blackfin/mach-common/ints-priority-dc.c
> > @@ -221,7 +221,7 @@ static unsigned int bf561_gpio_irq_startup(unsigned int irq)
> >
> > if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
> >
> > -ret = gpio_request(gpionr, NULL);
> > +ret = gpio_request(gpionr, "IRQ");
> > if (ret)
> > return ret;
> >
> > @@ -261,7 +261,7 @@ static int bf561_gpio_irq_type(unsigned int irq, unsigned int type)
> >
> > if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
> >
> > -ret = gpio_request(gpionr, NULL);
> > +ret = gpio_request(gpionr, "IRQ");
> > if (ret)
> > return ret;
> >
>
> Just for the record, this is an unusual way to use these calls.
>
> Other platforms completely decouple these issues from the
> IRQ infrastructure ... doing the pinmux and gpio claiming
> separately from the request_irq()/free_irq() paths, mostly
> as part of board setup.  Doing all of that "early":
>
>  - keeps those error returns from causing hard-to-track-down
>    runtime bugs;
>
>  - works always, even on platforms where a given IRQ may
>    appear on any of several pins/balls;
>
>  - makes it easier to cross-check against board schematics,
>    by keeping most board-specific setup in one source file;
>
>  - shrinks the kernel's runtime footprint;
>
>  - allows the label to be more descriptive ... describeing
>    exactly *which* IRQ, so that using the labels for better
>    diagnostics actually gives better diagnostics.
>
> Again, not "wrong"; but probably sub-optimal.  You might
> want to move towards earlier binding now, while Linux is
> still young on Blackfin and you don't have legacy code to
> worry about.

in the Blackfin port, if you want to use a pin as an IRQ rather than
GPIO, you use the normal request_irq/free_irq API ... those functions
will call back into the proper GPIO/PORTMUX code so that the pin is
setup properly.  this is done so that code isnt duplicated across
files and so that we can easily detect if someone does something
incorrect like try to take the same pin and use it as
irq/gpio/whatever at the same time ...

are you saying that other ports dont unify the backend code paths at all ?
-mike

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[David Brownell]

On Friday 17 August 2007, Mike Frysinger wrote:
> On 8/17/07, David Brownell <david-b@pacbell.net> wrote:
> > ...
> > Just for the record, this is an unusual way to use these calls.
> >
> > Other platforms completely decouple these issues from the
> > IRQ infrastructure ... doing the pinmux and gpio claiming
> > separately from the request_irq()/free_irq() paths, mostly
> > as part of board setup.  Doing all of that "early":
> >
> >  - keeps those error returns from causing hard-to-track-down
> >    runtime bugs;
> >
> >  - works always, even on platforms where a given IRQ may
> >    appear on any of several pins/balls;
> >
> >  - makes it easier to cross-check against board schematics,
> >    by keeping most board-specific setup in one source file;
> >
> >  - shrinks the kernel's runtime footprint;
> >
> >  - allows the label to be more descriptive ... describeing
> >    exactly *which* IRQ, so that using the labels for better
> >    diagnostics actually gives better diagnostics.
> >
> > Again, not "wrong"; but probably sub-optimal.  You might
> > want to move towards earlier binding now, while Linux is
> > still young on Blackfin and you don't have legacy code to
> > worry about.
> 
> in the Blackfin port, if you want to use a pin as an IRQ rather than
> GPIO, you use the normal request_irq/free_irq API ... those functions
> will call back into the proper GPIO/PORTMUX code so that the pin is
> setup properly.  this is done so that code isnt duplicated across
> files and so that we can easily detect if someone does something
> incorrect like try to take the same pin and use it as
> irq/gpio/whatever at the same time ...
> 
> are you saying that other ports dont unify the backend code paths at all ?

Some platforms try to "unify" the pin setup in the boot loader.
Most of them cope with bogus bootloaders by doing it in the board
setup code.

I don't know of any who try to do it "late" as you summarized.

See above why "late" unification is not necessarily as good as
"early" unification.

And then there's the OMAP1 example, where for example you might
know that you want MPUIO-0 but that's insufficient to tell whether
you must mux ball F18 or R13 ... so it's impossible to do the kind
of "late" unification done here, and pinmux *MUST* be separate from
IRQ setup.

- Dave

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[Mike Frysinger]

On 8/17/07, David Brownell <david-b@pacbell.net> wrote:
> On Friday 17 August 2007, Mike Frysinger wrote:
> > On 8/17/07, David Brownell <david-b@pacbell.net> wrote:
> > > ...
> > > Just for the record, this is an unusual way to use these calls.
> > >
> > > Other platforms completely decouple these issues from the
> > > IRQ infrastructure ... doing the pinmux and gpio claiming
> > > separately from the request_irq()/free_irq() paths, mostly
> > > as part of board setup.  Doing all of that "early":
> > >
> > >  - keeps those error returns from causing hard-to-track-down
> > >    runtime bugs;
> > >
> > >  - works always, even on platforms where a given IRQ may
> > >    appear on any of several pins/balls;
> > >
> > >  - makes it easier to cross-check against board schematics,
> > >    by keeping most board-specific setup in one source file;
> > >
> > >  - shrinks the kernel's runtime footprint;
> > >
> > >  - allows the label to be more descriptive ... describeing
> > >    exactly *which* IRQ, so that using the labels for better
> > >    diagnostics actually gives better diagnostics.
> > >
> > > Again, not "wrong"; but probably sub-optimal.  You might
> > > want to move towards earlier binding now, while Linux is
> > > still young on Blackfin and you don't have legacy code to
> > > worry about.
> >
> > in the Blackfin port, if you want to use a pin as an IRQ rather than
> > GPIO, you use the normal request_irq/free_irq API ... those functions
> > will call back into the proper GPIO/PORTMUX code so that the pin is
> > setup properly.  this is done so that code isnt duplicated across
> > files and so that we can easily detect if someone does something
> > incorrect like try to take the same pin and use it as
> > irq/gpio/whatever at the same time ...
> >
> > are you saying that other ports dont unify the backend code paths at all ?
>
> Some platforms try to "unify" the pin setup in the boot loader.
> Most of them cope with bogus bootloaders by doing it in the board
> setup code.
>
> I don't know of any who try to do it "late" as you summarized.
>
> See above why "late" unification is not necessarily as good as
> "early" unification.
>
> And then there's the OMAP1 example, where for example you might
> know that you want MPUIO-0 but that's insufficient to tell whether
> you must mux ball F18 or R13 ... so it's impossible to do the kind
> of "late" unification done here, and pinmux *MUST* be separate from
> IRQ setup.

sorry, i'm not familiar at all with anything OMAP, but the Blackfin
architecture is such that all of its pins can be flipped on the fly
between modes ... i have seen once or twice some drivers which
actually needed this ability at runtime as they were muxing things
(maybe our fault there ... give someone the ability and they'll use
it).

as Michael pointed out, in the Blackfin world we tend to keep things
very dynamic as we have dev systems which allow for dropping in of
optional cards at will, so doing this in the bootloader is way too
inflexible.
-mike

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[Mike Frysinger]

On 8/17/07, Mike Frysinger <vapier.adi@gmail.com> wrote:
> as Michael pointed out, in the Blackfin world we tend to keep things
> very dynamic as we have dev systems which allow for dropping in of
> optional cards at will, so doing this in the bootloader is way too
> inflexible.

oh, and another [smallish] data point.  the Blackfin processor has a
small bootrom on it that could be likened to a very micro bios.  so
it's possible to actually boot the linux kernel straight without a
boot loader.  send the kernel over the UART to a Blackfin and watch it
go go go :)
-mike

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[David Brownell]

On Friday 17 August 2007, Mike Frysinger wrote:

> as Michael pointed out, in the Blackfin world we tend to keep things
> very dynamic as we have dev systems which allow for dropping in of
> optional cards at will, so doing this in the bootloader is way too
> inflexible.

That's the tradeoff:  optimize for development boards, or
instead for more fixed-function product boards.


> oh, and another [smallish] data point.  the Blackfin processor has a
> small bootrom on it that could be likened to a very micro bios.  so
> it's possible to actually boot the linux kernel straight without a
> boot loader.  send the kernel over the UART to a Blackfin and watch it
> go go go :)

That's not uncommon, although I'm more used to seeing the
on-chip ROMs relying on a second stage loader for stuff like
getting memory and other clocks going at optimal speeds,
and loading "big" images (that won't fit on-chip SRAM).

- Dave

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[Robin Getz]

On Fri 17 Aug 2007 14:24, David Brownell pondered:
> Just for the record, this is an unusual way to use these calls.

That is part of the natural evolution of the kernel isn't it - per James's 
keynote at OLS - you release something, and see how people [ab]use it until 
it either grows, evolves, or it dies.

> Other platforms completely decouple these issues from the
> IRQ infrastructure ... doing the pinmux and gpio claiming
> separately from the request_irq()/free_irq() paths, mostly
> as part of board setup.  Doing all of that "early":

is early:
 - early in the kernel?
 - early before the kernel? (in the bootloader).

>  - keeps those error returns from causing hard-to-track-down
>    runtime bugs;

The current Blackfin implementation causes a run time message:
"the pin xxxx driver requested, was already claimed by yyy driver".

I don't think that is too bad?

>  - works always, even on platforms where a given IRQ may
>    appear on any of several pins/balls;

But requires custom bootloaders or board setup for every hardware platform? 
Most of our users would not like that, since they do as you say - use the 
same kernel - with different drivers on multiple platforms.

>  - makes it easier to cross-check against board schematics,
>    by keeping most board-specific setup in one source file;

Yes - but we are not talking about muxing a common peripheral (like a single 
UART) out many different pins (A or B or C). The UART pins are fixed. If you 
want the UART, you need to use pin A. If you want to use the I2C that also 
sits on pin A, you will get the message:
"pin A, requested by I2C, was already claimed by UART driver".

>  - shrinks the kernel's runtime footprint;

I agree - making things more flexible/easier to use - is normally more 
complex/larger/slower. (I know - easier to use is a matter of opinion). Since 
this is normally done once, in _init functions, I'm not sure that makes much 
of a difference here.
 
>  - allows the label to be more descriptive ... describeing
>    exactly *which* IRQ, so that using the labels for better
>    diagnostics actually gives better diagnostics.

I'm not sure what you mean?

> Again, not "wrong"; but probably sub-optimal.  You might
> want to move towards earlier binding now, while Linux is
> still young on Blackfin and you don't have legacy code to
> worry about.

Our overall goal is to keep as much code - including bootloader - platform 
agnostic, and not require people to write any of code/configuration data to 
boot up something, and get things working in a semi-standard manner.

This still has it's limits - which is why we publish all our hardware designs. 
If you implement things the similar way (because for the most part it is 
fixed by the processor designer) - the bootloader/kernel/driver will just 
work.

I would rather force a little extra complexity on me (as a kernel developer) 
than have to answer thousands of questions from end users, who are trying to 
move the kernel onto their hardware.

-Robin

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[David Brownell]

On Friday 17 August 2007, Robin Getz wrote:
> On Fri 17 Aug 2007 14:24, David Brownell pondered:
> > Just for the record, this is an unusual way to use these calls.
> 
> That is part of the natural evolution of the kernel isn't it - per James's 
> keynote at OLS - you release something, and see how people [ab]use it until 
> it either grows, evolves, or it dies.

Yep ... and it's worth knowing when you're doing
something different.  Different isn't always worse,
isn't always better.


> > Other platforms completely decouple these issues from the
> > IRQ infrastructure ... doing the pinmux and gpio claiming
> > separately from the request_irq()/free_irq() paths, mostly
> > as part of board setup.  Doing all of that "early":
> 
> is early:
>  - early in the kernel?
>  - early before the kernel? (in the bootloader).

Both of those are "earlier", yes.  Different product developers
may argue for either placement.

 
> >  - keeps those error returns from causing hard-to-track-down
> >    runtime bugs;
> 
> The current Blackfin implementation causes a run time message:
> "the pin xxxx driver requested, was already claimed by yyy driver".
> 
> I don't think that is too bad?

Given some product with a Blackfin chip, would you expect a
customer -- who may not even see the Linux bits!! -- to be
able to solve such problems?  If it's not possible for such
problems to crop up in the field, product support (and field
troubleshooting) gets easier...


> >  - works always, even on platforms where a given IRQ may
> >    appear on any of several pins/balls;
> 
> But requires custom bootloaders or board setup for every hardware platform?

One or both, yes.  That's typical in embedded setups.
They're not necessarily all that different, but that
code does need to handle the hardware differences.


> Most of our users would not like that, since they do as you say - use the 
> same kernel - with different drivers on multiple platforms.

I thought I referred to different revisions of one platform... :)

 
> >  - makes it easier to cross-check against board schematics,
> >    by keeping most board-specific setup in one source file;
> 
> Yes - but we are not talking about muxing a common peripheral (like a single 
> UART) out many different pins (A or B or C). The UART pins are fixed. If you 
> want the UART, you need to use pin A. If you want to use the I2C that also 
> sits on pin A, you will get the message:
> "pin A, requested by I2C, was already claimed by UART driver".

Not all platforms work that way though.  There can often be several
options for where a given signal gets routed.

And then there are the errors where someone accidentally copies
something like "GPIO 29" to two places ... invariants like "only
one GPIO requestor at a time" are needed to turn up such stuff.


> >  - shrinks the kernel's runtime footprint;
> 
> I agree - making things more flexible/easier to use - is normally more 
> complex/larger/slower. (I know - easier to use is a matter of opinion). Since 
> this is normally done once, in _init functions, I'm not sure that makes much 
> of a difference here.
>  
> >  - allows the label to be more descriptive ... describeing
> >    exactly *which* IRQ, so that using the labels for better
> >    diagnostics actually gives better diagnostics.
> 
> I'm not sure what you mean?

The $SUBJECT patch uses the string "IRQ" in all cases.
But "smc_irq" and "codec_irq" would be more informative
as entries in a list of even just a handful of GPIOs.
And with a few dozen, I'd find "IRQ" not at all helpful.


> > Again, not "wrong"; but probably sub-optimal.  You might
> > want to move towards earlier binding now, while Linux is
> > still young on Blackfin and you don't have legacy code to
> > worry about.
> 
> Our overall goal is to keep as much code - including bootloader - platform 
> agnostic, and not require people to write any of code/configuration data to 
> boot up something, and get things working in a semi-standard manner.

The issue is just where those limits lie.  IMO it's not at
all unreasonable to require board-specific code.  External
chips will need board-specfic glue data in most cases (how
they're addressed, what IRQs they use, and so on); and you
may have drivers available that correspond to devices that
are not wired up on that particular hardware.


> This still has it's limits - which is why we publish all our hardware designs. 
> If you implement things the similar way (because for the most part it is 
> fixed by the processor designer) - the bootloader/kernel/driver will just 
> work.

Sure ... you'd need to say "this board uses <these devices>"
and if integrated in the SOC that's often enough.  External
devices need more configuration.  Even for integrated ones,
that knowledge doesn't belong in the driver ... "which of the
many UARTS to use as console" isn't standard, and neither
is "what hardware handshaking pins are in use".

 
> I would rather force a little extra complexity on me (as a kernel developer) 
> than have to answer thousands of questions from end users, who are trying to 
> move the kernel onto their hardware.

Just remember that "Aunt Tilly" doesn't configure kernels.
And that even deeply technical people who do configure
them may not have the details fresh in mind a few months
later.  ;)

- Dave

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[Robin Getz]

On Fri 17 Aug 2007 18:34, David Brownell pondered:
> On Friday 17 August 2007, Robin Getz wrote:
> > On Fri 17 Aug 2007 14:24, David Brownell pondered:
> > > Just for the record, this is an unusual way to use these calls.
> > 
> > That is part of the natural evolution of the kernel isn't it - per
> > James's keynote at OLS - you release something, and see how people 
> > [ab]use it until it either grows, evolves, or it dies.
> 
> Yep ... and it's worth knowing when you're doing
> something different.  Different isn't always worse,
> isn't always better.

No disagreements here.

> > > Other platforms completely decouple these issues from the
> > > IRQ infrastructure ... doing the pinmux and gpio claiming
> > > separately from the request_irq()/free_irq() paths, mostly
> > > as part of board setup.  Doing all of that "early":
> > 
> > is early:
> >  - early in the kernel?
> >  - early before the kernel? (in the bootloader).
> 
> Both of those are "earlier", yes.  Different product developers
> may argue for either placement.

Just like we say things are better/easier for us later.

> > >  - keeps those error returns from causing hard-to-track-down
> > >    runtime bugs;
> > 
> > The current Blackfin implementation causes a run time message:
> > "the pin xxxx driver requested, was already claimed by yyy driver".
> > 
> > I don't think that is too bad?
> 
> Given some product with a Blackfin chip, would you expect a
> customer -- who may not even see the Linux bits!! -- to be
> able to solve such problems?  If it's not possible for such
> problems to crop up in the field, product support (and field
> troubleshooting) gets easier...

Typically customers who are not familiar with the linux bits are not doing 
modprobe either...

I don't see how early/late makes the problem easier/worse to debug. No matter 
when you do it - the driver refuses to install (or at least should).

> > >  - works always, even on platforms where a given IRQ may
> > >    appear on any of several pins/balls;
> > 
> > But requires custom bootloaders or board setup for every hardware
> > platform?
> 
> One or both, yes.  That's typical in embedded setups.
> They're not necessarily all that different, but that
> code does need to handle the hardware differences.

Right - for us - the code handing the hardware differences is easier in the 
drivers, rather than the bootloaders.

For other systems - where you can have a UART on any pin - I completely 
understand your point.

> > Most of our users would not like that, since they do as you say - use
> > the same kernel - with different drivers on multiple platforms.
> 
> I thought I referred to different revisions of one platform... :)

You did - I was just saying that some of our customers don't do it the way you 
were thinking.

> > >  - makes it easier to cross-check against board schematics,
> > >    by keeping most board-specific setup in one source file;
> > 
> > Yes - but we are not talking about muxing a common peripheral (like a
> > single UART) out many different pins (A or B or C). The UART pins are
> > fixed. If you want the UART, you need to use pin A. If you want to use
> > the I2C that also sits on pin A, you will get the message:
> > "pin A, requested by I2C, was already claimed by UART driver".
> 
> Not all platforms work that way though.  There can often be several
> options for where a given signal gets routed.

But this is how it _always_ works on Blackfin. For other systems - like ARM, 
where n+1 silicon manufactures are all implementing things differently - I 
can understand your comments.


> > >  - allows the label to be more descriptive ... describeing
> > >    exactly *which* IRQ, so that using the labels for better
> > >    diagnostics actually gives better diagnostics.
> > 
> > I'm not sure what you mean?
> 
> The $SUBJECT patch uses the string "IRQ" in all cases.
> But "smc_irq" and "codec_irq" would be more informative
> as entries in a list of even just a handful of GPIOs.
> And with a few dozen, I'd find "IRQ" not at all helpful.

I agree - things can always be more descriptive.

> > > Again, not "wrong"; but probably sub-optimal.  You might
> > > want to move towards earlier binding now, while Linux is
> > > still young on Blackfin and you don't have legacy code to
> > > worry about.
> > 
> > Our overall goal is to keep as much code - including bootloader -
> > platform  agnostic, and not require people to write any of
> > code/configuration data to boot up something, and get things
> > working in a semi-standard manner. 
> 
> The issue is just where those limits lie.  IMO it's not at
> all unreasonable to require board-specific code.  External
> chips will need board-specfic glue data in most cases (how
> they're addressed, what IRQs they use, and so on); and you
> may have drivers available that correspond to devices that
> are not wired up on that particular hardware.
> 
> 
> > This still has it's limits - which is why we publish all our hardware
> > designs. If you implement things the similar way (because for the
> > most part it is fixed by the processor designer) - the
> > bootloader/kernel/driver will just work.
> 
> Sure ... you'd need to say "this board uses <these devices>"
> and if integrated in the SOC that's often enough. 

with the kernel .config - that is what happens. If you have 2 serial drivers 
connected - you enable 2 serial drivers in Kconfig.

> External 
> devices need more configuration.  Even for integrated ones,
> that knowledge doesn't belong in the driver ... "which of the
> many UARTS to use as console" isn't standard, and neither
> is "what hardware handshaking pins are in use".

When hardware handshaking pins are fixed - it sure is. When they are not (when 
the hardware doesn't support hardware handshaking, and you need to do it in 
software) - we still allow you do to it via Kconfig.

linux-2.6.x/drivers/serial/Kconfig:

config UART0_CTS_PIN
        int "UART0 CTS pin"
        depends on BFIN_UART0_CTSRTS
        default 23
        help
          The default pin is GPIO_GP7.
          Refer to ./include/asm-blackfin/gpio.h to see the GPIO map.

config UART0_RTS_PIN
        int "UART0 RTS pin"
        depends on BFIN_UART0_CTSRTS
        default 22
        help
          The default pin is GPIO_GP6.
          Refer to ./include/asm-blackfin/gpio.h to see the GPIO map.

Board configs are in one place - under source control - the kernel .config
  
> > I would rather force a little extra complexity on me (as a kernel
> > developer) than have to answer thousands of questions from end 
> > users, who are trying to move the kernel onto their hardware.
> 
> Just remember that "Aunt Tilly" doesn't configure kernels.
> And that even deeply technical people who do configure
> them may not have the details fresh in mind a few months
> later.  ;)

I wish some of our customers where as good as my aunt Tilly when it comes to 
kernel config, or could remember as well as she can.

I guess we thought it was easier for people to select a few things in config, 
rather than have to write C code/include files for board specific 
implementations options - It is like you said - everything is all in one 
place...

-Robin

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[David Brownell]

On Saturday 18 August 2007, Robin Getz wrote:

> I don't see how early/late makes the problem easier/worse to debug. No matter 
> when you do it - the driver refuses to install (or at least should).

If you arrange to *reliably* detect the pinmux/setup problems by
the time the system starts ""init" (early), that means one large
class of hard-to-sort problems never needs runtime troubleshooting.

Think of it this way:  folk have observed that pin setup issues can
be painful to sort out.  So they adopt a strategy ("failfast"/"early")
which helps surface them early and basically removes them as issues
in later debugging.  I think you're hoping that by adding extra
resource tracking code, you can make that later debugging easier
even though, by "late" binding, you've introduced extra error paths.


> Right - for us - the code handing the hardware differences is easier in the 
> drivers, rather than the bootloaders.

Remember that I didn't argue in favor of putting that code into
boot loaders ... I just pointed out that some product lines work
that way, so Linux needs to cope with that strategy.  (One of the
many examples involves OpenFirmware device tables.)

But regardless:  I can't buy any argument that it's better to put
lots of board-specific code into drivers.  That adds up quickly,
making maintaining the drivers painful.  "Real" updates (bugfixes,
new features, API updates, cleanup, and so on) regularly end up
in conflict with patches to support a few more boards, and board
support patches must then always involve those driver maintainers.
So merging new boards involves many more people than necessary...


> For other systems - where you can have a UART on any pin - I completely 
> understand your point.

UART on any pin?  Few kernels dynamically reprogram FPGAs!  :)


> > Sure ... you'd need to say "this board uses <these devices>"
> > and if integrated in the SOC that's often enough. 
> 
> with the kernel .config - that is what happens. If you have 2 serial drivers 
> connected - you enable 2 serial drivers in Kconfig.

Your language is incorrect here.  What your Kconfig does is
not configure two different *drivers* ... but some number of
different serial *devices* handled by the same driver.

One obvious downside of that is that making it needlessly hard
to support several boards with one kernel.  As a rule, those
boards can have different serial devices, and the devices can
be configured differently.  Yet you said you wanted to make it
easy to support many boards with one kernel...


> > External 
> > devices need more configuration.  Even for integrated ones,
> > that knowledge doesn't belong in the driver ... "which of the
> > many UARTs to use as console" isn't standard, and neither
> > is "what hardware handshaking pins are in use".
> 
> When hardware handshaking pins are fixed - it sure is.

Not unless the UART for some odd reason *requires* those pins to work.

There's almost always support for pure software handshaking (XON/XOFF),
with one board option being "don't handshake".  Board A might use two
pins for UART2 RTS/CTS; board B might use UART as well, but use those
two pins for another I2C bus.  The differences belong in board-specific
configuration, not in drivers.


> When they are not (when  
> the hardware doesn't support hardware handshaking, and you need to do it in 
> software) - we still allow you do to it via Kconfig.
> 
> linux-2.6.x/drivers/serial/Kconfig:

That can work, at least for *single-board* kernel builds.  Of course,
that gets into territory some people will say is Kconfig abuse ...
and the need for many ugly #ifdefs is very obvious.  :)

In fact one could argue that those bits of Kconfig syntax are really
just support for one Blackfin board (ezkit), and so they don't belong
in that Kconfig file or with those names...

Plus, that approach only works with fairly simple types of device glue.
It's routine to find chip hookups that can't fit smoothly into some
pre-planned Kconfig, since they require board-specific function hooks.
(Sometimes even with UARTs, but clearly not in this case.)


> Board configs are in one place - under source control - the kernel .config

And in arch/blackfin/mach-*/boards/*.c ... all that stuff you set up
in Kconfig could as easily have been coded in those files, without any
need for #ifdefs or confusing Kconfig.  Still under source control,
plus it's a lot harder to break.  :)


> I guess we thought it was easier for people to select a few things in config, 
> rather than have to write C code/include files for board specific 
> implementations options - It is like you said - everything is all in one 
> place...

Doing that in Kconfig is atypical ... it may well be a bit easier to
pick up at the beginning of a developer's learning curve, but I think
it doesn't scale very well as multi-board product lines evolve.

- Dave

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[Robin Getz]

On Sun 19 Aug 2007 17:54, David Brownell pondered:
> On Saturday 18 August 2007, Robin Getz wrote:
> 
> > I don't see how early/late makes the problem easier/worse to debug. No
> > matter when you do it - the driver refuses to install (or at least
> > should). 
> 
> If you arrange to *reliably* detect the pinmux/setup problems by
> the time the system starts ""init" (early), that means one large
> class of hard-to-sort problems never needs runtime troubleshooting.

Sure it does - it just needs to do it in the bootloader, not the kernel. You 
haven't eliminated the problem - or made it any easier to debug - it is just 
moved somewhere else. Again - this is not bad. We decided/are attempting to 
do it in the kernel. Normally what we find is there are more kernel people on 
a project than bootloader (whether this makes this easier - or not - is still 
TDB :)

> Think of it this way:  folk have observed that pin setup issues can
> be painful to sort out. 

Absolutely - the problem that everyone is trying to solve - is how to do plug 
and play, with no enumeration?

Doing it early in the bootloader is akin to historical PC solution where early 
ISA PNP (shudder) filled out a table in memory, and passed it to the OS.

> So they adopt a strategy ("failfast"/"early") 
> which helps surface them early and basically removes them as issues
> in later debugging.

When the kernel engineer runs into a problem because a driver won't load, is 
it better that he can fix it himself (hopefully), or that they have to call 
the person maintaining the bootloader? There are pro/cons of either. I can 
see the value in both. 

We choose to do it in the driver, not the bootloader.

> > Right - for us - the code handing the hardware differences is easier
> > in the drivers, rather than the bootloaders.
> 
> Remember that I didn't argue in favor of putting that code into
> boot loaders ... I just pointed out that some product lines work
> that way, so Linux needs to cope with that strategy.  (One of the
> many examples involves OpenFirmware device tables.)
> 
> But regardless:  I can't buy any argument that it's better to put
> lots of board-specific code into drivers. 

I don't think we are putting board specific code in drivers (or if there is - 
it should get removed).

I did a quick look, and the only place this has happened is in some of our 
drivers that have not made it to main line yet - where we accidently put some 
mtd_partitions in the drivers, rather than the boards file. I know we are 
working on fixing this.

> That adds up quickly, 
> making maintaining the drivers painful.  "Real" updates (bugfixes,
> new features, API updates, cleanup, and so on) regularly end up
> in conflict with patches to support a few more boards, and board
> support patches must then always involve those driver maintainers.
> So merging new boards involves many more people than necessary...

I agree - which is why don't do this either. Board specific info does not go 
into drivers. I think this is something that Michael, Bryan and others 
working on the Blackfin arch & drivers will agree to 100%.

What we do is try to make the driver agnostic to the hardware as much as 
possible, where hardware specifics (chip selects, IRQ, GPIO, etc) are managed 
in Kconfig.

I can see the value of doing the initialisation in the bootloader - since this 
would allow you have a common driver - and hardware customisation is done in 
the bootloader.

>
> > 
> > linux-2.6.x/drivers/serial/Kconfig:
> 
> That can work, at least for *single-board* kernel builds.  Of course,
> that gets into territory some people will say is Kconfig abuse ...
> and the need for many ugly #ifdefs is very obvious.  :)

We have been trying to minimise that - and I think we have been doing a pretty 
good job. There doesn't seem to be any platform specific ifdefs in our 
drivers that are not abstracted out.

> Doing that in Kconfig is atypical ... it may well be a bit easier to
> pick up at the beginning of a developer's learning curve, but I think
> it doesn't scale very well as multi-board product lines evolve.

We have lots of end users (obviously not as many as ARM or PPC yet), and they 
have not been complaining, in fact some say that this is easier to deploy.

But - I think we both agree - that what we are doing is just an alternative 
implemention of hardware abstraction - that is different than the way that 
some others are doing it. Not better/worse (from what I can tell) - it just 
has different tradeoffs.

-Robin

Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

[David Brownell]

On Sunday 19 August 2007, Robin Getz wrote:
> On Sun 19 Aug 2007 17:54, David Brownell pondered:
> > On Saturday 18 August 2007, Robin Getz wrote:
> > 
> > > I don't see how early/late makes the problem easier/worse to debug. No
> > > matter when you do it - the driver refuses to install (or at least
> > > should). 
> > 
> > If you arrange to *reliably* detect the pinmux/setup problems by
> > the time the system starts ""init" (early), that means one large
> > class of hard-to-sort problems never needs runtime troubleshooting.
> 
> Sure it does - it just needs to do it in the bootloader, not the kernel.

Kernel arch/.../board-X.c code is the place to "reliably" handle
that.  Bootloader is out-of-scope here; it's a separate codebase.


> You  
> haven't eliminated the problem - or made it any easier to debug - it is just 
> moved somewhere else.

Moved it out of runtime, to a small window before "init" sections
get discarded and "init" is invoked.  So if it ever *could* show
up, it'll show up then ... every time the system boots, a message
will appear.  Easy to notice while the bug is fresh, just from a
developer's routine scan of bootup messages.

The alternative lets the problem appear later, almost randomly,
based on whether the conflicting drivers happen to be loaded
at the same time.  Which *IS* harder to debug.


I still remember the time a board had to be respun because the
hardware guys goofed on use of one signal.  Two different drivers
both needed it ... but until later in system integration, they
were never tested together, so that conflict never showed up.
(It was a non-obvious thing, for on-chip signal routing not the
on-board type verified by module tests in hardware QA and design
review.)

Doing that setup "early" would have prevented that problem, and
avoided one annoying schedule slip and hardware respin.


> Again - this is not bad. We decided/are attempting to  
> do it in the kernel. Normally what we find is there are more kernel people on 
> a project than bootloader (whether this makes this easier - or not - is still 
> TDB :)

I don't know why you're reading what I write as any kind of
preference for doing that in the bootloader.  It's nothing
more than an *acknowledgement* that some vendors work that
way, so Linux is better off with an approach which won't be
broken when they do.  (That is, always having finished pin
mux setup before drivers get probed.)

When Linux does that setup in arch/..../board-X.c files, mostly
before drivers come into play, it's normal for driver code to
ignore it ... which means the bootloader might also have done
it, for those kinds of product.

Usually I'd expect one person on the bootloader, plus ideally
understudies.  Not a full time job; that person might mostly
do kernel development, hardware test/bringup, or something
else depending on how the product team was structured.


> > Remember that I didn't argue in favor of putting that code into
> > boot loaders ... I just pointed out that some product lines work
> > that way, so Linux needs to cope with that strategy.  (One of the
> > many examples involves OpenFirmware device tables.)
> > 
> > But regardless:  I can't buy any argument that it's better to put
> > lots of board-specific code into drivers. 
> 
> I don't think we are putting board specific code in drivers (or if there is - 
> it should get removed).
> 
> I did a quick look, and the only place this has happened is in some of our 
> drivers that have not made it to main line yet

Good!!  If it's not in drivers, I suspect you'll agree that it
will probably all land in that arch/..../board-X.c setup code.


> I agree - which is why don't do this either. Board specific info does not go 
> into drivers. I think this is something that Michael, Bryan and others 
> working on the Blackfin arch & drivers will agree to 100%.
> 
> What we do is try to make the driver agnostic to the hardware as much as 
> possible,

Good ...

> where hardware specifics (chip selects, IRQ, GPIO, etc) are managed  
> in Kconfig.

... IMO less good, but that's all your worry.  I suspect that
by the time you handle a few dozen boards, you'll find Kconfig
is not well suited to this.  (Because of the way it prevents one
kernel from handling multiple boards, unless they really aren't
very different.)


> > Doing that in Kconfig is atypical ... it may well be a bit easier to
> > pick up at the beginning of a developer's learning curve, but I think
> > it doesn't scale very well as multi-board product lines evolve.
> 
> We have lots of end users (obviously not as many as ARM or PPC yet), and they 
> have not been complaining, in fact some say that this is easier to deploy.

These are people already familiar with ARM or PPC embedded Linux?
I guess one question is "easier than what".

Kconfig doesn't try to help with the "give me a minimal .config
for <THIS> hardware" problem; say, by reading "lspci -n" output
or some other system description.  It expects people to know their
way around.  Maybe a bit of a "20 questions" style interface
comes across as providing useful hand-holding; some of that can
be done in Kconfig, but it's still a kind of menu maze.

I'd hope people don't still get degrees for writing automagic
system configuration tools.  Even though it's still not easy.  :)


> But - I think we both agree - that what we are doing is just an alternative 
> implemention of hardware abstraction - that is different than the way that 
> some others are doing it. Not better/worse (from what I can tell) - it just 
> has different tradeoffs.

We could have that discussion in a few years.  I suspect you'll
find less going into Kconfig and more into C code, if for no
other reason than to make sure things scale sanely.  Or if lots
of stuff is in Kconfig ... it'll be in the arch part of the tree
associated with boards, not the driver parts.

- Dave