[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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[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 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

Re: [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.

Re: [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

Re: [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.