Messages in this thread | | | Date | Tue, 13 May 2008 13:41:01 -0400 | From | "Mike Frysinger" <> | Subject | Re: [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/
| |