On 2/9/07, Ulf Samuelsson ulf@atmel.com wrote:
The only thimg I'm not really happy with (whithout having any better suggestion) is to move this into drivers - drivers is intended for general, CPU and board independent code, where this is obviously specific to a certain class of processors.
It's somewhat cpu- and board-independent. at32 and at91 are based on two different architectures, so they don't have any directories in common apart from the ones common to everyone.
We could perhaps create a drivers/atmel subdirectory or, as Ulf suggests, boards/atmel/drivers or cpu/atmel/drivers.
I think at45.c is CPU independent, but spi.c is closely tied to Atmel processors. spi.c consist of:
SpiInit - Clearly CPU dependent due to pin mux. Only reason that sam926x chips can use a common file is the #ifdefs...
Hmm...how about moving the cpu-dependent bits into inline functions somewhere under asm/arch? For example
static inline void portmux_enable_spi(unsigned int id) { /* do chip-dependent PIO stuff here */ }
asm/arch is chip-specific so there shouldn't be any need for #ifdefs there.
While the functions in at45.c are called AT91xxx they really do not depend on any specific SPI H/W and it can thus be used with any chip which implements the SPI API defined by cpu/arm920t/at91rm9200/spi.c
Yeah, that's why the AT91 prefixing should be dropped. But that's for much later.
Btw, looks like there's another SPI API in u-boot as well. Probably a good idea to convert the at91/atmel spi stuff over to implementing that one. No point in having several APIs doing the same thing, and the other API uses the much more sensible function name spi_xfer for doing SPI transfers, as opposed to AT91F_SpiWrite which is totally misleading since it's being used for reading as well.
If you let it remain in the board directories as is, then you duplicate this for each board.
I don't think anyone wants that. Although you're the one suggesting we add a _third_ identical driver ;-)
I think a good place for any driver stuff which is useable both by at91 and ap7xxx chips could be an board/atmel/drivers directory. An alternative would be a cpu/atmel/drivers directory.
Are any other drivers organized this way?
I do not think it belongs inside the drivers directory.
I'm sorry but I fail to see why not. There are several other platform-specific drivers in there. And I don't really see the big advantage of spreading drivers around all over the place...
By putting spi.c in drivers you have to be really careful not to compile this for chips which does not have this SPI macro.
Yeah, but if it's protected by an easy-to-understand #ifdef that shouldn't be a problem. Although I don't think we should call it spi.c then. It should be atmel_spi.c or something.
The cpu/arm920t/at91rm9200/spi.c should at least contain the spi init.
Or do it in a chip-specific header file.
I still would like to have the complete sam926x patch set implemented before we start to "play" with it
Yeah, but I still think some preparatory patches are a good thing in order to make things easier later on.
Haavard