[PATCH] Create PNP device attributes via dev_attrs field of struct device

[PATCH] Create PNP device attributes via dev_attrs field of struct device

[Drew Moseley]

Hello,

I have seen an issue where the sysfs entries for a PNP device are
created nonatomically resulting in a race condition with freedesktop
HAL.  The patch below is my first attempt at addressing this by creating
the device attributes as default bus attributes and allowing the
device_register() call to create them.  Any nasty side effects I am not
addressing?  Comments?

Regards,
Drew Moseley


Signed-off-by: Drew Moseley <dmoseley@mvista.com>
diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
index 9fd7bb9..b07787f 100644
--- a/drivers/pnp/base.h
+++ b/drivers/pnp/base.h
@@ -16,7 +16,7 @@ struct pnp_card *pnp_alloc_card(struct pnp_protocol *,
int id, char *pnpid);

 int pnp_add_device(struct pnp_dev *dev);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
-int pnp_interface_attach_device(struct pnp_dev *dev);
+void pnp_interface_attach_device(struct pnp_dev *dev);

 int pnp_add_card(struct pnp_card *card);
 void pnp_remove_card(struct pnp_card *card);
diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
index a411582..bcd49ba 100644
--- a/drivers/pnp/core.c
+++ b/drivers/pnp/core.c
@@ -159,8 +159,6 @@ struct pnp_dev *pnp_alloc_dev(struct pnp_protocol
*protocol, int id, char *pnpid

 int __pnp_add_device(struct pnp_dev *dev)
 {
-	int ret;
-
 	pnp_fixup_device(dev);
 	dev->status = PNP_READY;
 	spin_lock(&pnp_lock);
@@ -168,12 +166,8 @@ int __pnp_add_device(struct pnp_dev *dev)
 	list_add_tail(&dev->protocol_list, &dev->protocol->devices);
 	spin_unlock(&pnp_lock);

-	ret = device_register(&dev->dev);
-	if (ret)
-		return ret;
-
 	pnp_interface_attach_device(dev);
-	return 0;
+	return device_register(&dev->dev);
 }

 /*
diff --git a/drivers/pnp/interface.c b/drivers/pnp/interface.c
index a876ecf..442684d 100644
--- a/drivers/pnp/interface.c
+++ b/drivers/pnp/interface.c
@@ -243,8 +243,6 @@ static ssize_t pnp_show_options(struct device *dmdev,
 	return ret;
 }

-static DEVICE_ATTR(options, S_IRUGO, pnp_show_options, NULL);
-
 static ssize_t pnp_show_current_resources(struct device *dmdev,
 					  struct device_attribute *attr,
 					  char *buf)
@@ -420,9 +418,6 @@ done:
 	return count;
 }

-static DEVICE_ATTR(resources, S_IRUGO | S_IWUSR,
-		   pnp_show_current_resources, pnp_set_current_resources);
-
 static ssize_t pnp_show_current_ids(struct device *dmdev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -437,27 +432,16 @@ static ssize_t pnp_show_current_ids(struct device
*dmdev,
 	return (str - buf);
 }

-static DEVICE_ATTR(id, S_IRUGO, pnp_show_current_ids, NULL);
+static struct device_attribute pnp_interface_attrs[] = {
+	__ATTR(resources, S_IRUGO | S_IWUSR,
+		   pnp_show_current_resources,
+		   pnp_set_current_resources),
+	__ATTR(options, S_IRUGO, pnp_show_options, NULL),
+	__ATTR(id, S_IRUGO, pnp_show_current_ids, NULL),
+	__ATTR_NULL,
+};

-int pnp_interface_attach_device(struct pnp_dev *dev)
+void pnp_interface_attach_device(struct pnp_dev *dev)
 {
-	int rc = device_create_file(&dev->dev, &dev_attr_options);
-
-	if (rc)
-		goto err;
-	rc = device_create_file(&dev->dev, &dev_attr_resources);
-	if (rc)
-		goto err_opt;
-	rc = device_create_file(&dev->dev, &dev_attr_id);
-	if (rc)
-		goto err_res;
-
-	return 0;
-
-err_res:
-	device_remove_file(&dev->dev, &dev_attr_resources);
-err_opt:
-	device_remove_file(&dev->dev, &dev_attr_options);
-err:
-	return rc;
+	dev->dev.bus->dev_attrs = pnp_interface_attrs;
 }

Re: [PATCH] Create PNP device attributes via dev_attrs field of struct device

[Kay Sievers]

On Tue, Sep 23, 2008 at 14:24, Drew Moseley <dmoseley@mvista.com> wrote:
> I have seen an issue where the sysfs entries for a PNP device are
> created nonatomically resulting in a race condition with freedesktop
> HAL.  The patch below is my first attempt at addressing this by creating
> the device attributes as default bus attributes and allowing the
> device_register() call to create them.  Any nasty side effects I am not
> addressing?  Comments?

> diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
> index 9fd7bb9..b07787f 100644
> --- a/drivers/pnp/base.h
> +++ b/drivers/pnp/base.h
> @@ -16,7 +16,7 @@ struct pnp_card *pnp_alloc_card(struct pnp_protocol *,
> int id, char *pnpid);
>
>  int pnp_add_device(struct pnp_dev *dev);
>  struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
> -int pnp_interface_attach_device(struct pnp_dev *dev);
> +void pnp_interface_attach_device(struct pnp_dev *dev);
>
>  int pnp_add_card(struct pnp_card *card);
>  void pnp_remove_card(struct pnp_card *card);
> diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
> index a411582..bcd49ba 100644
> --- a/drivers/pnp/core.c
> +++ b/drivers/pnp/core.c
> @@ -159,8 +159,6 @@ struct pnp_dev *pnp_alloc_dev(struct pnp_protocol
> *protocol, int id, char *pnpid
>
>  int __pnp_add_device(struct pnp_dev *dev)
>  {
> -       int ret;
> -
>        pnp_fixup_device(dev);
>        dev->status = PNP_READY;
>        spin_lock(&pnp_lock);
> @@ -168,12 +166,8 @@ int __pnp_add_device(struct pnp_dev *dev)
>        list_add_tail(&dev->protocol_list, &dev->protocol->devices);
>        spin_unlock(&pnp_lock);
>
> -       ret = device_register(&dev->dev);
> -       if (ret)
> -               return ret;
> -
>        pnp_interface_attach_device(dev);
> -       return 0;
> +       return device_register(&dev->dev);
>  }
>
>  /*
> diff --git a/drivers/pnp/interface.c b/drivers/pnp/interface.c
> index a876ecf..442684d 100644
> --- a/drivers/pnp/interface.c
> +++ b/drivers/pnp/interface.c
> @@ -243,8 +243,6 @@ static ssize_t pnp_show_options(struct device *dmdev,
>        return ret;
>  }
>
> -static DEVICE_ATTR(options, S_IRUGO, pnp_show_options, NULL);
> -
>  static ssize_t pnp_show_current_resources(struct device *dmdev,
>                                          struct device_attribute *attr,
>                                          char *buf)
> @@ -420,9 +418,6 @@ done:
>        return count;
>  }
>
> -static DEVICE_ATTR(resources, S_IRUGO | S_IWUSR,
> -                  pnp_show_current_resources, pnp_set_current_resources);
> -
>  static ssize_t pnp_show_current_ids(struct device *dmdev,
>                                    struct device_attribute *attr, char *buf)
>  {
> @@ -437,27 +432,16 @@ static ssize_t pnp_show_current_ids(struct device
> *dmdev,
>        return (str - buf);
>  }
>
> -static DEVICE_ATTR(id, S_IRUGO, pnp_show_current_ids, NULL);
> +static struct device_attribute pnp_interface_attrs[] = {
> +       __ATTR(resources, S_IRUGO | S_IWUSR,
> +                  pnp_show_current_resources,
> +                  pnp_set_current_resources),
> +       __ATTR(options, S_IRUGO, pnp_show_options, NULL),
> +       __ATTR(id, S_IRUGO, pnp_show_current_ids, NULL),
> +       __ATTR_NULL,
> +};
>
> -int pnp_interface_attach_device(struct pnp_dev *dev)
> +void pnp_interface_attach_device(struct pnp_dev *dev)
>  {
> -       int rc = device_create_file(&dev->dev, &dev_attr_options);
> -
> -       if (rc)
> -               goto err;
> -       rc = device_create_file(&dev->dev, &dev_attr_resources);
> -       if (rc)
> -               goto err_opt;
> -       rc = device_create_file(&dev->dev, &dev_attr_id);
> -       if (rc)
> -               goto err_res;
> -
> -       return 0;
> -
> -err_res:
> -       device_remove_file(&dev->dev, &dev_attr_resources);
> -err_opt:
> -       device_remove_file(&dev->dev, &dev_attr_options);
> -err:
> -       return rc;
> +       dev->dev.bus->dev_attrs = pnp_interface_attrs;

Any reason not to assign it statically to pnp_bus_type at in
drivers/pnp/driver.c?

>  }

Thanks,
Kay

Re: [PATCH] Create PNP device attributes via dev_attrs field of struct device

[Drew Moseley]

Kay Sievers wrote:
> 
> Any reason not to assign it statically to pnp_bus_type at in
> drivers/pnp/driver.c?
> 
> Thanks,
> Kay

Not really but since everything in the dev_attrs array is statically
defined in interface.c it was simpler to implement it this way.  To
assign it in driver.c, the array can be made non-static and an extern
added in driver.c, or the array definition can be moved to driver.c and
all the set and show functions be made non-static.  Is there a preference?

Thanks,
Drew

Re: [PATCH] Create PNP device attributes via dev_attrs field of struct device

[Kay Sievers]

On Wed, Sep 24, 2008 at 10:22, Drew Moseley <dmoseley@mvista.com> wrote:
> Kay Sievers wrote:
>>
>> Any reason not to assign it statically to pnp_bus_type at in
>> drivers/pnp/driver.c?

> Not really but since everything in the dev_attrs array is statically
> defined in interface.c it was simpler to implement it this way.  To
> assign it in driver.c, the array can be made non-static and an extern
> added in driver.c, or the array definition can be moved to driver.c and
> all the set and show functions be made non-static.  Is there a preference?

Yeah, it looks weird to mangle the bus_type values from a device
routine, and set the same value again and again with every device
creation. Just declare the array non-static, if moving the code around
between files does not allow a static declaration.

Thanks,
Kay

Re: [PATCH] Create PNP device attributes via dev_attrs field of struct device

[Drew Moseley]

Kay Sievers wrote:
> Yeah, it looks weird to mangle the bus_type values from a device
> routine, and set the same value again and again with every device
> creation. Just declare the array non-static, if moving the code around
> between files does not allow a static declaration.

Updated patch.

Drew

Signed-off-by: Drew Moseley <dmoseley@mvista.com>
Index: linux-2.6.21/drivers/pnp/base.h
===================================================================
--- linux-2.6.21.orig/drivers/pnp/base.h
+++ linux-2.6.21/drivers/pnp/base.h
@@ -1,6 +1,5 @@
 extern spinlock_t pnp_lock;
 void *pnp_alloc(long size);
-int pnp_interface_attach_device(struct pnp_dev *dev);
 void pnp_fixup_device(struct pnp_dev *dev);
 void pnp_free_option(struct pnp_option *option);
 int __pnp_add_device(struct pnp_dev *dev);
Index: linux-2.6.21/drivers/pnp/core.c
===================================================================
--- linux-2.6.21.orig/drivers/pnp/core.c
+++ linux-2.6.21/drivers/pnp/core.c
@@ -111,7 +111,6 @@ static void pnp_release_device(struct de

 int __pnp_add_device(struct pnp_dev *dev)
 {
-	int ret;
 	pnp_fixup_device(dev);
 	dev->dev.bus = &pnp_bus_type;
 	dev->dev.release = &pnp_release_device;
@@ -120,11 +119,7 @@ int __pnp_add_device(struct pnp_dev *dev
 	list_add_tail(&dev->global_list, &pnp_global);
 	list_add_tail(&dev->protocol_list, &dev->protocol->devices);
 	spin_unlock(&pnp_lock);
-
-	ret = device_register(&dev->dev);
-	if (ret == 0)
-		pnp_interface_attach_device(dev);
-	return ret;
+	return device_register(&dev->dev);
 }

 /*
Index: linux-2.6.21/drivers/pnp/interface.c
===================================================================
--- linux-2.6.21.orig/drivers/pnp/interface.c
+++ linux-2.6.21/drivers/pnp/interface.c
@@ -233,9 +233,6 @@ static ssize_t pnp_show_options(struct d
 	return ret;
 }

-static DEVICE_ATTR(options,S_IRUGO,pnp_show_options,NULL);
-
-
 static ssize_t pnp_show_current_resources(struct device *dmdev, struct
device_attribute *attr, char *buf)
 {
 	struct pnp_dev *dev = to_pnp_dev(dmdev);
@@ -441,9 +438,6 @@ pnp_set_current_resources(struct device
 	return count;
 }

-static DEVICE_ATTR(resources,S_IRUGO | S_IWUSR,
-		   pnp_show_current_resources,pnp_set_current_resources);
-
 static ssize_t pnp_show_current_ids(struct device *dmdev, struct
device_attribute *attr, char *buf)
 {
 	char *str = buf;
@@ -457,23 +451,11 @@ static ssize_t pnp_show_current_ids(stru
 	return (str - buf);
 }

-static DEVICE_ATTR(id,S_IRUGO,pnp_show_current_ids,NULL);
-
-int pnp_interface_attach_device(struct pnp_dev *dev)
-{
-	int rc = device_create_file(&dev->dev,&dev_attr_options);
-	if (rc) goto err;
-	rc = device_create_file(&dev->dev,&dev_attr_resources);
-	if (rc) goto err_opt;
-	rc = device_create_file(&dev->dev,&dev_attr_id);
-	if (rc) goto err_res;
-
-	return 0;
-
-err_res:
-	device_remove_file(&dev->dev,&dev_attr_resources);
-err_opt:
-	device_remove_file(&dev->dev,&dev_attr_options);
-err:
-	return rc;
-}
+struct device_attribute pnp_interface_attrs[] = {
+	__ATTR(resources, S_IRUGO | S_IWUSR,
+		   pnp_show_current_resources,
+		   pnp_set_current_resources),
+	__ATTR(options, S_IRUGO, pnp_show_options, NULL),
+	__ATTR(id, S_IRUGO, pnp_show_current_ids, NULL),
+	__ATTR_NULL,
+};
Index: linux-2.6.21/drivers/pnp/driver.c
===================================================================
--- linux-2.6.21.orig/drivers/pnp/driver.c
+++ linux-2.6.21/drivers/pnp/driver.c
@@ -191,6 +191,7 @@ static int pnp_bus_resume(struct device
 	return 0;
 }

+extern struct device_attribute pnp_interface_attrs[];
 struct bus_type pnp_bus_type = {
 	.name	= "pnp",
 	.match	= pnp_bus_match,
@@ -198,6 +199,7 @@ struct bus_type pnp_bus_type = {
 	.remove	= pnp_device_remove,
 	.suspend = pnp_bus_suspend,
 	.resume = pnp_bus_resume,
+	.dev_attrs = pnp_interface_attrs,
 };

 int pnp_register_driver(struct pnp_driver *drv)

Re: [PATCH] Create PNP device attributes via dev_attrs field of struct device

[Kay Sievers]

On Wed, Sep 24, 2008 at 12:37, Drew Moseley <dmoseley@mvista.com> wrote:
> Kay Sievers wrote:
>> Yeah, it looks weird to mangle the bus_type values from a device
>> routine, and set the same value again and again with every device
>> creation. Just declare the array non-static, if moving the code around
>> between files does not allow a static declaration.
>
> Updated patch.

> Signed-off-by: Drew Moseley <dmoseley@mvista.com>
> Index: linux-2.6.21/drivers/pnp/base.h

This seems not to apply. Oh, you are patching a 1.5 years old kernel?
:) Care to redo this for a recent one?

Thanks,
Kay

Re: [PATCH] Create PNP device attributes via dev_attrs field of struct device

[Drew Moseley]

Kay Sievers wrote:
> This seems not to apply. Oh, you are patching a 1.5 years old kernel?
> :) Care to redo this for a recent one?

Ack.  Sorry about that.  Wrong tree.  Here's the right one. ;-)

Signed-off-by: Drew Moseley <dmoseley@mvista.com>
diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
index 9fd7bb9..3532984 100644
--- a/drivers/pnp/base.h
+++ b/drivers/pnp/base.h
@@ -4,6 +4,7 @@
  */

 extern spinlock_t pnp_lock;
+extern struct device_attribute pnp_interface_attrs[];
 void *pnp_alloc(long size);

 int pnp_register_protocol(struct pnp_protocol *protocol);
@@ -16,7 +17,6 @@ struct pnp_card *pnp_alloc_card(struct pnp_protocol *,
int id, char *pnpid);

 int pnp_add_device(struct pnp_dev *dev);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
-int pnp_interface_attach_device(struct pnp_dev *dev);

 int pnp_add_card(struct pnp_card *card);
 void pnp_remove_card(struct pnp_card *card);
diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
index a411582..7d65da8 100644
--- a/drivers/pnp/core.c
+++ b/drivers/pnp/core.c
@@ -159,21 +159,13 @@ struct pnp_dev *pnp_alloc_dev(struct pnp_protocol
*protocol, int id, char *pnpid

 int __pnp_add_device(struct pnp_dev *dev)
 {
-	int ret;
-
 	pnp_fixup_device(dev);
 	dev->status = PNP_READY;
 	spin_lock(&pnp_lock);
 	list_add_tail(&dev->global_list, &pnp_global);
 	list_add_tail(&dev->protocol_list, &dev->protocol->devices);
 	spin_unlock(&pnp_lock);
-
-	ret = device_register(&dev->dev);
-	if (ret)
-		return ret;
-
-	pnp_interface_attach_device(dev);
-	return 0;
+	return device_register(&dev->dev);
 }

 /*
diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index d3f869e..e3f7e89 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -206,6 +206,7 @@ struct bus_type pnp_bus_type = {
 	.remove  = pnp_device_remove,
 	.suspend = pnp_bus_suspend,
 	.resume  = pnp_bus_resume,
+	.dev_attrs = pnp_interface_attrs,
 };

 int pnp_register_driver(struct pnp_driver *drv)
diff --git a/drivers/pnp/interface.c b/drivers/pnp/interface.c
index a876ecf..478a4a7 100644
--- a/drivers/pnp/interface.c
+++ b/drivers/pnp/interface.c
@@ -243,8 +243,6 @@ static ssize_t pnp_show_options(struct device *dmdev,
 	return ret;
 }

-static DEVICE_ATTR(options, S_IRUGO, pnp_show_options, NULL);
-
 static ssize_t pnp_show_current_resources(struct device *dmdev,
 					  struct device_attribute *attr,
 					  char *buf)
@@ -420,9 +418,6 @@ done:
 	return count;
 }

-static DEVICE_ATTR(resources, S_IRUGO | S_IWUSR,
-		   pnp_show_current_resources, pnp_set_current_resources);
-
 static ssize_t pnp_show_current_ids(struct device *dmdev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -437,27 +432,11 @@ static ssize_t pnp_show_current_ids(struct device
*dmdev,
 	return (str - buf);
 }

-static DEVICE_ATTR(id, S_IRUGO, pnp_show_current_ids, NULL);
-
-int pnp_interface_attach_device(struct pnp_dev *dev)
-{
-	int rc = device_create_file(&dev->dev, &dev_attr_options);
-
-	if (rc)
-		goto err;
-	rc = device_create_file(&dev->dev, &dev_attr_resources);
-	if (rc)
-		goto err_opt;
-	rc = device_create_file(&dev->dev, &dev_attr_id);
-	if (rc)
-		goto err_res;
-
-	return 0;
-
-err_res:
-	device_remove_file(&dev->dev, &dev_attr_resources);
-err_opt:
-	device_remove_file(&dev->dev, &dev_attr_options);
-err:
-	return rc;
-}
+struct device_attribute pnp_interface_attrs[] = {
+	__ATTR(resources, S_IRUGO | S_IWUSR,
+		   pnp_show_current_resources,
+		   pnp_set_current_resources),
+	__ATTR(options, S_IRUGO, pnp_show_options, NULL),
+	__ATTR(id, S_IRUGO, pnp_show_current_ids, NULL),
+	__ATTR_NULL,
+};

Re: [PATCH] Create PNP device attributes via dev_attrs field of struct device

[Kay Sievers]

On Thu, 2008-09-25 at 09:42 -0700, Drew Moseley wrote:
> Kay Sievers wrote:
> > This seems not to apply. Oh, you are patching a 1.5 years old kernel?
> > :) Care to redo this for a recent one?
> 
> Ack.  Sorry about that.  Wrong tree.  Here's the right one. ;-)

Looks good to me, and seems to work fine here. Thanks for the patch.

Greg, can you pick up the patch below?

Thanks,
Kay


From: Drew Moseley <dmoseley@mvista.com>
Subject: PNP: create device attributes via default device attributes

This creates the attributes before the uevent is sent.

Signed-off-by: Drew Moseley <dmoseley@mvista.com>
Acked-by: Kay Sievers <kay.sievers@vrfy.org>
---

diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
index 9fd7bb9..3532984 100644
--- a/drivers/pnp/base.h
+++ b/drivers/pnp/base.h
@@ -4,6 +4,7 @@
  */

 extern spinlock_t pnp_lock;
+extern struct device_attribute pnp_interface_attrs[];
 void *pnp_alloc(long size);

 int pnp_register_protocol(struct pnp_protocol *protocol);
@@ -16,7 +17,6 @@ struct pnp_card *pnp_alloc_card(struct pnp_protocol *, int id, char *pnpid);

 int pnp_add_device(struct pnp_dev *dev);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
-int pnp_interface_attach_device(struct pnp_dev *dev);

 int pnp_add_card(struct pnp_card *card);
 void pnp_remove_card(struct pnp_card *card);
diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
index a411582..7d65da8 100644
--- a/drivers/pnp/core.c
+++ b/drivers/pnp/core.c
@@ -159,21 +159,13 @@ struct pnp_dev *pnp_alloc_dev(struct pnp_protocol *protocol, int id, char *pnpid

 int __pnp_add_device(struct pnp_dev *dev)
 {
-	int ret;
-
 	pnp_fixup_device(dev);
 	dev->status = PNP_READY;
 	spin_lock(&pnp_lock);
 	list_add_tail(&dev->global_list, &pnp_global);
 	list_add_tail(&dev->protocol_list, &dev->protocol->devices);
 	spin_unlock(&pnp_lock);
-
-	ret = device_register(&dev->dev);
-	if (ret)
-		return ret;
-
-	pnp_interface_attach_device(dev);
-	return 0;
+	return device_register(&dev->dev);
 }

 /*
diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index d3f869e..e3f7e89 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -206,6 +206,7 @@ struct bus_type pnp_bus_type = {
 	.remove  = pnp_device_remove,
 	.suspend = pnp_bus_suspend,
 	.resume  = pnp_bus_resume,
+	.dev_attrs = pnp_interface_attrs,
 };

 int pnp_register_driver(struct pnp_driver *drv)
diff --git a/drivers/pnp/interface.c b/drivers/pnp/interface.c
index a876ecf..478a4a7 100644
--- a/drivers/pnp/interface.c
+++ b/drivers/pnp/interface.c
@@ -243,8 +243,6 @@ static ssize_t pnp_show_options(struct device *dmdev,
 	return ret;
 }

-static DEVICE_ATTR(options, S_IRUGO, pnp_show_options, NULL);
-
 static ssize_t pnp_show_current_resources(struct device *dmdev,
 					  struct device_attribute *attr,
 					  char *buf)
@@ -420,9 +418,6 @@ done:
 	return count;
 }

-static DEVICE_ATTR(resources, S_IRUGO | S_IWUSR,
-		   pnp_show_current_resources, pnp_set_current_resources);
-
 static ssize_t pnp_show_current_ids(struct device *dmdev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -437,27 +432,11 @@ static ssize_t pnp_show_current_ids(struct device *dmdev,
 	return (str - buf);
 }

-static DEVICE_ATTR(id, S_IRUGO, pnp_show_current_ids, NULL);
-
-int pnp_interface_attach_device(struct pnp_dev *dev)
-{
-	int rc = device_create_file(&dev->dev, &dev_attr_options);
-
-	if (rc)
-		goto err;
-	rc = device_create_file(&dev->dev, &dev_attr_resources);
-	if (rc)
-		goto err_opt;
-	rc = device_create_file(&dev->dev, &dev_attr_id);
-	if (rc)
-		goto err_res;
-
-	return 0;
-
-err_res:
-	device_remove_file(&dev->dev, &dev_attr_resources);
-err_opt:
-	device_remove_file(&dev->dev, &dev_attr_options);
-err:
-	return rc;
-}
+struct device_attribute pnp_interface_attrs[] = {
+	__ATTR(resources, S_IRUGO | S_IWUSR,
+		   pnp_show_current_resources,
+		   pnp_set_current_resources),
+	__ATTR(options, S_IRUGO, pnp_show_options, NULL),
+	__ATTR(id, S_IRUGO, pnp_show_current_ids, NULL),
+	__ATTR_NULL,
+};