Re: [PATCH] TESTCASE: of: OOPS when disabling node via OF_DYNAMIC

[Pantelis Antoniou <pantelis.antoniou@konsulko.com>]

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

Hi Wolfram,

> On Apr 14, 2015, at 16:27 , Wolfram Sang <wsa@the-dreams.de> wrote:
> 
> Hi Pantelis,
> 
> thanks for your prompt reply. Unfortunately, I had to wait until I could
> access the test system again.
> 

[snip]

Sorry for the non-prompt reply; but just for kicks, can you try the attached patch?

I have a hunch this might be the problem.

Regards

— Pantelis



[-- Attachment #2: 0001-pdev-Fix-platform-device-resource-linking.patch --]
[-- Type: application/octet-stream, Size: 5926 bytes --]

From ac2df3acf04afbd9ab7166a4858ce05bb995c4bb Mon Sep 17 00:00:00 2001
From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Date: Fri, 28 Dec 2012 11:34:15 +0200
Subject: [PATCH] pdev: Fix platform device resource linking

Platform device removal uncovered a number of problems with
the way resources are handled in the core platform code.

Resources now form child/parent linkages and this requires
proper linking of the resources. On top of that the OF core
directly creates it's own platform devices. Simplify things
by providing helper functions that manage the linking properly.

Two functions are provided:

platform_device_link_resources(), which links all the
linkable resources (if not already linked).

and platform_device_unlink_resources(), which unlinks all the
resources.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/base/platform.c         | 122 +++++++++++++++++++++++++++-------------
 include/linux/platform_device.h |   4 ++
 2 files changed, 87 insertions(+), 39 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ebf034b..08226e2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -298,6 +298,80 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
 }
 EXPORT_SYMBOL_GPL(platform_device_add_data);
 
+static struct resource *platform_device_parent_resource(
+		struct platform_device *pdev, struct resource *r)
+{
+	unsigned long type;
+
+	if (r->parent)
+		return r->parent;
+
+	type = resource_type(r);
+	switch (type) {
+	case IORESOURCE_MEM:
+		return &iomem_resource;
+	case IORESOURCE_IO:
+		return &ioport_resource;
+	/* the other resource types are not linked, so no action */
+	default:
+		break;
+	}
+	pr_debug("%s: no parent for resource %p type 0x%lx\n",
+			dev_name(&pdev->dev), r, resource_type(r));
+	return NULL;
+}
+
+int platform_device_unlink_resources(struct platform_device *pdev)
+{
+	struct resource *r;
+	int i;
+
+	for (i = pdev->num_resources - 1; i >= 0; i--) {
+		r = &pdev->resource[i];
+		if (r->parent == NULL)
+			continue;
+		release_resource(r);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(platform_device_unlink_resources);
+
+int platform_device_link_resources(struct platform_device *pdev)
+{
+	int i;
+	struct resource *p, *r;
+
+	for (i = 0; i < pdev->num_resources; i++) {
+		r = &pdev->resource[i];
+
+		if (r->name == NULL)
+			r->name = dev_name(&pdev->dev);
+
+		/* already linked */
+		if (r->parent != NULL)
+			continue;
+
+		p = platform_device_parent_resource(pdev, r);
+		if (p && insert_resource(p, r)) {
+			pr_err("%s: failed to claim resource %d\n",
+			       dev_name(&pdev->dev), i);
+			goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	while (--i >= 0) {
+		r = &pdev->resource[i];
+		if (r->parent == NULL)
+			continue;
+		release_resource(r);
+	}
+	return -EBUSY;
+}
+EXPORT_SYMBOL_GPL(platform_device_link_resources);
+
 /**
  * platform_device_add - add a platform device to device hierarchy
  * @pdev: platform device we're adding
@@ -307,7 +381,7 @@ EXPORT_SYMBOL_GPL(platform_device_add_data);
  */
 int platform_device_add(struct platform_device *pdev)
 {
-	int i, ret;
+	int ret;
 
 	if (!pdev)
 		return -EINVAL;
@@ -339,26 +413,10 @@ int platform_device_add(struct platform_device *pdev)
 		break;
 	}
 
-	for (i = 0; i < pdev->num_resources; i++) {
-		struct resource *p, *r = &pdev->resource[i];
-
-		if (r->name == NULL)
-			r->name = dev_name(&pdev->dev);
-
-		p = r->parent;
-		if (!p) {
-			if (resource_type(r) == IORESOURCE_MEM)
-				p = &iomem_resource;
-			else if (resource_type(r) == IORESOURCE_IO)
-				p = &ioport_resource;
-		}
-
-		if (p && insert_resource(p, r)) {
-			dev_err(&pdev->dev, "failed to claim resource %d\n", i);
-			ret = -EBUSY;
-			goto failed;
-		}
-	}
+	/* make sure the resources are linked properly */
+	ret = platform_device_link_resources(pdev);
+	if (ret != 0)
+		goto failed_res;
 
 	pr_debug("Registering platform device '%s'. Parent at %s\n",
 		 dev_name(&pdev->dev), dev_name(pdev->dev.parent));
@@ -367,20 +425,14 @@ int platform_device_add(struct platform_device *pdev)
 	if (ret == 0)
 		return ret;
 
- failed:
+	platform_device_unlink_resources(pdev);
+
+ failed_res:
 	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);
-
-		if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
-			release_resource(r);
-	}
-
  err_out:
 	return ret;
 }
@@ -396,8 +448,6 @@ EXPORT_SYMBOL_GPL(platform_device_add);
  */
 void platform_device_del(struct platform_device *pdev)
 {
-	int i;
-
 	if (pdev) {
 		device_del(&pdev->dev);
 
@@ -406,13 +456,7 @@ void platform_device_del(struct platform_device *pdev)
 			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);
-
-			if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
-				release_resource(r);
-		}
+		platform_device_unlink_resources(pdev);
 	}
 }
 EXPORT_SYMBOL_GPL(platform_device_del);
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 58f1e75..eabfd9c 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -331,4 +331,8 @@ extern int platform_pm_restore(struct device *dev);
 #define USE_PLATFORM_PM_SLEEP_OPS
 #endif
 
+/* helper functions for resource list management */
+int platform_device_unlink_resources(struct platform_device *pdev);
+int platform_device_link_resources(struct platform_device *pdev);
+
 #endif /* _PLATFORM_DEVICE_H_ */
-- 
1.7.12