Patchwork [U-Boot,3/3] usb: Correct CLEAR_FEATURE code in ehci-hcd

login
register
mail settings
Submitter Simon Glass
Date May 11, 2013, 2:49 a.m.
Message ID <1368240540-11741-3-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/243107/
State Awaiting Upstream
Delegated to: Marek Vasut
Headers show

Comments

Simon Glass - May 11, 2013, 2:49 a.m.
This commit broke USB2 on link (Chromebook Pixel):

  020bbcb usb: hub: Power-cycle on root-hub ports

However the root cause seems to be a missing mask and missing 'break'
in ehci-hcd.c. This patch fixes both.

On link, 'usb start' with a USB keyboard and memory stick inserted now
finds both. The keyboard works as expected. Also ext2ls shows a directory
listing from the memory stick.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/host/ehci-hcd.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
Marek Vasut - May 11, 2013, 4:39 p.m.
Dear Simon Glass,

> This commit broke USB2 on link (Chromebook Pixel):
> 
>   020bbcb usb: hub: Power-cycle on root-hub ports
> 
> However the root cause seems to be a missing mask and missing 'break'
> in ehci-hcd.c. This patch fixes both.
> 
> On link, 'usb start' with a USB keyboard and memory stick inserted now
> finds both. The keyboard works as expected. Also ext2ls shows a directory
> listing from the memory stick.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Picking up all three, thanks!

Best regards,
Marek Vasut
Simon Glass - May 11, 2013, 5:27 p.m.
On Sat, May 11, 2013 at 10:39 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
>> This commit broke USB2 on link (Chromebook Pixel):
>>
>>   020bbcb usb: hub: Power-cycle on root-hub ports
>>
>> However the root cause seems to be a missing mask and missing 'break'
>> in ehci-hcd.c. This patch fixes both.
>>
>> On link, 'usb start' with a USB keyboard and memory stick inserted now
>> finds both. The keyboard works as expected. Also ext2ls shows a directory
>> listing from the memory stick.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Picking up all three, thanks!

Thanks, also Julius may have some comments on Monday. Two of the
patches were reviewed by him, but not the last one, although he
suggested it.

Regards,
Simon

>
> Best regards,
> Marek Vasut
Marek Vasut - May 11, 2013, 5:59 p.m.
Dear Simon Glass,

> On Sat, May 11, 2013 at 10:39 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Simon Glass,
> > 
> >> This commit broke USB2 on link (Chromebook Pixel):
> >>   020bbcb usb: hub: Power-cycle on root-hub ports
> >> 
> >> However the root cause seems to be a missing mask and missing 'break'
> >> in ehci-hcd.c. This patch fixes both.
> >> 
> >> On link, 'usb start' with a USB keyboard and memory stick inserted now
> >> finds both. The keyboard works as expected. Also ext2ls shows a
> >> directory listing from the memory stick.
> >> 
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> > 
> > Picking up all three, thanks!
> 
> Thanks, also Julius may have some comments on Monday. Two of the
> patches were reviewed by him, but not the last one, although he
> suggested it.

I just build-tested them, seem OK.

btw. I might be less available this month, sorry about that.

Best regards,
Marek Vasut
Julius Werner - May 13, 2013, 6:19 p.m.
Thanks for taking charge of this Simon, looks good to me. Good catch
with the missing break!

Now that I look at it, the FEAT_C_ENABLE section seems to be broken as
well, it should be setting EHCI_PS_PEC instead of EHCI_PS_PE. Maybe
that code path is never used in practice.

Acked-by: Julius Werner <jwerner@chromium.org>

Patch

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index e0f3e4b..445759b 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -809,21 +809,23 @@  ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
 		break;
 	case USB_REQ_CLEAR_FEATURE | ((USB_DIR_OUT | USB_RT_PORT) << 8):
 		reg = ehci_readl(status_reg);
+		reg &= ~EHCI_PS_CLEAR;
 		switch (le16_to_cpu(req->value)) {
 		case USB_PORT_FEAT_ENABLE:
 			reg &= ~EHCI_PS_PE;
 			break;
 		case USB_PORT_FEAT_C_ENABLE:
-			reg = (reg & ~EHCI_PS_CLEAR) | EHCI_PS_PE;
+			reg |= EHCI_PS_PE;
 			break;
 		case USB_PORT_FEAT_POWER:
 			if (HCS_PPC(ehci_readl(&ctrl->hccr->cr_hcsparams)))
-				reg = reg & ~(EHCI_PS_CLEAR | EHCI_PS_PP);
+				reg &= ~EHCI_PS_PP;
+			break;
 		case USB_PORT_FEAT_C_CONNECTION:
-			reg = (reg & ~EHCI_PS_CLEAR) | EHCI_PS_CSC;
+			reg |= EHCI_PS_CSC;
 			break;
 		case USB_PORT_FEAT_OVER_CURRENT:
-			reg = (reg & ~EHCI_PS_CLEAR) | EHCI_PS_OCC;
+			reg |= EHCI_PS_OCC;
 			break;
 		case USB_PORT_FEAT_C_RESET:
 			ctrl->portreset &= ~(1 << port);