From patchwork Wed Aug 3 15:04:01 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benoit Cousson X-Patchwork-Id: 1031792 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p73F4VbZ013990 for ; Wed, 3 Aug 2011 15:04:31 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754846Ab1HCPET (ORCPT ); Wed, 3 Aug 2011 11:04:19 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:56839 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754820Ab1HCPEQ (ORCPT ); Wed, 3 Aug 2011 11:04:16 -0400 Received: from dlep33.itg.ti.com ([157.170.170.112]) by bear.ext.ti.com (8.13.7/8.13.7) with ESMTP id p73F44S0014009 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 3 Aug 2011 10:04:04 -0500 Received: from dlep26.itg.ti.com (smtp-le.itg.ti.com [157.170.170.27]) by dlep33.itg.ti.com (8.13.7/8.13.8) with ESMTP id p73F435l014484; Wed, 3 Aug 2011 10:04:03 -0500 (CDT) Received: from dlee73.ent.ti.com (localhost [127.0.0.1]) by dlep26.itg.ti.com (8.13.8/8.13.8) with ESMTP id p73F436c024717; Wed, 3 Aug 2011 10:04:03 -0500 (CDT) Received: from dlelxv22.itg.ti.com (172.17.1.197) by DLEE73.ent.ti.com (157.170.170.88) with Microsoft SMTP Server id 8.3.106.1; Wed, 3 Aug 2011 10:04:03 -0500 Received: from [137.167.125.104] (cnc0919096.emea.dhcp.ti.com [137.167.125.104]) by dlelxv22.itg.ti.com (8.13.8/8.13.8) with ESMTP id p73F41KK012399; Wed, 3 Aug 2011 10:04:01 -0500 Message-ID: <4E396361.3060307@ti.com> Date: Wed, 3 Aug 2011 17:04:01 +0200 From: "Cousson, Benoit" Organization: Texas Instruments User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:5.0) Gecko/20110624 Thunderbird/5.0 MIME-Version: 1.0 To: Grant Likely CC: "G, Manjunath Kondaiah" , "Nayak, Rajendra" , "Hilman, Kevin" , Paul Walmsley , linux-omap , "devicetree-discuss@lists.ozlabs.org" , "linux-arm-kernel@lists.infradead.org" Subject: Shouldn't DT preserve pdev name and id to allow platform_match to work? Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Wed, 03 Aug 2011 15:04:32 +0000 (UTC) Hi Grant, Going further with the usage of OF_DEV_AUXDATA_ID, I realized that this is is not doing what I was expecting. My expectation might be silly, but in order to make platform_match to work without DT matching mechanism, you need to have the driver name in the pdev->name field: /* fall-back to driver name match */ return (strcmp(pdev->name, drv->name) == 0); Except that the of_device_add function is doing that: pdev->name = dev_name(&ofdev->dev); pdev->id = -1; Thus overwriting the original pdev->name with the dev_name / bus_id in DT case. Whereas the regular naming convention with pdev is to have dev_name = .". Because of that, we cannot match the proper driver name with this entry: OF_DEV_AUXDATA_ID("ti,omap-i2c", 0x48000100, "omap-i2c.1", 1, &i2c_pdata) pdev->id will be properly overwritten after device creation, but not the pdev->name. pdev->name will be "omap-i2c.1" instead of "omap-i2c". By changing a little bit the format like that (removing the id from bus_id that is BTW redundant in that case): OF_DEV_AUXDATA_ID("ti,omap-i2c", 0x48000100, "omap-i2c", 1, &i2c_pdata) We will be able to maintain the original driver name inside pdev->name and thus match correctly during probe with the legacy method. Since DT is building a real platform_device, it seems to be reasonable to maintain the legacy name. Moreover that will allow a basic driver to still probe correctly its devices. Please find after a quick and dirty proof of concept. Does that make sense to you? Regards, Benoit --- -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/of/device.c b/drivers/of/device.c index 62b4b32..04727e4 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -51,11 +51,6 @@ int of_device_add(struct platform_device *ofdev) { BUG_ON(ofdev->dev.of_node == NULL); - /* name and id have to be set so that the platform bus doesn't get - * confused on matching */ - ofdev->name = dev_name(&ofdev->dev); - ofdev->id = -1; - /* device_add will assume that this device is on the same node as * the parent. If there is no parent defined, set the node * explicitly */ diff --git a/drivers/of/platform.c b/drivers/of/platform.c index ebbbf42..f7b6ec1 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -130,13 +130,14 @@ void of_device_make_bus_id(struct device *dev) */ struct platform_device *of_device_alloc(struct device_node *np, const char *bus_id, - struct device *parent) + struct device *parent, + int id) { struct platform_device *dev; int rc, i, num_reg = 0, num_irq; struct resource *res, temp_res; - dev = platform_device_alloc("", -1); + dev = platform_device_alloc(bus_id ? bus_id : "", id); if (!dev) return NULL; @@ -169,7 +170,10 @@ struct platform_device *of_device_alloc(struct device_node *np, dev->dev.parent = parent; if (bus_id) - dev_set_name(&dev->dev, "%s", bus_id); + if (id == -1) + dev_set_name(&dev->dev, "%s", bus_id); + else + dev_set_name(&dev->dev, "%s.%d", bus_id, id); else of_device_make_bus_id(&dev->dev); @@ -191,14 +195,15 @@ struct platform_device *of_platform_device_create_pdata( struct device_node *np, const char *bus_id, void *platform_data, - struct device *parent) + struct device *parent, + int id) { struct platform_device *dev; if (!of_device_is_available(np)) return NULL; - dev = of_device_alloc(np, bus_id, parent); + dev = of_device_alloc(np, bus_id, parent, id); if (!dev) return NULL; @@ -235,7 +240,7 @@ struct platform_device *of_platform_device_create(struct device_node *np, const char *bus_id, struct device *parent) { - return of_platform_device_create_pdata(np, bus_id, NULL, parent); + return of_platform_device_create_pdata(np, bus_id, NULL, parent, -1); } EXPORT_SYMBOL(of_platform_device_create); @@ -595,11 +600,7 @@ static int of_platform_bus_create(struct device_node *bus, return 0; } - dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent); - - /* override the id if auxdata gives an id */ - if (id != -1) - dev->id = id; + dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent, id); if (!dev || !of_match_node(matches, bus)) return 0; diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h index 252246c..9d3cbbe 100644 --- a/include/linux/of_platform.h +++ b/include/linux/of_platform.h @@ -84,7 +84,7 @@ extern const struct of_device_id of_default_bus_match_table[]; /* Platform drivers register/unregister */ extern struct platform_device *of_device_alloc(struct device_node *np, const char *bus_id, - struct device *parent); + struct device *parent, int id); extern struct platform_device *of_find_device_by_node(struct device_node *np); #if !defined(CONFIG_SPARC) /* SPARC has its own device registration method */