2006-11-07 11:42:09

by Haavard Skinnemoen

[permalink] [raw]
Subject: [-mm patch 0/4] Atmel SPI driver and related AVR32 changes

Hi all,

This series contains a SPI master driver for the Atmel AT32/AT91 SPI
controller along with a GPIO API for avr32 and a few other changes this
driver depends on to work.

I have compile-tested the driver on arm, but someone please verify that
it works on at91 boards. It should be usable for both at91rm9200 and
at91sam926x after the necessary platform-specific code is added. David
sent me some code to do this on at91 once, but I dropped it from the
patch since I don't think it will work anymore. Please let me know if
you want med to send those changes as a separate patch anyway.

The driver has been tested with mtd_dataflash and a LCD panel driver on
atstk1000 (avr32). Unfortunately, I haven't been able to use it with
jffs2, as the jffs2 wbuf code BUGs on me with mtd_dataflash (this has
been reported by several people on linux-mtd, but there doesn't seem to
be a solution yet.)

I'd appreciate comments on both the SPI driver itself (big thanks to
David for all the comments I got so far) and the GPIO API. I think we
should work towards minimizing the differences between the at32 and
at91 gpio apis so that we can eliminate a few #ifdefs in the common
drivers.

Haavard


2006-11-07 11:42:38

by Haavard Skinnemoen

[permalink] [raw]
Subject: [-mm patch 2/4] AVR32: spi/ethernet platform_device update

Make the spi platform_device definitions match the driver and add
definitions for the second spi controller on at32ap7000. The driver
expects the device name to be "atmel_spi" and the main clock to be
"pclk".

Also add platform_device definitions for the second ethernet controller
on at32ap7000.

Signed-off-by: Haavard Skinnemoen <[email protected]>
---
arch/avr32/mach-at32ap/at32ap7000.c | 64 ++++++++++++++++++++++++++++++++----
1 file changed, 57 insertions(+), 7 deletions(-)

Index: linux-2.6.19-rc4-mm2/arch/avr32/mach-at32ap/at32ap7000.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/avr32/mach-at32ap/at32ap7000.c 2006-11-07 10:30:59.000000000 +0100
+++ linux-2.6.19-rc4-mm2/arch/avr32/mach-at32ap/at32ap7000.c 2006-11-07 10:46:12.000000000 +0100
@@ -650,6 +650,15 @@ DEFINE_DEV_DATA(macb, 0);
DEV_CLK(hclk, macb0, hsb, 8);
DEV_CLK(pclk, macb0, pbb, 6);

+static struct eth_platform_data macb1_data;
+static struct resource macb1_resource[] = {
+ PBMEM(0xfff01c00),
+ IRQ(26),
+};
+DEFINE_DEV_DATA(macb, 1);
+DEV_CLK(hclk, macb1, hsb, 9);
+DEV_CLK(pclk, macb1, pbb, 7);
+
struct platform_device *__init
at32_add_device_eth(unsigned int id, struct eth_platform_data *data)
{
@@ -683,6 +692,33 @@ at32_add_device_eth(unsigned int id, str
}
break;

+ case 1:
+ pdev = &macb1_device;
+
+ select_peripheral(PD13, B); /* TXD0 */
+ select_peripheral(PD14, B); /* TXD1 */
+ select_peripheral(PD11, B); /* TXEN */
+ select_peripheral(PD12, B); /* TXCK */
+ select_peripheral(PD10, B); /* RXD0 */
+ select_peripheral(PD6, B); /* RXD1 */
+ select_peripheral(PD5, B); /* RXER */
+ select_peripheral(PD4, B); /* RXDV */
+ select_peripheral(PD3, B); /* MDC */
+ select_peripheral(PD2, B); /* MDIO */
+
+ if (!data->is_rmii) {
+ select_peripheral(PC19, B); /* COL */
+ select_peripheral(PC23, B); /* CRS */
+ select_peripheral(PC26, B); /* TXER */
+ select_peripheral(PC27, B); /* TXD2 */
+ select_peripheral(PC28, B); /* TXD3 */
+ select_peripheral(PC29, B); /* RXD2 */
+ select_peripheral(PC30, B); /* RXD3 */
+ select_peripheral(PC24, B); /* RXCK */
+ select_peripheral(PD15, B); /* SPD */
+ }
+ break;
+
default:
return NULL;
}
@@ -700,8 +736,15 @@ static struct resource atmel_spi0_resour
PBMEM(0xffe00000),
IRQ(3),
};
-DEFINE_DEV(spi, 0);
-DEV_CLK(mck, spi0, pba, 0);
+DEFINE_DEV(atmel_spi, 0);
+DEV_CLK(pclk, atmel_spi0, pba, 0);
+
+static struct resource atmel_spi1_resource[] = {
+ PBMEM(0xffe00400),
+ IRQ(4),
+};
+DEFINE_DEV(atmel_spi, 1);
+DEV_CLK(pclk, atmel_spi1, pba, 1);

struct platform_device *__init
at32_add_device_spi(unsigned int id)
@@ -711,13 +754,17 @@ at32_add_device_spi(unsigned int id)

switch (id) {
case 0:
- pdev = &spi0_device;
+ pdev = &atmel_spi0_device;
select_peripheral(PA0, A); /* MISO */
select_peripheral(PA1, A); /* MOSI */
select_peripheral(PA2, A); /* SCK */
- select_peripheral(PA3, A); /* NPCS0 */
- select_peripheral(PA4, A); /* NPCS1 */
- select_peripheral(PA5, A); /* NPCS2 */
+ break;
+
+ case 1:
+ pdev = &atmel_spi1_device;
+ select_peripheral(PB0, B); /* MISO */
+ select_peripheral(PB1, B); /* MOSI */
+ select_peripheral(PB5, B); /* SCK */
break;

default:
@@ -836,7 +883,10 @@ struct clk *at32_clock_list[] = {
&atmel_usart3_usart,
&macb0_hclk,
&macb0_pclk,
- &spi0_mck,
+ &macb1_hclk,
+ &macb1_pclk,
+ &atmel_spi0_pclk,
+ &atmel_spi1_pclk,
&lcdc0_hclk,
&lcdc0_pixclk,
};

2006-11-07 11:42:54

by Haavard Skinnemoen

[permalink] [raw]
Subject: [-mm patch 1/4] GPIO framework for AVR32

Add a simple GPIO framework for AVR32. This should be fairly similar
to the AT91 GPIO API, but there are still a couple of differences:

* Naming. We prefix all functions with gpio_ instead of at91_
* request_gpio() and free_gpio() should be called to make sure
the required pins are available, but it will still work if you
don't.

Signed-off-by: Haavard Skinnemoen <[email protected]>
---
arch/avr32/mach-at32ap/at32ap7000.c | 154 ++++++++++++++---------------
arch/avr32/mach-at32ap/pio.c | 139 ++++++++++++++++++++++++--
include/asm-avr32/arch-at32ap/at32ap7000.h | 141 ++++++++++++++++++++++++++
include/asm-avr32/arch-at32ap/gpio.h | 14 ++
include/asm-avr32/arch-at32ap/irq.h | 11 ++
include/asm-avr32/arch-at32ap/portmux.h | 16 ---
include/asm-avr32/irq.h | 8 +
7 files changed, 380 insertions(+), 103 deletions(-)

Index: linux-2.6.19-rc4-mm2/arch/avr32/mach-at32ap/at32ap7000.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/avr32/mach-at32ap/at32ap7000.c 2006-11-07 10:26:59.000000000 +0100
+++ linux-2.6.19-rc4-mm2/arch/avr32/mach-at32ap/at32ap7000.c 2006-11-07 10:30:59.000000000 +0100
@@ -11,8 +11,9 @@

#include <asm/io.h>

+#include <asm/arch/at32ap7000.h>
#include <asm/arch/board.h>
-#include <asm/arch/portmux.h>
+#include <asm/arch/gpio.h>
#include <asm/arch/sm.h>

#include "clock.h"
@@ -67,17 +68,12 @@ static struct clk devname##_##_name = {
.index = _index, \
}

-enum {
- PIOA,
- PIOB,
- PIOC,
- PIOD,
-};
-
-enum {
- FUNC_A,
- FUNC_B,
-};
+/*
+ * We call this lots of times, so we want everything to fit in a
+ * single line...
+ */
+#define select_peripheral(pin, func) \
+ portmux_select_peripheral(GPIO_PIN_##pin, GPIO_PERIPH_##func)

unsigned long at32ap7000_osc_rates[3] = {
[0] = 32768,
@@ -569,26 +565,26 @@ DEV_CLK(usart, atmel_usart3, pba, 6);

static inline void configure_usart0_pins(void)
{
- portmux_set_func(PIOA, 8, FUNC_B); /* RXD */
- portmux_set_func(PIOA, 9, FUNC_B); /* TXD */
+ select_peripheral(PA8, B); /* RXD */
+ select_peripheral(PA9, B); /* TXD */
}

static inline void configure_usart1_pins(void)
{
- portmux_set_func(PIOA, 17, FUNC_A); /* RXD */
- portmux_set_func(PIOA, 18, FUNC_A); /* TXD */
+ select_peripheral(PA17, A); /* RXD */
+ select_peripheral(PA18, A); /* TXD */
}

static inline void configure_usart2_pins(void)
{
- portmux_set_func(PIOB, 26, FUNC_B); /* RXD */
- portmux_set_func(PIOB, 27, FUNC_B); /* TXD */
+ select_peripheral(PB26, B); /* RXD */
+ select_peripheral(PB27, B); /* TXD */
}

static inline void configure_usart3_pins(void)
{
- portmux_set_func(PIOB, 18, FUNC_B); /* RXD */
- portmux_set_func(PIOB, 17, FUNC_B); /* TXD */
+ select_peripheral(PB18, B); /* RXD */
+ select_peripheral(PB17, B); /* TXD */
}

static struct platform_device *at32_usarts[4];
@@ -663,27 +659,27 @@ at32_add_device_eth(unsigned int id, str
case 0:
pdev = &macb0_device;

- portmux_set_func(PIOC, 3, FUNC_A); /* TXD0 */
- portmux_set_func(PIOC, 4, FUNC_A); /* TXD1 */
- portmux_set_func(PIOC, 7, FUNC_A); /* TXEN */
- portmux_set_func(PIOC, 8, FUNC_A); /* TXCK */
- portmux_set_func(PIOC, 9, FUNC_A); /* RXD0 */
- portmux_set_func(PIOC, 10, FUNC_A); /* RXD1 */
- portmux_set_func(PIOC, 13, FUNC_A); /* RXER */
- portmux_set_func(PIOC, 15, FUNC_A); /* RXDV */
- portmux_set_func(PIOC, 16, FUNC_A); /* MDC */
- portmux_set_func(PIOC, 17, FUNC_A); /* MDIO */
+ select_peripheral(PC3, A); /* TXD0 */
+ select_peripheral(PC4, A); /* TXD1 */
+ select_peripheral(PC7, A); /* TXEN */
+ select_peripheral(PC8, A); /* TXCK */
+ select_peripheral(PC9, A); /* RXD0 */
+ select_peripheral(PC10, A); /* RXD1 */
+ select_peripheral(PC13, A); /* RXER */
+ select_peripheral(PC15, A); /* RXDV */
+ select_peripheral(PC16, A); /* MDC */
+ select_peripheral(PC17, A); /* MDIO */

if (!data->is_rmii) {
- portmux_set_func(PIOC, 0, FUNC_A); /* COL */
- portmux_set_func(PIOC, 1, FUNC_A); /* CRS */
- portmux_set_func(PIOC, 2, FUNC_A); /* TXER */
- portmux_set_func(PIOC, 5, FUNC_A); /* TXD2 */
- portmux_set_func(PIOC, 6, FUNC_A); /* TXD3 */
- portmux_set_func(PIOC, 11, FUNC_A); /* RXD2 */
- portmux_set_func(PIOC, 12, FUNC_A); /* RXD3 */
- portmux_set_func(PIOC, 14, FUNC_A); /* RXCK */
- portmux_set_func(PIOC, 18, FUNC_A); /* SPD */
+ select_peripheral(PC0, A); /* COL */
+ select_peripheral(PC1, A); /* CRS */
+ select_peripheral(PC2, A); /* TXER */
+ select_peripheral(PC5, A); /* TXD2 */
+ select_peripheral(PC6, A); /* TXD3 */
+ select_peripheral(PC11, A); /* RXD2 */
+ select_peripheral(PC12, A); /* RXD3 */
+ select_peripheral(PC14, A); /* RXCK */
+ select_peripheral(PC18, A); /* SPD */
}
break;

@@ -700,26 +696,28 @@ at32_add_device_eth(unsigned int id, str
/* --------------------------------------------------------------------
* SPI
* -------------------------------------------------------------------- */
-static struct resource spi0_resource[] = {
+static struct resource atmel_spi0_resource[] = {
PBMEM(0xffe00000),
IRQ(3),
};
DEFINE_DEV(spi, 0);
DEV_CLK(mck, spi0, pba, 0);

-struct platform_device *__init at32_add_device_spi(unsigned int id)
+struct platform_device *__init
+at32_add_device_spi(unsigned int id)
{
struct platform_device *pdev;
+ unsigned int i;

switch (id) {
case 0:
pdev = &spi0_device;
- portmux_set_func(PIOA, 0, FUNC_A); /* MISO */
- portmux_set_func(PIOA, 1, FUNC_A); /* MOSI */
- portmux_set_func(PIOA, 2, FUNC_A); /* SCK */
- portmux_set_func(PIOA, 3, FUNC_A); /* NPCS0 */
- portmux_set_func(PIOA, 4, FUNC_A); /* NPCS1 */
- portmux_set_func(PIOA, 5, FUNC_A); /* NPCS2 */
+ select_peripheral(PA0, A); /* MISO */
+ select_peripheral(PA1, A); /* MOSI */
+ select_peripheral(PA2, A); /* SCK */
+ select_peripheral(PA3, A); /* NPCS0 */
+ select_peripheral(PA4, A); /* NPCS1 */
+ select_peripheral(PA5, A); /* NPCS2 */
break;

default:
@@ -762,37 +760,37 @@ at32_add_device_lcdc(unsigned int id, st
switch (id) {
case 0:
pdev = &lcdc0_device;
- portmux_set_func(PIOC, 19, FUNC_A); /* CC */
- portmux_set_func(PIOC, 20, FUNC_A); /* HSYNC */
- portmux_set_func(PIOC, 21, FUNC_A); /* PCLK */
- portmux_set_func(PIOC, 22, FUNC_A); /* VSYNC */
- portmux_set_func(PIOC, 23, FUNC_A); /* DVAL */
- portmux_set_func(PIOC, 24, FUNC_A); /* MODE */
- portmux_set_func(PIOC, 25, FUNC_A); /* PWR */
- portmux_set_func(PIOC, 26, FUNC_A); /* DATA0 */
- portmux_set_func(PIOC, 27, FUNC_A); /* DATA1 */
- portmux_set_func(PIOC, 28, FUNC_A); /* DATA2 */
- portmux_set_func(PIOC, 29, FUNC_A); /* DATA3 */
- portmux_set_func(PIOC, 30, FUNC_A); /* DATA4 */
- portmux_set_func(PIOC, 31, FUNC_A); /* DATA5 */
- portmux_set_func(PIOD, 0, FUNC_A); /* DATA6 */
- portmux_set_func(PIOD, 1, FUNC_A); /* DATA7 */
- portmux_set_func(PIOD, 2, FUNC_A); /* DATA8 */
- portmux_set_func(PIOD, 3, FUNC_A); /* DATA9 */
- portmux_set_func(PIOD, 4, FUNC_A); /* DATA10 */
- portmux_set_func(PIOD, 5, FUNC_A); /* DATA11 */
- portmux_set_func(PIOD, 6, FUNC_A); /* DATA12 */
- portmux_set_func(PIOD, 7, FUNC_A); /* DATA13 */
- portmux_set_func(PIOD, 8, FUNC_A); /* DATA14 */
- portmux_set_func(PIOD, 9, FUNC_A); /* DATA15 */
- portmux_set_func(PIOD, 10, FUNC_A); /* DATA16 */
- portmux_set_func(PIOD, 11, FUNC_A); /* DATA17 */
- portmux_set_func(PIOD, 12, FUNC_A); /* DATA18 */
- portmux_set_func(PIOD, 13, FUNC_A); /* DATA19 */
- portmux_set_func(PIOD, 14, FUNC_A); /* DATA20 */
- portmux_set_func(PIOD, 15, FUNC_A); /* DATA21 */
- portmux_set_func(PIOD, 16, FUNC_A); /* DATA22 */
- portmux_set_func(PIOD, 17, FUNC_A); /* DATA23 */
+ select_peripheral(PC19, A); /* CC */
+ select_peripheral(PC20, A); /* HSYNC */
+ select_peripheral(PC21, A); /* PCLK */
+ select_peripheral(PC22, A); /* VSYNC */
+ select_peripheral(PC23, A); /* DVAL */
+ select_peripheral(PC24, A); /* MODE */
+ select_peripheral(PC25, A); /* PWR */
+ select_peripheral(PC26, A); /* DATA0 */
+ select_peripheral(PC27, A); /* DATA1 */
+ select_peripheral(PC28, A); /* DATA2 */
+ select_peripheral(PC29, A); /* DATA3 */
+ select_peripheral(PC30, A); /* DATA4 */
+ select_peripheral(PC31, A); /* DATA5 */
+ select_peripheral(PD0, A); /* DATA6 */
+ select_peripheral(PD1, A); /* DATA7 */
+ select_peripheral(PD2, A); /* DATA8 */
+ select_peripheral(PD3, A); /* DATA9 */
+ select_peripheral(PD4, A); /* DATA10 */
+ select_peripheral(PD5, A); /* DATA11 */
+ select_peripheral(PD6, A); /* DATA12 */
+ select_peripheral(PD7, A); /* DATA13 */
+ select_peripheral(PD8, A); /* DATA14 */
+ select_peripheral(PD9, A); /* DATA15 */
+ select_peripheral(PD10, A); /* DATA16 */
+ select_peripheral(PD11, A); /* DATA17 */
+ select_peripheral(PD12, A); /* DATA18 */
+ select_peripheral(PD13, A); /* DATA19 */
+ select_peripheral(PD14, A); /* DATA20 */
+ select_peripheral(PD15, A); /* DATA21 */
+ select_peripheral(PD16, A); /* DATA22 */
+ select_peripheral(PD17, A); /* DATA23 */

clk_set_parent(&lcdc0_pixclk, &pll0);
clk_set_rate(&lcdc0_pixclk, clk_get_rate(&pll0));
Index: linux-2.6.19-rc4-mm2/arch/avr32/mach-at32ap/pio.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/avr32/mach-at32ap/pio.c 2006-11-07 10:26:59.000000000 +0100
+++ linux-2.6.19-rc4-mm2/arch/avr32/mach-at32ap/pio.c 2006-11-07 10:27:01.000000000 +0100
@@ -14,8 +14,9 @@
#include <linux/platform_device.h>

#include <asm/io.h>
+#include <asm/irq.h>

-#include <asm/arch/portmux.h>
+#include <asm/arch/gpio.h>

#include "pio.h"

@@ -31,17 +32,141 @@ struct pio_device {

static struct pio_device pio_dev[MAX_NR_PIO_DEVICES];

-void portmux_set_func(unsigned int portmux_id, unsigned int pin_id,
- unsigned int function_id)
+/* GPIO API */
+
+static struct pio_device *get_pio_for_pin(unsigned int pin)
+{
+ struct pio_device *pio;
+ unsigned int index;
+
+ index = (pin - GPIO_IRQ_BASE) >> 5;
+ if (index > MAX_NR_PIO_DEVICES)
+ return NULL;
+ pio = &pio_dev[index];
+ if (!pio->regs)
+ return NULL;
+
+ return pio;
+}
+
+int request_gpio(unsigned int pin)
+{
+ struct pio_device *pio;
+ unsigned int pio_pin;
+ u32 mask;
+
+ pio = get_pio_for_pin(pin);
+ if (!pio)
+ return -ENODEV;
+
+ pio_pin = pin & 0x1f;
+ if (test_and_set_bit(pio_pin, &pio->alloc_mask))
+ return -EBUSY;
+
+ mask = 1 << pio_pin;
+ pio_writel(pio, PUER, mask);
+ pio_writel(pio, ODR, mask);
+ pio_writel(pio, PER, mask);
+
+ return 0;
+}
+EXPORT_SYMBOL(request_gpio);
+
+void free_gpio(unsigned int pin)
+{
+ struct pio_device *pio;
+ unsigned int pio_pin;
+ u32 mask;
+
+ pio = get_pio_for_pin(pin);
+ BUG_ON(!pio);
+
+ pio_pin = pin & 0x1f;
+ mask = 1 << pio_pin;
+ pio_writel(pio, PUER, mask);
+ pio_writel(pio, ODR, mask);
+ pio_writel(pio, PER, mask);
+
+ clear_bit(pio_pin, &pio->alloc_mask);
+}
+EXPORT_SYMBOL(free_gpio);
+
+void gpio_set_value(unsigned int pin, int value)
+{
+ struct pio_device *pio;
+ u32 mask;
+
+ /* If you want gentle error handling, call request_gpio() first */
+ pio = get_pio_for_pin(pin);
+ BUG_ON(!pio);
+
+ mask = 1 << (pin & 0x1f);
+ if (value)
+ pio_writel(pio, SODR, mask);
+ else
+ pio_writel(pio, CODR, mask);
+}
+EXPORT_SYMBOL(gpio_set_value);
+
+int gpio_get_value(unsigned int pin)
+{
+ struct pio_device *pio;
+ unsigned int pio_pin;
+
+ pio = get_pio_for_pin(pin);
+ BUG_ON(!pio);
+
+ pio_pin = pin & 0x1f;
+ return (pio_readl(pio, PDSR) >> pio_pin) & 1;
+}
+EXPORT_SYMBOL(gpio_get_value);
+
+void gpio_set_output_enable(unsigned int pin, int enabled)
+{
+ struct pio_device *pio;
+ u32 mask;
+
+ pio = get_pio_for_pin(pin);
+ BUG_ON(!pio);
+
+ mask = 1 << (pin & 0x1f);
+ if (enabled)
+ pio_writel(pio, OER, mask);
+ else
+ pio_writel(pio, ODR, mask);
+}
+EXPORT_SYMBOL(gpio_set_output_enable);
+
+void gpio_set_pullup_enable(unsigned int pin, int enabled)
+{
+ struct pio_device *pio;
+ u32 mask;
+
+ pio = get_pio_for_pin(pin);
+ BUG_ON(!pio);
+
+ mask = 1 << (pin & 0x1f);
+ if (enabled)
+ pio_writel(pio, PUER, mask);
+ else
+ pio_writel(pio, PUDR, mask);
+}
+EXPORT_SYMBOL(gpio_set_pullup_enable);
+
+void __init portmux_select_peripheral(unsigned int pin, unsigned int function)
{
struct pio_device *pio;
- u32 mask = 1 << pin_id;
+ unsigned int pio_pin = pin & 0x1f;
+ u32 mask = 1 << pio_pin;

- BUG_ON(portmux_id >= MAX_NR_PIO_DEVICES);
+ pio = get_pio_for_pin(pin);
+ BUG_ON(!pio);

- pio = &pio_dev[portmux_id];
+ if (test_and_set_bit(pin & 0x1f, &pio->alloc_mask))
+ printk(KERN_ERR "%s: pin %d allocated twice\n",
+ pio->name, pio_pin);

- if (function_id)
+ if (function)
pio_writel(pio, BSR, mask);
else
pio_writel(pio, ASR, mask);
Index: linux-2.6.19-rc4-mm2/include/asm-avr32/arch-at32ap/at32ap7000.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.19-rc4-mm2/include/asm-avr32/arch-at32ap/at32ap7000.h 2006-11-07 10:27:01.000000000 +0100
@@ -0,0 +1,141 @@
+#ifndef __ASM_ARCH_AT32AP7000_H
+#define __ASM_ARCH_AT32AP7000_H
+
+#include <asm/irq.h>
+
+#define GPIO_PERIPH_A 0
+#define GPIO_PERIPH_B 1
+
+#define NR_GPIO_CONTROLLERS 4
+
+/*
+ * Pin numbers identifying specific GPIO pins on the chip. They can
+ * also be used as IRQ numbers.
+ */
+#define GPIO_PIOA_BASE (GPIO_IRQ_BASE)
+#define GPIO_PIN_PA0 (GPIO_PIOA_BASE + 0)
+#define GPIO_PIN_PA1 (GPIO_PIOA_BASE + 1)
+#define GPIO_PIN_PA2 (GPIO_PIOA_BASE + 2)
+#define GPIO_PIN_PA3 (GPIO_PIOA_BASE + 3)
+#define GPIO_PIN_PA4 (GPIO_PIOA_BASE + 4)
+#define GPIO_PIN_PA5 (GPIO_PIOA_BASE + 5)
+#define GPIO_PIN_PA6 (GPIO_PIOA_BASE + 6)
+#define GPIO_PIN_PA7 (GPIO_PIOA_BASE + 7)
+#define GPIO_PIN_PA8 (GPIO_PIOA_BASE + 8)
+#define GPIO_PIN_PA9 (GPIO_PIOA_BASE + 9)
+#define GPIO_PIN_PA10 (GPIO_PIOA_BASE + 10)
+#define GPIO_PIN_PA11 (GPIO_PIOA_BASE + 11)
+#define GPIO_PIN_PA12 (GPIO_PIOA_BASE + 12)
+#define GPIO_PIN_PA13 (GPIO_PIOA_BASE + 13)
+#define GPIO_PIN_PA14 (GPIO_PIOA_BASE + 14)
+#define GPIO_PIN_PA15 (GPIO_PIOA_BASE + 15)
+#define GPIO_PIN_PA16 (GPIO_PIOA_BASE + 16)
+#define GPIO_PIN_PA17 (GPIO_PIOA_BASE + 17)
+#define GPIO_PIN_PA18 (GPIO_PIOA_BASE + 18)
+#define GPIO_PIN_PA19 (GPIO_PIOA_BASE + 19)
+#define GPIO_PIN_PA20 (GPIO_PIOA_BASE + 20)
+#define GPIO_PIN_PA21 (GPIO_PIOA_BASE + 21)
+#define GPIO_PIN_PA22 (GPIO_PIOA_BASE + 22)
+#define GPIO_PIN_PA23 (GPIO_PIOA_BASE + 23)
+#define GPIO_PIN_PA24 (GPIO_PIOA_BASE + 24)
+#define GPIO_PIN_PA25 (GPIO_PIOA_BASE + 25)
+#define GPIO_PIN_PA26 (GPIO_PIOA_BASE + 26)
+#define GPIO_PIN_PA27 (GPIO_PIOA_BASE + 27)
+#define GPIO_PIN_PA28 (GPIO_PIOA_BASE + 28)
+#define GPIO_PIN_PA29 (GPIO_PIOA_BASE + 29)
+#define GPIO_PIN_PA30 (GPIO_PIOA_BASE + 30)
+#define GPIO_PIN_PA31 (GPIO_PIOA_BASE + 31)
+
+#define GPIO_PIOB_BASE (GPIO_PIOA_BASE + 32)
+#define GPIO_PIN_PB0 (GPIO_PIOB_BASE + 0)
+#define GPIO_PIN_PB1 (GPIO_PIOB_BASE + 1)
+#define GPIO_PIN_PB2 (GPIO_PIOB_BASE + 2)
+#define GPIO_PIN_PB3 (GPIO_PIOB_BASE + 3)
+#define GPIO_PIN_PB4 (GPIO_PIOB_BASE + 4)
+#define GPIO_PIN_PB5 (GPIO_PIOB_BASE + 5)
+#define GPIO_PIN_PB6 (GPIO_PIOB_BASE + 6)
+#define GPIO_PIN_PB7 (GPIO_PIOB_BASE + 7)
+#define GPIO_PIN_PB8 (GPIO_PIOB_BASE + 8)
+#define GPIO_PIN_PB9 (GPIO_PIOB_BASE + 9)
+#define GPIO_PIN_PB10 (GPIO_PIOB_BASE + 10)
+#define GPIO_PIN_PB11 (GPIO_PIOB_BASE + 11)
+#define GPIO_PIN_PB12 (GPIO_PIOB_BASE + 12)
+#define GPIO_PIN_PB13 (GPIO_PIOB_BASE + 13)
+#define GPIO_PIN_PB14 (GPIO_PIOB_BASE + 14)
+#define GPIO_PIN_PB15 (GPIO_PIOB_BASE + 15)
+#define GPIO_PIN_PB16 (GPIO_PIOB_BASE + 16)
+#define GPIO_PIN_PB17 (GPIO_PIOB_BASE + 17)
+#define GPIO_PIN_PB18 (GPIO_PIOB_BASE + 18)
+#define GPIO_PIN_PB19 (GPIO_PIOB_BASE + 19)
+#define GPIO_PIN_PB20 (GPIO_PIOB_BASE + 20)
+#define GPIO_PIN_PB21 (GPIO_PIOB_BASE + 21)
+#define GPIO_PIN_PB22 (GPIO_PIOB_BASE + 22)
+#define GPIO_PIN_PB23 (GPIO_PIOB_BASE + 23)
+#define GPIO_PIN_PB24 (GPIO_PIOB_BASE + 24)
+#define GPIO_PIN_PB25 (GPIO_PIOB_BASE + 25)
+#define GPIO_PIN_PB26 (GPIO_PIOB_BASE + 26)
+#define GPIO_PIN_PB27 (GPIO_PIOB_BASE + 27)
+#define GPIO_PIN_PB28 (GPIO_PIOB_BASE + 28)
+#define GPIO_PIN_PB29 (GPIO_PIOB_BASE + 29)
+#define GPIO_PIN_PB30 (GPIO_PIOB_BASE + 30)
+
+#define GPIO_PIOC_BASE (GPIO_PIOB_BASE + 32)
+#define GPIO_PIN_PC0 (GPIO_PIOC_BASE + 0)
+#define GPIO_PIN_PC1 (GPIO_PIOC_BASE + 1)
+#define GPIO_PIN_PC2 (GPIO_PIOC_BASE + 2)
+#define GPIO_PIN_PC3 (GPIO_PIOC_BASE + 3)
+#define GPIO_PIN_PC4 (GPIO_PIOC_BASE + 4)
+#define GPIO_PIN_PC5 (GPIO_PIOC_BASE + 5)
+#define GPIO_PIN_PC6 (GPIO_PIOC_BASE + 6)
+#define GPIO_PIN_PC7 (GPIO_PIOC_BASE + 7)
+#define GPIO_PIN_PC8 (GPIO_PIOC_BASE + 8)
+#define GPIO_PIN_PC9 (GPIO_PIOC_BASE + 9)
+#define GPIO_PIN_PC10 (GPIO_PIOC_BASE + 10)
+#define GPIO_PIN_PC11 (GPIO_PIOC_BASE + 11)
+#define GPIO_PIN_PC12 (GPIO_PIOC_BASE + 12)
+#define GPIO_PIN_PC13 (GPIO_PIOC_BASE + 13)
+#define GPIO_PIN_PC14 (GPIO_PIOC_BASE + 14)
+#define GPIO_PIN_PC15 (GPIO_PIOC_BASE + 15)
+#define GPIO_PIN_PC16 (GPIO_PIOC_BASE + 16)
+#define GPIO_PIN_PC17 (GPIO_PIOC_BASE + 17)
+#define GPIO_PIN_PC18 (GPIO_PIOC_BASE + 18)
+#define GPIO_PIN_PC19 (GPIO_PIOC_BASE + 19)
+#define GPIO_PIN_PC20 (GPIO_PIOC_BASE + 20)
+#define GPIO_PIN_PC21 (GPIO_PIOC_BASE + 21)
+#define GPIO_PIN_PC22 (GPIO_PIOC_BASE + 22)
+#define GPIO_PIN_PC23 (GPIO_PIOC_BASE + 23)
+#define GPIO_PIN_PC24 (GPIO_PIOC_BASE + 24)
+#define GPIO_PIN_PC25 (GPIO_PIOC_BASE + 25)
+#define GPIO_PIN_PC26 (GPIO_PIOC_BASE + 26)
+#define GPIO_PIN_PC27 (GPIO_PIOC_BASE + 27)
+#define GPIO_PIN_PC28 (GPIO_PIOC_BASE + 28)
+#define GPIO_PIN_PC29 (GPIO_PIOC_BASE + 29)
+#define GPIO_PIN_PC30 (GPIO_PIOC_BASE + 30)
+#define GPIO_PIN_PC31 (GPIO_PIOC_BASE + 31)
+
+#define GPIO_PIOD_BASE (GPIO_PIOC_BASE + 32)
+#define GPIO_PIN_PD0 (GPIO_PIOD_BASE + 0)
+#define GPIO_PIN_PD1 (GPIO_PIOD_BASE + 1)
+#define GPIO_PIN_PD2 (GPIO_PIOD_BASE + 2)
+#define GPIO_PIN_PD3 (GPIO_PIOD_BASE + 3)
+#define GPIO_PIN_PD4 (GPIO_PIOD_BASE + 4)
+#define GPIO_PIN_PD5 (GPIO_PIOD_BASE + 5)
+#define GPIO_PIN_PD6 (GPIO_PIOD_BASE + 6)
+#define GPIO_PIN_PD7 (GPIO_PIOD_BASE + 7)
+#define GPIO_PIN_PD8 (GPIO_PIOD_BASE + 8)
+#define GPIO_PIN_PD9 (GPIO_PIOD_BASE + 9)
+#define GPIO_PIN_PD10 (GPIO_PIOD_BASE + 10)
+#define GPIO_PIN_PD11 (GPIO_PIOD_BASE + 11)
+#define GPIO_PIN_PD12 (GPIO_PIOD_BASE + 12)
+#define GPIO_PIN_PD13 (GPIO_PIOD_BASE + 13)
+#define GPIO_PIN_PD14 (GPIO_PIOD_BASE + 14)
+#define GPIO_PIN_PD15 (GPIO_PIOD_BASE + 15)
+#define GPIO_PIN_PD16 (GPIO_PIOD_BASE + 16)
+#define GPIO_PIN_PD17 (GPIO_PIOD_BASE + 17)
+
+/*
+ * Technically, PIOE is also available, but we'll leave it alone since
+ * the SDRAM interface depends on it.
+ */
+
+#endif /* __ASM_ARCH_AT32AP7000_H */
Index: linux-2.6.19-rc4-mm2/include/asm-avr32/arch-at32ap/gpio.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.19-rc4-mm2/include/asm-avr32/arch-at32ap/gpio.h 2006-11-07 10:27:01.000000000 +0100
@@ -0,0 +1,14 @@
+#ifndef __ASM_AVR32_GPIO_H
+#define __ASM_AVR32_GPIO_H
+
+void portmux_select_peripheral(unsigned int pin, unsigned int function);
+
+int request_gpio(unsigned int pin);
+void free_gpio(unsigned int pin);
+
+void gpio_set_value(unsigned int pin, int value);
+int gpio_get_value(unsigned int pin);
+void gpio_set_output_enable(unsigned int pin, int enabled);
+void gpio_set_pullup_enable(unsigned int pin, int enabled);
+
+#endif /* __ASM_AVR32_GPIO_H */
Index: linux-2.6.19-rc4-mm2/include/asm-avr32/arch-at32ap/irq.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.19-rc4-mm2/include/asm-avr32/arch-at32ap/irq.h 2006-11-07 10:27:01.000000000 +0100
@@ -0,0 +1,11 @@
+#ifndef __ASM_AVR32_ARCH_IRQ_H
+#define __ASM_AVR32_ARCH_IRQ_H
+
+#define EXTERNAL_IRQ_BASE NR_INTERNAL_IRQS
+#define NR_EXTERNAL_IRQS 32
+#define GPIO_IRQ_BASE (EXTERNAL_IRQ_BASE + NR_EXTERNAL_IRQS)
+#define NR_GPIO_IRQS (4 * 32)
+
+#define NR_IRQS (GPIO_IRQ_BASE + NR_GPIO_IRQS)
+
+#endif /* __ASM_AVR32_ARCH_IRQ_H */
Index: linux-2.6.19-rc4-mm2/include/asm-avr32/arch-at32ap/portmux.h
===================================================================
--- linux-2.6.19-rc4-mm2.orig/include/asm-avr32/arch-at32ap/portmux.h 2006-11-07 10:26:59.000000000 +0100
+++ /dev/null 1970-01-01 00:00:00.000000000 +0000
@@ -1,16 +0,0 @@
-/*
- * AT32 portmux interface.
- *
- * Copyright (C) 2006 Atmel Corporation
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#ifndef __ASM_AVR32_AT32_PORTMUX_H__
-#define __ASM_AVR32_AT32_PORTMUX_H__
-
-void portmux_set_func(unsigned int portmux_id, unsigned int pin_id,
- unsigned int function_id);
-
-#endif /* __ASM_AVR32_AT32_PORTMUX_H__ */
Index: linux-2.6.19-rc4-mm2/include/asm-avr32/irq.h
===================================================================
--- linux-2.6.19-rc4-mm2.orig/include/asm-avr32/irq.h 2006-11-07 10:26:59.000000000 +0100
+++ linux-2.6.19-rc4-mm2/include/asm-avr32/irq.h 2006-11-07 10:27:01.000000000 +0100
@@ -2,8 +2,12 @@
#define __ASM_AVR32_IRQ_H

#define NR_INTERNAL_IRQS 64
-#define NR_EXTERNAL_IRQS 64
-#define NR_IRQS (NR_INTERNAL_IRQS + NR_EXTERNAL_IRQS)
+
+#include <asm/arch/irq.h>
+
+#ifndef NR_IRQS
+#define NR_IRQS (NR_INTERNAL_IRQS)
+#endif

#define irq_canonicalize(i) (i)

2006-11-07 11:42:11

by Haavard Skinnemoen

[permalink] [raw]
Subject: [-mm patch 3/4] AVR32: Move spi device definitions into main board setup file

There's no point in having a separate file just to set up the board-
specific data for spi. By moving it into the rest of the board-
specific setup code, we can also make sure that the data is
registered before we register the spi master controller.

This patch also records the GPIO pin to use as chip select in the
controller_data member of the spi_board_info data for each device.

Signed-off-by: Haavard Skinnemoen <[email protected]>
---
arch/avr32/boards/atstk1000/Makefile | 2 +-
arch/avr32/boards/atstk1000/atstk1002.c | 15 +++++++++++++++
arch/avr32/boards/atstk1000/spi.c | 27 ---------------------------
3 files changed, 16 insertions(+), 28 deletions(-)

Index: linux-2.6.19-rc4-mm2/arch/avr32/boards/atstk1000/Makefile
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/avr32/boards/atstk1000/Makefile 2006-11-07 11:29:15.000000000 +0100
+++ linux-2.6.19-rc4-mm2/arch/avr32/boards/atstk1000/Makefile 2006-11-07 11:31:00.000000000 +0100
@@ -1,2 +1,2 @@
-obj-y += setup.o spi.o flash.o
+obj-y += setup.o flash.o
obj-$(CONFIG_BOARD_ATSTK1002) += atstk1002.o
Index: linux-2.6.19-rc4-mm2/arch/avr32/boards/atstk1000/atstk1002.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/avr32/boards/atstk1000/atstk1002.c 2006-11-07 11:29:15.000000000 +0100
+++ linux-2.6.19-rc4-mm2/arch/avr32/boards/atstk1000/atstk1002.c 2006-11-07 11:33:09.000000000 +0100
@@ -7,11 +7,24 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+#include <linux/device.h>
#include <linux/init.h>
+#include <linux/spi/spi.h>

+#include <asm/arch/at32ap7000.h>
#include <asm/arch/board.h>
#include <asm/arch/init.h>

+static struct spi_board_info spi_board_info[] __initdata = {
+ {
+ .modalias = "ltv350qv",
+ .controller_data = (void *)GPIO_PIN_PA4,
+ .max_speed_hz = 16000000,
+ .bus_num = 0,
+ .chip_select = 1,
+ },
+};
+
struct eth_platform_data __initdata eth0_data = {
.valid = 1,
.mii_phy_addr = 0x10,
@@ -39,6 +52,8 @@ static int __init atstk1002_init(void)
at32_add_device_usart(2);

at32_add_device_eth(0, &eth0_data);
+
+ spi_register_board_info(spi_board_info, ARRAY_SIZE(spi_board_info));
at32_add_device_spi(0);
at32_add_device_lcdc(0, &atstk1000_fb0_data);

Index: linux-2.6.19-rc4-mm2/arch/avr32/boards/atstk1000/spi.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/avr32/boards/atstk1000/spi.c 2006-11-07 11:29:15.000000000 +0100
+++ /dev/null 1970-01-01 00:00:00.000000000 +0000
@@ -1,27 +0,0 @@
-/*
- * ATSTK1000 SPI devices
- *
- * Copyright (C) 2005 Atmel Norway
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/device.h>
-#include <linux/spi/spi.h>
-
-static struct spi_board_info spi_board_info[] __initdata = {
- {
- .modalias = "ltv350qv",
- .max_speed_hz = 16000000,
- .bus_num = 0,
- .chip_select = 1,
- },
-};
-
-static int board_init_spi(void)
-{
- spi_register_board_info(spi_board_info, ARRAY_SIZE(spi_board_info));
- return 0;
-}
-arch_initcall(board_init_spi);

2006-11-07 11:42:58

by Haavard Skinnemoen

[permalink] [raw]
Subject: [-mm patch 4/4] Atmel SPI driver

SPI master driver for the Atmel AT32/AT91 on-chip SPI controller. This
incarnation of the driver ignores the automatic chip select logic in
the controller and controls the chip selects "manually" using gpio. It
also takes care of a few DMA-related pitfalls like misaligned buffers
or missing rx buffer by using a temporary per-device DMA buffer and
copying as necessary.

The DMA buffer code is a bit defensive in that it waits for each
transfer to run to completion before starting the next, so the
performance is a little disappointing. I want it to work on most/all
at32 and at91 controllers before optimizing this.

Signed-off-by: Haavard Skinnemoen <[email protected]>
---
drivers/spi/Kconfig | 7
drivers/spi/Makefile | 1
drivers/spi/atmel_spi.c | 681 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/spi/atmel_spi.h | 167 +++++++++++
4 files changed, 856 insertions(+)

Index: linux-2.6.19-rc4-mm2/drivers/spi/Kconfig
===================================================================
--- linux-2.6.19-rc4-mm2.orig/drivers/spi/Kconfig 2006-11-07 10:34:52.000000000 +0100
+++ linux-2.6.19-rc4-mm2/drivers/spi/Kconfig 2006-11-07 10:38:58.000000000 +0100
@@ -51,6 +51,13 @@ config SPI_MASTER
comment "SPI Master Controller Drivers"
depends on SPI_MASTER

+config SPI_ATMEL
+ tristate "Atmel SPI Controller"
+ depends on (ARCH_AT91 || AVR32) && SPI_MASTER
+ help
+ This selects a driver for the Atmel SPI Controller, present on
+ many AT32 (AVR32) and AT91 (ARM) chips.
+
config SPI_BITBANG
tristate "Bitbanging SPI master"
depends on SPI_MASTER && EXPERIMENTAL
Index: linux-2.6.19-rc4-mm2/drivers/spi/Makefile
===================================================================
--- linux-2.6.19-rc4-mm2.orig/drivers/spi/Makefile 2006-11-07 10:34:52.000000000 +0100
+++ linux-2.6.19-rc4-mm2/drivers/spi/Makefile 2006-11-07 10:38:58.000000000 +0100
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_MASTER) += spi.o

# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o
+obj-$(CONFIG_SPI_ATMEL) += atmel_spi.o
obj-$(CONFIG_SPI_BUTTERFLY) += spi_butterfly.o
obj-$(CONFIG_SPI_PXA2XX) += pxa2xx_spi.o
obj-$(CONFIG_SPI_MPC83xx) += spi_mpc83xx.o
Index: linux-2.6.19-rc4-mm2/drivers/spi/atmel_spi.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.19-rc4-mm2/drivers/spi/atmel_spi.c 2006-11-07 11:12:12.000000000 +0100
@@ -0,0 +1,681 @@
+/*
+ * Driver for Atmel AT32 and AT91 SPI Controllers
+ *
+ * Copyright (C) 2006 Atmel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/spi/spi.h>
+
+#include <asm/io.h>
+#include <asm/arch/board.h>
+#include <asm/arch/gpio.h>
+
+#include "atmel_spi.h"
+
+/*
+ * The core SPI transfer engine just talks to a register bank to set up
+ * DMA transfers; transfer queue progress is driven by IRQs. The clock
+ * framework provides the base clock, subdivided for each spi_device.
+ *
+ * Newer controllers, marked with "new_1" flag, have:
+ * - CR.LASTXFER
+ * - SPI_MR.DIV32 may become FDIV or must-be-zero (here: always zero)
+ * - SPI_SR.TXEMPTY, SPI_SR.NSSR (and corresponding irqs)
+ * - SPI_CSRx.CSAAT
+ * - SPI_CSRx.SBCR allows faster clocking
+ */
+struct atmel_spi {
+ spinlock_t lock;
+
+ void __iomem *regs;
+ int irq;
+ struct clk *clk;
+ struct platform_device *pdev;
+ unsigned new_1:1;
+
+ u8 stopping;
+ struct list_head queue;
+ struct spi_transfer *current_transfer;
+ unsigned long remaining_bytes;
+
+ void *buffer;
+ dma_addr_t buffer_dma;
+};
+
+#define BUFFER_SIZE PAGE_SIZE
+#define INVALID_DMA_ADDRESS 0xffffffff
+
+/*
+ * TODO: We really want to use the same GPIO API on both architectures.
+ */
+#ifdef CONFIG_ARM
+
+static inline int request_gpio(unsigned int pin)
+{
+ return 0;
+}
+
+static inline void free_gpio(unsigned int pin)
+{
+
+}
+
+static inline void gpio_set_value(unsigned int pin, int value)
+{
+ at91_set_gpio_value(pin, value);
+}
+
+static inline void gpio_set_output_enable(unsigned int pin, int enabled)
+{
+ at91_set_gpio_output(pin, enabled);
+}
+
+static inline void gpio_set_pullup_enable(unsigned int pin, int enabled)
+{
+
+}
+#endif
+
+/*
+ * Earlier SPI controllers (e.g. on at91rm9200) have a design bug whereby
+ * they assume that spi slave device state will not change on deselect, so
+ * that automagic deselection is OK. Not so! Workaround uses nCSx pins
+ * as GPIOs; or newer controllers have CSAAT and friends.
+ *
+ * Since the CSAAT functionality is a bit weird on newer controllers
+ * as well, we use GPIO to control nCSx pins on all controllers.
+ */
+
+static inline void cs_activate(struct spi_device *spi)
+{
+ unsigned gpio = (unsigned) spi->controller_data;
+
+ dev_dbg(&spi->dev, "activate %u\n", gpio);
+ gpio_set_value(gpio, 0);
+}
+
+static inline void cs_deactivate(struct spi_device *spi)
+{
+ unsigned gpio = (unsigned) spi->controller_data;
+
+ dev_dbg(&spi->dev, "DEactivate %u\n", gpio);
+ gpio_set_value(gpio, 1);
+}
+
+/*
+ * Submit next transfer for DMA.
+ * lock is held, spi irq is blocked
+ */
+static void atmel_spi_next_xfer(struct spi_master *master,
+ struct spi_message *msg)
+{
+ struct atmel_spi *as = spi_master_get_devdata(master);
+ struct spi_transfer *xfer;
+ u32 imr = 0;
+ u32 len;
+ dma_addr_t tx_dma, rx_dma;
+
+ xfer = as->current_transfer;
+ if (!xfer || as->remaining_bytes == 0) {
+ if (xfer)
+ xfer = list_entry(xfer->transfer_list.next,
+ struct spi_transfer, transfer_list);
+ else
+ xfer = list_entry(msg->transfers.next, struct spi_transfer,
+ transfer_list);
+ as->remaining_bytes = xfer->len;
+ as->current_transfer = xfer;
+ }
+
+ len = as->remaining_bytes;
+
+ tx_dma = xfer->tx_dma;
+ rx_dma = xfer->rx_dma;
+
+ if (rx_dma == INVALID_DMA_ADDRESS) {
+ rx_dma = as->buffer_dma;
+ if (len > BUFFER_SIZE)
+ len = BUFFER_SIZE;
+ }
+ if (tx_dma == INVALID_DMA_ADDRESS) {
+ if (xfer->tx_buf) {
+ tx_dma = as->buffer_dma;
+ if (len > BUFFER_SIZE)
+ len = BUFFER_SIZE;
+ memcpy(as->buffer, xfer->tx_buf, len);
+ dma_sync_single_for_device(&as->pdev->dev,
+ as->buffer_dma, len,
+ DMA_TO_DEVICE);
+ } else {
+ /* Send undefined data; rx_dma is handy */
+ tx_dma = rx_dma;
+ }
+ }
+
+ spi_writel(as, RPR, rx_dma);
+ spi_writel(as, TPR, tx_dma);
+
+ as->remaining_bytes -= len;
+ if (msg->spi->bits_per_word > 8)
+ len >>= 1;
+
+ /* REVISIT: when xfer->delay_usecs == 0, the PDC "next transfer"
+ * mechanism might help avoid the IRQ latency between transfers
+ *
+ * We're also waiting for ENDRX before we start the next
+ * transfer because we need to handle some difficult timing
+ * issues otherwise. If we wait for ENDTX in one transfer and
+ * then starts waiting for ENDRX in the next, it's difficult
+ * to tell the difference between the ENDRX interrupt we're
+ * actually waiting for and the ENDRX interrupt of the
+ * previous transfer.
+ *
+ * It should be doable, though. Just not now...
+ */
+ spi_writel(as, TNCR, 0);
+ spi_writel(as, RNCR, 0);
+ imr = SPI_BIT(ENDRX);
+
+ dev_dbg(&msg->spi->dev,
+ "start xfer %p: len %u tx %p/%08x rx %p/%08x imr %08x\n",
+ xfer, xfer->len, xfer->tx_buf, xfer->tx_dma,
+ xfer->rx_buf, xfer->rx_dma, imr);
+
+ wmb();
+ spi_writel(as, TCR, len);
+ spi_writel(as, RCR, len);
+ spi_writel(as, PTCR, SPI_BIT(TXTEN) | SPI_BIT(RXTEN));
+ spi_writel(as, IER, imr);
+}
+
+static void atmel_spi_next_message(struct spi_master *master)
+{
+ struct atmel_spi *as = spi_master_get_devdata(master);
+ struct spi_message *msg;
+ u32 mr;
+
+ BUG_ON(as->current_transfer);
+
+ msg = list_entry(as->queue.next, struct spi_message, queue);
+
+ /* Select the chip */
+ mr = spi_readl(as, MR);
+ mr = SPI_BFINS(PCS, ~(1 << msg->spi->chip_select), mr);
+ spi_writel(as, MR, mr);
+ cs_activate(msg->spi);
+
+ atmel_spi_next_xfer(master, msg);
+}
+
+static void atmel_spi_dma_map_xfer(struct atmel_spi *as,
+ struct spi_transfer *xfer)
+{
+ xfer->tx_dma = xfer->rx_dma = INVALID_DMA_ADDRESS;
+ if (!(xfer->len & (L1_CACHE_BYTES - 1))) {
+ if (xfer->tx_buf
+ && !((unsigned long)xfer->tx_buf & (L1_CACHE_BYTES - 1)))
+ xfer->tx_dma = dma_map_single(&as->pdev->dev,
+ xfer->tx_buf,
+ xfer->len,
+ DMA_TO_DEVICE);
+ if (xfer->rx_buf
+ && !((unsigned long)xfer->rx_buf & (L1_CACHE_BYTES - 1)))
+ xfer->rx_dma = dma_map_single(&as->pdev->dev,
+ xfer->rx_buf,
+ xfer->len,
+ DMA_FROM_DEVICE);
+ }
+}
+
+static irqreturn_t
+atmel_spi_interrupt(int irq, void *dev_id)
+{
+ struct spi_master *master = dev_id;
+ struct atmel_spi *as = spi_master_get_devdata(master);
+ struct spi_message *msg;
+ struct spi_transfer *xfer;
+ u32 status, pending, imr;
+ int ret = IRQ_NONE;
+
+ imr = spi_readl(as, IMR);
+ status = spi_readl(as, SR);
+ pending = status & imr;
+pr_debug("spi irq: stat %05x imr %05x pend %05x\n", status, imr, pending);
+
+ if (pending & (SPI_BIT(ENDTX) | SPI_BIT(ENDRX))) {
+ ret = IRQ_HANDLED;
+
+ spi_writel(as, IDR, pending);
+ spin_lock(&as->lock);
+
+ xfer = as->current_transfer;
+ msg = list_entry(as->queue.next, struct spi_message, queue);
+
+ /*
+ * If the rx buffer wasn't aligned, we used a bounce
+ * buffer for the transfer. Copy the data back and
+ * make the bounce buffer ready for re-use.
+ */
+ if (xfer->rx_buf && xfer->rx_dma == INVALID_DMA_ADDRESS) {
+ unsigned int len = xfer->len;
+ if (len > BUFFER_SIZE)
+ len = BUFFER_SIZE;
+
+ dma_sync_single_for_cpu(&as->pdev->dev, as->buffer_dma,
+ len, DMA_FROM_DEVICE);
+ memcpy((xfer->rx_buf + xfer->len
+ - len - as->remaining_bytes),
+ as->buffer, len);
+ }
+
+
+ if (as->remaining_bytes == 0) {
+ msg->actual_length += xfer->len;
+
+ if (!msg->is_dma_mapped) {
+ if (xfer->tx_dma != INVALID_DMA_ADDRESS)
+ dma_unmap_single(master->cdev.dev,
+ xfer->tx_dma,
+ xfer->len,
+ DMA_TO_DEVICE);
+ if (xfer->rx_dma != INVALID_DMA_ADDRESS)
+ dma_unmap_single(master->cdev.dev,
+ xfer->rx_dma,
+ xfer->len,
+ DMA_FROM_DEVICE);
+ }
+
+ /* REVISIT: udelay in irq is unfriendly */
+ if (xfer->delay_usecs)
+ udelay(xfer->delay_usecs);
+
+ if (msg->transfers.prev == &xfer->transfer_list) {
+
+ /* report completed message */
+ cs_deactivate(msg->spi);
+ list_del(&msg->queue);
+ msg->status = 0;
+
+ dev_dbg(master->cdev.dev,
+ "xfer complete: %u bytes transferred\n",
+ msg->actual_length);
+
+ spin_unlock(&as->lock);
+ msg->complete(msg->context);
+ spin_lock(&as->lock);
+
+ as->current_transfer = NULL;
+
+ /* continue; complete() may have queued requests */
+ if (list_empty(&as->queue) || as->stopping)
+ spi_writel(as, PTCR, SPI_BIT(RXTDIS)
+ | SPI_BIT(TXTDIS));
+ else
+ atmel_spi_next_message(master);
+ } else {
+ if (xfer->cs_change) {
+ cs_deactivate(msg->spi);
+ udelay(1);
+ cs_activate(msg->spi);
+ }
+
+ /*
+ * Not done yet. Submit the next transfer.
+ *
+ * FIXME handle protocol options for xfer
+ */
+ atmel_spi_next_xfer(master, msg);
+ }
+ } else {
+ /*
+ * Keep going, we still have data to send in
+ * the current transfer.
+ */
+ atmel_spi_next_xfer(master, msg);
+ }
+ spin_unlock(&as->lock);
+ }
+
+ return ret;
+}
+
+static int atmel_spi_setup(struct spi_device *spi)
+{
+ struct atmel_spi *as;
+ u32 scbr, csr;
+ unsigned int bits = spi->bits_per_word;
+ unsigned long bus_hz, sck_hz;
+ unsigned int npcs_pin;
+ int ret;
+
+ as = spi_master_get_devdata(spi->master);
+
+ if (as->stopping)
+ return -ESHUTDOWN;
+
+ if (spi->chip_select > spi->master->num_chipselect) {
+ dev_dbg(&spi->dev,
+ "setup: invalid chipselect %u (%u defined)\n",
+ spi->chip_select, spi->master->num_chipselect);
+ return -EINVAL;
+ }
+
+ if (bits == 0)
+ bits = 8;
+ if (bits < 8 || bits > 16) {
+ dev_dbg(&spi->dev,
+ "setup: invalid bits_per_word %u (8 to 16)\n",
+ bits);
+ return -EINVAL;
+ }
+
+ if (spi->mode & (SPI_CS_HIGH | SPI_LSB_FIRST)) {
+ dev_dbg(&spi->dev, "setup: unsupported mode %u\n", spi->mode);
+ return -EINVAL;
+ }
+
+ /* speed zero convention is used by some upper layers */
+ bus_hz = clk_get_rate(as->clk);
+ if (spi->max_speed_hz) {
+ /* assume div32/fdiv/mbz == 0 */
+ if (!as->new_1)
+ bus_hz /= 2;
+ scbr = ((bus_hz + spi->max_speed_hz - 1)
+ / spi->max_speed_hz);
+ if (scbr >= (1 << SPI_SCBR_SIZE)) {
+ dev_dbg(&spi->dev, "setup: %d Hz too slow, scbr %u\n",
+ spi->max_speed_hz, scbr);
+ return -EINVAL;
+ }
+ } else
+ scbr = 0xff;
+ sck_hz = bus_hz / scbr;
+
+ csr = SPI_BF(SCBR, scbr) | SPI_BF(BITS, bits - 8);
+ if (spi->mode & SPI_CPOL)
+ csr |= SPI_BIT(CPOL);
+ if (!(spi->mode & SPI_CPHA))
+ csr |= SPI_BIT(NCPHA);
+
+ /* TODO: DLYBS and DLYBCT */
+ csr |= SPI_BF(DLYBS, 10);
+ csr |= SPI_BF(DLYBCT, 10);
+
+ npcs_pin = (unsigned int)spi->controller_data;
+ if (!spi->controller_state) {
+ ret = request_gpio(npcs_pin);
+ if (ret)
+ return ret;
+ spi->controller_state = (void *)npcs_pin;
+ }
+
+ gpio_set_value(npcs_pin, 1);
+ gpio_set_output_enable(npcs_pin, 1);
+ gpio_set_pullup_enable(npcs_pin, 0);
+
+ dev_dbg(&spi->dev,
+ "setup: %lu Hz bpw %u mode 0x%x -> csr%d %08x\n",
+ sck_hz, bits, spi->mode, spi->chip_select, csr);
+
+ spi_writel(as, CSR0 + 4 * spi->chip_select, csr);
+
+ return 0;
+}
+
+static int atmel_spi_transfer(struct spi_device *spi, struct spi_message *msg)
+{
+ struct atmel_spi *as;
+ struct spi_transfer *xfer;
+ unsigned long flags;
+ struct device *controller = spi->master->cdev.dev;
+
+ as = spi_master_get_devdata(spi->master);
+
+ dev_dbg(controller, "new message %p submitted for %s\n",
+ msg, spi->dev.bus_id);
+
+ if (unlikely(list_empty(&msg->transfers)
+ || !spi->max_speed_hz))
+ return -EINVAL;
+
+ if (as->stopping)
+ return -ESHUTDOWN;
+
+ list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+ if (!(xfer->tx_buf || xfer->rx_buf)) {
+ dev_dbg(&spi->dev, "missing rx or tx buf\n");
+ return -EINVAL;
+ }
+
+ /* FIXME implement these protocol options!! */
+ if (xfer->bits_per_word || xfer->speed_hz) {
+ dev_dbg(&spi->dev, "no protocol options yet\n");
+ return -ENOPROTOOPT;
+ }
+ }
+
+ /* scrub dcache "early" */
+ if (!msg->is_dma_mapped) {
+ list_for_each_entry(xfer, &msg->transfers, transfer_list)
+ atmel_spi_dma_map_xfer(as, xfer);
+ }
+
+ list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+ dev_dbg(controller,
+ " xfer %p: len %u tx %p/%08x rx %p/%08x\n",
+ xfer, xfer->len,
+ xfer->tx_buf, xfer->tx_dma,
+ xfer->rx_buf, xfer->rx_dma);
+ }
+
+ msg->status = -EINPROGRESS;
+ msg->actual_length = 0;
+
+ spin_lock_irqsave(&as->lock, flags);
+ list_add_tail(&msg->queue, &as->queue);
+ if (!as->current_transfer)
+ atmel_spi_next_message(spi->master);
+ spin_unlock_irqrestore(&as->lock, flags);
+
+ return 0;
+}
+
+static void atmel_spi_cleanup(const struct spi_device *spi)
+{
+ if (spi->controller_state)
+ free_gpio((unsigned int)spi->controller_data);
+}
+
+/*-------------------------------------------------------------------------*/
+
+static int __devinit atmel_spi_probe(struct platform_device *pdev)
+{
+ struct resource *regs;
+ int irq;
+ struct clk *clk;
+ int ret;
+ struct spi_master *master;
+ struct atmel_spi *as;
+
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!regs)
+ return -ENXIO;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ clk = clk_get(&pdev->dev, "pclk");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ /* setup spi core then atmel-specific driver state */
+ ret = -ENOMEM;
+ master = spi_alloc_master(&pdev->dev, sizeof *as);
+ if (!master)
+ goto out_free;
+
+ master->bus_num = pdev->id;
+ master->num_chipselect = 4;
+ master->setup = atmel_spi_setup;
+ master->transfer = atmel_spi_transfer;
+ master->cleanup = atmel_spi_cleanup;
+ platform_set_drvdata(pdev, master);
+
+ as = spi_master_get_devdata(master);
+
+ as->buffer = dma_alloc_coherent(&pdev->dev, BUFFER_SIZE,
+ &as->buffer_dma, GFP_KERNEL);
+ if (!as->buffer)
+ goto out_free;
+
+ spin_lock_init(&as->lock);
+ INIT_LIST_HEAD(&as->queue);
+ as->pdev = pdev;
+ as->regs = ioremap(regs->start, (regs->end - regs->start) + 1);
+ if (!as->regs)
+ goto out_free_buffer;
+ as->irq = irq;
+ as->clk = clk;
+#if !defined(CONFIG_ARCH_AT91RM9200)
+ /* if (!cpu_is_at91rm9200()) */
+ as->new_1 = 1;
+#endif
+
+ ret = request_irq(irq, atmel_spi_interrupt, 0,
+ pdev->dev.bus_id, master);
+ if (ret)
+ goto out_unmap_regs;
+
+ /* Initialize the hardware */
+ clk_enable(clk);
+ spi_writel(as, CR, SPI_BIT(SWRST));
+ spi_writel(as, MR, SPI_BIT(MSTR) | SPI_BIT(MODFDIS));
+ spi_writel(as, PTCR, SPI_BIT(RXTDIS) | SPI_BIT(TXTDIS));
+ spi_writel(as, CR, SPI_BIT(SPIEN));
+
+ /* go! */
+ dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
+ (unsigned long)regs->start, irq);
+
+ ret = spi_register_master(master);
+ if (ret)
+ goto out_reset_hw;
+
+ return 0;
+
+out_reset_hw:
+ spi_writel(as, CR, SPI_BIT(SWRST));
+ clk_disable(clk);
+ free_irq(irq, master);
+out_unmap_regs:
+ iounmap(as->regs);
+out_free_buffer:
+ dma_free_coherent(&pdev->dev, BUFFER_SIZE, as->buffer,
+ as->buffer_dma);
+out_free:
+ clk_put(clk);
+ spi_master_put(master);
+ return ret;
+}
+
+static int __devexit atmel_spi_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct atmel_spi *as = spi_master_get_devdata(master);
+ struct spi_message *msg;
+
+ /* reset the hardware and block queue progress */
+ spin_lock_irq(&as->lock);
+ as->stopping = 1;
+ spi_writel(as, CR, SPI_BIT(SWRST));
+ spi_readl(as, SR);
+ spin_unlock_irq(&as->lock);
+
+ /* Terminate remaining queued transfers */
+ list_for_each_entry(msg, &as->queue, queue) {
+ /* REVISIT unmapping the dma is sort of a NOP on ARM,
+ * but we shouldn't depend on that...
+ */
+ msg->status = -ESHUTDOWN;
+ msg->complete(msg->context);
+ }
+
+ dma_free_coherent(&pdev->dev, BUFFER_SIZE, as->buffer,
+ as->buffer_dma);
+
+ clk_disable(as->clk);
+ clk_put(as->clk);
+ free_irq(as->irq, master);
+ iounmap(as->regs);
+
+ spi_unregister_master(master);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+
+static int atmel_spi_suspend(struct platform_device *pdev, pm_message_t mesg)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct atmel_spi *as = spi_master_get_devdata(master);
+
+ clk_disable(as->clk);
+ return 0;
+}
+
+static int atmel_spi_resume(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct atmel_spi *as = spi_master_get_devdata(master);
+
+ clk_enable(as->clk);
+ return 0;
+}
+
+#else
+#define atmel_spi_suspend NULL
+#define atmel_spi_resume NULL
+#endif
+
+
+static struct platform_driver atmel_spi_driver = {
+ .driver = {
+ .name = "atmel_spi",
+ .owner = THIS_MODULE,
+ },
+ .probe = atmel_spi_probe,
+ .suspend = atmel_spi_suspend,
+ .resume = atmel_spi_resume,
+ .remove = __devexit_p(atmel_spi_remove),
+};
+
+static int __init atmel_spi_init(void)
+{
+ return platform_driver_register(&atmel_spi_driver);
+}
+module_init(atmel_spi_init);
+
+static void __exit atmel_spi_exit(void)
+{
+ platform_driver_unregister(&atmel_spi_driver);
+}
+module_exit(atmel_spi_exit);
+
+MODULE_DESCRIPTION("Atmel AT32/AT91 SPI Controller driver");
+MODULE_AUTHOR("Haavard Skinnemoen <[email protected]>");
+MODULE_LICENSE("GPL");
Index: linux-2.6.19-rc4-mm2/drivers/spi/atmel_spi.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.19-rc4-mm2/drivers/spi/atmel_spi.h 2006-11-07 10:38:58.000000000 +0100
@@ -0,0 +1,167 @@
+/*
+ * Register definitions for Atmel Serial Peripheral Interface (SPI)
+ *
+ * Copyright (C) 2006 Atmel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __ATMEL_SPI_H__
+#define __ATMEL_SPI_H__
+
+/* SPI register offsets */
+#define SPI_CR 0x0000
+#define SPI_MR 0x0004
+#define SPI_RDR 0x0008
+#define SPI_TDR 0x000c
+#define SPI_SR 0x0010
+#define SPI_IER 0x0014
+#define SPI_IDR 0x0018
+#define SPI_IMR 0x001c
+#define SPI_CSR0 0x0030
+#define SPI_CSR1 0x0034
+#define SPI_CSR2 0x0038
+#define SPI_CSR3 0x003c
+#define SPI_RPR 0x0100
+#define SPI_RCR 0x0104
+#define SPI_TPR 0x0108
+#define SPI_TCR 0x010c
+#define SPI_RNPR 0x0110
+#define SPI_RNCR 0x0114
+#define SPI_TNPR 0x0118
+#define SPI_TNCR 0x011c
+#define SPI_PTCR 0x0120
+#define SPI_PTSR 0x0124
+
+/* Bitfields in CR */
+#define SPI_SPIEN_OFFSET 0
+#define SPI_SPIEN_SIZE 1
+#define SPI_SPIDIS_OFFSET 1
+#define SPI_SPIDIS_SIZE 1
+#define SPI_SWRST_OFFSET 7
+#define SPI_SWRST_SIZE 1
+#define SPI_LASTXFER_OFFSET 24
+#define SPI_LASTXFER_SIZE 1
+
+/* Bitfields in MR */
+#define SPI_MSTR_OFFSET 0
+#define SPI_MSTR_SIZE 1
+#define SPI_PS_OFFSET 1
+#define SPI_PS_SIZE 1
+#define SPI_PCSDEC_OFFSET 2
+#define SPI_PCSDEC_SIZE 1
+#define SPI_FDIV_OFFSET 3
+#define SPI_FDIV_SIZE 1
+#define SPI_MODFDIS_OFFSET 4
+#define SPI_MODFDIS_SIZE 1
+#define SPI_LLB_OFFSET 7
+#define SPI_LLB_SIZE 1
+#define SPI_PCS_OFFSET 16
+#define SPI_PCS_SIZE 4
+#define SPI_DLYBCS_OFFSET 24
+#define SPI_DLYBCS_SIZE 8
+
+/* Bitfields in RDR */
+#define SPI_RD_OFFSET 0
+#define SPI_RD_SIZE 16
+
+/* Bitfields in TDR */
+#define SPI_TD_OFFSET 0
+#define SPI_TD_SIZE 16
+
+/* Bitfields in SR */
+#define SPI_RDRF_OFFSET 0
+#define SPI_RDRF_SIZE 1
+#define SPI_TDRE_OFFSET 1
+#define SPI_TDRE_SIZE 1
+#define SPI_MODF_OFFSET 2
+#define SPI_MODF_SIZE 1
+#define SPI_OVRES_OFFSET 3
+#define SPI_OVRES_SIZE 1
+#define SPI_ENDRX_OFFSET 4
+#define SPI_ENDRX_SIZE 1
+#define SPI_ENDTX_OFFSET 5
+#define SPI_ENDTX_SIZE 1
+#define SPI_RXBUFF_OFFSET 6
+#define SPI_RXBUFF_SIZE 1
+#define SPI_TXBUFE_OFFSET 7
+#define SPI_TXBUFE_SIZE 1
+#define SPI_NSSR_OFFSET 8
+#define SPI_NSSR_SIZE 1
+#define SPI_TXEMPTY_OFFSET 9
+#define SPI_TXEMPTY_SIZE 1
+#define SPI_SPIENS_OFFSET 16
+#define SPI_SPIENS_SIZE 1
+
+/* Bitfields in CSR0 */
+#define SPI_CPOL_OFFSET 0
+#define SPI_CPOL_SIZE 1
+#define SPI_NCPHA_OFFSET 1
+#define SPI_NCPHA_SIZE 1
+#define SPI_CSAAT_OFFSET 3
+#define SPI_CSAAT_SIZE 1
+#define SPI_BITS_OFFSET 4
+#define SPI_BITS_SIZE 4
+#define SPI_SCBR_OFFSET 8
+#define SPI_SCBR_SIZE 8
+#define SPI_DLYBS_OFFSET 16
+#define SPI_DLYBS_SIZE 8
+#define SPI_DLYBCT_OFFSET 24
+#define SPI_DLYBCT_SIZE 8
+
+/* Bitfields in RCR */
+#define SPI_RXCTR_OFFSET 0
+#define SPI_RXCTR_SIZE 16
+
+/* Bitfields in TCR */
+#define SPI_TXCTR_OFFSET 0
+#define SPI_TXCTR_SIZE 16
+
+/* Bitfields in RNCR */
+#define SPI_RXNCR_OFFSET 0
+#define SPI_RXNCR_SIZE 16
+
+/* Bitfields in TNCR */
+#define SPI_TXNCR_OFFSET 0
+#define SPI_TXNCR_SIZE 16
+
+/* Bitfields in PTCR */
+#define SPI_RXTEN_OFFSET 0
+#define SPI_RXTEN_SIZE 1
+#define SPI_RXTDIS_OFFSET 1
+#define SPI_RXTDIS_SIZE 1
+#define SPI_TXTEN_OFFSET 8
+#define SPI_TXTEN_SIZE 1
+#define SPI_TXTDIS_OFFSET 9
+#define SPI_TXTDIS_SIZE 1
+
+/* Constants for BITS */
+#define SPI_BITS_8_BPT 0
+#define SPI_BITS_9_BPT 1
+#define SPI_BITS_10_BPT 2
+#define SPI_BITS_11_BPT 3
+#define SPI_BITS_12_BPT 4
+#define SPI_BITS_13_BPT 5
+#define SPI_BITS_14_BPT 6
+#define SPI_BITS_15_BPT 7
+#define SPI_BITS_16_BPT 8
+
+/* Bit manipulation macros */
+#define SPI_BIT(name) \
+ (1 << SPI_##name##_OFFSET)
+#define SPI_BF(name,value) \
+ (((value) & ((1 << SPI_##name##_SIZE) - 1)) << SPI_##name##_OFFSET)
+#define SPI_BFEXT(name,value) \
+ (((value) >> SPI_##name##_OFFSET) & ((1 << SPI_##name##_SIZE) - 1))
+#define SPI_BFINS(name,value,old) \
+ ( ((old) & ~(((1 << SPI_##name##_SIZE) - 1) << SPI_##name##_OFFSET)) \
+ | SPI_BF(name,value))
+
+/* Register access macros */
+#define spi_readl(port,reg) \
+ readl((port)->regs + SPI_##reg)
+#define spi_writel(port,reg,value) \
+ writel((value), (port)->regs + SPI_##reg)
+
+#endif /* __ATMEL_SPI_H__ */

2006-11-07 14:13:56

by Haavard Skinnemoen

[permalink] [raw]
Subject: [-mm patch 5/4] Atmel SPI driver: MAINTAINERS entry

Add myself as maintainer of the atmel_spi driver.

Signed-off-by: Haavard Skinnemoen <[email protected]>
---
MAINTAINERS | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d708702..401fa5d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -426,6 +426,13 @@ L: [email protected]
W: http://linux-atm.sourceforge.net
S: Maintained

+ATMEL SPI DRIVER
+P: Atmel AVR32 Support Team
+M: [email protected]
+P: Haavard Skinnemoen
+M: [email protected]
+S: Supported
+
ATMEL WIRELESS DRIVER
P: Simon Kelley
M: [email protected]
--
1.4.3.2

2006-11-07 21:10:35

by Andrew Morton

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

On Tue, 7 Nov 2006 12:27:15 +0100
Haavard Skinnemoen <[email protected]> wrote:

> Add a simple GPIO framework for AVR32. This should be fairly similar
> to the AT91 GPIO API, but there are still a couple of differences:
>
> * Naming. We prefix all functions with gpio_ instead of at91_
> * request_gpio() and free_gpio() should be called to make sure
> the required pins are available, but it will still work if you
> don't.

+EXPORT_SYMBOL(request_gpio);
+EXPORT_SYMBOL(free_gpio);
+EXPORT_SYMBOL(gpio_set_value);
+EXPORT_SYMBOL(gpio_get_value);
+EXPORT_SYMBOL(gpio_set_output_enable);
+EXPORT_SYMBOL(gpio_set_pullup_enable);

I wonder about this naming choice. I'd have though that if/when the kernel
gets a generic GPIO driver or layer, these avr32-specific symbols will need
renaming.

h8300 uses h8300_free_gpio(), and there's also omap_free_gpio(). Perhaps
this patch should have added avr32_free_gpio()?

2006-11-07 21:36:16

by David Brownell

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

> +#define EXTERNAL_IRQ_BASE NR_INTERNAL_IRQS
> +#define NR_EXTERNAL_IRQS 32
> +#define GPIO_IRQ_BASE (EXTERNAL_IRQ_BASE + NR_EXTERNAL_IRQS)
> +#define NR_GPIO_IRQS (4 * 32)
> +
> +#define NR_IRQS (GPIO_IRQ_BASE + NR_GPIO_IRQS)

Did I miss something, or are the IRQs starting at GPIO_IRQ_BASE
not actually implemented? There's no irq_chip with name "GPIO"
or anything. The AT91 code should be almost a drop-in there...

- Dave

2006-11-07 22:37:47

by David Brownell

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

On Tue, 7 Nov 2006 13:10:14 -0800
quoth Andrew Morton <[email protected]>:
>
> On Tue, 7 Nov 2006 12:27:15 +0100
> Haavard Skinnemoen <[email protected]> wrote:
>
> > Add a simple GPIO framework for AVR32. This should be fairly similar
> > to the AT91 GPIO API, but there are still a couple of differences:
> >
> > * Naming. We prefix all functions with gpio_ instead of at91_
> > * request_gpio() and free_gpio() should be called to make sure
> > the required pins are available, but it will still work if you
> > don't.
>
> +EXPORT_SYMBOL(request_gpio);
> +EXPORT_SYMBOL(free_gpio);

Well, those are clearly not *prefixed* with gpio_ ... :)


> +EXPORT_SYMBOL(gpio_set_value);
> +EXPORT_SYMBOL(gpio_get_value);
> +EXPORT_SYMBOL(gpio_set_output_enable);
> +EXPORT_SYMBOL(gpio_set_pullup_enable);
>
> I wonder about this naming choice. I'd have though that if/when the kernel
> gets a generic GPIO driver or layer, these avr32-specific symbols will need
> renaming.

I'd rather just see a "convention". In some important cases, these calls
should map to a handful of instructions to read or write chip registers.
Talking about a "driver" or "layer" implies hundreds of instructions,
which means people _will_ regularly need to bypass it for bitbanging.

A convention for how to package those features would make sense, if for
no other reason than to avoid "how does _this_ platform do it?" confusion.
Most code touching GPIOs is platform-specific, but maybe not all of it...
I can see it including:

- GPIOs are identified by platform-specific unsigned integers;
in the range 0..INT_MAX (i.e. "negative" means reserved/invalid).

- "#include <asm/arch/gpio.h>" (or <asm/gpio.h> ?) providing these
calls, which may be used from IRQ handlers:

* int gpio_get_value(unsigned gpio)
... returning 0, 1, or negative errno (for invalid gpio)
* int gpio_set_value(unsigned gpio, int is_set)
... returning 0 or negative errno (for invalid gpio)
* int gpio_set_direction(unsigned gpio, int is_in /* or is_out? */)
... returning 0 or negative errno (for invalid gpio)
* int gpio_request(unsigned gpio)
... returning 0 or negative errno (for invalid gpio or "busy")
* int gpio_free(unsigned gpio)
... returning 0 or negative errno (for invalid gpio)
* and platform-specific additional mechanisms.
... like open-drain drive modes, ganged get/set, etc

That should work OK for AVR32 (by demonstration!) and many ARMs (including
OMAP, AT91, PXA, DaVinci, and more). So well that providing those calls as
inlined wrappers around existing calls would be trivial!

But it wouldn't handle the other common case of chips -- like a pcf8574 I2C
gpio expander -- providing GPIO-like functionality through message passing
infrastructure. They might need a similar API; extgpio_*() maybe. And
the common case of "use GPIO N as an IRQ" merits thought too (for both
"real" GPIOs and external ones like that pcf8574).


But pullups are not just a GPIO thing; they're a pin configuration thing
(even on AT91 and AVR32) that can apply even if the pin is not usable as
a GPIO (e.g. as on some non-Atmel platforms). So that stuff certainly
does not belong in generic infrastructure.


> h8300 uses h8300_free_gpio(), and there's also omap_free_gpio(). Perhaps
> this patch should have added avr32_free_gpio()?

If the parameters are the same -- GPIO IDs, unsigned integers with
platforms-specific semantics -- I don't see much of a point in
requiring a platform-specific convention for naming those functions.

But sticking to a consistent *prefix* convention would be healthy.
Like gpio_request()/gpio_free(), as I stuck in the list above. :)

- Dave

2006-11-08 10:30:53

by Haavard Skinnemoen

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

On Tue, 07 Nov 2006 13:36:04 -0800
David Brownell <[email protected]> wrote:

> > +#define EXTERNAL_IRQ_BASE NR_INTERNAL_IRQS
> > +#define NR_EXTERNAL_IRQS 32
> > +#define GPIO_IRQ_BASE (EXTERNAL_IRQ_BASE +
> > NR_EXTERNAL_IRQS) +#define NR_GPIO_IRQS (4 * 32)
> > +
> > +#define NR_IRQS (GPIO_IRQ_BASE +
> > NR_GPIO_IRQS)
>
> Did I miss something, or are the IRQs starting at GPIO_IRQ_BASE
> not actually implemented? There's no irq_chip with name "GPIO"
> or anything. The AT91 code should be almost a drop-in there...

No, you didn't miss anything, but I do want to implement a "GPIO"
irq_chip like at91 does. I suppose I can reduce NR_IRQS a bit until
it happens, but then again it's perhaps better to just implement the
irq_chip thing...

Haavard

2006-11-08 11:48:45

by Haavard Skinnemoen

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

On Tue, 07 Nov 2006 14:37:41 -0800
David Brownell <[email protected]> wrote:
> On Tue, 7 Nov 2006 13:10:14 -0800
> quoth Andrew Morton <[email protected]>:
> > +EXPORT_SYMBOL(request_gpio);
> > +EXPORT_SYMBOL(free_gpio);
>
> Well, those are clearly not *prefixed* with gpio_ ... :)

Heh, right. I guess I was thinking about request_irq/free_irq when I
chose those names. I'll change them.

> > +EXPORT_SYMBOL(gpio_set_value);
> > +EXPORT_SYMBOL(gpio_get_value);
> > +EXPORT_SYMBOL(gpio_set_output_enable);
> > +EXPORT_SYMBOL(gpio_set_pullup_enable);
> >
> > I wonder about this naming choice. I'd have though that if/when
> > the kernel gets a generic GPIO driver or layer, these
> > avr32-specific symbols will need renaming.
>
> I'd rather just see a "convention". In some important cases, these
> calls should map to a handful of instructions to read or write chip
> registers. Talking about a "driver" or "layer" implies hundreds of
> instructions, which means people _will_ regularly need to bypass it
> for bitbanging.

Exactly. If we can decide on what the prototypes should look like and
which header they should be in, the architecture is free to implement
it any way it chooses. If speed is more important than
chip-independence, they can even be inlines accessing the hardware
directly.

> A convention for how to package those features would make sense, if
> for no other reason than to avoid "how does _this_ platform do it?"
> confusion. Most code touching GPIOs is platform-specific, but maybe
> not all of it... I can see it including:

The SPI driver in this series is an example of an at least somewhat
platform-independent driver touching GPIOs. MMC and CompactFlash drivers
also come to mind.

> - GPIOs are identified by platform-specific unsigned integers;
> in the range 0..INT_MAX (i.e. "negative" means
> reserved/invalid).
>
> - "#include <asm/arch/gpio.h>" (or <asm/gpio.h> ?) providing these
> calls, which may be used from IRQ handlers:

Should probably be <asm/gpio.h> since not all arches actually have an
asm/arch directory. Nothing wrong with <asm/gpio.h> including
<asm/arch/gpio.h> though...

> * int gpio_get_value(unsigned gpio)
> ... returning 0, 1, or negative errno (for invalid gpio)
> * int gpio_set_value(unsigned gpio, int is_set)
> ... returning 0 or negative errno (for invalid gpio)
> * int gpio_set_direction(unsigned gpio, int is_in /* or
> is_out? */) ... returning 0 or negative errno (for invalid gpio)

I think set_output_enable makes more sense, but maybe it's just me.

> * int gpio_request(unsigned gpio)
> ... returning 0 or negative errno (for invalid gpio or
> "busy")
> * int gpio_free(unsigned gpio)
> ... returning 0 or negative errno (for invalid gpio)
> * and platform-specific additional mechanisms.
> ... like open-drain drive modes, ganged get/set, etc
>
> That should work OK for AVR32 (by demonstration!)

Definitely.

> and many ARMs
> (including OMAP, AT91, PXA, DaVinci, and more). So well that
> providing those calls as inlined wrappers around existing calls would
> be trivial!

Yes, it looks to me that most platforms do basically the same thing
with different names.

> But it wouldn't handle the other common case of chips -- like a
> pcf8574 I2C gpio expander -- providing GPIO-like functionality
> through message passing infrastructure. They might need a similar
> API; extgpio_*() maybe. And the common case of "use GPIO N as an
> IRQ" merits thought too (for both "real" GPIOs and external ones like
> that pcf8574).

Handling chips like this would need some rethinking as they can be
added dynamically, so static pin numbers won't do. Handling this could
perhaps complicate the API enough to prevent people from adopting it...

> But pullups are not just a GPIO thing; they're a pin configuration
> thing (even on AT91 and AVR32) that can apply even if the pin is not
> usable as a GPIO (e.g. as on some non-Atmel platforms). So that
> stuff certainly does not belong in generic infrastructure.

But maybe it could be done as part of the gpio_request() call, if we
specify an additional `flags' parameter? gpio_request() needs to know
about pin configuration anyway, as it needs to set up the pin for gpio.

> > h8300 uses h8300_free_gpio(), and there's also omap_free_gpio().
> > Perhaps this patch should have added avr32_free_gpio()?
>
> If the parameters are the same -- GPIO IDs, unsigned integers with
> platforms-specific semantics -- I don't see much of a point in
> requiring a platform-specific convention for naming those functions.

I'll make a few adjustments to the avr32 api to make it conform with
your proposed api, as well as rename portmux_select_peripheral to
at32_select_peripheral (since it was never really intended to be
platform-independent).

Then I'll send a patch to do the same thing for at91, if Andrew V is ok
with it.

> But sticking to a consistent *prefix* convention would be healthy.
> Like gpio_request()/gpio_free(), as I stuck in the list above. :)

Point taken ;)

Haavard

2006-11-08 18:01:33

by David Brownell

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

> > I'd rather just see a "convention". In some important cases, these
> > calls should map to a handful of instructions to read or write chip
> > registers. Talking about a "driver" or "layer" implies hundreds of
> > instructions, which means people _will_ regularly need to bypass it
> > for bitbanging.
>
> Exactly. If we can decide on what the prototypes should look like and
> which header they should be in, the architecture is free to implement
> it any way it chooses. If speed is more important than
> chip-independence, they can even be inlines accessing the hardware
> directly.

Yes. And that set of prototypes I sketched would seem to suffice.

Maybe the best way to proceed with such a thing is to change $SUBJECT
and include a doc patch, then prepare some patches to submit. The
minimal patchset would cover AVR32 and AT91. I'll light the fire
involved with the doc patch; not all concerned parties will have
been reading this thread. Later in the week, unless someone makes
a better suggestion.


> > A convention for how to package those features would make sense, if
> > for no other reason than to avoid "how does _this_ platform do it?"
> > confusion. Most code touching GPIOs is platform-specific, but maybe
> > not all of it... I can see it including:
>
> The SPI driver in this series is an example of an at least somewhat
> platform-independent driver touching GPIOs. MMC and CompactFlash drivers
> also come to mind.

Whoops, yes that's very true. It is in fact the reason why _now_ we
may actually need such a cross-platform convention, and are having
this thread ... one is needed that works on both AVR32 and AT91, but
previously the main needs have been within a single architecture.

Previously the discussions about generic GPIO support have foundered (IMO)
on lack of "real" need for such a thing, short of a userspace API (which
seems to me a low value thing, in most contexts, but which could be
layered on top of the convention we've outlined).


> > - GPIOs are identified by platform-specific unsigned integers;
> > in the range 0..INT_MAX (i.e. "negative" means
> > reserved/invalid).

Where, to be clear, "platform" includes things like which SOC chip,
which external chips on the board (many x86 south bridges have GPIO
pins, maybe not very flexible), how the FPGAs are programmed, etc.

There will also need to be a convention for mappings between the GPIO
namespace and the IRQ namespace. It's polite if that mapping is the
identity mapping, but more often they're distinct, with GPIO 10 and
IRQ 10 having the same numeric value.

So I'll propose gpio_to_irq() and irq_to_gpio() as the convention for
that mapping; negative errno returned for "no such mapping".


> > - "#include <asm/arch/gpio.h>" (or <asm/gpio.h> ?) providing these
> > calls, which may be used from IRQ handlers:
>
> Should probably be <asm/gpio.h> since not all arches actually have an
> asm/arch directory. Nothing wrong with <asm/gpio.h> including
> <asm/arch/gpio.h> though...

Right.


> > * int gpio_get_value(unsigned gpio)
> > ... returning 0, 1, or negative errno (for invalid gpio)
> > * int gpio_set_value(unsigned gpio, int is_set)
> > ... returning 0 or negative errno (for invalid gpio)

Note that all these "negative errno" returns could be eliminated if
we required the gpio to have been claimed in advance, which would
reduce the cost of those calls. Unfortunately such claim/release
calls are not currently portable cross-platform.

Also, it's going to be platform-specific whether an "output" gpio
value can be read; and if so, whether that input value matches the
selected output value.


> > * int gpio_set_direction(unsigned gpio, int is_in /* or
> > is_out? */)
> > ... returning 0 or negative errno (for invalid gpio)
>
> I think set_output_enable makes more sense, but maybe it's just me.

It's just you. :)

A "set enable" idiom is linguistically redundant too; "set" suffices,
or "enable". Both imply a need for an opposite "clear" or "disable.
"Direction" is a more obvious notion; the parameter should likely be
a symbol like GPIO_IN or GPIO_OUT.


> > * int gpio_request(unsigned gpio)
> > ... returning 0 or negative errno (for invalid gpio or
> > "busy")
> > * int gpio_free(unsigned gpio)
> > ... returning 0 or negative errno (for invalid gpio)
> > * and platform-specific additional mechanisms.
> > ... like open-drain drive modes, ganged get/set, etc
> >
> > That should work OK for AVR32 (by demonstration!)
>
> Definitely.
>
> > and many ARMs
> > (including OMAP, AT91, PXA, DaVinci, and more). So well that
> > providing those calls as inlined wrappers around existing calls would
> > be trivial!
>
> Yes, it looks to me that most platforms do basically the same thing
> with different names.

Though not all provide request/free calls, and there are those funky
"advanced" features like open drain etc.


> > But it wouldn't handle the other common case of chips -- like a
> > pcf8574 I2C gpio expander -- providing GPIO-like functionality
> > through message passing infrastructure. They might need a similar
> > API; extgpio_*() maybe. And the common case of "use GPIO N as an
> > IRQ" merits thought too (for both "real" GPIOs and external ones like
> > that pcf8574).
>
> Handling chips like this would need some rethinking as they can be
> added dynamically, so static pin numbers won't do. Handling this could
> perhaps complicate the API enough to prevent people from adopting it...

CPLD based GPIOs could be added dynamically too, but one hopes that's
not common. Such dynamic GPIOs might be better off using struct pointers
as GPIO ids, with methods (or an ops vector) in the struct and structural
addressing ... e.g. "GPIO3 on this chip". (And its chained IRQ ...)

You could do "real" GPIOs through such a "fat" API/layer, but the converse
is not true.


> > But pullups are not just a GPIO thing; they're a pin configuration
> > thing (even on AT91 and AVR32) that can apply even if the pin is not
> > usable as a GPIO (e.g. as on some non-Atmel platforms). So that
> > stuff certainly does not belong in generic infrastructure.
>
> But maybe it could be done as part of the gpio_request() call, if we
> specify an additional `flags' parameter? gpio_request() needs to know
> about pin configuration anyway, as it needs to set up the pin for gpio.

Nope, on several counts in addition to "it's not just for GPIOs":

- First, not all platforms integrate such pullups; ISTR PXA doesn't,
and DaVinci.

- Second, some platforms offer either pullups or pulldowns; newer
OMAPs offer that choice, for example.

- Third, some platforms (like OMAP1) have lots of options about
how to pin out GPIOs (e.g. OMAP 5912 GPIO7 can be on any of three
different balls, and ball R13 can be GPIO36 -or- MPUIO0). Plus
there are package options where the same "logical" pin is wired
out to different "physical" balls.

Best all around to keep those issues completely orthogonal, IMO!


> > > h8300 uses h8300_free_gpio(), and there's also omap_free_gpio().
> > > Perhaps this patch should have added avr32_free_gpio()?
> >
> > If the parameters are the same -- GPIO IDs, unsigned integers with
> > platforms-specific semantics -- I don't see much of a point in
> > requiring a platform-specific convention for naming those functions.
>
> I'll make a few adjustments to the avr32 api to make it conform with
> your proposed api, as well as rename portmux_select_peripheral to
> at32_select_peripheral (since it was never really intended to be
> platform-independent).
>
> Then I'll send a patch to do the same thing for at91, if Andrew V is ok
> with it.

Sounds good, and I'll draft a doc patch to send around to seed the
discussion about other platforms adopting the same convention.

- Dave

2006-11-08 18:58:33

by Haavard Skinnemoen

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

On Wed, 08 Nov 2006 10:00:59 -0800
David Brownell <[email protected]> wrote:

> > Exactly. If we can decide on what the prototypes should look like
> > and which header they should be in, the architecture is free to
> > implement it any way it chooses. If speed is more important than
> > chip-independence, they can even be inlines accessing the hardware
> > directly.
>
> Yes. And that set of prototypes I sketched would seem to suffice.

Indeed.

> Maybe the best way to proceed with such a thing is to change $SUBJECT
> and include a doc patch, then prepare some patches to submit. The
> minimal patchset would cover AVR32 and AT91. I'll light the fire
> involved with the doc patch; not all concerned parties will have
> been reading this thread. Later in the week, unless someone makes
> a better suggestion.

That would be great. Thanks.

> Previously the discussions about generic GPIO support have foundered
> (IMO) on lack of "real" need for such a thing, short of a userspace
> API (which seems to me a low value thing, in most contexts, but which
> could be layered on top of the convention we've outlined).

I do actually have a userspace API patch as well, using configfs, but I
want to give it some more testing internally, to see how it works out
for real applications, before submitting it. It's probably a good idea
to get the in-kernel API in place first too.

> > > - GPIOs are identified by platform-specific unsigned integers;
> > > in the range 0..INT_MAX (i.e. "negative" means
> > > reserved/invalid).
>
> Where, to be clear, "platform" includes things like which SOC chip,
> which external chips on the board (many x86 south bridges have GPIO
> pins, maybe not very flexible), how the FPGAs are programmed, etc.
>
> There will also need to be a convention for mappings between the GPIO
> namespace and the IRQ namespace. It's polite if that mapping is the
> identity mapping, but more often they're distinct, with GPIO 10 and
> IRQ 10 having the same numeric value.
>
> So I'll propose gpio_to_irq() and irq_to_gpio() as the convention for
> that mapping; negative errno returned for "no such mapping".

Good idea. For identity mappings, they can simply be inlines
introducing no overhead at all.

> > > * int gpio_get_value(unsigned gpio)
> > > ... returning 0, 1, or negative errno (for invalid
> > > gpio)
> > > * int gpio_set_value(unsigned gpio, int is_set)
> > > ... returning 0 or negative errno (for invalid gpio)
>
> Note that all these "negative errno" returns could be eliminated if
> we required the gpio to have been claimed in advance, which would
> reduce the cost of those calls. Unfortunately such claim/release
> calls are not currently portable cross-platform.

I doubt that many callers will actually check the return value. If it
works once, it should work every time...

If you actually care about whether the pin id you got is usable, you
need to claim it anyway, as these functions won't check for any
conflicting usage.

> Also, it's going to be platform-specific whether an "output" gpio
> value can be read; and if so, whether that input value matches the
> selected output value.

Yeah, it might depend on which register you read (the at32/at91 gpio
controller has one that returns the value you wrote and one that returns
the actual state of the pins)

> > > * int gpio_set_direction(unsigned gpio, int is_in /* or
> > > is_out? */)
> > > ... returning 0 or negative errno (for invalid gpio)
> >
> > I think set_output_enable makes more sense, but maybe it's just me.
>
> It's just you. :)
>
> A "set enable" idiom is linguistically redundant too; "set" suffices,
> or "enable". Both imply a need for an opposite "clear" or "disable.
> "Direction" is a more obvious notion; the parameter should likely be
> a symbol like GPIO_IN or GPIO_OUT.

Ok, fine with me.

> > Yes, it looks to me that most platforms do basically the same thing
> > with different names.
>
> Though not all provide request/free calls, and there are those funky
> "advanced" features like open drain etc.

The request/free calls aren't really arch-specific, are they? I
implement the actual allocation mechanism using atomic bitops.

And like you said, the "advanced" features can still be
platform-specific. Getting a common api for just the "basic" gpio
operations would be a huge improvement IMO.

> You could do "real" GPIOs through such a "fat" API/layer, but the
> converse is not true.

Yeah, I suggest we handle the simple cases first and see how it works
out.

> > > But pullups are not just a GPIO thing; they're a pin configuration
> > > thing (even on AT91 and AVR32) that can apply even if the pin is
> > > not usable as a GPIO (e.g. as on some non-Atmel platforms). So
> > > that stuff certainly does not belong in generic infrastructure.
> >
> > But maybe it could be done as part of the gpio_request() call, if we
> > specify an additional `flags' parameter? gpio_request() needs to
> > know about pin configuration anyway, as it needs to set up the pin
> > for gpio.
>
> Nope, on several counts in addition to "it's not just for GPIOs":
>
> - First, not all platforms integrate such pullups; ISTR PXA doesn't,
> and DaVinci.
>
> - Second, some platforms offer either pullups or pulldowns; newer
> OMAPs offer that choice, for example.
>
> - Third, some platforms (like OMAP1) have lots of options about
> how to pin out GPIOs (e.g. OMAP 5912 GPIO7 can be on any of three
> different balls, and ball R13 can be GPIO36 -or- MPUIO0). Plus
> there are package options where the same "logical" pin is wired
> out to different "physical" balls.

- Fourth, only the board-specific code really knows whether there's
an external pullup in place. If there is, turning on the internal
pullup might result in a too strong pullup.

> Best all around to keep those issues completely orthogonal, IMO!

I agree.

> > I'll make a few adjustments to the avr32 api to make it conform with
> > your proposed api, as well as rename portmux_select_peripheral to
> > at32_select_peripheral (since it was never really intended to be
> > platform-independent).
> >
> > Then I'll send a patch to do the same thing for at91, if Andrew V
> > is ok with it.
>
> Sounds good, and I'll draft a doc patch to send around to seed the
> discussion about other platforms adopting the same convention.

Great. I'll probably sit on the patches until you're done with the
docs, to make sure that we're talking about the same thing.

Haavard

2006-11-08 22:12:33

by David Brownell

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

> The request/free calls aren't really arch-specific, are they?

Remember that where one platform may use numbers 32-159 for GPIOs, another
might use 0-71 ... GPIO numbering has an arch-specific core, but whether
a given board adds more GPIOs from an FPGA or other non-SOC chip is even
more variable than "arch-specific".


> I implement the actual allocation mechanism using atomic bitops.

That's a fair way to implement it, sure; but if you look at e.g. how
OMAP does it, the bitmap is inside a per-controller structure. When
one chip has two different _types_ of GPIO controller, and multiple
instances of one (plus restrictions applying to specific instances),
the notion of an arch-neutral implementation there seems unworkable.

- Dave

2006-11-08 22:47:39

by Håvard Skinnemoen

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

On 11/8/06, David Brownell <[email protected]> wrote:
> > The request/free calls aren't really arch-specific, are they?
>
> Remember that where one platform may use numbers 32-159 for GPIOs, another
> might use 0-71 ... GPIO numbering has an arch-specific core, but whether
> a given board adds more GPIOs from an FPGA or other non-SOC chip is even
> more variable than "arch-specific".

Sorry, I'm having trouble expressing myself clearly today ;-)

I didn't mean to suggest that the interpretation of the numbers or the
gpio_request() implementation should be arch-independent. I was merely
suggesting that the _concept_ of requesting GPIO pins before using
them, keeping track of which pins have been allocated before, isn't
something inherently arch-specific.

I'm all for leaving it up to each arch or possibly each sub-arch how
to implement the request/free functions. They can even be implemented
as no-op stubs that always succeed for all I care. But I don't think
any arch or platform will really have much trouble with implementing
some kind of "please reserve this gpio pin represented as an unsigned
int for me" call.

> > I implement the actual allocation mechanism using atomic bitops.
>
> That's a fair way to implement it, sure; but if you look at e.g. how
> OMAP does it, the bitmap is inside a per-controller structure. When
> one chip has two different _types_ of GPIO controller, and multiple
> instances of one (plus restrictions applying to specific instances),
> the notion of an arch-neutral implementation there seems unworkable.

Agreed. The avr32 implementation also uses a per-controller bitmap and
supports any number of instances (up to a compile-time limit since it
is initialized too early to be able to allocate any memory.) It
doesn't support different type of controllers though; that would
require some kind of demuxing and should probably be avoided on
platforms that don't need it.

Haavard

2006-11-09 07:35:10

by Andrew Victor

[permalink] [raw]
Subject: Re: [-mm patch 1/4] GPIO framework for AVR32

hi David,

> > > * int gpio_set_direction(unsigned gpio, int is_in /* or
> > > is_out? */)
> > > ... returning 0 or negative errno (for invalid gpio)
> >
> > I think set_output_enable makes more sense, but maybe it's just me.
>
> It's just you. :)
>
> A "set enable" idiom is linguistically redundant too; "set" suffices,
> or "enable". Both imply a need for an opposite "clear" or "disable.
> "Direction" is a more obvious notion; the parameter should likely be
> a symbol like GPIO_IN or GPIO_OUT.

We originally had at91_set_gpio_direction() in the AT91 GPIO layer, and
that seemed to cause confusion (eg, do I pass a 1 or 0 to enable output
mode?)

So I'd personally prefer to keep gpio_set_input() and
gpio_set_output(). (alternative is "enable" instead of "set"). I think
it's more readable.


Regards,
Andrew Victor


2006-11-09 20:41:39

by David Brownell

[permalink] [raw]
Subject: Re: [Bulk] Re: [-mm patch 1/4] GPIO framework for AVR32

> We originally had at91_set_gpio_direction() in the AT91 GPIO layer, and
> that seemed to cause confusion (eg, do I pass a 1 or 0 to enable output
> mode?)

I was thinking the __bitwise annotation on GPIO_IN and GPIO_OUT should
address that problem, but for some reason it isn't doing that. I must
be doing something wrong; even "sparse" isn't warning when passing a
bogus parameter.


> So I'd personally prefer to keep gpio_set_input() and
> gpio_set_output(). (alternative is "enable" instead of "set").
> I think it's more readable.

To be clear ... having two different function calls is a brand
new proposal. :)

Agreed on readable, and I do recall the problem. If I can't get
the __bitwise annotation to behave, that's how I'll do it.

- Dave