On 21:52 Sat 04 Apr , Wolfgang Denk wrote:
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 1238840105-19029-1-git-send-email-plagnioj@jcrosoft.com you wrote:
The new Framework is not yet activated by default, it will be next release at the same time of the removal of the DATAFLASH support
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
... ...
diff --git a/board/afeb9260/afeb9260.c b/board/afeb9260/afeb9260.c index 024db2b..d1606b6 100644 --- a/board/afeb9260/afeb9260.c +++ b/board/afeb9260/afeb9260.c
...
+/* SPI chip select control */ +#ifdef CONFIG_ATMEL_SPI +#include <spi.h>
Please no #includes in the middle of the file.
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- return bus == 0 && cs < 2;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
- switch(slave->cs) {
- case 1:
at91_set_gpio_value(AT91_PIN_PC11, 0);break;- case 0:
- default:
at91_set_gpio_value(AT91_PIN_PA3, 0);break;- }
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
- switch(slave->cs) {
- case 1:
at91_set_gpio_value(AT91_PIN_PC11, 1);break;- case 0:
- default:
at91_set_gpio_value(AT91_PIN_PA3, 1);break;- }
+}
Are these really board specific, or is it likely that we will see the same code for other boards as well?
yes these code are board specific because you can use the cs you want
diff --git a/board/atmel/at91cap9adk/at91cap9adk.c b/board/atmel/at91cap9adk/at91cap9adk.c index f52edaa..1e99bba 100644 --- a/board/atmel/at91cap9adk/at91cap9adk.c +++ b/board/atmel/at91cap9adk/at91cap9adk.c
...
+/* SPI chip select control */ +#ifdef CONFIG_ATMEL_SPI +#include <spi.h>
See above.
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- return bus == 0 && cs == 0;
+}
Duplicated code. Move to common.
It's board specific
+void spi_cs_activate(struct spi_slave *slave) +{
- at91_set_gpio_value(AT91_PIN_PA5, 0);
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
- at91_set_gpio_value(AT91_PIN_PA5, 1);
+}
How about making these inline?
it's the spi framework
diff --git a/board/atmel/at91sam9260ek/at91sam9260ek.c b/board/atmel/at91sam9260ek/at91sam9260ek.c index 6bd3b44..c4eaafd 100644 --- a/board/atmel/at91sam9260ek/at91sam9260ek.c +++ b/board/atmel/at91sam9260ek/at91sam9260ek.c
...
index fbc82d1..e73f450 100644 --- a/cpu/arm926ejs/at91/Makefile +++ b/cpu/arm926ejs/at91/Makefile @@ -28,30 +28,36 @@ LIB = $(obj)lib$(SOC).a ifdef CONFIG_AT91CAP9 COBJS-$(CONFIG_MACB) += at91cap9_macb.o COBJS-y += at91cap9_serial.o +COBJS-$(CONFIG_ATMEL_SPI) += at91cap9_spi.o COBJS-$(CONFIG_HAS_DATAFLASH) += at91cap9_spi.o endif ifdef CONFIG_AT91SAM9260 COBJS-$(CONFIG_MACB) += at91sam9260_macb.o COBJS-y += at91sam9260_serial.o +COBJS-$(CONFIG_ATMEL_SPI) += at91sam9260_spi.o COBJS-$(CONFIG_HAS_DATAFLASH) += at91sam9260_spi.o endif ifdef CONFIG_AT91SAM9G20 COBJS-$(CONFIG_MACB) += at91sam9260_macb.o COBJS-y += at91sam9260_serial.o +COBJS-$(CONFIG_ATMEL_SPI) += at91sam9260_spi.o COBJS-$(CONFIG_HAS_DATAFLASH) += at91sam9260_spi.o endif ifdef CONFIG_AT91SAM9261 COBJS-y += at91sam9261_serial.o +COBJS-$(CONFIG_ATMEL_SPI) += at91sam9261_spi.o COBJS-$(CONFIG_HAS_DATAFLASH) += at91sam9261_spi.o endif ifdef CONFIG_AT91SAM9263 COBJS-$(CONFIG_MACB) += at91sam9263_macb.o COBJS-y += at91sam9263_serial.o +COBJS-$(CONFIG_ATMEL_SPI) += at91sam9263_spi.o COBJS-$(CONFIG_HAS_DATAFLASH) += at91sam9263_spi.o COBJS-$(CONFIG_USB_OHCI_NEW) += at91sam9263_usb.o endif ifdef CONFIG_AT91SAM9RL COBJS-y += at91sam9rl_serial.o +COBJS-$(CONFIG_ATMEL_SPI) += at91sam9rl_spi.o COBJS-$(CONFIG_HAS_DATAFLASH) += at91sam9rl_spi.o endif COBJS-$(CONFIG_AT91_LED) += led.o
This needs to be cleand up as well. Define some variable for the board name (don't we actually have one already?), and then use a single copy of
it's not board name it's cpu name
and due to multiple soc support for one file we can not do this
COBJS-$(CONFIG_MACB) += $(BOARD)_macb.o COBJS-y += $(BOARD)_serial.o COBJS-$(CONFIG_ATMEL_SPI) += $(BOARD)_spi.o COBJS-$(CONFIG_HAS_DATAFLASH) += $(BOARD)_spi.o
instead of such an endless list if "ifdef"ed code.
diff --git a/include/asm-arm/arch-at91/hardware.h b/include/asm-arm/arch-at91/hardware.h index 8704106..0403b09 100644 --- a/include/asm-arm/arch-at91/hardware.h +++ b/include/asm-arm/arch-at91/hardware.h @@ -21,25 +21,34 @@ #elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) #include <asm/arch/at91sam9260.h> #define AT91_BASE_SPI AT91SAM9260_BASE_SPI0 +#define SPI0_BASE AT91SAM9260_BASE_SPI0 +#define SPI1_BASE AT91SAM9260_BASE_SPI1 #define AT91_ID_UHP AT91SAM9260_ID_UHP #define AT91_PMC_UHP AT91SAM926x_PMC_UHP #elif defined(CONFIG_AT91SAM9261) #include <asm/arch/at91sam9261.h> #define AT91_BASE_SPI AT91SAM9261_BASE_SPI0 +#define SPI0_BASE AT91SAM9261_BASE_SPI0 +#define SPI1_BASE AT91SAM9261_BASE_SPI1 #define AT91_ID_UHP AT91SAM9261_ID_UHP #define AT91_PMC_UHP AT91SAM926x_PMC_UHP #elif defined(CONFIG_AT91SAM9263) #include <asm/arch/at91sam9263.h> #define AT91_BASE_SPI AT91SAM9263_BASE_SPI0 +#define SPI0_BASE AT91SAM9263_BASE_SPI0 +#define SPI1_BASE AT91SAM9263_BASE_SPI1 #define AT91_ID_UHP AT91SAM9263_ID_UHP #define AT91_PMC_UHP AT91SAM926x_PMC_UHP #elif defined(CONFIG_AT91SAM9RL) #include <asm/arch/at91sam9rl.h> #define AT91_BASE_SPI AT91SAM9RL_BASE_SPI +#define SPI0_BASE AT91SAM9RL_BASE_SPI #define AT91_ID_UHP AT91SAM9RL_ID_UHP #elif defined(CONFIG_AT91CAP9) #include <asm/arch/at91cap9.h> #define AT91_BASE_SPI AT91CAP9_BASE_SPI0 +#define SPI0_BASE AT91CAP9_BASE_SPI0 +#define SPI1_BASE AT91CAP9_BASE_SPI1 #define AT91_ID_UHP AT91CAP9_ID_UHP #define AT91_PMC_UHP AT91CAP9_PMC_UHP #elif defined(CONFIG_AT91X40)
Same here. With a little clever use of macros we probably can get rid of all these "if defined" stuff.
we can not because it's shared with diffent soc and with AVR32
diff --git a/include/configs/afeb9260.h b/include/configs/afeb9260.h index 89b16fe..a897c62 100644 --- a/include/configs/afeb9260.h +++ b/include/configs/afeb9260.h @@ -84,6 +84,12 @@ #define PHYS_SDRAM_SIZE 0x04000000 /* 64 megs */
/* DataFlash */ +#ifdef CONFIG_ATMEL_SPI +#define CONFIG_CMD_SF +#define CONFIG_CMD_SPI +#define CONFIG_SPI_FLASH 1 +#define CONFIG_SPI_FLASH_ATMEL 1 +#else
In which way is this board specific configuration when you copy it to all (?) AT91 boards?
you can enalbe what you want as flash I can not force at91 boards evenif actually they use the same
It's be more generic when we will have Kconfig
Best Regards, J.