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(-)
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; }
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; }
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
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
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.
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.
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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
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.
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.
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.
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)
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)
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 *,
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;
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);
dri-devel@lists.freedesktop.org
-
Inki Dae -
Philipp Zabel -
Russell King -
Russell King - ARM Linux