From c334940ea26cb58f5514dcbb225a3f397b2684ad Mon Sep 17 00:00:00 2001 From: Russell King Date: Wed, 23 Apr 2014 10:52:17 +0100 Subject: [PATCH 1/5] component: fix missed cleanup in case of devres failure 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. Acked-by: Laurent Pinchart Signed-off-by: Russell King --- drivers/base/component.c | 68 ++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 34 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; - } - - if (!devres_open_group(master->dev, NULL, GFP_KERNEL)) { - ret = -ENOMEM; - 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; - } - - master->bound = true; - ret = 1; + /* + * 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 (component && component->master != master) { + ret = 0; + goto out; + } + + if (!devres_open_group(master->dev, NULL, GFP_KERNEL)) { + ret = -ENOMEM; + 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); + goto out; + } + + master->bound = true; + return 1; + out: + master_remove_components(master); return ret; } From fcbcebce7159c928692dc6a5e88869f6e44438b9 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 18 Apr 2014 20:16:22 +0100 Subject: [PATCH 2/5] component: ignore multiple additions of the same component 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. Acked-by: Laurent Pinchart Signed-off-by: Russell King --- 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; } From 6955b58254c2bcee8a7b55ce06468a645dc98ec5 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sat, 19 Apr 2014 11:18:01 +0100 Subject: [PATCH 3/5] component: add support for component match array 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. Acked-by: Laurent Pinchart Signed-off-by: Russell King --- 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 #include +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 From b509cc802239a8b5ba7d1d2cc5adfb9d984b7ed8 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 4 Jul 2014 13:23:46 +0100 Subject: [PATCH 4/5] component: fix bug with legacy API Sachin Kamat reports that "component: add support for component match array" broke Exynos DRM due to a NULL pointer deref. Fix this. Reported-by: Sachin Kamat Tested-by: Sachin Kamat Signed-off-by: Russell King --- drivers/base/component.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/base/component.c b/drivers/base/component.c index b4236daed4fa..f748430bb654 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -293,10 +293,12 @@ int component_master_add_with_match(struct device *dev, 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); + if (match) { + /* 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) From 9e5e2ffda320bacb761e4812e3ef7a7decd592a9 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sat, 19 Apr 2014 11:21:33 +0100 Subject: [PATCH 5/5] imx-drm: update to use component match support Update the imx-drm driver to use the component match support rather than add_components. Signed-off-by: Russell King --- 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)