Re: [PATCH v10 2/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU

[liuqi (BA)]

On 2021/10/6 22:06, Jonathan Cameron wrote:
> On Wed, 15 Sep 2021 15:45:24 +0800
> Qi Liu <liuqi115@xxxxxxxxxx> wrote:
>
> > PCIe PMU Root Complex Integrated End Point(RCiEP) device is supported
> > to sample bandwidth, latency, buffer occupation etc.
> >
> > Each PMU RCiEP device monitors multiple Root Ports, and each RCiEP is
> > registered as a PMU in /sys/bus/event_source/devices, so users can
> > select target PMU, and use filter to do further sets.
> >
> > Filtering options contains:
> > event     - select the event.
> > port      - select target Root Ports. Information of Root Ports are
> >              shown under sysfs.
> > bdf       - select requester_id of target EP device.
> > trig_len  - set trigger condition for starting event statistics.
> > trig_mode - set trigger mode. 0 means starting to statistic when bigger
> >              than trigger condition, and 1 means smaller.
> > thr_len   - set threshold for statistics.
> > thr_mode  - set threshold mode. 0 means count when bigger than threshold,
> >              and 1 means smaller.
> >
> > Reviewed-by: John Garry <john.garry@xxxxxxxxxx>
> > Signed-off-by: Qi Liu <liuqi115@xxxxxxxxxx>
> > ---
> Hi Qi,
>
> One trivial thing I just noticed below that can be easily fixed up by a follow up
> patch if that makes sense.
>
> Thanks,
>
> Jonathan
>
> > +/*
> > + * Events with the "dl" suffix in their names count performance in DL layer,
> > + * otherswise, events count performance in TL layer.
>
> This comment looks to be out of date..
Hi all,

Yes, this comment should be deleted, and I'll drop it in next version, 
thanks.

@Will, do you have any comments about this patchset?

Thanks,
Qi

> > + */
> > +static struct attribute *hisi_pcie_pmu_events_attr[] = {
> > +	HISI_PCIE_PMU_EVENT_ATTR(rx_mwr_latency, 0x0010),
> > +	HISI_PCIE_PMU_EVENT_ATTR(rx_mwr_cnt, 0x10010),
> > +	HISI_PCIE_PMU_EVENT_ATTR(rx_mrd_latency, 0x0210),
> > +	HISI_PCIE_PMU_EVENT_ATTR(rx_mrd_cnt, 0x10210),
> > +	HISI_PCIE_PMU_EVENT_ATTR(tx_mrd_latency, 0x0011),
> > +	HISI_PCIE_PMU_EVENT_ATTR(tx_mrd_cnt, 0x10011),
> > +	HISI_PCIE_PMU_EVENT_ATTR(rx_mrd_flux, 0x1005),
> > +	HISI_PCIE_PMU_EVENT_ATTR(rx_mrd_time, 0x11005),
> > +	HISI_PCIE_PMU_EVENT_ATTR(tx_mrd_flux, 0x2004),
> > +	HISI_PCIE_PMU_EVENT_ATTR(tx_mrd_time, 0x12004),
> > +	NULL
> > +};
> .