On 2/9/07, Ulf Samuelsson ulf@atmel.com wrote:
I suggest we try to create a patch series doing the following:
- Eliminate any differences between the two at45.c drivers
- Consolidate the two drivers into drivers/at45.c
- Do a codingstyle cleanup of drivers/at45.c
- Split out the SPI part into a separate driver
- Make everything portable. This includes getting rid of all the AT91FOO prefixing.
<insert additional cleanup steps here>
My priority is to have AT91SAM9x support inside U-Boot ASAP. This approach creates all kinds of unneccessary critical paths which will delay this
Hmm...at first I thought you disagreed with the whole approach, but I think what you _really_ disagree with is the ordering of step 3 and 4. I have no problem with doing these steps the other way around.
I would like to see the following atpproach.
- at45_split is applied splitting into cpu independent part and cpu dependent part. the drivers/at45.c is siglty modified to handle at91rm9200dk and cmc_pu2 boards as well as the at91sam926xek boards
The problem with your patchset is that it first creates a _third_ identical driver before removing the other two. I think it would be cleaner to consolidate the existing two first, and it's not even a big deal -- if it still builds after the move, it works because the code will be the same.
After we then split out the spi part, you can apply your at91sam926 stuff on top of that. This will hopefully be a very small patch.
Btw, why is at45.c going to be modified? It already supports both at91rm9200dk and cmc_pu2. The differences are minimal and purely cosmetic.
- cpu/arm926ejs/at91sam926x AT91SAM9 support This has an spi.c which is different from
Different from what? AFAIK the hardware is the same as on AT91RM9200 and AT32AP7000 modulo a couple of extra features which probably aren't useful to u-boot.
If you're talking about the port muxing, I think it's _really_ overkill to add a new driver for each mux configuration. Maybe it would be better not to split it spi vs. dataflash, but rather "chip-dependent stuff like port muxing" vs. "chip-independent driver code"?
- board patches applied board/at91sam9260ek board/at91sam9261ek board/at91sam9263ek board/at91rm9200ek board/at91rm9200df drivers/dataflash.c update to support linux-2.6 instead of linux-2.4
After that is in place, then it is time for your proposal.
Fine. If it will give us more at91 testers, I'm all for it.
- Do a codingstyle cleanup of drivers/at45.c
- Make everything portable. This includes getting rid of all the AT91FOO prefixing.
If we follow your approach, then the at91sam926x SPI support has first to be inserted into at45.c and retested only to be split up at a later stage.
We can do the spi split before the code cleanup if that helps. But I can't help but think that a per-chip spi driver is mighty strange...
I do not see anyone from the AT91 team participating in this effort right now, and that is why I think we should apply *tested* code to establish a base which is useful to at91sam9 users.
Good point. But code that can't be changed because it's been tested once and for all is going to suck in the long term.
I do think that it is better to submit the AT91SAM9 patches first and only then create a common driver.
If the AT91SAM9 patches include another at45.c driver, I disagree.
The at91sam9 patchset has a board/at45.c and it is this that is available in in the "at45_split" patch.
What's the difference between this driver and the existing two?
If at91_split is applied there will be no modification of this file when the at91sam9 patchset is submitted.
That's good.
Better consolidate first and add new stuff on top of that. But the driver should be usable for new boards after step #2, so you probably don't have to wait very long before you can start submitting the SAM stuff.
the spi.c is very small and msot of the code relies on the pin multiplexing of the part. This is why I think it may make little sense to have a common driver.
This is why I think it isn't the spi stuff that ought to get split out -- it's the pin multiplexing stuff. Maybe it makes sense to split out the spi stuff as well, if it could have other uses beyond dataflash. In fact, the STK1000 has an LCD panel which is powered on using SPI, so a generic spi driver would be nice if we're going to support boot logo on this board later.
Haavard