Messages in this thread Patch in this message | | | From | Gowthami Thiagarajan <> | Subject | RE: [RESEND PATCH v3] perf/marvell: Marvell PEM performance monitor support | Date | Wed, 3 Apr 2024 05:22:22 +0000 |
| |
Hi Andrew,
Please find the responses inline.
Thanks, Gowthami
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Wednesday, March 27, 2024 6:41 PM > To: Gowthami Thiagarajan <gthiagarajan@marvell.com> > Cc: will@kernel.org; mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com>; George Cherian > <gcherian@marvell.com>; Linu Cherian <lcherian@marvell.com> > Subject: [EXTERNAL] Re: [RESEND PATCH v3] perf/marvell: Marvell PEM performance monitor support > > On Wed, Mar 27, 2024 at 12:51:17PM +0530, Gowthami Thiagarajan wrote: > > PCI Express Interface PMU includes various performance counters > > to monitor the data that is transmitted over the PCIe link. The > > counters track various inbound and outbound transactions which > > includes separate counters for posted/non-posted/completion TLPs. > > Also, inbound and outbound memory read requests along with their > > latencies can also be monitored. Address Translation Services(ATS)events > > such as ATS Translation, ATS Page Request, ATS Invalidation along with > > their corresponding latencies are also supported. > > > > The performance counters are 64 bits wide. > > > > For instance, > > perf stat -e ib_tlp_pr <workload> > > tracks the inbound posted TLPs for the workload. > > > > Signed-off-by: Gowthami Thiagarajan <gthiagarajan@marvell.com> > > --- > > v2->v3 changes: > > - Dropped device tree support as the acpi table based probing is used. > > So people using DT cannot use this driver? Can they use the PCIe > interface? > > There does not appear to be any ACPI binding, it is not reading any > properties from ACPI tables etc. So the DT binding should be > trivial...
The ACPI bindings are present in the driver. Have the ACPI ID MRVL000E tied here and the resources are fetched from the ACPI table.
static const struct acpi_device_id pem_pmu_acpi_match[] = { {"MRVL000E", 0}, {}, }; MODULE_DEVICE_TABLE(acpi, pem_pmu_acpi_match);
base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> > > index 000000000000..d4175483b982 > > --- /dev/null > > +++ b/drivers/perf/marvell_pem_pmu.c > > @@ -0,0 +1,428 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Marvell PEM(PCIe RC) Performance Monitor Driver > > + * > > + * Copyright (C) 2024 Marvell. > > + */ > > + > > +#include <linux/acpi.h> > > +#include <linux/init.h> > > +#include <linux/io.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_address.h> > > +#include <linux/of_device.h> > Why do you need these header files? I don't see any calls to of_ > functions. > These header files are not needed. I will remove them in the next version.
> > +static int pem_perf_probe(struct platform_device *pdev) > > +{ > > + struct pem_pmu *pem_pmu; > > + struct resource *res; > > + void __iomem *base; > > + char *name; > > + int ret; > > + > > + pem_pmu = devm_kzalloc(&pdev->dev, sizeof(*pem_pmu), GFP_KERNEL); > > + if (!pem_pmu) > > + return -ENOMEM; > > + > > + pem_pmu->dev = &pdev->dev; > > + platform_set_drvdata(pdev, pem_pmu); > > + > > + base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); > > + if (IS_ERR(base)) > > + return PTR_ERR(base); > > + > > + pem_pmu->base = base; > > + > > + pem_pmu->pmu = (struct pmu) { > > + .module = THIS_MODULE, > > + .capabilities = PERF_PMU_CAP_NO_EXCLUDE, > > + .task_ctx_nr = perf_invalid_context, > > + .attr_groups = pem_perf_attr_groups, > > + .event_init = pem_perf_event_init, > > + .add = pem_perf_event_add, > > + .del = pem_perf_event_del, > > + .start = pem_perf_event_start, > > + .stop = pem_perf_event_stop, > > + .read = pem_perf_event_update, > > + }; > > + > > + /* Choose this cpu to collect perf data */ > > + pem_pmu->cpu = raw_smp_processor_id(); > > + > > + name = devm_kasprintf(pem_pmu->dev, GFP_KERNEL, "mrvl_pcie_rc_pmu_%llx", > > + res->start); > > + if (!name) > > + return -ENOMEM; > > + > > + cpuhp_state_add_instance_nocalls > > + (CPUHP_AP_PERF_ARM_MARVELL_PEM_ONLINE, > > + &pem_pmu->node); > > + > > + ret = perf_pmu_register(&pem_pmu->pmu, name, -1); > > + if (ret) > > + goto error; > > + > > + pr_info("Marvell PEM(PCIe RC) PMU Driver for pem@%llx\n", res->start); > > Please don't spam the kernel log like this.
Will get this message removed. > > Andrew
| |