Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit db690874 authored by David Brownell's avatar David Brownell Committed by Greg Kroah-Hartman
Browse files

[PATCH] usb_interface power state 



This updates the handling of power state for USB interfaces.

  - Formalizes an existing invariant:  interface "power state" is a boolean:
    ON when I/O is allowed, and FREEZE otherwise.  It does so by defining
    some inlined helpers, then using them.

  - Adds a useful invariant:  the only interfaces marked active are those
    bound to non-suspended drivers.  Later patches build on this invariant.

  - Simplifies the interface driver API (and removes some error paths) by
    removing the requirement that they record power state changes during
    suspend and resume callbacks.  Now usbcore does that.

A few drivers were simplified to address that last change.

Signed-off-by: default avatarDavid Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>

 drivers/usb/core/hub.c       |   33 +++++++++------------
 drivers/usb/core/message.c   |    1
 drivers/usb/core/usb.c       |   65 +++++++++++++++++++++++++++++++++----------
 drivers/usb/core/usb.h       |   18 +++++++++++
 drivers/usb/input/hid-core.c |    2 -
 drivers/usb/misc/usbtest.c   |   10 ------
 drivers/usb/net/pegasus.c    |    2 -
 drivers/usb/net/usbnet.c     |    2 -
 8 files changed, 85 insertions(+), 48 deletions(-)
parent 7586269c

drivers/usb/core/hub.c

+15 −18
Original line number Diff line number Diff line
@@ -1624,7 +1624,7 @@ static int __usb_suspend_device (struct usb_device *udev, int port1, 
			struct usb_driver	*driver;



			intf = udev->actconfig->interface[i];

			if (state.event <= intf->dev.power.power_state.event)

			if (!is_active(intf))

				continue;

			if (!intf->dev.driver)

				continue;
@@ -1632,11 +1632,12 @@ static int __usb_suspend_device (struct usb_device *udev, int port1, 


			if (driver->suspend) {

				status = driver->suspend(intf, state);

				if (intf->dev.power.power_state.event != state.event

						|| status)

				if (status == 0)

					mark_quiesced(intf);

				else

					dev_err(&intf->dev,

						"suspend %d fail, code %d\n",

						state.event, status);

						"suspend error %d\n",

						status);

			}



			/* only drivers with suspend() can ever resume();
@@ -1787,7 +1788,7 @@ static int finish_port_resume(struct usb_device *udev) 
			struct usb_driver	*driver;



			intf = udev->actconfig->interface[i];

			if (intf->dev.power.power_state.event == PM_EVENT_ON)

			if (is_active(intf))

				continue;

			if (!intf->dev.driver) {

				/* FIXME maybe force to alt 0 */
@@ -1800,12 +1801,14 @@ static int finish_port_resume(struct usb_device *udev) 
				continue;



			/* can we do better than just logging errors? */

			mark_active(intf);

			status = driver->resume(intf);

			if (intf->dev.power.power_state.event != PM_EVENT_ON

					|| status)

			if (status < 0) {

				mark_quiesced(intf);

				dev_dbg(&intf->dev,

					"resume fail, state %d code %d\n",

					intf->dev.power.power_state.event, status);

					"resume error %d\n",

					status);

			}

		}

		status = 0;
@@ -1952,7 +1955,7 @@ static int remote_wakeup(struct usb_device *udev) 
	return status;

}



static int hub_suspend(struct usb_interface *intf, pm_message_t state)

static int hub_suspend(struct usb_interface *intf, pm_message_t msg)

{

	struct usb_hub		*hub = usb_get_intfdata (intf);

	struct usb_device	*hdev = hub->hdev;
@@ -1970,14 +1973,13 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t state) 
		if (!udev)

			continue;

		down(&udev->serialize);

		status = __usb_suspend_device(udev, port1, state);

		status = __usb_suspend_device(udev, port1, msg);

		up(&udev->serialize);

		if (status < 0)

			dev_dbg(&intf->dev, "suspend port %d --> %d\n",

				port1, status);

	}



	intf->dev.power.power_state = state;

	return 0;

}
@@ -1988,9 +1990,6 @@ static int hub_resume(struct usb_interface *intf) 
	unsigned		port1;

	int			status;



	if (intf->dev.power.power_state.event == PM_EVENT_ON)

		return 0;



	for (port1 = 1; port1 <= hdev->maxchild; port1++) {

		struct usb_device	*udev;

		u16			portstat, portchange;
@@ -2024,8 +2023,6 @@ static int hub_resume(struct usb_interface *intf) 
		}

		up(&udev->serialize);

	}

	intf->dev.power.power_state = PMSG_ON;



	hub->resume_root_hub = 0;

	hub_activate(hub);

	return 0;

drivers/usb/core/message.c

+1 −0
Original line number Diff line number Diff line
@@ -1393,6 +1393,7 @@ free_interfaces: 
			intf->dev.dma_mask = dev->dev.dma_mask;

			intf->dev.release = release_interface;

			device_initialize (&intf->dev);

			mark_quiesced(intf);

			sprintf (&intf->dev.bus_id[0], "%d-%s:%d.%d",

				 dev->bus->busnum, dev->devpath,

				 configuration,

drivers/usb/core/usb.c

+51 −14
Original line number Diff line number Diff line
@@ -107,10 +107,19 @@ static int usb_probe_interface(struct device *dev) 
	id = usb_match_id (intf, driver->id_table);

	if (id) {

		dev_dbg (dev, "%s - got id\n", __FUNCTION__);



		/* Interface "power state" doesn't correspond to any hardware

		 * state whatsoever.  We use it to record when it's bound to

		 * a driver that may start I/0:  it's not frozen/quiesced.

		 */

		mark_active(intf);

		intf->condition = USB_INTERFACE_BINDING;

		error = driver->probe (intf, id);

		intf->condition = error ? USB_INTERFACE_UNBOUND :

				USB_INTERFACE_BOUND;

		if (error) {

			mark_quiesced(intf);

			intf->condition = USB_INTERFACE_UNBOUND;

		} else

			intf->condition = USB_INTERFACE_BOUND;

	}



	return error;
@@ -136,6 +145,7 @@ static int usb_unbind_interface(struct device *dev) 
			0);

	usb_set_intfdata(intf, NULL);

	intf->condition = USB_INTERFACE_UNBOUND;

	mark_quiesced(intf);



	return 0;

}
@@ -299,6 +309,7 @@ int usb_driver_claim_interface(struct usb_driver *driver, 
	dev->driver = &driver->driver;

	usb_set_intfdata(iface, priv);

	iface->condition = USB_INTERFACE_BOUND;

	mark_active(iface);



	/* if interface was already added, bind now; else let

	 * the future device_add() bind it, bypassing probe()
@@ -345,6 +356,7 @@ void usb_driver_release_interface(struct usb_driver *driver, 
	dev->driver = NULL;

	usb_set_intfdata(iface, NULL);

	iface->condition = USB_INTERFACE_UNBOUND;

	mark_quiesced(iface);

}



/**
@@ -1406,6 +1418,7 @@ static int usb_generic_suspend(struct device *dev, pm_message_t message) 
{

	struct usb_interface	*intf;

	struct usb_driver	*driver;

	int			status;



	if (dev->driver == &usb_generic_driver)

		return usb_suspend_device (to_usb_device(dev), message);
@@ -1417,21 +1430,34 @@ static int usb_generic_suspend(struct device *dev, pm_message_t message) 
	intf = to_usb_interface(dev);

	driver = to_usb_driver(dev->driver);



	/* there's only one USB suspend state */

	if (intf->dev.power.power_state.event)

	/* with no hardware, USB interfaces only use FREEZE and ON states */

	if (!is_active(intf))

		return 0;



	if (driver->suspend)

		return driver->suspend(intf, message);

	return 0;

	if (driver->suspend && driver->resume) {

		status = driver->suspend(intf, message);

		if (status)

			dev_err(dev, "%s error %d\n", "suspend", status);

		else

			mark_quiesced(intf);

	} else {

		// FIXME else if there's no suspend method, disconnect...

		dev_warn(dev, "no %s?\n", "suspend");

		status = 0;

	}

	return status;

}



static int usb_generic_resume(struct device *dev)

{

	struct usb_interface	*intf;

	struct usb_driver	*driver;

	int			status;



	if (dev->power.power_state.event == PM_EVENT_ON)

		return 0;



	/* devices resume through their hub */

	/* devices resume through their hubs */

	if (dev->driver == &usb_generic_driver)

		return usb_resume_device (to_usb_device(dev));
@@ -1442,8 +1468,19 @@ static int usb_generic_resume(struct device *dev) 
	intf = to_usb_interface(dev);

	driver = to_usb_driver(dev->driver);



	if (driver->resume)

		return driver->resume(intf);

	/* if driver was suspended, it has a resume method;

	 * however, sysfs can wrongly mark things as suspended

	 * (on the "no suspend method" FIXME path above)

	 */

	mark_active(intf);

	if (driver->resume) {

		status = driver->resume(intf);

		if (status) {

			dev_err(dev, "%s error %d\n", "resume", status);

			mark_quiesced(intf);

		}

	} else

		dev_warn(dev, "no %s?\n", "resume");

	return 0;

}

drivers/usb/core/usb.h

+18 −0
Original line number Diff line number Diff line
@@ -28,6 +28,24 @@ extern void usb_major_cleanup(void); 
extern int usb_host_init(void);

extern void usb_host_cleanup(void);



/* Interfaces and their "power state" are owned by usbcore */



static inline void mark_active(struct usb_interface *f)

{

	f->dev.power.power_state.event = PM_EVENT_ON;

}



static inline void mark_quiesced(struct usb_interface *f)

{

	f->dev.power.power_state.event = PM_EVENT_FREEZE;

}



static inline int is_active(struct usb_interface *f)

{

	return f->dev.power.power_state.event == PM_EVENT_ON;

}





/* for labeling diagnostics */

extern const char *usbcore_name;

drivers/usb/input/hid-core.c

+0 −2
Original line number Diff line number Diff line
@@ -1887,7 +1887,6 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message) 
	struct hid_device *hid = usb_get_intfdata (intf);



	usb_kill_urb(hid->urbin);

	intf->dev.power.power_state = PMSG_SUSPEND;

	dev_dbg(&intf->dev, "suspend\n");

	return 0;

}
@@ -1897,7 +1896,6 @@ static int hid_resume(struct usb_interface *intf) 
	struct hid_device *hid = usb_get_intfdata (intf);

	int status;



	intf->dev.power.power_state = PMSG_ON;

	if (hid->open)

		status = usb_submit_urb(hid->urbin, GFP_NOIO);

	else