[PATCH] Fix dock station status detection

Danny Kukawka danny.kukawka at web.de
Thu Mar 13 13:24:03 PDT 2008


On Donnerstag, 13. März 2008, Holger Macht wrote:
> 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().

Go ahead and commit with this change (to be paranoid):

>  static gboolean
> +platform_refresh_undock (gpointer data)
> +{
> +	HalDevice *d = (HalDevice *) data;
> +	gint flags, docked;
> +	const gchar *sysfs_path;

	HalDevice *d;

	if (data == NULL) {
		return FALSE;
	}

	d = (HalDevice *) data;


Danny


More information about the hal mailing list