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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 6FC04CDB474 for ; Fri, 20 Oct 2023 16:50:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:CC:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=4MoQrSo60XvBNgvQetCfrGMk8hVVgnwNnf38yKyQAaw=; b=KzthCtPF8nBUIr cA81R+ikNxnaOpDx9vVa6JoNybZ/fjrlkJuuDu4RzWHsd8krR4dqDcQAx4ZH9x/tRhPaw6xRvG+wh Zk/t4nqkefQ7TySRYlIhonRDv0w/eASCrcesWU/JZLB+jX8OaorrEGsZMQ8VzBbsXN7tyo+4F6mY4 GaPgRbDm2IJYPnC8267BZIBRf67XkH6ww7HPkOlffkEU2rQ3hy70bp7VeJEqPEspJgN0AcC1g37fv 4rG8dEZTMvKuwcjQtuQOjnXt7jS0SF7mWIKWFmOUtGLQ5vabreAka1omWAC+tqrYRdMWBePIIOwUW JXoTFr1G2Kp44ipGwU5g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qtshB-002ilc-1s; Fri, 20 Oct 2023 16:49:57 +0000 Received: from frasgout.his.huawei.com ([185.176.79.56]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qtsh7-002ikI-1k for linux-arm-kernel@lists.infradead.org; Fri, 20 Oct 2023 16:49:55 +0000 Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.207]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4SBr7n3bsHz6J9dR; Sat, 21 Oct 2023 00:46:13 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Fri, 20 Oct 2023 17:49:41 +0100 Date: Fri, 20 Oct 2023 17:49:40 +0100 From: Jonathan Cameron To: Shuai Xue CC: , , , , , , , , , , , , , Subject: Re: [PATCH v9 3/4] drivers/perf: add DesignWare PCIe PMU driver Message-ID: <20231020174940.0000429e@Huawei.com> In-Reply-To: <20231020134230.53342-4-xueshuai@linux.alibaba.com> References: <20231020134230.53342-1-xueshuai@linux.alibaba.com> <20231020134230.53342-4-xueshuai@linux.alibaba.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml500003.china.huawei.com (7.191.162.67) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231020_094954_100792_869F3653 X-CRM114-Status: GOOD ( 23.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 20 Oct 2023 21:42:29 +0800 Shuai Xue wrote: > This commit adds the PCIe Performance Monitoring Unit (PMU) driver support > for T-Head Yitian SoC chip. Yitian is based on the Synopsys PCI Express > Core controller IP which provides statistics feature. The PMU is a PCIe > configuration space register block provided by each PCIe Root Port in a > Vendor-Specific Extended Capability named RAS D.E.S (Debug, Error > injection, and Statistics). > > To facilitate collection of statistics the controller provides the > following two features for each Root Port: > > - one 64-bit counter for Time Based Analysis (RX/TX data throughput and > time spent in each low-power LTSSM state) and > - one 32-bit counter for Event Counting (error and non-error events for > a specified lane) > > Note: There is no interrupt for counter overflow. > > This driver adds PMU devices for each PCIe Root Port. And the PMU device is > named based the BDF of Root Port. For example, > > 30:03.0 PCI bridge: Device 1ded:8000 (rev 01) > > the PMU device name for this Root Port is dwc_rootport_3018. > > Example usage of counting PCIe RX TLP data payload (Units of bytes):: > > $# perf stat -a -e dwc_rootport_3018/Rx_PCIe_TLP_Data_Payload/ > > average RX bandwidth can be calculated like this: > > PCIe TX Bandwidth = Rx_PCIe_TLP_Data_Payload / Measure_Time_Window > > Signed-off-by: Shuai Xue LGTM other than some really trivial stuff inline if you are doing a v10 Reviewed-by: Jonathan Cameron > +static int dwc_pcie_pmu_probe(struct platform_device *plat_dev) > +{ > + struct pci_dev *pdev = NULL; > + struct dwc_pcie_pmu *pcie_pmu; > + bool notify = false; > + char *name; > + u32 bdf; > + int ret; > + > + /* Match the rootport with VSEC_RAS_DES_ID, and register a PMU for it */ > + for_each_pci_dev(pdev) { > + u16 vsec; > + u32 val; > + > + if (!(pci_is_pcie(pdev) && > + pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)) > + continue; > + > + vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_ALIBABA, > + DWC_PCIE_VSEC_RAS_DES_ID); > + if (!vsec) > + continue; > + > + pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val); > + if (PCI_VNDR_HEADER_REV(val) != 0x04) > + continue; > + pci_dbg(pdev, > + "Detected PCIe Vendor-Specific Extended Capability RAS DES\n"); > + > + bdf = PCI_DEVID(pdev->bus->number, pdev->devfn); > + name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", > + bdf); > + if (!name) { > + ret = -ENOMEM; > + goto out; > + } > + > + /* All checks passed, go go go */ > + pcie_pmu = devm_kzalloc(&plat_dev->dev, sizeof(*pcie_pmu), GFP_KERNEL); > + if (!pcie_pmu) { > + ret = -ENOMEM; > + goto out; > + } > + > + pcie_pmu->pdev = pdev; > + pcie_pmu->ras_des_offset = vsec; > + pcie_pmu->nr_lanes = pcie_get_width_cap(pdev); > + pcie_pmu->on_cpu = -1; > + pcie_pmu->pmu = (struct pmu){ > + .module = THIS_MODULE, > + .attr_groups = dwc_pcie_attr_groups, > + .capabilities = PERF_PMU_CAP_NO_EXCLUDE, > + .task_ctx_nr = perf_invalid_context, > + .event_init = dwc_pcie_pmu_event_init, > + .add = dwc_pcie_pmu_event_add, > + .del = dwc_pcie_pmu_event_del, > + .start = dwc_pcie_pmu_event_start, > + .stop = dwc_pcie_pmu_event_stop, > + .read = dwc_pcie_pmu_event_update, > + }; > + > + /* Add this instance to the list used by the offline callback */ > + ret = cpuhp_state_add_instance(dwc_pcie_pmu_hp_state, > + &pcie_pmu->cpuhp_node); > + if (ret) { > + pci_err(pdev, > + "Error %d registering hotplug @%x\n", ret, bdf); > + goto out; > + } > + > + /* Unwind when platform driver removes */ > + ret = devm_add_action_or_reset( > + &plat_dev->dev, dwc_pcie_pmu_remove_cpuhp_instance, > + &pcie_pmu->cpuhp_node); > + if (ret) > + goto out; > + > + ret = perf_pmu_register(&pcie_pmu->pmu, name, -1); > + if (ret) { > + pci_err(pdev, > + "Error %d registering PMU @%x\n", ret, bdf); > + goto out; > + } > + > + /* Cache PMU to handle pci device hotplug */ > + list_add(&pcie_pmu->pmu_node, &dwc_pcie_pmu_head); > + pcie_pmu->registered = true; > + notify = true; > + > + ret = devm_add_action_or_reset( > + &plat_dev->dev, dwc_pcie_pmu_unregister_pmu, pcie_pmu); line wrapping is a bit ugly - I would move the &plat_dev->dev to previous line. and I think you can get away with aligning the rest just after the ( > + if (ret) > + goto out; > + } > + > + if (notify && !bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb)) > + return devm_add_action_or_reset( > + &plat_dev->dev, dwc_pcie_pmu_unregister_nb, NULL); > + > + return 0; > + > +out: > + pci_dev_put(pdev); > + > + return ret; > +} > +static int __init dwc_pcie_pmu_init(void) > +{ > + int ret; > + > + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, > + "perf/dwc_pcie_pmu:online", > + dwc_pcie_pmu_online_cpu, > + dwc_pcie_pmu_offline_cpu); > + if (ret < 0) > + return ret; > + > + dwc_pcie_pmu_hp_state = ret; > + > + ret = platform_driver_register(&dwc_pcie_pmu_driver); > + if (ret) > + goto platform_driver_register_err; > + > + dwc_pcie_pmu_dev = platform_device_register_simple( > + "dwc_pcie_pmu", PLATFORM_DEVID_NONE, NULL, 0); > + if (IS_ERR(dwc_pcie_pmu_dev)) { > + ret = PTR_ERR(dwc_pcie_pmu_dev); > + goto platform_device_register_error; > + } > + > + return 0; > + > +platform_device_register_error: Trivial but I'd standardize on err or error, not mix them. > + platform_driver_unregister(&dwc_pcie_pmu_driver); > +platform_driver_register_err: > + cpuhp_remove_multi_state(dwc_pcie_pmu_hp_state); > + > + return ret; > +} _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel