Dear Simon Glass, > Hi Julius, > > On Mon, Sep 23, 2013 at 8:55 PM, Julius Werner <jwer...@chromium.org> wrote: > > Hi Simon, > > > > > It seems like you could do something like > > > > > > /* > > > > > > * This is the delay for ...the spec requires a minimum of ... > > > */ > > > > > > #define SOME_SUITABLE_NAME 8000 > > > > > > at the top of the file and then use it twice in your function. > > > > The file contains a dozen handshake() calls that have a literal "100 * > > 1000" in there, and none of them have any special meaning in regards > > to the spec... it's all just a random guess. I'd be happy to change > > the timeouts in my patch to 100 for consistency, but it really doesn't > > make sense to single this one out with a #define. (Maybe we should > > make it a #define DEFAULT_TIMEOUT 100000 for all of them, but not in > > this patch.) > > I guess we are trying to distinguish between how the code is and how we > would like it to me. From that point of view your patch is independent of > the existing code. Since you have a clear reason for the timeout you have > chose, you can document that and set a good example for others!
ACK on that. We should focus on improvement, not stagnate. Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
