From mboxrd@z Thu Jan  1 00:00:00 1970
From: Haavard Skinnemoen <hskinnemoen@atmel.com>
Date: Fri, 9 Feb 2007 16:45:20 +0100
Subject: [U-Boot-Users] AT91 NAND om AT91SAM9260EK
In-Reply-To: <45CBA247.209@atmel.com>
References: <c88e466f0702050752t16c18251gcddf40a7ec95469@mail.gmail.com>
	<03d701c74b14$07ec1510$c58e11d4@atmel.com>
	<200702080706.48809.sr@denx.de>
	<c88e466f0702080034l33c06d91tdcf7503cd1ea2d83@mail.gmail.com>
	<45CB7932.3020901@atmel.com>
	<1defaf580702081258j5566c646yf2808b8795b14bbe@mail.gmail.com>
	<45CBA247.209@atmel.com>
Message-ID: <20070209164520.68b3980c@dhcp-252-105.norway.atmel.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Thu, 08 Feb 2007 23:20:55 +0100
Ulf Samuelsson <ulf@atmel.com> wrote:

> Here is it again, was not sent as a tarball, but as an attachement.
> This was a patchset of three patches, the remaining
> two patches just removes
> "board/at91rm9200dk/at45.c"     and
> "board/cmc_pu2/at45.c"
> They are not used any longer.

Thanks. When I replied last time, I thought for some reason that this
was a new driver even though I knew perfectly well it isn't. Anyway, I
guess it makes the review process slightly different...

I think the best way forward is probably to first get rid of the
duplication by moving everything as-is into drivers. That way, there's
only a single driver to clean up, not two.

I suggest we try to create a patch series doing the following:
  1. Eliminate any differences between the two at45.c drivers
  2. Consolidate the two drivers into drivers/at45.c
  3. Do a codingstyle cleanup of drivers/at45.c
  4. Split out the SPI part into a separate driver
  5. Make everything portable. This includes getting rid of all the
     AT91FOO prefixing.
  <insert additional cleanup steps here>

Wolfgang, does this sound like a good plan to you?

Step #1 is easy:

$ diff -up board/cmc_pu2/at45.c board/at91rm9200dk/at45.c
--- board/cmc_pu2/at45.c        2006-08-30 21:53:30.000000000 +0200
+++ board/at91rm9200dk/at45.c   2006-08-30 21:53:30.000000000 +0200
@@ -593,7 +593,7 @@ int AT91F_DataFlashRead(
                if (AT91F_DataFlashWaitReady(pDataFlash->pDataFlashDesc, AT91C_TIMEOUT_WRDY) != DATAFLASH_OK)
                        return -1;

-               if (AT91F_DataFlashContinuousRead (pDataFlash, addr, buffer, SizeToRead) != DATAFLASH_OK)
+               if (AT91F_DataFlashContinuousRead (pDataFlash, addr, (uchar *)buffer, SizeToRead) != DATAFLASH_OK)
                        return -1;

                size -= SizeToRead;

We'll just have to decide whether or not to keep the cast. buffer is a
char * anyway, so I don't think it makes any difference at all.

Step #2 will probably generate an oversize patch unless we do it as a
git patch, which will basically just boil down to a rename and a
deletion (where the deletion will be the biggest part, but still under
40KB I hope.)

Step #3 is where the fun begins...

I have a prototype board with DataFlash for ATSTK1000, so I can start
testing on AVR32 around step #5. Before that, I'd appreciate some help
testing the individual patches (I can do build testing on ARM though.)

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

Btw, Wolfgang: I'd really appreciate if you could pull the avr32 update
from last november (which I rebased and resent last week):

	git://www.atmel.no/~hskinnemoen/u-boot/avr32.git for-wolfgang

It will be a lot easier to support avr32 with a single, unified at45
driver if those changes are in mainline. They simplify much of the
portmux- clock- and I/O management code, thus making it easier to see
what it takes to make the at45 and spi drivers portable across ARM and
AVR32.

Besides, you were the one who specifically asked me to add relocation
support, which is what the update is really all about...

Haavard