On Mon, 23 Jul 2012, Lan Tianyu wrote:
> hi alan:
> I accord to your advice to implement usb port power off mechanism
> for port with device (add "auto" option for portX/control to allow port
> to be power off during device being suspended and power on when resuming).
>
> http://marc.info/?l=linux-usb&m=133675841421390&w=2
> > 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.
>
> But the device is still disconnected after powering on the port during
> resuming. Caused by that port_power had been set to "on" when connect-change
> event was processed. My patch is at the bottom of the mail. If something
> wrong, please help me to correct. Thanks.
> @@ -3027,6 +3070,24 @@ int usb_port_resume(struct usb_device *u
> int status;
> u16 portchange, portstatus;
>
> + if (hub->ports[port1 - 1]->port_power_policy == USB_PORT_POWER_AUTO
> + && hub->ports[port1 - 1]->power_state ==
> USB_PORT_POWER_STATE_OFF) {
> + set_port_feature(udev->parent, port1, USB_PORT_FEAT_POWER);
> +
> + /*
> + * 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 " \
> + "power on, status %d\n", status);
> + return status;
> + }
> + hub->ports[port1 - 1]->power_state = USB_PORT_POWER_STATE_ON;
> + pr_info("%s: port%d connect state on %ld\n", __func__, port1,
> jiffies);
> + }
> +
> /* 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))
A few lines later the driver does:
set_bit(port1, hub->busy_bits);
You merely need to move this line up before the point where you turn
port power back on. Make it the first executable line of the function.
> @@ -3058,7 +3119,6 @@ int usb_port_resume(struct usb_device *u
> * sequence.
> */
> status = hub_port_status(hub, port1, &portstatus, &portchange);
> -
You don't need to remove this blank line.
> /* TRSMRCY = 10 msec */
> msleep(10);
> }
> @@ -4177,6 +4237,13 @@ static void hub_port_connect_change(stru
> }
> }
>
> + if (hub->ports[port1 - 1]->port_power_policy == USB_PORT_POWER_AUTO
> + && hub->ports[port1 - 1]->power_state ==
> USB_PORT_POWER_STATE_OFF) {
Does the policy matter here? I suspect only the power_state is important.
> + clear_bit(port1, hub->change_bits);
> + return;
> + }
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
