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.

> 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?

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

Reply via email to