lkml.org 
[lkml]   [2008]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/1] [MTD/MAPS] Blackfin Async Flash Maps: Handle the case where flash memory and ethernet mac/phy are mapped onto the same async bank
On Tue, May 13, 2008 at 4:07 AM, Jörn Engel <joern@logfs.org> wrote:
> On Tue, 13 May 2008 12:38:45 +0800, Bryan Wu wrote:
> > +static void bfin_copy_from(struct map_info *map, void *to, unsigned long from, ssize_t len)
> > +{
> > + size_t i;
> > + map_word test;
> > +
> > + if ((unsigned long)to & 0x1) {
> > + for (i = 0; i < len; i += 2) {
> > + u16 *dst = (u16 *)(to + i);
> > + test = bfin_read(map, from + i);
> > + put_unaligned(test.x[0], dst);
> > + }
> > + } else {
> > + for (i = 0; i < len; i += 2) {
> > + u16 *dst = (u16 *)(to + i);
> > + test = bfin_read(map, from + i);
> > + *dst = test.x[0];
> > + }
> > + }
> > +
> > + if (len & 0x1) {
> > + u8 *last_to_byte = (u8 *)(to + i);
> > + test = bfin_read(map, from + i);
> > + *last_to_byte = (u8)test.x[0];
> > + }
> > +}
>
> The pointer casts are superfluous. Linus prefers variable declarations
> up front (sorry for my bad example). The "+ i" in the last conditional
> is a bit dangerous as any changes to the loop can break it. Linux code
> has lots of churn and not everyone is careful enough to spot such
> subtleties.
> And I believe you can improve performance by killing the put_unaligned
> in the loop. So if we put it all together the end result should be
> something like this:
>
>
> static void bfin_copy_from(struct map_info *map, void *to, unsigned long from, ssize_t len)
> {
> size_t i;
> map_word test;
> u8 *byte;
> u16 *dst;
>
>
> if ((unsigned long)to & 0x1) {
> byte = to;
> test = bfin_read(map, from);
> *byte = test.x[0] >> 8;
> to++;
> from++;
> len--;
>
> }
>
> for (i = 0; i < len; i += 2) {
> dst = to + i;
>
> test = bfin_read(map, from + i);
> *dst = test.x[0];
> }
>
> if ((len & 0x1) {
> byte = to + len - 1;
> test = bfin_read(map, from + len - 1);
> *byte = test.x[0] & 0xff;
> }
> }
>
> What do you think?

actually, i think it'll be simpler to just scrap the whole function:
static void bfin_copy_from(struct map_info *map, void *to, unsigned
long from, ssize_t len)
{
struct async_state *state = (struct async_state *)map->map_priv_1;

switch_to_flash(state);

memcpy(to, map->virt + from, len);

switch_back(state);
}
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2008-05-13 19:43    [W:2.070 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site