[{"id":1711410,"web_url":"http://patchwork.ozlabs.org/comment/1711410/","msgid":"<87k23k91rn.fsf@concordia.ellerman.id.au>","date":"2017-07-07T06:53:00","subject":"Re: [PATCH v12 02/10] powerpc/powernv: Autoload IMC device driver\n\tmodule","submitter":{"id":46580,"url":"http://patchwork.ozlabs.org/api/people/46580/","name":"Michael Ellerman","email":"mpe@ellerman.id.au"},"content":"Hi Maddy/Anju,\n\nComments below :)\n\nAnju T Sudhakar writes:\n> Code to create platform device for the IMC counters.\n> Paltform devices are created based on the IMC compatibility\n> string.\n>\n> New Config flag \"CONFIG_HV_PERF_IMC_CTRS\" add to contain the\n> IMC counter changes.\n\nI don't think we need a separate config, it can just use\nCONFIG_PPC_POWERNV.\n\nI don't think we'll ever want to turn it off for powernv, unless we're\ntrying to build a small kernel, in which case we'll turn of PERF\nentirely.\n\n> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c\n> new file mode 100644\n> index 000000000000..5b1045c81af4\n> --- /dev/null\n> +++ b/arch/powerpc/platforms/powernv/opal-imc.c\n> @@ -0,0 +1,73 @@\n> +/*\n> + * OPAL IMC interface detection driver\n> + * Supported on POWERNV platform\n> + *\n> + * Copyright\t(C) 2017 Madhavan Srinivasan, IBM Corporation.\n> + *\t\t(C) 2017 Anju T Sudhakar, IBM Corporation.\n> + *\t\t(C) 2017 Hemant K Shaw, IBM Corporation.\n> + *\n> + * This program is free software; you can redistribute it and/or\n> + * modify it under the terms of the GNU General Public License\n> + * as published by the Free Software Foundation; either version\n> + * 2 of the License, or later version.\n> + *\n> + * This program is distributed in the hope that it will be useful,\n> + * but WITHOUT ANY WARRANTY; without even the implied warranty of\n> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n> + * GNU General Public License for more details.\n\nWe usually don't include that part in every file.\n\n> + */\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +static int opal_imc_counters_probe(struct platform_device *pdev)\n> +{\n> +\tstruct device_node *imc_dev = NULL;\n> +\n> +\tif (!pdev || !pdev->dev.of_node)\n> +\t\treturn -ENODEV;\n\nWe don't need that level of paranoia :)\n\n> +\t/*\n> +\t * Check whether this is kdump kernel. If yes, just return.\n> +\t */\n> +\tif (is_kdump_kernel())\n> +\t\treturn -ENODEV;\n\nHmm, that's a bit unusual. Is there any particular reason to do that for\nthis driver?\n\n> +\timc_dev = pdev->dev.of_node;\n> +\tif (!imc_dev)\n> +\t\treturn -ENODEV;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct of_device_id opal_imc_match[] = {\n> +\t{ .compatible = IMC_DTB_COMPAT },\n> +\t{},\n> +};\n> +\n> +static struct platform_driver opal_imc_driver = {\n> +\t.driver = {\n> +\t\t.name = \"opal-imc-counters\",\n> +\t\t.of_match_table = opal_imc_match,\n> +\t},\n> +\t.probe = opal_imc_counters_probe,\n> +};\n> +\n\nThis can't be built as a module, so it should not be using MODULE macros.\n\n> +MODULE_DEVICE_TABLE(of, opal_imc_match);\n\nDrop that.\n\n> +module_platform_driver(opal_imc_driver);\n\nUse builtin_platform_driver().\n\n> +MODULE_DESCRIPTION(\"PowerNV OPAL IMC driver\");\n> +MODULE_LICENSE(\"GPL\");\n\nDrop those.\n\n> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c\n> index 59684b4af4d1..fbdca259ea76 100644\n> --- a/arch/powerpc/platforms/powernv/opal.c\n> +++ b/arch/powerpc/platforms/powernv/opal.c\n> @@ -705,6 +707,17 @@ static void opal_pdev_init(const char *compatible)\n> \t\tof_platform_device_create(np, NULL, NULL);\n> }\n> \n> +#ifdef CONFIG_HV_PERF_IMC_CTRS\n> +static void __init opal_imc_init_dev(void)\n> +{\n> +\tstruct device_node *np;\n> +\n> +\tnp = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT);\n> +\tif (np)\n> +\t\tof_platform_device_create(np, NULL, NULL);\n> +}\n> +#endif\n\nThat doesn't need the #ifdef.\n\n> static int kopald(void *unused)\n> {\n> \tunsigned long timeout = msecs_to_jiffies(opal_heartbeat) + 1;\n> @@ -778,6 +791,11 @@ static int __init opal_init(void)\n> \t/* Setup a heatbeat thread if requested by OPAL */\n> \topal_init_heartbeat();\n> \n> +#ifdef CONFIG_HV_PERF_IMC_CTRS\n> +\t/* Detect IMC pmu counters support and create PMUs */\n> +\topal_imc_init_dev();\n> +#endif\n> +\n\nNeither here.\n\n> \t/* Create leds platform devices */\n> \tleds = of_find_node_by_path(\"/ibm,opal/leds\");\n> \tif (leds) {\n> -- \n> 2.11.0\n\n\ncheers","headers":{"Return-Path":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3x3lh63wfGz9s4s\n\tfor ;\n\tFri, 7 Jul 2017 16:53:58 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3x3lh639XpzDr93\n\tfor ;\n\tFri, 7 Jul 2017 16:53:58 +1000 (AEST)","from ozlabs.org (ozlabs.org [103.22.144.67])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3x3lg354vvzDqj8\n\tfor ;\n\tFri, 7 Jul 2017 16:53:03 +1000 (AEST)","from authenticated.ozlabs.org (localhost [127.0.0.1])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPSA id 3x3lg33l9Tz9s4s;\n\tFri, 7 Jul 2017 16:53:03 +1000 (AEST)"],"From":"Michael Ellerman ","To":"Anju T Sudhakar ","Subject":"Re: [PATCH v12 02/10] powerpc/powernv: Autoload IMC device driver\n\tmodule","In-Reply-To":"<1499074673-30576-3-git-send-email-anju@linux.vnet.ibm.com>","References":"<1499074673-30576-1-git-send-email-anju@linux.vnet.ibm.com>\n\t<1499074673-30576-3-git-send-email-anju@linux.vnet.ibm.com>","User-Agent":"Notmuch/0.21 (https://notmuchmail.org)","Date":"Fri, 07 Jul 2017 16:53:00 +1000","Message-ID":"<87k23k91rn.fsf@concordia.ellerman.id.au>","MIME-Version":"1.0","Content-Type":"text/plain","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"stewart@linux.vnet.ibm.com, ego@linux.vnet.ibm.com, mikey@neuling.org,\n\tmaddy@linux.vnet.ibm.com, hemant@linux.vnet.ibm.com,\n\tlinux-kernel@vger.kernel.org, eranian@google.com,\n\tanju@linux.vnet.ibm.com, anton@samba.org, sukadev@linux.vnet.ibm.com,\n\tlinuxppc-dev@lists.ozlabs.org, dja@axtens.net","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}}]