From: pawel.moll@arm.com (Pawel Moll)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] of: Keep track of populated platform devices
Date: Thu, 15 May 2014 16:08:53 +0100 [thread overview]
Message-ID: <1400166533.3672.3.camel@hornet> (raw)
In-Reply-To: <20140514105657.CF966C4153D@trevor.secretlab.ca>
On Wed, 2014-05-14 at 11:56 +0100, Grant Likely wrote:
> > Agreed, but, unless I'm missing some fundamental issue, I believe we can
> > solve the flag clearing problem in a generic way:
> >
> > --8<-----------
> > diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> > index 3cf61a1..0f489fb 100644
> > --- a/drivers/amba/bus.c
> > +++ b/drivers/amba/bus.c
> > @@ -17,6 +17,7 @@
> > #include <linux/pm_runtime.h>
> > #include <linux/amba/bus.h>
> > #include <linux/sizes.h>
> > +#include <linux/of_device.h>
> >
> > #include <asm/irq.h>
> >
> > @@ -268,6 +269,7 @@ static void amba_device_release(struct device *dev)
> > {
> > struct amba_device *d = to_amba_device(dev);
> >
> > + of_device_node_put(dev);
> > if (d->res.parent)
> > release_resource(&d->res);
> > kfree(d);
> > diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> > index ef37021..fd14d46 100644
> > --- a/include/linux/of_device.h
> > +++ b/include/linux/of_device.h
> > @@ -41,6 +41,8 @@ extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env
> >
> > static inline void of_device_node_put(struct device *dev)
> > {
> > + if (dev->of_node)
> > + of_node_clear_flag(dev->of_node, OF_POPULATED);
> > of_node_put(dev->of_node);
> > }
> > --8<-----------
> >
> > This will work even if one manually unregisters a DT-originating device,
> > because of_device_node_put() would be called in both amba and platform
> > device (kobj) release path.
>
> Doing it that way will also catch platform_devices that were created
> apart from of_platform_populate() and manually attached to an of_node,
> which is done some times. It will also break whenever multiple devices
> reference the same node if they call of_device_node_put().
>
> For example, a platform device providing an i2c bus will have a child
> device that represents the i2c bus, and that i2c device will point to
> the same node.
Yes, can I see it doing this. Without
> The flag clear really does need to be kept in the
> platform_device_unpopulate path because it is only to be used internally
> by of_platform_{populate,depopulate}
Ok, will move it there.
> > By the way, the fact that today amba_device_release() doesn't do
> > of_device_node_put() seems like a bug to me? (node's reference counter
> > won't be decremented)
>
> Yes, that is a bug. I want to get some unittests added to the DT code to
> verify that reference counts are being tracked correctly. Right now it
> is an utter mess.
Ok. If I move the flag, this bit is not longer an integral part of the
patch so will split it into a separate one, but merge the _depopulate()
one instead.
Thanks!
Pawe?
next prev parent reply other threads:[~2014-05-15 15:08 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-28 17:57 [PATCH 00/10] Versatile Express changes for 3.16 Pawel Moll
2014-04-28 17:57 ` [PATCH 01/10] of: Keep track of populated platform devices Pawel Moll
2014-04-28 18:02 ` Rob Herring
2014-04-29 12:56 ` Grant Likely
2014-04-30 11:48 ` Pawel Moll
2014-04-30 14:05 ` [PATCH v2] " Pawel Moll
2014-04-30 15:22 ` Rob Herring
2014-05-01 9:26 ` Grant Likely
2014-05-01 9:43 ` Grant Likely
2014-05-07 14:37 ` Pawel Moll
2014-05-14 10:56 ` Grant Likely
2014-05-15 15:08 ` Pawel Moll [this message]
2014-04-28 17:57 ` [PATCH 02/10] mfd: vexpress: Convert custom func API to regmap Pawel Moll
2014-04-29 22:21 ` Mark Brown
2014-04-30 13:58 ` Lee Jones
2014-04-30 14:13 ` Pawel Moll
2014-04-30 14:29 ` Lee Jones
2014-04-30 14:38 ` Pawel Moll
2014-04-30 16:01 ` [PATCH v2] " Pawel Moll
2014-04-30 17:05 ` Guenter Roeck
2014-05-01 18:58 ` Mike Turquette
2014-04-28 17:57 ` [PATCH 03/10] mfd: syscon: Add platform data with a regmap config name Pawel Moll
2014-04-28 17:57 ` [PATCH 04/10] mfd: vexpress: Define the device as MFD cells Pawel Moll
2014-05-09 11:24 ` Lee Jones
2014-04-28 17:57 ` [PATCH 05/10] clk: versatile: Split config options for sp810 and vexpress_osc Pawel Moll
2014-05-01 18:55 ` Mike Turquette
2014-04-28 17:57 ` [PATCH 06/10] clocksource: Sched clock source for Versatile Express Pawel Moll
2014-04-28 17:57 ` [PATCH 07/10] ARM: vexpress: remove redundant vexpress_dt_cpus_num to get cpu count Pawel Moll
2014-04-28 17:57 ` [PATCH 08/10] ARM: vexpress: Simplify SMP operations for DT-powered system Pawel Moll
2014-04-28 17:57 ` [PATCH 09/10] ARM: vexpress: move HBI check to sysreg driver Pawel Moll
2014-04-30 14:02 ` Lee Jones
2014-04-28 17:57 ` [PATCH 10/10] hwmon: vexpress: Use devm helper for hwmon device registration Pawel Moll
2014-04-28 22:59 ` Guenter Roeck
2014-04-30 15:16 ` Pawel Moll
2014-04-30 15:27 ` Guenter Roeck
2014-04-30 15:33 ` Pawel Moll
[not found] <Message-ID: <1399473437.3706.25.camel@hornet>
2014-05-13 11:48 ` [PATCH v3] of: Keep track of populated platform devices Pawel Moll
2014-05-13 12:24 ` Rob Herring
2014-05-14 11:05 ` Grant Likely
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=1400166533.3672.3.camel@hornet \
--to=pawel.moll@arm.com \
--cc=linux-arm-kernel@lists.infradead.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 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 NNTP newsgroup(s).