* [-mm patch 0/4] Atmel SPI driver and related AVR32 changes @ 2006-11-07 11:25 Haavard Skinnemoen 2006-11-07 11:27 ` [-mm patch 1/4] GPIO framework for AVR32 Haavard Skinnemoen 0 siblings, 1 reply; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-07 11:25 UTC (permalink / raw) To: Andrew Morton; +Cc: David Brownell, Andrew Victor, Linux Kernel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [-mm patch 1/4] GPIO framework for AVR32 2006-11-07 11:25 [-mm patch 0/4] Atmel SPI driver and related AVR32 changes Haavard Skinnemoen @ 2006-11-07 11:27 ` Haavard Skinnemoen 2006-11-07 11:31 ` [-mm patch 2/4] AVR32: spi/ethernet platform_device update Haavard Skinnemoen ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-07 11:27 UTC (permalink / raw) To: Andrew Morton; +Cc: David Brownell, Andrew Victor, Linux Kernel 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 <hskinnemoen@atmel.com> --- 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) ^ permalink raw reply [flat|nested] 17+ messages in thread
* [-mm patch 2/4] AVR32: spi/ethernet platform_device update 2006-11-07 11:27 ` [-mm patch 1/4] GPIO framework for AVR32 Haavard Skinnemoen @ 2006-11-07 11:31 ` Haavard Skinnemoen 2006-11-07 11:33 ` [-mm patch 3/4] AVR32: Move spi device definitions into main board setup file Haavard Skinnemoen 2006-11-07 21:10 ` [-mm patch 1/4] GPIO framework for AVR32 Andrew Morton 2006-11-07 21:36 ` David Brownell 2 siblings, 1 reply; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-07 11:31 UTC (permalink / raw) To: Andrew Morton; +Cc: David Brownell, Andrew Victor, Linux Kernel 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 <hskinnemoen@atmel.com> --- 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, }; ^ permalink raw reply [flat|nested] 17+ messages in thread
* [-mm patch 3/4] AVR32: Move spi device definitions into main board setup file 2006-11-07 11:31 ` [-mm patch 2/4] AVR32: spi/ethernet platform_device update Haavard Skinnemoen @ 2006-11-07 11:33 ` Haavard Skinnemoen 2006-11-07 11:41 ` [-mm patch 4/4] Atmel SPI driver Haavard Skinnemoen 0 siblings, 1 reply; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-07 11:33 UTC (permalink / raw) To: Andrew Morton; +Cc: David Brownell, Andrew Victor, Linux Kernel 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 <hskinnemoen@atmel.com> --- 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, ð0_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); ^ permalink raw reply [flat|nested] 17+ messages in thread
* [-mm patch 4/4] Atmel SPI driver 2006-11-07 11:33 ` [-mm patch 3/4] AVR32: Move spi device definitions into main board setup file Haavard Skinnemoen @ 2006-11-07 11:41 ` Haavard Skinnemoen 2006-11-07 14:12 ` [-mm patch 5/4] Atmel SPI driver: MAINTAINERS entry Haavard Skinnemoen 0 siblings, 1 reply; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-07 11:41 UTC (permalink / raw) To: Andrew Morton; +Cc: David Brownell, Andrew Victor, Linux Kernel 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 <hskinnemoen@atmel.com> --- 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 <hskinnemoen@atmel.com>"); +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__ */ ^ permalink raw reply [flat|nested] 17+ messages in thread
* [-mm patch 5/4] Atmel SPI driver: MAINTAINERS entry 2006-11-07 11:41 ` [-mm patch 4/4] Atmel SPI driver Haavard Skinnemoen @ 2006-11-07 14:12 ` Haavard Skinnemoen 0 siblings, 0 replies; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-07 14:12 UTC (permalink / raw) To: Andrew Morton; +Cc: David Brownell, Andrew Victor, Linux Kernel Add myself as maintainer of the atmel_spi driver. Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com> --- 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: linux-atm-general@lists.sourceforge.n W: http://linux-atm.sourceforge.net S: Maintained +ATMEL SPI DRIVER +P: Atmel AVR32 Support Team +M: avr32@atmel.com +P: Haavard Skinnemoen +M: hskinnemoen@atmel.com +S: Supported + ATMEL WIRELESS DRIVER P: Simon Kelley M: simon@thekelleys.org.uk -- 1.4.3.2 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-07 11:27 ` [-mm patch 1/4] GPIO framework for AVR32 Haavard Skinnemoen 2006-11-07 11:31 ` [-mm patch 2/4] AVR32: spi/ethernet platform_device update Haavard Skinnemoen @ 2006-11-07 21:10 ` Andrew Morton 2006-11-07 22:37 ` David Brownell 2006-11-07 21:36 ` David Brownell 2 siblings, 1 reply; 17+ messages in thread From: Andrew Morton @ 2006-11-07 21:10 UTC (permalink / raw) To: Haavard Skinnemoen; +Cc: David Brownell, Andrew Victor, Linux Kernel On Tue, 7 Nov 2006 12:27:15 +0100 Haavard Skinnemoen <hskinnemoen@atmel.com> 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()? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-07 21:10 ` [-mm patch 1/4] GPIO framework for AVR32 Andrew Morton @ 2006-11-07 22:37 ` David Brownell 2006-11-08 11:48 ` Haavard Skinnemoen 0 siblings, 1 reply; 17+ messages in thread From: David Brownell @ 2006-11-07 22:37 UTC (permalink / raw) To: hskinnemoen, akpm; +Cc: linux-kernel, andrew On Tue, 7 Nov 2006 13:10:14 -0800 quoth Andrew Morton <akpm@osdl.org>: > > On Tue, 7 Nov 2006 12:27:15 +0100 > Haavard Skinnemoen <hskinnemoen@atmel.com> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-07 22:37 ` David Brownell @ 2006-11-08 11:48 ` Haavard Skinnemoen 2006-11-08 18:00 ` David Brownell 0 siblings, 1 reply; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-08 11:48 UTC (permalink / raw) To: David Brownell; +Cc: akpm, linux-kernel, andrew On Tue, 07 Nov 2006 14:37:41 -0800 David Brownell <david-b@pacbell.net> wrote: > On Tue, 7 Nov 2006 13:10:14 -0800 > quoth Andrew Morton <akpm@osdl.org>: > > +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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-08 11:48 ` Haavard Skinnemoen @ 2006-11-08 18:00 ` David Brownell 2006-11-08 18:57 ` Haavard Skinnemoen 2006-11-09 7:26 ` Andrew Victor 0 siblings, 2 replies; 17+ messages in thread From: David Brownell @ 2006-11-08 18:00 UTC (permalink / raw) To: hskinnemoen; +Cc: linux-kernel, andrew, akpm > > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-08 18:00 ` David Brownell @ 2006-11-08 18:57 ` Haavard Skinnemoen 2006-11-08 22:12 ` David Brownell 2006-11-09 7:26 ` Andrew Victor 1 sibling, 1 reply; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-08 18:57 UTC (permalink / raw) To: David Brownell; +Cc: linux-kernel, andrew, akpm On Wed, 08 Nov 2006 10:00:59 -0800 David Brownell <david-b@pacbell.net> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-08 18:57 ` Haavard Skinnemoen @ 2006-11-08 22:12 ` David Brownell 2006-11-08 22:47 ` Haavard Skinnemoen 0 siblings, 1 reply; 17+ messages in thread From: David Brownell @ 2006-11-08 22:12 UTC (permalink / raw) To: hskinnemoen; +Cc: linux-kernel, andrew, akpm > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-08 22:12 ` David Brownell @ 2006-11-08 22:47 ` Haavard Skinnemoen 0 siblings, 0 replies; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-08 22:47 UTC (permalink / raw) To: David Brownell; +Cc: hskinnemoen, linux-kernel, andrew, akpm On 11/8/06, David Brownell <david-b@pacbell.net> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-08 18:00 ` David Brownell 2006-11-08 18:57 ` Haavard Skinnemoen @ 2006-11-09 7:26 ` Andrew Victor 2006-11-09 20:41 ` [Bulk] " David Brownell 1 sibling, 1 reply; 17+ messages in thread From: Andrew Victor @ 2006-11-09 7:26 UTC (permalink / raw) To: David Brownell; +Cc: hskinnemoen, linux-kernel, akpm 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bulk] Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-09 7:26 ` Andrew Victor @ 2006-11-09 20:41 ` David Brownell 0 siblings, 0 replies; 17+ messages in thread From: David Brownell @ 2006-11-09 20:41 UTC (permalink / raw) To: andrew; +Cc: linux-kernel, hskinnemoen, akpm > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-07 11:27 ` [-mm patch 1/4] GPIO framework for AVR32 Haavard Skinnemoen 2006-11-07 11:31 ` [-mm patch 2/4] AVR32: spi/ethernet platform_device update Haavard Skinnemoen 2006-11-07 21:10 ` [-mm patch 1/4] GPIO framework for AVR32 Andrew Morton @ 2006-11-07 21:36 ` David Brownell 2006-11-08 10:30 ` Haavard Skinnemoen 2 siblings, 1 reply; 17+ messages in thread From: David Brownell @ 2006-11-07 21:36 UTC (permalink / raw) To: hskinnemoen, akpm; +Cc: linux-kernel, andrew > +#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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [-mm patch 1/4] GPIO framework for AVR32 2006-11-07 21:36 ` David Brownell @ 2006-11-08 10:30 ` Haavard Skinnemoen 0 siblings, 0 replies; 17+ messages in thread From: Haavard Skinnemoen @ 2006-11-08 10:30 UTC (permalink / raw) To: David Brownell; +Cc: akpm, linux-kernel, andrew On Tue, 07 Nov 2006 13:36:04 -0800 David Brownell <david-b@pacbell.net> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2006-11-09 20:41 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2006-11-07 11:25 [-mm patch 0/4] Atmel SPI driver and related AVR32 changes Haavard Skinnemoen 2006-11-07 11:27 ` [-mm patch 1/4] GPIO framework for AVR32 Haavard Skinnemoen 2006-11-07 11:31 ` [-mm patch 2/4] AVR32: spi/ethernet platform_device update Haavard Skinnemoen 2006-11-07 11:33 ` [-mm patch 3/4] AVR32: Move spi device definitions into main board setup file Haavard Skinnemoen 2006-11-07 11:41 ` [-mm patch 4/4] Atmel SPI driver Haavard Skinnemoen 2006-11-07 14:12 ` [-mm patch 5/4] Atmel SPI driver: MAINTAINERS entry Haavard Skinnemoen 2006-11-07 21:10 ` [-mm patch 1/4] GPIO framework for AVR32 Andrew Morton 2006-11-07 22:37 ` David Brownell 2006-11-08 11:48 ` Haavard Skinnemoen 2006-11-08 18:00 ` David Brownell 2006-11-08 18:57 ` Haavard Skinnemoen 2006-11-08 22:12 ` David Brownell 2006-11-08 22:47 ` Haavard Skinnemoen 2006-11-09 7:26 ` Andrew Victor 2006-11-09 20:41 ` [Bulk] " David Brownell 2006-11-07 21:36 ` David Brownell 2006-11-08 10:30 ` Haavard Skinnemoen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®