On Sun, 2009-11-15 at 02:19 -0700, David Brownell wrote:
> On Sunday 15 November 2009, Dean Glazeski wrote:
> > I've done an implementation of reading using an on-chip algorithm for NAND
> > devices attached to ARM chips. It seems to work on my AT91SAM9260 board
> > increasing nand dump from 0.2 kb/s to 9.2 kb/s. Not sure if this is the
> > best implementation, so take a look. By the way, there is some room for
> > refactoring between this function and the write version. I'm not sure how
> > to do that so I just posted this patch with the implementation.
>
> At first glance it seemed OK; comments still talk about "write" not "read"
> though. :)
>
> Minor style issues ... a few overlong lines, and don't do functions like
>
> int f(...) {
> ...
> }
>
> Always have that left bracket at beginning-of-line.
>
> Refactoring can come later.
Personally, I think refactoring should come first, along with a patch to
hook up this routine into the system. Right now, it's dangling unused.
I have been working hard to eliminate duplicated code, so I am not eager
to see more entering the tree. Others should take responsibility for
handling this work when it is required; remember, all patch series
should be finished before it's pushed. That includes refactoring, which
will be easier _before_ adding code. If such changes would clearly be
best for the code, then that seems like a clear reason for those patches
to be part of a series.
My recent NAND write/dump refactoring shows one way how this can be
done: add new helpers, use them in existing code, add your new code to
use them. That may not be the order you do the development, and other
approaches for structuring the series would work too. In any case, I
hope I have made a convincing argument for finding free time to factor
facsimiles first.
--Z
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
