On Tue, 30 Jan 2018, Mathias Nyman wrote:
> On 26.01.2018 03:47, Yoshihiro Shimoda wrote:
> > I resent this email with adding Mathias, Alan and Greg as CC.
> >
> > -----Original Message-----
> > From: Yoshihiro Shimoda, Sent: Thursday, January 18, 2018 6:03 PM
> > To: linux-usb@xxxxxxxxxxxxxxx
> > Subject: Questions about "usb usb8-port1: cannot disable (err = -32)"
> >
> > Hi,
> >
> > My environment (R-Car H3) outputs the following error when I disconnected
> > a usb 3.0 SSD device from the xhci host controller.
> >
> > [ 267.755777] xhci-hcd ee000000.usb: Cannot set link state.
> > [ 267.761188] usb usb8-port1: cannot disable (err = -32)
> > [ 267.772166] usb 8-1: USB disconnect, device number 2
> >
> > But, the environment can detect connection again.
> >
> > I investigate this issue and found the following commit is related:
> > 37be6676 usb: hub: Fix auto-remount of safely removed or ejected USB-3 devices
> >
> > The xhci-hub.c has the following code:
> > if ((temp & PORT_PE) == 0 ||
> > (link_state > USB_SS_PORT_LS_U3)) {
> > xhci_warn(xhci, "Cannot set link state.\n");
> > goto error;
> > }
> >
> > And, the code above will be called by hub_set_port_link_state(..., USB_SS_PORT_LS_U3)
> > in hub_port_disable():
> > /*
> > * USB-3 does not have a similar link state as USB-2 that will avoid negotiating
> > * a connection with a plugged-in cable but will signal the host when the cable
> > * is unplugged. Disable remote wake and set link state to U3 for USB-3 devices
> > */
> > static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
> > {
> > struct usb_port *port_dev = hub->ports[port1 - 1];
> > struct usb_device *hdev = hub->hdev;
> > int ret = 0;
> >
> > if (!hub->error) {
> > if (hub_is_superspeed(hub->hdev)) {
> > hub_usb3_port_prepare_disable(hub, port_dev);
> > ret = hub_set_port_link_state(hub, port_dev->portnum,
> > USB_SS_PORT_LS_U3);
> >
> > The hub_port_disable() will be called by port_event():
> > /*
> > * Warm reset a USB3 protocol port if it's in
> > * SS.Inactive state.
> > */
> > if (hub_port_warm_reset_required(hub, port1, portstatus)) {
> > dev_dbg(&port_dev->dev, "do warm reset\n");
> > if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION)
> > || udev->state == USB_STATE_NOTATTACHED) {
> > if (hub_port_reset(hub, port1, NULL,
> > HUB_BH_RESET_TIME, true) < 0)
> > hub_port_disable(hub, port1, 1);
> >
> > However, according to Figure 35 in xhci spec, the PED will be set to 0
> > after the usb3 root hub enters "Error" or "Disconnected" state.
> > So, IIUC, a usb driver should not call hub_set_port_link_state() in such a case.
> >
> > Question 1:
> > - Is my understanding correct?
>
> Yes, The idea behind using hub_set_port_link_state(U3) in hub_port_disable() is for
> cases when device is _not_ physically unplugged, but instead user does a "eject" for
> a usb mass storage device which will end up calling hub_port_disable() as well.
> Setting it to U3 will prevent a "ejected" device from being re-enumerated and mounted
> one second later.
>
> >
> > Question 2:
> > - How do I resolve the issue? (I don't have any idea for now...)
>
> Is there any other issue than the error messags?
>
> we could read the portstatus in hub_port_disable() and only set the port link state
> to U3 if the port is in currently in a suitable (U0) state.
>
> Or then just make sure xhci_hub_control() doesn't print out any warning when setting
> link state to U3 when PORT_PE is not set, and silence hub_port_disable() messages as well.
I like the second idea. If xhci_hub_control() returned -ENODEV when
asked to set the USB_PORT_FEAT_LINK_STATE feature to U3 and PORT_PE
was clear, then hub_port_disable() wouldn't complain.
Alan Stern
--
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
