Hi,
-----Original Message----- From: Scott Wood [mailto:scottwood@freescale.com] Sent: Tuesday, June 30, 2009 1:26 AM To: Kyungmin Park Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH 4/6] S5PC100: onenand driver for SMDKC100 support
On Sat, Jun 27, 2009 at 04:32:35PM +0900, Kyungmin Park wrote:
+/**
- onenand_read_burst
- 16 Burst read: performance is improved up to 40%.
- */
+static void onenand_read_burst(void *dest, const void *src, size_t
len)
+{
- int count;
- if (len % 16 != 0)
- return;
- count = len / 16;
- __asm__ __volatile__(
- " stmdb r13!, {r0-r3,r9-r12}\n"
- " mov r2, %0\n"
- "1:\n"
- " ldmia r1, {r9-r12}\n"
- " stmia r0!, {r9-r12}\n"
- " subs r2, r2, #0x1\n"
- " bne 1b\n"
- " ldmia r13!, {r0-r3,r9-r12}\n"::"r" (count));
+}
What is this doing that we couldn't generically make memcpy do?
Even though It looks some strange. it has some performance gain. but not general.
I guess that's because you're reading from the same 16 bytes each loop iteration. Perhaps repeated 16-byte calls to memcpy could be used, combined with a suitably optimized memcpy (possibly with inline asm in the arch headers for certain constant sizes).
Also, relying on r0/r1 to still contain dest/src after the compiler has had a chance to mess with things is dangerous. Better to use the asm constraints properly. I also don't see why you need to save r3.
Actually I dont' write this code .... I'm not sure why he did like it. IMHO, I also don't like this sub-optimal approach.
Is there any chance that this driver could be applicable to something that isn't ARM? Is this programming interface part of a host controller, or is it embedded in the OneNAND chip?
He assume the it's only used at s3c64xx/s5pc100. As you can see the source address is fixed So it's difficult to use generally.
Thank you, Kyungmin Park