[PATCH RFC v2 0/8] component helper improvements

[PATCH RFC v2 0/8] component helper improvements

[Russell King - ARM Linux]

A while back, Laurent raised some comments about the component helper,
which this patch set starts to address.

The first point it addresses is the repeated parsing inefficiency when
deferred probing occurs.  When DT is used, the structure of the
component helper today means that masters end up parsing the device
tree for each attempt to re-bind the driver.
									        
We remove this inefficiency by creating an array of matching data and
functions, which the component helper can use internally to match up
components to their master.

The second point was the inefficiency of destroying the list of
components each time we saw a failure.	We did this to ensure that
we kept things correctly ordered: component bind order matters.
As we have an array instead, the array is already ordered, so we
use this array to store the component pointers instead of a list,
and remember which are duplicates (and thus should be avoided.)
Avoiding the right duplicates matters as we walk the array in the
opposite direction at tear down.

 drivers/base/component.c               | 249 ++++++++++++++++++++++-----------
 drivers/gpu/drm/msm/msm_drv.c          |  83 +++++------
 drivers/staging/imx-drm/imx-drm-core.c |  57 +-------
 include/linux/component.h              |   8 +-
 4 files changed, 208 insertions(+), 189 deletions(-)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH RFC v2 1/8] component: fix missed cleanup in case of devres failure

[Russell King]

In try_to_bring_up_master(), we tear down the master's component list
for each error case, except for devres group failure.  Fix this
oversight by making the code less prone to such mistakes.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/component.c | 62 ++++++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index c4778995cd72..d0ebd4431736 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -113,44 +113,44 @@ static void master_remove_components(struct master *master)
 static int try_to_bring_up_master(struct master *master,
 	struct component *component)
 {
-	int ret = 0;
+	int ret;
 
-	if (!master->bound) {
-		/*
-		 * Search the list of components, looking for components that
-		 * belong to this master, and attach them to the master.
-		 */
-		if (master->ops->add_components(master->dev, master)) {
-			/* Failed to find all components */
-			master_remove_components(master);
-			ret = 0;
-			goto out;
-		}
+	if (master->bound)
+		return 0;
 
-		if (component && component->master != master) {
-			master_remove_components(master);
-			ret = 0;
-			goto out;
-		}
+	/*
+	 * Search the list of components, looking for components that
+	 * belong to this master, and attach them to the master.
+	 */
+	if (master->ops->add_components(master->dev, master)) {
+		/* Failed to find all components */
+		ret = 0;
+		goto out;
+	}
 
-		if (!devres_open_group(master->dev, NULL, GFP_KERNEL)) {
-			ret = -ENOMEM;
-			goto out;
-		}
+	if (component && component->master != master) {
+		ret = 0;
+		goto out;
+	}
 
-		/* Found all components */
-		ret = master->ops->bind(master->dev);
-		if (ret < 0) {
-			devres_release_group(master->dev, NULL);
-			dev_info(master->dev, "master bind failed: %d\n", ret);
-			master_remove_components(master);
-			goto out;
-		}
+	if (!devres_open_group(master->dev, NULL, GFP_KERNEL)) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
-		master->bound = true;
-		ret = 1;
+	/* Found all components */
+	ret = master->ops->bind(master->dev);
+	if (ret < 0) {
+		devres_release_group(master->dev, NULL);
+		dev_info(master->dev, "master bind failed: %d\n", ret);
+		goto out;
 	}
+
+	master->bound = true;
+	return 1;
+
 out:
+	master_remove_components(master);
 
 	return ret;
 }
-- 
1.8.3.1

[PATCH RFC v2 2/8] component: ignore multiple additions of the same component

[Russell King]

Permit masters to call component_master_add_child() and match the same
child multiple times.  This may happen if there's multiple connections
to a single component device from other devices.  In such scenarios,
we should not return a failure, but instead ignore the attempt.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/component.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index d0ebd4431736..55813e91bf0d 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -69,6 +69,11 @@ static void component_detach_master(struct master *master, struct component *c)
 	c->master = NULL;
 }
 
+/*
+ * Add a component to a master, finding the component via the compare
+ * function and compare data.  This is safe to call for duplicate matches
+ * and will not result in the same component being added multiple times.
+ */
 int component_master_add_child(struct master *master,
 	int (*compare)(struct device *, void *), void *compare_data)
 {
@@ -76,11 +81,12 @@ int component_master_add_child(struct master *master,
 	int ret = -ENXIO;
 
 	list_for_each_entry(c, &component_list, node) {
-		if (c->master)
+		if (c->master && c->master != master)
 			continue;
 
 		if (compare(c->dev, compare_data)) {
-			component_attach_master(master, c);
+			if (!c->master)
+				component_attach_master(master, c);
 			ret = 0;
 			break;
 		}
-- 
1.8.3.1

[PATCH RFC v2 3/8] component: add support for component match array

[Russell King]

Add support for generating a set of component matches at master probe
time, and submitting them to the component layer.  This allows the
component layer to perform the matches internally without needing to
call into the master driver, and allows for further restructuring of
the component helper.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/component.c  | 119 ++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/component.h |   7 +++
 2 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 55813e91bf0d..bd8b4908593b 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -18,6 +18,15 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 
+struct component_match {
+	size_t alloc;
+	size_t num;
+	struct {
+		void *data;
+		int (*fn)(struct device *, void *);
+	} compare[0];
+};
+
 struct master {
 	struct list_head node;
 	struct list_head components;
@@ -25,6 +34,7 @@ struct master {
 
 	const struct component_master_ops *ops;
 	struct device *dev;
+	struct component_match *match;
 };
 
 struct component {
@@ -96,6 +106,34 @@ int component_master_add_child(struct master *master,
 }
 EXPORT_SYMBOL_GPL(component_master_add_child);
 
+static int find_components(struct master *master)
+{
+	struct component_match *match = master->match;
+	size_t i;
+	int ret = 0;
+
+	if (!match) {
+		/*
+		 * Search the list of components, looking for components that
+		 * belong to this master, and attach them to the master.
+		 */
+		return master->ops->add_components(master->dev, master);
+	}
+
+	/*
+	 * Scan the array of match functions and attach
+	 * any components which are found to this master.
+	 */
+	for (i = 0; i < match->num; i++) {
+		ret = component_master_add_child(master,
+						 match->compare[i].fn,
+						 match->compare[i].data);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
 /* Detach all attached components from this master */
 static void master_remove_components(struct master *master)
 {
@@ -128,7 +166,7 @@ static int try_to_bring_up_master(struct master *master,
 	 * Search the list of components, looking for components that
 	 * belong to this master, and attach them to the master.
 	 */
-	if (master->ops->add_components(master->dev, master)) {
+	if (find_components(master)) {
 		/* Failed to find all components */
 		ret = 0;
 		goto out;
@@ -186,18 +224,86 @@ static void take_down_master(struct master *master)
 	master_remove_components(master);
 }
 
-int component_master_add(struct device *dev,
-	const struct component_master_ops *ops)
+static size_t component_match_size(size_t num)
+{
+	return offsetof(struct component_match, compare[num]);
+}
+
+static struct component_match *component_match_realloc(struct device *dev,
+	struct component_match *match, size_t num)
+{
+	struct component_match *new;
+
+	if (match && match->alloc == num)
+		return match;
+
+	new = devm_kmalloc(dev, component_match_size(num), GFP_KERNEL);
+	if (!new)
+		return ERR_PTR(-ENOMEM);
+
+	if (match) {
+		memcpy(new, match, component_match_size(min(match->num, num)));
+		devm_kfree(dev, match);
+	} else {
+		new->num = 0;
+	}
+
+	new->alloc = num;
+
+	return new;
+}
+
+/*
+ * Add a component to be matched.
+ *
+ * The match array is first created or extended if necessary.
+ */
+void component_match_add(struct device *dev, struct component_match **matchptr,
+	int (*compare)(struct device *, void *), void *compare_data)
+{
+	struct component_match *match = *matchptr;
+
+	if (IS_ERR(match))
+		return;
+
+	if (!match || match->num == match->alloc) {
+		size_t new_size = match ? match->alloc + 16 : 15;
+
+		match = component_match_realloc(dev, match, new_size);
+
+		*matchptr = match;
+
+		if (IS_ERR(match))
+			return;
+	}
+
+	match->compare[match->num].fn = compare;
+	match->compare[match->num].data = compare_data;
+	match->num++;
+}
+
+int component_master_add_with_match(struct device *dev,
+	const struct component_master_ops *ops,
+	struct component_match *match)
 {
 	struct master *master;
 	int ret;
 
+	if (ops->add_components && match)
+		return -EINVAL;
+
+	/* Reallocate the match array for its true size */
+	match = component_match_realloc(dev, match, match->num);
+	if (IS_ERR(match))
+		return PTR_ERR(match);
+
 	master = kzalloc(sizeof(*master), GFP_KERNEL);
 	if (!master)
 		return -ENOMEM;
 
 	master->dev = dev;
 	master->ops = ops;
+	master->match = match;
 	INIT_LIST_HEAD(&master->components);
 
 	/* Add to the list of available masters. */
@@ -215,6 +321,13 @@ int component_master_add(struct device *dev,
 
 	return ret < 0 ? ret : 0;
 }
+EXPORT_SYMBOL_GPL(component_master_add_with_match);
+
+int component_master_add(struct device *dev,
+	const struct component_master_ops *ops)
+{
+	return component_master_add_with_match(dev, ops, NULL);
+}
 EXPORT_SYMBOL_GPL(component_master_add);
 
 void component_master_del(struct device *dev,
diff --git a/include/linux/component.h b/include/linux/component.h
index 68870182ca1e..c00dcc302611 100644
--- a/include/linux/component.h
+++ b/include/linux/component.h
@@ -29,4 +29,11 @@ void component_master_del(struct device *,
 int component_master_add_child(struct master *master,
 	int (*compare)(struct device *, void *), void *compare_data);
 
+struct component_match;
+
+int component_master_add_with_match(struct device *,
+	const struct component_master_ops *, struct component_match *);
+void component_match_add(struct device *, struct component_match **,
+	int (*compare)(struct device *, void *), void *compare_data);
+
 #endif
-- 
1.8.3.1

[PATCH RFC v2 3/8] component: add support for component match array

[Philipp Zabel]

Hi Russell,

On Tue, Jun 24, 2014 at 9:29 PM, Russell King
<rmk+kernel@arm.linux.org.uk> wrote:
[...]
> +/*
> + * Add a component to be matched.
> + *
> + * The match array is first created or extended if necessary.
> + */
> +void component_match_add(struct device *dev, struct component_match **matchptr,
> +       int (*compare)(struct device *, void *), void *compare_data)
> +{
> +       struct component_match *match = *matchptr;
> +
> +       if (IS_ERR(match))
> +               return;
> +
> +       if (!match || match->num == match->alloc) {
> +               size_t new_size = match ? match->alloc + 16 : 15;
> +
> +               match = component_match_realloc(dev, match, new_size);
> +
> +               *matchptr = match;
> +
> +               if (IS_ERR(match))
> +                       return;
> +       }
> +
> +       match->compare[match->num].fn = compare;
> +       match->compare[match->num].data = compare_data;
> +       match->num++;
> +}

component_match_add should be exported.

regards
Philipp

[PATCH RFC v2 3/8] component: add support for component match array

[Russell King - ARM Linux]

On Thu, Jun 26, 2014 at 02:34:17PM +0200, Philipp Zabel wrote:
> Hi Russell,
> 
> On Tue, Jun 24, 2014 at 9:29 PM, Russell King
> <rmk+kernel@arm.linux.org.uk> wrote:
> [...]
> > +/*
> > + * Add a component to be matched.
> > + *
> > + * The match array is first created or extended if necessary.
> > + */
> > +void component_match_add(struct device *dev, struct component_match **matchptr,
> > +       int (*compare)(struct device *, void *), void *compare_data)
> > +{
> > +       struct component_match *match = *matchptr;
> > +
> > +       if (IS_ERR(match))
> > +               return;
> > +
> > +       if (!match || match->num == match->alloc) {
> > +               size_t new_size = match ? match->alloc + 16 : 15;
> > +
> > +               match = component_match_realloc(dev, match, new_size);
> > +
> > +               *matchptr = match;
> > +
> > +               if (IS_ERR(match))
> > +                       return;
> > +       }
> > +
> > +       match->compare[match->num].fn = compare;
> > +       match->compare[match->num].data = compare_data;
> > +       match->num++;
> > +}
> 
> component_match_add should be exported.

Fixed, thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH RFC v2 3/8] component: add support for component match array

[Russell King - ARM Linux]

On Thu, Jun 26, 2014 at 03:46:01PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 26, 2014 at 02:34:17PM +0200, Philipp Zabel wrote:
> > Hi Russell,
> > 
> > On Tue, Jun 24, 2014 at 9:29 PM, Russell King
> > <rmk+kernel@arm.linux.org.uk> wrote:
> > [...]
> > > +/*
> > > + * Add a component to be matched.
> > > + *
> > > + * The match array is first created or extended if necessary.
> > > + */
> > > +void component_match_add(struct device *dev, struct component_match **matchptr,
> > > +       int (*compare)(struct device *, void *), void *compare_data)
> > > +{
> > > +       struct component_match *match = *matchptr;
> > > +
> > > +       if (IS_ERR(match))
> > > +               return;
> > > +
> > > +       if (!match || match->num == match->alloc) {
> > > +               size_t new_size = match ? match->alloc + 16 : 15;
> > > +
> > > +               match = component_match_realloc(dev, match, new_size);
> > > +
> > > +               *matchptr = match;
> > > +
> > > +               if (IS_ERR(match))
> > > +                       return;
> > > +       }
> > > +
> > > +       match->compare[match->num].fn = compare;
> > > +       match->compare[match->num].data = compare_data;
> > > +       match->num++;
> > > +}
> > 
> > component_match_add should be exported.
> 
> Fixed, thanks.

As there's no further comments, and Inki Dae has not responded, I'm
going to send these out without the RFC tag in the hope that people
will provide acks.  This allows us to move forward with this despite
the Exynos DRM blockage.

The ultimate plan is for patches 1 to 3 inclusive to be merged into
Greg's driver tree, 1 to 3 and 5 into Greg's staging tree, and 1 to
3 and 4 for David Airlie's DRM tree - patches 1 to 3 are needed for
both patches 4 and 5.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH RFC v2 3/8] component: add support for component match array

[Inki Dae]

2014-07-01 23:22 GMT+09:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Thu, Jun 26, 2014 at 03:46:01PM +0100, Russell King - ARM Linux wrote:
>> On Thu, Jun 26, 2014 at 02:34:17PM +0200, Philipp Zabel wrote:
>> > Hi Russell,
>> >
>> > On Tue, Jun 24, 2014 at 9:29 PM, Russell King
>> > <rmk+kernel@arm.linux.org.uk> wrote:
>> > [...]
>> > > +/*
>> > > + * Add a component to be matched.
>> > > + *
>> > > + * The match array is first created or extended if necessary.
>> > > + */
>> > > +void component_match_add(struct device *dev, struct component_match **matchptr,
>> > > +       int (*compare)(struct device *, void *), void *compare_data)
>> > > +{
>> > > +       struct component_match *match = *matchptr;
>> > > +
>> > > +       if (IS_ERR(match))
>> > > +               return;
>> > > +
>> > > +       if (!match || match->num == match->alloc) {
>> > > +               size_t new_size = match ? match->alloc + 16 : 15;
>> > > +
>> > > +               match = component_match_realloc(dev, match, new_size);
>> > > +
>> > > +               *matchptr = match;
>> > > +
>> > > +               if (IS_ERR(match))
>> > > +                       return;
>> > > +       }
>> > > +
>> > > +       match->compare[match->num].fn = compare;
>> > > +       match->compare[match->num].data = compare_data;
>> > > +       match->num++;
>> > > +}
>> >
>> > component_match_add should be exported.
>>
>> Fixed, thanks.
>
> As there's no further comments, and Inki Dae has not responded, I'm

It's has been just a week. I will check and look into your patch
series. I think Exynos drm should also be considered for the use of
component match array.

Thanks,
Inki Dae

> going to send these out without the RFC tag in the hope that people
> will provide acks.  This allows us to move forward with this despite
> the Exynos DRM blockage.
>
> The ultimate plan is for patches 1 to 3 inclusive to be merged into
> Greg's driver tree, 1 to 3 and 5 into Greg's staging tree, and 1 to
> 3 and 4 for David Airlie's DRM tree - patches 1 to 3 are needed for
> both patches 4 and 5.
>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH RFC v2 3/8] component: add support for component match array

[Russell King - ARM Linux]

On Thu, Jul 03, 2014 at 12:26:39AM +0900, Inki Dae wrote:
> It's has been just a week. I will check and look into your patch
> series. I think Exynos drm should also be considered for the use of
> component match array.

Actually, this series has been around for much longer than just a
week.  Your new usage introduced in the recent merge window is what
has resulted in you becoming aware of this series.

It was developed before April through discussions, and then shared
with those people.  Then, April 27th, it was posted publically to
all the recipients except yourself (because Exynos hadn't visibly
been converted).  At that time, two reviewed-by tags were given.

It was also sent out last week, with yourself added to the recepient
list because there is now a dependency with some of your work.

However, what I'm asking for is *not* the entire series to be merged
at this point - that would break Exynos DRM.  I'm asking for the first
three to be merged initially by Greg, which gets the new interface in
place without breaking existing users.  We can then convert existing
users at a slower rate, and remove the old interface once everyone
has caught up.

So, I'd ask that you give priority to looking at the first three
patches and deciding whether you find them acceptable as a replacement
interface, rather than trying to review the entire set of six core
patches (1,2,3,6,7,8).

What I'm trying to avoid here is for all these patches to be delayed
past the next merge window, and pushed into the next cycle.  That's
likely to end up in the same scenario as exists with Exynos DRM today,
only with other new users of the existing interface - and then have to
repeat this whole "try to get the new users to review this set of
changes" cycle again.

We're half way through -rc3 right now.  -final occurs anytime between
-rc6 and -rc9, which could be just three and a half weeks away.

I have other changes which I need to get out onto the list(s) which
depend on this too (for DRM) which I'm hoping to also make this coming
merge window, but I can't start that process until I know where I
stand with these.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH RFC v2 3/8] component: add support for component match array

[Russell King - ARM Linux]

On Thu, Jul 03, 2014 at 12:26:39AM +0900, Inki Dae wrote:
> 2014-07-01 23:22 GMT+09:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > On Thu, Jun 26, 2014 at 03:46:01PM +0100, Russell King - ARM Linux wrote:
> >> On Thu, Jun 26, 2014 at 02:34:17PM +0200, Philipp Zabel wrote:
> >> > Hi Russell,
> >> >
> >> > On Tue, Jun 24, 2014 at 9:29 PM, Russell King
> >> > <rmk+kernel@arm.linux.org.uk> wrote:
> >> > [...]
> >> > > +/*
> >> > > + * Add a component to be matched.
> >> > > + *
> >> > > + * The match array is first created or extended if necessary.
> >> > > + */
> >> > > +void component_match_add(struct device *dev, struct component_match **matchptr,
> >> > > +       int (*compare)(struct device *, void *), void *compare_data)
> >> > > +{
> >> > > +       struct component_match *match = *matchptr;
> >> > > +
> >> > > +       if (IS_ERR(match))
> >> > > +               return;
> >> > > +
> >> > > +       if (!match || match->num == match->alloc) {
> >> > > +               size_t new_size = match ? match->alloc + 16 : 15;
> >> > > +
> >> > > +               match = component_match_realloc(dev, match, new_size);
> >> > > +
> >> > > +               *matchptr = match;
> >> > > +
> >> > > +               if (IS_ERR(match))
> >> > > +                       return;
> >> > > +       }
> >> > > +
> >> > > +       match->compare[match->num].fn = compare;
> >> > > +       match->compare[match->num].data = compare_data;
> >> > > +       match->num++;
> >> > > +}
> >> >
> >> > component_match_add should be exported.
> >>
> >> Fixed, thanks.
> >
> > As there's no further comments, and Inki Dae has not responded, I'm
> 
> It's has been just a week. I will check and look into your patch
> series. I think Exynos drm should also be considered for the use of
> component match array.

It has now been almost two months.  What's happening on this?

Please note that I'm planning to push the rest of the component updates
during the next merge window, which will result in unconverted drivers
breaking.

Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH RFC v2 3/8] component: add support for component match array

[Inki Dae]

On 2014? 08? 31? 06:33, Russell King - ARM Linux wrote:
> On Thu, Jul 03, 2014 at 12:26:39AM +0900, Inki Dae wrote:
>> 2014-07-01 23:22 GMT+09:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
>>> On Thu, Jun 26, 2014 at 03:46:01PM +0100, Russell King - ARM Linux wrote:
>>>> On Thu, Jun 26, 2014 at 02:34:17PM +0200, Philipp Zabel wrote:
>>>>> Hi Russell,
>>>>>
>>>>> On Tue, Jun 24, 2014 at 9:29 PM, Russell King
>>>>> <rmk+kernel@arm.linux.org.uk> wrote:
>>>>> [...]
>>>>>> +/*
>>>>>> + * Add a component to be matched.
>>>>>> + *
>>>>>> + * The match array is first created or extended if necessary.
>>>>>> + */
>>>>>> +void component_match_add(struct device *dev, struct component_match **matchptr,
>>>>>> +       int (*compare)(struct device *, void *), void *compare_data)
>>>>>> +{
>>>>>> +       struct component_match *match = *matchptr;
>>>>>> +
>>>>>> +       if (IS_ERR(match))
>>>>>> +               return;
>>>>>> +
>>>>>> +       if (!match || match->num == match->alloc) {
>>>>>> +               size_t new_size = match ? match->alloc + 16 : 15;
>>>>>> +
>>>>>> +               match = component_match_realloc(dev, match, new_size);
>>>>>> +
>>>>>> +               *matchptr = match;
>>>>>> +
>>>>>> +               if (IS_ERR(match))
>>>>>> +                       return;
>>>>>> +       }
>>>>>> +
>>>>>> +       match->compare[match->num].fn = compare;
>>>>>> +       match->compare[match->num].data = compare_data;
>>>>>> +       match->num++;
>>>>>> +}
>>>>>
>>>>> component_match_add should be exported.
>>>>
>>>> Fixed, thanks.
>>>
>>> As there's no further comments, and Inki Dae has not responded, I'm
>>
>> It's has been just a week. I will check and look into your patch
>> series. I think Exynos drm should also be considered for the use of
>> component match array.
> 
> It has now been almost two months.  What's happening on this?
> 
> Please note that I'm planning to push the rest of the component updates
> during the next merge window, which will result in unconverted drivers
> breaking.
> 

Sorry for this. I was busy with other works. I will update and post it
until this week.

Thanks,
Inki Dae

> Thanks.
> 

[PATCH RFC v2 4/8] drm: msm: update to use component match support

[Russell King]

Update MSM's DRM driver to use the component match support rather than
add_components.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/msm/msm_drv.c | 83 ++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 0d2562fb681e..b6dc5573e445 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -905,12 +905,41 @@ static int compare_of(struct device *dev, void *data)
 {
 	return dev->of_node == data;
 }
+#else
+static int compare_dev(struct device *dev, void *data)
+{
+	return dev == data;
+}
+#endif
+
+static int msm_drm_bind(struct device *dev)
+{
+	return drm_platform_init(&msm_driver, to_platform_device(dev));
+}
+
+static void msm_drm_unbind(struct device *dev)
+{
+	drm_put_dev(platform_get_drvdata(to_platform_device(dev)));
+}
+
+static const struct component_master_ops msm_drm_ops = {
+	.bind = msm_drm_bind,
+	.unbind = msm_drm_unbind,
+};
+
+/*
+ * Platform driver:
+ */
 
-static int msm_drm_add_components(struct device *master, struct master *m)
+static int msm_pdev_probe(struct platform_device *pdev)
 {
-	struct device_node *np = master->of_node;
+	struct component_match *match = NULL;
+#ifdef CONFIG_OF
+	/* NOTE: the CONFIG_OF case duplicates the same code as exynos or imx
+	 * (or probably any other).. so probably some room for some helpers
+	 */
+	struct device_node *np = pdev->dev.of_node;
 	unsigned i;
-	int ret;
 
 	for (i = 0; ; i++) {
 		struct device_node *node;
@@ -919,22 +948,9 @@ static int msm_drm_add_components(struct device *master, struct master *m)
 		if (!node)
 			break;
 
-		ret = component_master_add_child(m, compare_of, node);
-		of_node_put(node);
-
-		if (ret)
-			return ret;
+		component_match_add(&pdev->dev, &match, compare_of, node);
 	}
-	return 0;
-}
 #else
-static int compare_dev(struct device *dev, void *data)
-{
-	return dev == data;
-}
-
-static int msm_drm_add_components(struct device *master, struct master *m)
-{
 	/* For non-DT case, it kinda sucks.  We don't actually have a way
 	 * to know whether or not we are waiting for certain devices (or if
 	 * they are simply not present).  But for non-DT we only need to
@@ -958,41 +974,12 @@ static int msm_drm_add_components(struct device *master, struct master *m)
 			return -EPROBE_DEFER;
 		}
 
-		ret = component_master_add_child(m, compare_dev, dev);
-		if (ret) {
-			DBG("could not add child: %d", ret);
-			return ret;
-		}
+		component_match_add(&pdev->dev, &match, compare_dev, dev);
 	}
-
-	return 0;
-}
 #endif
 
-static int msm_drm_bind(struct device *dev)
-{
-	return drm_platform_init(&msm_driver, to_platform_device(dev));
-}
-
-static void msm_drm_unbind(struct device *dev)
-{
-	drm_put_dev(platform_get_drvdata(to_platform_device(dev)));
-}
-
-static const struct component_master_ops msm_drm_ops = {
-		.add_components = msm_drm_add_components,
-		.bind = msm_drm_bind,
-		.unbind = msm_drm_unbind,
-};
-
-/*
- * Platform driver:
- */
-
-static int msm_pdev_probe(struct platform_device *pdev)
-{
 	pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
-	return component_master_add(&pdev->dev, &msm_drm_ops);
+	return component_master_add_with_match(&pdev->dev, &msm_drm_ops, match);
 }
 
 static int msm_pdev_remove(struct platform_device *pdev)
-- 
1.8.3.1

[PATCH RFC v2 5/8] imx-drm: update to use component match support

[Russell King]

Update the imx-drm driver to use the component match support rather than
add_components.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/staging/imx-drm/imx-drm-core.c | 57 +++-------------------------------
 1 file changed, 4 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index def8280d7ee6..47ee6c79857a 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -570,22 +570,6 @@ static int compare_of(struct device *dev, void *data)
 	return dev->of_node == np;
 }
 
-static LIST_HEAD(imx_drm_components);
-
-static int imx_drm_add_components(struct device *master, struct master *m)
-{
-	struct imx_drm_component *component;
-	int ret;
-
-	list_for_each_entry(component, &imx_drm_components, list) {
-		ret = component_master_add_child(m, compare_of,
-						 component->of_node);
-		if (ret)
-			return ret;
-	}
-	return 0;
-}
-
 static int imx_drm_bind(struct device *dev)
 {
 	return drm_platform_init(&imx_drm_driver, to_platform_device(dev));
@@ -597,43 +581,14 @@ static void imx_drm_unbind(struct device *dev)
 }
 
 static const struct component_master_ops imx_drm_ops = {
-	.add_components = imx_drm_add_components,
 	.bind = imx_drm_bind,
 	.unbind = imx_drm_unbind,
 };
 
-static struct imx_drm_component *imx_drm_find_component(struct device *dev,
-		struct device_node *node)
-{
-	struct imx_drm_component *component;
-
-	list_for_each_entry(component, &imx_drm_components, list)
-		if (component->of_node == node)
-			return component;
-
-	return NULL;
-}
-
-static int imx_drm_add_component(struct device *dev, struct device_node *node)
-{
-	struct imx_drm_component *component;
-
-	if (imx_drm_find_component(dev, node))
-		return 0;
-
-	component = devm_kzalloc(dev, sizeof(*component), GFP_KERNEL);
-	if (!component)
-		return -ENOMEM;
-
-	component->of_node = node;
-	list_add_tail(&component->list, &imx_drm_components);
-
-	return 0;
-}
-
 static int imx_drm_platform_probe(struct platform_device *pdev)
 {
 	struct device_node *ep, *port, *remote;
+	struct component_match *match = NULL;
 	int ret;
 	int i;
 
@@ -647,9 +602,7 @@ static int imx_drm_platform_probe(struct platform_device *pdev)
 		if (!port)
 			break;
 
-		ret = imx_drm_add_component(&pdev->dev, port);
-		if (ret < 0)
-			return ret;
+		component_match_add(&pdev->dev, &match, compare_of, port);
 	}
 
 	if (i == 0) {
@@ -675,10 +628,8 @@ static int imx_drm_platform_probe(struct platform_device *pdev)
 				continue;
 			}
 
-			ret = imx_drm_add_component(&pdev->dev, remote);
+			component_match_add(&pdev->dev, &match, compare_of, remote);
 			of_node_put(remote);
-			if (ret < 0)
-				return ret;
 		}
 		of_node_put(port);
 	}
@@ -687,7 +638,7 @@ static int imx_drm_platform_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return component_master_add(&pdev->dev, &imx_drm_ops);
+	return component_master_add_with_match(&pdev->dev, &imx_drm_ops, match);
 }
 
 static int imx_drm_platform_remove(struct platform_device *pdev)
-- 
1.8.3.1

[PATCH RFC v2 6/8] component: remove old add_components method

[Russell King]

Now that drivers create an array of component matches at probe time, we
can retire the old methods.  This involves removing the add_components
master method, and removing component_master_add_child() from public
view.  We also remove component_add_master() as that interface is no
longer useful.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/component.c  | 21 +--------------------
 include/linux/component.h |  5 -----
 2 files changed, 1 insertion(+), 25 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index bd8b4908593b..c0a5b1abc931 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -84,7 +84,7 @@ static void component_detach_master(struct master *master, struct component *c)
  * function and compare data.  This is safe to call for duplicate matches
  * and will not result in the same component being added multiple times.
  */
-int component_master_add_child(struct master *master,
+static int component_master_add_child(struct master *master,
 	int (*compare)(struct device *, void *), void *compare_data)
 {
 	struct component *c;
@@ -104,7 +104,6 @@ int component_master_add_child(struct master *master,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(component_master_add_child);
 
 static int find_components(struct master *master)
 {
@@ -112,14 +111,6 @@ static int find_components(struct master *master)
 	size_t i;
 	int ret = 0;
 
-	if (!match) {
-		/*
-		 * Search the list of components, looking for components that
-		 * belong to this master, and attach them to the master.
-		 */
-		return master->ops->add_components(master->dev, master);
-	}
-
 	/*
 	 * Scan the array of match functions and attach
 	 * any components which are found to this master.
@@ -289,9 +280,6 @@ int component_master_add_with_match(struct device *dev,
 	struct master *master;
 	int ret;
 
-	if (ops->add_components && match)
-		return -EINVAL;
-
 	/* Reallocate the match array for its true size */
 	match = component_match_realloc(dev, match, match->num);
 	if (IS_ERR(match))
@@ -323,13 +311,6 @@ int component_master_add_with_match(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(component_master_add_with_match);
 
-int component_master_add(struct device *dev,
-	const struct component_master_ops *ops)
-{
-	return component_master_add_with_match(dev, ops, NULL);
-}
-EXPORT_SYMBOL_GPL(component_master_add);
-
 void component_master_del(struct device *dev,
 	const struct component_master_ops *ops)
 {
diff --git a/include/linux/component.h b/include/linux/component.h
index c00dcc302611..71c434a6a5ee 100644
--- a/include/linux/component.h
+++ b/include/linux/component.h
@@ -17,18 +17,13 @@ void component_unbind_all(struct device *, void *);
 struct master;
 
 struct component_master_ops {
-	int (*add_components)(struct device *, struct master *);
 	int (*bind)(struct device *);
 	void (*unbind)(struct device *);
 };
 
-int component_master_add(struct device *, const struct component_master_ops *);
 void component_master_del(struct device *,
 	const struct component_master_ops *);
 
-int component_master_add_child(struct master *master,
-	int (*compare)(struct device *, void *), void *compare_data);
-
 struct component_match;
 
 int component_master_add_with_match(struct device *,
-- 
1.8.3.1

[PATCH RFC v2 7/8] component: move check for unbound master into try_to_bring_up_masters()

[Russell King]

Clean up the code a little; we don't need to check that the master is
unbound for every invocation of try_to_bring_up_master(), so let's move
it to where it's really needed - try_to_bring_up_masters(), where we may
encounter already bound masters.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/component.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index c0a5b1abc931..0863cfeea769 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -150,13 +150,6 @@ static int try_to_bring_up_master(struct master *master,
 {
 	int ret;
 
-	if (master->bound)
-		return 0;
-
-	/*
-	 * Search the list of components, looking for components that
-	 * belong to this master, and attach them to the master.
-	 */
 	if (find_components(master)) {
 		/* Failed to find all components */
 		ret = 0;
@@ -196,9 +189,11 @@ static int try_to_bring_up_masters(struct component *component)
 	int ret = 0;
 
 	list_for_each_entry(m, &masters, node) {
-		ret = try_to_bring_up_master(m, component);
-		if (ret != 0)
-			break;
+		if (!m->bound) {
+			ret = try_to_bring_up_master(m, component);
+			if (ret != 0)
+				break;
+		}
 	}
 
 	return ret;
-- 
1.8.3.1

[PATCH RFC v2 8/8] component: track components via array rather than list

[Russell King]

Since we now have an array which defines each component, maintain the
components to be bound in the array rather than a separate list.  We
also need duplicate tracking so we can eliminate multiple bind calls
for the same component: we preserve the list-based component order in
that the first match which adds the component determines its position.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/component.c | 140 +++++++++++++++++++++--------------------------
 1 file changed, 62 insertions(+), 78 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 0863cfeea769..8a1d784a9759 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -18,18 +18,21 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 
+struct component;
+
 struct component_match {
 	size_t alloc;
 	size_t num;
 	struct {
 		void *data;
 		int (*fn)(struct device *, void *);
+		struct component *component;
+		bool duplicate;
 	} compare[0];
 };
 
 struct master {
 	struct list_head node;
-	struct list_head components;
 	bool bound;
 
 	const struct component_master_ops *ops;
@@ -39,7 +42,6 @@ struct master {
 
 struct component {
 	struct list_head node;
-	struct list_head master_node;
 	struct master *master;
 	bool bound;
 
@@ -63,46 +65,20 @@ static struct master *__master_find(struct device *dev,
 	return NULL;
 }
 
-/* Attach an unattached component to a master. */
-static void component_attach_master(struct master *master, struct component *c)
-{
-	c->master = master;
-
-	list_add_tail(&c->master_node, &master->components);
-}
-
-/* Detach a component from a master. */
-static void component_detach_master(struct master *master, struct component *c)
-{
-	list_del(&c->master_node);
-
-	c->master = NULL;
-}
-
-/*
- * Add a component to a master, finding the component via the compare
- * function and compare data.  This is safe to call for duplicate matches
- * and will not result in the same component being added multiple times.
- */
-static int component_master_add_child(struct master *master,
+static struct component *find_component(struct master *master,
 	int (*compare)(struct device *, void *), void *compare_data)
 {
 	struct component *c;
-	int ret = -ENXIO;
 
 	list_for_each_entry(c, &component_list, node) {
 		if (c->master && c->master != master)
 			continue;
 
-		if (compare(c->dev, compare_data)) {
-			if (!c->master)
-				component_attach_master(master, c);
-			ret = 0;
-			break;
-		}
+		if (compare(c->dev, compare_data))
+			return c;
 	}
 
-	return ret;
+	return NULL;
 }
 
 static int find_components(struct master *master)
@@ -116,26 +92,35 @@ static int find_components(struct master *master)
 	 * any components which are found to this master.
 	 */
 	for (i = 0; i < match->num; i++) {
-		ret = component_master_add_child(master,
-						 match->compare[i].fn,
-						 match->compare[i].data);
-		if (ret)
+		struct component *c;
+
+		if (match->compare[i].component)
+			continue;
+
+		c = find_component(master, match->compare[i].fn,
+				   match->compare[i].data);
+		if (!c) {
+			ret = -ENXIO;
 			break;
+		}
+
+		/* Attach this component to the master */
+		match->compare[i].duplicate = !!c->master;
+		match->compare[i].component = c;
+		c->master = master;
 	}
 	return ret;
 }
 
-/* Detach all attached components from this master */
-static void master_remove_components(struct master *master)
+/* Detach component from associated master */
+static void remove_component(struct master *master, struct component *c)
 {
-	while (!list_empty(&master->components)) {
-		struct component *c = list_first_entry(&master->components,
-					struct component, master_node);
-
-		WARN_ON(c->master != master);
+	size_t i;
 
-		component_detach_master(master, c);
-	}
+	/* Detach the component from this master. */
+	for (i = 0; i < master->match->num; i++)
+		if (master->match->compare[i].component == c)
+			master->match->compare[i].component = NULL;
 }
 
 /*
@@ -150,37 +135,25 @@ static int try_to_bring_up_master(struct master *master,
 {
 	int ret;
 
-	if (find_components(master)) {
-		/* Failed to find all components */
-		ret = 0;
-		goto out;
-	}
+	if (find_components(master))
+		return 0;
 
-	if (component && component->master != master) {
-		ret = 0;
-		goto out;
-	}
+	if (component && component->master != master)
+		return 0;
 
-	if (!devres_open_group(master->dev, NULL, GFP_KERNEL)) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!devres_open_group(master->dev, NULL, GFP_KERNEL))
+		return -ENOMEM;
 
 	/* Found all components */
 	ret = master->ops->bind(master->dev);
 	if (ret < 0) {
 		devres_release_group(master->dev, NULL);
 		dev_info(master->dev, "master bind failed: %d\n", ret);
-		goto out;
+		return ret;
 	}
 
 	master->bound = true;
 	return 1;
-
-out:
-	master_remove_components(master);
-
-	return ret;
 }
 
 static int try_to_bring_up_masters(struct component *component)
@@ -206,8 +179,6 @@ static void take_down_master(struct master *master)
 		devres_release_group(master->dev, NULL);
 		master->bound = false;
 	}
-
-	master_remove_components(master);
 }
 
 static size_t component_match_size(size_t num)
@@ -265,6 +236,7 @@ void component_match_add(struct device *dev, struct component_match **matchptr,
 
 	match->compare[match->num].fn = compare;
 	match->compare[match->num].data = compare_data;
+	match->compare[match->num].component = NULL;
 	match->num++;
 }
 
@@ -287,7 +259,6 @@ int component_master_add_with_match(struct device *dev,
 	master->dev = dev;
 	master->ops = ops;
 	master->match = match;
-	INIT_LIST_HEAD(&master->components);
 
 	/* Add to the list of available masters. */
 	mutex_lock(&component_mutex);
@@ -339,6 +310,7 @@ void component_unbind_all(struct device *master_dev, void *data)
 {
 	struct master *master;
 	struct component *c;
+	size_t i;
 
 	WARN_ON(!mutex_is_locked(&component_mutex));
 
@@ -346,8 +318,12 @@ void component_unbind_all(struct device *master_dev, void *data)
 	if (!master)
 		return;
 
-	list_for_each_entry_reverse(c, &master->components, master_node)
-		component_unbind(c, master, data);
+	/* Unbind components in reverse order */
+	for (i = master->match->num; i--; )
+		if (!master->match->compare[i].duplicate) {
+			c = master->match->compare[i].component;
+			component_unbind(c, master, data);
+		}
 }
 EXPORT_SYMBOL_GPL(component_unbind_all);
 
@@ -407,6 +383,7 @@ int component_bind_all(struct device *master_dev, void *data)
 {
 	struct master *master;
 	struct component *c;
+	size_t i;
 	int ret = 0;
 
 	WARN_ON(!mutex_is_locked(&component_mutex));
@@ -415,16 +392,21 @@ int component_bind_all(struct device *master_dev, void *data)
 	if (!master)
 		return -EINVAL;
 
-	list_for_each_entry(c, &master->components, master_node) {
-		ret = component_bind(c, master, data);
-		if (ret)
-			break;
-	}
+	/* Bind components in match order */
+	for (i = 0; i < master->match->num; i++)
+		if (!master->match->compare[i].duplicate) {
+			c = master->match->compare[i].component;
+			ret = component_bind(c, master, data);
+			if (ret)
+				break;
+		}
 
 	if (ret != 0) {
-		list_for_each_entry_continue_reverse(c, &master->components,
-						     master_node)
-			component_unbind(c, master, data);
+		for (; i--; )
+			if (!master->match->compare[i].duplicate) {
+				c = master->match->compare[i].component;
+				component_unbind(c, master, data);
+			}
 	}
 
 	return ret;
@@ -472,8 +454,10 @@ void component_del(struct device *dev, const struct component_ops *ops)
 			break;
 		}
 
-	if (component && component->master)
+	if (component && component->master) {
 		take_down_master(component->master);
+		remove_component(component->master, component);
+	}
 
 	mutex_unlock(&component_mutex);
 
-- 
1.8.3.1