SRU reguest for lp#254783

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

SRU reguest for lp#254783

Stefan Bader-2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/254783

Note: the patch is mostly a cherry-pick but needed one function moved around.

SRU justification:

Impact: The USB-PERSIST feature (0458d5b4c9cc4ca0f62625d0144ddc4b4bc97a3c) lets
USB mounted filesystems remain intact (the USB device not removed) during power
loss. This is important for small machines that have their root fs on a USB
device. However this did not work in all cases.

Fix: A patch (5e6effaed6da94e727cd45f945ad2489af8570b3) that followed later
fixed the persist problem to work on all sleep states instead of supporting
only those where the hub lost power.

Testcase: Suspend fails on the classmate PC without this patch.

- --

When all other means of communication fail, try words!


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIl38SP+TjRTJVqvQRApsNAKC1lUMN73pcw23zw7tLEm+mzEvikQCgszNe
1Et0n+U/Bk1puP9MG3uv0+w=
=TPeX
-----END PGP SIGNATURE-----

From b81441bc55d46c30e84c7d4fb15a8bf40ddb4726 Mon Sep 17 00:00:00 2001
From: Stefan Bader <[hidden email]>
Date: Thu, 31 Jul 2008 18:09:54 -0400
Subject: [PATCH] UBUNTU: Backport make USB-PERSIST work after every system sleep

commit 5e6effaed6da94e727cd45f945ad2489af8570b3
Author: Alan Stern <[hidden email]>
Date:   Mon Mar 3 15:15:51 2008 -0500

USB: make USB-PERSIST work after every system sleep

This patch (as1046) makes USB-PERSIST work more in accordance with
the documentation.  Currently it takes effect only in cases where the
root hub has lost power or been reset, but it is supposed to operate
whenever a power session was dropped during a system sleep.

A new hub_restart() routine carries out the duties required during a
reset or a reset-resume.  It checks to see whether occupied ports are
still enabled, and if they aren't then it clears the enable-change and
connect-change features (to prevent interference by khubd) and sets
the child device's reset_resume flag.  It also checks ports that are
supposed to be unoccupied to verify that the firmware hasn't left the
port in an enabled state.

Signed-off-by: Alan Stern <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
Signed-off-by: Stefan Bader <[hidden email]>
---
 drivers/usb/core/hub.c |  157 +++++++++++++++++++++++++++++++-----------------
 1 files changed, 101 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 1b17f63..7fba625 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -327,6 +327,27 @@ static int get_port_status(struct usb_device *hdev, int port1,
  return status;
 }
 
+static int hub_port_status(struct usb_hub *hub, int port1,
+       u16 *status, u16 *change)
+{
+ int ret;
+
+ mutex_lock(&hub->status_mutex);
+ ret = get_port_status(hub->hdev, port1, &hub->status->port);
+ if (ret < 4) {
+ dev_err (hub->intfdev,
+ "%s failed (err = %d)\n", __FUNCTION__, ret);
+ if (ret >= 0)
+ ret = -EIO;
+ } else {
+ *status = le16_to_cpu(hub->status->port.wPortStatus);
+ *change = le16_to_cpu(hub->status->port.wPortChange);
+ ret = 0;
+ }
+ mutex_unlock(&hub->status_mutex);
+ return ret;
+}
+
 static void kick_khubd(struct usb_hub *hub)
 {
  unsigned long flags;
@@ -602,6 +623,81 @@ static void hub_port_logical_disconnect(struct usb_hub *hub, int port1)
  kick_khubd(hub);
 }
 
+#define HUB_RESET 1
+#define HUB_RESUME 2
+#define HUB_RESET_RESUME 3
+
+#ifdef CONFIG_PM
+
+static void hub_restart(struct usb_hub *hub, int type)
+{
+ struct usb_device *hdev = hub->hdev;
+ int port1;
+
+ /* Check each of the children to see if they require
+ * USB-PERSIST handling or disconnection.  Also check
+ * each unoccupied port to make sure it is still disabled.
+ */
+ for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
+ struct usb_device *udev = hdev->children[port1-1];
+ int status = 0;
+ u16 portstatus, portchange;
+
+ if (!udev || udev->state == USB_STATE_NOTATTACHED) {
+ if (type != HUB_RESET) {
+ status = hub_port_status(hub, port1,
+ &portstatus, &portchange);
+ if (status == 0 && (portstatus &
+ USB_PORT_STAT_ENABLE))
+ clear_port_feature(hdev, port1,
+ USB_PORT_FEAT_ENABLE);
+ }
+ continue;
+ }
+
+ /* Was the power session lost while we were suspended? */
+ switch (type) {
+ case HUB_RESET_RESUME:
+ portstatus = 0;
+ portchange = USB_PORT_STAT_C_CONNECTION;
+ break;
+
+ case HUB_RESET:
+ case HUB_RESUME:
+ status = hub_port_status(hub, port1,
+ &portstatus, &portchange);
+ break;
+ }
+
+ /* For "USB_PERSIST"-enabled children we must
+ * mark the child device for reset-resume and
+ * turn off the various status changes to prevent
+ * khubd from disconnecting it later.
+ */
+ if (USB_PERSIST && udev->persist_enabled && status == 0 &&
+ !(portstatus & USB_PORT_STAT_ENABLE)) {
+ if (portchange & USB_PORT_STAT_C_ENABLE)
+ clear_port_feature(hub->hdev, port1,
+ USB_PORT_FEAT_C_ENABLE);
+ if (portchange & USB_PORT_STAT_C_CONNECTION)
+ clear_port_feature(hub->hdev, port1,
+ USB_PORT_FEAT_C_CONNECTION);
+ udev->reset_resume = 1;
+ }
+
+ /* Otherwise for a reset_resume we must disconnect the child,
+ * but as we may not lock the child device here
+ * we have to do a "logical" disconnect.
+ */
+ else if (type == HUB_RESET_RESUME)
+ hub_port_logical_disconnect(hub, port1);
+ }
+
+ hub_activate(hub);
+}
+
+#endif /* CONFIG_PM */
+
 /* caller has locked the hub device */
 static int hub_pre_reset(struct usb_interface *intf)
 {
@@ -1490,28 +1586,6 @@ out_authorized:
 }
 
 
-static int hub_port_status(struct usb_hub *hub, int port1,
-       u16 *status, u16 *change)
-{
- int ret;
-
- mutex_lock(&hub->status_mutex);
- ret = get_port_status(hub->hdev, port1, &hub->status->port);
- if (ret < 4) {
- dev_err (hub->intfdev,
- "%s failed (err = %d)\n", __FUNCTION__, ret);
- if (ret >= 0)
- ret = -EIO;
- } else {
- *status = le16_to_cpu(hub->status->port.wPortStatus);
- *change = le16_to_cpu(hub->status->port.wPortChange);
- ret = 0;
- }
- mutex_unlock(&hub->status_mutex);
- return ret;
-}
-
-
 /* Returns 1 if @hub is a WUSB root hub, 0 otherwise */
 static unsigned hub_is_wusb(struct usb_hub *hub)
 {
@@ -1989,49 +2063,20 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
 
 static int hub_resume(struct usb_interface *intf)
 {
- struct usb_hub *hub = usb_get_intfdata (intf);
-
- dev_dbg(&intf->dev, "%s\n", __FUNCTION__);
+ struct usb_hub *hub = usb_get_intfdata(intf);
 
- /* tell khubd to look for changes on this hub */
- hub_activate(hub);
+ dev_dbg(&intf->dev, "%s\n", __func__);
+ hub_restart(hub, HUB_RESUME);
  return 0;
 }
 
 static int hub_reset_resume(struct usb_interface *intf)
 {
  struct usb_hub *hub = usb_get_intfdata(intf);
- struct usb_device *hdev = hub->hdev;
- int port1;
 
+ dev_dbg(&intf->dev, "%s\n", __func__);
  hub_power_on(hub);
-
- for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
- struct usb_device *child = hdev->children[port1-1];
-
- if (child) {
-
- /* For "USB_PERSIST"-enabled children we must
- * mark the child device for reset-resume and
- * turn off the connect-change status to prevent
- * khubd from disconnecting it later.
- */
- if (USB_PERSIST && child->persist_enabled) {
- child->reset_resume = 1;
- clear_port_feature(hdev, port1,
- USB_PORT_FEAT_C_CONNECTION);
-
- /* Otherwise we must disconnect the child,
- * but as we may not lock the child device here
- * we have to do a "logical" disconnect.
- */
- } else {
- hub_port_logical_disconnect(hub, port1);
- }
- }
- }
-
- hub_activate(hub);
+ hub_restart(hub, HUB_RESET_RESUME);
  return 0;
 }
 
--
1.5.4.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: SRU reguest for lp#254783

Ben Collins-4
On Mon, 2008-08-04 at 18:13 -0400, Stefan Bader wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/254783
>
> Note: the patch is mostly a cherry-pick but needed one function moved around.
>
> SRU justification:
>
> Impact: The USB-PERSIST feature (0458d5b4c9cc4ca0f62625d0144ddc4b4bc97a3c) lets
> USB mounted filesystems remain intact (the USB device not removed) during power
> loss. This is important for small machines that have their root fs on a USB
> device. However this did not work in all cases.
>
> Fix: A patch (5e6effaed6da94e727cd45f945ad2489af8570b3) that followed later
> fixed the persist problem to work on all sleep states instead of supporting
> only those where the hub lost power.
>
> Testcase: Suspend fails on the classmate PC without this patch.

Instead of moving the function, can we just get a forward declaration?
Will make it easier to see how much is really changing in this diff.

Thanks

> - --
>
> When all other means of communication fail, try words!
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFIl38SP+TjRTJVqvQRApsNAKC1lUMN73pcw23zw7tLEm+mzEvikQCgszNe
> 1Et0n+U/Bk1puP9MG3uv0+w=
> =TPeX
> -----END PGP SIGNATURE-----


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: SRU reguest for lp#254783

Stefan Bader-2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ben Collins wrote:

> On Mon, 2008-08-04 at 18:13 -0400, Stefan Bader wrote:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/254783
>
> Note: the patch is mostly a cherry-pick but needed one function moved around.
>
> SRU justification:
>
> Impact: The USB-PERSIST feature (0458d5b4c9cc4ca0f62625d0144ddc4b4bc97a3c) lets
> USB mounted filesystems remain intact (the USB device not removed) during power
> loss. This is important for small machines that have their root fs on a USB
> device. However this did not work in all cases.
>
> Fix: A patch (5e6effaed6da94e727cd45f945ad2489af8570b3) that followed later
> fixed the persist problem to work on all sleep states instead of supporting
> only those where the hub lost power.
>
> Testcase: Suspend fails on the classmate PC without this patch.
>
>> Instead of moving the function, can we just get a forward declaration?
>> Will make it easier to see how much is really changing in this diff.
>
>> Thanks
>
Tried to keep close to upstream, which moved the function (but not with the
same commit). But I changed that to a forward declaration.

Applicable: Hardy only

- --

When all other means of communication fail, try words!


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFImLDnP+TjRTJVqvQRAuESAKCustLkKfJ27q/SlAJGW0TqPeh/uACg6dcB
Eaqw/O/mH0M71kcRDuFiPC8=
=9oOZ
-----END PGP SIGNATURE-----

From ad2c02caeaf7c73a02241ba22eb60b2c9ff5e20e Mon Sep 17 00:00:00 2001
From: Stefan Bader <[hidden email]>
Date: Tue, 5 Aug 2008 15:31:35 -0400
Subject: [PATCH] UBUNTU: Backport make USB-PERSIST work after every system sleep

Bug: #254783

commit 5e6effaed6da94e727cd45f945ad2489af8570b3
Author: Alan Stern <[hidden email]>
Date:   Mon Mar 3 15:15:51 2008 -0500

USB: make USB-PERSIST work after every system sleep

This patch (as1046) makes USB-PERSIST work more in accordance with
the documentation.  Currently it takes effect only in cases where the
root hub has lost power or been reset, but it is supposed to operate
whenever a power session was dropped during a system sleep.

A new hub_restart() routine carries out the duties required during a
reset or a reset-resume.  It checks to see whether occupied ports are
still enabled, and if they aren't then it clears the enable-change and
connect-change features (to prevent interference by khubd) and sets
the child device's reset_resume flag.  It also checks ports that are
supposed to be unoccupied to verify that the firmware hasn't left the
port in an enabled state.

Signed-off-by: Alan Stern <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
Signed-off-by: Stefan Bader <[hidden email]>
---
 drivers/usb/core/hub.c |  117 ++++++++++++++++++++++++++++++++++--------------
 1 files changed, 83 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 1b17f63..e7dab8e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -327,6 +327,9 @@ static int get_port_status(struct usb_device *hdev, int port1,
  return status;
 }
 
+static int hub_port_status(struct usb_hub *hub, int port1,
+       u16 *status, u16 *change);
+
 static void kick_khubd(struct usb_hub *hub)
 {
  unsigned long flags;
@@ -602,6 +605,81 @@ static void hub_port_logical_disconnect(struct usb_hub *hub, int port1)
  kick_khubd(hub);
 }
 
+#define HUB_RESET 1
+#define HUB_RESUME 2
+#define HUB_RESET_RESUME 3
+
+#ifdef CONFIG_PM
+
+static void hub_restart(struct usb_hub *hub, int type)
+{
+ struct usb_device *hdev = hub->hdev;
+ int port1;
+
+ /* Check each of the children to see if they require
+ * USB-PERSIST handling or disconnection.  Also check
+ * each unoccupied port to make sure it is still disabled.
+ */
+ for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
+ struct usb_device *udev = hdev->children[port1-1];
+ int status = 0;
+ u16 portstatus, portchange;
+
+ if (!udev || udev->state == USB_STATE_NOTATTACHED) {
+ if (type != HUB_RESET) {
+ status = hub_port_status(hub, port1,
+ &portstatus, &portchange);
+ if (status == 0 && (portstatus &
+ USB_PORT_STAT_ENABLE))
+ clear_port_feature(hdev, port1,
+ USB_PORT_FEAT_ENABLE);
+ }
+ continue;
+ }
+
+ /* Was the power session lost while we were suspended? */
+ switch (type) {
+ case HUB_RESET_RESUME:
+ portstatus = 0;
+ portchange = USB_PORT_STAT_C_CONNECTION;
+ break;
+
+ case HUB_RESET:
+ case HUB_RESUME:
+ status = hub_port_status(hub, port1,
+ &portstatus, &portchange);
+ break;
+ }
+
+ /* For "USB_PERSIST"-enabled children we must
+ * mark the child device for reset-resume and
+ * turn off the various status changes to prevent
+ * khubd from disconnecting it later.
+ */
+ if (USB_PERSIST && udev->persist_enabled && status == 0 &&
+ !(portstatus & USB_PORT_STAT_ENABLE)) {
+ if (portchange & USB_PORT_STAT_C_ENABLE)
+ clear_port_feature(hub->hdev, port1,
+ USB_PORT_FEAT_C_ENABLE);
+ if (portchange & USB_PORT_STAT_C_CONNECTION)
+ clear_port_feature(hub->hdev, port1,
+ USB_PORT_FEAT_C_CONNECTION);
+ udev->reset_resume = 1;
+ }
+
+ /* Otherwise for a reset_resume we must disconnect the child,
+ * but as we may not lock the child device here
+ * we have to do a "logical" disconnect.
+ */
+ else if (type == HUB_RESET_RESUME)
+ hub_port_logical_disconnect(hub, port1);
+ }
+
+ hub_activate(hub);
+}
+
+#endif /* CONFIG_PM */
+
 /* caller has locked the hub device */
 static int hub_pre_reset(struct usb_interface *intf)
 {
@@ -1989,49 +2067,20 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
 
 static int hub_resume(struct usb_interface *intf)
 {
- struct usb_hub *hub = usb_get_intfdata (intf);
-
- dev_dbg(&intf->dev, "%s\n", __FUNCTION__);
+ struct usb_hub *hub = usb_get_intfdata(intf);
 
- /* tell khubd to look for changes on this hub */
- hub_activate(hub);
+ dev_dbg(&intf->dev, "%s\n", __func__);
+ hub_restart(hub, HUB_RESUME);
  return 0;
 }
 
 static int hub_reset_resume(struct usb_interface *intf)
 {
  struct usb_hub *hub = usb_get_intfdata(intf);
- struct usb_device *hdev = hub->hdev;
- int port1;
 
+ dev_dbg(&intf->dev, "%s\n", __func__);
  hub_power_on(hub);
-
- for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
- struct usb_device *child = hdev->children[port1-1];
-
- if (child) {
-
- /* For "USB_PERSIST"-enabled children we must
- * mark the child device for reset-resume and
- * turn off the connect-change status to prevent
- * khubd from disconnecting it later.
- */
- if (USB_PERSIST && child->persist_enabled) {
- child->reset_resume = 1;
- clear_port_feature(hdev, port1,
- USB_PORT_FEAT_C_CONNECTION);
-
- /* Otherwise we must disconnect the child,
- * but as we may not lock the child device here
- * we have to do a "logical" disconnect.
- */
- } else {
- hub_port_logical_disconnect(hub, port1);
- }
- }
- }
-
- hub_activate(hub);
+ hub_restart(hub, HUB_RESET_RESUME);
  return 0;
 }
 
--
1.5.4.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: SRU reguest for lp#254783

Ben Collins-4
On Tue, 2008-08-05 at 15:58 -0400, Stefan Bader wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ben Collins wrote:
> > On Mon, 2008-08-04 at 18:13 -0400, Stefan Bader wrote:
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/254783
> >
> > Note: the patch is mostly a cherry-pick but needed one function moved around.
> >
> > SRU justification:
> >
> > Impact: The USB-PERSIST feature (0458d5b4c9cc4ca0f62625d0144ddc4b4bc97a3c) lets
> > USB mounted filesystems remain intact (the USB device not removed) during power
> > loss. This is important for small machines that have their root fs on a USB
> > device. However this did not work in all cases.
> >
> > Fix: A patch (5e6effaed6da94e727cd45f945ad2489af8570b3) that followed later
> > fixed the persist problem to work on all sleep states instead of supporting
> > only those where the hub lost power.
> >
> > Testcase: Suspend fails on the classmate PC without this patch.
> >
> >> Instead of moving the function, can we just get a forward declaration?
> >> Will make it easier to see how much is really changing in this diff.
> >
> >> Thanks
> >
>
> Tried to keep close to upstream, which moved the function (but not with the
> same commit). But I changed that to a forward declaration.
>
> Applicable: Hardy only

ACK on this version of the patch.

> - --
>
> When all other means of communication fail, try words!
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFImLDnP+TjRTJVqvQRAuESAKCustLkKfJ27q/SlAJGW0TqPeh/uACg6dcB
> Eaqw/O/mH0M71kcRDuFiPC8=
> =9oOZ
> -----END PGP SIGNATURE-----


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team