Re: Values for SetHubDepth and GetPortErrorCount

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 7, 2015 at 8:31 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 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 <gregkh@xxxxxxxxxxxxxxxxxxx> 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 <tlinder@xxxxxxxxxxxxxx>
>> >> 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 <tlinder@xxxxxxxxxxxxxx>
>> >>     Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
>> >>
>> >> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux