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?
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.
+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?
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
...
+/* 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 < 2;
+}
Duplicated code. Move to common.
+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;- }
+}
Duplicated code. Move to common.
+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;- }
+}
Duplicated code. Move to common.
diff --git a/board/atmel/at91sam9261ek/at91sam9261ek.c b/board/atmel/at91sam9261ek/at91sam9261ek.c index a89cb8b..9d31324 100644 --- a/board/atmel/at91sam9261ek/at91sam9261ek.c +++ b/board/atmel/at91sam9261ek/at91sam9261ek.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 || cs == 3);
+}
+void spi_cs_activate(struct spi_slave *slave) +{
- switch(slave->cs) {
- case 3:
at91_set_gpio_value(AT91_PIN_PA6, 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 3:
at91_set_gpio_value(AT91_PIN_PA6, 1);break;- case 0:
- default:
at91_set_gpio_value(AT91_PIN_PA3, 1);break;- }
+}
This is again nearly the same code as before.
May be it can be generalized with some parameters so we don;t need to duplicate it again and again?
diff --git a/board/atmel/at91sam9263ek/at91sam9263ek.c b/board/atmel/at91sam9263ek/at91sam9263ek.c index 57d5c95..e9c73af 100644 --- a/board/atmel/at91sam9263ek/at91sam9263ek.c +++ b/board/atmel/at91sam9263ek/at91sam9263ek.c
...
+/* SPI chip select control */ +#ifdef CONFIG_ATMEL_SPI +#include <spi.h>
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- return bus == 0 && cs == 0;
+}
+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);
+}
Gain: all repeated code.
+#endif /* CONFIG_ATMEL_SPI */ diff --git a/board/atmel/at91sam9rlek/at91sam9rlek.c b/board/atmel/at91sam9rlek/at91sam9rlek.c index 7013ba2..35adfcd 100644 --- a/board/atmel/at91sam9rlek/at91sam9rlek.c +++ b/board/atmel/at91sam9rlek/at91sam9rlek.c
...
+/* SPI chip select control */ +#ifdef CONFIG_ATMEL_SPI +#include <spi.h>
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- return bus == 0 && cs == 0;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
- at91_set_gpio_value(AT91_PIN_PA28, 0);
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
- at91_set_gpio_value(AT91_PIN_PA28, 1);
+}
And again.
diff --git a/board/ronetix/pm9263/pm9263.c b/board/ronetix/pm9263/pm9263.c index 801b268..c16bea0 100644 --- a/board/ronetix/pm9263/pm9263.c +++ b/board/ronetix/pm9263/pm9263.c
...
+/* SPI chip select control */ +#ifdef CONFIG_ATMEL_SPI +#include <spi.h>
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- return bus == 0 && cs == 0;
+}
+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);
+}
And again.
Sorry, I'm not going to accept this.
diff --git a/cpu/arm926ejs/at91/Makefile b/cpu/arm926ejs/at91/Makefile 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
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.
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?
Best regards,
Wolfgang Denk