[PATCH v3 0/8] component helper improvements

[PATCH v3 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.

I hope that this is the final submission in patch form.  Ultimately,
these patches need to be handled carefully:

Patch		Trees
1, 2, 3		Driver, Staging, DRM
4		Driver, DRM
5		Driver, Staging
6, 7, 8		To be held back until all users are updated (Exynos DRM)

 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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 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>
---
This patch is destined for Greg's driver and staging trees, and David's DRM
tree.

 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 v3 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>
---
This patch is destined for Greg's driver and staging trees, and David's DRM
tree.

 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 v3 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>
---
This patch is destined for Greg's driver and staging trees, and David's DRM
tree.

 drivers/base/component.c  | 120 ++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/component.h |   7 +++
 2 files changed, 124 insertions(+), 3 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 55813e91bf0d..b4236daed4fa 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,87 @@ 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++;
+}
+EXPORT_SYMBOL(component_match_add);
+
+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 +322,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 v3 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>
---
This patch is destined for Greg's driver and David's DRM trees.

 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 9a5d87db5c23..a322029983ce 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 v3 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>
---
This patch is destined for Greg's driver and staging trees.

 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 v3 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>
---
This patch should not be applied until all users are updated.

 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 b4236daed4fa..2ca22738ae92 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.
@@ -290,9 +281,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))
@@ -324,13 +312,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 v3 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>
---
This patch should not be applied until all users are updated.

 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 2ca22738ae92..cd70b68d9780 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 v3 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>
---
This patch should not be applied until all users are updated.

 drivers/base/component.c | 154 ++++++++++++++++++++++++-----------------------
 1 file changed, 80 insertions(+), 74 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index cd70b68d9780..a6f2050d17b7 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,39 @@ 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;
+
+		dev_dbg(master->dev, "Looking for component %u\n", i);
+
+		if (match->compare[i].component)
+			continue;
+
+		c = find_component(master, match->compare[i].fn,
+				   match->compare[i].data);
+		if (!c) {
+			ret = -ENXIO;
 			break;
+		}
+
+		dev_dbg(master->dev, "found component %s, duplicate %u\n", dev_name(c->dev), !!c->master);
+
+		/* 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 +139,32 @@ static int try_to_bring_up_master(struct master *master,
 {
 	int ret;
 
+	dev_dbg(master->dev, "trying to bring up master\n");
+
 	if (find_components(master)) {
-		/* Failed to find all components */
-		ret = 0;
-		goto out;
+		dev_dbg(master->dev, "master has incomplete components\n");
+		return 0;
 	}
 
 	if (component && component->master != master) {
-		ret = 0;
-		goto out;
+		dev_dbg(master->dev, "master is not for this component (%s)\n",
+			dev_name(component->dev));
+		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 +190,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 +247,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++;
 }
 EXPORT_SYMBOL(component_match_add);
@@ -288,7 +271,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);
@@ -311,13 +293,24 @@ void component_master_del(struct device *dev,
 	const struct component_master_ops *ops)
 {
 	struct master *master;
+	int i;
 
 	mutex_lock(&component_mutex);
 	master = __master_find(dev, ops);
 	if (master) {
+		struct component_match *match = master->match;
+
 		take_down_master(master);
 
 		list_del(&master->node);
+
+		if (match) {
+			for (i = 0; i < match->num; i++) {
+				struct component *c = match->compare[i].component;
+				if (c)
+					c->master = NULL;
+			}
+		}
 		kfree(master);
 	}
 	mutex_unlock(&component_mutex);
@@ -340,6 +333,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));
 
@@ -347,8 +341,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);
 
@@ -408,6 +406,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));
@@ -416,16 +415,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;
@@ -473,8 +477,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

[PATCH v3 0/8] component helper improvements

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 03:40:11PM +0100, Russell King - ARM Linux wrote:
> A while back, Laurent raised some comments about the component helper,
> which this patch set starts to address.

I looked back over the two other times which this series has posted,
and noticed that two patches had been reviewed, so I've added those
tags.

Unless there's any objections from anyone, I'll send the first three
off to Greg either tonight or tomorrow night, which at least gets us
moving forward on this.

If anyone has any objections, please shout ASAP.  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 v3 0/8] component helper improvements

[Laurent Pinchart]

Hi Russell,

Sorry for the late review.

On Wednesday 02 July 2014 15:59:04 Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 03:40:11PM +0100, Russell King - ARM Linux wrote:
> > A while back, Laurent raised some comments about the component helper,
> > which this patch set starts to address.
> 
> I looked back over the two other times which this series has posted,
> and noticed that two patches had been reviewed, so I've added those
> tags.
> 
> Unless there's any objections from anyone, I'll send the first three
> off to Greg either tonight or tomorrow night, which at least gets us
> moving forward on this.
> 
> If anyone has any objections, please shout ASAP.  Thanks.

No objection from my side at all, this is a nice improvement. For the first 
three patches,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

[PATCH v3 0/8] component helper improvements

[Russell King - ARM Linux]

On Thu, Jul 03, 2014 at 01:51:19AM +0200, Laurent Pinchart wrote:
> On Wednesday 02 July 2014 15:59:04 Russell King - ARM Linux wrote:
> > On Tue, Jul 01, 2014 at 03:40:11PM +0100, Russell King - ARM Linux wrote:
> > > A while back, Laurent raised some comments about the component helper,
> > > which this patch set starts to address.
> > 
> > I looked back over the two other times which this series has posted,
> > and noticed that two patches had been reviewed, so I've added those
> > tags.
> > 
> > Unless there's any objections from anyone, I'll send the first three
> > off to Greg either tonight or tomorrow night, which at least gets us
> > moving forward on this.
> > 
> > If anyone has any objections, please shout ASAP.  Thanks.
> 
> No objection from my side at all, this is a nice improvement. For the first 
> three patches,
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, ack added.

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