2006-12-04 22:55:04

by Kristen Carlson Accardi

[permalink] [raw]
Subject: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

From: [email protected]

Add 2 sysfs files for user interface.
1) docked - 1/0 (read only) - indicates whether the software believes the
laptop is docked in a docking station.
2) undock - (write only) - writing to this file causes the software to
initiate an undock request to the firmware.

Signed-off-by: Brandon Philips <[email protected]>
Signed-off-by: Kristen Carlson Accardi <[email protected]>
---
drivers/acpi/dock.c | 95 +++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 79 insertions(+), 16 deletions(-)

--- kristen-2.6.orig/drivers/acpi/dock.c
+++ kristen-2.6/drivers/acpi/dock.c
@@ -514,6 +514,37 @@ void unregister_hotplug_dock_device(acpi
EXPORT_SYMBOL_GPL(unregister_hotplug_dock_device);

/**
+ * handle_eject_request - handle an undock request checking for error conditions
+ *
+ * Check to make sure the dock device is still present, then undock and
+ * hotremove all the devices that may need removing.
+ */
+static int handle_eject_request(struct dock_station *ds, u32 event)
+{
+ if (!dock_present(ds))
+ return -ENODEV;
+
+ if (dock_in_progress(ds))
+ return -EBUSY;
+
+ /*
+ * here we need to generate the undock
+ * event prior to actually doing the undock
+ * so that the device struct still exists.
+ */
+ dock_event(ds, event, UNDOCK_EVENT);
+ hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST);
+ undock(ds);
+ eject_dock(ds);
+ if (dock_present(ds)) {
+ printk(KERN_ERR PREFIX "Unable to undock!\n");
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+/**
* dock_notify - act upon an acpi dock notification
* @handle: the dock station handle
* @event: the acpi event
@@ -521,9 +552,7 @@ EXPORT_SYMBOL_GPL(unregister_hotplug_doc
*
* If we are notified to dock, then check to see if the dock is
* present and then dock. Notify all drivers of the dock event,
- * and then hotplug and devices that may need hotplugging. For undock
- * check to make sure the dock device is still present, then undock
- * and hotremove all the devices that may need removing.
+ * and then hotplug and devices that may need hotplugging.
*/
static void dock_notify(acpi_handle handle, u32 event, void *data)
{
@@ -555,19 +584,7 @@ static void dock_notify(acpi_handle hand
* to the driver who wish to hotplug.
*/
case ACPI_NOTIFY_EJECT_REQUEST:
- if (!dock_in_progress(ds) && dock_present(ds)) {
- /*
- * here we need to generate the undock
- * event prior to actually doing the undock
- * so that the device struct still exists.
- */
- dock_event(ds, event, UNDOCK_EVENT);
- hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST);
- undock(ds);
- eject_dock(ds);
- if (dock_present(ds))
- printk(KERN_ERR PREFIX "Unable to undock!\n");
- }
+ handle_eject_request(ds, event);
break;
default:
printk(KERN_ERR PREFIX "Unknown dock event %d\n", event);
@@ -606,6 +623,33 @@ find_dock_devices(acpi_handle handle, u3
return AE_OK;
}

+/*
+ * show_docked - read method for "docked" file in sysfs
+ */
+static ssize_t show_docked(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", dock_present(dock_station));
+
+}
+DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
+
+/*
+ * write_undock - write method for "undock" file in sysfs
+ */
+static ssize_t write_undock(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ if (!count)
+ return -EINVAL;
+
+ ret = handle_eject_request(dock_station, ACPI_NOTIFY_EJECT_REQUEST);
+ return ret ? ret: count;
+}
+DEVICE_ATTR(undock, S_IWUSR, NULL, write_undock);
+
/**
* dock_add - add a new dock station
* @handle: the dock station handle
@@ -639,6 +683,21 @@ static int dock_add(acpi_handle handle)
kfree(dock_station);
return ret;
}
+ ret = device_create_file(&dock_device.dev, &dev_attr_docked);
+ if (ret) {
+ printk("Error %d adding sysfs file\n", ret);
+ platform_device_unregister(&dock_device);
+ kfree(dock_station);
+ return ret;
+ }
+ ret = device_create_file(&dock_device.dev, &dev_attr_undock);
+ if (ret) {
+ printk("Error %d adding sysfs file\n", ret);
+ device_remove_file(&dock_device.dev, &dev_attr_docked);
+ platform_device_unregister(&dock_device);
+ kfree(dock_station);
+ return ret;
+ }

/* Find dependent devices */
acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
@@ -672,6 +731,8 @@ static int dock_add(acpi_handle handle)
dock_add_err:
kfree(dd);
dock_add_err_unregister:
+ device_remove_file(&dock_device.dev, &dev_attr_docked);
+ device_remove_file(&dock_device.dev, &dev_attr_undock);
platform_device_unregister(&dock_device);
kfree(dock_station);
return ret;
@@ -701,6 +762,8 @@ static int dock_remove(void)
printk(KERN_ERR "Error removing notify handler\n");

/* cleanup sysfs */
+ device_remove_file(&dock_device.dev, &dev_attr_docked);
+ device_remove_file(&dock_device.dev, &dev_attr_undock);
platform_device_unregister(&dock_device);

/* free dock station memory */

--


2006-12-09 12:00:26

by Holger Macht

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

On Mon 04. Dec - 14:49:58, Kristen Carlson Accardi wrote:
> From: [email protected]
>
> Add 2 sysfs files for user interface.
> 1) docked - 1/0 (read only) - indicates whether the software believes the
> laptop is docked in a docking station.
> 2) undock - (write only) - writing to this file causes the software to
> initiate an undock request to the firmware.

Thanks for doin this!

However, the function dock_event(...) still contains this comment:

/*
* we don't do events until someone tells me that
* they would like to have them.
*/

Well, I like to have them ;-)

The sysfs exports are more or less useless without events. When the dock
driver is loaded, you get a platform device below /sys. But you never know
if there will ever be a docking station connected to your
system. Userspace caring about this will have to poll the 'docked' file,
which is obviously bad.

Thanks,
Holger

2006-12-11 20:05:29

by Kristen Carlson Accardi

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

On Sat, 9 Dec 2006 12:59:58 +0100
Holger Macht <[email protected]> wrote:

> Well, I like to have them ;-)

Ok - how is this?

Send a uevent to indicate a device change whenever we dock or
undock, so that userspace may now check the dock status via
sysfs.

Signed-off-by: Kristen Carlson Accardi <[email protected]>
---
drivers/acpi/dock.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

--- kristen-2.6.orig/drivers/acpi/dock.c
+++ kristen-2.6/drivers/acpi/dock.c
@@ -326,10 +326,12 @@ static void hotplug_dock_devices(struct

static void dock_event(struct dock_station *ds, u32 event, int num)
{
+ struct device *dev = &dock_device.dev;
/*
- * we don't do events until someone tells me that
- * they would like to have them.
+ * Indicate that the status of the dock station has
+ * changed.
*/
+ kobject_uevent(&dev->kobj, KOBJ_CHANGE);
}

/**

2006-12-12 22:13:35

by Stefan Schmidt

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

Hello.

On Mon, 2006-12-11 at 12:05, Kristen Carlson Accardi wrote:
> On Sat, 9 Dec 2006 12:59:58 +0100
> Holger Macht <[email protected]> wrote:
>
> > Well, I like to have them ;-)
>
> Ok - how is this?
>
> Send a uevent to indicate a device change whenever we dock or
> undock, so that userspace may now check the dock status via
> sysfs.

I would like to have two different events for dock and undock.

This way the userspace listener don't need to check the status file in
sysfs to know if there was a dock or undock after getting the event.

Anyway the status file is still usefull for programs don't react on
the events, but like to know if the laptop is docked before starting
for example.

regards
Stefan Schmidt


Attachments:
(No filename) (722.00 B)
signature.asc (241.00 B)
Digital signature
Download all attachments

2006-12-12 22:31:45

by Jesse Barnes

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

On Tuesday, December 12, 2006 2:15 pm, Stefan Schmidt wrote:
> Hello.
>
> On Mon, 2006-12-11 at 12:05, Kristen Carlson Accardi wrote:
> > On Sat, 9 Dec 2006 12:59:58 +0100
> >
> > Holger Macht <[email protected]> wrote:
> > > Well, I like to have them ;-)
> >
> > Ok - how is this?
> >
> > Send a uevent to indicate a device change whenever we dock or
> > undock, so that userspace may now check the dock status via
> > sysfs.
>
> I would like to have two different events for dock and undock.
>
> This way the userspace listener don't need to check the status file
> in sysfs to know if there was a dock or undock after getting the
> event.
>
> Anyway the status file is still usefull for programs don't react on
> the events, but like to know if the laptop is docked before starting
> for example.

FWIW, Kay and Neil recently went back and forth regarding what sorts of
events to generate for MD online/offline events. In concept md
online/offline and dock/undock seem similar enough that the 'change'
events Kay requested for md probably make sense in the dock/undock
context as well, but I've Cc'd him just in case.

Jesse

2006-12-12 23:05:54

by Kristen Carlson Accardi

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

On Tue, 12 Dec 2006 14:31:10 -0800
Jesse Barnes <[email protected]> wrote:

> On Tuesday, December 12, 2006 2:15 pm, Stefan Schmidt wrote:
> > Hello.
> >
> > On Mon, 2006-12-11 at 12:05, Kristen Carlson Accardi wrote:
> > > On Sat, 9 Dec 2006 12:59:58 +0100
> > >
> > > Holger Macht <[email protected]> wrote:
> > > > Well, I like to have them ;-)
> > >
> > > Ok - how is this?
> > >
> > > Send a uevent to indicate a device change whenever we dock or
> > > undock, so that userspace may now check the dock status via
> > > sysfs.
> >
> > I would like to have two different events for dock and undock.
> >
> > This way the userspace listener don't need to check the status file
> > in sysfs to know if there was a dock or undock after getting the
> > event.
> >
> > Anyway the status file is still usefull for programs don't react on
> > the events, but like to know if the laptop is docked before starting
> > for example.
>
> FWIW, Kay and Neil recently went back and forth regarding what sorts of
> events to generate for MD online/offline events. In concept md
> online/offline and dock/undock seem similar enough that the 'change'
> events Kay requested for md probably make sense in the dock/undock
> context as well, but I've Cc'd him just in case.
>
> Jesse
>

I did have different dock/undock events a few months ago - but
after some discussion we scrapped them because Kay wants to avoid driver
specific events. The "change" event is the only thing that makes sense,
given the set of uevents available right now, and userspace should be
able to handle checking a file to get driver specific details (i.e. dock
and undock status). If you have a specific reason why this won't work,
let me know.

2006-12-13 00:03:12

by Stefan Schmidt

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

Hello.

On Tue, 2006-12-12 at 15:00, Kristen Carlson Accardi wrote:
>
> I did have different dock/undock events a few months ago - but
> after some discussion we scrapped them because Kay wants to avoid driver
> specific events. The "change" event is the only thing that makes sense,
> given the set of uevents available right now, and userspace should be
> able to handle checking a file to get driver specific details (i.e. dock
> and undock status). If you have a specific reason why this won't work,
> let me know.

It's fine with me. I just find two different events more handy.
Checking the file after the event in userspace should not be aproblem.

regards
Stefan Schmidt


Attachments:
(No filename) (684.00 B)
signature.asc (241.00 B)
Digital signature
Download all attachments

2006-12-13 09:19:51

by Kay Sievers

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

On Wed, 2006-12-13 at 00:26 +0100, Stefan Schmidt wrote:
> On Tue, 2006-12-12 at 15:00, Kristen Carlson Accardi wrote:
> >
> > I did have different dock/undock events a few months ago - but
> > after some discussion we scrapped them because Kay wants to avoid driver
> > specific events. The "change" event is the only thing that makes sense,
> > given the set of uevents available right now, and userspace should be
> > able to handle checking a file to get driver specific details (i.e. dock
> > and undock status). If you have a specific reason why this won't work,
> > let me know.
>
> It's fine with me. I just find two different events more handy.
> Checking the file after the event in userspace should not be aproblem.

The thing is that we try to avoid driver-core "features" that are
specific to a single subsystem or driver.

You can easily add additional environment variables today, while sending
a "change"-event with kobject_uevent_env(), like
ACPI_DOCK={lock,unlock,insert,remove,...}. Just pass any driver-specific
string you like along with the event, and it will be available just like
the "action" string.

This should fit all requirements, without the need to introduce all
sorts of new generic action-strings, that can almost never be changed
later for compatibility reasons. That way, if "drivers" later find out,
that they need to send different actions/flags, they can just add as
many new strings as they like on top of the event. :)

Thanks,
Kay

2006-12-13 09:55:35

by Stefan Schmidt

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

Hello.

On Wed, 2006-12-13 at 10:14, Kay Sievers wrote:
> On Wed, 2006-12-13 at 00:26 +0100, Stefan Schmidt wrote:
> > On Tue, 2006-12-12 at 15:00, Kristen Carlson Accardi wrote:
> > >
> > > I did have different dock/undock events a few months ago - but
> > > after some discussion we scrapped them because Kay wants to avoid driver
> > > specific events. The "change" event is the only thing that makes sense,
> > > given the set of uevents available right now, and userspace should be
> > > able to handle checking a file to get driver specific details (i.e. dock
> > > and undock status). If you have a specific reason why this won't work,
> > > let me know.
> >
> > It's fine with me. I just find two different events more handy.
> > Checking the file after the event in userspace should not be aproblem.
>
> The thing is that we try to avoid driver-core "features" that are
> specific to a single subsystem or driver.
>
> You can easily add additional environment variables today, while sending
> a "change"-event with kobject_uevent_env(), like
> ACPI_DOCK={lock,unlock,insert,remove,...}. Just pass any driver-specific
> string you like along with the event, and it will be available just like
> the "action" string.

Thanks for the explanation. I can live with both solutions. It's up to
Kristen.

> This should fit all requirements, without the need to introduce all
> sorts of new generic action-strings, that can almost never be changed
> later for compatibility reasons. That way, if "drivers" later find out,
> that they need to send different actions/flags, they can just add as
> many new strings as they like on top of the event. :)

Fair enough.

regards
Stefan Schmidt


Attachments:
(No filename) (1.66 kB)
signature.asc (241.00 B)
Digital signature
Download all attachments

2006-12-14 07:30:09

by Holger Macht

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

On Mon 11. Dec - 12:05:08, Kristen Carlson Accardi wrote:
> On Sat, 9 Dec 2006 12:59:58 +0100
> Holger Macht <[email protected]> wrote:
>
> > Well, I like to have them ;-)
>
> Ok - how is this?

Looks good to me, thanks!

> Send a uevent to indicate a device change whenever we dock or
> undock, so that userspace may now check the dock status via
> sysfs.
>
> Signed-off-by: Kristen Carlson Accardi <[email protected]>

Signed-off-by: Holger Macht <[email protected]>

Regards,
Holger

2006-12-14 23:05:10

by Len Brown

[permalink] [raw]
Subject: Re: [patch 2/3] acpi: Add a docked sysfs file to the dock driver.

On Thursday 14 December 2006 02:16, Holger Macht wrote:
> On Mon 11. Dec - 12:05:08, Kristen Carlson Accardi wrote:

> > Ok - how is this?
>
> Looks good to me, thanks!

> > Signed-off-by: Kristen Carlson Accardi <[email protected]>
>
> Signed-off-by: Holger Macht <[email protected]>

Applied.
thanks,
-Len

commit 8ea86e0ba7c9d16ae0f35cb0c4165194fa573f7a
Author: Kristen Carlson Accardi <[email protected]>
Date: Mon Dec 11 12:05:08 2006 -0800

ACPI: dock: add uevent to indicate change in device status

Send a uevent to indicate a device change whenever we dock or
undock, so that userspace may now check the dock status via sysfs.

Signed-off-by: Kristen Carlson Accardi <[email protected]>
Signed-off-by: Holger Macht <[email protected]>
Signed-off-by: Len Brown <[email protected]>

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 8c6828b..215f5b3 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -326,10 +326,12 @@ static void hotplug_dock_devices(struct dock_station
*ds, u32 event)

static void dock_event(struct dock_station *ds, u32 event, int num)
{
+ struct device *dev = &dock_device.dev;
/*
- * we don't do events until someone tells me that
- * they would like to have them.
+ * Indicate that the status of the dock station has
+ * changed.
*/
+ kobject_uevent(&dev->kobj, KOBJ_CHANGE);
}

/**