From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Tue, 24 Sep 2013 08:59:42 +0200
Subject: [U-Boot] [PATCH] usb: ehci: Fix test mode for connected ports
In-Reply-To: <CAPnjgZ2sNAsuK4WqOrx-b0RYgJY7pq5ZNf5Kh3g_A2rye9Pn3g@mail.gmail.com>
References: <1379464155-13250-1-git-send-email-jwerner@chromium.org>
	<CAODwPW-qwBXtvEm-1viUp5RHCbOq9shs7Go72MYJq0ZSLNxVgg@mail.gmail.com>
	<CAPnjgZ2sNAsuK4WqOrx-b0RYgJY7pq5ZNf5Kh3g_A2rye9Pn3g@mail.gmail.com>
Message-ID: <201309240859.43105.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Simon Glass,

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