From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?= <benoit.thebaudeau@advansee.com>
Date: Fri, 10 Aug 2012 01:13:33 +0200 (CEST)
Subject: [U-Boot] [PATCH v3 1/8] ehci: cosmetic: Define used constants
In-Reply-To: <201208100025.46390.marex@denx.de>
Message-ID: <282669172.2237969.1344554013524.JavaMail.root@advansee.com>
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 Marek Vasut,

> Dear Beno?t Th?baudeau,
> 
> > Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Ilya Yanok <ilya.yanok@cogentembedded.com>
> > Cc: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>
> > ---
> > Changes for v2: N/A.
> > Changes for v3:
> >  - New patch.
> > 
> >  .../drivers/usb/host/ehci-hcd.c                    |  135
> > +++++++++++--------- .../drivers/usb/host/ehci.h
> >                        |
> >  75 ++++++++++- 2 files changed, 150 insertions(+), 60 deletions(-)
> > 
> > diff --git u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c
> > u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c index
> > 5b3b906..1977c28
> > 100644
> > --- u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c
> > +++ u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c
> > @@ -163,7 +163,7 @@ static int ehci_reset(void)
> > 
> >  #ifdef CONFIG_USB_EHCI_TXFIFO_THRESH
> >  	cmd = ehci_readl(&hcor->or_txfilltuning);
> > -	cmd &= ~TXFIFO_THRESH(0x3f);
> > +	cmd &= ~TXFIFO_THRESH_MASK;
> >  	cmd |= TXFIFO_THRESH(CONFIG_USB_EHCI_TXFIFO_THRESH);
> >  	ehci_writel(&hcor->or_txfilltuning, cmd);
> >  #endif
> > @@ -186,7 +186,7 @@ static int ehci_td_buffer(struct qTD *td, void
> > *buf,
> > size_t sz) while (idx < QT_BUFFER_CNT) {
> >  		td->qt_buffer[idx] = cpu_to_hc32(addr);
> >  		td->qt_buffer_hi[idx] = 0;
> > -		next = (addr + 4096) & ~4095;
> > +		next = (addr + EHCI_PAGE_SIZE) & ~(EHCI_PAGE_SIZE - 1);
> >  		delta = next - addr;
> >  		if (delta >= sz)
> >  			break;
> > @@ -208,7 +208,8 @@ ehci_submit_async(struct usb_device *dev,
> > unsigned long
> > pipe, void *buffer, int length, struct devrequest *req)
> >  {
> >  	ALLOC_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN);
> > -	ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN);
> > +#define QTD_COUNT	3
> > +	ALLOC_ALIGN_BUFFER(struct qTD, qtd, QTD_COUNT, USB_DMA_MINALIGN);
> >  	int qtd_counter = 0;
> > 
> >  	volatile struct qTD *vtd;
> > @@ -230,7 +231,7 @@ ehci_submit_async(struct usb_device *dev,
> > unsigned long
> > pipe, void *buffer, le16_to_cpu(req->index));
> > 
> >  	memset(qh, 0, sizeof(struct QH));
> > -	memset(qtd, 0, 3 * sizeof(*qtd));
> > +	memset(qtd, 0, QTD_COUNT * sizeof(*qtd));
> > 
> >  	toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe),
> >  	usb_pipeout(pipe));
> > 
> > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev,
> > unsigned
> > long pipe, void *buffer, */
> >  	qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH);
> >  	c = (usb_pipespeed(pipe) != USB_SPEED_HIGH &&
> > -	     usb_pipeendpoint(pipe) == 0) ? 1 : 0;
> > -	endpt = (8 << 28) |
> > -	    (c << 27) |
> > -	    (usb_maxpacket(dev, pipe) << 16) |
> > -	    (0 << 15) |
> > -	    (1 << 14) |
> > -	    (usb_pipespeed(pipe) << 12) |
> > -	    (usb_pipeendpoint(pipe) << 8) |
> > -	    (0 << 7) | (usb_pipedevice(pipe) << 0);
> > +	     usb_pipeendpoint(pipe) == 0);
> > +	endpt = (8 << QH_ENDPT1_RL) |
> > +	    (c << QH_ENDPT1_C) |
> 
> Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ?
> [...]

For all of these?

> > @@ -398,50 +408,53 @@ ehci_submit_async(struct usb_device *dev,
> > unsigned
> > long pipe, void *buffer, ALIGN((uint32_t)buffer + length,
> > ARCH_DMA_MINALIGN));
> > 
> >  	/* Check that the TD processing happened */
> > -	if (token & 0x80) {
> > +	if (token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS))
> >  		printf("EHCI timed out on TD - token=%#x\n", token);
> > -	}
> > 
> >  	/* Disable async schedule. */
> >  	cmd = ehci_readl(&hcor->or_usbcmd);
> >  	cmd &= ~CMD_ASE;
> >  	ehci_writel(&hcor->or_usbcmd, cmd);
> > 
> > -	ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, 0,
> > +	ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, 0,
> 
> Ooooh, nice catch :)
> 
> [...]
> The rest is cool.
> 
> btw when (I hope you will) resubmitting next time, just submit the
> whole series
> under 0/8 patch (or 1/8) of the old one to make it a nice thread.

OK, so with 2/8 removed since you have applied it. What is the rule here? I
thought that a new version of a patch should be posted as a reply to the
previous version so that patchwork could mark the old version as superseded
(even if this feature is not yet available for U-Boot). Does it apply only to
single patches while series should be replied to the previous 0/n?

And should 3/n be a reply to 2/n, to 0/n (or to 1/n if no 0/n), or to nothing?

I will wait for your review of 8/8.

Best regards,
Beno?t