[PATCH v2] platform: Add support for automatic device IDs

[PATCH v2] platform: Add support for automatic device IDs

[Jean Delvare]

Right now we have support for explicit platform device IDs, as well as
ID-less platform devices when a given device type can only have one
instance. However there are cases where multiple instances of a device
type can exist, and their IDs aren't (and can't be) known in advance
and do not matter. In that case we need automatic device IDs to avoid
device name collisions.

I am using magic ID value -2 (PLATFORM_DEVID_AUTO) for this, similar
to -1 for ID-less devices. The automatically allocated device IDs are
global (to avoid an additional per-driver cost.) We keep note that the
ID was automatically allocated so that it can be freed later.

Note that we also restore the ID to PLATFORM_DEVID_AUTO on error and
device deletion, to avoid avoid unexpected behavior on retry. I don't
really expect retries on platform device addition, but better safe
than sorry.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Alternatively, platform_device.id_auto could be an int, used to store
the automatically allocated ID. This would avoid overwriting/restoring
platform_device.id itself.

Changes since v1:
* New struct member id_auto is used to distinguish between automatic
  and explicit IDs.
* Automatic IDs are no longer stored as negative numbers.
* PLATFORM_DEVID_AUTO changed from -4 to -2, short of a good reason to
  not use -2.

Code not tested yet, my test machine is currently used for a different
task that cannot be interrupted.

 drivers/base/platform.c         |   38 +++++++++++++++++++++++++++++++++++---
 include/linux/platform_device.h |    4 ++++
 2 files changed, 39 insertions(+), 3 deletions(-)

--- linux-3.5.orig/drivers/base/platform.c	2012-07-26 08:36:58.000000000 +0200
+++ linux-3.5/drivers/base/platform.c	2012-07-27 21:55:14.401830188 +0200
@@ -20,9 +20,13 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
+#include <linux/idr.h>
 
 #include "base.h"
 
+/* For automatically allocated device IDs */
+static DEFINE_IDA(platform_devid_ida);
+
 #define to_platform_driver(drv)	(container_of((drv), struct platform_driver, \
 				 driver))
 
@@ -263,7 +267,7 @@ EXPORT_SYMBOL_GPL(platform_device_add_da
  */
 int platform_device_add(struct platform_device *pdev)
 {
-	int i, ret = 0;
+	int i, ret;
 
 	if (!pdev)
 		return -EINVAL;
@@ -273,10 +277,27 @@ int platform_device_add(struct platform_
 
 	pdev->dev.bus = &platform_bus_type;
 
-	if (pdev->id != -1)
+	switch (pdev->id) {
+	default:
 		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
-	else
+		break;
+	case PLATFORM_DEVID_NONE:
 		dev_set_name(&pdev->dev, "%s", pdev->name);
+		break;
+	case PLATFORM_DEVID_AUTO:
+		/*
+		 * Automatically allocated device ID. We mark it as such so
+		 * that we remember it must be freed, and we append a suffix
+		 * to avoid namespace collision with explicit IDs.
+		 */
+		ret = ida_simple_get(&platform_devid_ida, 0, 0, GFP_KERNEL);
+		if (ret < 0)
+			goto err_out;
+		pdev->id = ret;
+		pdev->id_auto = true;
+		dev_set_name(&pdev->dev, "%s.%d.auto", pdev->name, pdev->id);
+		break;
+	}
 
 	for (i = 0; i < pdev->num_resources; i++) {
 		struct resource *p, *r = &pdev->resource[i];
@@ -309,6 +330,11 @@ int platform_device_add(struct platform_
 		return ret;
 
  failed:
+	if (pdev->id_auto) {
+		ida_simple_remove(&platform_devid_ida, pdev->id);
+		pdev->id = PLATFORM_DEVID_AUTO;
+	}
+
 	while (--i >= 0) {
 		struct resource *r = &pdev->resource[i];
 		unsigned long type = resource_type(r);
@@ -317,6 +343,7 @@ int platform_device_add(struct platform_
 			release_resource(r);
 	}
 
+ err_out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(platform_device_add);
@@ -336,6 +363,11 @@ void platform_device_del(struct platform
 	if (pdev) {
 		device_del(&pdev->dev);
 
+		if (pdev->id_auto) {
+			ida_simple_remove(&platform_devid_ida, pdev->id);
+			pdev->id = PLATFORM_DEVID_AUTO;
+		}
+
 		for (i = 0; i < pdev->num_resources; i++) {
 			struct resource *r = &pdev->resource[i];
 			unsigned long type = resource_type(r);
--- linux-3.5.orig/include/linux/platform_device.h	2012-07-21 22:58:29.000000000 +0200
+++ linux-3.5/include/linux/platform_device.h	2012-07-27 21:10:25.217340345 +0200
@@ -14,11 +14,15 @@
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 
+#define PLATFORM_DEVID_NONE	(-1)
+#define PLATFORM_DEVID_AUTO	(-2)
+
 struct mfd_cell;
 
 struct platform_device {
 	const char	* name;
 	int		id;
+	bool		id_auto;
 	struct device	dev;
 	u32		num_resources;
 	struct resource	* resource;

-- 
Jean Delvare

Re: [PATCH v2] platform: Add support for automatic device IDs

[Jean Delvare]

On Fri, 27 Jul 2012 22:14:59 +0200, Jean Delvare wrote:
> Code not tested yet, my test machine is currently used for a different
> task that cannot be interrupted.

Tested now, works fine.

-- 
Jean Delvare

Re: [PATCH v2] platform: Add support for automatic device IDs

[Greg KH]

On Fri, Jul 27, 2012 at 10:14:59PM +0200, Jean Delvare wrote:
> Right now we have support for explicit platform device IDs, as well as
> ID-less platform devices when a given device type can only have one
> instance. However there are cases where multiple instances of a device
> type can exist, and their IDs aren't (and can't be) known in advance
> and do not matter. In that case we need automatic device IDs to avoid
> device name collisions.
> 
> I am using magic ID value -2 (PLATFORM_DEVID_AUTO) for this, similar
> to -1 for ID-less devices. The automatically allocated device IDs are
> global (to avoid an additional per-driver cost.) We keep note that the
> ID was automatically allocated so that it can be freed later.
> 
> Note that we also restore the ID to PLATFORM_DEVID_AUTO on error and
> device deletion, to avoid avoid unexpected behavior on retry. I don't
> really expect retries on platform device addition, but better safe
> than sorry.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Looks sane to me, want me to queue it up for 3.7?

thanks,

greg k-h

Re: [PATCH v2] platform: Add support for automatic device IDs

[Jean Delvare]

On Tue, 31 Jul 2012 16:50:59 -0700, Greg KH wrote:
> On Fri, Jul 27, 2012 at 10:14:59PM +0200, Jean Delvare wrote:
> > Right now we have support for explicit platform device IDs, as well as
> > ID-less platform devices when a given device type can only have one
> > instance. However there are cases where multiple instances of a device
> > type can exist, and their IDs aren't (and can't be) known in advance
> > and do not matter. In that case we need automatic device IDs to avoid
> > device name collisions.
> > 
> > I am using magic ID value -2 (PLATFORM_DEVID_AUTO) for this, similar
> > to -1 for ID-less devices. The automatically allocated device IDs are
> > global (to avoid an additional per-driver cost.) We keep note that the
> > ID was automatically allocated so that it can be freed later.
> > 
> > Note that we also restore the ID to PLATFORM_DEVID_AUTO on error and
> > device deletion, to avoid avoid unexpected behavior on retry. I don't
> > really expect retries on platform device addition, but better safe
> > than sorry.
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Looks sane to me, want me to queue it up for 3.7?

Yes, please!

Thanks,
-- 
Jean Delvare