lkml.org 
[lkml]   [2022]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 3/8] hisi_ptt: Register PMU device for PTT trace
    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.

    > +}
    > +


    \
     
     \ /
      Last update: 2022-03-08 11:23    [W:2.973 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site