[U-Boot] Is it an error in function ehci_submit_root() in ehci-hcd.c
Hi All,
I'm trying to enable ehci usb in our board and I found that there
maybe two issues in funcion ehci_submit_root() in ehci-hcd.c.
1)
In ehci_submit_root(), the function will do the following operation.
>From line 553 in ehci-hcd.c:
typeReq = req->request << 8 | req->requesttype;
switch (le16_to_cpu(typeReq)) {
case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
...
...
case USB_REQ_GET_DESCRIPTOR | ((USB_DIR_IN | USB_RT_HUB) << 8):
...
...
}
As in function usb_get_descriptor() in usb.c,
res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
(type << 8) + index, 0,
buf, size, USB_CNTL_TIMEOUT);
req->request will be assigned USB_REQ_GET_DESCRIPTOR,
req->requesttype will be assigned USB_DIR_IN.
The value of typeReq will be 0x680 which can't match any value in
switch (le16_to_cpu(typeReq)) .
Do I miss any config here?
2)
In funcion usb_get_descriptor() in usb.c, it will pass index 0 to
usb_control_msg. setup_packet.index will be assigned 0.
res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
(type << 8) + index, 0,
buf, size, USB_CNTL_TIMEOUT); /* The sixty parameter
is index */
Then in funcion ehci_submit_root() in ehci-hcd.c,
status_reg = (uint32_t *)&hcor->or_portsc[le16_to_cpu(req->index) -
1];
(le16_to_cpu(req->index) - 1) will be -1 here.
Is it correct?
Pls correct me.
Thanks a lot in advance.
Yours
Terry
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Is it an error in function ehci_submit_root() in ehci-hcd.c
Hi,
2009/7/29 Lv Terry-R65388 :
> Hi All,
>
> I'm trying to enable ehci usb in our board and I found that there
> maybe two issues in funcion ehci_submit_root() in ehci-hcd.c.
>
> 1)
> In ehci_submit_root(), the function will do the following operation.
>
> From line 553 in ehci-hcd.c:
>
> typeReq = req->request << 8 | req->requesttype;
>
> switch (le16_to_cpu(typeReq)) {
> case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
> ...
> ...
> case USB_REQ_GET_DESCRIPTOR | ((USB_DIR_IN | USB_RT_HUB) << 8):
> ...
> ...
> }
>
> As in function usb_get_descriptor() in usb.c,
>
> res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> (type << 8) + index, 0,
> buf, size, USB_CNTL_TIMEOUT);
>
> req->request will be assigned USB_REQ_GET_DESCRIPTOR,
> req->requesttype will be assigned USB_DIR_IN.
> The value of typeReq will be 0x680 which can't match any value in
> switch (le16_to_cpu(typeReq)) .
In current mainline this le16_to_cpu() macro has been removed.
Does that change anything?
> Do I miss any config here?
I guess not.
> 2)
> In funcion usb_get_descriptor() in usb.c, it will pass index 0 to
> usb_control_msg. setup_packet.index will be assigned 0.
>
> res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> (type << 8) + index, 0,
> buf, size, USB_CNTL_TIMEOUT); /* The sixty parameter
> is index */
>
> Then in funcion ehci_submit_root() in ehci-hcd.c,
>
> status_reg = (uint32_t *)&hcor->or_portsc[le16_to_cpu(req->index) -
> 1];
>
> (le16_to_cpu(req->index) - 1) will be -1 here.
>
> Is it correct?
Nice catch!
Prafulla/Michael, what do you think?
Kind Regards,
Remy
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Is it an error in function ehci_submit_root() in ehci-hcd.c
Remy Bohmer wrote:
> Hi,
>
> 2009/7/29 Lv Terry-R65388 :
>
>> Hi All,
>>
>>I'm trying to enable ehci usb in our board and I found that there
>> maybe two issues in funcion ehci_submit_root() in ehci-hcd.c.
>>
>> 1)
>>In ehci_submit_root(), the function will do the following operation.
>>
>> From line 553 in ehci-hcd.c:
>>
>>typeReq = req->request << 8 | req->requesttype;
>>
>> switch (le16_to_cpu(typeReq)) {
>> case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
>>...
>>...
>>case USB_REQ_GET_DESCRIPTOR | ((USB_DIR_IN | USB_RT_HUB) << 8):
>>...
>>...
>>}
>>
>>As in function usb_get_descriptor() in usb.c,
>>
>>res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
>> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
>> (type << 8) + index, 0,
>> buf, size, USB_CNTL_TIMEOUT);
>>
>>req->request will be assigned USB_REQ_GET_DESCRIPTOR,
>> req->requesttype will be assigned USB_DIR_IN.
>>The value of typeReq will be 0x680 which can't match any value in
>> switch (le16_to_cpu(typeReq)) .
>>
>
> In current mainline this le16_to_cpu() macro has been removed.
> Does that change anything?
>
>
>>Do I miss any config here?
>>
>
> I guess not.
>
>
>> 2)
>>In funcion usb_get_descriptor() in usb.c, it will pass index 0 to
>> usb_control_msg. setup_packet.index will be assigned 0.
>>
>>res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
>> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
>> (type << 8) + index, 0,
>> buf, size, USB_CNTL_TIMEOUT); /* The sixty parameter
>> is index */
>>
>>Then in funcion ehci_submit_root() in ehci-hcd.c,
>>
>>status_reg = (uint32_t *)&hcor->or_portsc[le16_to_cpu(req->index) -
>> 1];
>>
>>(le16_to_cpu(req->index) - 1) will be -1 here.
>>
>>Is it correct?
>>
>
> Nice catch!
> Prafulla/Michael, what do you think?
>
That we need a patch here urgent :).
Terry, Can you provide the proper fix?
Michael
> Kind Regards,
>
> Remy
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Is it an error in function ehci_submit_root() in ehci-hcd.c
> -Original Message-
> From: Michael Trimarchi [mailto:trimar...@gandalf.sssup.it]
> Sent: Wednesday, July 29, 2009 8:41 PM
> To: Remy Bohmer
> Cc: Lv Terry-R65388; Prafulla Wadaskar; u-boot@lists.denx.de
> Subject: Re: [U-Boot] Is it an error in function
> ehci_submit_root() in ehci-hcd.c
>
> Remy Bohmer wrote:
> > Hi,
> >
> > 2009/7/29 Lv Terry-R65388 :
> >
> >> Hi All,
> >>
> >>I'm trying to enable ehci usb in our board and I found
> that there
> >> maybe two issues in funcion ehci_submit_root() in ehci-hcd.c.
> >>
> >> 1)
> >>In ehci_submit_root(), the function will do the
> following operation.
> >>
> >> From line 553 in ehci-hcd.c:
> >>
> >>typeReq = req->request << 8 | req->requesttype;
> >>
> >> switch (le16_to_cpu(typeReq)) {
> >> case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
> >>...
> >>...
> >>case USB_REQ_GET_DESCRIPTOR | ((USB_DIR_IN | USB_RT_HUB) << 8):
> >>...
> >>...
> >>}
> >>
> >>As in function usb_get_descriptor() in usb.c,
> >>
> >>res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
> >> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> >> (type << 8) + index, 0,
> >> buf, size, USB_CNTL_TIMEOUT);
> >>
> >>req->request will be assigned USB_REQ_GET_DESCRIPTOR,
> >> req->requesttype will be assigned USB_DIR_IN.
> >>The value of typeReq will be 0x680 which can't match
> any value in
> >> switch (le16_to_cpu(typeReq)) .
> >>
> >
> > In current mainline this le16_to_cpu() macro has been removed.
> > Does that change anything?
> >
> >
> >>Do I miss any config here?
> >>
> >
> > I guess not.
> >
> >
> >> 2)
> >>In funcion usb_get_descriptor() in usb.c, it will pass
> index 0 to
> >> usb_control_msg. setup_packet.index will be assigned 0.
> >>
> >>res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
> >> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> >> (type << 8) + index, 0,
> >> buf, size, USB_CNTL_TIMEOUT); /* The sixty
> >> parameter is index */
> >>
> >>Then in funcion ehci_submit_root() in ehci-hcd.c,
> >>
> >>status_reg = (uint32_t
> *)&hcor->or_portsc[le16_to_cpu(req->index)
> >> - 1];
> >>
> >>(le16_to_cpu(req->index) - 1) will be -1 here.
> >>
> >>Is it correct?
> >>
> >
> > Nice catch!
> > Prafulla/Michael, what do you think?
Hi Terry
BTW: Which is your board?
Is it big endian machine? I am curious about it :)
Regards..
Prafulla . . .
> >
> That we need a patch here urgent :).
>
> Terry, Can you provide the proper fix?
>
> Michael
> > Kind Regards,
> >
> > Remy
> >
> >
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Is it an error in function ehci_submit_root() in ehci-hcd.c
Hi Prafulla,
I'm using a little-endian machine.
My board is freescale i.mx51, the core is arm12.
I will try to correct these two places and test it in my board.
Hope it can work.
Thanks~~
Yours
Terry
-Original Message-
From: Prafulla Wadaskar [mailto:prafu...@marvell.com]
Sent: 2009年7月30日 1:30
To: Michael Trimarchi; Remy Bohmer
Cc: Lv Terry-R65388; u-boot@lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
Subject: RE: [U-Boot] Is it an error in function ehci_submit_root() in
ehci-hcd.c
> -Original Message-
> From: Michael Trimarchi [mailto:trimar...@gandalf.sssup.it]
> Sent: Wednesday, July 29, 2009 8:41 PM
> To: Remy Bohmer
> Cc: Lv Terry-R65388; Prafulla Wadaskar; u-boot@lists.denx.de
> Subject: Re: [U-Boot] Is it an error in function
> ehci_submit_root() in ehci-hcd.c
>
> Remy Bohmer wrote:
> > Hi,
> >
> > 2009/7/29 Lv Terry-R65388 :
> >
> >> Hi All,
> >>
> >>I'm trying to enable ehci usb in our board and I found
> that there
> >> maybe two issues in funcion ehci_submit_root() in ehci-hcd.c.
> >>
> >> 1)
> >>In ehci_submit_root(), the function will do the
> following operation.
> >>
> >> From line 553 in ehci-hcd.c:
> >>
> >>typeReq = req->request << 8 | req->requesttype;
> >>
> >> switch (le16_to_cpu(typeReq)) {
> >> case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
> >>...
> >>...
> >>case USB_REQ_GET_DESCRIPTOR | ((USB_DIR_IN | USB_RT_HUB) << 8):
> >>...
> >>...
> >>}
> >>
> >>As in function usb_get_descriptor() in usb.c,
> >>
> >>res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
> >> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> >> (type << 8) + index, 0,
> >> buf, size, USB_CNTL_TIMEOUT);
> >>
> >>req->request will be assigned USB_REQ_GET_DESCRIPTOR,
> >> req->requesttype will be assigned USB_DIR_IN.
> >>The value of typeReq will be 0x680 which can't match
> any value in
> >> switch (le16_to_cpu(typeReq)) .
> >>
> >
> > In current mainline this le16_to_cpu() macro has been removed.
> > Does that change anything?
> >
> >
> >>Do I miss any config here?
> >>
> >
> > I guess not.
> >
> >
> >> 2)
> >>In funcion usb_get_descriptor() in usb.c, it will pass
> index 0 to
> >> usb_control_msg. setup_packet.index will be assigned 0.
> >>
> >>res = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
> >> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> >> (type << 8) + index, 0,
> >> buf, size, USB_CNTL_TIMEOUT); /* The sixty
> >> parameter is index */
> >>
> >>Then in funcion ehci_submit_root() in ehci-hcd.c,
> >>
> >>status_reg = (uint32_t
> *)&hcor->or_portsc[le16_to_cpu(req->index)
> >> - 1];
> >>
> >>(le16_to_cpu(req->index) - 1) will be -1 here.
> >>
> >>Is it correct?
> >>
> >
> > Nice catch!
> > Prafulla/Michael, what do you think?
Hi Terry
BTW: Which is your board?
Is it big endian machine? I am curious about it :)
Regards..
Prafulla . . .
> >
> That we need a patch here urgent :).
>
> Terry, Can you provide the proper fix?
>
> Michael
> > Kind Regards,
> >
> > Remy
> >
> >
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Is it an error in function ehci_submit_root() in ehci-hcd.c
Not yet tested. Cleanup code and fix rootdev GET_DESCRIPTOR request
Signed-off-by: Michael Trimarchi
---
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 324c308..3d495ca 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -317,16 +317,21 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
uint32_t endpt, token, usbsts;
uint32_t c, toggle;
uint32_t cmd;
+ uint16_t wValue, wIndex, wLength;
int ret = 0;
+ wIndex = le16_to_cpu(req->index);
+ wValue = le16_to_cpu(req->value);
+ wLength = le16_to_cpu(req->length);
+
debug("dev=%p, pipe=%lx, buffer=%p, length=%d, req=%p\n", dev, pipe,
buffer, length, req);
if (req != NULL)
debug("req=%u (%#x), type=%u (%#x), value=%u (%#x), index=%u\n",
req->request, req->request,
req->requesttype, req->requesttype,
- le16_to_cpu(req->value), le16_to_cpu(req->value),
- le16_to_cpu(req->index));
+ wValue, wValue,
+ wIndex);
qh = ehci_alloc(sizeof(struct QH), 32);
if (qh == NULL) {
@@ -535,26 +540,34 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
int len, srclen;
uint32_t reg;
uint32_t *status_reg;
+ uint16_t wValue, wIndex, wLength;
+
+ wIndex = le16_to_cpu(req->index);
+ wValue = le16_to_cpu(req->value);
+ wLength = le16_to_cpu(req->length);
- if (le16_to_cpu(req->index) >= CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS) {
+ if (wIndex >= CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS) {
printf("The request port(%d) is not configured\n",
- le16_to_cpu(req->index) - 1);
+ wIndex - 1);
return -1;
}
- status_reg = (uint32_t *)&hcor->or_portsc[
- le16_to_cpu(req->index) - 1];
+
+ /* We don't need the status reg for the rootdev */
+ if (wIndex > 0)
+ status_reg = (uint32_t *)&hcor->or_portsc[wIndex - 1];
+
srclen = 0;
debug("req=%u (%#x), type=%u (%#x), value=%u, index=%u\n",
req->request, req->request,
req->requesttype, req->requesttype,
- le16_to_cpu(req->value), le16_to_cpu(req->index));
+ wValue, wIndex);
typeReq = req->request | req->requesttype << 8;
switch (typeReq) {
case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
- switch (le16_to_cpu(req->value) >> 8) {
+ switch (wValue >> 8) {
case USB_DT_DEVICE:
debug("USB_DT_DEVICE request\n");
srcptr = &descriptor.device;
@@ -567,7 +580,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
break;
case USB_DT_STRING:
debug("USB_DT_STRING config\n");
- switch (le16_to_cpu(req->value) & 0xff) {
+ switch (wValue & 0xff) {
case 0: /* Language */
srcptr = "\4\3\1\0";
srclen = 4;
@@ -584,30 +597,30 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
break;
default:
debug("unknown value DT_STRING %x\n",
- le16_to_cpu(req->value));
+ wValue);
goto unknown;
}
break;
default:
- debug("unknown value %x\n", le16_to_cpu(req->value));
+ debug("unknown value %x\n", wValue);
goto unknown;
}
break;
case USB_REQ_GET_DESCRIPTOR | ((USB_DIR_IN | USB_RT_HUB) << 8):
- switch (le16_to_cpu(req->value) >> 8) {
+ switch (wValue >> 8) {
case USB_DT_HUB:
debug("USB_DT_HUB config\n");
srcptr = &descriptor.hub;
srclen = 0x8;
break;
default:
- debug("unknown value %x\n", le16_to_cpu(req->value));
+ debug("unknown value %x\n", wValue);
goto unknown;
}
break;
case USB_REQ_SET_ADDRESS | (USB_RECIP_DEVICE << 8):
debug("USB_REQ_SET_ADDRESS\n");
- rootdev = le16_to_cpu(req->value);
+ rootdev = wValue;
break;
case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
debug("USB_REQ_SET_CONFIGURATION\n");
@@ -631,7 +644,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
if (reg & EHCI_PS_OCA)
tmpbuf[0] |= USB_PORT_STAT_OVERCURRENT;
if (reg & EHCI_PS_PR &&
- (portreset & (1 << le16_to_cpu(req->index {
+ (portreset & (1 << wIndex))) {
int ret;
/* force reset to complete */
reg = reg & ~(EHCI_PS_PR | EHCI_PS_CLEAR);
@@ -641,7 +654,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
tmpbuf[0] |= USB_PORT_STAT_RESET;
else
printf("port(%d) reset error\n",
- le16_to_cpu(req->index) - 1);
+ wIndex - 1);
}
if (reg & EHCI_PS_PP)
tmpbuf[1] |= USB_PORT_STAT_POWER >> 8;
@@ -668,7 +681,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
tmpbuf[2] |= USB_PORT_STAT_C_ENABLE;
if (reg & EHCI_PS_OCC)
tmpbuf[2] |= USB_PORT_STAT_C_OVERCURRENT;
- if (portreset & (1 << le16_to_cpu(req->index)))
+ if (portreset & (1 << wIndex))
tmpbuf[2] |= USB_PORT_STAT_C_RESET;
srcptr = tmpbuf;
@@ -677,7 +690,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer,
case USB_REQ_SET_FEATURE | ((USB_DIR_OUT | USB_RT_PORT) << 8):
reg = ehci_readl(status_reg);
reg &= ~EHCI_PS_CLEAR;
- switch (l
Re: [U-Boot] Is it an error in function ehci_submit_root() in ehci-hcd.c
Hello Michael, > I would prefer patches on the mailinglist to be inlined and tested. But, no further remarks on this patch. Maybe Prafulla or Terry can provide some help in testing this patch? Kind Regards, Remy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
