diff mbox

[RFC,v3,4/4] drm/armada: Convert the probe function to the generic drm_of_component_probe()

Message ID 1445267270-23126-5-git-send-email-Liviu.Dudau@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liviu Dudau Oct. 19, 2015, 3:07 p.m. UTC
The armada DRM driver keeps some old platform data compatibility in the
probe function that makes moving to the generic drm_of_component_probe()
a bit more complicated that it should. Refactor the probe function to do
the platform_data processing after the generic probe (and only if that
fails). This way future cleanup can further remove support for it.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/armada/armada_drv.c | 73 +++++++++++--------------------------
 1 file changed, 22 insertions(+), 51 deletions(-)

Comments

Russell King - ARM Linux Oct. 19, 2015, 3:17 p.m. UTC | #1
On Mon, Oct 19, 2015 at 04:07:50PM +0100, Liviu Dudau wrote:
> The armada DRM driver keeps some old platform data compatibility in the
> probe function that makes moving to the generic drm_of_component_probe()
> a bit more complicated that it should. Refactor the probe function to do
> the platform_data processing after the generic probe (and only if that
> fails). This way future cleanup can further remove support for it.

I'm mainly using the old code, even with DT based platforms, because it
needs a block of reserved memory which I haven't got around to converting
to DT stuff (each time I've looked into doing that, bits for dealing with
reserved memory nodes were missing.)

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Liviu Dudau Oct. 19, 2015, 3:23 p.m. UTC | #2
On Mon, Oct 19, 2015 at 04:17:14PM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 19, 2015 at 04:07:50PM +0100, Liviu Dudau wrote:
> > The armada DRM driver keeps some old platform data compatibility in the
> > probe function that makes moving to the generic drm_of_component_probe()
> > a bit more complicated that it should. Refactor the probe function to do
> > the platform_data processing after the generic probe (and only if that
> > fails). This way future cleanup can further remove support for it.
> 
> I'm mainly using the old code, even with DT based platforms, because it
> needs a block of reserved memory which I haven't got around to converting
> to DT stuff (each time I've looked into doing that, bits for dealing with
> reserved memory nodes were missing.)

Thanks for explaining this!

> 
> Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

And thanks for all the ACKs!

Best regards,
Liviu

> 
> -- 
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
>
kernel test robot Oct. 19, 2015, 10:07 p.m. UTC | #3
Hi Liviu,

[auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Liviu-Dudau/drm-Introduce-generic-probe-function-for-component-based-masters/20151019-231229
config: arm-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/armada/armada_drv.c: In function 'armada_drm_probe':
>> drivers/gpu/drm/armada/armada_drv.c:291:4: warning: passing argument 2 of 'component_match_add' from incompatible pointer type
       component_match_add(&pdev->dev, match, compare_dev_name,
       ^
   In file included from drivers/gpu/drm/armada/armada_drv.c:9:0:
   include/linux/component.h:36:6: note: expected 'struct component_match **' but argument is of type 'struct component_match *'
    void component_match_add(struct device *, struct component_match **,
         ^
>> drivers/gpu/drm/armada/armada_drv.c:304:6: warning: passing argument 2 of 'armada_add_endpoints' from incompatible pointer type
         armada_add_endpoints(&pdev->dev, match,
         ^
   drivers/gpu/drm/armada/armada_drv.c:247:13: note: expected 'struct component_match **' but argument is of type 'struct component_match *'
    static void armada_add_endpoints(struct device *dev,
                ^

vim +/component_match_add +291 drivers/gpu/drm/armada/armada_drv.c

   285			char **devices = pdev->dev.platform_data;
   286			struct device_node *port;
   287			struct device *d;
   288			int i;
   289	
   290			for (i = 0; devices[i]; i++)
 > 291				component_match_add(&pdev->dev, match, compare_dev_name,
   292						    devices[i]);
   293	
   294			if (i == 0) {
   295				dev_err(&pdev->dev, "missing 'ports' property\n");
   296				return -ENODEV;
   297			}
   298	
   299			for (i = 0; devices[i]; i++) {
   300				d = bus_find_device_by_name(&platform_bus_type, NULL,
   301							    devices[i]);
   302				if (d && d->of_node) {
   303					for_each_child_of_node(d->of_node, port)
 > 304						armada_add_endpoints(&pdev->dev, match,
   305								     port);
   306				}
   307				put_device(d);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 63d909e..25c42f3 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -11,6 +11,7 @@ 
 #include <linux/of_graph.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_of.h>
 #include "armada_crtc.h"
 #include "armada_drm.h"
 #include "armada_gem.h"
@@ -265,78 +266,48 @@  static void armada_add_endpoints(struct device *dev,
 	}
 }
 
-static int armada_drm_find_components(struct device *dev,
-	struct component_match **match)
-{
-	struct device_node *port;
-	int i;
-
-	if (dev->of_node) {
-		struct device_node *np = dev->of_node;
-
-		for (i = 0; ; i++) {
-			port = of_parse_phandle(np, "ports", i);
-			if (!port)
-				break;
-
-			component_match_add(dev, match, compare_of, port);
-			of_node_put(port);
-		}
+static const struct component_master_ops armada_master_ops = {
+	.bind = armada_drm_bind,
+	.unbind = armada_drm_unbind,
+};
 
-		if (i == 0) {
-			dev_err(dev, "missing 'ports' property\n");
-			return -ENODEV;
-		}
+static int armada_drm_probe(struct platform_device *pdev)
+{
+	struct component_match *match = NULL;
+	int ret;
 
-		for (i = 0; ; i++) {
-			port = of_parse_phandle(np, "ports", i);
-			if (!port)
-				break;
+	ret = drm_of_component_probe(&pdev->dev, compare_dev_name,
+				     &armada_master_ops);
+	if (ret != -EINVAL)
+		return ret;
 
-			armada_add_endpoints(dev, match, port);
-			of_node_put(port);
-		}
-	} else if (dev->platform_data) {
-		char **devices = dev->platform_data;
+	if (pdev->dev.platform_data) {
+		char **devices = pdev->dev.platform_data;
+		struct device_node *port;
 		struct device *d;
+		int i;
 
 		for (i = 0; devices[i]; i++)
-			component_match_add(dev, match, compare_dev_name,
+			component_match_add(&pdev->dev, match, compare_dev_name,
 					    devices[i]);
 
 		if (i == 0) {
-			dev_err(dev, "missing 'ports' property\n");
+			dev_err(&pdev->dev, "missing 'ports' property\n");
 			return -ENODEV;
 		}
 
 		for (i = 0; devices[i]; i++) {
 			d = bus_find_device_by_name(&platform_bus_type, NULL,
-					devices[i]);
+						    devices[i]);
 			if (d && d->of_node) {
 				for_each_child_of_node(d->of_node, port)
-					armada_add_endpoints(dev, match, port);
+					armada_add_endpoints(&pdev->dev, match,
+							     port);
 			}
 			put_device(d);
 		}
 	}
 
-	return 0;
-}
-
-static const struct component_master_ops armada_master_ops = {
-	.bind = armada_drm_bind,
-	.unbind = armada_drm_unbind,
-};
-
-static int armada_drm_probe(struct platform_device *pdev)
-{
-	struct component_match *match = NULL;
-	int ret;
-
-	ret = armada_drm_find_components(&pdev->dev, &match);
-	if (ret < 0)
-		return ret;
-
 	return component_master_add_with_match(&pdev->dev, &armada_master_ops,
 					       match);
 }