LKML Archive mirror
 help / color / mirror / Atom feed
* Implement class_device_update_dev() function
@ 2006-07-06 22:59 Marcel Holtmann
  2006-07-06 23:57 ` Greg KH
  0 siblings, 1 reply; 9+ messages in thread
From: Marcel Holtmann @ 2006-07-06 22:59 UTC (permalink / raw
  To: Greg KH; +Cc: linux-kernel

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

Hi Greg,

for the Bluetooth subsystem integration into the driver model it is
required that we can update the device of a class device at any time.

For the RFCOMM TTY device for example we create the TTY device and only
when it got opened we create the Bluetooth connection. Once this new
connection has been created we have a device to attach to the class
device of the TTY.

I came up with the attached patch and it worked fine with the Bluetooth
RFCOMM layer.

Regards

Marcel


[-- Attachment #2: patch-class-device-update-dev --]
[-- Type: text/plain, Size: 5841 bytes --]

[PATCH] Allow dynamically changing the device of a class device

This patch implements the class_device_update_dev() function which
allows to change the device of a class device after its creation.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit 0a887bfed61fe16e47a57f11144ef830eef86c5f
tree 0c520a89cdef0260d2dd5ed9aa60debac3e3cee9
parent 120bda20c6f64b32e8bfbdd7b34feafaa5f5332e
author Marcel Holtmann <marcel@holtmann.org> Fri, 07 Jul 2006 00:44:41 +0200
committer Marcel Holtmann <marcel@holtmann.org> Fri, 07 Jul 2006 00:44:41 +0200

 drivers/base/class.c   |   93 ++++++++++++++++++++++++++++++++++--------------
 include/linux/device.h |    1 +
 2 files changed, 66 insertions(+), 28 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index de89083..33956ae 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -485,6 +485,43 @@ static void class_device_remove_groups(s
 	}
 }
 
+static int class_device_add_dev(struct class_device *class_dev)
+{
+	char *class_name = NULL;
+	int error = 0;
+
+	if (class_dev->dev) {
+		class_name = make_class_name(class_dev->class->name,
+					     &class_dev->kobj);
+		error = sysfs_create_link(&class_dev->kobj,
+					  &class_dev->dev->kobj, "device");
+		if (error)
+			goto out;
+
+		error = sysfs_create_link(&class_dev->dev->kobj,
+					  &class_dev->kobj, class_name);
+		if (error)
+			sysfs_remove_link(&class_dev->kobj, "device");
+	}
+
+ out:
+	kfree(class_name);
+	return error;
+}
+
+static void class_device_remove_dev(struct class_device *class_dev)
+{
+	char *class_name;
+
+	if (class_dev->dev) {
+		class_name = make_class_name(class_dev->class->name,
+					     &class_dev->kobj);
+		sysfs_remove_link(&class_dev->kobj, "device");
+		sysfs_remove_link(&class_dev->dev->kobj, class_name);
+		kfree(class_name);
+	}
+}
+
 static ssize_t show_dev(struct class_device *class_dev, char *buf)
 {
 	return print_dev_t(buf, class_dev->devt);
@@ -526,7 +563,6 @@ int class_device_add(struct class_device
 	struct class *parent_class = NULL;
 	struct class_device *parent_class_dev = NULL;
 	struct class_interface *class_intf;
-	char *class_name = NULL;
 	int error = -EINVAL;
 
 	class_dev = class_device_get(class_dev);
@@ -593,22 +629,13 @@ int class_device_add(struct class_device
 	if (error)
 		goto out5;
 
-	if (class_dev->dev) {
-		class_name = make_class_name(class_dev->class->name,
-					     &class_dev->kobj);
-		error = sysfs_create_link(&class_dev->kobj,
-					  &class_dev->dev->kobj, "device");
-		if (error)
-			goto out6;
-		error = sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
-					  class_name);
-		if (error)
-			goto out7;
-	}
+	error = class_device_add_dev(class_dev);
+	if (error)
+		goto out6;
 
 	error = class_device_add_groups(class_dev);
 	if (error)
-		goto out8;
+		goto out7;
 
 	kobject_uevent(&class_dev->kobj, KOBJ_ADD);
 
@@ -623,12 +650,8 @@ int class_device_add(struct class_device
 
 	goto out1;
 
- out8:
-	if (class_dev->dev)
-		sysfs_remove_link(&class_dev->kobj, class_name);
  out7:
-	if (class_dev->dev)
-		sysfs_remove_link(&class_dev->kobj, "device");
+	class_device_remove_dev(class_dev);
  out6:
 	class_device_remove_attrs(class_dev);
  out5:
@@ -644,7 +667,6 @@ int class_device_add(struct class_device
 	class_put(parent_class);
  out1:
 	class_device_put(class_dev);
-	kfree(class_name);
 	return error;
 }
 
@@ -720,7 +742,6 @@ void class_device_del(struct class_devic
 	struct class *parent_class = class_dev->class;
 	struct class_device *parent_device = class_dev->parent;
 	struct class_interface *class_intf;
-	char *class_name = NULL;
 
 	if (parent_class) {
 		down(&parent_class->sem);
@@ -731,12 +752,7 @@ void class_device_del(struct class_devic
 		up(&parent_class->sem);
 	}
 
-	if (class_dev->dev) {
-		class_name = make_class_name(class_dev->class->name,
-					     &class_dev->kobj);
-		sysfs_remove_link(&class_dev->kobj, "device");
-		sysfs_remove_link(&class_dev->dev->kobj, class_name);
-	}
+	class_device_remove_dev(class_dev);
 	sysfs_remove_link(&class_dev->kobj, "subsystem");
 	class_device_remove_file(class_dev, &class_dev->uevent_attr);
 	if (class_dev->devt_attr)
@@ -749,7 +765,6 @@ void class_device_del(struct class_devic
 
 	class_device_put(parent_device);
 	class_put(parent_class);
-	kfree(class_name);
 }
 
 void class_device_unregister(struct class_device *class_dev)
@@ -821,6 +836,27 @@ int class_device_rename(struct class_dev
 	return error;
 }
 
+int class_device_update_dev(struct class_device *class_dev, struct device *dev)
+{
+	int error = 0;
+
+	class_dev = class_device_get(class_dev);
+	if (!class_dev)
+		return -EINVAL;
+
+	if (class_dev->dev != dev) {
+		class_device_remove_dev(class_dev);
+
+		class_dev->dev = dev;
+
+		error = class_device_add_dev(class_dev);
+	}
+
+	class_device_put(class_dev);
+
+	return error;
+}
+
 struct class_device * class_device_get(struct class_device *class_dev)
 {
 	if (class_dev)
@@ -911,6 +947,7 @@ EXPORT_SYMBOL_GPL(class_device_get);
 EXPORT_SYMBOL_GPL(class_device_put);
 EXPORT_SYMBOL_GPL(class_device_create);
 EXPORT_SYMBOL_GPL(class_device_destroy);
+EXPORT_SYMBOL_GPL(class_device_update_dev);
 EXPORT_SYMBOL_GPL(class_device_create_file);
 EXPORT_SYMBOL_GPL(class_device_remove_file);
 EXPORT_SYMBOL_GPL(class_device_create_bin_file);
diff --git a/include/linux/device.h b/include/linux/device.h
index 1e5f30d..d76072b 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -249,6 +249,7 @@ extern int class_device_add(struct class
 extern void class_device_del(struct class_device *);
 
 extern int class_device_rename(struct class_device *, char *);
+extern int class_device_update_dev(struct class_device *, struct device *);
 
 extern struct class_device * class_device_get(struct class_device *);
 extern void class_device_put(struct class_device *);

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-06 22:59 Implement class_device_update_dev() function Marcel Holtmann
@ 2006-07-06 23:57 ` Greg KH
  2006-07-07  7:42   ` Marcel Holtmann
  0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2006-07-06 23:57 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: linux-kernel

On Fri, Jul 07, 2006 at 12:59:52AM +0200, Marcel Holtmann wrote:
> Hi Greg,
> 
> for the Bluetooth subsystem integration into the driver model it is
> required that we can update the device of a class device at any time.

You can?  Ick.

That messes with my "get rid of struct class_device" plans a bit...

> For the RFCOMM TTY device for example we create the TTY device and only
> when it got opened we create the Bluetooth connection. Once this new
> connection has been created we have a device to attach to the class
> device of the TTY.
> 
> I came up with the attached patch and it worked fine with the Bluetooth
> RFCOMM layer.

But userspace should also find out about this change, and this patch
prevents that from happening.  What about just tearing down the class
device and creating a new one?  That way userspace knows about the new
linkage properly, and any device naming and permission issues can be
handled anew?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-06 23:57 ` Greg KH
@ 2006-07-07  7:42   ` Marcel Holtmann
  2006-07-08  0:26     ` Kay Sievers
  2006-07-11 23:18     ` Greg KH
  0 siblings, 2 replies; 9+ messages in thread
From: Marcel Holtmann @ 2006-07-07  7:42 UTC (permalink / raw
  To: Greg KH; +Cc: linux-kernel

Hi Greg,

> > for the Bluetooth subsystem integration into the driver model it is
> > required that we can update the device of a class device at any time.
> 
> You can?  Ick.
> 
> That messes with my "get rid of struct class_device" plans a bit...

this must not be a class device, but at the moment TTY, network and
input are still class devices. The Bluetooth subsystem moved away from
using class devices.

> > For the RFCOMM TTY device for example we create the TTY device and only
> > when it got opened we create the Bluetooth connection. Once this new
> > connection has been created we have a device to attach to the class
> > device of the TTY.
> > 
> > I came up with the attached patch and it worked fine with the Bluetooth
> > RFCOMM layer.
> 
> But userspace should also find out about this change, and this patch
> prevents that from happening.  What about just tearing down the class
> device and creating a new one?  That way userspace knows about the new
> linkage properly, and any device naming and permission issues can be
> handled anew?

This won't work for Bluetooth. We create the TTY and its class device
with tty_register_device() and then the device node is present. Then at
some point later we open that device and the Bluetooth connection gets
established. Only when the connection has been established we know the
device that represents it. So tearing down the class device and creating
a new one will screw up the application that is using this device node.

Would reissuing the uevent of the class device help here?

Regards

Marcel



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-07  7:42   ` Marcel Holtmann
@ 2006-07-08  0:26     ` Kay Sievers
  2006-07-08  9:27       ` Marcel Holtmann
  2006-07-11 23:18     ` Greg KH
  1 sibling, 1 reply; 9+ messages in thread
From: Kay Sievers @ 2006-07-08  0:26 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: Greg KH, linux-kernel

On Fri, 2006-07-07 at 09:42 +0200, Marcel Holtmann wrote:
> Hi Greg,
> 
> > > for the Bluetooth subsystem integration into the driver model it is
> > > required that we can update the device of a class device at any time.
> > 
> > You can?  Ick.
> > 
> > That messes with my "get rid of struct class_device" plans a bit...
> 
> this must not be a class device, but at the moment TTY, network and
> input are still class devices. The Bluetooth subsystem moved away from
> using class devices.
> 
> > > For the RFCOMM TTY device for example we create the TTY device and only
> > > when it got opened we create the Bluetooth connection. Once this new
> > > connection has been created we have a device to attach to the class
> > > device of the TTY.
> > > 
> > > I came up with the attached patch and it worked fine with the Bluetooth
> > > RFCOMM layer.
> > 
> > But userspace should also find out about this change, and this patch
> > prevents that from happening.  What about just tearing down the class
> > device and creating a new one?  That way userspace knows about the new
> > linkage properly, and any device naming and permission issues can be
> > handled anew?
> 
> This won't work for Bluetooth. We create the TTY and its class device
> with tty_register_device() and then the device node is present. Then at
> some point later we open that device and the Bluetooth connection gets
> established. Only when the connection has been established we know the
> device that represents it. So tearing down the class device and creating
> a new one will screw up the application that is using this device node.
> 
> Would reissuing the uevent of the class device help here?

How about KOBJ_ONLINE/OFFLINE?

Thanks,
Kay


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-08  0:26     ` Kay Sievers
@ 2006-07-08  9:27       ` Marcel Holtmann
  2006-07-08 13:00         ` Kay Sievers
  0 siblings, 1 reply; 9+ messages in thread
From: Marcel Holtmann @ 2006-07-08  9:27 UTC (permalink / raw
  To: Kay Sievers; +Cc: Greg KH, linux-kernel

Hi Kay,

> > > But userspace should also find out about this change, and this patch
> > > prevents that from happening.  What about just tearing down the class
> > > device and creating a new one?  That way userspace knows about the new
> > > linkage properly, and any device naming and permission issues can be
> > > handled anew?
> > 
> > This won't work for Bluetooth. We create the TTY and its class device
> > with tty_register_device() and then the device node is present. Then at
> > some point later we open that device and the Bluetooth connection gets
> > established. Only when the connection has been established we know the
> > device that represents it. So tearing down the class device and creating
> > a new one will screw up the application that is using this device node.
> > 
> > Would reissuing the uevent of the class device help here?
> 
> How about KOBJ_ONLINE/OFFLINE?

I am not that familiar with the internals of kobject. Can you give me an
example on how to do that?

Regards

Marcel



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-08  9:27       ` Marcel Holtmann
@ 2006-07-08 13:00         ` Kay Sievers
  2006-07-08 13:28           ` Marcel Holtmann
  0 siblings, 1 reply; 9+ messages in thread
From: Kay Sievers @ 2006-07-08 13:00 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: Greg KH, linux-kernel

On Sat, 2006-07-08 at 11:27 +0200, Marcel Holtmann wrote:
> > > > But userspace should also find out about this change, and this patch
> > > > prevents that from happening.  What about just tearing down the class
> > > > device and creating a new one?  That way userspace knows about the new
> > > > linkage properly, and any device naming and permission issues can be
> > > > handled anew?
> > > 
> > > This won't work for Bluetooth. We create the TTY and its class device
> > > with tty_register_device() and then the device node is present. Then at
> > > some point later we open that device and the Bluetooth connection gets
> > > established. Only when the connection has been established we know the
> > > device that represents it. So tearing down the class device and creating
> > > a new one will screw up the application that is using this device node.
> > > 
> > > Would reissuing the uevent of the class device help here?
> > 
> > How about KOBJ_ONLINE/OFFLINE?
> 
> I am not that familiar with the internals of kobject. Can you give me an
> example on how to do that?

Just send another event (but not add or remove), for the already created
object. CPU hotplug uses ONLINE/OFFLINE, and we also use it to get
notified when the device mapper table is set up (not upstream). Udev is
able to update symlinks, or run actions on "online" events if asked
for. 

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;h=52674323b14da5724bfdc4ffee830609f116d248;hb=HEAD;f=drivers/acpi/processor_core.c#l714


Kay


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-08 13:00         ` Kay Sievers
@ 2006-07-08 13:28           ` Marcel Holtmann
  2006-07-08 17:27             ` Marcel Holtmann
  0 siblings, 1 reply; 9+ messages in thread
From: Marcel Holtmann @ 2006-07-08 13:28 UTC (permalink / raw
  To: Kay Sievers; +Cc: Greg KH, linux-kernel

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

Hi Kay,

> > > > > But userspace should also find out about this change, and this patch
> > > > > prevents that from happening.  What about just tearing down the class
> > > > > device and creating a new one?  That way userspace knows about the new
> > > > > linkage properly, and any device naming and permission issues can be
> > > > > handled anew?
> > > > 
> > > > This won't work for Bluetooth. We create the TTY and its class device
> > > > with tty_register_device() and then the device node is present. Then at
> > > > some point later we open that device and the Bluetooth connection gets
> > > > established. Only when the connection has been established we know the
> > > > device that represents it. So tearing down the class device and creating
> > > > a new one will screw up the application that is using this device node.
> > > > 
> > > > Would reissuing the uevent of the class device help here?
> > > 
> > > How about KOBJ_ONLINE/OFFLINE?
> > 
> > I am not that familiar with the internals of kobject. Can you give me an
> > example on how to do that?
> 
> Just send another event (but not add or remove), for the already created
> object. CPU hotplug uses ONLINE/OFFLINE, and we also use it to get
> notified when the device mapper table is set up (not upstream). Udev is
> able to update symlinks, or run actions on "online" events if asked
> for. 

the attached patch sends ONLINE when a device has been attached to a
class device and OFFLINE when it has been removed.

Regards

Marcel


[-- Attachment #2: patch-class-device-update-dev --]
[-- Type: text/plain, Size: 5964 bytes --]

[PATCH] Allow dynamically changing the device of a class device

This patch implements the class_device_update_dev() function which
allows to change the device of a class device after its creation.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit 97372f657bbb68ee1c4119bb21642fc700317ce3
tree bd46b493dd75b09515897eb0d12b2fd30a234e3c
parent 120bda20c6f64b32e8bfbdd7b34feafaa5f5332e
author Marcel Holtmann <marcel@holtmann.org> Sat, 08 Jul 2006 15:24:15 +0200
committer Marcel Holtmann <marcel@holtmann.org> Sat, 08 Jul 2006 15:24:15 +0200

 drivers/base/class.c   |   98 ++++++++++++++++++++++++++++++++++--------------
 include/linux/device.h |    1 
 2 files changed, 71 insertions(+), 28 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index de89083..1789a6b 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -485,6 +485,48 @@ static void class_device_remove_groups(s
 	}
 }
 
+static int class_device_add_dev(struct class_device *class_dev)
+{
+	char *class_name = NULL;
+	int error = 0;
+
+	if (class_dev->dev) {
+		class_name = make_class_name(class_dev->class->name,
+					     &class_dev->kobj);
+		error = sysfs_create_link(&class_dev->kobj,
+					  &class_dev->dev->kobj, "device");
+		if (error)
+			goto out;
+
+		error = sysfs_create_link(&class_dev->dev->kobj,
+					  &class_dev->kobj, class_name);
+		if (error) {
+			sysfs_remove_link(&class_dev->kobj, "device");
+			goto out;
+		}
+
+		kobject_uevent(&class_dev->kobj, KOBJ_ONLINE);
+	}
+
+ out:
+	kfree(class_name);
+	return error;
+}
+
+static void class_device_remove_dev(struct class_device *class_dev)
+{
+	char *class_name;
+
+	if (class_dev->dev) {
+		class_name = make_class_name(class_dev->class->name,
+					     &class_dev->kobj);
+		sysfs_remove_link(&class_dev->kobj, "device");
+		sysfs_remove_link(&class_dev->dev->kobj, class_name);
+		kobject_uevent(&class_dev->kobj, KOBJ_OFFLINE);
+		kfree(class_name);
+	}
+}
+
 static ssize_t show_dev(struct class_device *class_dev, char *buf)
 {
 	return print_dev_t(buf, class_dev->devt);
@@ -526,7 +568,6 @@ int class_device_add(struct class_device
 	struct class *parent_class = NULL;
 	struct class_device *parent_class_dev = NULL;
 	struct class_interface *class_intf;
-	char *class_name = NULL;
 	int error = -EINVAL;
 
 	class_dev = class_device_get(class_dev);
@@ -593,22 +634,13 @@ int class_device_add(struct class_device
 	if (error)
 		goto out5;
 
-	if (class_dev->dev) {
-		class_name = make_class_name(class_dev->class->name,
-					     &class_dev->kobj);
-		error = sysfs_create_link(&class_dev->kobj,
-					  &class_dev->dev->kobj, "device");
-		if (error)
-			goto out6;
-		error = sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
-					  class_name);
-		if (error)
-			goto out7;
-	}
+	error = class_device_add_dev(class_dev);
+	if (error)
+		goto out6;
 
 	error = class_device_add_groups(class_dev);
 	if (error)
-		goto out8;
+		goto out7;
 
 	kobject_uevent(&class_dev->kobj, KOBJ_ADD);
 
@@ -623,12 +655,8 @@ int class_device_add(struct class_device
 
 	goto out1;
 
- out8:
-	if (class_dev->dev)
-		sysfs_remove_link(&class_dev->kobj, class_name);
  out7:
-	if (class_dev->dev)
-		sysfs_remove_link(&class_dev->kobj, "device");
+	class_device_remove_dev(class_dev);
  out6:
 	class_device_remove_attrs(class_dev);
  out5:
@@ -644,7 +672,6 @@ int class_device_add(struct class_device
 	class_put(parent_class);
  out1:
 	class_device_put(class_dev);
-	kfree(class_name);
 	return error;
 }
 
@@ -720,7 +747,6 @@ void class_device_del(struct class_devic
 	struct class *parent_class = class_dev->class;
 	struct class_device *parent_device = class_dev->parent;
 	struct class_interface *class_intf;
-	char *class_name = NULL;
 
 	if (parent_class) {
 		down(&parent_class->sem);
@@ -731,12 +757,7 @@ void class_device_del(struct class_devic
 		up(&parent_class->sem);
 	}
 
-	if (class_dev->dev) {
-		class_name = make_class_name(class_dev->class->name,
-					     &class_dev->kobj);
-		sysfs_remove_link(&class_dev->kobj, "device");
-		sysfs_remove_link(&class_dev->dev->kobj, class_name);
-	}
+	class_device_remove_dev(class_dev);
 	sysfs_remove_link(&class_dev->kobj, "subsystem");
 	class_device_remove_file(class_dev, &class_dev->uevent_attr);
 	if (class_dev->devt_attr)
@@ -749,7 +770,6 @@ void class_device_del(struct class_devic
 
 	class_device_put(parent_device);
 	class_put(parent_class);
-	kfree(class_name);
 }
 
 void class_device_unregister(struct class_device *class_dev)
@@ -821,6 +841,27 @@ int class_device_rename(struct class_dev
 	return error;
 }
 
+int class_device_update_dev(struct class_device *class_dev, struct device *dev)
+{
+	int error = 0;
+
+	class_dev = class_device_get(class_dev);
+	if (!class_dev)
+		return -EINVAL;
+
+	if (class_dev->dev != dev) {
+		class_device_remove_dev(class_dev);
+
+		class_dev->dev = dev;
+
+		error = class_device_add_dev(class_dev);
+	}
+
+	class_device_put(class_dev);
+
+	return error;
+}
+
 struct class_device * class_device_get(struct class_device *class_dev)
 {
 	if (class_dev)
@@ -911,6 +952,7 @@ EXPORT_SYMBOL_GPL(class_device_get);
 EXPORT_SYMBOL_GPL(class_device_put);
 EXPORT_SYMBOL_GPL(class_device_create);
 EXPORT_SYMBOL_GPL(class_device_destroy);
+EXPORT_SYMBOL_GPL(class_device_update_dev);
 EXPORT_SYMBOL_GPL(class_device_create_file);
 EXPORT_SYMBOL_GPL(class_device_remove_file);
 EXPORT_SYMBOL_GPL(class_device_create_bin_file);
diff --git a/include/linux/device.h b/include/linux/device.h
index 1e5f30d..d76072b 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -249,6 +249,7 @@ extern int class_device_add(struct class
 extern void class_device_del(struct class_device *);
 
 extern int class_device_rename(struct class_device *, char *);
+extern int class_device_update_dev(struct class_device *, struct device *);
 
 extern struct class_device * class_device_get(struct class_device *);
 extern void class_device_put(struct class_device *);

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-08 13:28           ` Marcel Holtmann
@ 2006-07-08 17:27             ` Marcel Holtmann
  0 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2006-07-08 17:27 UTC (permalink / raw
  To: Kay Sievers; +Cc: Greg KH, linux-kernel

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

Hi Kay,

> > > > > > But userspace should also find out about this change, and this patch
> > > > > > prevents that from happening.  What about just tearing down the class
> > > > > > device and creating a new one?  That way userspace knows about the new
> > > > > > linkage properly, and any device naming and permission issues can be
> > > > > > handled anew?
> > > > > 
> > > > > This won't work for Bluetooth. We create the TTY and its class device
> > > > > with tty_register_device() and then the device node is present. Then at
> > > > > some point later we open that device and the Bluetooth connection gets
> > > > > established. Only when the connection has been established we know the
> > > > > device that represents it. So tearing down the class device and creating
> > > > > a new one will screw up the application that is using this device node.
> > > > > 
> > > > > Would reissuing the uevent of the class device help here?
> > > > 
> > > > How about KOBJ_ONLINE/OFFLINE?
> > > 
> > > I am not that familiar with the internals of kobject. Can you give me an
> > > example on how to do that?
> > 
> > Just send another event (but not add or remove), for the already created
> > object. CPU hotplug uses ONLINE/OFFLINE, and we also use it to get
> > notified when the device mapper table is set up (not upstream). Udev is
> > able to update symlinks, or run actions on "online" events if asked
> > for. 
> 
> the attached patch sends ONLINE when a device has been attached to a
> class device and OFFLINE when it has been removed.

the order of ADD and ONLINE was wrong in some situations and also uevent
forgot to send out the extra ONLINE when a device is attached. The
attached patch fixes all this stuff. Any comments?

Regards

Marcel


[-- Attachment #2: patch-class-device-update-dev --]
[-- Type: text/plain, Size: 6739 bytes --]

[PATCH] Allow dynamically changing the device of a class device

This patch implements the class_device_update_dev() function which
allows to change the device of a class device after its creation.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit efdb7efa5ffbbc6096ce776b909b235f70b23802
tree db15b1182a0ba7620335f474d31398c950947130
parent 120bda20c6f64b32e8bfbdd7b34feafaa5f5332e
author Marcel Holtmann <marcel@holtmann.org> Sat, 08 Jul 2006 18:59:55 +0200
committer Marcel Holtmann <marcel@holtmann.org> Sat, 08 Jul 2006 18:59:55 +0200

 drivers/base/class.c   |  111 ++++++++++++++++++++++++++++++++++++------------
 include/linux/device.h |    1 
 2 files changed, 84 insertions(+), 28 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index de89083..866f91b 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -485,6 +485,43 @@ static void class_device_remove_groups(s
 	}
 }
 
+static int class_device_add_dev(struct class_device *class_dev)
+{
+	char *class_name = NULL;
+	int error = 0;
+
+	if (class_dev->dev) {
+		class_name = make_class_name(class_dev->class->name,
+					     &class_dev->kobj);
+		error = sysfs_create_link(&class_dev->kobj,
+					  &class_dev->dev->kobj, "device");
+		if (error)
+			goto out;
+
+		error = sysfs_create_link(&class_dev->dev->kobj,
+					  &class_dev->kobj, class_name);
+		if (error)
+			sysfs_remove_link(&class_dev->kobj, "device");
+	}
+
+ out:
+	kfree(class_name);
+	return error;
+}
+
+static void class_device_remove_dev(struct class_device *class_dev)
+{
+	char *class_name;
+
+	if (class_dev->dev) {
+		class_name = make_class_name(class_dev->class->name,
+					     &class_dev->kobj);
+		sysfs_remove_link(&class_dev->kobj, "device");
+		sysfs_remove_link(&class_dev->dev->kobj, class_name);
+		kfree(class_name);
+	}
+}
+
 static ssize_t show_dev(struct class_device *class_dev, char *buf)
 {
 	return print_dev_t(buf, class_dev->devt);
@@ -494,6 +531,10 @@ static ssize_t store_uevent(struct class
 			    const char *buf, size_t count)
 {
 	kobject_uevent(&class_dev->kobj, KOBJ_ADD);
+
+	if (class_dev->dev)
+		kobject_uevent(&class_dev->kobj, KOBJ_ONLINE);
+
 	return count;
 }
 
@@ -526,7 +567,6 @@ int class_device_add(struct class_device
 	struct class *parent_class = NULL;
 	struct class_device *parent_class_dev = NULL;
 	struct class_interface *class_intf;
-	char *class_name = NULL;
 	int error = -EINVAL;
 
 	class_dev = class_device_get(class_dev);
@@ -593,25 +633,19 @@ int class_device_add(struct class_device
 	if (error)
 		goto out5;
 
-	if (class_dev->dev) {
-		class_name = make_class_name(class_dev->class->name,
-					     &class_dev->kobj);
-		error = sysfs_create_link(&class_dev->kobj,
-					  &class_dev->dev->kobj, "device");
-		if (error)
-			goto out6;
-		error = sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
-					  class_name);
-		if (error)
-			goto out7;
-	}
+	error = class_device_add_dev(class_dev);
+	if (error)
+		goto out6;
 
 	error = class_device_add_groups(class_dev);
 	if (error)
-		goto out8;
+		goto out7;
 
 	kobject_uevent(&class_dev->kobj, KOBJ_ADD);
 
+	if (class_dev->dev)
+		kobject_uevent(&class_dev->kobj, KOBJ_ONLINE);
+
 	/* notify any interfaces this device is now here */
 	down(&parent_class->sem);
 	list_add_tail(&class_dev->node, &parent_class->children);
@@ -623,12 +657,8 @@ int class_device_add(struct class_device
 
 	goto out1;
 
- out8:
-	if (class_dev->dev)
-		sysfs_remove_link(&class_dev->kobj, class_name);
  out7:
-	if (class_dev->dev)
-		sysfs_remove_link(&class_dev->kobj, "device");
+	class_device_remove_dev(class_dev);
  out6:
 	class_device_remove_attrs(class_dev);
  out5:
@@ -644,7 +674,6 @@ int class_device_add(struct class_device
 	class_put(parent_class);
  out1:
 	class_device_put(class_dev);
-	kfree(class_name);
 	return error;
 }
 
@@ -720,7 +749,6 @@ void class_device_del(struct class_devic
 	struct class *parent_class = class_dev->class;
 	struct class_device *parent_device = class_dev->parent;
 	struct class_interface *class_intf;
-	char *class_name = NULL;
 
 	if (parent_class) {
 		down(&parent_class->sem);
@@ -731,12 +759,7 @@ void class_device_del(struct class_devic
 		up(&parent_class->sem);
 	}
 
-	if (class_dev->dev) {
-		class_name = make_class_name(class_dev->class->name,
-					     &class_dev->kobj);
-		sysfs_remove_link(&class_dev->kobj, "device");
-		sysfs_remove_link(&class_dev->dev->kobj, class_name);
-	}
+	class_device_remove_dev(class_dev);
 	sysfs_remove_link(&class_dev->kobj, "subsystem");
 	class_device_remove_file(class_dev, &class_dev->uevent_attr);
 	if (class_dev->devt_attr)
@@ -744,12 +767,14 @@ void class_device_del(struct class_devic
 	class_device_remove_attrs(class_dev);
 	class_device_remove_groups(class_dev);
 
+	if (class_dev->dev)
+		kobject_uevent(&class_dev->kobj, KOBJ_OFFLINE);
+
 	kobject_uevent(&class_dev->kobj, KOBJ_REMOVE);
 	kobject_del(&class_dev->kobj);
 
 	class_device_put(parent_device);
 	class_put(parent_class);
-	kfree(class_name);
 }
 
 void class_device_unregister(struct class_device *class_dev)
@@ -821,6 +846,35 @@ int class_device_rename(struct class_dev
 	return error;
 }
 
+int class_device_update_dev(struct class_device *class_dev, struct device *dev)
+{
+	int error = 0;
+
+	class_dev = class_device_get(class_dev);
+	if (!class_dev)
+		return -EINVAL;
+
+	if (class_dev->dev != dev) {
+		if (class_dev->dev)
+			kobject_uevent(&class_dev->kobj, KOBJ_OFFLINE);
+
+		class_device_remove_dev(class_dev);
+
+		class_dev->dev = dev;
+
+		error = class_device_add_dev(class_dev);
+		if (error)
+			class_dev->dev = NULL;
+
+		if (class_dev->dev)
+			kobject_uevent(&class_dev->kobj, KOBJ_ONLINE);
+	}
+
+	class_device_put(class_dev);
+
+	return error;
+}
+
 struct class_device * class_device_get(struct class_device *class_dev)
 {
 	if (class_dev)
@@ -911,6 +965,7 @@ EXPORT_SYMBOL_GPL(class_device_get);
 EXPORT_SYMBOL_GPL(class_device_put);
 EXPORT_SYMBOL_GPL(class_device_create);
 EXPORT_SYMBOL_GPL(class_device_destroy);
+EXPORT_SYMBOL_GPL(class_device_update_dev);
 EXPORT_SYMBOL_GPL(class_device_create_file);
 EXPORT_SYMBOL_GPL(class_device_remove_file);
 EXPORT_SYMBOL_GPL(class_device_create_bin_file);
diff --git a/include/linux/device.h b/include/linux/device.h
index 1e5f30d..d76072b 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -249,6 +249,7 @@ extern int class_device_add(struct class
 extern void class_device_del(struct class_device *);
 
 extern int class_device_rename(struct class_device *, char *);
+extern int class_device_update_dev(struct class_device *, struct device *);
 
 extern struct class_device * class_device_get(struct class_device *);
 extern void class_device_put(struct class_device *);

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Implement class_device_update_dev() function
  2006-07-07  7:42   ` Marcel Holtmann
  2006-07-08  0:26     ` Kay Sievers
@ 2006-07-11 23:18     ` Greg KH
  1 sibling, 0 replies; 9+ messages in thread
From: Greg KH @ 2006-07-11 23:18 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: linux-kernel

On Fri, Jul 07, 2006 at 09:42:32AM +0200, Marcel Holtmann wrote:
> Hi Greg,
> 
> > > for the Bluetooth subsystem integration into the driver model it is
> > > required that we can update the device of a class device at any time.
> > 
> > You can?  Ick.
> > 
> > That messes with my "get rid of struct class_device" plans a bit...
> 
> this must not be a class device, but at the moment TTY, network and
> input are still class devices. The Bluetooth subsystem moved away from
> using class devices.
> 
> > > For the RFCOMM TTY device for example we create the TTY device and only
> > > when it got opened we create the Bluetooth connection. Once this new
> > > connection has been created we have a device to attach to the class
> > > device of the TTY.
> > > 
> > > I came up with the attached patch and it worked fine with the Bluetooth
> > > RFCOMM layer.
> > 
> > But userspace should also find out about this change, and this patch
> > prevents that from happening.  What about just tearing down the class
> > device and creating a new one?  That way userspace knows about the new
> > linkage properly, and any device naming and permission issues can be
> > handled anew?
> 
> This won't work for Bluetooth. We create the TTY and its class device
> with tty_register_device() and then the device node is present. Then at
> some point later we open that device and the Bluetooth connection gets
> established. Only when the connection has been established we know the
> device that represents it. So tearing down the class device and creating
> a new one will screw up the application that is using this device node.

So to start with, you have a class device with no attached "struct
device"?  And then after opening the device node, you want to create the
symlink?

> Would reissuing the uevent of the class device help here?

Not really, because as you state, we aren't changing the class device
itself, and throwing another uevent for it would probably just confuse
userspace :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2006-07-11 23:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-06 22:59 Implement class_device_update_dev() function Marcel Holtmann
2006-07-06 23:57 ` Greg KH
2006-07-07  7:42   ` Marcel Holtmann
2006-07-08  0:26     ` Kay Sievers
2006-07-08  9:27       ` Marcel Holtmann
2006-07-08 13:00         ` Kay Sievers
2006-07-08 13:28           ` Marcel Holtmann
2006-07-08 17:27             ` Marcel Holtmann
2006-07-11 23:18     ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).