[PATCH] Fix dock station status detection

[Holger Macht]

On Do 13. Mär - 19:38:42, Holger Macht wrote:
> On Do 13. Mär - 19:25:14, Danny Kukawka wrote:
> > On Donnerstag, 13. März 2008, Holger Macht wrote:
> > > When the user triggers an undock operation, the kernel assumes that the
> > > undock cannot fail (which is pretty true) and sends out the 'UNDOCK' udev
> > > event before actually doing it. So when HAL comes down to
> > > platform_refresh() and reads the 'docked' sysfs file, it still contains
> > > 'docked' (at odd times, it's a race).
> > 
> > Sound to me like a kernel bug. Should the kernel get fixed to send the UNDOCK 
> > first if the sysfs attribute is changed?
> 
> Same considerations over here ;-) but...
> 
> AFAIK, it can't, because for sending the udev event, the device structure
> has to be still present.
> 
> The more clean solution would be to catch the UNDOCK_EVENT and to wait in
> a loop for the undock to fail or succeed (according to the flags). The
> only thing the dock driver signals is "now I'm starting the undock
> operation", which is correct. Userspace can check if it succeeded or not.
> 
> But I think the overhead would be too high in comparison to what it
> gains.

Well, cleanest way is to add a g_timeout_add in case the undock is still
in progress. I used a interval of 300 milliseconds. This was enough for
the callback to be called only once, even under heavy load on my X60. And
note: This timeout is only installed when you're actually doing a undock
operation, not on every platform_refresh. Please change if you prefer to
use g_timout_add_seconds().

Regards,
	Holger

---

diff --git a/hald/linux/device.c b/hald/linux/device.c
index c41edf3..c648c04 100644
--- a/hald/linux/device.c
+++ b/hald/linux/device.c
@@ -75,6 +75,7 @@ gboolean _have_sysfs_sleep_button = FALSE;
 gboolean _have_sysfs_power_supply = FALSE; 
 
 #define POWER_SUPPLY_BATTERY_POLL_INTERVAL 30  /* in seconds */
+#define DOCK_STATION_UNDOCK_POLL_INTERVAL 300  /* in milliseconds */
 
 /* we must use this kernel-compatible implementation */
 #define BITS_PER_LONG (sizeof(long) * 8)
@@ -2016,10 +2017,30 @@ platform_compute_udi (HalDevice *d)
 }
 
 static gboolean
+platform_refresh_undock (gpointer data)
+{
+	HalDevice *d = (HalDevice *) data;
+	gint flags, docked;
+	const gchar *sysfs_path;
+
+	sysfs_path = hal_device_property_get_string(d, "linux.sysfs_path");
+	hal_util_get_int_from_file (sysfs_path, "flags", &flags, 0);
+
+	/* check for != 0, maybe the user did an immediate dock */
+	if (flags != 0)
+		return TRUE;
+
+	hal_util_get_int_from_file (sysfs_path, "docked", &docked, 0);
+	hal_device_property_set_bool (d, "info.docked", docked);
+
+	return FALSE;
+}
+
+static gboolean
 platform_refresh (HalDevice *d)
 {
 	const gchar *id, *sysfs_path;
-	int docked;
+	gint docked, flags;
 
 	id = hal_device_property_get_string (d, "platform.id");
 	if (strncmp (id, "dock", 4) != 0)
@@ -2027,8 +2048,18 @@ platform_refresh (HalDevice *d)
 
 	sysfs_path = hal_device_property_get_string(d, "linux.sysfs_path");
 	hal_util_get_int_from_file (sysfs_path, "docked", &docked, 0);
-	hal_device_property_set_bool (d, "info.docked", docked);
 
+	if (docked == 1) {
+		/* undock still in progress? */
+		hal_util_get_int_from_file (sysfs_path, "flags", &flags, 0);
+		if (flags == 2) {
+			g_timeout_add (DOCK_STATION_UNDOCK_POLL_INTERVAL,
+				       platform_refresh_undock, d);
+			return TRUE;
+		}
+	}
+
+	hal_device_property_set_bool (d, "info.docked", docked);
 	return TRUE;
 }