2013/5/13 Marek Vasut <ma...@denx.de>:
> Dear Kuo-Jung Su,
>
>> From: Kuo-Jung Su <dant...@faraday-tech.com>
>>
>> There is at least one non-EHCI compliant controller (i.e. Faraday EHCI)
>> known to implement a non-standard TDI stuff.
>> Futhermore, it not only leave reserved and CONFIGFLAG registers
>> un-implemented but also has their address spaces removed.
>>
>> And thus, we need weak-aliased functions to both TDI stuff
>> and PORTSC registers for interface abstraction.
>>
>> Signed-off-by: Kuo-Jung Su <dant...@faraday-tech.com>
>> CC: Marek Vasut <ma...@denx.de>
>> ---
>> Changes for v6:
>> - Simplify weak aliased function declaration
>> - Drop redundant line feed
>>
>> Changes for v5:
>> - Split up from Faraday EHCI patch
>>
>> Changes for v2 - v4:
>> - See 'usb: ehci: add Faraday USB 2.0 EHCI support'
>>
>> drivers/usb/host/ehci-hcd.c | 91
>> ++++++++++++++++++++++++++----------------- 1 file changed, 55
>> insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>> index c816878..ae3f2a4 100644
>> --- a/drivers/usb/host/ehci-hcd.c
>> +++ b/drivers/usb/host/ehci-hcd.c
>> @@ -117,10 +117,44 @@ static struct descriptor {
>> };
>>
>> #if defined(CONFIG_EHCI_IS_TDI)
>> -#define ehci_is_TDI() (1)
>> -#else
>> -#define ehci_is_TDI() (0)
>> +# define ehci_is_TDI() (1)
>
> btw you can remove those braces around (1) and (0) below. But I have one more
> question ...
>
Got it, thanks
> [...]
>
>> @@ -609,13 +644,10 @@ ehci_submit_root(struct usb_device *dev, unsigned
>> long pipe, void *buffer, uint32_t *status_reg;
>> struct ehci_ctrl *ctrl = dev->controller;
>>
>> - if (le16_to_cpu(req->index) > CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS) {
>> - printf("The request port(%d) is not configured\n",
>> - le16_to_cpu(req->index) - 1);
>> + status_reg = ehci_get_portsc_register(ctrl->hcor,
>> + le16_to_cpu(req->index) - 1);
>> + if (!status_reg)
>
> What happens here if req->index is zero ?
>
> Hint: the above code always does unsigned comparison ...
>
> I think you should make the second argument of ehci_get_portsc_register()
> unsigned short too (as is req->index in struct devrequest).
>
Sorry, but I'll prefer 'int' over 'unsigned short', since it looks to me that
the u-boot would set 'req->index' to 0 at startup, which results in
a 'port = -1' to be passed to ehci_get_portsc_register().
And I think '-1' is a better self-explain value, so I'd like to stick with 'int'
>> return -1;
>> - }
>> - status_reg = (uint32_t *)&ctrl->hcor->or_portsc[
>> - le16_to_cpu(req->index) - 1];
>> srclen = 0;
>>
>> debug("req=%u (%#x), type=%u (%#x), value=%u, index=%u\n",
>
> [...]
>
> Best regards,
> Marek Vasut
--
Best wishes,
Kuo-Jung Su
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
