On Tue, Sep 13, 2011 at 09:11:00PM +0200, Harald Brennich wrote:
> Hello Sarah,
> Sorry, the third patch (adding PORT_WRC to mask in
> xhci_hub_status_data) does not make xhci detect the disconnect.
Please send me your dmesg with the three patches applied (and none of
your own changes, please).
Also, please reply-to-all, as you keep dropping the Linux USB mailing
list from your responses. Since bugzilla isn't working, it's best to
have the tests documented in the mailing list archives so we can add a
link to this thread later.
Sarah Sharp
> Am 13.09.2011, 19:21 Uhr, schrieb Sarah Sharp
> <sarah.a.sharp@xxxxxxxxxxxxxxx>:
>
> >On Tue, Sep 13, 2011 at 06:01:32PM +0200, Harald Brennich wrote:
> >>Hello Sarah,
> >>
> >>The patches you mailed me fix
> >>Bug 41752 - USB 3.0 port does not recognize external disk
> >
> >Ok, great!
> >
> >>However,
> >>[Bug 42472] USB 3.0 port does not detect disconnect
> >>is not fixed.
> >>I however succeeded by replacing
> >> if (portchange & USB_PORT_STAT_C_BH_RESET)
> >> return 0;
> >>in the warm reset branch of the patch by
> >> if (portchange & USB_PORT_STAT_C_BH_RESET) {
> >> /* clear link state and connection interrupts */
> >> if (portchange & USB_PORT_STAT_C_LINK_STATE)
> >> clear_port_feature(hub->hdev, port1,
> >> USB_PORT_FEAT_C_PORT_LINK_STATE);
> >> if (portchange & USB_PORT_STAT_C_CONNECTION) {
> >> clear_port_feature(hub->hdev, port1,
> >> USB_PORT_FEAT_C_CONNECTION);
> >> dev_warn(hub->intfdev,
> >> "device on port %d bounced (status = 0x%x) during wait for reset
> >>(%d msec)\n",
> >> port1, portstatus, delay_time);
> >> }
> >> return 0;
> >> }
> >
> >I think you might also need the attached patch; please apply
> >it on top of the other two.
> >
> >>Will the patches you mailed me by incorporated in some future kernel
> >>version?
> >
> >Yes, I will send all three patches off to Greg, and it will be in the
> >3.2 kernel release. However, I think Andiry's warm reset extension is a
> >bit big to backport to the stable kernels.
> >
> >Sarah Sharp
> >
> >>Am 12.09.2011, 21:24 Uhr, schrieb Sarah Sharp
> >><sarah.a.sharp@xxxxxxxxxxxxxxx>:
> >>
> >>>Hi Harald,
> >>>
> >>>It looks like the warm port reset took longer to complete than the USB
> >>>core expected. The xHCI driver tried to issue an address
> >>device command
> >>>while the reset was still going on, so it's no wonder it failed.
> >>>
> >>>Can you try applying the attached patches? The first one extends the
> >>>time the USB core will wait for a warm reset, and the second one makes
> >>>the USB core issue a warm reset if a disconnect is reported with the
> >>>link in Inactive during a hot reset.
> >>>
> >>>Sarah Sharp
> >>>
> >>>On Thu, Sep 08, 2011 at 08:05:54AM -0700, Sarah Sharp wrote:
> >>>>(Switching to email)
> >>>>
> >>>>On Thu, Sep 08, 2011 at 12:04:06PM +0000,
> >>>>bugzilla-daemon@xxxxxxxxxxxxxxxxxxx wrote:
> >>>>> https://bugzilla.kernel.org/show_bug.cgi?id=42472
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --- Comment #2 from Harald Brennich <harald.brennich@xxxxxx>
> >>>>2011-09-08 12:03:56 ---
> >>>>> Hello Sarah,
> >>>>> the Renesas (formerly NEC) uPD720200 USB 3.0 Host works fine
> >>>>with the following
> >>>>> change in function hub_port_wait_reset:
> >>>>
> >>>>Look, I really don't think this is the correct solution. If a device
> >>>>disconnects in the middle of a port reset, then we need to treat it as
> >>>>if it's a new device. Someone could have yanked out the device during
> >>>>the port reset and inserted a new one.
> >>>>
> >>>>Let me take a look at why the warm port reset patch didn't
> >>work and get
> >>>>back to you next week, after I'm back from a conference.
> >>>>
> >>>>Do other USB 3.0 devices work under this host controller? Is it just
> >>>>this device?
> >>>>
> >>>>Sarah Sharp
> >>>>
> >>>>> Current version:
> >>>>> static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> >>>>> struct usb_device *udev, unsigned int delay)
> >>>>> {
> >>>>> int delay_time, ret;
> >>>>> u16 portstatus;
> >>>>> u16 portchange;
> >>>>>
> >>>>> for (delay_time = 0;
> >>>>> delay_time < HUB_RESET_TIMEOUT;
> >>>>> delay_time += delay) {
> >>>>> /* wait to give the device a chance to reset */
> >>>>> msleep(delay);
> >>>>>
> >>>>> /* read and decode port status */
> >>>>> ret = hub_port_status(hub, port1, &portstatus, &portchange);
> >>>>> if (ret < 0)
> >>>>> return ret;
> >>>>>
> >>>>> /* Device went away? */
> >>>>> if (!(portstatus & USB_PORT_STAT_CONNECTION))
> >>>>> return -ENOTCONN;
> >>>>>
> >>>>> /* bomb out completely if the connection bounced */
> >>>>> if ((portchange & USB_PORT_STAT_C_CONNECTION))
> >>>>> return -ENOTCONN;
> >>>>>
> >>>>> /* if we`ve finished resetting, then break out of
> >>the loop */
> >>>>> if (!(portstatus & USB_PORT_STAT_RESET) &&
> >>>>> (portstatus & USB_PORT_STAT_ENABLE)) {
> >>>>> if (hub_is_wusb(hub))
> >>>>> udev->speed = USB_SPEED_WIRELESS;
> >>>>> else if (hub_is_superspeed(hub->hdev))
> >>>>> udev->speed = USB_SPEED_SUPER;
> >>>>> else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
> >>>>> udev->speed = USB_SPEED_HIGH;
> >>>>> else if (portstatus & USB_PORT_STAT_LOW_SPEED)
> >>>>> udev->speed = USB_SPEED_LOW;
> >>>>> else
> >>>>> udev->speed = USB_SPEED_FULL;
> >>>>> return 0;
> >>>>> }
> >>>>>
> >>>>> /* switch to the long delay after two short delay
> >>failures */
> >>>>> if (delay_time >= 2 * HUB_SHORT_RESET_TIME)
> >>>>> delay = HUB_LONG_RESET_TIME;
> >>>>>
> >>>>> dev_dbg (hub->intfdev,
> >>>>> "port %d not reset yet, waiting %dms\n",
> >>>>> port1, delay);
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> Modified version:
> >>>>> In the first modification (see report 41752), the portchange
> >>>>was ignored. Then
> >>>>> the external drive could be used, but the disconnect was not
> >>>>detected. This is
> >>>>> now resolved with checking portstatus for
> >>>>USB_PORT_STAT_C_CONNECTION and
> >>>>> USB_PORT_FEAT_C_PORT_LINK_STATE (the flags set by the uPD720200).
> >>>>> Is there a part in the USB spec that disallows disconnecting a
> >>>>port by the hub
> >>>>> during RESET? Or do you have another solution?
> >>>>>
> >>>>> static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> >>>>> struct usb_device *udev, unsigned int delay)
> >>>>> {
> >>>>> int delay_time, ret;
> >>>>> u16 portstatus;
> >>>>> u16 portchange;
> >>>>>
> >>>>> for (delay_time = 0;
> >>>>> delay_time < HUB_RESET_TIMEOUT;
> >>>>> delay_time += delay) {
> >>>>> /* wait to give the device a chance to reset */
> >>>>> msleep(delay);
> >>>>>
> >>>>> /* read and decode port status */
> >>>>> ret = hub_port_status(hub, port1, &portstatus, &portchange);
> >>>>> if (ret < 0)
> >>>>> return ret;
> >>>>>
> >>>>> /* if we`ve finished resetting, then break out of
> >>the loop */
> >>>>> if (!(portstatus & USB_PORT_STAT_RESET) &&
> >>>>> (portstatus & USB_PORT_STAT_ENABLE)) {
> >>>>> /* ADDED 2011.09.08 BRE BEGIN */
> >>>>> /* Device went away? */
> >>>>> if (!(portstatus & USB_PORT_STAT_CONNECTION))
> >>>>> return -ENOTCONN;
> >>>>> if (portchange & USB_PORT_STAT_C_CONNECTION)
> >>>>> clear_port_feature(hub->hdev, port1,
> >>>>> USB_PORT_FEAT_C_CONNECTION);
> >>>>> if (portchange & USB_PORT_STAT_C_LINK_STATE)
> >>>>> clear_port_feature(hub->hdev, port1,
> >>>>> USB_PORT_FEAT_C_PORT_LINK_STATE);
> >>>>> /* ADDED 2011.09.08 BRE END */
> >>>>> if (hub_is_wusb(hub))
> >>>>> udev->speed = USB_SPEED_WIRELESS;
> >>>>> else if (hub_is_superspeed(hub->hdev))
> >>>>> udev->speed = USB_SPEED_SUPER;
> >>>>> else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
> >>>>> udev->speed = USB_SPEED_HIGH;
> >>>>> else if (portstatus & USB_PORT_STAT_LOW_SPEED)
> >>>>> udev->speed = USB_SPEED_LOW;
> >>>>> else
> >>>>> udev->speed = USB_SPEED_FULL;
> >>>>> return 0;
> >>>>> }
> >>>>>
> >>>>> /* switch to the long delay after two short delay
> >>failures */
> >>>>> if (delay_time >= 2 * HUB_SHORT_RESET_TIME)
> >>>>> delay = HUB_LONG_RESET_TIME;
> >>>>>
> >>>>> dev_dbg (hub->intfdev,
> >>>>> "port %d not reset yet, waiting %dms\n",
> >>>>> port1, delay);
> >>>>> }
> >>>>>
> >>>>> /* ADDED 2011.09.05 BRE BEGIN */
> >>>>> /* Device went away? */
> >>>>> if (!(portstatus & USB_PORT_STAT_CONNECTION))
> >>>>> return -ENOTCONN;
> >>>>> /* bomb out completely if the connection bounced */
> >>>>> if ((portchange & USB_PORT_STAT_C_CONNECTION))
> >>>>> return -ENOTCONN;
> >>>>> /* ADDED 2011.09.05 BRE END */
> >>>>> return -EBUSY;
> >>>>> }
> >>>>>
> >>>>> --
> >>>>> Configure bugmail:
> >>https://bugzilla.kernel.org/userprefs.cgi?tab=email
> >>>>> ------- You are receiving this mail because: -------
> >>>>> You are the assignee for the bug.
> >>>>--
> >>>>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
> >>
> >>
> >>--
> >>Erstellt mit Operas revolutionärem E-Mail-Modul:
> >>http://www.opera.com/mail/
>
>
> --
> Erstellt mit Operas revolutionärem E-Mail-Modul: http://www.opera.com/mail/
--
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
