All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grant Likely <grant.likely@secretlab.ca>
To: Pantelis Antoniou
	<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>,
	Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>,
	Laurent Pinchart
	<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
	Geert Uytterhoeven
	<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] TESTCASE: of: OOPS when disabling node via OF_DYNAMIC
Date: Sun, 07 Jun 2015 11:16:32 +0000	[thread overview]
Message-ID: <20150607111632.CC7A4C410F0@trevor.secretlab.ca> (raw)
In-Reply-To: <83B5C458-8D49-4EF6-9F4A-6C0E2579AB9F-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

On Wed, 22 Apr 2015 15:30:28 +0300
, Pantelis Antoniou <pantelis.antoniou@konsulko.com>
 wrote:
> 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
> 
> 

I played around with this some today. If I'm reading it correctly, the
following patch reproduces the same problem:

(continued below patch)
---
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 18016341d5a9..0a27b38c3041 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -753,6 +753,11 @@ static void __init of_unittest_match_node(void)
 	}
 }
 
+static struct resource test_bus_res = {
+	.start = 0xfffffff8,
+	.end = 0xfffffff9,
+	.flags = IORESOURCE_MEM,
+};
 static const struct platform_device_info test_bus_info = {
 	.name = "unittest-bus",
 };
@@ -795,6 +800,7 @@ static void __init of_unittest_platform_populate(void)
 	if (rc)
 		return;
 	test_bus->dev.of_node = np;
+	platform_device_add_resources(test_bus, &test_bus_res, 1);
 
 	of_platform_populate(np, match, NULL, &test_bus->dev);
 	for_each_child_of_node(np, child) {

---

I think the fixup patch boils down to the following. It's hard to tell
because it combines refactoring with the bug fix. Do I have it correct?
If so, I'd rather commit the simple fix which can be committed now for
v4.1, and the refactoring can be pushed for v4.2

---

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ebf034b97278..b3042e6ee3ef 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -377,7 +377,7 @@ int platform_device_add(struct platform_device *pdev)
 		struct resource *r = &pdev->resource[i];
 		unsigned long type = resource_type(r);
 
-		if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
+		if (r->parent)
 			release_resource(r);
 	}
 
@@ -410,7 +410,7 @@ void platform_device_del(struct platform_device *pdev)
 			struct resource *r = &pdev->resource[i];
 			unsigned long type = resource_type(r);
 
-			if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
+			if (r->parent)
 				release_resource(r);
 		}
 	}


WARNING: multiple messages have this Message-ID (diff)
From: Grant Likely <grant.likely@secretlab.ca>
To: Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
	Wolfram Sang <wsa@the-dreams.de>
Cc: devicetree@vger.kernel.org, linux-sh@vger.kernel.org,
	Magnus Damm <magnus.damm@gmail.com>,
	Simon Horman <horms@verge.net.au>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Rob Herring <robh+dt@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] TESTCASE: of: OOPS when disabling node via OF_DYNAMIC
Date: Sun, 07 Jun 2015 12:16:32 +0100	[thread overview]
Message-ID: <20150607111632.CC7A4C410F0@trevor.secretlab.ca> (raw)
In-Reply-To: <83B5C458-8D49-4EF6-9F4A-6C0E2579AB9F@konsulko.com>

On Wed, 22 Apr 2015 15:30:28 +0300
, Pantelis Antoniou <pantelis.antoniou@konsulko.com>
 wrote:
> 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
> 
> 

I played around with this some today. If I'm reading it correctly, the
following patch reproduces the same problem:

(continued below patch)
---
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 18016341d5a9..0a27b38c3041 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -753,6 +753,11 @@ static void __init of_unittest_match_node(void)
 	}
 }
 
+static struct resource test_bus_res = {
+	.start = 0xfffffff8,
+	.end = 0xfffffff9,
+	.flags = IORESOURCE_MEM,
+};
 static const struct platform_device_info test_bus_info = {
 	.name = "unittest-bus",
 };
@@ -795,6 +800,7 @@ static void __init of_unittest_platform_populate(void)
 	if (rc)
 		return;
 	test_bus->dev.of_node = np;
+	platform_device_add_resources(test_bus, &test_bus_res, 1);
 
 	of_platform_populate(np, match, NULL, &test_bus->dev);
 	for_each_child_of_node(np, child) {

---

I think the fixup patch boils down to the following. It's hard to tell
because it combines refactoring with the bug fix. Do I have it correct?
If so, I'd rather commit the simple fix which can be committed now for
v4.1, and the refactoring can be pushed for v4.2

---

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ebf034b97278..b3042e6ee3ef 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -377,7 +377,7 @@ int platform_device_add(struct platform_device *pdev)
 		struct resource *r = &pdev->resource[i];
 		unsigned long type = resource_type(r);
 
-		if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
+		if (r->parent)
 			release_resource(r);
 	}
 
@@ -410,7 +410,7 @@ void platform_device_del(struct platform_device *pdev)
 			struct resource *r = &pdev->resource[i];
 			unsigned long type = resource_type(r);
 
-			if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
+			if (r->parent)
 				release_resource(r);
 		}
 	}


WARNING: multiple messages have this Message-ID (diff)
From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
To: Pantelis Antoniou
	<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>,
	Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>,
	Laurent Pinchart
	<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
	Geert Uytterhoeven
	<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] TESTCASE: of: OOPS when disabling node via OF_DYNAMIC
Date: Sun, 07 Jun 2015 12:16:32 +0100	[thread overview]
Message-ID: <20150607111632.CC7A4C410F0@trevor.secretlab.ca> (raw)
In-Reply-To: <83B5C458-8D49-4EF6-9F4A-6C0E2579AB9F-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

On Wed, 22 Apr 2015 15:30:28 +0300
, Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
 wrote:
> Hi Wolfram,
> 
> > On Apr 14, 2015, at 16:27 , Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 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
> 
> 

I played around with this some today. If I'm reading it correctly, the
following patch reproduces the same problem:

(continued below patch)
---
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 18016341d5a9..0a27b38c3041 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -753,6 +753,11 @@ static void __init of_unittest_match_node(void)
 	}
 }
 
+static struct resource test_bus_res = {
+	.start = 0xfffffff8,
+	.end = 0xfffffff9,
+	.flags = IORESOURCE_MEM,
+};
 static const struct platform_device_info test_bus_info = {
 	.name = "unittest-bus",
 };
@@ -795,6 +800,7 @@ static void __init of_unittest_platform_populate(void)
 	if (rc)
 		return;
 	test_bus->dev.of_node = np;
+	platform_device_add_resources(test_bus, &test_bus_res, 1);
 
 	of_platform_populate(np, match, NULL, &test_bus->dev);
 	for_each_child_of_node(np, child) {

---

I think the fixup patch boils down to the following. It's hard to tell
because it combines refactoring with the bug fix. Do I have it correct?
If so, I'd rather commit the simple fix which can be committed now for
v4.1, and the refactoring can be pushed for v4.2

---

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ebf034b97278..b3042e6ee3ef 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -377,7 +377,7 @@ int platform_device_add(struct platform_device *pdev)
 		struct resource *r = &pdev->resource[i];
 		unsigned long type = resource_type(r);
 
-		if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
+		if (r->parent)
 			release_resource(r);
 	}
 
@@ -410,7 +410,7 @@ void platform_device_del(struct platform_device *pdev)
 			struct resource *r = &pdev->resource[i];
 			unsigned long type = resource_type(r);
 
-			if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
+			if (r->parent)
 				release_resource(r);
 		}
 	}

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2015-06-07 11:16 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-31 15:12 [PATCH] TESTCASE: of: OOPS when disabling node via OF_DYNAMIC Wolfram Sang
2015-03-31 15:12 ` Wolfram Sang
2015-03-31 16:23 ` Pantelis Antoniou
2015-03-31 16:23   ` Pantelis Antoniou
2015-04-14 13:27   ` Wolfram Sang
2015-04-14 13:27     ` Wolfram Sang
2015-04-22 12:30     ` Pantelis Antoniou
2015-04-22 12:30       ` Pantelis Antoniou
2015-04-23  8:33       ` Wolfram Sang
2015-04-23  8:33         ` Wolfram Sang
2015-04-23  8:41         ` Pantelis Antoniou
2015-04-23  8:41           ` Pantelis Antoniou
2015-04-23  8:49           ` Wolfram Sang
2015-04-23  8:49             ` Wolfram Sang
     [not found]       ` <83B5C458-8D49-4EF6-9F4A-6C0E2579AB9F-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2015-06-07 11:16         ` Grant Likely [this message]
2015-06-07 11:16           ` Grant Likely
2015-06-07 11:16           ` Grant Likely
2015-06-09 16:50           ` Laurent Pinchart
2015-06-09 16:50             ` Laurent Pinchart
2015-06-10 16:53             ` Grant Likely
2015-06-10 16:53               ` Grant Likely
2015-06-10 16:53               ` Grant Likely
2015-06-11 11:14           ` Pantelis Antoniou
2015-06-11 11:14             ` Pantelis Antoniou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150607111632.CC7A4C410F0@trevor.secretlab.ca \
    --to=grant.likely@secretlab.ca \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org \
    --cc=horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org \
    --cc=laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.