From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Murphy Subject: Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support Date: Wed, 24 Apr 2019 12:08:19 +0100 Message-ID: References: <1555447156-28306-1-git-send-email-Frank.Li@nxp.com> <1555447156-28306-2-git-send-email-Frank.Li@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1555447156-28306-2-git-send-email-Frank.Li@nxp.com> Content-Language: en-GB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Frank Li , "mark.rutland@arm.com" , "will.deacon@arm.com" , "shawnguo@kernel.org" , "s.hauer@pengutronix.de" , "kernel@pengutronix.de" , "festevam@gmail.com" , dl-linux-imx , "robh+dt@kernel.org" , Aisheng Dong , "devicetree@vger.kernel.org" , "lznuaa@gmail.com" , "linux-arm-kernel@lists.infradead.org" List-Id: devicetree@vger.kernel.org On 16/04/2019 21:39, Frank Li wrote: [...] > +static int ddr_perf_probe(struct platform_device *pdev) > +{ > + struct ddr_pmu *pmu; > + struct device_node *np; > + void __iomem *base; > + struct resource *iomem; > + char *name; > + char *hpname; > + int num; > + int ret; > + u32 irq; > + const struct of_device_id *of_id = > + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev); > + > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(&pdev->dev, iomem); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + np = pdev->dev.of_node; > + > + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL); > + if (!pmu) > + return -ENOMEM; > + > + num = ddr_perf_init(pmu, base, &pdev->dev); > + > + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num); > + if (!name) > + return -ENOMEM; > + > + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL, > + "perf/imx/ddr%d:online", num); > + if (!hpname) > + return -ENOMEM; > + > + pmu->flags = (uintptr_t) of_id->data; > + > + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu); > + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL, > + ddr_perf_offline_cpu); > + > + if (ret < 0) { > + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n"); > + goto ddr_perf_err; > + } Since you don't seem to ever remove this state again, this looks like it would make subsequent loads of the module always fail after it's been loaded and unloaded once - have you tried that? Robin. > + > + pmu->cpuhp_state = ret; > + > + /* Register the pmu instance for cpu hotplug */ > + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node); > + > + ret = perf_pmu_register(&(pmu->pmu), name, -1); > + if (ret) > + goto ddr_perf_err; > + > + /* Request irq */ > + irq = of_irq_get(np, 0); > + if (irq < 0) { > + dev_err(&pdev->dev, "Failed to get irq: %d", irq); > + ret = irq; > + goto ddr_perf_irq_err; > + } > + > + ret = devm_request_irq(&pdev->dev, irq, > + ddr_perf_irq_handler, > + IRQF_TRIGGER_RISING | IRQF_ONESHOT, > + DDR_PERF_DEV_NAME, > + pmu); > + if (ret < 0) { > + dev_err(&pdev->dev, "Request irq failed: %d", ret); > + goto ddr_perf_irq_err; > + } > + > + return 0; > + > +ddr_perf_irq_err: > + perf_pmu_unregister(&(pmu->pmu)); > + > +ddr_perf_err: > + if (pmu->cpuhp_state) > + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node); > + > + dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret); > + return ret; > +} > + > +static int ddr_perf_remove(struct platform_device *pdev) > +{ > + struct ddr_pmu *pmu = platform_get_drvdata(pdev); > + > + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node); > + perf_pmu_unregister(&pmu->pmu); > + > + return 0; > +} > + > +static struct platform_driver imx_ddr_pmu_driver = { > + .driver = { > + .name = "imx-ddr-pmu", > + .of_match_table = imx_ddr_pmu_dt_ids, > + }, > + .probe = ddr_perf_probe, > + .remove = ddr_perf_remove, > +}; > + > +static int __init imx_ddr_pmu_init(void) > +{ > + return platform_driver_register(&imx_ddr_pmu_driver); > +} > + > +module_init(imx_ddr_pmu_init); > + > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C018DC10F11 for ; Wed, 24 Apr 2019 11:08:32 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 906112175B for ; Wed, 24 Apr 2019 11:08:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="l2jdSjz+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 906112175B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Cc:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mDfz0xw+XrhoMln/Yp9Ur2aBWAlI3Zdw4innbFkUTI8=; b=l2jdSjz+h1whAxvowE5X7SDxN JJpINEV3Af2EIESYuLFOYIIysGrv05aMbyWInzqcIT4Nhbkm/T1APfraU8tmxPtolqIK7txOH+Uoz sltUXHQquhoyRfIC2P+C4Yr0oYhF/NlPEsvPUA80B35l7UvH8YW1aS42obDoWKHnpHvQgf0Y1m18y Bubgu/ZCWqfmxet0YVGi+5XDQzlCUxBbcEpkaIHhA3b7lf4o+JCfzq2oODG6J1TTJRxOR4nR8XhxN YMIDT/2Fbl7NgEEkfchckH0PARtUBHuhmyt7fK4oYmLJP1fKjuP5aySG6TZWuk5v5y6TTJSCls1wj tSoVTNJ1w==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJFlL-0001lN-Ju; Wed, 24 Apr 2019 11:08:27 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJFlI-0001kx-QD for linux-arm-kernel@lists.infradead.org; Wed, 24 Apr 2019 11:08:26 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D531FA78; Wed, 24 Apr 2019 04:08:22 -0700 (PDT) Received: from [10.1.196.75] (e110467-lin.cambridge.arm.com [10.1.196.75]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9EE1C3F5AF; Wed, 24 Apr 2019 04:08:20 -0700 (PDT) Subject: Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support To: Frank Li , "mark.rutland@arm.com" , "will.deacon@arm.com" , "shawnguo@kernel.org" , "s.hauer@pengutronix.de" , "kernel@pengutronix.de" , "festevam@gmail.com" , dl-linux-imx , "robh+dt@kernel.org" , Aisheng Dong , "devicetree@vger.kernel.org" , "lznuaa@gmail.com" , "linux-arm-kernel@lists.infradead.org" References: <1555447156-28306-1-git-send-email-Frank.Li@nxp.com> <1555447156-28306-2-git-send-email-Frank.Li@nxp.com> From: Robin Murphy Message-ID: Date: Wed, 24 Apr 2019 12:08:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <1555447156-28306-2-git-send-email-Frank.Li@nxp.com> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190424_040824_858887_3C70DC37 X-CRM114-Status: GOOD ( 16.36 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 16/04/2019 21:39, Frank Li wrote: [...] > +static int ddr_perf_probe(struct platform_device *pdev) > +{ > + struct ddr_pmu *pmu; > + struct device_node *np; > + void __iomem *base; > + struct resource *iomem; > + char *name; > + char *hpname; > + int num; > + int ret; > + u32 irq; > + const struct of_device_id *of_id = > + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev); > + > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(&pdev->dev, iomem); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + np = pdev->dev.of_node; > + > + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL); > + if (!pmu) > + return -ENOMEM; > + > + num = ddr_perf_init(pmu, base, &pdev->dev); > + > + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num); > + if (!name) > + return -ENOMEM; > + > + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL, > + "perf/imx/ddr%d:online", num); > + if (!hpname) > + return -ENOMEM; > + > + pmu->flags = (uintptr_t) of_id->data; > + > + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu); > + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL, > + ddr_perf_offline_cpu); > + > + if (ret < 0) { > + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n"); > + goto ddr_perf_err; > + } Since you don't seem to ever remove this state again, this looks like it would make subsequent loads of the module always fail after it's been loaded and unloaded once - have you tried that? Robin. > + > + pmu->cpuhp_state = ret; > + > + /* Register the pmu instance for cpu hotplug */ > + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node); > + > + ret = perf_pmu_register(&(pmu->pmu), name, -1); > + if (ret) > + goto ddr_perf_err; > + > + /* Request irq */ > + irq = of_irq_get(np, 0); > + if (irq < 0) { > + dev_err(&pdev->dev, "Failed to get irq: %d", irq); > + ret = irq; > + goto ddr_perf_irq_err; > + } > + > + ret = devm_request_irq(&pdev->dev, irq, > + ddr_perf_irq_handler, > + IRQF_TRIGGER_RISING | IRQF_ONESHOT, > + DDR_PERF_DEV_NAME, > + pmu); > + if (ret < 0) { > + dev_err(&pdev->dev, "Request irq failed: %d", ret); > + goto ddr_perf_irq_err; > + } > + > + return 0; > + > +ddr_perf_irq_err: > + perf_pmu_unregister(&(pmu->pmu)); > + > +ddr_perf_err: > + if (pmu->cpuhp_state) > + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node); > + > + dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret); > + return ret; > +} > + > +static int ddr_perf_remove(struct platform_device *pdev) > +{ > + struct ddr_pmu *pmu = platform_get_drvdata(pdev); > + > + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node); > + perf_pmu_unregister(&pmu->pmu); > + > + return 0; > +} > + > +static struct platform_driver imx_ddr_pmu_driver = { > + .driver = { > + .name = "imx-ddr-pmu", > + .of_match_table = imx_ddr_pmu_dt_ids, > + }, > + .probe = ddr_perf_probe, > + .remove = ddr_perf_remove, > +}; > + > +static int __init imx_ddr_pmu_init(void) > +{ > + return platform_driver_register(&imx_ddr_pmu_driver); > +} > + > +module_init(imx_ddr_pmu_init); > + > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel