Rework of request firmware

Rework of request firmware

[Jon Smirl]

[-- Attachment #1: Type: text/plain, Size: 29341 bytes --]

On Sat, 26 Feb 2005 20:41:16 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> I think the request_firmware() should be redesigned in a generic
> request_user_data(kobj) call. This event would copy arbitrary data into
> a sysfs file and something like KOBJ_REQUEST_DATA would do it.
> 
> The call should create a file inside of a existing device and the event
> handler should copy the data into this file instead of creating a own
> class device for the firmware as we do today.

This is a rework of request firmware to move the attributes from their
own class into the device sysfs directory. I am also generalizing
things so that I can request a post (this is what I need) as well as
firmware. This is a first post of the code, please tell me what I can
do to improve it before I send it to lkml. The changes in radeonfb are
so that I can test it. There are also some debug printf's still in.
Use the attachment with patch, gmail word wraps and I can't stop it.

I made a few changes in the base code:
1) I modified kobj_hotplug() to take an extra function for adding
variables. This lets the event add the normal variable for the kset
and then I can tack mine on too.
2) New event KOBJ_POST. $POST and $FIRMWARE are used to differentiate
post versus firmware load in the script. Should this be two events
instead?
3) The script now needs to be in
/etc/hotplug.d/default/30-post.hotplug. firmware.agent isn't used
anymore. The firmware doesn't need to change or move.
4) I added a pointer to 'struct device' to track the firmware in
sysfs. Is there a better way to do this?
5) I added the time out to /sys/firmware/post_timeout since there is
no /sys/class/firmware any more.
6) How should everything be named? firmware, post, initialization, etc??
7) Should request_firmware() be deprecated forcing the move to
request_firmware_nowait?
8) Should I keep the different signatures on the nowait callbacks?

-- 
Jon Smirl
jonsmirl@gmail.com

===== drivers/acpi/container.c 1.2 vs edited =====
--- 1.2/drivers/acpi/container.c	2004-11-11 02:56:31 -05:00
+++ edited/drivers/acpi/container.c	2005-03-19 01:38:11 -05:00
@@ -182,16 +182,16 @@
 			if (ACPI_FAILURE(status) || !device) {
 				result = container_device_add(&device, handle);
 				if (!result)
-					kobject_hotplug(&device->kobj, KOBJ_ONLINE);
+					kobject_hotplug(&device->kobj, KOBJ_ONLINE, NULL);
 			} else {
 				/* device exist and this is a remove request */
-				kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+				kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 			}
 		}
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		if (!acpi_bus_get_device(handle, &device) && device) {
-			kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 		}
 		break;
 	default:
===== drivers/acpi/processor_core.c 1.80 vs edited =====
--- 1.80/drivers/acpi/processor_core.c	2004-12-14 07:14:54 -05:00
+++ edited/drivers/acpi/processor_core.c	2005-03-19 01:38:57 -05:00
@@ -730,7 +730,7 @@
 		return_VALUE(-ENODEV);
 
 	if ((pr->id >=0) && (pr->id < NR_CPUS)) {
-		kobject_hotplug(&(*device)->kobj, KOBJ_ONLINE);
+		kobject_hotplug(&(*device)->kobj, KOBJ_ONLINE, NULL);
 	}
 	return_VALUE(0);
 }
@@ -774,13 +774,13 @@
 		}
 
 		if (pr->id >= 0 && (pr->id < NR_CPUS)) {
-			kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 			break;
 		}
 
 		result = acpi_processor_start(device);
 		if ((!result) && ((pr->id >=0) && (pr->id < NR_CPUS))) {
-			kobject_hotplug(&device->kobj, KOBJ_ONLINE);
+			kobject_hotplug(&device->kobj, KOBJ_ONLINE, NULL);
 		} else {
 			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
 				"Device [%s] failed to start\n",
@@ -801,7 +801,7 @@
 		}
 
 		if ((pr->id < NR_CPUS) && (cpu_present(pr->id)))
-			kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
===== drivers/base/cpu.c 1.22 vs edited =====
--- 1.22/drivers/base/cpu.c	2005-01-31 01:33:46 -05:00
+++ edited/drivers/base/cpu.c	2005-03-19 01:39:16 -05:00
@@ -33,7 +33,7 @@
 	case '0':
 		ret = cpu_down(cpu->sysdev.id);
 		if (!ret)
-			kobject_hotplug(&dev->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&dev->kobj, KOBJ_OFFLINE, NULL);
 		break;
 	case '1':
 		ret = cpu_up(cpu->sysdev.id);
===== drivers/base/firmware.c 1.9 vs edited =====
--- 1.9/drivers/base/firmware.c	2004-09-24 14:45:35 -04:00
+++ edited/drivers/base/firmware.c	2005-03-19 11:16:01 -05:00
@@ -14,21 +14,57 @@
 
 static decl_subsys(firmware, NULL, NULL);
 
+int post_timeout = 10; /* In seconds */
+EXPORT_SYMBOL_GPL(post_timeout);
+
 int firmware_register(struct subsystem * s)
 {
 	kset_set_kset_s(s, firmware_subsys);
 	return subsystem_register(s);
 }
+EXPORT_SYMBOL_GPL(firmware_register);
 
 void firmware_unregister(struct subsystem * s)
 {
 	subsystem_unregister(s);
 }
+EXPORT_SYMBOL_GPL(firmware_unregister);
+
+/**
+ * post_timeout_store:
+ * Description:
+ *	Sets the number of seconds to wait for the post/firmware load.
+ *	Once this expires an error will be return to the driver and no
+ *	firmware will be provided.
+ *
+ *	Note: zero means 'wait for ever'
+ *
+ **/
+static ssize_t post_timeout_store(struct subsystem *subsys, const
char *buf, size_t count)
+{
+	post_timeout = simple_strtol(buf, NULL, 10);
+	return count;
+}
+static ssize_t post_timeout_show(struct subsystem *subsys, char *buf)
+{
+	return sprintf(buf, "%d\n", post_timeout);
+}
+static struct subsys_attribute post_timeout_attr = {
+	.attr = {.name = "post_timeout", .mode = 0644, .owner = THIS_MODULE},
+	.show = post_timeout_show,
+	.store = post_timeout_store
+};
 
 int __init firmware_init(void)
 {
-	return subsystem_register(&firmware_subsys);
+	int error;
+	if ((error = subsystem_register(&firmware_subsys)))
+		return error;
+	
+	if ((error = subsys_create_file(&firmware_subsys, &post_timeout_attr))) {
+		subsystem_unregister(&firmware_subsys);
+		return error;
+	}
+	return 0;
 }
 
-EXPORT_SYMBOL_GPL(firmware_register);
-EXPORT_SYMBOL_GPL(firmware_unregister);
===== drivers/base/firmware_class.c 1.25 vs edited =====
--- 1.25/drivers/base/firmware_class.c	2004-11-26 15:26:48 -05:00
+++ edited/drivers/base/firmware_class.c	2005-03-19 21:11:15 -05:00
@@ -1,7 +1,8 @@
 /*
- * firmware_class.c - Multi purpose firmware loading support
+ * firmware_class.c - Multi purpose post support
  *
  * Copyright (c) 2003 Manuel Estrada Sainz <ranty@debian.org>
+ * Copyright (c) 2005 Jon Smirl <jonsmirl@gmail.com>
  *
  * Please see Documentation/firmware_class/ for more information.
  *
@@ -20,7 +21,7 @@
 #include "base.h"
 
 MODULE_AUTHOR("Manuel Estrada Sainz <ranty@debian.org>");
-MODULE_DESCRIPTION("Multi purpose firmware loading support");
+MODULE_DESCRIPTION("Multi purpose post support");
 MODULE_LICENSE("GPL");
 
 enum {
@@ -30,8 +31,6 @@
 	FW_STATUS_READY,
 };
 
-static int loading_timeout = 10;	/* In seconds */
-
 /* fw_lock could be moved to 'struct firmware_priv' but since it is just
  * guarding for corner cases a global lock should be OK */
 static DECLARE_MUTEX(fw_lock);
@@ -49,69 +48,41 @@
 static inline void
 fw_load_abort(struct firmware_priv *fw_priv)
 {
+	printk(KERN_ERR "fw_load_abort\n");
 	set_bit(FW_STATUS_ABORT, &fw_priv->status);
 	wmb();
 	complete(&fw_priv->completion);
 }
 
-static ssize_t
-firmware_timeout_show(struct class *class, char *buf)
-{
-	return sprintf(buf, "%d\n", loading_timeout);
-}
-
-/**
- * firmware_timeout_store:
- * Description:
- *	Sets the number of seconds to wait for the firmware.  Once
- *	this expires an error will be return to the driver and no
- *	firmware will be provided.
- *
- *	Note: zero means 'wait for ever'
- *
- **/
-static ssize_t
-firmware_timeout_store(struct class *class, const char *buf, size_t count)
-{
-	loading_timeout = simple_strtol(buf, NULL, 10);
-	return count;
-}
-
-static CLASS_ATTR(timeout, 0644, firmware_timeout_show,
firmware_timeout_store);
-
-static void  fw_class_dev_release(struct class_device *class_dev);
-int firmware_class_hotplug(struct class_device *dev, char **envp,
-			   int num_envp, char *buffer, int buffer_size);
-
-static struct class firmware_class = {
-	.name		= "firmware",
-	.hotplug	= firmware_class_hotplug,
-	.release	= fw_class_dev_release,
-};
-
 int
-firmware_class_hotplug(struct class_device *class_dev, char **envp,
+post_hotplug(struct kset *kset, struct kobject *kobj, char **envp,
 		       int num_envp, char *buffer, int buffer_size)
 {
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct firmware_priv *fw_priv = dev->post_data;
 	int i = 0, len = 0;
 
 	if (!test_bit(FW_STATUS_READY, &fw_priv->status))
 		return -ENODEV;
 
-	if (add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size, &len,
-			"FIRMWARE=%s", fw_priv->fw_id))
-		return -ENOMEM;
-
+	if (fw_priv->fw) {
+		if (add_hotplug_env_var(envp, num_envp, &i, buffer,
+		    buffer_size, &len, "FIRMWARE=%s", fw_priv->fw_id))
+			return -ENOMEM;
+	} else {
+		if (add_hotplug_env_var(envp, num_envp, &i, buffer,
+		    buffer_size, &len, "POST=%s", fw_priv->fw_id))
+			return -ENOMEM;
+	}
 	envp[i] = NULL;
 
 	return 0;
 }
 
 static ssize_t
-firmware_loading_show(struct class_device *class_dev, char *buf)
+post_status_show(struct device *dev, char *buf)
 {
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
+	struct firmware_priv *fw_priv = dev->post_data;
 	int loading = test_bit(FW_STATUS_LOADING, &fw_priv->status);
 	return sprintf(buf, "%d\n", loading);
 }
@@ -126,13 +97,13 @@
  *	-1: Conclude the load with an error and discard any written data.
  **/
 static ssize_t
-firmware_loading_store(struct class_device *class_dev,
-		       const char *buf, size_t count)
+post_status_store(struct device *dev, const char *buf, size_t count)
 {
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
-	int loading = simple_strtol(buf, NULL, 10);
+	struct firmware_priv *fw_priv = dev->post_data;
+	int status = simple_strtol(buf, NULL, 10);
 
-	switch (loading) {
+	printk(KERN_ERR "post_status_store: status %d\n", status);
+	switch (status) {
 	case 1:
 		down(&fw_lock);
 		vfree(fw_priv->fw->data);
@@ -151,7 +122,7 @@
 		/* fallthrough */
 	default:
 		printk(KERN_ERR "%s: unexpected value (%d)\n", __FUNCTION__,
-		       loading);
+		       status);
 		/* fallthrough */
 	case -1:
 		fw_load_abort(fw_priv);
@@ -161,15 +132,14 @@
 	return count;
 }
 
-static CLASS_DEVICE_ATTR(loading, 0644,
-			firmware_loading_show, firmware_loading_store);
+static DEVICE_ATTR(post_status, 0644, post_status_show, post_status_store);
 
 static ssize_t
-firmware_data_read(struct kobject *kobj,
-		   char *buffer, loff_t offset, size_t count)
+post_data_read(struct kobject *kobj, 
+	       char *buffer, loff_t offset, size_t count)
 {
-	struct class_device *class_dev = to_class_dev(kobj);
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct firmware_priv *fw_priv = dev->post_data;
 	struct firmware *fw;
 	ssize_t ret_count = count;
 
@@ -191,6 +161,7 @@
 	up(&fw_lock);
 	return ret_count;
 }
+
 static int
 fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size)
 {
@@ -225,8 +196,8 @@
  *	the driver as a firmware image.
  **/
 static ssize_t
-firmware_data_write(struct kobject *kobj,
-		    char *buffer, loff_t offset, size_t count)
+post_data_write(struct kobject *kobj, 
+		char *buffer, loff_t offset, size_t count)
 {
 	struct class_device *class_dev = to_class_dev(kobj);
 	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
@@ -254,124 +225,82 @@
 	return retval;
 }
 static struct bin_attribute firmware_attr_data_tmpl = {
-	.attr = {.name = "data", .mode = 0644, .owner = THIS_MODULE},
+	.attr = {.name = "post_data", .mode = 0644, .owner = THIS_MODULE},
 	.size = 0,
-	.read = firmware_data_read,
-	.write = firmware_data_write,
+	.read = post_data_read,
+	.write = post_data_write,
 };
 
 static void
-fw_class_dev_release(struct class_device *class_dev)
-{
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
-
-	kfree(fw_priv);
-	kfree(class_dev);
-
-	module_put(THIS_MODULE);
-}
-
-static void
-firmware_class_timeout(u_long data)
+post_timeout_abort(u_long data)
 {
 	struct firmware_priv *fw_priv = (struct firmware_priv *) data;
+	printk(KERN_ERR "post_timeout_abort: time out\n");
 	fw_load_abort(fw_priv);
 }
 
-static inline void
-fw_setup_class_device_id(struct class_device *class_dev, struct device *dev)
-{
-	/* XXX warning we should watch out for name collisions */
-	strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
-}
-
 static int
-fw_register_class_device(struct class_device **class_dev_p,
-			 const char *fw_name, struct device *device)
+fw_register_device(const char *fw_name, struct device *device)
 {
 	int retval;
 	struct firmware_priv *fw_priv = kmalloc(sizeof (struct firmware_priv),
 						GFP_KERNEL);
-	struct class_device *class_dev = kmalloc(sizeof (struct class_device),
-						 GFP_KERNEL);
-
-	*class_dev_p = NULL;
 
-	if (!fw_priv || !class_dev) {
+	if (!fw_priv) {
 		printk(KERN_ERR "%s: kmalloc failed\n", __FUNCTION__);
 		retval = -ENOMEM;
 		goto error_kfree;
 	}
 	memset(fw_priv, 0, sizeof (*fw_priv));
-	memset(class_dev, 0, sizeof (*class_dev));
 
 	init_completion(&fw_priv->completion);
 	fw_priv->attr_data = firmware_attr_data_tmpl;
 	strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX);
 
-	fw_priv->timeout.function = firmware_class_timeout;
+	fw_priv->timeout.function = post_timeout_abort;
 	fw_priv->timeout.data = (u_long) fw_priv;
 	init_timer(&fw_priv->timeout);
+	
+	device->post_data = fw_priv;
 
-	fw_setup_class_device_id(class_dev, device);
-	class_dev->dev = device;
-	class_dev->class = &firmware_class;
-	class_set_devdata(class_dev, fw_priv);
-	retval = class_device_register(class_dev);
-	if (retval) {
-		printk(KERN_ERR "%s: class_device_register failed\n",
-		       __FUNCTION__);
-		goto error_kfree;
-	}
-	*class_dev_p = class_dev;
 	return 0;
 
 error_kfree:
 	kfree(fw_priv);
-	kfree(class_dev);
 	return retval;
 }
 
 static int
-fw_setup_class_device(struct firmware *fw, struct class_device **class_dev_p,
-		      const char *fw_name, struct device *device)
+fw_setup_device(struct firmware *fw, const char *fw_name, struct
device *device)
 {
-	struct class_device *class_dev;
 	struct firmware_priv *fw_priv;
 	int retval;
 
-	*class_dev_p = NULL;
-	retval = fw_register_class_device(&class_dev, fw_name, device);
+	retval = fw_register_device(fw_name, device);
 	if (retval)
 		goto out;
 
-	/* Need to pin this module until class device is destroyed */
-	__module_get(THIS_MODULE);
-
-	fw_priv = class_get_devdata(class_dev);
-
+	fw_priv = device->post_data;
 	fw_priv->fw = fw;
-	retval = sysfs_create_bin_file(&class_dev->kobj, &fw_priv->attr_data);
+	
+	retval = sysfs_create_bin_file(&device->kobj, &fw_priv->attr_data);
 	if (retval) {
 		printk(KERN_ERR "%s: sysfs_create_bin_file failed\n",
 		       __FUNCTION__);
-		goto error_unreg;
+		goto out;
 	}
 
-	retval = class_device_create_file(class_dev,
-					  &class_device_attr_loading);
-	if (retval) {
-		printk(KERN_ERR "%s: class_device_create_file failed\n",
-		       __FUNCTION__);
-		goto error_unreg;
+	if (fw) {
+		retval = device_create_file(device, &dev_attr_post_status);
+		if (retval) {
+			printk(KERN_ERR "%s: device_create_file failed\n",
+			       __FUNCTION__);
+			goto out;
+		}
 	}
-
 	set_bit(FW_STATUS_READY, &fw_priv->status);
-	*class_dev_p = class_dev;
 	goto out;
 
-error_unreg:
-	class_device_unregister(class_dev);
 out:
 	return retval;
 }
@@ -388,62 +317,79 @@
  *	should be distinctive enough not to be confused with any other
  *	firmware image for this or any other device.
  **/
-int
-request_firmware(const struct firmware **firmware_p, const char *name,
+static int
+request(const struct firmware **firmware_p, const char *name,
 		 struct device *device)
 {
-	struct class_device *class_dev;
 	struct firmware_priv *fw_priv;
-	struct firmware *firmware;
+	struct firmware *firmware = NULL;
 	int retval;
 
-	if (!firmware_p)
-		return -EINVAL;
-
-	*firmware_p = firmware = kmalloc(sizeof (struct firmware), GFP_KERNEL);
-	if (!firmware) {
-		printk(KERN_ERR "%s: kmalloc(struct firmware) failed\n",
-		       __FUNCTION__);
-		retval = -ENOMEM;
-		goto out;
+	if (firmware_p) {
+		*firmware_p = firmware = kmalloc(sizeof (struct firmware), GFP_KERNEL);
+		if (!firmware) {
+			printk(KERN_ERR "%s: kmalloc(struct firmware) failed\n",
+			       __FUNCTION__);
+			retval = -ENOMEM;
+			goto out;
+		}
+		memset(firmware, 0, sizeof (*firmware));
 	}
-	memset(firmware, 0, sizeof (*firmware));
 
-	retval = fw_setup_class_device(firmware, &class_dev, name, device);
+	retval = fw_setup_device(firmware, name, device);
 	if (retval)
 		goto error_kfree_fw;
 
-	fw_priv = class_get_devdata(class_dev);
+	fw_priv = device->post_data;
 
-	if (loading_timeout) {
-		fw_priv->timeout.expires = jiffies + loading_timeout * HZ;
+	if (post_timeout) {
+		fw_priv->timeout.expires = jiffies + post_timeout * HZ;
 		add_timer(&fw_priv->timeout);
 	}
 
-	kobject_hotplug(&class_dev->kobj, KOBJ_ADD);
+	printk(KERN_ERR "request_firmware: hotplug event\n");
+	kobject_hotplug(&device->kobj, KOBJ_POST, post_hotplug);
 	wait_for_completion(&fw_priv->completion);
 	set_bit(FW_STATUS_DONE, &fw_priv->status);
+	printk(KERN_ERR "request_firmware: complete\n");
 
 	del_timer_sync(&fw_priv->timeout);
 
-	down(&fw_lock);
-	if (!fw_priv->fw->size || test_bit(FW_STATUS_ABORT, &fw_priv->status)) {
-		retval = -ENOENT;
-		release_firmware(fw_priv->fw);
-		*firmware_p = NULL;
+	if (firmware_p) {
+		down(&fw_lock);
+		if (!fw_priv->fw->size || test_bit(FW_STATUS_ABORT, &fw_priv->status)) {
+			retval = -ENOENT;
+			release_firmware(fw_priv->fw);
+			*firmware_p = NULL;
+		}
+		fw_priv->fw = NULL;
+		up(&fw_lock);
+	
+		sysfs_remove_bin_file(&device->kobj, &fw_priv->attr_data);
 	}
-	fw_priv->fw = NULL;
-	up(&fw_lock);
-	class_device_unregister(class_dev);
+	device_remove_file(device, &dev_attr_post_status);
+	
 	goto out;
 
 error_kfree_fw:
-	kfree(firmware);
-	*firmware_p = NULL;
+	if (firmware) {
+		kfree(firmware);
+		*firmware_p = NULL;
+	}
 out:
 	return retval;
 }
 
+int
+request_firmware(const struct firmware **firmware_p, const char *name,
+		 struct device *device)
+{
+	if (!firmware_p)
+		return -EINVAL;
+	return request(firmware_p, name, device);
+}
+EXPORT_SYMBOL(request_firmware);
+
 /**
  * release_firmware: - release the resource associated with a firmware image
  **/
@@ -455,6 +401,7 @@
 		kfree(fw);
 	}
 }
+EXPORT_SYMBOL(release_firmware);
 
 /**
  * register_firmware: - provide a firmware image for later usage
@@ -472,6 +419,7 @@
 	 * decide if firmware caching is reasonable just leave it as a
 	 * noop */
 }
+EXPORT_SYMBOL(register_firmware);
 
 /* Async support */
 struct firmware_work {
@@ -480,11 +428,12 @@
 	const char *name;
 	struct device *device;
 	void *context;
-	void (*cont)(const struct firmware *fw, void *context);
+	void (*cont_fw)(const struct firmware *fw, void *context);
+	void (*cont_post)(void *context);
 };
 
 static int
-request_firmware_work_func(void *arg)
+request_post_work_func(void *arg)
 {
 	struct firmware_work *fw_work = arg;
 	const struct firmware *fw;
@@ -492,10 +441,15 @@
 		WARN_ON(1);
 		return 0;
 	}
-	daemonize("%s/%s", "firmware", fw_work->name);
-	request_firmware(&fw, fw_work->name, fw_work->device);
-	fw_work->cont(fw, fw_work->context);
-	release_firmware(fw);
+	daemonize("%s/%s", "post", fw_work->name);
+	if (fw_work->cont_fw) {
+		request(&fw, fw_work->name, fw_work->device);
+		fw_work->cont_fw(fw, fw_work->context);
+		release_firmware(fw);
+	} else {
+		request(NULL, fw_work->name, fw_work->device);
+		fw_work->cont_post(fw_work->context);
+	}
 	module_put(fw_work->module);
 	kfree(fw_work);
 	return 0;
@@ -515,11 +469,12 @@
  *	@fw may be %NULL if firmware request fails.
  *
  **/
-int
-request_firmware_nowait(
+static int
+request_nowait(
 	struct module *module,
 	const char *name, struct device *device, void *context,
-	void (*cont)(const struct firmware *fw, void *context))
+	void (*cont_fw)(const struct firmware *fw, void *context),
+	void (*cont_post)(void *context))
 {
 	struct firmware_work *fw_work = kmalloc(sizeof (struct firmware_work),
 						GFP_ATOMIC);
@@ -537,47 +492,37 @@
 		.name = name,
 		.device = device,
 		.context = context,
-		.cont = cont,
+		.cont_fw = cont_fw,
+		.cont_post = cont_post,
 	};
 
-	ret = kernel_thread(request_firmware_work_func, fw_work,
+	ret = kernel_thread(request_post_work_func, fw_work,
 			    CLONE_FS | CLONE_FILES);
 
 	if (ret < 0) {
-		fw_work->cont(NULL, fw_work->context);
+		if (fw_work->cont_fw)
+			fw_work->cont_fw(NULL, fw_work->context);
+		if (fw_work->cont_post)
+			fw_work->cont_post(fw_work->context);
 		return ret;
 	}
 	return 0;
 }
 
-static int __init
-firmware_class_init(void)
+int
+request_firmware_nowait(struct module *module, const char *name,
+			struct device *device, void *context,
+			void (*cont_fw)(const struct firmware *fw, void *context))
 {
-	int error;
-	error = class_register(&firmware_class);
-	if (error) {
-		printk(KERN_ERR "%s: class_register failed\n", __FUNCTION__);
-		return error;
-	}
-	error = class_create_file(&firmware_class, &class_attr_timeout);
-	if (error) {
-		printk(KERN_ERR "%s: class_create_file failed\n",
-		       __FUNCTION__);
-		class_unregister(&firmware_class);
-	}
-	return error;
-
+	return request_nowait(module, name, device, context, cont_fw, NULL);
 }
-static void __exit
-firmware_class_exit(void)
+EXPORT_SYMBOL(request_firmware_nowait);
+
+int
+request_post_nowait(struct module *module, const char *name,
+		    struct device *device, void *context,
+		    void (*cont_post)(void *context))
 {
-	class_unregister(&firmware_class);
+	return request_nowait(module, name, device, context, NULL, cont_post);
 }
-
-module_init(firmware_class_init);
-module_exit(firmware_class_exit);
-
-EXPORT_SYMBOL(release_firmware);
-EXPORT_SYMBOL(request_firmware);
-EXPORT_SYMBOL(request_firmware_nowait);
-EXPORT_SYMBOL(register_firmware);
+EXPORT_SYMBOL(request_post_nowait);
===== drivers/video/aty/radeon_base.c 1.44 vs edited =====
--- 1.44/drivers/video/aty/radeon_base.c	2005-03-10 03:39:11 -05:00
+++ edited/drivers/video/aty/radeon_base.c	2005-03-19 21:17:49 -05:00
@@ -71,6 +71,7 @@
 #include <linux/vmalloc.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
+#include <linux/firmware.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>
@@ -2206,6 +2207,9 @@
 	.read	= radeon_show_edid2,
 };
 
+static void radeon_post_finished(void *data) {
+	printk(KERN_ERR "radeon_post_finished\n");
+}
 
 static int radeonfb_pci_register (struct pci_dev *pdev,
 				  const struct pci_device_id *ent)
@@ -2410,6 +2414,8 @@
 	}
 #endif
 
+	request_post_nowait(THIS_MODULE, "vbios.vm86", &pdev->dev, rinfo,
radeon_post_finished);
+
 	printk ("radeonfb (%s): %s\n", pci_name(rinfo->pdev), rinfo->name);
 
 	if (rinfo->bios_seg)
@@ -2453,7 +2459,19 @@
         if (!rinfo)
                 return;
  
+	/* Register some sysfs stuff (should be done better) */
+	if (rinfo->mon1_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid1_attr);
+	if (rinfo->mon2_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid2_attr);
+
 	radeonfb_pm_exit(rinfo);
+
+	/* Register some sysfs stuff (should be done better) */
+	if (rinfo->mon1_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid1_attr);
+	if (rinfo->mon2_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid2_attr);
 
 #if 0
 	/* restore original state
===== include/linux/device.h 1.138 vs edited =====
--- 1.138/include/linux/device.h	2005-03-09 12:03:56 -05:00
+++ edited/include/linux/device.h	2005-03-19 11:11:34 -05:00
@@ -271,6 +271,7 @@
 	void		*driver_data;	/* data private to the driver */
 	void		*platform_data;	/* Platform specific data (e.g. ACPI,
 					   BIOS data relevant to device) */
+	void		*post_data;	/* active during firmware load or post */
 	struct dev_pm_info	power;
 
 	u32		detach_state;	/* State to enter when device is
@@ -399,6 +400,7 @@
 /* drivers/base/firmware.c */
 extern int firmware_register(struct subsystem *);
 extern void firmware_unregister(struct subsystem *);
+extern int post_timeout; /* In seconds */
 
 /* debugging and troubleshooting/diagnostic helpers. */
 #define dev_printk(level, dev, format, arg...)	\
===== include/linux/firmware.h 1.3 vs edited =====
--- 1.3/include/linux/firmware.h	2005-02-02 04:49:28 -05:00
+++ edited/include/linux/firmware.h	2005-03-19 21:12:52 -05:00
@@ -1,20 +1,27 @@
 #ifndef _LINUX_FIRMWARE_H
 #define _LINUX_FIRMWARE_H
+#include <linux/device.h>
 #include <linux/module.h>
 #include <linux/types.h>
+
 #define FIRMWARE_NAME_MAX 30 
+
 struct firmware {
 	size_t size;
 	u8 *data;
 };
-struct device;
+
+int request_post_nowait(struct module *module, const char *name,
+			struct device *device, void *context,
+			void (*cont_post)(void *context));
+
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
-int request_firmware_nowait(
-	struct module *module,
-	const char *name, struct device *device, void *context,
-	void (*cont)(const struct firmware *fw, void *context));
-
+int request_firmware_nowait(struct module *module, const char *name,
+		struct device *device, void *context, 
+		void (*cont_fw)(const struct firmware *fw, void *context));
 void release_firmware(const struct firmware *fw);
+
 void register_firmware(const char *name, const u8 *data, size_t size);
+
 #endif
===== include/linux/kobject.h 1.39 vs edited =====
--- 1.39/include/linux/kobject.h	2005-03-09 12:04:09 -05:00
+++ edited/include/linux/kobject.h	2005-03-19 01:32:55 -05:00
@@ -242,13 +242,19 @@
 extern void subsys_remove_file(struct subsystem * , struct subsys_attribute *);
 
 #ifdef CONFIG_HOTPLUG
-void kobject_hotplug(struct kobject *kobj, enum kobject_action action);
+void kobject_hotplug(struct kobject *kobj, enum kobject_action action,
+		     int (*hotplug)(struct kset *kset, struct kobject *kobj, char **envp,
+		     int num_envp, char *buffer, int buffer_size)
+		    );
 int add_hotplug_env_var(char **envp, int num_envp, int *cur_index,
 			char *buffer, int buffer_size, int *cur_len,
 			const char *format, ...)
 	__attribute__((format (printf, 7, 8)));
 #else
-static inline void kobject_hotplug(struct kobject *kobj, enum
kobject_action action) { }
+static inline void kobject_hotplug(struct kobject *kobj, enum
kobject_action action,
+				   int (*hotplug)(struct kset *kset, struct kobject *kobj, char **envp,
+				   int num_envp, char *buffer, int buffer_size)
+				  ) { }
 static inline int add_hotplug_env_var(char **envp, int num_envp, int
*cur_index,
 				      char *buffer, int buffer_size, int *cur_len, 
 				      const char *format, ...)
===== include/linux/kobject_uevent.h 1.6 vs edited =====
--- 1.6/include/linux/kobject_uevent.h	2004-11-08 14:43:30 -05:00
+++ edited/include/linux/kobject_uevent.h	2005-03-17 10:36:00 -05:00
@@ -29,6 +29,7 @@
 	KOBJ_UMOUNT	= (__force kobject_action_t) 0x05,	/* umount event for
block devices */
 	KOBJ_OFFLINE	= (__force kobject_action_t) 0x06,	/* offline event for
hotplug devices */
 	KOBJ_ONLINE	= (__force kobject_action_t) 0x07,	/* online event for
hotplug devices */
+	KOBJ_POST 	= (__force kobject_action_t) 0x08,	/* post event supports
user space initialization */
 };
 
 
===== lib/kobject.c 1.58 vs edited =====
--- 1.58/lib/kobject.c	2005-03-09 12:04:09 -05:00
+++ edited/lib/kobject.c	2005-03-19 01:39:37 -05:00
@@ -185,7 +185,7 @@
 		if (parent)
 			kobject_put(parent);
 	} else {
-		kobject_hotplug(kobj, KOBJ_ADD);
+		kobject_hotplug(kobj, KOBJ_ADD, NULL);
 	}
 
 	return error;
@@ -301,7 +301,7 @@
 
 void kobject_del(struct kobject * kobj)
 {
-	kobject_hotplug(kobj, KOBJ_REMOVE);
+	kobject_hotplug(kobj, KOBJ_REMOVE, NULL);
 	sysfs_remove_dir(kobj);
 	unlink(kobj);
 }
===== lib/kobject_uevent.c 1.18 vs edited =====
--- 1.18/lib/kobject_uevent.c	2005-01-08 00:44:13 -05:00
+++ edited/lib/kobject_uevent.c	2005-03-19 01:48:04 -05:00
@@ -44,6 +44,8 @@
 		return "offline";
 	case KOBJ_ONLINE:
 		return "online";
+	case KOBJ_POST:
+		return "post";
 	default:
 		return NULL;
 	}
@@ -187,7 +189,9 @@
  * @action: action that is happening (usually "ADD" or "REMOVE")
  * @kobj: struct kobject that the action is happening to
  */
-void kobject_hotplug(struct kobject *kobj, enum kobject_action action)
+void kobject_hotplug(struct kobject *kobj, enum kobject_action action,
+		int (*hotplug)(struct kset *kset, struct kobject *kobj, char **envp,
+		int num_envp, char *buffer, int buffer_size))
 {
 	char *argv [3];
 	char **envp = NULL;
@@ -279,6 +283,17 @@
 	if (hotplug_ops->hotplug) {
 		/* have the kset specific function add its stuff */
 		retval = hotplug_ops->hotplug (kset, kobj,
+				  &envp[i], NUM_ENVP - i, scratch,
+				  BUFFER_SIZE - (scratch - buffer));
+		if (retval) {
+			pr_debug ("%s - hotplug() returned %d\n",
+				  __FUNCTION__, retval);
+			goto exit;
+		}
+	}
+	if (hotplug) {
+		/* have the call specific function add on its stuff */
+		retval = hotplug (kset, kobj,
 				  &envp[i], NUM_ENVP - i, scratch,
 				  BUFFER_SIZE - (scratch - buffer));
 		if (retval) {

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: firmware.patch --]
[-- Type: text/x-diff; name="firmware.patch", Size: 28315 bytes --]

===== drivers/acpi/container.c 1.2 vs edited =====
--- 1.2/drivers/acpi/container.c	2004-11-11 02:56:31 -05:00
+++ edited/drivers/acpi/container.c	2005-03-19 01:38:11 -05:00
@@ -182,16 +182,16 @@
 			if (ACPI_FAILURE(status) || !device) {
 				result = container_device_add(&device, handle);
 				if (!result)
-					kobject_hotplug(&device->kobj, KOBJ_ONLINE);
+					kobject_hotplug(&device->kobj, KOBJ_ONLINE, NULL);
 			} else {
 				/* device exist and this is a remove request */
-				kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+				kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 			}
 		}
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		if (!acpi_bus_get_device(handle, &device) && device) {
-			kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 		}
 		break;
 	default:
===== drivers/acpi/processor_core.c 1.80 vs edited =====
--- 1.80/drivers/acpi/processor_core.c	2004-12-14 07:14:54 -05:00
+++ edited/drivers/acpi/processor_core.c	2005-03-19 01:38:57 -05:00
@@ -730,7 +730,7 @@
 		return_VALUE(-ENODEV);
 
 	if ((pr->id >=0) && (pr->id < NR_CPUS)) {
-		kobject_hotplug(&(*device)->kobj, KOBJ_ONLINE);
+		kobject_hotplug(&(*device)->kobj, KOBJ_ONLINE, NULL);
 	}
 	return_VALUE(0);
 }
@@ -774,13 +774,13 @@
 		}
 
 		if (pr->id >= 0 && (pr->id < NR_CPUS)) {
-			kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 			break;
 		}
 
 		result = acpi_processor_start(device);
 		if ((!result) && ((pr->id >=0) && (pr->id < NR_CPUS))) {
-			kobject_hotplug(&device->kobj, KOBJ_ONLINE);
+			kobject_hotplug(&device->kobj, KOBJ_ONLINE, NULL);
 		} else {
 			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
 				"Device [%s] failed to start\n",
@@ -801,7 +801,7 @@
 		}
 
 		if ((pr->id < NR_CPUS) && (cpu_present(pr->id)))
-			kobject_hotplug(&device->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&device->kobj, KOBJ_OFFLINE, NULL);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
===== drivers/base/cpu.c 1.22 vs edited =====
--- 1.22/drivers/base/cpu.c	2005-01-31 01:33:46 -05:00
+++ edited/drivers/base/cpu.c	2005-03-19 01:39:16 -05:00
@@ -33,7 +33,7 @@
 	case '0':
 		ret = cpu_down(cpu->sysdev.id);
 		if (!ret)
-			kobject_hotplug(&dev->kobj, KOBJ_OFFLINE);
+			kobject_hotplug(&dev->kobj, KOBJ_OFFLINE, NULL);
 		break;
 	case '1':
 		ret = cpu_up(cpu->sysdev.id);
===== drivers/base/firmware.c 1.9 vs edited =====
--- 1.9/drivers/base/firmware.c	2004-09-24 14:45:35 -04:00
+++ edited/drivers/base/firmware.c	2005-03-19 11:16:01 -05:00
@@ -14,21 +14,57 @@
 
 static decl_subsys(firmware, NULL, NULL);
 
+int post_timeout = 10; /* In seconds */
+EXPORT_SYMBOL_GPL(post_timeout);
+
 int firmware_register(struct subsystem * s)
 {
 	kset_set_kset_s(s, firmware_subsys);
 	return subsystem_register(s);
 }
+EXPORT_SYMBOL_GPL(firmware_register);
 
 void firmware_unregister(struct subsystem * s)
 {
 	subsystem_unregister(s);
 }
+EXPORT_SYMBOL_GPL(firmware_unregister);
+
+/**
+ * post_timeout_store:
+ * Description:
+ *	Sets the number of seconds to wait for the post/firmware load.
+ *	Once this expires an error will be return to the driver and no
+ *	firmware will be provided.
+ *
+ *	Note: zero means 'wait for ever'
+ *
+ **/
+static ssize_t post_timeout_store(struct subsystem *subsys, const char *buf, size_t count)
+{
+	post_timeout = simple_strtol(buf, NULL, 10);
+	return count;
+}
+static ssize_t post_timeout_show(struct subsystem *subsys, char *buf)
+{
+	return sprintf(buf, "%d\n", post_timeout);
+}
+static struct subsys_attribute post_timeout_attr = {
+	.attr = {.name = "post_timeout", .mode = 0644, .owner = THIS_MODULE},
+	.show = post_timeout_show,
+	.store = post_timeout_store
+};
 
 int __init firmware_init(void)
 {
-	return subsystem_register(&firmware_subsys);
+	int error;
+	if ((error = subsystem_register(&firmware_subsys)))
+		return error;
+	
+	if ((error = subsys_create_file(&firmware_subsys, &post_timeout_attr))) {
+		subsystem_unregister(&firmware_subsys);
+		return error;
+	}
+	return 0;
 }
 
-EXPORT_SYMBOL_GPL(firmware_register);
-EXPORT_SYMBOL_GPL(firmware_unregister);
===== drivers/base/firmware_class.c 1.25 vs edited =====
--- 1.25/drivers/base/firmware_class.c	2004-11-26 15:26:48 -05:00
+++ edited/drivers/base/firmware_class.c	2005-03-19 21:11:15 -05:00
@@ -1,7 +1,8 @@
 /*
- * firmware_class.c - Multi purpose firmware loading support
+ * firmware_class.c - Multi purpose post support
  *
  * Copyright (c) 2003 Manuel Estrada Sainz <ranty@debian.org>
+ * Copyright (c) 2005 Jon Smirl <jonsmirl@gmail.com>
  *
  * Please see Documentation/firmware_class/ for more information.
  *
@@ -20,7 +21,7 @@
 #include "base.h"
 
 MODULE_AUTHOR("Manuel Estrada Sainz <ranty@debian.org>");
-MODULE_DESCRIPTION("Multi purpose firmware loading support");
+MODULE_DESCRIPTION("Multi purpose post support");
 MODULE_LICENSE("GPL");
 
 enum {
@@ -30,8 +31,6 @@
 	FW_STATUS_READY,
 };
 
-static int loading_timeout = 10;	/* In seconds */
-
 /* fw_lock could be moved to 'struct firmware_priv' but since it is just
  * guarding for corner cases a global lock should be OK */
 static DECLARE_MUTEX(fw_lock);
@@ -49,69 +48,41 @@
 static inline void
 fw_load_abort(struct firmware_priv *fw_priv)
 {
+	printk(KERN_ERR "fw_load_abort\n");
 	set_bit(FW_STATUS_ABORT, &fw_priv->status);
 	wmb();
 	complete(&fw_priv->completion);
 }
 
-static ssize_t
-firmware_timeout_show(struct class *class, char *buf)
-{
-	return sprintf(buf, "%d\n", loading_timeout);
-}
-
-/**
- * firmware_timeout_store:
- * Description:
- *	Sets the number of seconds to wait for the firmware.  Once
- *	this expires an error will be return to the driver and no
- *	firmware will be provided.
- *
- *	Note: zero means 'wait for ever'
- *
- **/
-static ssize_t
-firmware_timeout_store(struct class *class, const char *buf, size_t count)
-{
-	loading_timeout = simple_strtol(buf, NULL, 10);
-	return count;
-}
-
-static CLASS_ATTR(timeout, 0644, firmware_timeout_show, firmware_timeout_store);
-
-static void  fw_class_dev_release(struct class_device *class_dev);
-int firmware_class_hotplug(struct class_device *dev, char **envp,
-			   int num_envp, char *buffer, int buffer_size);
-
-static struct class firmware_class = {
-	.name		= "firmware",
-	.hotplug	= firmware_class_hotplug,
-	.release	= fw_class_dev_release,
-};
-
 int
-firmware_class_hotplug(struct class_device *class_dev, char **envp,
+post_hotplug(struct kset *kset, struct kobject *kobj, char **envp,
 		       int num_envp, char *buffer, int buffer_size)
 {
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct firmware_priv *fw_priv = dev->post_data;
 	int i = 0, len = 0;
 
 	if (!test_bit(FW_STATUS_READY, &fw_priv->status))
 		return -ENODEV;
 
-	if (add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size, &len,
-			"FIRMWARE=%s", fw_priv->fw_id))
-		return -ENOMEM;
-
+	if (fw_priv->fw) {
+		if (add_hotplug_env_var(envp, num_envp, &i, buffer,
+		    buffer_size, &len, "FIRMWARE=%s", fw_priv->fw_id))
+			return -ENOMEM;
+	} else {
+		if (add_hotplug_env_var(envp, num_envp, &i, buffer,
+		    buffer_size, &len, "POST=%s", fw_priv->fw_id))
+			return -ENOMEM;
+	}
 	envp[i] = NULL;
 
 	return 0;
 }
 
 static ssize_t
-firmware_loading_show(struct class_device *class_dev, char *buf)
+post_status_show(struct device *dev, char *buf)
 {
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
+	struct firmware_priv *fw_priv = dev->post_data;
 	int loading = test_bit(FW_STATUS_LOADING, &fw_priv->status);
 	return sprintf(buf, "%d\n", loading);
 }
@@ -126,13 +97,13 @@
  *	-1: Conclude the load with an error and discard any written data.
  **/
 static ssize_t
-firmware_loading_store(struct class_device *class_dev,
-		       const char *buf, size_t count)
+post_status_store(struct device *dev, const char *buf, size_t count)
 {
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
-	int loading = simple_strtol(buf, NULL, 10);
+	struct firmware_priv *fw_priv = dev->post_data;
+	int status = simple_strtol(buf, NULL, 10);
 
-	switch (loading) {
+	printk(KERN_ERR "post_status_store: status %d\n", status);
+	switch (status) {
 	case 1:
 		down(&fw_lock);
 		vfree(fw_priv->fw->data);
@@ -151,7 +122,7 @@
 		/* fallthrough */
 	default:
 		printk(KERN_ERR "%s: unexpected value (%d)\n", __FUNCTION__,
-		       loading);
+		       status);
 		/* fallthrough */
 	case -1:
 		fw_load_abort(fw_priv);
@@ -161,15 +132,14 @@
 	return count;
 }
 
-static CLASS_DEVICE_ATTR(loading, 0644,
-			firmware_loading_show, firmware_loading_store);
+static DEVICE_ATTR(post_status, 0644, post_status_show, post_status_store);
 
 static ssize_t
-firmware_data_read(struct kobject *kobj,
-		   char *buffer, loff_t offset, size_t count)
+post_data_read(struct kobject *kobj, 
+	       char *buffer, loff_t offset, size_t count)
 {
-	struct class_device *class_dev = to_class_dev(kobj);
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct firmware_priv *fw_priv = dev->post_data;
 	struct firmware *fw;
 	ssize_t ret_count = count;
 
@@ -191,6 +161,7 @@
 	up(&fw_lock);
 	return ret_count;
 }
+
 static int
 fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size)
 {
@@ -225,8 +196,8 @@
  *	the driver as a firmware image.
  **/
 static ssize_t
-firmware_data_write(struct kobject *kobj,
-		    char *buffer, loff_t offset, size_t count)
+post_data_write(struct kobject *kobj, 
+		char *buffer, loff_t offset, size_t count)
 {
 	struct class_device *class_dev = to_class_dev(kobj);
 	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
@@ -254,124 +225,82 @@
 	return retval;
 }
 static struct bin_attribute firmware_attr_data_tmpl = {
-	.attr = {.name = "data", .mode = 0644, .owner = THIS_MODULE},
+	.attr = {.name = "post_data", .mode = 0644, .owner = THIS_MODULE},
 	.size = 0,
-	.read = firmware_data_read,
-	.write = firmware_data_write,
+	.read = post_data_read,
+	.write = post_data_write,
 };
 
 static void
-fw_class_dev_release(struct class_device *class_dev)
-{
-	struct firmware_priv *fw_priv = class_get_devdata(class_dev);
-
-	kfree(fw_priv);
-	kfree(class_dev);
-
-	module_put(THIS_MODULE);
-}
-
-static void
-firmware_class_timeout(u_long data)
+post_timeout_abort(u_long data)
 {
 	struct firmware_priv *fw_priv = (struct firmware_priv *) data;
+	printk(KERN_ERR "post_timeout_abort: time out\n");
 	fw_load_abort(fw_priv);
 }
 
-static inline void
-fw_setup_class_device_id(struct class_device *class_dev, struct device *dev)
-{
-	/* XXX warning we should watch out for name collisions */
-	strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
-}
-
 static int
-fw_register_class_device(struct class_device **class_dev_p,
-			 const char *fw_name, struct device *device)
+fw_register_device(const char *fw_name, struct device *device)
 {
 	int retval;
 	struct firmware_priv *fw_priv = kmalloc(sizeof (struct firmware_priv),
 						GFP_KERNEL);
-	struct class_device *class_dev = kmalloc(sizeof (struct class_device),
-						 GFP_KERNEL);
-
-	*class_dev_p = NULL;
 
-	if (!fw_priv || !class_dev) {
+	if (!fw_priv) {
 		printk(KERN_ERR "%s: kmalloc failed\n", __FUNCTION__);
 		retval = -ENOMEM;
 		goto error_kfree;
 	}
 	memset(fw_priv, 0, sizeof (*fw_priv));
-	memset(class_dev, 0, sizeof (*class_dev));
 
 	init_completion(&fw_priv->completion);
 	fw_priv->attr_data = firmware_attr_data_tmpl;
 	strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX);
 
-	fw_priv->timeout.function = firmware_class_timeout;
+	fw_priv->timeout.function = post_timeout_abort;
 	fw_priv->timeout.data = (u_long) fw_priv;
 	init_timer(&fw_priv->timeout);
+	
+	device->post_data = fw_priv;
 
-	fw_setup_class_device_id(class_dev, device);
-	class_dev->dev = device;
-	class_dev->class = &firmware_class;
-	class_set_devdata(class_dev, fw_priv);
-	retval = class_device_register(class_dev);
-	if (retval) {
-		printk(KERN_ERR "%s: class_device_register failed\n",
-		       __FUNCTION__);
-		goto error_kfree;
-	}
-	*class_dev_p = class_dev;
 	return 0;
 
 error_kfree:
 	kfree(fw_priv);
-	kfree(class_dev);
 	return retval;
 }
 
 static int
-fw_setup_class_device(struct firmware *fw, struct class_device **class_dev_p,
-		      const char *fw_name, struct device *device)
+fw_setup_device(struct firmware *fw, const char *fw_name, struct device *device)
 {
-	struct class_device *class_dev;
 	struct firmware_priv *fw_priv;
 	int retval;
 
-	*class_dev_p = NULL;
-	retval = fw_register_class_device(&class_dev, fw_name, device);
+	retval = fw_register_device(fw_name, device);
 	if (retval)
 		goto out;
 
-	/* Need to pin this module until class device is destroyed */
-	__module_get(THIS_MODULE);
-
-	fw_priv = class_get_devdata(class_dev);
-
+	fw_priv = device->post_data;
 	fw_priv->fw = fw;
-	retval = sysfs_create_bin_file(&class_dev->kobj, &fw_priv->attr_data);
+	
+	retval = sysfs_create_bin_file(&device->kobj, &fw_priv->attr_data);
 	if (retval) {
 		printk(KERN_ERR "%s: sysfs_create_bin_file failed\n",
 		       __FUNCTION__);
-		goto error_unreg;
+		goto out;
 	}
 
-	retval = class_device_create_file(class_dev,
-					  &class_device_attr_loading);
-	if (retval) {
-		printk(KERN_ERR "%s: class_device_create_file failed\n",
-		       __FUNCTION__);
-		goto error_unreg;
+	if (fw) {
+		retval = device_create_file(device, &dev_attr_post_status);
+		if (retval) {
+			printk(KERN_ERR "%s: device_create_file failed\n",
+			       __FUNCTION__);
+			goto out;
+		}
 	}
-
 	set_bit(FW_STATUS_READY, &fw_priv->status);
-	*class_dev_p = class_dev;
 	goto out;
 
-error_unreg:
-	class_device_unregister(class_dev);
 out:
 	return retval;
 }
@@ -388,62 +317,79 @@
  *	should be distinctive enough not to be confused with any other
  *	firmware image for this or any other device.
  **/
-int
-request_firmware(const struct firmware **firmware_p, const char *name,
+static int
+request(const struct firmware **firmware_p, const char *name,
 		 struct device *device)
 {
-	struct class_device *class_dev;
 	struct firmware_priv *fw_priv;
-	struct firmware *firmware;
+	struct firmware *firmware = NULL;
 	int retval;
 
-	if (!firmware_p)
-		return -EINVAL;
-
-	*firmware_p = firmware = kmalloc(sizeof (struct firmware), GFP_KERNEL);
-	if (!firmware) {
-		printk(KERN_ERR "%s: kmalloc(struct firmware) failed\n",
-		       __FUNCTION__);
-		retval = -ENOMEM;
-		goto out;
+	if (firmware_p) {
+		*firmware_p = firmware = kmalloc(sizeof (struct firmware), GFP_KERNEL);
+		if (!firmware) {
+			printk(KERN_ERR "%s: kmalloc(struct firmware) failed\n",
+			       __FUNCTION__);
+			retval = -ENOMEM;
+			goto out;
+		}
+		memset(firmware, 0, sizeof (*firmware));
 	}
-	memset(firmware, 0, sizeof (*firmware));
 
-	retval = fw_setup_class_device(firmware, &class_dev, name, device);
+	retval = fw_setup_device(firmware, name, device);
 	if (retval)
 		goto error_kfree_fw;
 
-	fw_priv = class_get_devdata(class_dev);
+	fw_priv = device->post_data;
 
-	if (loading_timeout) {
-		fw_priv->timeout.expires = jiffies + loading_timeout * HZ;
+	if (post_timeout) {
+		fw_priv->timeout.expires = jiffies + post_timeout * HZ;
 		add_timer(&fw_priv->timeout);
 	}
 
-	kobject_hotplug(&class_dev->kobj, KOBJ_ADD);
+	printk(KERN_ERR "request_firmware: hotplug event\n");
+	kobject_hotplug(&device->kobj, KOBJ_POST, post_hotplug);
 	wait_for_completion(&fw_priv->completion);
 	set_bit(FW_STATUS_DONE, &fw_priv->status);
+	printk(KERN_ERR "request_firmware: complete\n");
 
 	del_timer_sync(&fw_priv->timeout);
 
-	down(&fw_lock);
-	if (!fw_priv->fw->size || test_bit(FW_STATUS_ABORT, &fw_priv->status)) {
-		retval = -ENOENT;
-		release_firmware(fw_priv->fw);
-		*firmware_p = NULL;
+	if (firmware_p) {
+		down(&fw_lock);
+		if (!fw_priv->fw->size || test_bit(FW_STATUS_ABORT, &fw_priv->status)) {
+			retval = -ENOENT;
+			release_firmware(fw_priv->fw);
+			*firmware_p = NULL;
+		}
+		fw_priv->fw = NULL;
+		up(&fw_lock);
+	
+		sysfs_remove_bin_file(&device->kobj, &fw_priv->attr_data);
 	}
-	fw_priv->fw = NULL;
-	up(&fw_lock);
-	class_device_unregister(class_dev);
+	device_remove_file(device, &dev_attr_post_status);
+	
 	goto out;
 
 error_kfree_fw:
-	kfree(firmware);
-	*firmware_p = NULL;
+	if (firmware) {
+		kfree(firmware);
+		*firmware_p = NULL;
+	}
 out:
 	return retval;
 }
 
+int
+request_firmware(const struct firmware **firmware_p, const char *name,
+		 struct device *device)
+{
+	if (!firmware_p)
+		return -EINVAL;
+	return request(firmware_p, name, device);
+}
+EXPORT_SYMBOL(request_firmware);
+
 /**
  * release_firmware: - release the resource associated with a firmware image
  **/
@@ -455,6 +401,7 @@
 		kfree(fw);
 	}
 }
+EXPORT_SYMBOL(release_firmware);
 
 /**
  * register_firmware: - provide a firmware image for later usage
@@ -472,6 +419,7 @@
 	 * decide if firmware caching is reasonable just leave it as a
 	 * noop */
 }
+EXPORT_SYMBOL(register_firmware);
 
 /* Async support */
 struct firmware_work {
@@ -480,11 +428,12 @@
 	const char *name;
 	struct device *device;
 	void *context;
-	void (*cont)(const struct firmware *fw, void *context);
+	void (*cont_fw)(const struct firmware *fw, void *context);
+	void (*cont_post)(void *context);
 };
 
 static int
-request_firmware_work_func(void *arg)
+request_post_work_func(void *arg)
 {
 	struct firmware_work *fw_work = arg;
 	const struct firmware *fw;
@@ -492,10 +441,15 @@
 		WARN_ON(1);
 		return 0;
 	}
-	daemonize("%s/%s", "firmware", fw_work->name);
-	request_firmware(&fw, fw_work->name, fw_work->device);
-	fw_work->cont(fw, fw_work->context);
-	release_firmware(fw);
+	daemonize("%s/%s", "post", fw_work->name);
+	if (fw_work->cont_fw) {
+		request(&fw, fw_work->name, fw_work->device);
+		fw_work->cont_fw(fw, fw_work->context);
+		release_firmware(fw);
+	} else {
+		request(NULL, fw_work->name, fw_work->device);
+		fw_work->cont_post(fw_work->context);
+	}
 	module_put(fw_work->module);
 	kfree(fw_work);
 	return 0;
@@ -515,11 +469,12 @@
  *	@fw may be %NULL if firmware request fails.
  *
  **/
-int
-request_firmware_nowait(
+static int
+request_nowait(
 	struct module *module,
 	const char *name, struct device *device, void *context,
-	void (*cont)(const struct firmware *fw, void *context))
+	void (*cont_fw)(const struct firmware *fw, void *context),
+	void (*cont_post)(void *context))
 {
 	struct firmware_work *fw_work = kmalloc(sizeof (struct firmware_work),
 						GFP_ATOMIC);
@@ -537,47 +492,37 @@
 		.name = name,
 		.device = device,
 		.context = context,
-		.cont = cont,
+		.cont_fw = cont_fw,
+		.cont_post = cont_post,
 	};
 
-	ret = kernel_thread(request_firmware_work_func, fw_work,
+	ret = kernel_thread(request_post_work_func, fw_work,
 			    CLONE_FS | CLONE_FILES);
 
 	if (ret < 0) {
-		fw_work->cont(NULL, fw_work->context);
+		if (fw_work->cont_fw)
+			fw_work->cont_fw(NULL, fw_work->context);
+		if (fw_work->cont_post)
+			fw_work->cont_post(fw_work->context);
 		return ret;
 	}
 	return 0;
 }
 
-static int __init
-firmware_class_init(void)
+int
+request_firmware_nowait(struct module *module, const char *name,
+			struct device *device, void *context,
+			void (*cont_fw)(const struct firmware *fw, void *context))
 {
-	int error;
-	error = class_register(&firmware_class);
-	if (error) {
-		printk(KERN_ERR "%s: class_register failed\n", __FUNCTION__);
-		return error;
-	}
-	error = class_create_file(&firmware_class, &class_attr_timeout);
-	if (error) {
-		printk(KERN_ERR "%s: class_create_file failed\n",
-		       __FUNCTION__);
-		class_unregister(&firmware_class);
-	}
-	return error;
-
+	return request_nowait(module, name, device, context, cont_fw, NULL);
 }
-static void __exit
-firmware_class_exit(void)
+EXPORT_SYMBOL(request_firmware_nowait);
+
+int
+request_post_nowait(struct module *module, const char *name,
+		    struct device *device, void *context,
+		    void (*cont_post)(void *context))
 {
-	class_unregister(&firmware_class);
+	return request_nowait(module, name, device, context, NULL, cont_post);
 }
-
-module_init(firmware_class_init);
-module_exit(firmware_class_exit);
-
-EXPORT_SYMBOL(release_firmware);
-EXPORT_SYMBOL(request_firmware);
-EXPORT_SYMBOL(request_firmware_nowait);
-EXPORT_SYMBOL(register_firmware);
+EXPORT_SYMBOL(request_post_nowait);
===== drivers/video/aty/radeon_base.c 1.44 vs edited =====
--- 1.44/drivers/video/aty/radeon_base.c	2005-03-10 03:39:11 -05:00
+++ edited/drivers/video/aty/radeon_base.c	2005-03-19 21:17:49 -05:00
@@ -71,6 +71,7 @@
 #include <linux/vmalloc.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
+#include <linux/firmware.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>
@@ -2206,6 +2207,9 @@
 	.read	= radeon_show_edid2,
 };
 
+static void radeon_post_finished(void *data) {
+	printk(KERN_ERR "radeon_post_finished\n");
+}
 
 static int radeonfb_pci_register (struct pci_dev *pdev,
 				  const struct pci_device_id *ent)
@@ -2410,6 +2414,8 @@
 	}
 #endif
 
+	request_post_nowait(THIS_MODULE, "vbios.vm86", &pdev->dev, rinfo, radeon_post_finished);
+
 	printk ("radeonfb (%s): %s\n", pci_name(rinfo->pdev), rinfo->name);
 
 	if (rinfo->bios_seg)
@@ -2453,7 +2459,19 @@
         if (!rinfo)
                 return;
  
+	/* Register some sysfs stuff (should be done better) */
+	if (rinfo->mon1_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid1_attr);
+	if (rinfo->mon2_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid2_attr);
+
 	radeonfb_pm_exit(rinfo);
+
+	/* Register some sysfs stuff (should be done better) */
+	if (rinfo->mon1_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid1_attr);
+	if (rinfo->mon2_EDID)
+		sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid2_attr);
 
 #if 0
 	/* restore original state
===== include/linux/device.h 1.138 vs edited =====
--- 1.138/include/linux/device.h	2005-03-09 12:03:56 -05:00
+++ edited/include/linux/device.h	2005-03-19 11:11:34 -05:00
@@ -271,6 +271,7 @@
 	void		*driver_data;	/* data private to the driver */
 	void		*platform_data;	/* Platform specific data (e.g. ACPI,
 					   BIOS data relevant to device) */
+	void		*post_data;	/* active during firmware load or post */
 	struct dev_pm_info	power;
 
 	u32		detach_state;	/* State to enter when device is
@@ -399,6 +400,7 @@
 /* drivers/base/firmware.c */
 extern int firmware_register(struct subsystem *);
 extern void firmware_unregister(struct subsystem *);
+extern int post_timeout; /* In seconds */
 
 /* debugging and troubleshooting/diagnostic helpers. */
 #define dev_printk(level, dev, format, arg...)	\
===== include/linux/firmware.h 1.3 vs edited =====
--- 1.3/include/linux/firmware.h	2005-02-02 04:49:28 -05:00
+++ edited/include/linux/firmware.h	2005-03-19 21:12:52 -05:00
@@ -1,20 +1,27 @@
 #ifndef _LINUX_FIRMWARE_H
 #define _LINUX_FIRMWARE_H
+#include <linux/device.h>
 #include <linux/module.h>
 #include <linux/types.h>
+
 #define FIRMWARE_NAME_MAX 30 
+
 struct firmware {
 	size_t size;
 	u8 *data;
 };
-struct device;
+
+int request_post_nowait(struct module *module, const char *name,
+			struct device *device, void *context,
+			void (*cont_post)(void *context));
+
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
-int request_firmware_nowait(
-	struct module *module,
-	const char *name, struct device *device, void *context,
-	void (*cont)(const struct firmware *fw, void *context));
-
+int request_firmware_nowait(struct module *module, const char *name,
+		struct device *device, void *context, 
+		void (*cont_fw)(const struct firmware *fw, void *context));
 void release_firmware(const struct firmware *fw);
+
 void register_firmware(const char *name, const u8 *data, size_t size);
+
 #endif
===== include/linux/kobject.h 1.39 vs edited =====
--- 1.39/include/linux/kobject.h	2005-03-09 12:04:09 -05:00
+++ edited/include/linux/kobject.h	2005-03-19 01:32:55 -05:00
@@ -242,13 +242,19 @@
 extern void subsys_remove_file(struct subsystem * , struct subsys_attribute *);
 
 #ifdef CONFIG_HOTPLUG
-void kobject_hotplug(struct kobject *kobj, enum kobject_action action);
+void kobject_hotplug(struct kobject *kobj, enum kobject_action action,
+		     int (*hotplug)(struct kset *kset, struct kobject *kobj, char **envp,
+		     int num_envp, char *buffer, int buffer_size)
+		    );
 int add_hotplug_env_var(char **envp, int num_envp, int *cur_index,
 			char *buffer, int buffer_size, int *cur_len,
 			const char *format, ...)
 	__attribute__((format (printf, 7, 8)));
 #else
-static inline void kobject_hotplug(struct kobject *kobj, enum kobject_action action) { }
+static inline void kobject_hotplug(struct kobject *kobj, enum kobject_action action,
+				   int (*hotplug)(struct kset *kset, struct kobject *kobj, char **envp,
+				   int num_envp, char *buffer, int buffer_size)
+				  ) { }
 static inline int add_hotplug_env_var(char **envp, int num_envp, int *cur_index, 
 				      char *buffer, int buffer_size, int *cur_len, 
 				      const char *format, ...)
===== include/linux/kobject_uevent.h 1.6 vs edited =====
--- 1.6/include/linux/kobject_uevent.h	2004-11-08 14:43:30 -05:00
+++ edited/include/linux/kobject_uevent.h	2005-03-17 10:36:00 -05:00
@@ -29,6 +29,7 @@
 	KOBJ_UMOUNT	= (__force kobject_action_t) 0x05,	/* umount event for block devices */
 	KOBJ_OFFLINE	= (__force kobject_action_t) 0x06,	/* offline event for hotplug devices */
 	KOBJ_ONLINE	= (__force kobject_action_t) 0x07,	/* online event for hotplug devices */
+	KOBJ_POST 	= (__force kobject_action_t) 0x08,	/* post event supports user space initialization */
 };
 
 
===== lib/kobject.c 1.58 vs edited =====
--- 1.58/lib/kobject.c	2005-03-09 12:04:09 -05:00
+++ edited/lib/kobject.c	2005-03-19 01:39:37 -05:00
@@ -185,7 +185,7 @@
 		if (parent)
 			kobject_put(parent);
 	} else {
-		kobject_hotplug(kobj, KOBJ_ADD);
+		kobject_hotplug(kobj, KOBJ_ADD, NULL);
 	}
 
 	return error;
@@ -301,7 +301,7 @@
 
 void kobject_del(struct kobject * kobj)
 {
-	kobject_hotplug(kobj, KOBJ_REMOVE);
+	kobject_hotplug(kobj, KOBJ_REMOVE, NULL);
 	sysfs_remove_dir(kobj);
 	unlink(kobj);
 }
===== lib/kobject_uevent.c 1.18 vs edited =====
--- 1.18/lib/kobject_uevent.c	2005-01-08 00:44:13 -05:00
+++ edited/lib/kobject_uevent.c	2005-03-19 01:48:04 -05:00
@@ -44,6 +44,8 @@
 		return "offline";
 	case KOBJ_ONLINE:
 		return "online";
+	case KOBJ_POST:
+		return "post";
 	default:
 		return NULL;
 	}
@@ -187,7 +189,9 @@
  * @action: action that is happening (usually "ADD" or "REMOVE")
  * @kobj: struct kobject that the action is happening to
  */
-void kobject_hotplug(struct kobject *kobj, enum kobject_action action)
+void kobject_hotplug(struct kobject *kobj, enum kobject_action action,
+		int (*hotplug)(struct kset *kset, struct kobject *kobj, char **envp,
+		int num_envp, char *buffer, int buffer_size))
 {
 	char *argv [3];
 	char **envp = NULL;
@@ -279,6 +283,17 @@
 	if (hotplug_ops->hotplug) {
 		/* have the kset specific function add its stuff */
 		retval = hotplug_ops->hotplug (kset, kobj,
+				  &envp[i], NUM_ENVP - i, scratch,
+				  BUFFER_SIZE - (scratch - buffer));
+		if (retval) {
+			pr_debug ("%s - hotplug() returned %d\n",
+				  __FUNCTION__, retval);
+			goto exit;
+		}
+	}
+	if (hotplug) {
+		/* have the call specific function add on its stuff */
+		retval = hotplug (kset, kobj,
 				  &envp[i], NUM_ENVP - i, scratch,
 				  BUFFER_SIZE - (scratch - buffer));
 		if (retval) {

[-- Attachment #3: 30-post.hotplug --]
[-- Type: application/octet-stream, Size: 1343 bytes --]

#!/bin/sh
#
# Firmware-specific hotplug policy agent.
#
# Kernel firmware hotplug params include:
#
#       ACTION=%s [add or remove]
#       DEVPATH=%s [in 2.5 kernels, /sys/$DEVPATH]
#       FIRMWARE=%s
#
# HISTORY:
#
# 24-Jul-2003   Initial version of "new" hotplug agent.
#
# $Id: firmware.agent,v 1.3 2004/03/14 15:52:56 ukai Exp $
#

cd /etc/hotplug
. ./hotplug.functions
# DEBUG=yes export DEBUG

# directory of the firmware files
FIRMWARE_DIR=/lib/firmware

# mountpoint of sysfs
SYSFS=$(sed -n 's/^.* \([^ ]*\) sysfs .*$/\1/p' /proc/mounts)

# use /proc for 2.4 kernels
if [ "$SYSFS" = "" ]; then
    SYSFS=/proc
fi

#
# What to do with this firmware hotplug event?
#
case "$ACTION" in

post)
    if [ ! -e $SYSFS/$DEVPATH/post_status ]; then
        sleep 1
    fi

    if [ "$FIRMWARE" != "" ]; then
        if [ -f "$FIRMWARE_DIR/$FIRMWARE" ]; then
            echo 1 > $SYSFS/$DEVPATH/post_status
            cp "$FIRMWARE_DIR/$FIRMWARE" $SYSFS/$DEVPATH/post_data
            echo 0 > $SYSFS/$DEVPATH/post_status
        else
            echo -1 > $SYSFS/$DEVPATH/post_status
        fi
    fi
    if [ "$POST" != "" ]; then
	echo "Posting! $POST" >/post
        echo 1 > $SYSFS/$DEVPATH/post_status
        exec $POST "$@"
        echo 0 > $SYSFS/$DEVPATH/post_status
	echo "Posted! $POST" >>/post
    fi
    ;;

*)
    ;;

esac

Re: Rework of request firmware

[Kay Sievers]

On Sat, 2005-03-19 at 23:06 -0500, Jon Smirl wrote:
> On Sat, 26 Feb 2005 20:41:16 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > I think the request_firmware() should be redesigned in a generic
> > request_user_data(kobj) call. This event would copy arbitrary data into
> > a sysfs file and something like KOBJ_REQUEST_DATA would do it.
> > 
> > The call should create a file inside of a existing device and the event
> > handler should copy the data into this file instead of creating a own
> > class device for the firmware as we do today.
> 
> This is a rework of request firmware to move the attributes from their
> own class into the device sysfs directory.

I don't think that we should change firmware_class that way. If we want
to have something that acts like this, we should provide a new interface
and not rename and change something that is widely used, seems to work
but has races and easy to trigger oops's. 

> I am also generalizing
> things so that I can request a post (this is what I need) as well as
> firmware. This is a first post of the code, please tell me what I can
> do to improve it before I send it to lkml.

What is the concept of a "post"? Does it receive data for the kernel to
use? What is the overlap with firmware loading here? You just need to
run a program from userspace, right? 

> I made a few changes in the base code:
> 1) I modified kobj_hotplug() to take an extra function for adding
> variables. This lets the event add the normal variable for the kset
> and then I can tack mine on too.
> 2) New event KOBJ_POST. $POST and $FIRMWARE are used to differentiate
> post versus firmware load in the script. Should this be two events
> instead?

Please explain what kind of event "post" is. Is it more than a
configuration-request that executes a userspace helper for a specific
device?

> 3) The script now needs to be in
> /etc/hotplug.d/default/30-post.hotplug. firmware.agent isn't used
> anymore. The firmware doesn't need to change or move.
> 4) I added a pointer to 'struct device' to track the firmware in
> sysfs. Is there a better way to do this?
> 5) I added the time out to /sys/firmware/post_timeout since there is
> no /sys/class/firmware any more.

Hmm, that place seems not in charge of this kind of "firmware", right?

> 6) How should everything be named? firmware, post, initialization, etc??
> 7) Should request_firmware() be deprecated forcing the move to
> request_firmware_nowait?

I still would like to see clearly defined list of requirements for:
  o async userspace data-requests into the kernel
  o async userspace configuration-requests from the kernel

before we start hacking on it. The current request_firmware() is a
not-so-nice example for doing it that way. All that stuff needs to play
with hotplug, initramfs/bootup, suspend/resume. And we should come up
with something that fits _all_ the needs and move the current
request_firmware-users over to it.

Thanks,
Kay




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Sun, 20 Mar 2005 14:37:16 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> What is the concept of a "post"? Does it receive data for the kernel to
> use? What is the overlap with firmware loading here? You just need to
> run a program from userspace, right?

Post is the commonly used term for what you do to hardware when it is
first turned on to get it running. For example the system BIOS posts
all the devices in the system.

The system BIOS does not post secondary video cards. For these cards
when the driver is first loaded I have to run a program that takes the
video BIOS ROM on the card and runs it in vm86/emu86. The ROM contents
are already exposed in sysfs. If I don't do this the hardware will not
respond. The system BIOS does this for you in real mode on the primary
card during the boot process. Another time the video BIOS needs to be
run is when graphics cards are being resumed. The posting program
needs to run in user space and be triggered from the probe function.

I initially started to add support for post as a standalone service
but received complaints on lkml that the code needed was almost
identical to request_firmware(). BenH has also pointed out that the
synchronous request_firmware() will deadlock in many cases during the
resume process. Video card posting is targeted for early user space
when complete.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Sun, 20 Mar 2005 14:37:16 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> I still would like to see clearly defined list of requirements for:
>   o async userspace data-requests into the kernel
>   o async userspace configuration-requests from the kernel

Here's another model that might be more general...

accept_user_data() -- adds the data/status attributes to the device sysfs entry

request_helper(completion, environment string) -- causes
ACTION=helper, the environment string is added onto the normal
environment for an event from this device class. Only async version.

In the completion routine...
release_user_data() -- deletes the attributes, frees the data

This model addresses both the need to receive data and run helpers
that don't need data.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 10:35 -0500, Jon Smirl wrote:
> On Sun, 20 Mar 2005 14:37:16 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > What is the concept of a "post"? Does it receive data for the kernel to
> > use? What is the overlap with firmware loading here? You just need to
> > run a program from userspace, right?
> 
> Post is the commonly used term for what you do to hardware when it is
> first turned on to get it running. For example the system BIOS posts
> all the devices in the system.

Ok, the Power On Self Test. That sounds a bit specfic for something that
is a generic configuration request.

> The system BIOS does not post secondary video cards. For these cards
> when the driver is first loaded I have to run a program that takes the
> video BIOS ROM on the card and runs it in vm86/emu86. The ROM contents
> are already exposed in sysfs. If I don't do this the hardware will not
> respond. The system BIOS does this for you in real mode on the primary
> card during the boot process. Another time the video BIOS needs to be
> run is when graphics cards are being resumed. The posting program
> needs to run in user space and be triggered from the probe function.

Ah, ok. Sounds reasonable and like a nice trick too, to run x86 hardware
on other platforms. :)

> I initially started to add support for post as a standalone service
> but received complaints on lkml that the code needed was almost
> identical to request_firmware(). 

Well running a configuration program is just a very small subset
compared to copying data into the kernel, but anyway...

> BenH has also pointed out that the
> synchronous request_firmware() will deadlock in many cases during the
> resume process.

I believe that and events may get lost too in such situations.

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[David Zeuthen]

Hey Kay,

On Sun, 2005-03-20 at 14:37 +0100, Kay Sievers wrote:
> before we start hacking on it. The current request_firmware() is a
> not-so-nice example for doing it that way. All that stuff needs to play
> with hotplug, initramfs/bootup, suspend/resume. And we should come up
> with something that fits _all_ the needs and move the current
> request_firmware-users over to it.

On that topic, one of the things I'd like a future scheme to do is not
only emitting the request_firmware() event, but also emitting events
like someone_uploaded_firmware() and timeout_waiting_for_firmware().
Plus some state in sysfs to keep track of the state. All but
request_firmware() probably don't need hotplug events; kobject_uevents
would be fine for me, since I'd want some daemon to listen to it anyway.

All that probably needs to be changed to fit in with what Jon is doing;
the idea here is that we want to notify the user, in the desktop
session, that the firmware/initialization needed is not available. Then
the user can take appropriate action. 

Thanks,
David



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 12:19 -0500, David Zeuthen wrote:
> Hey Kay,
> 
> On Sun, 2005-03-20 at 14:37 +0100, Kay Sievers wrote:
> > before we start hacking on it. The current request_firmware() is a
> > not-so-nice example for doing it that way. All that stuff needs to play
> > with hotplug, initramfs/bootup, suspend/resume. And we should come up
> > with something that fits _all_ the needs and move the current
> > request_firmware-users over to it.
> 
> On that topic, one of the things I'd like a future scheme to do is not
> only emitting the request_firmware() event, but also emitting events
> like someone_uploaded_firmware() and timeout_waiting_for_firmware().
> Plus some state in sysfs to keep track of the state. All but
> request_firmware() probably don't need hotplug events; kobject_uevents
> would be fine for me, since I'd want some daemon to listen to it anyway.

We need to cover the early boot and initramfs anyway and therefore need
to fork something here. But that will be tunneled
through /proc/sys/kernel/hotplug, so if userspace has taken over the
hotplug events, it can switch that off and get everything from the
netlink events.

> All that probably needs to be changed to fit in with what Jon is doing;
> the idea here is that we want to notify the user, in the desktop
> session, that the firmware/initialization needed is not available. Then
> the user can take appropriate action. 

Hmm, we will need a sane way to propagate errors form the kernel to
userspace anyway. Currently there is nothing like that available and I
really see the need for a more generic solution then reading dmesg. :)

Remember the block I/O errors we wanted to send over netlink. We need
something more generic here, that is able to relay classes of errors and
error-data to userspace. That errors would be nice to catch with HAL and
give them a sane context to be handled in userspace.

Everything else will just be another dirty hack, I think.

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 11:47 -0500, Jon Smirl wrote:
> On Sun, 20 Mar 2005 14:37:16 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > I still would like to see clearly defined list of requirements for:
> >   o async userspace data-requests into the kernel
> >   o async userspace configuration-requests from the kernel
> 
> Here's another model that might be more general...
> 
> accept_user_data() -- adds the data/status attributes to the device sysfs entry
> 
> request_helper(completion, environment string) -- causes
> ACTION=helper, the environment string is added onto the normal
> environment for an event from this device class. Only async version.
> 
> In the completion routine...
> release_user_data() -- deletes the attributes, frees the data
> 
> This model addresses both the need to receive data and run helpers
> that don't need data.

Ok, let's collect what's need to get that:

o Emit events for devices to request a userspace-action on that
  device like copying data into a sysfs file or run a userspace
  program to setup this device.
  -Is this limited to physical devices?

o Events can happen anytime, no only on device creation time.

o We need an efficient way for the dumb hotplug-multiplexer to recognize
  that kind of events to prevent the execution of just another script
  for every hotplug event.

o The events should act asynchronously and the kernel should detect or is
  to be notified that the request has finished.
  -Do we need any timeout here?

o For data-loading-requests like the firmware case we need to be able to
  request more than one file for one single device.

o The DEVPATH of the request should be the device itself, that asks for data
  or setup.
  -Should the requests create a own child at the device directory or just
   attributes there?
  -How should attributes be named if we have multiple request for the
   same device?

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Darren Salt]

I demand that Jon Smirl may or may not have written...

> On Sun, 20 Mar 2005 14:37:16 +0100, Kay Sievers <kay.sievers@vrfy.org>
> wrote:
>> What is the concept of a "post"? Does it receive data for the kernel to
>> use? What is the overlap with firmware loading here? You just need to run
>> a program from userspace, right?

> Post is the commonly used term for what you do to hardware when it is first
> turned on to get it running. For example the system BIOS posts all the
> devices in the system.

No. That's POST. "Post" is what comes through the letterbox. ;-)

[snip]
-- 
| Darren Salt | nr. Ashington, | d youmustbejoking,demon,co,uk
| Debian,     | Northumberland | s zap,tartarus,org
| RISC OS     | Toon Army      | @
|   Retrocomputing: a PC card in a Risc PC

Build a system even a fool can use, and only a fool will use it.


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 18:39 +0100, Kay Sievers wrote:
> On Sun, 2005-03-20 at 12:19 -0500, David Zeuthen wrote:
> > On Sun, 2005-03-20 at 14:37 +0100, Kay Sievers wrote:
> > > before we start hacking on it. The current request_firmware() is a
> > > not-so-nice example for doing it that way. All that stuff needs to play
> > > with hotplug, initramfs/bootup, suspend/resume. And we should come up
> > > with something that fits _all_ the needs and move the current
> > > request_firmware-users over to it.
> > 
> > On that topic, one of the things I'd like a future scheme to do is not
> > only emitting the request_firmware() event, but also emitting events
> > like someone_uploaded_firmware() and timeout_waiting_for_firmware().
> > Plus some state in sysfs to keep track of the state. All but
> > request_firmware() probably don't need hotplug events; kobject_uevents
> > would be fine for me, since I'd want some daemon to listen to it anyway.
> 
> We need to cover the early boot and initramfs anyway and therefore need
> to fork something here. But that will be tunneled
> through /proc/sys/kernel/hotplug, so if userspace has taken over the
> hotplug events, it can switch that off and get everything from the
> netlink events.
> 
> > All that probably needs to be changed to fit in with what Jon is doing;
> > the idea here is that we want to notify the user, in the desktop
> > session, that the firmware/initialization needed is not available. Then
> > the user can take appropriate action. 
> 
> Hmm, we will need a sane way to propagate errors form the kernel to
> userspace anyway. Currently there is nothing like that available and I
> really see the need for a more generic solution then reading dmesg. :)
> 
> Remember the block I/O errors we wanted to send over netlink. We need
> something more generic here, that is able to relay classes of errors and
> error-data to userspace. That errors would be nice to catch with HAL and
> give them a sane context to be handled in userspace.
> 
> Everything else will just be another dirty hack, I think.

Last year Dan Stekloff started: "A list of actionable error events that
includes the current message string, possible causes, and possible
actions..."

  http://linux-diag.sourceforge.net/first_failure/FirstFailure.html

We may work on something like this and add support for it to HAL. Any
ideas to let that happen or just start with something "small", but in
that direction?

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Sun, 20 Mar 2005 18:52:12 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> o Emit events for devices to request a userspace-action on that
>   device like copying data into a sysfs file or run a userspace
>   program to setup this device.
>   -Is this limited to physical devices?

Do virtual devices have a 'struct device' too? It could be generalized
even more to work off from a kobject instead of device.

> o Events can happen anytime, not only on device creation time.

true, I also have a problem with monitor hotplug needing to run a user
space helper.

> o We need an efficient way for the dumb hotplug-multiplexer to recognize
>   that kind of events to prevent the execution of just another script
>   for every hotplug event.
> 
> o The events should act asynchronously and the kernel should detect or is
>   to be notified that the request has finished.
>   -Do we need any timeout here?

Timeout may the source of a problem. If user space is messed up and
the event doesn't get run correctly the timeout happens. Timeout
removes the attributes and reports an error. Now the user fixes
whatever was wrong in user space. How do they restart the event?

It might be better to get rid of the timeouts and just leave
everything in place. Then you could just rerun the user space app that
failed.

> o For data-loading-requests like the firmware case we need to be able to
>   request more than one file for one single device.

I would just do these sequentially. If everything is working the loads
happen quickly.

> o The DEVPATH of the request should be the device itself, that asks for data
>   or setup.
>   -Should the requests create a own child at the device directory or just
>    attributes there?

Doesn't creating a directory cause a hotplug add event? There are only
two attributes: status and data. We could just pick standard names for
them. Alternatively those two attributes could always be present in
the device directory reporting a status of inactive.

>   -How should attributes be named if we have multiple request for the
>    same device?

I would not allow this

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Sun, 20 Mar 2005 12:19:38 -0500, David Zeuthen <david@fubar.dk> wrote:
> 
> Hey Kay,
> 
> On Sun, 2005-03-20 at 14:37 +0100, Kay Sievers wrote:
> > before we start hacking on it. The current request_firmware() is a
> > not-so-nice example for doing it that way. All that stuff needs to play
> > with hotplug, initramfs/bootup, suspend/resume. And we should come up
> > with something that fits _all_ the needs and move the current
> > request_firmware-users over to it.
> 
> On that topic, one of the things I'd like a future scheme to do is not
> only emitting the request_firmware() event, but also emitting events
> like someone_uploaded_firmware() and timeout_waiting_for_firmware().
> Plus some state in sysfs to keep track of the state. All but
> request_firmware() probably don't need hotplug events; kobject_uevents
> would be fine for me, since I'd want some daemon to listen to it anyway.
> 
> All that probably needs to be changed to fit in with what Jon is doing;
> the idea here is that we want to notify the user, in the desktop
> session, that the firmware/initialization needed is not available. Then
> the user can take appropriate action.

Instead of deleting the attributes and returning to the user at
timeout expiration, maybe we should leave the attributes in place and
generate an event that the use can see. Hopefully the user will see
this event and fix the problem. By leaving the attributes in place the
task can be restarted without mucking with the driver.


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 13:24 -0500, Jon Smirl wrote:
> On Sun, 20 Mar 2005 18:52:12 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > o Emit events for devices to request a userspace-action on that
> >   device like copying data into a sysfs file or run a userspace
> >   program to setup this device.
> >   -Is this limited to physical devices?
> 
> Do virtual devices have a 'struct device' too? It could be generalized
> even more to work off from a kobject instead of device.

No, class devices have only a pointer to struct device.

> > o Events can happen anytime, not only on device creation time.
> 
> true, I also have a problem with monitor hotplug needing to run a user
> space helper.
> 
> > o We need an efficient way for the dumb hotplug-multiplexer to recognize
> >   that kind of events to prevent the execution of just another script
> >   for every hotplug event.
> > 
> > o The events should act asynchronously and the kernel should detect or is
> >   to be notified that the request has finished.
> >   -Do we need any timeout here?
> 
> Timeout may the source of a problem. If user space is messed up and
> the event doesn't get run correctly the timeout happens. Timeout
> removes the attributes and reports an error. Now the user fixes
> whatever was wrong in user space. How do they restart the event?
> 
> It might be better to get rid of the timeouts and just leave
> everything in place. Then you could just rerun the user space app that
> failed.

How can we remove the module with a waiting request? Cancel the request
it sysfs first?

And yes, good question: How do we trigger a new init if it has failed
the first time? Seems we need the userspace driver binding stuff. Greg?

> > o For data-loading-requests like the firmware case we need to be able to
> >   request more than one file for one single device.
> 
> I would just do these sequentially. If everything is working the loads
> happen quickly.
> 
> > o The DEVPATH of the request should be the device itself, that asks for data
> >   or setup.
> >   -Should the requests create a own child at the device directory or just
> >    attributes there?
> 
> Doesn't creating a directory cause a hotplug add event?

You can easily prevent that with a hotplug-filter.

> There are only
> two attributes: status and data. We could just pick standard names for
> them. Alternatively those two attributes could always be present in
> the device directory reporting a status of inactive.

It would be nice to get the state of the request-waiting device at any
time from sysfs and not only with the environment of the hotplug event.

> >   -How should attributes be named if we have multiple request for the
> >    same device?
> 
> I would not allow this

Hmm, if we never need this, that's fine. But we need to make sure before
we implement it. Otherwise we will run into problems and start
everything again. :)

If we create child devices below the requesting object, we would get our
own subsystem parameter and /sbin/hotplug can act only on that and does
not need to filter all events...
Attributes at the device itself may look easier in the first place, but
we need to check that carefully to cover the requirements.

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 14:02 -0500, Jon Smirl wrote:
> On Sun, 20 Mar 2005 12:19:38 -0500, David Zeuthen <david@fubar.dk> wrote:
> > 
> > Hey Kay,
> > 
> > On Sun, 2005-03-20 at 14:37 +0100, Kay Sievers wrote:
> > > before we start hacking on it. The current request_firmware() is a
> > > not-so-nice example for doing it that way. All that stuff needs to play
> > > with hotplug, initramfs/bootup, suspend/resume. And we should come up
> > > with something that fits _all_ the needs and move the current
> > > request_firmware-users over to it.
> > 
> > On that topic, one of the things I'd like a future scheme to do is not
> > only emitting the request_firmware() event, but also emitting events
> > like someone_uploaded_firmware() and timeout_waiting_for_firmware().
> > Plus some state in sysfs to keep track of the state. All but
> > request_firmware() probably don't need hotplug events; kobject_uevents
> > would be fine for me, since I'd want some daemon to listen to it anyway.
> > 
> > All that probably needs to be changed to fit in with what Jon is doing;
> > the idea here is that we want to notify the user, in the desktop
> > session, that the firmware/initialization needed is not available. Then
> > the user can take appropriate action.
> 
> Instead of deleting the attributes and returning to the user at
> timeout expiration, maybe we should leave the attributes in place and
> generate an event that the use can see.

But if the config event does not know how to fulfill the request, it may
cancel the operation by triggering the sysfs attribute. And that will
lead to a failing device initialization which is an event that userspace
is interested in.
Again, the way such errors are send to userspace in a sane way, needs to
be solved for this to happen ...

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Sun, 20 Mar 2005 20:27:24 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> But if the config event does not know how to fulfill the request, it may
> cancel the operation by triggering the sysfs attribute. And that will
> lead to a failing device initialization which is an event that userspace
> is interested in.
> Again, the way such errors are send to userspace in a sane way, needs to
> be solved for this to happen ...

I was going to remove the cancel operation. Userspace would only be
able to signal: completed or restart.

Status and user_data would be permanent attributes on the device. I
added a field to 'struct device' to track the progress of the loading.
If the driver is removed and there is a pending kernel thread waiting,
the device layer has enough info to get rid of it.

status=0 offline
status=1 on-line
status=2 waiting for user space action
status=3 user action complete

Setting status=2 would clear the data and get ready to receive new data
Setting  status=3 indicates that load/action is complete.
User space can't set states 0/1

A timeout would leave everything as is, but generate a user readable
message to fix things.
Exiting the module would remove all pending threads, cleanup, and set status=0.

Userspace could possibly set a state=4 which would reset the data load
and regenerate the event. All that need to be remember is the
environment variable string.


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 14:50 -0500, Jon Smirl wrote:
> On Sun, 20 Mar 2005 20:27:24 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > But if the config event does not know how to fulfill the request, it may
> > cancel the operation by triggering the sysfs attribute. And that will
> > lead to a failing device initialization which is an event that userspace
> > is interested in.
> > Again, the way such errors are send to userspace in a sane way, needs to
> > be solved for this to happen ...
> 
> I was going to remove the cancel operation. Userspace would only be
> able to signal: completed or restart.

If I don't have the requested data to copy to the driver. What should I
do?

> Status and user_data would be permanent attributes on the device.

If the driver needs 2 requests to set up and something goes wrong: Which
one should I load into the attribute now? I'm lost lost with that
layout.

> I added a field to 'struct device' to track the progress of the loading.

Could you please define the requirements for that subsystem _first_ and
make sure that everything we currently know of is covered before adding
stuff to the core. I'm pretty sure that this will have to change several
times before we get that right.

> If the driver is removed and there is a pending kernel thread waiting,
> the device layer has enough info to get rid of it.

How do you remove the busy driver?

> status=0 offline
> status=1 on-line
> status=2 waiting for user space action
> status=3 user action complete
> 
> Setting status=2 would clear the data and get ready to receive new data
> Setting  status=3 indicates that load/action is complete.
> User space can't set states 0/1
> 
> A timeout would leave everything as is, but generate a user readable
> message to fix things.

Why should we do that? If userspace can not fulfill the request, why
should we keep the kernel state around?

> Exiting the module would remove all pending threads, cleanup, and set status=0.

Yeah, sounds very much like the current oops'ing firmware_class. :)

> Userspace could possibly set a state=4 which would reset the data load
> and regenerate the event. All that need to be remember is the
> environment variable string.

That's the problem. Who remembers what?

How do we handle devices which are not fully registered until they got
the request handled successfully?

Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Sun, 20 Mar 2005 21:25:17 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> Could you please define the requirements for that subsystem _first_ and
> make sure that everything we currently know of is covered before adding
> stuff to the core. I'm pretty sure that this will have to change several
> times before we get that right.

The base device support is missing two things I need:
1) some mechanism to run the POST program
2) subdirectories under class_device for hotplug monitors

I don't know enough about the details of how the hotplug subsystem
works, plus I have a lot of work I still need to do the framebuffer
subsystem.  I'll work around these items until a fix is in place.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Sun, 2005-03-20 at 15:39 -0500, Jon Smirl wrote:
> On Sun, 20 Mar 2005 21:25:17 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > Could you please define the requirements for that subsystem _first_ and
> > make sure that everything we currently know of is covered before adding
> > stuff to the core. I'm pretty sure that this will have to change several
> > times before we get that right.
> 
> The base device support is missing two things I need:
> 1) some mechanism to run the POST program

I've put that on the list below.

> 2) subdirectories under class_device for hotplug monitors

Childs of class devices is somewhere on Greg's list, I think. :)
Don't know how close we are to allow that. Greg?

> I don't know enough about the details of how the hotplug subsystem
> works, plus I have a lot of work I still need to do the framebuffer
> subsystem.  I'll work around these items until a fix is in place.

We want:
====
 o To request data(file) like firmware or setup data from userspace
   into kernel memory, to use it in the device driver.
 o To request the execution of a userspace program to setup the device
   for the kernel while the kernel waits for the program to complete.

We currently have:
=========
 o call_usermodehelper()
 o request_firmware()
 o kobject_hotplug() executes /sbin/hotplug

Current problems:
======== o call_usermodehelper will not work if userspace is not ready to fulfill
   the request at bootup or we are in suspend/resume.
 o Same for request_firmware, it has a async version nobody uses,
   but it may be good starting point. I have seen several oops's of
   request_firmware if the timeout tries to cancel the request.
 o Both versions can not be replayed without the content of the original
   hotplug environment.
 o If a request fails and the module can not be removed, how can we
   probe the device again?

Requirements:
====== o Provide core functionality to request a userspace action for a
   specific device.
 o The events should be send through /sbin/hotplug and we get the
   netlink events along with the same call.
 o The request should be able to wait as long as the system/userspace
   needs to fulfill the request or actively cancels it.
 o It should be possible to fulfill the request later, without
   the hotplug environment available.
 o userspace should be notified about any failing operation to react
   accordingly.
 o Be able to request more than one action for a single device:
   one after the other or in parallel, but with its current state
   visible to userspace.
 o We need a way to let /sbin/hotplug recognize the kind of event,
   to avoid running an agent for every device.

Questions:
=====
 o How do we reprobe a device if we can't unload the module?
 o From which DEVPATH should the requests originate from?
   The module itself in sysfs, the physical device, the class
   device? Or stick with with our own temporary class device
   for the requests?
 o Do we need any timeout? If yes, how is that to be handled?
 o Will all users/probers be able to schedule its work
   until the request comes back from userspace? 

Possible implementations:
============1. We keep the current class_device logic for loading firmware but
   switch everything over to the async version without the timeout.
   And we add new functionality for the configuration requests.

2. Drivers they want to emit requests create attributes in the devices
   sysfs representation and send out hotplug events with a specific
   action.

3. We add a child for every request below the device that requests
   the actual action. This child device has the attributes to control
   the request.


Any ideas?

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Linas Vepstas]

Hi,

One minor, possibly irritating comment:

On Sat, Mar 19, 2005 at 11:06:59PM -0500, Jon Smirl was heard to remark:
> 2) New event KOBJ_POST. $POST and

Isn't the standard usage BIST these days, instead of POST?

Its not power-on self test if you run the built-in self-test 
long after its been powered on. ... 

Besides, there's a namespace collision: "post" usually means "after",
which is confusing.  "bist" doesn't have any other meanings.

--linas




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Mon, 2005-03-21 at 03:37 +0100, Kay Sievers wrote:
> On Sun, 2005-03-20 at 15:39 -0500, Jon Smirl wrote:
> > On Sun, 20 Mar 2005 21:25:17 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > > Could you please define the requirements for that subsystem _first_ and
> > > make sure that everything we currently know of is covered before adding
> > > stuff to the core. I'm pretty sure that this will have to change several
> > > times before we get that right.
> > 
> > The base device support is missing two things I need:
> > 1) some mechanism to run the POST program

> We want:
> ====
>  o To request data(file) like firmware or setup data from userspace
>    into kernel memory, to use it in the device driver.
>  o To request the execution of a userspace program to setup the device
>    for the kernel while the kernel waits for the program to complete.

Continuing working on the firmware_class requirement definition I
have another idea to check out:

Ben already pointed out the problems with the current firmware loader
during power management state transitions, which causes deadlock
situations with the kernel executed helpers while userspace
is not able to handle that.

What about moving the call_usermodehelper users over to some kind of
kobject_hotplug() call?
We would convert everything to the current hotplug logic and dispatch
all events from here. All events would originate from a sane DEVPATH
and if userspace is running after bootup, it can disable the execution
of /sbin/hotplug and listens for events only on the netlink socket.

If the hotplug handling daemon (a version of udevd already does this)
can not respond to netlink messages, the kernel will queue the events
without any further control needed. If the daemon wakes up again, the
kernel delivers all queued event-packets over the socket.

For the firmware and config-request-events we would need some kind of
async hotplug transaction:
Drivers would request a transaction that must be acknowledged from the
hotplug helper for the driver to continue. For these transactions we
could use a model similar to the current firmware_class.
Every transaction creates a directory in sysfs that contains attributes
with everything needed to handle the request. If userspace has finished
handling the event, it signifies that in with a sysfs attribute and the
driver can continue its work.

This way we can replay or cancel events at any time if something goes
wrong. We would get a generic transaction model for the kernel to
request a configuration action from userspace like copying a firmware
image, setup a device or run any other userspace helper.

How does that sound? Can this work?

Thanks,
Kay




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Tue, 22 Mar 2005 03:25:05 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> This way we can replay or cancel events at any time if something goes
> wrong. We would get a generic transaction model for the kernel to
> request a configuration action from userspace like copying a firmware
> image, setup a device or run any other userspace helper.
> 
> How does that sound? Can this work?

That's what I was trying to get at. It's the abort of the event that
causes problems. The better model is that the event stays active until
it is satisfied -- there is no way to automatically abort it. If the
event has not been satisfied by module exit time it just gets cleaned
up.

It would be convenient to have a sysfs variable on an event like
'replay'. If I set 'replay' to 1 the event will get regenerated to
/sbin/hotplug. After I fix things  I could manually retrigger the
event and it would run in the hotplug execution environment. You might
not even need to be root to trigger a replay.

Timeout is still useful. Timeout tells the system to generate a user
message saying that the device is stuck. But timeout should not abort
the event.

What about a universal driver model for this? Right now we have
probe(). Should we add post()? If post() is null just call probe(). If
the driver has a post() function call it instead. The completion event
would then call probe(). probe() could still fail and the driver would
not be activated.

Doing it in the driver layer makes this process standard. Now we can
add a uniform device attribute like status. status=offline,
status=posting, status=online.

You could probably cancel an event. Canceling an event would remove it
from sysfs and make sure probe() never gets called. Status would stay
at offline. The only way to reactivate the device would be to remove
the driver. I don't think we need cancel but if it is there it would
be a user controlled action.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Roman Kagan]

On Mon, Mar 21, 2005 at 10:06:30PM -0500, Jon Smirl wrote:
> You could probably cancel an event. Canceling an event would remove it
> from sysfs and make sure probe() never gets called. Status would stay
> at offline. The only way to reactivate the device would be to remove
> the driver.

This looks like defeating the whole purpose of the discussed changes.
Unloading the driver has undesired side effects, like unbinding other
devices handled by the same driver, and takeover of the device by
another matching driver.

I suspect most of the problems you are trying to solve can be worked
around by a generic interface to bind / unbind a driver to a device.  If
probe() fails, the device is left unbound, just like it is now.  The
user fixes the problem and causes the driver to bind to the device (ala
ln -s /sys/path/to/driver /sys/path/to/device, hehe) and execute probe()
again.  This model already sort of works for USB via USBDEVFS_CONNECT /
USBDEVFS_DISCONNECT usbfs ioctls. Dunno if it covers everything power
management needs, though.

Roman.


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Mon, 2005-03-21 at 22:06 -0500, Jon Smirl wrote:
> On Tue, 22 Mar 2005 03:25:05 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > This way we can replay or cancel events at any time if something goes
> > wrong. We would get a generic transaction model for the kernel to
> > request a configuration action from userspace like copying a firmware
> > image, setup a device or run any other userspace helper.
> > 
> > How does that sound? Can this work?
> 
> That's what I was trying to get at. It's the abort of the event that
> causes problems. The better model is that the event stays active until
> it is satisfied -- there is no way to automatically abort it. If the
> event has not been satisfied by module exit time it just gets cleaned
> up.

You are lost with that model. How do yo handle more than one device? If
you have one device already using the module, how do you cancel a
request for the other one? I don't see how this can work.

> It would be convenient to have a sysfs variable on an event like
> 'replay'. If I set 'replay' to 1 the event will get regenerated to
> /sbin/hotplug. After I fix things  I could manually retrigger the
> event and it would run in the hotplug execution environment. You might
> not even need to be root to trigger a replay.

I'm more for representing the necessary information with sysfs
attributes. So userspace can retry to fulfill the request or abort it.
And the same process that replays the transaction, can trigger the
requested action or cancel it.

> Timeout is still useful. Timeout tells the system to generate a user
> message saying that the device is stuck. But timeout should not abort
> the event.

I don't see the need for a timeout. The goal is to get rid of the
timeouts, right? We never know what time userspace is able to fulfill
the requests. We just need a way that events don't get lost and let
userspace handle that at _any_ time it is running again. And timeouts
that do nothing but send messages are not needed, I think.

> What about a universal driver model for this? Right now we have
> probe(). Should we add post()? If post() is null just call probe(). If
> the driver has a post() function call it instead. The completion event
> would then call probe(). probe() could still fail and the driver would
> not be activated.

As already discussed in earlier mails, I don't see what this will give
us. Why should we code that into the driver probing? We just need a
generic userspace transaction a driver can request at any time, right?

> Doing it in the driver layer makes this process standard. Now we can
> add a uniform device attribute like status. status=offline,
> status=posting, status=online.

How does that help? If we get a way to bind and unbind devices from a
driver through sysfs, why would we need that.

> You could probably cancel an event. Canceling an event would remove it
> from sysfs and make sure probe() never gets called. Status would stay
> at offline. The only way to reactivate the device would be to remove
> the driver. I don't think we need cancel but if it is there it would
> be a user controlled action.

If userspace is not able to fulfill a request it seems logical to be
able to cancel a transaction. If the requested firmware image file is
not available, userspace should cancel the request immediately to leave
the kernel in a clean state and let userspace act on the error.

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Kay Sievers]

On Tue, 2005-03-22 at 11:27 +0300, Roman Kagan wrote:
> On Mon, Mar 21, 2005 at 10:06:30PM -0500, Jon Smirl wrote:
> > You could probably cancel an event. Canceling an event would remove it
> > from sysfs and make sure probe() never gets called. Status would stay
> > at offline. The only way to reactivate the device would be to remove
> > the driver.
> 
> This looks like defeating the whole purpose of the discussed changes.
> Unloading the driver has undesired side effects, like unbinding other
> devices handled by the same driver, and takeover of the device by
> another matching driver.

I agree.

> I suspect most of the problems you are trying to solve can be worked
> around by a generic interface to bind / unbind a driver to a device.  If
> probe() fails, the device is left unbound, just like it is now.

That sounds reasonable and it leaves the kernel in a clean state.

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Tue, 22 Mar 2005 11:45:08 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > That's what I was trying to get at. It's the abort of the event that
> > causes problems. The better model is that the event stays active until
> > it is satisfied -- there is no way to automatically abort it. If the
> > event has not been satisfied by module exit time it just gets cleaned
> > up.
> 
> You are lost with that model. How do yo handle more than one device? If
> you have one device already using the module, how do you cancel a
> request for the other one? I don't see how this can work.

There is one active request per device. The events are hung off the
device nodes in sysfs. If a driver needs multiple events per device
they are serialized. This model lets a single driver handle multiple
devices in parallel.

> > It would be convenient to have a sysfs variable on an event like
> > 'replay'. If I set 'replay' to 1 the event will get regenerated to
> > /sbin/hotplug. After I fix things  I could manually retrigger the
> > event and it would run in the hotplug execution environment. You might
> > not even need to be root to trigger a replay.
> 
> I'm more for representing the necessary information with sysfs
> attributes. So userspace can retry to fulfill the request or abort it.
> And the same process that replays the transaction, can trigger the
> requested action or cancel it.

It doesn't matter if the event replay happens from the kernel or a
user space script that picks the info up out of sysfs variables. But I
do think a replay should look just as if /sbin/hotplug were called
again with the same environment/context.

> > Timeout is still useful. Timeout tells the system to generate a user
> > message saying that the device is stuck. But timeout should not abort
> > the event.
> 
> I don't see the need for a timeout. The goal is to get rid of the
> timeouts, right? We never know what time userspace is able to fulfill
> the requests. We just need a way that events don't get lost and let
> userspace handle that at _any_ time it is running again. And timeouts
> that do nothing but send messages are not needed, I think.

Timeout does nothing except generate a hal/dbus event that alerts the
user that manual intervention is required.

> > What about a universal driver model for this? Right now we have
> > probe(). Should we add post()? If post() is null just call probe(). If
> > the driver has a post() function call it instead. The completion event
> > would then call probe(). probe() could still fail and the driver would
> > not be activated.
> 
> As already discussed in earlier mails, I don't see what this will give
> us. Why should we code that into the driver probing? We just need a
> generic userspace transaction a driver can request at any time, right?

probe() returns T/F if the driver can handle the device. For video
cards I need a user space event to complete before I know the answer
T/F.  I have two choices, trigger an async event from probe() and
always return T, or do the event synchronously (like request_firmware)
and deadlock suspend. If I always return T later info from the event
may have made we want to return F.

> > Doing it in the driver layer makes this process standard. Now we can
> > add a uniform device attribute like status. status=offline,
> > status=posting, status=online.
> 
> How does that help? If we get a way to bind and unbind devices from a
> driver through sysfs, why would we need that.
> 
> > You could probably cancel an event. Canceling an event would remove it
> > from sysfs and make sure probe() never gets called. Status would stay
> > at offline. The only way to reactivate the device would be to remove
> > the driver. I don't think we need cancel but if it is there it would
> > be a user controlled action.
> 
> If userspace is not able to fulfill a request it seems logical to be
> able to cancel a transaction. If the requested firmware image file is
> not available, userspace should cancel the request immediately to leave
> the kernel in a clean state and let userspace act on the error.

Say I have a single driver that is controlling two hardware instances.
One instance is working fine. The other gets an event error. If the
event is canceled the only way to get it back is to stop the other
device. Alternatively, if the event just hangs around until module
unload time I can fix it whenever I figure out how to. What does
cancel really gain? The downside is that you can be forced to restart
a working device.

> 
> Thanks,
> Kay
> 

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Linas Vepstas]

On Sun, Mar 20, 2005 at 02:50:04PM -0500, Jon Smirl was heard to remark:
> 
> status=0 offline
> status=1 on-line
> status=2 waiting for user space action
> status=3 user action complete
> 
> Setting  status=3 indicates that load/action is complete.

What are the valid state transitions from 3 to 2 or 1? 
Who would change the 3 into a 1 or 2 (or 0)? 

--linas


-------------------------------------------------------
This SF.net email is sponsored by: 2005 Windows Mobile Application Contest
Submit applications for Windows Mobile(tm)-based Pocket PCs or Smartphones
for the chance to win $25,000 and application distribution. Enter today at
http://ads.osdn.com/?ad_idh82&alloc_id\x15148&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Linas Vepstas]

On Tue, Mar 22, 2005 at 11:27:33AM +0300, Roman Kagan was heard to remark:
> Unloading the driver has undesired side effects, like unbinding other
> devices handled by the same driver, and takeover of the device by
> another matching driver.

Heh. Unhappiness ensues if the device is e.g. a disk controller,
and there is a mounted file system on top of it. Current unix semantics
don't allow a post-facto unmount if the block device under it
disappears.

This is a bit off-topic, but we are having a discussion on the PCI
mailing list about how to auto-recover from PCI bus errors.  Currently,
(and this is very arch & driver specific) drivers do things like
post/bist and probe again to recover, and get back to a reasonable
working state (without unbinding the driver).

Any interest in disucussing this from a generic bus viewpoint?
Or would this be a distraction at this point?

--linas


-------------------------------------------------------
This SF.net email is sponsored by: 2005 Windows Mobile Application Contest
Submit applications for Windows Mobile(tm)-based Pocket PCs or Smartphones
for the chance to win $25,000 and application distribution. Enter today at
http://ads.osdn.com/?ad_idh82&alloc_id\x15148&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Jon Smirl]

On Tue, 22 Mar 2005 11:53:55 -0600, Linas Vepstas <linas@austin.ibm.com> wrote:
> On Sun, Mar 20, 2005 at 02:50:04PM -0500, Jon Smirl was heard to remark:
> >
> > status=0 offline
> > status=1 on-line
> > status=2 waiting for user space action
> > status=3 user action complete
> >
> > Setting  status=3 indicates that load/action is complete.
> 
> What are the valid state transitions from 3 to 2 or 1?

3-2 would be if the driver requested another user space action without
finishing probe
3-1 probe completes and returns true
3-0 probe completes and returns false


> Who would change the 3 into a 1 or 2 (or 0)?
> 
> --linas
> 


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
This SF.net email is sponsored by: 2005 Windows Mobile Application Contest
Submit applications for Windows Mobile(tm)-based Pocket PCs or Smartphones
for the chance to win $25,000 and application distribution. Enter today at
http://ads.osdn.com/?ad_idh82&alloc_id\x15148&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: Rework of request firmware

[Greg KH]

On Mon, Mar 21, 2005 at 03:37:00AM +0100, Kay Sievers wrote:
> On Sun, 2005-03-20 at 15:39 -0500, Jon Smirl wrote:
> > On Sun, 20 Mar 2005 21:25:17 +0100, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > > Could you please define the requirements for that subsystem _first_ and
> > > make sure that everything we currently know of is covered before adding
> > > stuff to the core. I'm pretty sure that this will have to change several
> > > times before we get that right.
> > 
> > The base device support is missing two things I need:
> > 1) some mechanism to run the POST program
> 
> I've put that on the list below.
> 
> > 2) subdirectories under class_device for hotplug monitors
> 
> Childs of class devices is somewhere on Greg's list, I think. :)
> Don't know how close we are to allow that. Greg?

Closer than we were last week, now that I have a lot of the class rework
done :)

thanks,

greg k-h
> Questions:
> =====
>  o How do we reprobe a device if we can't unload the module?

Kick it from userspace.  See pci for an example of how to do this.

>  o From which DEVPATH should the requests originate from?
>    The module itself in sysfs, the physical device, the class
>    device? Or stick with with our own temporary class device
>    for the requests?

I think the "physical device" or whatever it is represented by in the
sysfs device tree.

>  o Do we need any timeout? If yes, how is that to be handled?

Yes.  Don't know.

>  o Will all users/probers be able to schedule its work
>    until the request comes back from userspace? 

They had better :)

thanks,

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: 2005 Windows Mobile Application Contest
Submit applications for Windows Mobile(tm)-based Pocket PCs or Smartphones
for the chance to win $25,000 and application distribution. Enter today at
http://ads.osdn.com/?ad_idh82&alloc_id\x15148&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel