[U-Boot] [PATCH] usb: ehci: Fix test mode for connected ports

Marek Vasut marex at denx.de
Tue Sep 24 08:59:42 CEST 2013


Dear Simon Glass,

> Hi Julius,
> 
> On Mon, Sep 23, 2013 at 8:55 PM, Julius Werner <jwerner at 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


More information about the U-Boot mailing list