[U-Boot] [PATCH 4/6] S5PC100: onenand driver for SMDKC100 support

[Kyungmin Park]

Hi,

> -----Original Message-----
> From: Scott Wood [mailto:scottwood at freescale.com]
> Sent: Tuesday, June 30, 2009 1:26 AM
> To: Kyungmin Park
> Cc: u-boot at 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 don’t' 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