[marcelo at kvack.org: [PATCH][RFC] switch ehci-hcd to new polling scheme]

[Marcelo Tosatti]

----- Forwarded message from Marcelo Tosatti <marcelo at kvack.org> -----

From: Marcelo Tosatti <marcelo at kvack.org>
Date: Thu, 28 Dec 2006 15:33:42 -0200
To: linux-usb-devel at lists.sourceforge.net, Greg KH <greg at kroah.com>
Cc: olpc-devel at laptop.org
Subject: [PATCH][RFC] switch ehci-hcd to new polling scheme

Hi,

The root hub status timer is problematic for tickless systems.

-bash-3.1# echo 1 > timer_stats ; sleep 5s ; cat timer_stats
Timer Stats Version: v0.1
Sample period: 5.014 s
   2,     0 swapper          hrtimer_restart_sched_tick (hrtimer_sched_tick)
  25,     1 swapper          fbcon_add_cursor_timer (cursor_timer_handler)
  84,     0 swapper          hrtimer_stop_sched_tick (hrtimer_sched_tick)
  24,   935 dhcdbd           schedule_timeout (process_timeout)
  19,     1 swapper          usb_hcd_submit_urb (rh_timer_func)
   5,     1 swapper          init_tsc_clocksource (verify_tsc_freq)
   1,     1 init             schedule_timeout (process_timeout)
   2,     1 swapper          schedule_delayed_work_on (delayed_work_timer_fn)
   1,     1 swapper          neigh_table_init_no_netlink (neigh_periodic_timer)
   1,     0 swapper          page_writeback_init (wb_timer_fn)
   1,  1128 sleep            do_nanosleep (hrtimer_wakeup)
165 total events, 33.32 events/sec

As can be seen by the timer_stats code, 19 timeouts were triggered in a
5s period (the rearming frequency for rh_timer is 250ms, or 4 timeouts
per second).

Greg added code to make the interrupt handler of the host controller
inform status change events, making polling unnecessary during normal
operation (new_polling flag).

OHCI and UHCI have been converted to the new scheme, but EHCI hasnt.

>From what I understand from the EHCI spec, the STS_PCD flag (status
change) of USBINTR register covers the following events:

4.15.2.1 Port Change Events

Port registers contain status and status change bits. When the status
change bits are set to a one, the host controller sets the Port Change
Detect bit in the USBSTS register to a one. If the Port Change Interrupt
Enable bit in the USBINTR register is a one, then the host controller
will issue a hardware interrupt. The port status change bits include:

• Connect Status Change
• Port Enable/Disable Change
• Over-current Change
• Force Port Resume

Which includes all events reported by ehci_hub_status_data.

So I've hacked up a patch to switch ehci-hcd to new_polling.
Unfortunately ehci_irq holds ehci->lock, so calling
usb_hcd_poll_rh_status() directly from IRQ context is not possible.

Due to that, I've moved the call to a workqueue instance.

Insertion/removal of cards works as expected, but I'm afraid that there
are corner cases which this simplistic code is not handling?

Is it necessary to poll status during certain situations?

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 025d333..2e98f67 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -405,6 +405,13 @@ #endif
 	dbg_status (ehci, "ehci_stop completed", readl (&ehci->regs->status));
 }
 
+static void run_rh_status_poll(struct work_struct *work)
+{
+	struct ehci_hcd	*ehci;
+	ehci = container_of(work, struct ehci_hcd, rh_status_worker);
+	usb_hcd_poll_rh_status(ehci_to_hcd(ehci));
+}
+
 /* one-time init, only for memory state */
 static int ehci_init(struct usb_hcd *hcd)
 {
@@ -419,6 +426,8 @@ static int ehci_init(struct usb_hcd *hcd
 	ehci->watchdog.function = ehci_watchdog;
 	ehci->watchdog.data = (unsigned long) ehci;
 
+	INIT_WORK(&ehci->rh_status_worker, run_rh_status_poll);
+
 	/*
 	 * hw default: 1K periodic list heads, one per frame.
 	 * periodic_size can shrink by USBCMD update if hcc_params allows.
@@ -496,6 +505,8 @@ static int ehci_run (struct usb_hcd *hcd
 	u32			temp;
 	u32			hcc_params;
 
+	hcd->uses_new_polling = 1;
+
 	/* EHCI spec section 4.1 */
 	if ((retval = ehci_reset(ehci)) != 0) {
 		ehci_mem_cleanup(ehci);
@@ -622,6 +633,9 @@ #endif
 		if (!(readl(&ehci->regs->command) & CMD_RUN))
 			usb_hcd_resume_root_hub(hcd);
 
+		/* Report port status change later on workqueue ctx */
+		schedule_work(&ehci->rh_status_worker);
+
 		while (i--) {
 			int pstatus = readl (&ehci->regs->port_status [i]);
 
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index bfe5f30..5bf0786 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -291,6 +291,7 @@ ehci_hub_status_data (struct usb_hcd *hc
 			status = STS_PCD;
 		}
 	}
+	hcd->poll_rh = 0;
 	/* FIXME autosuspend idle root hubs */
 	spin_unlock_irqrestore (&ehci->lock, flags);
 	return status ? retval : 0;
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 74dbc6c..1ae114c 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -87,6 +87,7 @@ #define	DEFAULT_I_TDPS		1024		/* some HC
 	unsigned		stamp;
 	unsigned long		next_statechange;
 	u32			command;
+	struct work_struct	rh_status_worker;
 
 	/* SILICON QUIRKS */
 	unsigned		is_tdi_rh_tt:1;	/* TDI roothub with TT */

----- End forwarded message -----