[RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

hi all:
	Currently, we are working on usb port power off mechanism. Our developing
machine provides usb port power control (a vbus switch)via ACPI power resource.
When the power resource turns off, usb port powers off and usb device loses
power. From usb hub side, just like the device being unplugged.

	Since usb port power off will affect hot-plug and devices remote wakeup
function, it should be careful to do that.
	We conclude three different situations for power off mechanism.
	(1) hard-wired port with device
	(2) hot-pluggable port without device
	(3) hot-pluggable port with device

For hard-wired port, the device will not be removed physically. So we can
power off it when device is suspended and remote wakeup is disabled without
concerning with hot-plug. This patch is dedicated to this siutation.

This patch is to provide usb acpi power control method and call them in the
usb_port_suspend() and usb_port_resume() when port can be power off. When the
usb port is in the power off state, usb core doesn't remove device which is
attached to the port. The device is still on the system and user can access
the device.

introduce three port's states.

USB_PORT_POWER_STATE_ON
USB_PORT_WAITING_FOR_CONNECTION
USB_PORT_POWER_STATE_OFF

"on"
	port power on

"waiting for connection"
	port power on but hub port has not detected the device or detect event has
not been processed.

"off"
	port power off

At first, port's state is "on". When the device is suspended, power off the port and
set port's state to "off". After the port powering off, the usb hub will detect a
connection change event. Normally, the device will be removed with regarding as being
unplugged. But in the power off mechanism, the device is still on the port and user
can still access the device. So ignore the event.

When the device is resumed, turn on the power resource and set port's state to
"waiting for connection". After the port powering on, the usb hub will detect a
connection change event which originally means a device plugged in and previous
device will be removed. But in the power offmechanism, the device is not changed
and so ignore the event. When port's state is "waiting for connection", receive
an event "connection" and the port's connection state is on. This means the usb
the device is detected by usb hub again after powering on port. Set port's state
to "on".

 "on"
  |
 "off"
  |
 "waiting for connection"
  |
 "on"

"waiting for connection" state is to avoid device to being removed.
If set to "on" after powering on, the connection event may not be processed at that
time. When it is processed, the port's state has been "on" and the device will be
removed. So introduce "waiting for connection" state.

We also have a proposal to add sys file for each port to control port power off
under usb hub sys directory. If the port's power off is supported by platform,
create a sys file e.g "port1_power"  for port one. Echo "on" to "port1_power" is
to not allow port to be power off. Echo "auto" to "port1_power" is to power off
port if possible.

Different type ports have different default values.
(1) hard-wired port with device				"auto"
(2) hot-pluggable port without device		"on"
(3) hot-pluggable port with device			"on"

Add member port_power_control, can_power_off  to struct usb_hub_port. port_power_control
records user choice. Can_power_off means the platform and device support to power off.
When a device is attached, check whether port can be power off if yes set can_power_off
to true. When device driver is load, the driver also can set value to can_power_off. When
try to power off port, can_power_off and port_power_control should be taken into account.
Only when these two members  are set to true, the port could be power off.

sys file operation
port with device
 port1_power "auto" => "on" or "on" => "auto" implement
	pm_runtime_get_syn(udev)
	port_power_control = "auto" or "on"
	pm_runtime_put_syn(udev)

port without device
	port can power on or power power off directly.

Suggestion and comments more welcome.
---
 drivers/usb/core/hub.c      |   95 +++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/core/usb-acpi.c |   33 ++++++++++++++-
 2 files changed, 127 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 6c16ff5..d28d605 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -42,6 +42,7 @@ struct usb_hub_port {
 	struct usb_device	*child;
 	unsigned long		platform_data;
 	enum usb_port_connect_type connect_type;
+	unsigned		power_state:2; /* the power state of usb port */
 };
 
 struct usb_hub {
@@ -161,8 +162,14 @@ EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem);
 #define HUB_DEBOUNCE_STEP	  25
 #define HUB_DEBOUNCE_STABLE	 100
 
+#define USB_PORT_POWER_STATE_ON		0
+#define USB_PORT_WAITING_FOR_CONNECTION 1
+#define USB_PORT_POWER_STATE_OFF	2
+
+#define HUB_PORT_RECONNECT_TRIES	20
 
 static int usb_reset_and_verify_device(struct usb_device *udev);
+static int hub_port_debounce(struct usb_hub *hub, int port1);
 
 static inline char *portspeed(struct usb_hub *hub, int portstatus)
 {
@@ -2518,6 +2525,24 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 		usb_set_device_state(udev, USB_STATE_SUSPENDED);
 		msleep(10);
 	}
+
+	/*
+	 * Check whether the usb port has acpi power control method.
+	 * Devices on the motherboard can be power off without
+ 	 * considing hot-plug. When the device's remote wakeup is
+	 * enabled, it can't be power off since the function will
+	 * loss when power off.
+	 */
+	If (usb_acpi_power_manageable(hub->hdev, port1) &&
+		hub->port_data[port1 - 1].connect_type ==
+		USB_PORT_CONNECT_TYPE_HARD_WIRED &&
+		!udev->do_remote_wakeup && !status) {
+		status = usb_acpi_set_power_state(hub->hdev, port1, false);
+		if (!status)
+			hub->port_data[port1 - 1].power_state
+				= USB_PORT_POWER_STATE_OFF;
+	}
+
 	usb_mark_last_busy(hub->hdev);
 	return status;
 }
@@ -2602,6 +2627,23 @@ static int finish_port_resume(struct usb_device *udev)
 }
 
 /*
+ * There is a latency  between usb port power on and usb hub port
+ * connect detection. The latency depends on devices. This routine
+ * is to wait for connect within 20 tries.
+ */
+static int usb_port_wait_for_connected(struct usb_hub *hub, int port1)
+{
+	int status, i;
+
+	for (i = 0; i < HUB_PORT_RECONNECT_TRIES; i++) {
+		status = hub_port_debounce(hub, port1);
+		if (status & USB_PORT_STAT_CONNECTION)
+			return 0;
+	}
+	return -ETIMEDOUT;
+}
+
+/*
  * usb_port_resume - re-activate a suspended usb device's upstream port
  * @udev: device to re-activate, not a root hub
  * Context: must be able to sleep; device not locked; pm locks held
@@ -2642,6 +2684,37 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 	int		status;
 	u16		portchange, portstatus;
 
+	/*
+	 * Check whether the usb port has acpi power control method
+	 * and if its power state is not on, power on the usb port.
+	 */
+	if (usb_acpi_power_manageable(hub->hdev, port1)
+		&& hub->port_data[port1 - 1].power_state
+		!= USB_PORT_POWER_STATE_ON) {
+		status = usb_acpi_set_power_state(hub->hdev, port1, true);
+		if (status < 0)
+			return status;
+
+		/*
+		 * After powering on, the port state is set to "waiting
+		 * for connection".
+		 */
+		hub->port_data[port1 - 1].power_state
+			= USB_PORT_WAITING_FOR_CONNECTION;
+
+		/*
+		 * Wait for usb hub port to be reconnected in order to make
+		 * the resume procedure successful.
+		 */
+		status = usb_port_wait_for_connected(hub, port1);
+		if (status < 0) {
+			dev_dbg(&udev->dev, "can't get reconnection after" \
+				" setting  port on, status %d\n", status);
+			return status;
+		}
+
+	}
+
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
 	if (status == 0 && !port_is_suspended(hub, portstatus))
@@ -3362,6 +3435,28 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 		}
 	}
 
+	/*
+	 * When the usb port's state are power off, the device
+	 * should not be removed in order to resume it if necessary.
+	 * When the usb port's states are waiting for connection,
+	 * not remove device and check the usb hub port's connect
+	 * state. If it has been connected, set the usb port's state
+	 * "on".
+	 */
+	if (hub->port_data[port1 - 1].power_state == USB_PORT_POWER_STATE_OFF) {
+		clear_bit(port1, hub->change_bits);
+		return;
+	} else if (hub->port_data[port1 - 1].power_state
+			 == USB_PORT_WAITING_FOR_CONNECTION) {
+		if (portstatus & USB_PORT_STAT_CONNECTION
+		    && portchange & USB_PORT_STAT_C_CONNECTION) {
+			hub->port_data[port1 - 1].power_state
+				= USB_PORT_POWER_STATE_ON;
+		}
+		clear_bit(port1, hub->change_bits);
+		return;
+	}
+
 	/* Disconnect any existing devices under this port */
 	if (udev)
 		usb_disconnect(&hub->port_data[port1-1].child);
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 02739b47..3b091e8 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -19,6 +19,32 @@
 
 #include "usb.h"
 
+bool usb_acpi_power_manageable(struct usb_device *hdev, int port1)
+{
+	acpi_handle port_handle;
+
+	port_handle = (acpi_handle)usb_get_hub_port_platform_data(hdev,
+		port1);
+	return port_handle ? acpi_bus_power_manageable(port_handle) : false;
+}
+
+int usb_acpi_set_power_state(struct usb_device *hdev, int port1, bool enable)
+{
+	acpi_handle port_handle;
+	unsigned char state;
+	int error = -EINVAL;
+
+	port_handle = (acpi_handle)usb_get_hub_port_platform_data(hdev,
+		port1);
+	state = enable ? ACPI_STATE_D0 : ACPI_STATE_D3_COLD;
+	error = acpi_bus_set_power(port_handle, state);
+	if (!error)
+		dev_dbg(&hdev->dev, "The power of hub port %d was set to %s\n",
+			port1, enable ? "enable" : "disabe");
+
+	return error;
+}
+
 static int usb_acpi_check_port_connect_type(struct usb_device *hdev,
 	acpi_handle handle, int port1)
 {
@@ -55,9 +81,14 @@ static int usb_acpi_check_port_connect_type(struct usb_device *hdev,
 				pld.user_visible ?
 					USB_PORT_CONNECT_TYPE_HOT_PLUG :
 					USB_PORT_CONNECT_TYPE_HARD_WIRED);
-	else if (!pld.user_visible)
+	else if (!pld.user_visible) {
 		usb_set_hub_port_connect_type(hdev, port1, USB_PORT_NOT_USED);
 
+		/* Power off the usb port which may not be used.*/
+		if (usb_acpi_power_manageable(hdev, port1))
+			usb_acpi_set_power_state(hdev, port1, false);
+	}
+
 out:
 	kfree(upc);
 	return ret;
-- 
1.7.6.rc2.8.g28eb

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Thu, 10 May 2012, Lan Tianyu wrote:

> hi all:
> 	Currently, we are working on usb port power off mechanism. Our developing
> machine provides usb port power control (a vbus switch)via ACPI power resource.
> When the power resource turns off, usb port powers off and usb device loses
> power. From usb hub side, just like the device being unplugged.
> 
> 	Since usb port power off will affect hot-plug and devices remote wakeup
> function, it should be careful to do that.
> 	We conclude three different situations for power off mechanism.
> 	(1) hard-wired port with device
> 	(2) hot-pluggable port without device
> 	(3) hot-pluggable port with device
> 
> For hard-wired port, the device will not be removed physically. So we can
> power off it when device is suspended and remote wakeup is disabled without
> concerning with hot-plug. This patch is dedicated to this siutation.
> 
> This patch is to provide usb acpi power control method and call them in the
> usb_port_suspend() and usb_port_resume() when port can be power off. When the
> usb port is in the power off state, usb core doesn't remove device which is
> attached to the port. The device is still on the system and user can access
> the device.

Can you provide any examples where this would be useful?  It won't end 
up saving very much power (although on a laptop even a little bit might 
help).

> introduce three port's states.
> 
> USB_PORT_POWER_STATE_ON
> USB_PORT_WAITING_FOR_CONNECTION
> USB_PORT_POWER_STATE_OFF
> 
> "on"
> 	port power on
> 
> "waiting for connection"
> 	port power on but hub port has not detected the device or detect event has
> not been processed.

This state is not needed.

> "off"
> 	port power off
> 
> At first, port's state is "on". When the device is suspended, power off the port and
> set port's state to "off". After the port powering off, the usb hub will detect a
> connection change event. Normally, the device will be removed with regarding as being
> unplugged. But in the power off mechanism, the device is still on the port and user
> can still access the device. So ignore the event.
> 
> When the device is resumed, turn on the power resource and set port's state to
> "waiting for connection". After the port powering on, the usb hub will detect a
> connection change event which originally means a device plugged in and previous
> device will be removed. But in the power offmechanism, the device is not changed
> and so ignore the event. When port's state is "waiting for connection", receive
> an event "connection" and the port's connection state is on. This means the usb
> the device is detected by usb hub again after powering on port. Set port's state
> to "on".
> 
>  "on"
>   |
>  "off"
>   |
>  "waiting for connection"
>   |
>  "on"
> 
> "waiting for connection" state is to avoid device to being removed.

Why would the device be removed?

> If set to "on" after powering on, the connection event may not be processed at that
> time. When it is processed, the port's state has been "on" and the device will be
> removed. So introduce "waiting for connection" state.

Instead you should simply delay setting the state back to "on" until
the device has connected.

> We also have a proposal to add sys file for each port to control port power off
> under usb hub sys directory. If the port's power off is supported by platform,
> create a sys file e.g "port1_power"  for port one. Echo "on" to "port1_power" is
> to not allow port to be power off. Echo "auto" to "port1_power" is to power off
> port if possible.
> 
> Different type ports have different default values.
> (1) hard-wired port with device				"auto"
> (2) hot-pluggable port without device		"on"
> (3) hot-pluggable port with device			"on"
> 
> Add member port_power_control, can_power_off  to struct usb_hub_port. port_power_control
> records user choice. Can_power_off means the platform and device support to power off.
> When a device is attached, check whether port can be power off if yes set can_power_off
> to true. When device driver is load, the driver also can set value to can_power_off. When

That's no good.  What happens if the driver sets the value to false and 
then the driver is unloaded?

> try to power off port, can_power_off and port_power_control should be taken into account.
> Only when these two members  are set to true, the port could be power off.
> 
> sys file operation
> port with device
>  port1_power "auto" => "on" or "on" => "auto" implement
> 	pm_runtime_get_syn(udev)
> 	port_power_control = "auto" or "on"
> 	pm_runtime_put_syn(udev)
> 
> port without device
> 	port can power on or power power off directly.

We have no code for powering off ports that don't have a device.  If 
you did power off such a port, it would not be able to detect when a 
device was plugged in.

> Suggestion and comments more welcome.

Your code style for continuation lines is wrong.  Continuation lines 
should be indented more than one tab stop -- otherwise they look like 
regular lines in an inner block.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Thu, May 10, 2012 at 11:54:04AM -0400, Alan Stern wrote:
> On Thu, 10 May 2012, Lan Tianyu wrote:
> 
> > hi all:
> > 	Currently, we are working on usb port power off mechanism. Our developing
> > machine provides usb port power control (a vbus switch)via ACPI power resource.
> > When the power resource turns off, usb port powers off and usb device loses
> > power. From usb hub side, just like the device being unplugged.
> > 
> > 	Since usb port power off will affect hot-plug and devices remote wakeup
> > function, it should be careful to do that.
> > 	We conclude three different situations for power off mechanism.
> > 	(1) hard-wired port with device
> > 	(2) hot-pluggable port without device
> > 	(3) hot-pluggable port with device
> > 
> > For hard-wired port, the device will not be removed physically. So we can
> > power off it when device is suspended and remote wakeup is disabled without
> > concerning with hot-plug. This patch is dedicated to this siutation.
> > 
> > This patch is to provide usb acpi power control method and call them in the
> > usb_port_suspend() and usb_port_resume() when port can be power off. When the
> > usb port is in the power off state, usb core doesn't remove device which is
> > attached to the port. The device is still on the system and user can access
> > the device.
> 
> Can you provide any examples where this would be useful?  It won't end 
> up saving very much power (although on a laptop even a little bit might 
> help).

Every little bit of power savings helps for the particular system that
implements the USB port power off.  For this particular platform, Intel
is looking at the system as a whole and trying to eek out power savings
where ever they can.  A little power savings in one particular subsystem
may not seem like a big deal, but when you look at the overall picture,
the long tail adds up.  Just trust me, I'm excited about this system. :)

As for examples of where the port power off would be useful, think about
a laptop with several internal ports.  The customer can save some money
by choosing not to purchase an internal USB bluetooth device.  The OEM
may have just one motherboard for those two choices, so the port that
would have held the bluetooth device will be empty.  In that case, we'll
never see a USB device connect on that empty port, so we may as well
power it down.  If we can further power off the internal webcam port
when the device is suspended, we can save more power.

Another example is when a user walks away from their laptop with some
USB devices plugged in.  If userspace can somehow know when the system
is idle (e.g. the screen blanks, the bluetooth/NFC radio no longer
detects the person's phone, etc), we can power off unconnected and
suspended external ports.  The hypothesis is that some users may not
care about USB device connects and disconnects when their system is
idle.  They really will only care that the changes get noticed when
they start using their system.

This breaks down for some users, of course.  Arjan has several desktops
under his desk that are always on, and he starts interacting with them
by plugging in a USB mouse and keyboard.  So obviously the "port power
off on screen blank" policy might not work for him.  It also won't work
for servers, where no one connects a real monitor to the server for
months, but server folks probably won't care about this mechanism
because their power budget is so much larger.

However, someone on an airplane, trying to eek out every mW out of their
battery, would want to power off any external unconnected or suspended
USB ports.

The point is that whether a user wants to allow the ports to be powered
off should be a userspace policy.  That's why we export the sysfs file,
so that desktop software can implement whatever their customers want.
Personally, I would want a checkbox in the Gnome display settings that
says "Power off USB ports on screen blank".

> > introduce three port's states.
> > 
> > USB_PORT_POWER_STATE_ON
> > USB_PORT_WAITING_FOR_CONNECTION
> > USB_PORT_POWER_STATE_OFF
> > 
> > "on"
> > 	port power on
> > 
> > "waiting for connection"
> > 	port power on but hub port has not detected the device or detect event has
> > not been processed.
> 
> This state is not needed.
> 
> > "off"
> > 	port power off
> > 
> > At first, port's state is "on". When the device is suspended, power off the port and
> > set port's state to "off". After the port powering off, the usb hub will detect a
> > connection change event. Normally, the device will be removed with regarding as being
> > unplugged. But in the power off mechanism, the device is still on the port and user
> > can still access the device. So ignore the event.
> > 
> > When the device is resumed, turn on the power resource and set port's state to
> > "waiting for connection". After the port powering on, the usb hub will detect a
> > connection change event which originally means a device plugged in and previous
> > device will be removed. But in the power offmechanism, the device is not changed
> > and so ignore the event. When port's state is "waiting for connection", receive
> > an event "connection" and the port's connection state is on. This means the usb
> > the device is detected by usb hub again after powering on port. Set port's state
> > to "on".
> > 
> >  "on"
> >   |
> >  "off"
> >   |
> >  "waiting for connection"
> >   |
> >  "on"
> > 
> > "waiting for connection" state is to avoid device to being removed.
> 
> Why would the device be removed?

When we turn the power back on, we'll get a connect status change bit
set.  We don't want the USB core to misinterpret that bit and try to
logically disconnect the device attached to the port.

> > If set to "on" after powering on, the connection event may not be processed at that
> > time. When it is processed, the port's state has been "on" and the device will be
> > removed. So introduce "waiting for connection" state.
> 
> Instead you should simply delay setting the state back to "on" until
> the device has connected.

What if the external device was suspended, we power off the port, and
then the user yanks the device while it was suspended?  A device may
never connect in that case.

> > We also have a proposal to add sys file for each port to control port power off
> > under usb hub sys directory. If the port's power off is supported by platform,
> > create a sys file e.g "port1_power"  for port one. Echo "on" to "port1_power" is
> > to not allow port to be power off. Echo "auto" to "port1_power" is to power off
> > port if possible.
> > 
> > Different type ports have different default values.
> > (1) hard-wired port with device				"auto"
> > (2) hot-pluggable port without device		"on"
> > (3) hot-pluggable port with device			"on"
> > 
> > Add member port_power_control, can_power_off  to struct usb_hub_port. port_power_control
> > records user choice. Can_power_off means the platform and device support to power off.
> > When a device is attached, check whether port can be power off if yes set can_power_off
> > to true. When device driver is load, the driver also can set value to can_power_off. When
> 
> That's no good.  What happens if the driver sets the value to false and 
> then the driver is unloaded?

Or the probe fails...

> > try to power off port, can_power_off and port_power_control should be taken into account.
> > Only when these two members  are set to true, the port could be power off.
> > 
> > sys file operation
> > port with device
> >  port1_power "auto" => "on" or "on" => "auto" implement
> > 	pm_runtime_get_syn(udev)
> > 	port_power_control = "auto" or "on"
> > 	pm_runtime_put_syn(udev)
> > 
> > port without device
> > 	port can power on or power power off directly.
> 
> We have no code for powering off ports that don't have a device.  If 
> you did power off such a port, it would not be able to detect when a 
> device was plugged in.
> 
> > Suggestion and comments more welcome.
> 
> Your code style for continuation lines is wrong.  Continuation lines 
> should be indented more than one tab stop -- otherwise they look like 
> regular lines in an inner block.

Let me know if you have any more questions about the mechanisms or
policy we're trying to set up.  Tianyu has been creating these patches,
but I've tried to provide some advice to him along the way.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Thu, 10 May 2012, Sarah Sharp wrote:

> On Thu, May 10, 2012 at 11:54:04AM -0400, Alan Stern wrote:
> > On Thu, 10 May 2012, Lan Tianyu wrote:
> > 
> > > hi all:
> > > 	Currently, we are working on usb port power off mechanism. Our developing
> > > machine provides usb port power control (a vbus switch)via ACPI power resource.
> > > When the power resource turns off, usb port powers off and usb device loses
> > > power. From usb hub side, just like the device being unplugged.
> > > 
> > > 	Since usb port power off will affect hot-plug and devices remote wakeup
> > > function, it should be careful to do that.
> > > 	We conclude three different situations for power off mechanism.
> > > 	(1) hard-wired port with device
> > > 	(2) hot-pluggable port without device
> > > 	(3) hot-pluggable port with device
> > > 
> > > For hard-wired port, the device will not be removed physically. So we can
> > > power off it when device is suspended and remote wakeup is disabled without
> > > concerning with hot-plug. This patch is dedicated to this siutation.
> > > 
> > > This patch is to provide usb acpi power control method and call them in the
> > > usb_port_suspend() and usb_port_resume() when port can be power off. When the
> > > usb port is in the power off state, usb core doesn't remove device which is
> > > attached to the port. The device is still on the system and user can access
> > > the device.
> > 
> > Can you provide any examples where this would be useful?  It won't end 
> > up saving very much power (although on a laptop even a little bit might 
> > help).
> 
> Every little bit of power savings helps for the particular system that
> implements the USB port power off.  For this particular platform, Intel
> is looking at the system as a whole and trying to eek out power savings
> where ever they can.  A little power savings in one particular subsystem
> may not seem like a big deal, but when you look at the overall picture,
> the long tail adds up.  Just trust me, I'm excited about this system. :)

I'll take your word for it.  :-)

> As for examples of where the port power off would be useful, think about
> a laptop with several internal ports.  The customer can save some money
> by choosing not to purchase an internal USB bluetooth device.  The OEM
> may have just one motherboard for those two choices, so the port that
> would have held the bluetooth device will be empty.  In that case, we'll
> never see a USB device connect on that empty port, so we may as well
> power it down.  If we can further power off the internal webcam port
> when the device is suspended, we can save more power.

The patch did not address the case of powering down ports that have no
devices attached.  That might be a better place to start, because it's
simpler, even though it might not yield as much power savings.

There's one more thing to consider, which was missing from the patch.  
When you power the port back up and resume the device, it will 
necessarily be a reset-resume.  You won't want to do this if any of the 
drivers attached to the device's interfaces doesn't have reset-resume 
support.

> Another example is when a user walks away from their laptop with some
> USB devices plugged in.  If userspace can somehow know when the system
> is idle (e.g. the screen blanks, the bluetooth/NFC radio no longer
> detects the person's phone, etc), we can power off unconnected and
> suspended external ports.  The hypothesis is that some users may not
> care about USB device connects and disconnects when their system is
> idle.  They really will only care that the changes get noticed when
> they start using their system.
> 
> This breaks down for some users, of course.  Arjan has several desktops
> under his desk that are always on, and he starts interacting with them
> by plugging in a USB mouse and keyboard.  So obviously the "port power
> off on screen blank" policy might not work for him.  It also won't work
> for servers, where no one connects a real monitor to the server for
> months, but server folks probably won't care about this mechanism
> because their power budget is so much larger.
> 
> However, someone on an airplane, trying to eek out every mW out of their
> battery, would want to power off any external unconnected or suspended
> USB ports.
> 
> The point is that whether a user wants to allow the ports to be powered
> off should be a userspace policy.  That's why we export the sysfs file,
> so that desktop software can implement whatever their customers want.
> Personally, I would want a checkbox in the Gnome display settings that
> says "Power off USB ports on screen blank".

Makes sense.  (But I foresee a lot of confusion among users when this
box is checked and the ports don't get powered down -- many of
today's laptops are incapable of powering down their USB ports.)

> > > "waiting for connection" state is to avoid device to being removed.
> > 
> > Why would the device be removed?
> 
> When we turn the power back on, we'll get a connect status change bit
> set.  We don't want the USB core to misinterpret that bit and try to
> logically disconnect the device attached to the port.

All you have to do is turn off the connect-change status bit.  In fact,
the debounce routine does this already.  Just leave the port state set
to "off" until the connection is debounced.

You may also have to turn off the enable-change status bit.

> What if the external device was suspended, we power off the port, and
> then the user yanks the device while it was suspended?  A device may
> never connect in that case.

Then the debounce loop in usb_port_wait_for_connected() will time out.  
At that point you'll know the device has been disconnected.

> Let me know if you have any more questions about the mechanisms or
> policy we're trying to set up.  Tianyu has been creating these patches,
> but I've tried to provide some advice to him along the way.

I think I get the picture.  It's a tricky job, because you can easily 
go too far and turn off something that needs to remain on.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Dan Williams]

On Thu, 2012-05-10 at 11:54 -0400, Alan Stern wrote:
> On Thu, 10 May 2012, Lan Tianyu wrote:
> 
> > hi all:
> > 	Currently, we are working on usb port power off mechanism. Our developing
> > machine provides usb port power control (a vbus switch)via ACPI power resource.
> > When the power resource turns off, usb port powers off and usb device loses
> > power. From usb hub side, just like the device being unplugged.
> > 
> > 	Since usb port power off will affect hot-plug and devices remote wakeup
> > function, it should be careful to do that.
> > 	We conclude three different situations for power off mechanism.
> > 	(1) hard-wired port with device
> > 	(2) hot-pluggable port without device
> > 	(3) hot-pluggable port with device
> > 
> > For hard-wired port, the device will not be removed physically. So we can
> > power off it when device is suspended and remote wakeup is disabled without
> > concerning with hot-plug. This patch is dedicated to this siutation.
> > 
> > This patch is to provide usb acpi power control method and call them in the
> > usb_port_suspend() and usb_port_resume() when port can be power off. When the
> > usb port is in the power off state, usb core doesn't remove device which is
> > attached to the port. The device is still on the system and user can access
> > the device.
> 
> Can you provide any examples where this would be useful?  It won't end 
> up saving very much power (although on a laptop even a little bit might 
> help).

I'd love to do this with ModemManager to shoot a modem in the head if
it's crashed.  Right now there's simply no way to reset a device that's
misbehaving or crashed.  Powering off the port would be a last resort,
but not all devices actually disconnect from the bus when they crash and
reconnect after the firmware reboots.

Dan

> > introduce three port's states.
> > 
> > USB_PORT_POWER_STATE_ON
> > USB_PORT_WAITING_FOR_CONNECTION
> > USB_PORT_POWER_STATE_OFF
> > 
> > "on"
> > 	port power on
> > 
> > "waiting for connection"
> > 	port power on but hub port has not detected the device or detect event has
> > not been processed.
> 
> This state is not needed.
> 
> > "off"
> > 	port power off
> > 
> > At first, port's state is "on". When the device is suspended, power off the port and
> > set port's state to "off". After the port powering off, the usb hub will detect a
> > connection change event. Normally, the device will be removed with regarding as being
> > unplugged. But in the power off mechanism, the device is still on the port and user
> > can still access the device. So ignore the event.
> > 
> > When the device is resumed, turn on the power resource and set port's state to
> > "waiting for connection". After the port powering on, the usb hub will detect a
> > connection change event which originally means a device plugged in and previous
> > device will be removed. But in the power offmechanism, the device is not changed
> > and so ignore the event. When port's state is "waiting for connection", receive
> > an event "connection" and the port's connection state is on. This means the usb
> > the device is detected by usb hub again after powering on port. Set port's state
> > to "on".
> > 
> >  "on"
> >   |
> >  "off"
> >   |
> >  "waiting for connection"
> >   |
> >  "on"
> > 
> > "waiting for connection" state is to avoid device to being removed.
> 
> Why would the device be removed?
> 
> > If set to "on" after powering on, the connection event may not be processed at that
> > time. When it is processed, the port's state has been "on" and the device will be
> > removed. So introduce "waiting for connection" state.
> 
> Instead you should simply delay setting the state back to "on" until
> the device has connected.
> 
> > We also have a proposal to add sys file for each port to control port power off
> > under usb hub sys directory. If the port's power off is supported by platform,
> > create a sys file e.g "port1_power"  for port one. Echo "on" to "port1_power" is
> > to not allow port to be power off. Echo "auto" to "port1_power" is to power off
> > port if possible.
> > 
> > Different type ports have different default values.
> > (1) hard-wired port with device				"auto"
> > (2) hot-pluggable port without device		"on"
> > (3) hot-pluggable port with device			"on"
> > 
> > Add member port_power_control, can_power_off  to struct usb_hub_port. port_power_control
> > records user choice. Can_power_off means the platform and device support to power off.
> > When a device is attached, check whether port can be power off if yes set can_power_off
> > to true. When device driver is load, the driver also can set value to can_power_off. When
> 
> That's no good.  What happens if the driver sets the value to false and 
> then the driver is unloaded?
> 
> > try to power off port, can_power_off and port_power_control should be taken into account.
> > Only when these two members  are set to true, the port could be power off.
> > 
> > sys file operation
> > port with device
> >  port1_power "auto" => "on" or "on" => "auto" implement
> > 	pm_runtime_get_syn(udev)
> > 	port_power_control = "auto" or "on"
> > 	pm_runtime_put_syn(udev)
> > 
> > port without device
> > 	port can power on or power power off directly.
> 
> We have no code for powering off ports that don't have a device.  If 
> you did power off such a port, it would not be able to detect when a 
> device was plugged in.
> 
> > Suggestion and comments more welcome.
> 
> Your code style for continuation lines is wrong.  Continuation lines 
> should be indented more than one tab stop -- otherwise they look like 
> regular lines in an inner block.
> 
> Alan Stern
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Thu, May 10, 2012 at 02:19:38PM -0500, Dan Williams wrote:
> On Thu, 2012-05-10 at 11:54 -0400, Alan Stern wrote:
> > On Thu, 10 May 2012, Lan Tianyu wrote:
> > 
> > > hi all:
> > > 	Currently, we are working on usb port power off mechanism. Our developing
> > > machine provides usb port power control (a vbus switch)via ACPI power resource.
> > > When the power resource turns off, usb port powers off and usb device loses
> > > power. From usb hub side, just like the device being unplugged.
> > > 
> > > 	Since usb port power off will affect hot-plug and devices remote wakeup
> > > function, it should be careful to do that.
> > > 	We conclude three different situations for power off mechanism.
> > > 	(1) hard-wired port with device
> > > 	(2) hot-pluggable port without device
> > > 	(3) hot-pluggable port with device
> > > 
> > > For hard-wired port, the device will not be removed physically. So we can
> > > power off it when device is suspended and remote wakeup is disabled without
> > > concerning with hot-plug. This patch is dedicated to this siutation.
> > > 
> > > This patch is to provide usb acpi power control method and call them in the
> > > usb_port_suspend() and usb_port_resume() when port can be power off. When the
> > > usb port is in the power off state, usb core doesn't remove device which is
> > > attached to the port. The device is still on the system and user can access
> > > the device.
> > 
> > Can you provide any examples where this would be useful?  It won't end 
> > up saving very much power (although on a laptop even a little bit might 
> > help).
> 
> I'd love to do this with ModemManager to shoot a modem in the head if
> it's crashed.  Right now there's simply no way to reset a device that's
> misbehaving or crashed.  Powering off the port would be a last resort,
> but not all devices actually disconnect from the bus when they crash and
> reconnect after the firmware reboots.

I'm not sure if the policy we set up would allow that, if the device was
still connected and not suspended.  Maybe we need an "off" setting that
forces the port off?

Also, setting the port power off via ACPI may not actually cut power,
because port power might be ganged.  Once we signal via ACPI that all
the ports that are ganged together can be powered off, they will.  So
there's no guarantees that you can power off the buggy modem unless you
power off the other ganged ports as well.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Peter Stuge]

Sarah Sharp wrote:
> Maybe we need an "off" setting that forces the port off?

FWIW this would be very welcome in libusb, to do a hard reset of
bus-powered devices.

It can be attempted via SET_FEATURE(PORT_POWER) to the hub, but a
"real" API would be nice.


//Peter

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Thu, 10 May 2012, Sarah Sharp wrote:

> Also, setting the port power off via ACPI may not actually cut power,
> because port power might be ganged.  Once we signal via ACPI that all
> the ports that are ganged together can be powered off, they will.  So
> there's no guarantees that you can power off the buggy modem unless you
> power off the other ganged ports as well.

That reminds me...  I think this should not be so closely linked with
ACPI.  There's a perfectly good USB Clear-Feature request for turning
off port power; that's what we should use.  If hooks are required for
interfacing with platform-specific code (such as ACPI), they can be
added at the appropriate places.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Fri, 11 May 2012, Peter Stuge wrote:

> Sarah Sharp wrote:
> > Maybe we need an "off" setting that forces the port off?
> 
> FWIW this would be very welcome in libusb, to do a hard reset of
> bus-powered devices.
> 
> It can be attempted via SET_FEATURE(PORT_POWER) to the hub, but a
> "real" API would be nice.

But remember that such an API would be very unreliable.  Many hubs and
root hubs cannot shut off port power at all, and some of the remaining
ones use ganged power switches (you have to turn off all the ports in a
gang before power is removed from any of them).

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Peter Stuge]

Alan Stern wrote:
> > > Maybe we need an "off" setting that forces the port off?
> > 
> > FWIW this would be very welcome in libusb, to do a hard reset of
> > bus-powered devices.
> > 
> > It can be attempted via SET_FEATURE(PORT_POWER) to the hub, but a
> > "real" API would be nice.
> 
> But remember that such an API would be very unreliable.  Many hubs and
> root hubs cannot shut off port power at all, and some of the remaining
> ones use ganged power switches (you have to turn off all the ports in a
> gang before power is removed from any of them).

Yes, I know. It would be extra special nice if the kernel exposes
these various capabilities, in case they are already known.


//Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

hi all:
	Great thanks for review.
On 2012年05月11日 01:44, Alan Stern wrote:
> On Thu, 10 May 2012, Sarah Sharp wrote:
>
>> On Thu, May 10, 2012 at 11:54:04AM -0400, Alan Stern wrote:
>>> On Thu, 10 May 2012, Lan Tianyu wrote:
>>>
>>>> hi all:
>>>> 	Currently, we are working on usb port power off mechanism. Our developing
>>>> machine provides usb port power control (a vbus switch)via ACPI power resource.
>>>> When the power resource turns off, usb port powers off and usb device loses
>>>> power. From usb hub side, just like the device being unplugged.
>>>>
>>>> 	Since usb port power off will affect hot-plug and devices remote wakeup
>>>> function, it should be careful to do that.
>>>> 	We conclude three different situations for power off mechanism.
>>>> 	(1) hard-wired port with device
>>>> 	(2) hot-pluggable port without device
>>>> 	(3) hot-pluggable port with device
>>>>
>>>> For hard-wired port, the device will not be removed physically. So we can
>>>> power off it when device is suspended and remote wakeup is disabled without
>>>> concerning with hot-plug. This patch is dedicated to this siutation.
>>>>
>>>> This patch is to provide usb acpi power control method and call them in the
>>>> usb_port_suspend() and usb_port_resume() when port can be power off. When the
>>>> usb port is in the power off state, usb core doesn't remove device which is
>>>> attached to the port. The device is still on the system and user can access
>>>> the device.
>>>
>>> Can you provide any examples where this would be useful?  It won't end
>>> up saving very much power (although on a laptop even a little bit might
>>> help).
>>
>> Every little bit of power savings helps for the particular system that
>> implements the USB port power off.  For this particular platform, Intel
>> is looking at the system as a whole and trying to eek out power savings
>> where ever they can.  A little power savings in one particular subsystem
>> may not seem like a big deal, but when you look at the overall picture,
>> the long tail adds up.  Just trust me, I'm excited about this system. :)
>
> I'll take your word for it.  :-)
>
    The power saving depends on devices. I test a usb3.0 ssd. The power saving of
power off is about 2.2w more than just selective suspend. In theory, power
off can help to save remaining power after selective suspend.

>> As for examples of where the port power off would be useful, think about
>> a laptop with several internal ports.  The customer can save some money
>> by choosing not to purchase an internal USB bluetooth device.  The OEM
>> may have just one motherboard for those two choices, so the port that
>> would have held the bluetooth device will be empty.  In that case, we'll
>> never see a USB device connect on that empty port, so we may as well
>> power it down.  If we can further power off the internal webcam port
>> when the device is suspended, we can save more power.
>
> The patch did not address the case of powering down ports that have no
> devices attached.  That might be a better place to start, because it's
> simpler, even though it might not yield as much power savings.

Do you mean internal ports?
 From my opinion, ACPI will tell us whether the port is connectable or not.
When the internal port is not connectable, this means the port is not used
and this patch will power down the port.

@@ -55,9 +81,14 @@ static int usb_acpi_check_port_connect_type(struct usb_device 
*hdev,
                 pld.user_visible ?
                     USB_PORT_CONNECT_TYPE_HOT_PLUG :
                     USB_PORT_CONNECT_TYPE_HARD_WIRED);
-   else if (!pld.user_visible)
+   else if (!pld.user_visible) {
         usb_set_hub_port_connect_type(hdev, port1, USB_PORT_NOT_USED);

+       /* Power off the usb port which may not be used.*/
+       if (usb_acpi_power_manageable(hdev, port1))
+           usb_acpi_set_power_state(hdev, port1, false);
+   }
+

For external ports, this should be associated with sys file control. The users
need to determine when they should be power off.

So I should work on the external ports without devices firstly and
add the sys file for user to control?

>
> There's one more thing to consider, which was missing from the patch.
> When you power the port back up and resume the device, it will
> necessarily be a reset-resume.  You won't want to do this if any of the
> drivers attached to the device's interfaces doesn't have reset-resume
> support.
>
>> Another example is when a user walks away from their laptop with some
>> USB devices plugged in.  If userspace can somehow know when the system
>> is idle (e.g. the screen blanks, the bluetooth/NFC radio no longer
>> detects the person's phone, etc), we can power off unconnected and
>> suspended external ports.  The hypothesis is that some users may not
>> care about USB device connects and disconnects when their system is
>> idle.  They really will only care that the changes get noticed when
>> they start using their system.
>>
>> This breaks down for some users, of course.  Arjan has several desktops
>> under his desk that are always on, and he starts interacting with them
>> by plugging in a USB mouse and keyboard.  So obviously the "port power
>> off on screen blank" policy might not work for him.  It also won't work
>> for servers, where no one connects a real monitor to the server for
>> months, but server folks probably won't care about this mechanism
>> because their power budget is so much larger.
>>
>> However, someone on an airplane, trying to eek out every mW out of their
>> battery, would want to power off any external unconnected or suspended
>> USB ports.
>>
>> The point is that whether a user wants to allow the ports to be powered
>> off should be a userspace policy.  That's why we export the sysfs file,
>> so that desktop software can implement whatever their customers want.
>> Personally, I would want a checkbox in the Gnome display settings that
>> says "Power off USB ports on screen blank".
>
> Makes sense.  (But I foresee a lot of confusion among users when this
> box is checked and the ports don't get powered down -- many of
> today's laptops are incapable of powering down their USB ports.)
>
How about to just provide sys files if the usb port can be power off
and the checkbox only is selectable when sys files exit.
>>>> "waiting for connection" state is to avoid device to being removed.
>>>
>>> Why would the device be removed?
>>
>> When we turn the power back on, we'll get a connect status change bit
>> set.  We don't want the USB core to misinterpret that bit and try to
>> logically disconnect the device attached to the port.
>
> All you have to do is turn off the connect-change status bit.  In fact,
> the debounce routine does this already.  Just leave the port state set
> to "off" until the connection is debounced.
This is my original version. :)
But after testing, I found that after the connection is debounced,
hub_port_connect_change() may not to be invoked or not reach place
"disconnect devices".

hub_port_connect_change() {
...
/* debounce may be completed before here and port's state has becomed
  * "on". The device also will be removed.
  */	
if (hub->port_data[port1 - 1].power_state == USB_PORT_POWER_STATE_OFF) {
	clear_bit(port1, hub->change_bits);
	return;
}
	
/* Disconnect any existing devices under this port */
if (udev)
	usb_disconnect(&hub->port_data[port1-1].child);
clear_bit(port1, hub->change_bits);

...
}

I spent a lot of time resolving the problem. At last, I found to add
"waiting for connection" state to resolve the problem.

After power on the port, hub_irq() invokes kick_khubd() to deal with
connect or enable change event. Connect debouncing does in the
usb_port_resume() and  connect-change event is processed in hub_thread().
It is hard to synchronize them and the port's state should be set to
"on" after the connect-change event finishing.
>
> You may also have to turn off the enable-change status bit.
>
You mean clear_port_feature(connect or enable change)?
 From my opinion, this will be done in the hub_event() since
hub is active and hub->quiescing should be 0 when the usb_port_resume()
is invoking. Every portIs that right?

>> What if the external device was suspended, we power off the port, and
>> then the user yanks the device while it was suspended?  A device may
>> never connect in that case.
>
> Then the debounce loop in usb_port_wait_for_connected() will time out.
> At that point you'll know the device has been disconnected.
>
>> Let me know if you have any more questions about the mechanisms or
>> policy we're trying to set up.  Tianyu has been creating these patches,
>> but I've tried to provide some advice to him along the way.
>
> I think I get the picture.  It's a tricky job, because you can easily
> go too far and turn off something that needs to remain on.
This is reason why
>
> Alan Stern
>

-- 
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012年05月12日 00:12, Lan Tianyu wrote:
> hi all:
> 	Great thanks for review.
> On 2012年05月11日 01:44, Alan Stern wrote:
>> On Thu, 10 May 2012, Sarah Sharp wrote:
>>
>>> On Thu, May 10, 2012 at 11:54:04AM -0400, Alan Stern wrote:
>>>> On Thu, 10 May 2012, Lan Tianyu wrote:
>>>>
>>>>> hi all:
>>>>> 	Currently, we are working on usb port power off mechanism. Our developing
>>>>> machine provides usb port power control (a vbus switch)via ACPI power resource.
>>>>> When the power resource turns off, usb port powers off and usb device loses
>>>>> power. From usb hub side, just like the device being unplugged.
>>>>>
>>>>> 	Since usb port power off will affect hot-plug and devices remote wakeup
>>>>> function, it should be careful to do that.
>>>>> 	We conclude three different situations for power off mechanism.
>>>>> 	(1) hard-wired port with device
>>>>> 	(2) hot-pluggable port without device
>>>>> 	(3) hot-pluggable port with device
>>>>>
>>>>> For hard-wired port, the device will not be removed physically. So we can
>>>>> power off it when device is suspended and remote wakeup is disabled without
>>>>> concerning with hot-plug. This patch is dedicated to this siutation.
>>>>>
>>>>> This patch is to provide usb acpi power control method and call them in the
>>>>> usb_port_suspend() and usb_port_resume() when port can be power off. When the
>>>>> usb port is in the power off state, usb core doesn't remove device which is
>>>>> attached to the port. The device is still on the system and user can access
>>>>> the device.
>>>>
>>>> Can you provide any examples where this would be useful?  It won't end
>>>> up saving very much power (although on a laptop even a little bit might
>>>> help).
>>>
>>> Every little bit of power savings helps for the particular system that
>>> implements the USB port power off.  For this particular platform, Intel
>>> is looking at the system as a whole and trying to eek out power savings
>>> where ever they can.  A little power savings in one particular subsystem
>>> may not seem like a big deal, but when you look at the overall picture,
>>> the long tail adds up.  Just trust me, I'm excited about this system. :)
>>
>> I'll take your word for it.  :-)
>>
>      The power saving depends on devices. I test a usb3.0 ssd. The power saving of
> power off is about 2.2w more than just selective suspend. In theory, power
> off can help to save remaining power after selective suspend.
>
>>> As for examples of where the port power off would be useful, think about
>>> a laptop with several internal ports.  The customer can save some money
>>> by choosing not to purchase an internal USB bluetooth device.  The OEM
>>> may have just one motherboard for those two choices, so the port that
>>> would have held the bluetooth device will be empty.  In that case, we'll
>>> never see a USB device connect on that empty port, so we may as well
>>> power it down.  If we can further power off the internal webcam port
>>> when the device is suspended, we can save more power.
>>
>> The patch did not address the case of powering down ports that have no
>> devices attached.  That might be a better place to start, because it's
>> simpler, even though it might not yield as much power savings.
>
> Do you mean internal ports?
>   From my opinion, ACPI will tell us whether the port is connectable or not.
> When the internal port is not connectable, this means the port is not used
> and this patch will power down the port.
>
> @@ -55,9 +81,14 @@ static int usb_acpi_check_port_connect_type(struct usb_device
> *hdev,
>                   pld.user_visible ?
>                       USB_PORT_CONNECT_TYPE_HOT_PLUG :
>                       USB_PORT_CONNECT_TYPE_HARD_WIRED);
> -   else if (!pld.user_visible)
> +   else if (!pld.user_visible) {
>           usb_set_hub_port_connect_type(hdev, port1, USB_PORT_NOT_USED);
>
> +       /* Power off the usb port which may not be used.*/
> +       if (usb_acpi_power_manageable(hdev, port1))
> +           usb_acpi_set_power_state(hdev, port1, false);
> +   }
> +
>
> For external ports, this should be associated with sys file control. The users
> need to determine when they should be power off.
>
> So I should work on the external ports without devices firstly and
> add the sys file for user to control?
>
>>
>> There's one more thing to consider, which was missing from the patch.
>> When you power the port back up and resume the device, it will
>> necessarily be a reset-resume.  You won't want to do this if any of the
>> drivers attached to the device's interfaces doesn't have reset-resume
>> support.
>>
>>> Another example is when a user walks away from their laptop with some
>>> USB devices plugged in.  If userspace can somehow know when the system
>>> is idle (e.g. the screen blanks, the bluetooth/NFC radio no longer
>>> detects the person's phone, etc), we can power off unconnected and
>>> suspended external ports.  The hypothesis is that some users may not
>>> care about USB device connects and disconnects when their system is
>>> idle.  They really will only care that the changes get noticed when
>>> they start using their system.
>>>
>>> This breaks down for some users, of course.  Arjan has several desktops
>>> under his desk that are always on, and he starts interacting with them
>>> by plugging in a USB mouse and keyboard.  So obviously the "port power
>>> off on screen blank" policy might not work for him.  It also won't work
>>> for servers, where no one connects a real monitor to the server for
>>> months, but server folks probably won't care about this mechanism
>>> because their power budget is so much larger.
>>>
>>> However, someone on an airplane, trying to eek out every mW out of their
>>> battery, would want to power off any external unconnected or suspended
>>> USB ports.
>>>
>>> The point is that whether a user wants to allow the ports to be powered
>>> off should be a userspace policy.  That's why we export the sysfs file,
>>> so that desktop software can implement whatever their customers want.
>>> Personally, I would want a checkbox in the Gnome display settings that
>>> says "Power off USB ports on screen blank".
>>
>> Makes sense.  (But I foresee a lot of confusion among users when this
>> box is checked and the ports don't get powered down -- many of
>> today's laptops are incapable of powering down their USB ports.)
>>
> How about to just provide sys files if the usb port can be power off
> and the checkbox only is selectable when sys files exit.
>>>>> "waiting for connection" state is to avoid device to being removed.
>>>>
>>>> Why would the device be removed?
>>>
>>> When we turn the power back on, we'll get a connect status change bit
>>> set.  We don't want the USB core to misinterpret that bit and try to
>>> logically disconnect the device attached to the port.
>>
>> All you have to do is turn off the connect-change status bit.  In fact,
>> the debounce routine does this already.  Just leave the port state set
>> to "off" until the connection is debounced.
> This is my original version. :)
> But after testing, I found that after the connection is debounced,
> hub_port_connect_change() may not to be invoked or not reach place
> "disconnect devices".
>
> hub_port_connect_change() {
> ...
> /* debounce may be completed before here and port's state has becomed
>    * "on". The device also will be removed.
>    */	
> if (hub->port_data[port1 - 1].power_state == USB_PORT_POWER_STATE_OFF) {
> 	clear_bit(port1, hub->change_bits);
> 	return;
> }
> 	
> /* Disconnect any existing devices under this port */
> if (udev)
> 	usb_disconnect(&hub->port_data[port1-1].child);
> clear_bit(port1, hub->change_bits);
>
> ...
> }
>
> I spent a lot of time resolving the problem. At last, I found to add
> "waiting for connection" state to resolve the problem.
>
> After power on the port, hub_irq() invokes kick_khubd() to deal with
> connect or enable change event. Connect debouncing does in the
> usb_port_resume() and  connect-change event is processed in hub_thread().
> It is hard to synchronize them and the port's state should be set to
> "on" after the connect-change event finishing.
>>
>> You may also have to turn off the enable-change status bit.
>>
> You mean clear_port_feature(connect or enable change)?
>   From my opinion, this will be done in the hub_event() since
> hub is active and hub->quiescing should be 0 when the usb_port_resume()
> is invoking. Every portIs that right?
>
>>> What if the external device was suspended, we power off the port, and
>>> then the user yanks the device while it was suspended?  A device may
>>> never connect in that case.
>>
>> Then the debounce loop in usb_port_wait_for_connected() will time out.
>> At that point you'll know the device has been disconnected.
>>
>>> Let me know if you have any more questions about the mechanisms or
>>> policy we're trying to set up.  Tianyu has been creating these patches,
>>> but I've tried to provide some advice to him along the way.
>>
>> I think I get the picture.  It's a tricky job, because you can easily
>> go too far and turn off something that needs to remain on.
> This is reason why
This is why I want to device driver to set can_power_off, driver may know
clear when the device can be power off.
>>
>> Alan Stern
>>
>

-- 
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Sat, 12 May 2012, Lan Tianyu wrote:

>     The power saving depends on devices. I test a usb3.0 ssd. The power saving of
> power off is about 2.2w more than just selective suspend. In theory, power
> off can help to save remaining power after selective suspend.

That's a lot of power!  Suspended USB devices aren't supposed to
consume more than 2.5 mA of bus current, which at 5 V amounts to <=
0.0125 W.  Does the port really use that much?  Or does the SSD have a
separate power supply that it disables when port power is removed?

> > The patch did not address the case of powering down ports that have no
> > devices attached.  That might be a better place to start, because it's
> > simpler, even though it might not yield as much power savings.
> 
> Do you mean internal ports?

Internal or external.

>  From my opinion, ACPI will tell us whether the port is connectable or not.

ACPI will tell you about some ports, not others (for example, ACPI
knows nothing about the ports on hubs that the user plugs in).  On
other systems, a Device Tree database might provide the same
information.

> When the internal port is not connectable, this means the port is not used
> and this patch will power down the port.

...

> For external ports, this should be associated with sys file control. The users
> need to determine when they should be power off.

You don't mean "external", you mean "not described as unconnectable by 
ACPI".

> So I should work on the external ports without devices firstly and
> add the sys file for user to control?

Yes, I think so.  It will be less controversial and probably simpler.  
When that mechanism is ready, you should be able to use it
automatically for unconnectable ports.

One tricky thing: In theory, there should be a separate sysfs file for 
each port.  That seems like a lot of overhead though; is there any way 
to present the information in a single file that won't offend sysfs 
purists?

> > Makes sense.  (But I foresee a lot of confusion among users when this
> > box is checked and the ports don't get powered down -- many of
> > today's laptops are incapable of powering down their USB ports.)
> >
> How about to just provide sys files if the usb port can be power off
> and the checkbox only is selectable when sys files exit.

The problem is that the kernel doesn't know whether a port can be
powered off.  For some ports, you may be able to rely on platform
information like ACPI.  But for other ports, you simply can't tell.

> >> When we turn the power back on, we'll get a connect status change bit
> >> set.  We don't want the USB core to misinterpret that bit and try to
> >> logically disconnect the device attached to the port.
> >
> > All you have to do is turn off the connect-change status bit.  In fact,
> > the debounce routine does this already.  Just leave the port state set
> > to "off" until the connection is debounced.
> This is my original version. :)
> But after testing, I found that after the connection is debounced,
> hub_port_connect_change() may not to be invoked or not reach place
> "disconnect devices".
> 
> hub_port_connect_change() {
> ...
> /* debounce may be completed before here and port's state has becomed
>   * "on". The device also will be removed.
>   */	
> if (hub->port_data[port1 - 1].power_state == USB_PORT_POWER_STATE_OFF) {
> 	clear_bit(port1, hub->change_bits);
> 	return;
> }
> 	
> /* Disconnect any existing devices under this port */
> if (udev)
> 	usb_disconnect(&hub->port_data[port1-1].child);
> clear_bit(port1, hub->change_bits);
> 
> ...
> }

This doesn't matter.  Disconnections will be handled by 
usb_reset_and_verify_device(), when it is called by 
finish_port_resume().

> I spent a lot of time resolving the problem. At last, I found to add
> "waiting for connection" state to resolve the problem.
> 
> After power on the port, hub_irq() invokes kick_khubd() to deal with
> connect or enable change event. Connect debouncing does in the
> usb_port_resume() and  connect-change event is processed in hub_thread().
> It is hard to synchronize them and the port's state should be set to
> "on" after the connect-change event finishing.

I still don't see what the problem is.  They don't have to be
synchronized; all you need to do is make sure that the port's state
remains set to "off" until the debouncing is finished and you have
turned off the connect-change and enable-change features.

(In the future, synchronization may become more of a problem.  For
example, there could be trouble if hub_events() is already running when
a bit in hub->busy_bits gets turned on.  As far as I can tell, though, 
this doesn't affect you.)

> > You may also have to turn off the enable-change status bit.
> >
> You mean clear_port_feature(connect or enable change)?
>  From my opinion, this will be done in the hub_event() since
> hub is active and hub->quiescing should be 0 when the usb_port_resume()
> is invoking. Every portIs that right?

I don't know; it seems more likely that you'll have a problem when the 
power is turned off, not when it is turned on.  That's when the connect 
and enable status changes occur.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 10:08:33AM -0400, Alan Stern wrote:
> On Thu, 10 May 2012, Sarah Sharp wrote:
> 
> > Also, setting the port power off via ACPI may not actually cut power,
> > because port power might be ganged.  Once we signal via ACPI that all
> > the ports that are ganged together can be powered off, they will.  So
> > there's no guarantees that you can power off the buggy modem unless you
> > power off the other ganged ports as well.
> 
> That reminds me...  I think this should not be so closely linked with
> ACPI.  There's a perfectly good USB Clear-Feature request for turning
> off port power; that's what we should use.  If hooks are required for
> interfacing with platform-specific code (such as ACPI), they can be
> added at the appropriate places.

So would you rather userspace issue a clear port power feature request
to the roothub through libusb than have a sysfs file per port in
/sys/bus/usb/devices/../power/ ?  Or are you just saying that the sysfs
interface should issue the request to the hub (which may be the
roothub), and the xHCI driver can just implement the ACPI calls in its
roothub control method?

Sarah Sharp

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 01:44:26PM -0400, Alan Stern wrote:
> On Sat, 12 May 2012, Lan Tianyu wrote:
> 
> >     The power saving depends on devices. I test a usb3.0 ssd. The power saving of
> > power off is about 2.2w more than just selective suspend. In theory, power
> > off can help to save remaining power after selective suspend.
> 
> That's a lot of power!  Suspended USB devices aren't supposed to
> consume more than 2.5 mA of bus current, which at 5 V amounts to <=
> 0.0125 W.  Does the port really use that much?  Or does the SSD have a
> separate power supply that it disables when port power is removed?

I actually suspect that the host controller is powering off several PLLs
if all the ports are powered off.  After all, if it doesn't have to pay
attention to connects, disconnects, or remote wakeup, it can power off a
lot more circuitry, and possibly even enter D3cold instead of D3hot when
the PCI device is suspended (depending on what board Tianyu is testing
on).  But I agree that it would be interesting to see if the SSD has a
separate power supply that it's turning off.

> > > The patch did not address the case of powering down ports that have no
> > > devices attached.  That might be a better place to start, because it's
> > > simpler, even though it might not yield as much power savings.
> > 
> > Do you mean internal ports?
> 
> Internal or external.
> 
> >  From my opinion, ACPI will tell us whether the port is connectable or not.
> 
> ACPI will tell you about some ports, not others (for example, ACPI
> knows nothing about the ports on hubs that the user plugs in).  On
> other systems, a Device Tree database might provide the same
> information.
> 
> > When the internal port is not connectable, this means the port is not used
> > and this patch will power down the port.
> 
> ...
> 
> > For external ports, this should be associated with sys file control. The users
> > need to determine when they should be power off.
> 
> You don't mean "external", you mean "not described as unconnectable by 
> ACPI".
> 
> > So I should work on the external ports without devices firstly and
> > add the sys file for user to control?
> 
> Yes, I think so.  It will be less controversial and probably simpler.  
> When that mechanism is ready, you should be able to use it
> automatically for unconnectable ports.
> 
> One tricky thing: In theory, there should be a separate sysfs file for 
> each port.  That seems like a lot of overhead though; is there any way 
> to present the information in a single file that won't offend sysfs 
> purists?

Tianyu proposed having one file per hub, with a bit field that
controlled each port power.  However, I was concerned about different
userspace applications racing with each other to turn or off ports.  For
example, one app could read the bit field, attempt to power off just
port 1, but before it can write to the sysfs file, a second app powers
on port2, and the first app then writes to the sysfs file, leaving port
1 powered off, and port 2 powered off, which is not what the second app
wanted.

But if you can think of a better way to coalesce the port power off
mechanisms into one file, we're all ears. :)

Sarah Sharp

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012年05月12日 01:44, Alan Stern wrote:
> On Sat, 12 May 2012, Lan Tianyu wrote:
>
>>      The power saving depends on devices. I test a usb3.0 ssd. The power saving of
>> power off is about 2.2w more than just selective suspend. In theory, power
>> off can help to save remaining power after selective suspend.
>
> That's a lot of power!  Suspended USB devices aren't supposed to
> consume more than 2.5 mA of bus current, which at 5 V amounts to<=
> 0.0125 W.  Does the port really use that much?  Or does the SSD have a
> separate power supply that it disables when port power is removed?

The device consumes about 3.1w. The selective suspend can cut down 0.7-0.9w
After power off port, all power will removed.
I test all the motherboard power consumption and get the result.
 From lsusb, it's self-powered.

lsusb
Bus 003 Device 006: ID 174c:55aa ASMedia Technology Inc.
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.10
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x174c ASMedia Technology Inc.
   idProduct          0x55aa
   bcdDevice            1.00
   iManufacturer           2 01234567890123456789012345678901234567890123
   iProduct                3 012345678901234567890123456789012345678901234567
   iSerial                 1 0123456789ABCDEF019F
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           32
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xc0
       Self Powered
     MaxPower                0mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           2
       bInterfaceClass         8 Mass Storage
       bInterfaceSubClass      6 SCSI
       bInterfaceProtocol     80 Bulk (Zip)
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x02  EP 2 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
Device Status:     0x0001
   Self Powered



-- 
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Greg KH]

On Fri, May 11, 2012 at 01:44:26PM -0400, Alan Stern wrote:
> On Sat, 12 May 2012, Lan Tianyu wrote:
> 
> >     The power saving depends on devices. I test a usb3.0 ssd. The power saving of
> > power off is about 2.2w more than just selective suspend. In theory, power
> > off can help to save remaining power after selective suspend.
> 
> That's a lot of power!  Suspended USB devices aren't supposed to
> consume more than 2.5 mA of bus current, which at 5 V amounts to <=
> 0.0125 W.  Does the port really use that much?  Or does the SSD have a
> separate power supply that it disables when port power is removed?
> 
> > > The patch did not address the case of powering down ports that have no
> > > devices attached.  That might be a better place to start, because it's
> > > simpler, even though it might not yield as much power savings.
> > 
> > Do you mean internal ports?
> 
> Internal or external.
> 
> >  From my opinion, ACPI will tell us whether the port is connectable or not.
> 
> ACPI will tell you about some ports, not others (for example, ACPI
> knows nothing about the ports on hubs that the user plugs in).  On
> other systems, a Device Tree database might provide the same
> information.
> 
> > When the internal port is not connectable, this means the port is not used
> > and this patch will power down the port.
> 
> ...
> 
> > For external ports, this should be associated with sys file control. The users
> > need to determine when they should be power off.
> 
> You don't mean "external", you mean "not described as unconnectable by 
> ACPI".
> 
> > So I should work on the external ports without devices firstly and
> > add the sys file for user to control?
> 
> Yes, I think so.  It will be less controversial and probably simpler.  
> When that mechanism is ready, you should be able to use it
> automatically for unconnectable ports.
> 
> One tricky thing: In theory, there should be a separate sysfs file for 
> each port.  That seems like a lot of overhead though; is there any way 
> to present the information in a single file that won't offend sysfs 
> purists?

Why is that a lot of "overhead"?  It's what, 7-9 files max?  As Sarah
points out, one file for all ports is racy and can get to be a mess.

But then again, I'm a "sysfs purist" :)

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Fri, 11 May 2012, Sarah Sharp wrote:

> > That reminds me...  I think this should not be so closely linked with
> > ACPI.  There's a perfectly good USB Clear-Feature request for turning
> > off port power; that's what we should use.  If hooks are required for
> > interfacing with platform-specific code (such as ACPI), they can be
> > added at the appropriate places.
> 
> So would you rather userspace issue a clear port power feature request
> to the roothub through libusb than have a sysfs file per port in
> /sys/bus/usb/devices/../power/ ?  Or are you just saying that the sysfs
> interface should issue the request to the hub (which may be the
> roothub), and the xHCI driver can just implement the ACPI calls in its
> roothub control method?

The latter.  Except that the ACPI calls may need to occur in more
places than just xhci-hcd (ehci-hcd, for example).  And what about
ports on the USB-2 "rate-matching" hubs that Intel now builds into its
chipsets?

For that matter, is it really necessary to involve ACPI in port power
changes at all?  Why can't xhci-hcd simply set the PP bit in the PORTSC
register, and rely on the PPC bit in the HCCPARAMS register to indicate
whether or not port power control is supported?  In other words, what 
advantage does ACPI have over USB native power control?

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Fri, 11 May 2012, Greg KH wrote:

> > One tricky thing: In theory, there should be a separate sysfs file for 
> > each port.  That seems like a lot of overhead though; is there any way 
> > to present the information in a single file that won't offend sysfs 
> > purists?
> 
> Why is that a lot of "overhead"?  It's what, 7-9 files max?  As Sarah
> points out, one file for all ports is racy and can get to be a mess.
> 
> But then again, I'm a "sysfs purist" :)

Having multiple files makes it impossible to change several ports at
once, or see the values for all the ports at once.  Not a big deal, 
mostly a matter of convenience.

On Fri, 11 May 2012, Sarah Sharp wrote:

> Tianyu proposed having one file per hub, with a bit field that
> controlled each port power.  However, I was concerned about different
> userspace applications racing with each other to turn or off ports.  For
> example, one app could read the bit field, attempt to power off just
> port 1, but before it can write to the sysfs file, a second app powers
> on port2, and the first app then writes to the sysfs file, leaving port
> 1 powered off, and port 2 powered off, which is not what the second app
> wanted.

Writes should indicate only the items to be changed; they shouldn't 
specify the values for things that are to be left alone.  Then there's 
no race unless two tasks try to modify the same port at the same time, 
which is hopeless anyway.

> But if you can think of a better way to coalesce the port power off
> mechanisms into one file, we're all ears. :)

I'm sure this has become up before for other sysfs files.  We're
talking about an array, and having one file per array value is just
clunky.

Actually we're probably talking about two arrays: one for
power-control and one for power-status.

So for example, reading the power-control file could return a list of 
ports, one per line, something like this:

1:auto
2:on
3:auto
4:off

(I find such lists easier to interpret than a sequence of flags, like
"A+A-"; other people may disagree.)  Writes could be parsed as a list
of elements to be changed, separated by whitespace, such as:

	echo '2:auto 4:on' >/sys/.../power_control

The power-status file would be similar, except that it wouldn't be
writable and the value would be the current power status: either on or
off (not auto).  Or if you wanted to be fancy, each line could contain 
all the information from a Get-Port-Status request.

That's one possibility; no doubt others would work just as well.

Alan Stern

RE: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan, Tianyu]

On 2012/5/12 1:44, Alan Stern wrote:
> On Sat, 12 May 2012, Lan Tianyu wrote:
>>  From my opinion, ACPI will tell us whether the port is connectable or not.
> ACPI will tell you about some ports, not others (for example, ACPI
> knows nothing about the ports on hubs that the user plugs in).  On
> other systems, a Device Tree database might provide the same
> information.
I think we can not power off ports on the hubs that the user plugs in.
Power off needs platform support.  So those port that can be power
off must be on the motherboard and platform knows these ports.

>>> Makes sense.  (But I foresee a lot of confusion among users when this
>>> box is checked and the ports don't get powered down -- many of
>>> today's laptops are incapable of powering down their USB ports.)
>>>
>> How about to just provide sys files if the usb port can be power off
>> and the checkbox only is selectable when sys files exit.
> The problem is that the kernel doesn't know whether a port can be
> powered off.  For some ports, you may be able to rely on platform
> information like ACPI.  But for other ports, you simply can't tell.
  The same as previous. Platform should know all the port that
Can be power off. These port must be on the motherboard.
>>>> When we turn the power back on, we'll get a connect status change bit
>>>> set.  We don't want the USB core to misinterpret that bit and try to
>>>> logically disconnect the device attached to the port.
>>> All you have to do is turn off the connect-change status bit.  In fact,
>>> the debounce routine does this already.  Just leave the port state set
>>> to "off" until the connection is debounced.
>> This is my original version. :)
>> But after testing, I found that after the connection is debounced,
>> hub_port_connect_change() may not to be invoked or not reach place
>> "disconnect devices".
>>
>> hub_port_connect_change() {
>> ...
>> /* debounce may be completed before here and port's state has becomed
>>   * "on". The device also will be removed.
>>   */	
>> if (hub->port_data[port1 - 1].power_state == USB_PORT_POWER_STATE_OFF) {
>> 	clear_bit(port1, hub->change_bits);
>> 	return;
>> }
>> 	
>> /* Disconnect any existing devices under this port */
>> if (udev)
>> 	usb_disconnect(&hub->port_data[port1-1].child);
>> clear_bit(port1, hub->change_bits);
>>
>> ...
>> }
> This doesn't matter.  Disconnections will be handled by 
> usb_reset_and_verify_device(), when it is called by 
> finish_port_resume().
This will cause the device to be re-enumerated. Just like
unplugged and plugged a device. I think this is not what
we want.  Disconnect and create the same device. device
num is also change.
>
>> I spent a lot of time resolving the problem. At last, I found to add
>> "waiting for connection" state to resolve the problem.
>>
>> After power on the port, hub_irq() invokes kick_khubd() to deal with
>> connect or enable change event. Connect debouncing does in the
>> usb_port_resume() and  connect-change event is processed in hub_thread().
>> It is hard to synchronize them and the port's state should be set to
>> "on" after the connect-change event finishing.
> I still don't see what the problem is.  They don't have to be
> synchronized; all you need to do is make sure that the port's state
> remains set to "off" until the debouncing is finished and you have
> turned off the connect-change and enable-change features.
    Do you mean during debouncing, the USB_PORT_FEAT_CONNECTION
will be clear. So the hub_irq() will not receive connect-change event?

> I don't know; it seems more likely that you'll have a problem when the 
> power is turned off, not when it is turned on.  That's when the connect 
> and enable status changes occur.

Connect-change will occur both in power on and power off. The port
state is different "". Just like unplug and plug.

What I see is that during the deboucing, the connect-change event also
will received. So there are two paths.  The usb_port_resume will set
port state to "on". hub_port_connect_change() needs port state to
determine whether disconnect device. If "on", disconnect device.
If "off", return directly. They are in two threads. So how to make
sure to set port state to "on" after hub_port_connect_change()
make decision whether to disconnect device or not.

 usb_port_resume()
        ||
 power on the port
        ||                                                          
    deboucing                      connect change (connect status => on)
        ||                                       ||
   set port state "on"               hub_irq() kick_khub() (connect-change event received)
                                                 ||
                                          hub_thread() hub_event()
                                           hub_port_connect_change()
																					



-- 
Best Regards
Tianyu Lan
linux kernel enabling team


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 11:35:43AM -0700, Greg KH wrote:
> On Fri, May 11, 2012 at 01:44:26PM -0400, Alan Stern wrote:
> > On Sat, 12 May 2012, Lan Tianyu wrote:
> > > So I should work on the external ports without devices firstly and
> > > add the sys file for user to control?
> > 
> > Yes, I think so.  It will be less controversial and probably simpler.  
> > When that mechanism is ready, you should be able to use it
> > automatically for unconnectable ports.
> > 
> > One tricky thing: In theory, there should be a separate sysfs file for 
> > each port.  That seems like a lot of overhead though; is there any way 
> > to present the information in a single file that won't offend sysfs 
> > purists?
> 
> Why is that a lot of "overhead"?  It's what, 7-9 files max?  As Sarah
> points out, one file for all ports is racy and can get to be a mess.
> 
> But then again, I'm a "sysfs purist" :)

Theoretically there's no maximum number for xHCI root ports, so I
wouldn't be so sure you'll only have 9 files on future xHCI hosts.

Sarah Sharp

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 07:54:04PM +0000, Lan, Tianyu wrote:
> On 2012/5/12 1:44, Alan Stern wrote:
> > This doesn't matter.  Disconnections will be handled by 
> > usb_reset_and_verify_device(), when it is called by 
> > finish_port_resume().
> This will cause the device to be re-enumerated. Just like
> unplugged and plugged a device. I think this is not what
> we want.  Disconnect and create the same device. device
> num is also change.

Tianyu, do you have CONFIG_USB_PERSIST turned on?

Alan, do you know if that will that only help in this case after the
host controller PCI device is in D3hot?

Sarah Sharp

RE: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Fri, 11 May 2012, Lan, Tianyu wrote:

> On 2012/5/12 1:44, Alan Stern wrote:
> > On Sat, 12 May 2012, Lan Tianyu wrote:
> >>  From my opinion, ACPI will tell us whether the port is connectable or not.
> > ACPI will tell you about some ports, not others (for example, ACPI
> > knows nothing about the ports on hubs that the user plugs in).  On
> > other systems, a Device Tree database might provide the same
> > information.
> I think we can not power off ports on the hubs that the user plugs in.

You are wrong.  Some hubs do allow ports to be powered off.  Most 
don't, but some do.

> Power off needs platform support.

No, it doesn't.

>  So those port that can be power
> off must be on the motherboard and platform knows these ports.

> > The problem is that the kernel doesn't know whether a port can be
> > powered off.  For some ports, you may be able to rely on platform
> > information like ACPI.  But for other ports, you simply can't tell.
>   The same as previous. Platform should know all the port that
> Can be power off. These port must be on the motherboard.

See above.

> > This doesn't matter.  Disconnections will be handled by 
> > usb_reset_and_verify_device(), when it is called by 
> > finish_port_resume().
> This will cause the device to be re-enumerated. Just like
> unplugged and plugged a device. I think this is not what
> we want.

It is exactly what you want if the device really has been unplugged and 
was never plugged back in again.

>  Disconnect and create the same device. device
> num is also change.

Do you know what a "reset-resume" is?  See 
Documentation/usb/persist.txt.

> > I still don't see what the problem is.  They don't have to be
> > synchronized; all you need to do is make sure that the port's state
> > remains set to "off" until the debouncing is finished and you have
> > turned off the connect-change and enable-change features.
>     Do you mean during debouncing, the USB_PORT_FEAT_CONNECTION

USB_PORT_STAT_C_CONNECTION, not USB_PORT_FEAT_CONNECTION.

> will be clear. So the hub_irq() will not receive connect-change event?

No, hub_irq() definitely will receive events.  But so long as the
port's state remains set to "off", those events will be ignored.  You
should know, since you added code in the patch to do this!  :-)

And after the port's power state is changed to "on", the events won't 
do anything because the connect-change status will have been turned 
off.

> What I see is that during the deboucing, the connect-change event also
> will received. So there are two paths.  The usb_port_resume will set
> port state to "on". hub_port_connect_change() needs port state to
> determine whether disconnect device. If "on", disconnect device.
> If "off", return directly. They are in two threads. So how to make
> sure to set port state to "on" after hub_port_connect_change()
> make decision whether to disconnect device or not.
> 
>  usb_port_resume()
>         ||
>  power on the port
>         ||                                                          
>     deboucing                      connect change (connect status => on)
>         ||                                       ||
>    set port state "on"               hub_irq() kick_khub() (connect-change event received)
>                                                  ||
>                                           hub_thread() hub_event()
>                                            hub_port_connect_change()

Why is hub_port_connect_change() going to get called?  hub_event() 
doesn't call hub_port_connect_change() when it doesn't need to.
In this case it won't need to, because debouncing turns off the 
connect-change status.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 03:14:37PM -0400, Alan Stern wrote:
> On Fri, 11 May 2012, Sarah Sharp wrote:
> 
> > > That reminds me...  I think this should not be so closely linked with
> > > ACPI.  There's a perfectly good USB Clear-Feature request for turning
> > > off port power; that's what we should use.  If hooks are required for
> > > interfacing with platform-specific code (such as ACPI), they can be
> > > added at the appropriate places.
> > 
> > So would you rather userspace issue a clear port power feature request
> > to the roothub through libusb than have a sysfs file per port in
> > /sys/bus/usb/devices/../power/ ?  Or are you just saying that the sysfs
> > interface should issue the request to the hub (which may be the
> > roothub), and the xHCI driver can just implement the ACPI calls in its
> > roothub control method?
> 
> The latter.  Except that the ACPI calls may need to occur in more
> places than just xhci-hcd (ehci-hcd, for example).

Only the xHCI host controller will have the port power off mechanism.

> And what about ports on the USB-2 "rate-matching" hubs that Intel now
> builds into its chipsets?

For the Intel platform that has the port power off mechanism, there are
EHCI host controllers, but the port switch over changes *all* the USB
ports under xHCI.

> For that matter, is it really necessary to involve ACPI in port power
> changes at all?  Why can't xhci-hcd simply set the PP bit in the PORTSC
> register, and rely on the PPC bit in the HCCPARAMS register to indicate
> whether or not port power control is supported?  In other words, what 
> advantage does ACPI have over USB native power control?

The port power off mechanism is controlled by some other chunk of
hardware outside the xHCI host controller.  I asked the architects why
they didn't just use the port power control bits in the port status
registers, but they had already made their design choices by then.  So
we're stuck with the ACPI method of powering off the ports.

Sarah Sharp

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Fri, 11 May 2012, Sarah Sharp wrote:

> On Fri, May 11, 2012 at 07:54:04PM +0000, Lan, Tianyu wrote:
> > On 2012/5/12 1:44, Alan Stern wrote:
> > > This doesn't matter.  Disconnections will be handled by 
> > > usb_reset_and_verify_device(), when it is called by 
> > > finish_port_resume().
> > This will cause the device to be re-enumerated. Just like
> > unplugged and plugged a device. I think this is not what
> > we want.  Disconnect and create the same device. device
> > num is also change.
> 
> Tianyu, do you have CONFIG_USB_PERSIST turned on?
> 
> Alan, do you know if that will that only help in this case after the
> host controller PCI device is in D3hot?

See check_port_resume_type() in hub.c.  It doesn't care what the reason
is for loss connectivity during suspend or what state the host
controller was in.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Fri, 11 May 2012, Sarah Sharp wrote:

> On Fri, May 11, 2012 at 03:14:37PM -0400, Alan Stern wrote:
> > On Fri, 11 May 2012, Sarah Sharp wrote:
> > 
> > > > That reminds me...  I think this should not be so closely linked with
> > > > ACPI.  There's a perfectly good USB Clear-Feature request for turning
> > > > off port power; that's what we should use.  If hooks are required for
> > > > interfacing with platform-specific code (such as ACPI), they can be
> > > > added at the appropriate places.
> > > 
> > > So would you rather userspace issue a clear port power feature request
> > > to the roothub through libusb than have a sysfs file per port in
> > > /sys/bus/usb/devices/../power/ ?  Or are you just saying that the sysfs
> > > interface should issue the request to the hub (which may be the
> > > roothub), and the xHCI driver can just implement the ACPI calls in its
> > > roothub control method?
> > 
> > The latter.  Except that the ACPI calls may need to occur in more
> > places than just xhci-hcd (ehci-hcd, for example).
> 
> Only the xHCI host controller will have the port power off mechanism.

You're only talking about the upcoming Intel implementation, right?

> > And what about ports on the USB-2 "rate-matching" hubs that Intel now
> > builds into its chipsets?
> 
> For the Intel platform that has the port power off mechanism, there are
> EHCI host controllers, but the port switch over changes *all* the USB
> ports under xHCI.

Leaving aside the matter of people who don't use the port switch-over, 
what about other platforms?

> > For that matter, is it really necessary to involve ACPI in port power
> > changes at all?  Why can't xhci-hcd simply set the PP bit in the PORTSC
> > register, and rely on the PPC bit in the HCCPARAMS register to indicate
> > whether or not port power control is supported?  In other words, what 
> > advantage does ACPI have over USB native power control?
> 
> The port power off mechanism is controlled by some other chunk of
> hardware outside the xHCI host controller.  I asked the architects why
> they didn't just use the port power control bits in the port status
> registers, but they had already made their design choices by then.  So
> we're stuck with the ACPI method of powering off the ports.

Then yes, xhci_hub_control() would need an ACPI hook.  The hook belongs 
there, not in usbcore.

Can you convince the architects to do it the right way next time?  :-)

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Peter Stuge]

Sarah Sharp wrote:
> Theoretically there's no maximum number for xHCI root ports

Hm? Please clarify this?

HCSPARAMS1 MaxPorts is 8 bits and "Valid values are in the range of
1h to FFh."


//Peter

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 04:36:44PM -0400, Alan Stern wrote:
> On Fri, 11 May 2012, Sarah Sharp wrote:
> 
> > On Fri, May 11, 2012 at 03:14:37PM -0400, Alan Stern wrote:
> > > On Fri, 11 May 2012, Sarah Sharp wrote:
> > > 
> > > > > That reminds me...  I think this should not be so closely linked with
> > > > > ACPI.  There's a perfectly good USB Clear-Feature request for turning
> > > > > off port power; that's what we should use.  If hooks are required for
> > > > > interfacing with platform-specific code (such as ACPI), they can be
> > > > > added at the appropriate places.
> > > > 
> > > > So would you rather userspace issue a clear port power feature request
> > > > to the roothub through libusb than have a sysfs file per port in
> > > > /sys/bus/usb/devices/../power/ ?  Or are you just saying that the sysfs
> > > > interface should issue the request to the hub (which may be the
> > > > roothub), and the xHCI driver can just implement the ACPI calls in its
> > > > roothub control method?
> > > 
> > > The latter.  Except that the ACPI calls may need to occur in more
> > > places than just xhci-hcd (ehci-hcd, for example).
> > 
> > Only the xHCI host controller will have the port power off mechanism.
> 
> You're only talking about the upcoming Intel implementation, right?

Yes.  The port power off mechanism is an Intel specific ACPI call.
AFAIK, there isn't anyone else who is going to have this mechanism.

> > > And what about ports on the USB-2 "rate-matching" hubs that Intel now
> > > builds into its chipsets?
> > 
> > For the Intel platform that has the port power off mechanism, there are
> > EHCI host controllers, but the port switch over changes *all* the USB
> > ports under xHCI.
> 
> Leaving aside the matter of people who don't use the port switch-over, 
> what about other platforms?

I'm not sure about other future Intel chipsets, like server chipsets.
As I said, I don't think the port power off mechanism will save servers
a large percentage of their power budget.  I don't think there's any
plans to add the port power off mechanism to EHCI.

> > > For that matter, is it really necessary to involve ACPI in port power
> > > changes at all?  Why can't xhci-hcd simply set the PP bit in the PORTSC
> > > register, and rely on the PPC bit in the HCCPARAMS register to indicate
> > > whether or not port power control is supported?  In other words, what 
> > > advantage does ACPI have over USB native power control?
> > 
> > The port power off mechanism is controlled by some other chunk of
> > hardware outside the xHCI host controller.  I asked the architects why
> > they didn't just use the port power control bits in the port status
> > registers, but they had already made their design choices by then.  So
> > we're stuck with the ACPI method of powering off the ports.
> 
> Then yes, xhci_hub_control() would need an ACPI hook.  The hook belongs 
> there, not in usbcore.

Ok, Tianyu will have to rework the patches for that.

> Can you convince the architects to do it the right way next time?  :-)

I will do my best. :)

Sarah

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Greg KH]

On Fri, May 11, 2012 at 04:59:22PM -0700, Sarah Sharp wrote:
> > > Only the xHCI host controller will have the port power off mechanism.
> > 
> > You're only talking about the upcoming Intel implementation, right?
> 
> Yes.  The port power off mechanism is an Intel specific ACPI call.
> AFAIK, there isn't anyone else who is going to have this mechanism.

That's not fair, you don't know what other vendors are going to do in
their controllers, or their system firmware, to be able to accurately say
this.

> > > > And what about ports on the USB-2 "rate-matching" hubs that Intel now
> > > > builds into its chipsets?
> > > 
> > > For the Intel platform that has the port power off mechanism, there are
> > > EHCI host controllers, but the port switch over changes *all* the USB
> > > ports under xHCI.
> > 
> > Leaving aside the matter of people who don't use the port switch-over, 
> > what about other platforms?
> 
> I'm not sure about other future Intel chipsets, like server chipsets.
> As I said, I don't think the port power off mechanism will save servers
> a large percentage of their power budget.  I don't think there's any
> plans to add the port power off mechanism to EHCI.

Power savings mean more in real dollars to server systems than to mobile
systems.  So don't be supprised to see this show up on some server
systems, especially with the wide range of "custom" motherboards that
people are creating for large "cloud" data centers.

Never say never :)

greg k-h

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sergei Shtylyov]

Hello.

On 11-05-2012 22:12, Sarah Sharp wrote:

>>> For external ports, this should be associated with sys file control. The users
>>> need to determine when they should be power off.

>> You don't mean "external", you mean "not described as unconnectable by
>> ACPI".

>>> So I should work on the external ports without devices firstly and
>>> add the sys file for user to control?

>> Yes, I think so.  It will be less controversial and probably simpler.
>> When that mechanism is ready, you should be able to use it
>> automatically for unconnectable ports.

>> One tricky thing: In theory, there should be a separate sysfs file for
>> each port.  That seems like a lot of overhead though; is there any way
>> to present the information in a single file that won't offend sysfs
>> purists?

> Tianyu proposed having one file per hub, with a bit field that
> controlled each port power.  However, I was concerned about different
> userspace applications racing with each other to turn or off ports.  For
> example, one app could read the bit field, attempt to power off just
> port 1, but before it can write to the sysfs file, a second app powers
> on port2, and the first app then writes to the sysfs file, leaving port
> 1 powered off, and port 2 powered off, which is not what the second app
> wanted.

> But if you can think of a better way to coalesce the port power off
> mechanisms into one file, we're all ears. :)

    How about two files? You write 1 to the bit that matcheas port you want 
powered on in the 'poweron' file, and vice versa, you write 1 to the bit that 
matches port you want powered off in the 'poweroff' file.

> Sarah Sharp

WBR, Sergei

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Fri, 11 May 2012, Greg KH wrote:

> On Fri, May 11, 2012 at 04:59:22PM -0700, Sarah Sharp wrote:
> > > > Only the xHCI host controller will have the port power off mechanism.
> > > 
> > > You're only talking about the upcoming Intel implementation, right?
> > 
> > Yes.  The port power off mechanism is an Intel specific ACPI call.
> > AFAIK, there isn't anyone else who is going to have this mechanism.
> 
> That's not fair, you don't know what other vendors are going to do in
> their controllers, or their system firmware, to be able to accurately say
> this.

Right.  Even if other vendors don't use this ACPI mechanism, some of
them may very well implement the native USB port power control
mechanism.  If we add port power support to our drivers, we should 
certainly include native support.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Greg KH]

On Sat, May 12, 2012 at 04:47:58PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 11-05-2012 22:12, Sarah Sharp wrote:
> 
> >>>For external ports, this should be associated with sys file control. The users
> >>>need to determine when they should be power off.
> 
> >>You don't mean "external", you mean "not described as unconnectable by
> >>ACPI".
> 
> >>>So I should work on the external ports without devices firstly and
> >>>add the sys file for user to control?
> 
> >>Yes, I think so.  It will be less controversial and probably simpler.
> >>When that mechanism is ready, you should be able to use it
> >>automatically for unconnectable ports.
> 
> >>One tricky thing: In theory, there should be a separate sysfs file for
> >>each port.  That seems like a lot of overhead though; is there any way
> >>to present the information in a single file that won't offend sysfs
> >>purists?
> 
> >Tianyu proposed having one file per hub, with a bit field that
> >controlled each port power.  However, I was concerned about different
> >userspace applications racing with each other to turn or off ports.  For
> >example, one app could read the bit field, attempt to power off just
> >port 1, but before it can write to the sysfs file, a second app powers
> >on port2, and the first app then writes to the sysfs file, leaving port
> >1 powered off, and port 2 powered off, which is not what the second app
> >wanted.
> 
> >But if you can think of a better way to coalesce the port power off
> >mechanisms into one file, we're all ears. :)
> 
>    How about two files? You write 1 to the bit that matcheas port
> you want powered on in the 'poweron' file, and vice versa, you write
> 1 to the bit that matches port you want powered off in the
> 'poweroff' file.

Ick, no, bit manipulation in sysfs files isn't the nicest thing at all
(especially as Sarah implied that we could have more than 64 ports for a
host, and then what, we have what, 128bit variables here?)

One file per port please, in a generic way.

greg k-h

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012/5/12 4:20, Alan Stern wrote:
> On Fri, 11 May 2012, Lan, Tianyu wrote:
>
>> On 2012/5/12 1:44, Alan Stern wrote:
>>> On Sat, 12 May 2012, Lan Tianyu wrote:
>>>>    From my opinion, ACPI will tell us whether the port is connectable or not.
>>> ACPI will tell you about some ports, not others (for example, ACPI
>>> knows nothing about the ports on hubs that the user plugs in).  On
>>> other systems, a Device Tree database might provide the same
>>> information.
>> I think we can not power off ports on the hubs that the user plugs in.
> You are wrong.  Some hubs do allow ports to be powered off.  Most
> don't, but some do.
Ok. Thanks for correct. So how to control ports' power on those
hub? via set or clear PORT_POWER?
>>> I still don't see what the problem is.  They don't have to be
>>> synchronized; all you need to do is make sure that the port's state
>>> remains set to "off" until the debouncing is finished and you have
>>> turned off the connect-change and enable-change features.
>>      Do you mean during debouncing, the USB_PORT_FEAT_CONNECTION
> USB_PORT_STAT_C_CONNECTION, not USB_PORT_FEAT_CONNECTION.
>> will be clear. So the hub_irq() will not receive connect-change event?
Sorry. I may misunderstand your statement. I try to make it clear. :)

>All you have to do is turn off the connect-change status bit.  In fact,
>the debounce routine does this already.  Just leave the port state set
>to "off" until the connection is debounced.

I have checked hub_port_debounce(). The routine only clear
port feature USB_PORT_FEAT_C_CONNECTION when connect-change event
is detected.

"turn off the connect-change status bit". you mean unmask the
connect and enable change status bit.
just like:
portchange&= ~(USB_PORT_STAT_C_CONNECTION |
		USB_PORT_STAT_C_ENABLE);
right?

> Why is hub_port_connect_change() going to get called?  hub_event()
> doesn't call hub_port_connect_change() when it doesn't need to.
> In this case it won't need to, because debouncing turns off the
> connect-change status.
in the hub_event(), the routine will get port's status and
port change status every time. How does hub_port_debounce()
turn off the the value of port change status in the hub_event?

hub_event() {
	...
	connect_change = test_bit(i, hub->change_bits);
	if (!test_and_clear_bit(i, hub->event_bits)&&
					!connect_change)
				continue;

	ret = hub_port_status(hub, i,
		&portstatus,&portchange);
	if (ret<  0)
		continue;

	/*
	 * if we unmask the portchange here,
	 * the hub_port_connect_change() will not be
	 * invoked.
	 * portchange&= ~(USB_PORT_STAT_C_CONNECTION |
	 * USB_PORT_STAT_C_ENABLE);
	 */

	if (portchange&  USB_PORT_STAT_C_CONNECTION) {
		clear_port_feature(hdev, i,
			USB_PORT_FEAT_C_CONNECTION);
		connect_change = 1;
	}
	...
}

Is there a way to forbid connect-change or enable-change bits
to be on when these events take place?

> Do you know what a "reset-resume" is? See Documentation/usb/persist.txt.
I know reset-resume. But if hub_port_connect_change() is invoked.
It just like unplug and plug device.

My main question is that how does usb_port_debounce() prevent the
connect-change and enable-change bits to be off.
-- 
Best Regards
Tianyu Lan
linux kernel enabling team

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012/5/12 2:12, Sarah Sharp wrote:
> On Fri, May 11, 2012 at 01:44:26PM -0400, Alan Stern wrote:
>> On Sat, 12 May 2012, Lan Tianyu wrote:
>>
>>>      The power saving depends on devices. I test a usb3.0 ssd. The power saving of
>>> power off is about 2.2w more than just selective suspend. In theory, power
>>> off can help to save remaining power after selective suspend.
>> That's a lot of power!  Suspended USB devices aren't supposed to
>> consume more than 2.5 mA of bus current, which at 5 V amounts to<=
>> 0.0125 W.  Does the port really use that much?  Or does the SSD have a
>> separate power supply that it disables when port power is removed?
> I actually suspect that the host controller is powering off several PLLs
> if all the ports are powered off.  After all, if it doesn't have to pay
> attention to connects, disconnects, or remote wakeup, it can power off a
> lot more circuitry, and possibly even enter D3cold instead of D3hot when
> the PCI device is suspended (depending on what board Tianyu is testing
> on).  But I agree that it would be interesting to see if the SSD has a
> separate power supply that it's turning off.
>
There is no a separate power supply. All power came from host desktop 
via usb cable.

-- 
Best Regards
Tianyu Lan
linux kernel enabling team

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012/5/13 1:47, Lan Tianyu wrote:
> My main question is that how does usb_port_debounce() prevent the
> connect-change and enable-change bits to be off.
Sorry. a typo.
My main question is that how usb_port_debounce() prevents the
connect-change and enable-change bits to be on.

-- 
Best Regards
Tianyu Lan
linux kernel enabling team

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Sun, 13 May 2012, Lan Tianyu wrote:

> >> I think we can not power off ports on the hubs that the user plugs in.
> > You are wrong.  Some hubs do allow ports to be powered off.  Most
> > don't, but some do.
> Ok. Thanks for correct. So how to control ports' power on those
> hub? via set or clear PORT_POWER?

That's the only way.  :-)

> Sorry. I may misunderstand your statement. I try to make it clear. :)
> 
> >All you have to do is turn off the connect-change status bit.  In fact,
> >the debounce routine does this already.  Just leave the port state set
> >to "off" until the connection is debounced.
> 
> I have checked hub_port_debounce(). The routine only clear
> port feature USB_PORT_FEAT_C_CONNECTION when connect-change event
> is detected.

Forget about hub_port_debounce().  You shouldn't be using it anyway; 
ports need to be debounced only when a cable is physically plugged in 
or unplugged.  Electrical switching doesn't need debouncing.

Instead, what you should do after turning on the port power is wait for
up to one second (say) for the connect status to turn on.  Then make
sure the port's connect-change and enable-change status bits are off
(*), set the port's status back to "on", and call finish_port_resume().

> "turn off the connect-change status bit". you mean unmask the
> connect and enable change status bit.
> just like:
> portchange&= ~(USB_PORT_STAT_C_CONNECTION |
> 		USB_PORT_STAT_C_ENABLE);
> right?

No, I mean send Clear-Port-Feature requests for
USB_PORT_FEAT_C_CONNECTION and USB_PORT_FEAT_C_ENABLE.  It might be
necessary to do the same for USB_PORT_FEAT_C_SUSPEND,
USB_PORT_FEAT_C_PORT_LINK_STATE, and maybe even
USB_PORT_FEAT_C_BH_PORT_RESET.  There's already some code in
hub_activate() that does something like this; you might be able to
reuse it.

> > Why is hub_port_connect_change() going to get called?  hub_event()
> > doesn't call hub_port_connect_change() when it doesn't need to.
> > In this case it won't need to, because debouncing turns off the
> > connect-change status.
> in the hub_event(), the routine will get port's status and
> port change status every time. How does hub_port_debounce()
> turn off the the value of port change status in the hub_event?
> 
> hub_event() {
> 	...
> 	connect_change = test_bit(i, hub->change_bits);
> 	if (!test_and_clear_bit(i, hub->event_bits)&&
> 					!connect_change)
> 				continue;
> 
> 	ret = hub_port_status(hub, i,
> 		&portstatus,&portchange);
> 	if (ret<  0)
> 		continue;
> 
> 	/*
> 	 * if we unmask the portchange here,
> 	 * the hub_port_connect_change() will not be
> 	 * invoked.
> 	 * portchange&= ~(USB_PORT_STAT_C_CONNECTION |
> 	 * USB_PORT_STAT_C_ENABLE);
> 	 */

Not needed.

> 	if (portchange&  USB_PORT_STAT_C_CONNECTION) {
> 		clear_port_feature(hdev, i,
> 			USB_PORT_FEAT_C_CONNECTION);
> 		connect_change = 1;
> 	}

Right.  But portchange & USB_PORT_STAT_C_CONNECTION will be 0 because 
at (*) we sent the Clear-Port-Feature request.

> 	...
> }
> 
> Is there a way to forbid connect-change or enable-change bits
> to be on when these events take place?

You don't need to forbid them.

> > Do you know what a "reset-resume" is? See Documentation/usb/persist.txt.
> I know reset-resume. But if hub_port_connect_change() is invoked.
> It just like unplug and plug device.

It won't be invoked.

> My main question is that how does usb_port_debounce() prevent the
> connect-change and enable-change bits to be off.

Forget about hub_port_debounce().  See above.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 05:17:22PM -0700, Greg KH wrote:
> On Fri, May 11, 2012 at 04:59:22PM -0700, Sarah Sharp wrote:
> > > > Only the xHCI host controller will have the port power off mechanism.
> > > 
> > > You're only talking about the upcoming Intel implementation, right?
> > 
> > Yes.  The port power off mechanism is an Intel specific ACPI call.
> > AFAIK, there isn't anyone else who is going to have this mechanism.
> 
> That's not fair, you don't know what other vendors are going to do in
> their controllers, or their system firmware, to be able to accurately say
> this.

Yes, you are correct, I don't know if other vendors will do this in the
future, or if they'll use the same ACPI API.

AFAIK, the port power off mechanism has only been discussed internally
within Intel until this thread.  So it's unlikely that anyone will have
this mechanism in the near future.  But as you said, never say never. :)

We'll strive to make the mechanism as generic as possible, so that other
vendors can add their own ACPI hooks into the xHCI driver if they choose
to do a similar port power off mechanism.

Sarah Sharp

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Fri, May 11, 2012 at 11:09:40PM +0200, Peter Stuge wrote:
> Sarah Sharp wrote:
> > Theoretically there's no maximum number for xHCI root ports
> 
> Hm? Please clarify this?
> 
> HCSPARAMS1 MaxPorts is 8 bits and "Valid values are in the range of
> 1h to FFh."

Sorry, yes, you're correct, the maximum number of ports for xHCI is 255.
I was just trying to point out that it's possible that xHCI hosts will
have a much larger number of USB ports than what we've previously seen
on EHCI host controllers.

Sarah Sharp

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Peter Stuge]

Sarah Sharp wrote:
> > > Theoretically there's no maximum number for xHCI root ports
> > 
> > Hm? Please clarify this?
> > 
> > HCSPARAMS1 MaxPorts is 8 bits and "Valid values are in the range of
> > 1h to FFh."
> 
> Sorry, yes, you're correct, the maximum number of ports for xHCI is 255.

Thanks for confirming!


> I was just trying to point out that it's possible that xHCI hosts will
> have a much larger number of USB ports than what we've previously seen
> on EHCI host controllers.

Yep, absolutely. I was just worried that I had missed something.


//Peter

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

Since alan suggested we should start simpler version(power
down port that have no devices attached) firstly. And we
also should provide general power off mechanism.

I have a proposal.
first:  add portx_power_control sys file.
      echo "auto" => portx_power_control
                     check whether there is a device. if no, send the clear
                     USB_PORT_FEAT_POWER request. (since general way
                     to control port power is via set or clear PORT_POWER)

     echo "on" =>  portx_power_control
                     send the set  USB_PORT_FEAT_POWER request.

second:
In the  xhci driver, add acpi hook in the xhci_hub_control()
                 where USB_PORT_FEAT_POWER request is dealt with.

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Greg KH]

On Tue, May 15, 2012 at 10:31:38PM +0800, Lan Tianyu wrote:
> Since alan suggested we should start simpler version(power
> down port that have no devices attached) firstly. And we
> also should provide general power off mechanism.
> 
> I have a proposal.
> first:  add portx_power_control sys file.
>      echo "auto" => portx_power_control
>                     check whether there is a device. if no, send the clear
>                     USB_PORT_FEAT_POWER request. (since general way
>                     to control port power is via set or clear PORT_POWER)
> 
>     echo "on" =>  portx_power_control
>                     send the set  USB_PORT_FEAT_POWER request.

No "off"?

> second:
> In the  xhci driver, add acpi hook in the xhci_hub_control()
>                 where USB_PORT_FEAT_POWER request is dealt with.

Do you have a patch showing that this works?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Tue, May 15, 2012 at 08:18:00AM -0700, Greg KH wrote:
> On Tue, May 15, 2012 at 10:31:38PM +0800, Lan Tianyu wrote:
> > Since alan suggested we should start simpler version(power
> > down port that have no devices attached) firstly. And we
> > also should provide general power off mechanism.
> > 
> > I have a proposal.
> > first:  add portx_power_control sys file.
> >      echo "auto" => portx_power_control
> >                     check whether there is a device. if no, send the clear
> >                     USB_PORT_FEAT_POWER request. (since general way
> >                     to control port power is via set or clear PORT_POWER)
> > 
> >     echo "on" =>  portx_power_control
> >                     send the set  USB_PORT_FEAT_POWER request.
> 
> No "off"?

Yeah, we probably need an "off" for those libusb and ModemManager
developers who want to try and simulate a disconnect of a misbehaving
device.

Sarah Sharp

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012年05月16日 04:00, Sarah Sharp wrote:
> On Tue, May 15, 2012 at 08:18:00AM -0700, Greg KH wrote:
>> On Tue, May 15, 2012 at 10:31:38PM +0800, Lan Tianyu wrote:
>>> Since alan suggested we should start simpler version(power
>>> down port that have no devices attached) firstly. And we
>>> also should provide general power off mechanism.
>>>
>>> I have a proposal.
>>> first:  add portx_power_control sys file.
>>>       echo "auto" =>  portx_power_control
>>>                      check whether there is a device. if no, send the clear
>>>                      USB_PORT_FEAT_POWER request. (since general way
>>>                      to control port power is via set or clear PORT_POWER)
>>>
>>>      echo "on" =>   portx_power_control
>>>                      send the set  USB_PORT_FEAT_POWER request.
>>
>> No "off"?
>
> Yeah, we probably need an "off" for those libusb and ModemManager
> developers who want to try and simulate a disconnect of a misbehaving
> device.
>
> Sarah Sharp

Ok. Let me reconfirm the meaning of these opinions.

"auto"
	if port without device, turn off power directly.
	if port with device, turn off power when the device is suspended.

"on"	the port's power must be on.

"off"	the port's power must be off.

Right?

> On Tue, May 15, 2012 at 08:18:00AM -0700, Greg KH wrote:
>> Do you have a patch showing that this works?
Currently no. I will write a RFC version and send out for discuss.

-- 
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Wed, 16 May 2012, Lan Tianyu wrote:

> Ok. Let me reconfirm the meaning of these opinions.
> 
> "auto"
> 	if port without device, turn off power directly.
> 	if port with device, turn off power when the device is suspended.
> 
> "on"	the port's power must be on.
> 
> "off"	the port's power must be off.
> 
> Right?

Maybe the "auto" setting should be renamed "active".  Then "auto" can 
be used for:

	If the port has a device, turn the power on.
	If the port doesn't have a device, turn the power off.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Greg KH]

On Wed, May 16, 2012 at 10:36:02AM -0400, Alan Stern wrote:
> On Wed, 16 May 2012, Lan Tianyu wrote:
> 
> > Ok. Let me reconfirm the meaning of these opinions.
> > 
> > "auto"
> > 	if port without device, turn off power directly.
> > 	if port with device, turn off power when the device is suspended.
> > 
> > "on"	the port's power must be on.
> > 
> > "off"	the port's power must be off.
> > 
> > Right?
> 
> Maybe the "auto" setting should be renamed "active".  Then "auto" can 
> be used for:
> 
> 	If the port has a device, turn the power on.
> 	If the port doesn't have a device, turn the power off.

And plugging a new device into a port that was powered off is going to
cause the power to come back on, right?

greg k-h

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012/5/16 22:39, Greg KH wrote:
> On Wed, May 16, 2012 at 10:36:02AM -0400, Alan Stern wrote:
>> On Wed, 16 May 2012, Lan Tianyu wrote:
>>
>>> Ok. Let me reconfirm the meaning of these opinions.
>>>
>>> "auto"
>>> 	if port without device, turn off power directly.
>>> 	if port with device, turn off power when the device is suspended.
>>>
>>> "on"	the port's power must be on.
>>>
>>> "off"	the port's power must be off.
>>>
>>> Right?
>> Maybe the "auto" setting should be renamed "active".  Then "auto" can
>> be used for:
>>
>> 	If the port has a device, turn the power on.
>> 	If the port doesn't have a device, turn the power off.
> And plugging a new device into a port that was powered off is going to
> cause the power to come back on, right?
When the port was powered off, the connect-change event will not be 
detected and
we don't know a device was plugged in. :(
>
> greg k-h


-- 
Best Regards
Tianyu Lan
linux kernel enabling team

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Greg KH]

On Wed, May 16, 2012 at 10:54:13PM +0800, Lan Tianyu wrote:
> On 2012/5/16 22:39, Greg KH wrote:
> >On Wed, May 16, 2012 at 10:36:02AM -0400, Alan Stern wrote:
> >>On Wed, 16 May 2012, Lan Tianyu wrote:
> >>
> >>>Ok. Let me reconfirm the meaning of these opinions.
> >>>
> >>>"auto"
> >>>	if port without device, turn off power directly.
> >>>	if port with device, turn off power when the device is suspended.
> >>>
> >>>"on"	the port's power must be on.
> >>>
> >>>"off"	the port's power must be off.
> >>>
> >>>Right?
> >>Maybe the "auto" setting should be renamed "active".  Then "auto" can
> >>be used for:
> >>
> >>	If the port has a device, turn the power on.
> >>	If the port doesn't have a device, turn the power off.
> >And plugging a new device into a port that was powered off is going to
> >cause the power to come back on, right?
> When the port was powered off, the connect-change event will not be
> detected and
> we don't know a device was plugged in. :(

Then that doesn't seem like a very good thing to ever do, right?

greg k-h

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Alan Stern]

On Wed, 16 May 2012, Lan Tianyu wrote:

> On 2012/5/16 22:39, Greg KH wrote:
> > On Wed, May 16, 2012 at 10:36:02AM -0400, Alan Stern wrote:
> >> On Wed, 16 May 2012, Lan Tianyu wrote:
> >>
> >>> Ok. Let me reconfirm the meaning of these opinions.
> >>>
> >>> "auto"
> >>> 	if port without device, turn off power directly.
> >>> 	if port with device, turn off power when the device is suspended.
> >>>
> >>> "on"	the port's power must be on.
> >>>
> >>> "off"	the port's power must be off.
> >>>
> >>> Right?
> >> Maybe the "auto" setting should be renamed "active".  Then "auto" can
> >> be used for:
> >>
> >> 	If the port has a device, turn the power on.
> >> 	If the port doesn't have a device, turn the power off.
> > And plugging a new device into a port that was powered off is going to
> > cause the power to come back on, right?
> When the port was powered off, the connect-change event will not be 
> detected and
> we don't know a device was plugged in. :(

Ah, yes.  Okay, then I guess we _don't_ want a setting like the one I 
suggested.  The original proposal was good.

Alan Stern

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Lan Tianyu]

On 2012/5/16 23:08, Greg KH wrote:
> On Wed, May 16, 2012 at 10:54:13PM +0800, Lan Tianyu wrote:
>> On 2012/5/16 22:39, Greg KH wrote:
>>> On Wed, May 16, 2012 at 10:36:02AM -0400, Alan Stern wrote:
>>>> On Wed, 16 May 2012, Lan Tianyu wrote:
>>>>
>>>>> Ok. Let me reconfirm the meaning of these opinions.
>>>>>
>>>>> "auto"
>>>>> 	if port without device, turn off power directly.
>>>>> 	if port with device, turn off power when the device is suspended.
>>>>>
>>>>> "on"	the port's power must be on.
>>>>>
>>>>> "off"	the port's power must be off.
>>>>>
>>>>> Right?
>>>> Maybe the "auto" setting should be renamed "active".  Then "auto" can
>>>> be used for:
>>>>
>>>> 	If the port has a device, turn the power on.
>>>> 	If the port doesn't have a device, turn the power off.
>>> And plugging a new device into a port that was powered off is going to
>>> cause the power to come back on, right?
>> When the port was powered off, the connect-change event will not be
>> detected and
>> we don't know a device was plugged in. :(
> Then that doesn't seem like a very good thing to ever do, right?
Maybe. In some situations, user don't care whether there is device 
plugged in
for example, the blank screen. This is user space policy. We provide a way
for them to control the power on the port and they control when power
off the port.
> greg k-h


-- 
Best Regards
Tianyu Lan
linux kernel enabling team

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sarah Sharp]

On Wed, May 16, 2012 at 08:08:46AM -0700, Greg KH wrote:
> On Wed, May 16, 2012 at 10:54:13PM +0800, Lan Tianyu wrote:
> > On 2012/5/16 22:39, Greg KH wrote:
> > >On Wed, May 16, 2012 at 10:36:02AM -0400, Alan Stern wrote:
> > >>On Wed, 16 May 2012, Lan Tianyu wrote:
> > >>
> > >>>Ok. Let me reconfirm the meaning of these opinions.
> > >>>
> > >>>"auto"
> > >>>	if port without device, turn off power directly.
> > >>>	if port with device, turn off power when the device is suspended.
> > >>>
> > >>>"on"	the port's power must be on.
> > >>>
> > >>>"off"	the port's power must be off.
> > >>>
> > >>>Right?
> > >>Maybe the "auto" setting should be renamed "active".  Then "auto" can
> > >>be used for:
> > >>
> > >>	If the port has a device, turn the power on.
> > >>	If the port doesn't have a device, turn the power off.
> > >And plugging a new device into a port that was powered off is going to
> > >cause the power to come back on, right?
> > When the port was powered off, the connect-change event will not be
> > detected and
> > we don't know a device was plugged in. :(
> 
> Then that doesn't seem like a very good thing to ever do, right?

When the port is powered off, VBUS is removed.  That means we don't get
connect, disconnect, or remote wakeup events.  The most useful case for
this is if we know there's a USB port that's internal (say inside a
laptop) that either is empty, or has a USB device that's suspended
without remote wakeup enabled.  Then we know we can safely power off the
port, because we shouldn't ever see a connect, disconnect, or remote
wakeup.

We can use ACPI tables to figure out whether a USB port is physically
connectable (e.g. internal) or not.  Of course, the ACPI tables may well
lie to us, so we need to export the sysfs interface so that userspace
can override any kernel defaults WRT powering off internal ports.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] usb/acpi: Add support usb port power off mechanism for device fixed on the motherboard

[Sergei Shtylyov]

Hello.

On 16-05-2012 18:39, Greg KH wrote:

>>> Ok. Let me reconfirm the meaning of these opinions.

>>> "auto"
>>> 	if port without device, turn off power directly.
>>> 	if port with device, turn off power when the device is suspended.

>>> "on"	the port's power must be on.

>>> "off"	the port's power must be off.

>>> Right?

>> Maybe the "auto" setting should be renamed "active".  Then "auto" can
>> be used for:

>> 	If the port has a device, turn the power on.
>> 	If the port doesn't have a device, turn the power off.

> And plugging a new device into a port that was powered off is going to
> cause the power to come back on, right?

    How plugging a device can be detected on unpowered port?

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html