Re: [PATCH 08/10] usb: add usb port auto power off mechanism

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 14 Dec 2012, Lan Tianyu wrote:

> Hi Alan:
> 	debounce is still needed. If connect status was not stable, resume
> operation will fail. So how about following?

Actually, I'm not sure that a debounce is needed.

Debouncing is used when a plug is physically inserted or removed, 
because the electrical terminals make fleeting contact with one another 
(they literally bounce on and off each other for a brief time).  But in 
this case there is no physical insertion.  The terminals are in stable 
physical contact with each other, so they can't bounce.

> int usb_port_wait_for_connected(struct usb_hub *hub, int port1)
> {
> 	u16 portchange, portstatus;
> 	int total_time, ret;
> 
> 	for (total_time = 0; total_time < 2000; total_time += 20) {
> 		ret = hub_port_status(hub, port1, &portstatus, &portchange);
> 		if (ret < 0)
> 			return ret;
> 
> 		if ((portstatus & USB_PORT_STAT_CONNECTION)
> 			&& (hub_port_debounce(hub, port1) & USB_PORT_STAT_CONNECTION)) {
> 			/*
> 			 * just clear enable-change feature since debounce
> 			 *  has cleared connect-change feature.
> 			 */
> 			clear_port_feature(hub->hdev, port1,
> 					USB_PORT_FEAT_C_ENABLE);
> 			return 0;
> 		}
> 		mdelay(20);
> 	}
> 	return -ETIMEDOUT;
> }

Why do you want to continue the loop when hub_port_debounce fails?

> > If you get a connect change immediately after turning off the port 
> > power, there's no way to know whether it was because the power is now 
> > off or because the device really disconnected.  So the best you can do 
> > is turn on the busy_bits entry, turn off the power, clear the 
> > connect-change and enable-change statuses, and turn off the busy_bits 
> > entry.
> > 
> You mean this?
> static int usb_port_runtime_suspend(struct device *dev)
> {
> 	struct usb_port *port_dev = to_usb_port(dev);
> 	struct usb_device *hdev =
> 		to_usb_device(dev->parent->parent);
> 	struct usb_hub *hub = hdev_to_hub(hdev);
> 	int port1 = port_dev->portnum;
> 	int retval;
> 
> 	if (dev_pm_qos_flags(&port_dev->dev, PM_QOS_FLAG_NO_POWER_OFF)
> 			== PM_QOS_FLAGS_ALL)
> 		return -EAGAIN;
> 
> 	set_bit(port1, hub->busy_bits);
> 	retval = usb_hub_set_port_power(hdev, port1, false);
> 	clear_port_feature(hdev, port1,
> 			USB_PORT_FEAT_C_CONNECTION);
> 	clear_port_feature(hdev, port1,
> 			USB_PORT_FEAT_C_ENABLE);
> 	clear_bit(port1, hub->busy_bits);
> 	return retval;		
> }

Yes, something like that.

We're going to have to take a little more care with busy_bits in the
future.  It's not protected by any lock, and there's not much to
prevent two different threads from using the same bit at the same time.  
Only the fact that this would mean they were trying to control the same
port at the same time.

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux