On Fri, Aug 7, 2015 at 8:31 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> On Fri, Aug 07, 2015 at 07:56:37PM +0300, Tal Shorer wrote:
>> On Fri, Aug 7, 2015 at 7:37 PM, Greg KH <gre...@linuxfoundation.org> wrote:
>> > On Fri, Aug 07, 2015 at 12:49:39PM +0300, Tal Shorer wrote:
>> >> In include/linux/usb/hcd.h, we have
>> >>
>> >> /* class requests from USB 3.0 hub spec, table 10-5 */
>> >> #define SetHubDepth (0x3000 | HUB_SET_DEPTH)
>> >> #define GetPortErrorCount (0x8000 | HUB_GET_PORT_ERR_COUNT)
>> >>
>> >> However, from the usb 3.1 spec I downloaded at
>> >> http://www.usb.org/developers/docs/,
>> >> table 10-5 appears to be unrelated (Table 10-5. Enhanced SuperSpeed
>> >> Hub Descriptor).
>> >
>> > Um, look at the comment you copied "3.0 hub spec", vs. the spec you
>> > downloaded and referred to "3.1 spec". Things might have changed :)
>> >
>> Yeah, my bad.
>> >> Table 10-7 (Table 10-7. Hub Class Requests) lists the two values as:
>> >> Request bmRequestType bRequest wValue wIndex wLength Data
>> >> SetHubDepth 00100000B SET_HUB_DEPTH Hub Depth Zero Zero None
>> >> GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of
>> >> Link Errors on this port
>> >>
>> >> It appears the correct bmRequestType values should be:
>> >> #define SetHubDepth (0x2000 | HUB_SET_DEPTH)
>> >> #define GetPortErrorCount (0xa300 | HUB_GET_PORT_ERR_COUNT)
>> >>
>> >> Which are the bmRequestType values of SetHubFeature and GetPortStatus
>> >> (and friends).
>> >>
>> >> These lines were introduced in
>> >> commit 0eadcc09203349b11ca477ec367079b23d32ab91
>> >> Author: Tatyana Brokhman <tlin...@codeaurora.org>
>> >> Date: Mon Nov 1 18:18:24 2010 +0200
>> >>
>> >> usb: USB3.0 ch11 definitions
>> >>
>> >> Adding hub SuperSpeed usb definitions as defined by ch10 of the USB3.0
>> >> spec.
>> >>
>> >> Signed-off-by: Tatyana Brokhman <tlin...@codeaurora.org>
>> >> Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>
>> >>
>> >> The values are only used by dummy_hcd which does nothing with them and
>> >> by max3421-hcd which returns an error.
>> >> Can anyone clarify what I'm missing or if the values are actually wrong?
>> >
>> > Can you compare the 3.1 to the 3.0 spec and see if they changed things
>> > here?
>> >
>> > thanks,
>> >
>> > greg k-h
>>
>> USB 3.0 spec download from
>> http://www.usb.org/developers/docs/documents_archive/
>> Table 10-6. Hub Class Requests
>> GetPortErrorCount 10000000B GET_PORT_ERR_COUNT Zero Port Two Number of
>> Link Errors on this port
>> SetHubDepth 00100000B SET_HUB_DEPTH Hub Depth Zero Zero None
>>
>> It appears it wasn't changed from 3.0 to 3.1.
>> Looking at the code in hub.c, it appears root hubs are never called
>> with SetHubDepth.
>>
>> 1049 if (hdev->parent && hub_is_superspeed(hdev)) {
>> 1050 ret = usb_control_msg(hdev,
>> usb_sndctrlpipe(hdev, 0),
>> 1051 HUB_SET_DEPTH, USB_RT_HUB,
>> 1052 hdev->level - 1, 0, NULL, 0,
>> 1053 USB_CTRL_SET_TIMEOUT);
>>
>> There are no more uses of HUB_SET_DEPTH except for this one and the one in
>> hcd.h
>>
>> tal@tal-H87-D3H:~/Dev/linux|0 $ git grep HUB_SET_DEPTH
>> drivers/usb/core/hub.c: HUB_SET_DEPTH,
>> USB_RT_HUB,
>> include/linux/usb/hcd.h:#define SetHubDepth (0x3000 |
>> HUB_SET_DEPTH)
>> include/uapi/linux/usb/ch11.h:#define HUB_SET_DEPTH 12
>
> People use SetHubDepth today.
>
>> And HUB_GET_PORT_ERR_COUNT is never used by the hub driver:
>>
>> tal@tal-H87-D3H:~/Dev/linux|1 $ git grep HUB_GET_PORT_ERR_COUNT
>> include/linux/usb/hcd.h:#define GetPortErrorCount (0x8000 |
>> HUB_GET_PORT_ERR_COUNT)
>> include/uapi/linux/usb/ch11.h:#define HUB_GET_PORT_ERR_COUNT 13
>>
>> So even if the constant was right, root hubs using it would never
>> reach the code handling it.
>
> But again, GetPortErrorCount is being used.
Yes, but with the current implementation of hub.c, these uses will never
run. I don't really think it's a good idea to delete these uses, since
this feature might be implemented in the future.
>
>> Would you prefer to eliminate these completely or make them right in
>> case anyone uses them in the future?
>
> We should fix these if they were incorrect.
>
> Care to make up a patch?
Sure, I'll send it shortly.
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
