Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/950/?format=api
{ "id": 950, "url": "http://patchwork.ozlabs.org/api/patches/950/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/200809222110.m8MLAL9w029890@imap1.linux-foundation.org/", "project": { "id": 7, "url": "http://patchwork.ozlabs.org/api/projects/7/?format=api", "name": "Linux network development", "link_name": "netdev", "list_id": "netdev.vger.kernel.org", "list_email": "netdev@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null, "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<200809222110.m8MLAL9w029890@imap1.linux-foundation.org>", "list_archive_url": null, "date": "2008-09-22T21:10:20", "name": "[for,2.6.27?,06/10] ne.c: fix rmmod, platform driver improvements", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "0f80b8c7bfc1186060023cfea2619b8551af3745", "submitter": { "id": 107, "url": "http://patchwork.ozlabs.org/api/people/107/?format=api", "name": "Andrew Morton", "email": "akpm@linux-foundation.org" }, "delegate": { "id": 36, "url": "http://patchwork.ozlabs.org/api/users/36/?format=api", "username": "jgarzik", "first_name": "Jeff", "last_name": "Garzik", "email": "jgarzik@pobox.com" }, "mbox": "http://patchwork.ozlabs.org/project/netdev/patch/200809222110.m8MLAL9w029890@imap1.linux-foundation.org/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/950/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/950/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<netdev-owner@vger.kernel.org>", "X-Original-To": "patchwork-incoming@ozlabs.org", "Delivered-To": "patchwork-incoming@ozlabs.org", "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.176.167])\n\tby ozlabs.org (Postfix) with ESMTP id 66820DDDE7\n\tfor <patchwork-incoming@ozlabs.org>;\n\tTue, 23 Sep 2008 07:17:34 +1000 (EST)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753410AbYIVVRH (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tMon, 22 Sep 2008 17:17:07 -0400", "(majordomo@vger.kernel.org) by vger.kernel.org id S1753409AbYIVVRG\n\t(ORCPT <rfc822; netdev-outgoing>); Mon, 22 Sep 2008 17:17:06 -0400", "from smtp1.linux-foundation.org ([140.211.169.13]:40094 \"EHLO\n\tsmtp1.linux-foundation.org\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753389AbYIVVRD (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Mon, 22 Sep 2008 17:17:03 -0400", "from imap1.linux-foundation.org (imap1.linux-foundation.org\n\t[140.211.169.55])\n\tby smtp1.linux-foundation.org (8.14.2/8.13.5/Debian-3ubuntu1.1) with\n\tESMTP id m8MLDDuN014806\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO);\n\tMon, 22 Sep 2008 14:15:44 -0700", "from localhost.localdomain (localhost [127.0.0.1])\n\tby imap1.linux-foundation.org\n\t(8.13.5.20060308/8.13.5/Debian-3ubuntu1.1) with ESMTP id\n\tm8MLAL9w029890; Mon, 22 Sep 2008 14:10:21 -0700" ], "Message-Id": "<200809222110.m8MLAL9w029890@imap1.linux-foundation.org>", "Subject": "[patch for 2.6.27? 06/10] ne.c: fix rmmod,\n\tplatform driver improvements", "To": "jeff@garzik.org", "Cc": "netdev@vger.kernel.org, akpm@linux-foundation.org, david@fries.net,\n\talan@lxorguk.ukuu.org.uk, anemo@mba.ocn.ne.jp, p_gortmaker@yahoo.com", "From": "akpm@linux-foundation.org", "Date": "Mon, 22 Sep 2008 14:10:20 -0700", "X-Spam-Status": "No, hits=-3.358 required=5 tests=AWL, BAYES_00,\n\tOSDL_HEADER_SUBJECT_BRACKETED", "X-Spam-Checker-Version": "SpamAssassin 3.2.4-osdl_revision__1.47__", "X-MIMEDefang-Filter": "lf$Revision: 1.188 $", "X-Scanned-By": "MIMEDefang 2.63 on 140.211.169.13", "Sender": "netdev-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<netdev.vger.kernel.org>", "X-Mailing-List": "netdev@vger.kernel.org" }, "content": "From: David Fries <david@fries.net>\n\nRemoving the module would cause a kernel oops as platform_driver_probe\nfailed to detect a device and unregistered the platform driver on module\ninit, and cleanup_module would unregister the already unregistered driver.\nThe suspend and resume functions weren't being called.\n\nplatform_driver support was added earlier, but without any\nplatform_device_register* calls I don't think it was being used. Now all\ndevices are registered using platform_device_register_simple and pointers\nare kept to unregister the ones that the probe failed for or unregister\nall devices on module shutdown. init_module no longer calls ne_init to\nreduce confusion (and multiple unregister paths that caused the rmmod\noops). With the devices now registered they are added to the platform\ndriver and get suspend and resume events.\n\nnetif_device_detach(dev) was added before unregister_netdev(dev) when\nremoving the region as occationally I would see a race condition where the\ndevice was still being used in unregister_netdev.\n\nSigned-off-by: David Fries <david@fries.net>\nCc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>\nCc: Paul Gortmaker <p_gortmaker@yahoo.com>\nCc: Alan Cox <alan@lxorguk.ukuu.org.uk>\nCc: Jeff Garzik <jeff@garzik.org>\nSigned-off-by: Andrew Morton <akpm@linux-foundation.org>\n---\n\n drivers/net/ne.c | 272 +++++++++++++++++++++++++++------------------\n 1 file changed, 164 insertions(+), 108 deletions(-)", "diff": "diff -puN drivers/net/ne.c~nec-fix-rmmod-platform-driver-improvements drivers/net/ne.c\n--- a/drivers/net/ne.c~nec-fix-rmmod-platform-driver-improvements\n+++ a/drivers/net/ne.c\n@@ -64,6 +64,25 @@ static const char version2[] =\n \n /* Do we support clones that don't adhere to 14,15 of the SAprom ? */\n #define SUPPORT_NE_BAD_CLONES\n+/* 0xbad = bad sig or no reset ack */\n+#define BAD 0xbad\n+\n+#define MAX_NE_CARDS\t4\t/* Max number of NE cards per module */\n+static struct platform_device *pdev_ne[MAX_NE_CARDS];\n+static int io[MAX_NE_CARDS];\n+static int irq[MAX_NE_CARDS];\n+static int bad[MAX_NE_CARDS];\n+\n+#ifdef MODULE\n+module_param_array(io, int, NULL, 0);\n+module_param_array(irq, int, NULL, 0);\n+module_param_array(bad, int, NULL, 0);\n+MODULE_PARM_DESC(io, \"I/O base address(es),required\");\n+MODULE_PARM_DESC(irq, \"IRQ number(s)\");\n+MODULE_PARM_DESC(bad, \"Accept card(s) with bad signatures\");\n+MODULE_DESCRIPTION(\"NE1000/NE2000 ISA/PnP Ethernet driver\");\n+MODULE_LICENSE(\"GPL\");\n+#endif /* MODULE */\n \n /* Do we perform extra sanity checks on stuff ? */\n /* #define NE_SANITY_CHECK */\n@@ -74,6 +93,10 @@ static const char version2[] =\n /* Do we have a non std. amount of memory? (in units of 256 byte pages) */\n /* #define PACKETBUF_MEMSIZE\t0x40 */\n \n+/* This is set up so that no ISA autoprobe takes place. We can't guarantee\n+that the ne2k probe is the last 8390 based probe to take place (as it\n+is at boot) and so the probe will get confused by any other 8390 cards.\n+ISA device autoprobes on a running machine are not recommended anyway. */\n #if !defined(MODULE) && (defined(CONFIG_ISA) || defined(CONFIG_M32R))\n /* Do we need a portlist for the ISA auto-probe ? */\n #define NEEDS_PORTLIST\n@@ -192,8 +215,13 @@ static int __init do_ne_probe(struct net\n #endif\n \n \t/* First check any supplied i/o locations. User knows best. <cough> */\n-\tif (base_addr > 0x1ff)\t/* Check a single specified location. */\n-\t\treturn ne_probe1(dev, base_addr);\n+\tif (base_addr > 0x1ff) {\t/* Check a single specified location. */\n+\t\tint ret = ne_probe1(dev, base_addr);\n+\t\tif (ret)\n+\t\t\tprintk(KERN_WARNING \"ne.c: No NE*000 card found at \"\n+\t\t\t\t\"i/o = %#lx\\n\", base_addr);\n+\t\treturn ret;\n+\t}\n \telse if (base_addr != 0)\t/* Don't probe at all. */\n \t\treturn -ENXIO;\n \n@@ -214,28 +242,6 @@ static int __init do_ne_probe(struct net\n \treturn -ENODEV;\n }\n \n-#ifndef MODULE\n-struct net_device * __init ne_probe(int unit)\n-{\n-\tstruct net_device *dev = alloc_eip_netdev();\n-\tint err;\n-\n-\tif (!dev)\n-\t\treturn ERR_PTR(-ENOMEM);\n-\n-\tsprintf(dev->name, \"eth%d\", unit);\n-\tnetdev_boot_setup_check(dev);\n-\n-\terr = do_ne_probe(dev);\n-\tif (err)\n-\t\tgoto out;\n-\treturn dev;\n-out:\n-\tfree_netdev(dev);\n-\treturn ERR_PTR(err);\n-}\n-#endif\n-\n static int __init ne_probe_isapnp(struct net_device *dev)\n {\n \tint i;\n@@ -329,7 +335,7 @@ static int __init ne_probe1(struct net_d\n \t with an otherwise unused dev->mem_end value of \"0xBAD\" will\n \t cause the driver to skip these parts of the probe. */\n \n-\tbad_card = ((dev->base_addr != 0) && (dev->mem_end == 0xbad));\n+\tbad_card = ((dev->base_addr != 0) && (dev->mem_end == BAD));\n \n \t/* Reset card. Who knows what dain-bramaged state it was left in. */\n \n@@ -806,39 +812,84 @@ retry:\n static int __init ne_drv_probe(struct platform_device *pdev)\n {\n \tstruct net_device *dev;\n+\tint err, this_dev = pdev->id;\n \tstruct resource *res;\n-\tint err, irq;\n-\n-\tres = platform_get_resource(pdev, IORESOURCE_IO, 0);\n-\tirq = platform_get_irq(pdev, 0);\n-\tif (!res || irq < 0)\n-\t\treturn -ENODEV;\n \n \tdev = alloc_eip_netdev();\n \tif (!dev)\n \t\treturn -ENOMEM;\n-\tdev->irq = irq;\n-\tdev->base_addr = res->start;\n+\n+\t/* ne.c doesn't populate resources in platform_device, but\n+\t * rbtx4927_ne_init and rbtx4938_ne_init do register devices\n+\t * with resources.\n+\t */\n+\tres = platform_get_resource(pdev, IORESOURCE_IO, 0);\n+\tif (res) {\n+\t\tdev->base_addr = res->start;\n+\t\tdev->irq = platform_get_irq(pdev, 0);\n+\t} else {\n+\t\tif (this_dev < 0 || this_dev >= MAX_NE_CARDS)\n+\t\t\treturn -EINVAL;\n+\t\tdev->base_addr = io[this_dev];\n+\t\tdev->irq = irq[this_dev];\n+\t\tdev->mem_end = bad[this_dev];\n+\t}\n \terr = do_ne_probe(dev);\n \tif (err) {\n \t\tfree_netdev(dev);\n \t\treturn err;\n \t}\n \tplatform_set_drvdata(pdev, dev);\n+\n+\t/* Update with any values found by probing, don't update if\n+\t * resources were specified.\n+\t */\n+\tif (!res) {\n+\t\tio[this_dev] = dev->base_addr;\n+\t\tirq[this_dev] = dev->irq;\n+\t}\n \treturn 0;\n }\n \n-static int __exit ne_drv_remove(struct platform_device *pdev)\n+static int ne_drv_remove(struct platform_device *pdev)\n {\n \tstruct net_device *dev = platform_get_drvdata(pdev);\n \n-\tunregister_netdev(dev);\n-\tfree_irq(dev->irq, dev);\n-\trelease_region(dev->base_addr, NE_IO_EXTENT);\n-\tfree_netdev(dev);\n+\tif (dev) {\n+\t\tstruct pnp_dev *idev = (struct pnp_dev *)ei_status.priv;\n+\t\tnetif_device_detach(dev);\n+\t\tunregister_netdev(dev);\n+\t\tif (idev)\n+\t\t\tpnp_device_detach(idev);\n+\t\t/* Careful ne_drv_remove can be called twice, once from\n+\t\t * the platform_driver.remove and again when the\n+\t\t * platform_device is being removed.\n+\t\t */\n+\t\tei_status.priv = 0;\n+\t\tfree_irq(dev->irq, dev);\n+\t\trelease_region(dev->base_addr, NE_IO_EXTENT);\n+\t\tfree_netdev(dev);\n+\t\tplatform_set_drvdata(pdev, NULL);\n+\t}\n \treturn 0;\n }\n \n+/* Remove unused devices or all if true. */\n+static void ne_loop_rm_unreg(int all)\n+{\n+\tint this_dev;\n+\tstruct platform_device *pdev;\n+\tfor (this_dev = 0; this_dev < MAX_NE_CARDS; this_dev++) {\n+\t\tpdev = pdev_ne[this_dev];\n+\t\t/* No network device == unused */\n+\t\tif (pdev && (!platform_get_drvdata(pdev) || all)) {\n+\t\t\tne_drv_remove(pdev);\n+\t\t\tplatform_device_unregister(pdev);\n+\t\t\tpdev_ne[this_dev] = NULL;\n+\t\t}\n+\t}\n+}\n+\n #ifdef CONFIG_PM\n static int ne_drv_suspend(struct platform_device *pdev, pm_message_t state)\n {\n@@ -866,7 +917,7 @@ static int ne_drv_resume(struct platform\n #endif\n \n static struct platform_driver ne_driver = {\n-\t.remove\t\t= __exit_p(ne_drv_remove),\n+\t.remove\t\t= ne_drv_remove,\n \t.suspend\t= ne_drv_suspend,\n \t.resume\t\t= ne_drv_resume,\n \t.driver\t\t= {\n@@ -875,91 +926,96 @@ static struct platform_driver ne_driver \n \t},\n };\n \n-static int __init ne_init(void)\n+static void __init ne_add_devices(void)\n {\n-\treturn platform_driver_probe(&ne_driver, ne_drv_probe);\n+\tint this_dev;\n+\tstruct platform_device *pdev;\n+\n+\tfor (this_dev = 0; this_dev < MAX_NE_CARDS; this_dev++) {\n+\t\tif (pdev_ne[this_dev])\n+\t\t\tcontinue;\n+\t\tpdev = platform_device_register_simple(\n+\t\t\tDRV_NAME, this_dev, NULL, 0);\n+\t\tif (IS_ERR(pdev))\n+\t\t\tcontinue;\n+\t\tpdev_ne[this_dev] = pdev;\n+\t}\n }\n \n-static void __exit ne_exit(void)\n+#ifdef MODULE\n+int __init init_module()\n {\n-\tplatform_driver_unregister(&ne_driver);\n+\tint retval;\n+\tne_add_devices();\n+\tretval = platform_driver_probe(&ne_driver, ne_drv_probe);\n+\tif (retval) {\n+\t\tif (io[0] == 0)\n+\t\t\tprintk(KERN_NOTICE \"ne.c: You must supply \\\"io=0xNNN\\\"\"\n+\t\t\t\t\" value(s) for ISA cards.\\n\");\n+\t\tne_loop_rm_unreg(1);\n+\t\treturn retval;\n+\t}\n+\n+\t/* Unregister unused platform_devices. */\n+\tne_loop_rm_unreg(0);\n+\treturn retval;\n }\n+#else /* MODULE */\n+static int __init ne_init(void)\n+{\n+\tint retval = platform_driver_probe(&ne_driver, ne_drv_probe);\n \n-#ifdef MODULE\n-#define MAX_NE_CARDS\t4\t/* Max number of NE cards per module */\n-static struct net_device *dev_ne[MAX_NE_CARDS];\n-static int io[MAX_NE_CARDS];\n-static int irq[MAX_NE_CARDS];\n-static int bad[MAX_NE_CARDS];\t/* 0xbad = bad sig or no reset ack */\n+\t/* Unregister unused platform_devices. */\n+\tne_loop_rm_unreg(0);\n+\treturn retval;\n+}\n+module_init(ne_init);\n \n-module_param_array(io, int, NULL, 0);\n-module_param_array(irq, int, NULL, 0);\n-module_param_array(bad, int, NULL, 0);\n-MODULE_PARM_DESC(io, \"I/O base address(es),required\");\n-MODULE_PARM_DESC(irq, \"IRQ number(s)\");\n-MODULE_PARM_DESC(bad, \"Accept card(s) with bad signatures\");\n-MODULE_DESCRIPTION(\"NE1000/NE2000 ISA/PnP Ethernet driver\");\n-MODULE_LICENSE(\"GPL\");\n+struct net_device * __init ne_probe(int unit)\n+{\n+\tint this_dev;\n+\tstruct net_device *dev;\n \n-/* This is set up so that no ISA autoprobe takes place. We can't guarantee\n-that the ne2k probe is the last 8390 based probe to take place (as it\n-is at boot) and so the probe will get confused by any other 8390 cards.\n-ISA device autoprobes on a running machine are not recommended anyway. */\n+\t/* Find an empty slot, that is no net_device and zero io port. */\n+\tthis_dev = 0;\n+\twhile ((pdev_ne[this_dev] && platform_get_drvdata(pdev_ne[this_dev])) ||\n+\t\tio[this_dev]) {\n+\t\tif (++this_dev == MAX_NE_CARDS)\n+\t\t\treturn ERR_PTR(-ENOMEM);\n+\t}\n \n-int __init init_module(void)\n-{\n-\tint this_dev, found = 0;\n-\tint plat_found = !ne_init();\n+\t/* Get irq, io from kernel command line */\n+\tdev = alloc_eip_netdev();\n+\tif (!dev)\n+\t\treturn ERR_PTR(-ENOMEM);\n \n+\tsprintf(dev->name, \"eth%d\", unit);\n+\tnetdev_boot_setup_check(dev);\n+\n+\tio[this_dev] = dev->base_addr;\n+\tirq[this_dev] = dev->irq;\n+\tbad[this_dev] = dev->mem_end;\n+\n+\tfree_netdev(dev);\n+\n+\tne_add_devices();\n+\n+\t/* return the first device found */\n \tfor (this_dev = 0; this_dev < MAX_NE_CARDS; this_dev++) {\n-\t\tstruct net_device *dev = alloc_eip_netdev();\n-\t\tif (!dev)\n-\t\t\tbreak;\n-\t\tdev->irq = irq[this_dev];\n-\t\tdev->mem_end = bad[this_dev];\n-\t\tdev->base_addr = io[this_dev];\n-\t\tif (do_ne_probe(dev) == 0) {\n-\t\t\tdev_ne[found++] = dev;\n-\t\t\tcontinue;\n+\t\tif (pdev_ne[this_dev]) {\n+\t\t\tdev = platform_get_drvdata(pdev_ne[this_dev]);\n+\t\t\tif (dev)\n+\t\t\t\treturn dev;\n \t\t}\n-\t\tfree_netdev(dev);\n-\t\tif (found || plat_found)\n-\t\t\tbreak;\n-\t\tif (io[this_dev] != 0)\n-\t\t\tprintk(KERN_WARNING \"ne.c: No NE*000 card found at i/o = %#x\\n\", io[this_dev]);\n-\t\telse\n-\t\t\tprintk(KERN_NOTICE \"ne.c: You must supply \\\"io=0xNNN\\\" value(s) for ISA cards.\\n\");\n-\t\treturn -ENXIO;\n \t}\n-\tif (found || plat_found)\n-\t\treturn 0;\n-\treturn -ENODEV;\n-}\n \n-static void cleanup_card(struct net_device *dev)\n-{\n-\tstruct pnp_dev *idev = (struct pnp_dev *)ei_status.priv;\n-\tif (idev)\n-\t\tpnp_device_detach(idev);\n-\tfree_irq(dev->irq, dev);\n-\trelease_region(dev->base_addr, NE_IO_EXTENT);\n+\treturn ERR_PTR(-ENODEV);\n }\n+#endif /* MODULE */\n \n-void __exit cleanup_module(void)\n+static void __exit ne_exit(void)\n {\n-\tint this_dev;\n-\n-\tne_exit();\n-\tfor (this_dev = 0; this_dev < MAX_NE_CARDS; this_dev++) {\n-\t\tstruct net_device *dev = dev_ne[this_dev];\n-\t\tif (dev) {\n-\t\t\tunregister_netdev(dev);\n-\t\t\tcleanup_card(dev);\n-\t\t\tfree_netdev(dev);\n-\t\t}\n-\t}\n+\tplatform_driver_unregister(&ne_driver);\n+\tne_loop_rm_unreg(1);\n }\n-#else /* MODULE */\n-module_init(ne_init);\n module_exit(ne_exit);\n-#endif /* MODULE */\n", "prefixes": [ "for", "2.6.27?", "06/10" ] }