From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Yicong Yang <yangyicong@hisilicon.com>
Cc: <gregkh@linuxfoundation.org>, <helgaas@kernel.org>,
<alexander.shishkin@linux.intel.com>, <lorenzo.pieralisi@arm.com>,
<will@kernel.org>, <mark.rutland@arm.com>,
<mathieu.poirier@linaro.org>, <suzuki.poulose@arm.com>,
<mike.leach@linaro.org>, <leo.yan@linaro.org>,
<daniel.thompson@linaro.org>, <joro@8bytes.org>,
<john.garry@huawei.com>, <shameerali.kolothum.thodi@huawei.com>,
<robin.murphy@arm.com>, <peterz@infradead.org>,
<mingo@redhat.com>, <acme@kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<coresight@lists.linaro.org>, <linux-pci@vger.kernel.org>,
<linux-perf-users@vger.kernel.org>,
<iommu@lists.linux-foundation.org>, <prime.zeng@huawei.com>,
<liuqi115@huawei.com>, <zhangshaokun@hisilicon.com>,
<linuxarm@huawei.com>, <song.bao.hua@hisilicon.com>
Subject: Re: [PATCH v5 3/8] hisi_ptt: Register PMU device for PTT trace
Date: Tue, 8 Mar 2022 10:21:57 +0000 [thread overview]
Message-ID: <20220308102157.00003725@Huawei.com> (raw)
In-Reply-To: <20220308084930.5142-4-yangyicong@hisilicon.com>
On Tue, 8 Mar 2022 16:49:25 +0800
Yicong Yang <yangyicong@hisilicon.com> wrote:
> Register PMU device of PTT trace, then users can use trace through perf
> command. The driver makes use of perf AUX trace and support following
> events to configure the trace:
>
> - filter: select Root port or Endpoint to trace
> - type: select the type of traced TLP headers
> - direction: select the direction of traced TLP headers
> - format: select the data format of the traced TLP headers
>
> This patch adds the PMU driver part of PTT trace. The perf command support
> of PTT trace is added in the following patch.
>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
It seems to me that you ended up doing both suggestions for
how to clean up the remove order when it was meant to be
a question of picking one or the other.
Otherwise this looks good to me - so with that tidied up
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> +
> +static int hisi_ptt_register_pmu(struct hisi_ptt *hisi_ptt)
> +{
> + u16 core_id, sicl_id;
> + char *pmu_name;
> + u32 reg;
> +
> + hisi_ptt->hisi_ptt_pmu = (struct pmu) {
> + .module = THIS_MODULE,
> + .capabilities = PERF_PMU_CAP_EXCLUSIVE | PERF_PMU_CAP_ITRACE,
> + .task_ctx_nr = perf_sw_context,
> + .attr_groups = hisi_ptt_pmu_groups,
> + .event_init = hisi_ptt_pmu_event_init,
> + .setup_aux = hisi_ptt_pmu_setup_aux,
> + .free_aux = hisi_ptt_pmu_free_aux,
> + .start = hisi_ptt_pmu_start,
> + .stop = hisi_ptt_pmu_stop,
> + .add = hisi_ptt_pmu_add,
> + .del = hisi_ptt_pmu_del,
> + };
> +
> + reg = readl(hisi_ptt->iobase + HISI_PTT_LOCATION);
> + core_id = FIELD_GET(HISI_PTT_CORE_ID, reg);
> + sicl_id = FIELD_GET(HISI_PTT_SICL_ID, reg);
> +
> + pmu_name = devm_kasprintf(&hisi_ptt->pdev->dev, GFP_KERNEL, "hisi_ptt%u_%u",
> + sicl_id, core_id);
> + if (!pmu_name)
> + return -ENOMEM;
> +
> + return perf_pmu_register(&hisi_ptt->hisi_ptt_pmu, pmu_name, -1);
As below, you can put back the devm cleanup that you had in v4 now you
have modified how the filter cleanup is done to also be devm managed.
> +}
> +
> /*
> * The DMA of PTT trace can only use direct mapping, due to some
> * hardware restriction. Check whether there is an IOMMU or the
> @@ -303,15 +825,32 @@ static int hisi_ptt_probe(struct pci_dev *pdev,
>
> pci_set_master(pdev);
>
> + ret = hisi_ptt_register_irq(hisi_ptt);
> + if (ret)
> + return ret;
> +
> ret = hisi_ptt_init_ctrls(hisi_ptt);
> if (ret) {
> pci_err(pdev, "failed to init controls, ret = %d.\n", ret);
> return ret;
> }
>
> + ret = hisi_ptt_register_pmu(hisi_ptt);
> + if (ret) {
> + pci_err(pdev, "failed to register pmu device, ret = %d", ret);
> + return ret;
> + }
> +
> return 0;
> }
>
> +void hisi_ptt_remove(struct pci_dev *pdev)
> +{
> + struct hisi_ptt *hisi_ptt = pci_get_drvdata(pdev);
> +
> + perf_pmu_unregister(&hisi_ptt->hisi_ptt_pmu);
Now you have the filter cleanup occurring using a devm_add_action_or_reset()
there is no need to have a manual cleanup of this - you can
use the approach of a devm_add_action_or_reset like you had in v4.
As it is the last call in the probe() order it will be the first one
called in the device managed cleanup.
> +}
> +
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-03-08 10:35 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-08 8:49 [PATCH v5 0/8] Add support for HiSilicon PCIe Tune and Trace device Yicong Yang
2022-03-08 8:49 ` [PATCH v5 1/8] iommu/arm-smmu-v3: Make default domain type of HiSilicon PTT device to identity Yicong Yang
2022-03-11 17:27 ` John Garry
2022-03-16 6:15 ` Yicong Yang
2022-03-08 8:49 ` [PATCH v5 2/8] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device Yicong Yang
2022-03-11 17:55 ` John Garry
2022-03-16 7:49 ` Yicong Yang
2022-03-08 8:49 ` [PATCH v5 3/8] hisi_ptt: Register PMU device for PTT trace Yicong Yang
2022-03-08 10:21 ` Jonathan Cameron [this message]
2022-03-08 11:13 ` Yicong Yang
2022-03-08 12:06 ` Jonathan Cameron
2022-03-08 13:20 ` Yicong Yang
2022-03-08 8:49 ` [PATCH v5 4/8] hisi_ptt: Add support for dynamically updating the filter list Yicong Yang
2022-03-08 10:23 ` Jonathan Cameron
2022-03-08 8:49 ` [PATCH v5 5/8] hisi_ptt: Add tune function support for HiSilicon PCIe Tune and Trace device Yicong Yang
2022-03-08 8:49 ` [PATCH v5 6/8] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver Yicong Yang
2022-03-08 8:49 ` [PATCH v5 7/8] docs: Add HiSilicon PTT device driver documentation Yicong Yang
2022-03-08 8:49 ` [PATCH v5 8/8] MAINTAINERS: Add maintainer for HiSilicon PTT driver Yicong Yang
2022-03-08 10:32 ` Jonathan Cameron
2022-03-08 11:49 ` Yicong Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220308102157.00003725@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=coresight@lists.linaro.org \
--cc=daniel.thompson@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=helgaas@kernel.org \
--cc=iommu@lists.linux-foundation.org \
--cc=john.garry@huawei.com \
--cc=joro@8bytes.org \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=liuqi115@huawei.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=mark.rutland@arm.com \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=prime.zeng@huawei.com \
--cc=robin.murphy@arm.com \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=song.bao.hua@hisilicon.com \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
--cc=yangyicong@hisilicon.com \
--cc=zhangshaokun@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).