Re: [RFC/PATCH 2/2] usb: Adding SuperSpeed support to dummy_hcd

[Brokhman Tatyana]

Hi Alan

Thanks you for your comments. Please see my reply inline.

Best Regards,
Tanya Brokhman

> On Sun, 10 Oct 2010, Tatyana Brokhman wrote:
>
>> USB 3.0 hub includes 2 hubs - HS and SS ones.
>> Thus, when dummy_hcd enabled it will register 2 root hubs (SS and HS).
>>
>> Signed-off-by: Tatyana Brokhman <tlinder@xxxxxxxxxxxxxx>
>> ---
>>  drivers/usb/gadget/dummy_hcd.c |  739
>> +++++++++++++++++++++++++++++++++-------
>>  1 files changed, 616 insertions(+), 123 deletions(-)
>
> I'd appreciate if you break this up into two patches.  Separating out
> handle_control_request into a separate function is independent of the
> USB-3.0 conversion.

You're right about that. Will do.
I'll wait for others to review and send an updated patch soon.

>Also, you should update the kerneldoc title line
> for handle_control_request: the format is wrong and it contains a
> spelling error.  (The format of the kerneldoc for the other new
> routines in this patch is also wrong.)

Will be fixed in the next patch version.

> In addition, I suspect the dummy_hcd driver structure shouldn't contain
> an address_device entry.  It should be present only in dummy_ss_hcd.

I'm not sure I follow...
According to the code and comments in hub.c address_device cb is used if
the host controller wishes to choose the device address itself instead of
the address chosen by the core. It seems to me that there is nothing
preventing the dummy_hcd from supplying such cb as well. Is there?
Having both HS and SS dummy_hcd determine the device address seems more
convenient for testing proposes.

> Alan Stern
>
>


-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/