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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
