[U-Boot] [PATCH] at91sam9/at91cap: add new spi framework and spi flash support

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090404194842.GH32409 at game.jcrosoft.org> you wrote:
>
> > 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

No, it is not, as many boards use exactly the same code.

> > > +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

It's DUPLICATED.

> > >  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

So use the CPU name.

> and due to multiple soc support for one file we can not do this

I think we can do this. Please try, and you will see how it will work
out.

> > > --- 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

The resulting code (when compiled) would be 100% the same, so why do
you think it cannot be done?

Just look at the cleanup Detlev Zundel just performed on the 16550
code. It is a wothwile goal to clean this up.

> > > --- 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

As long as you add it to ALL boards you can hardly call this board
specific.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Artificial Intelligence is the study of how to  make  real  computers
act like the ones in movies.