Message-ID: <20230522152859.0000429e@Huawei.com>
Date:   Mon, 22 May 2023 15:28:59 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Shuai Xue <xueshuai@...ux.alibaba.com>
CC:     <chengyou@...ux.alibaba.com>, <kaishen@...ux.alibaba.com>,
        <helgaas@...nel.org>, <yangyicong@...wei.com>, <will@...nel.org>,
        <baolin.wang@...ux.alibaba.com>, <robin.murphy@....com>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-pci@...r.kernel.org>, <rdunlap@...radead.org>,
        <mark.rutland@....com>, <zhuo.song@...ux.alibaba.com>
Subject: Re: [PATCH v5 0/4] drivers/perf: add Synopsys DesignWare PCIe PMU
 driver support

On Mon, 22 May 2023 11:54:24 +0800
Shuai Xue <xueshuai@...ux.alibaba.com> wrote:

Hi,

Very rarely a good idea to send a new patch set version in reply to an old
one. Tends to just resort in it being way off the top of people's most recent
email (depending on client of course!)

Jonathan

> changes since v4:
> 
> 1. addressing commens from Bjorn Helgaas:
> - reorder the includes by alpha
> - change all macros with upper-case hex
> - change ras_des type into u16
> - remove unnecessary outer "()"
> - minor format changes
> 
> 2. Address commensts from Jonathan Cameron:
> - rewrite doc and add a example to show how to use lane event
> 
> 3. fix compile error reported by: kernel test robot
> - remove COMPILE_TEST and add depend on PCI in kconfig
> - add Reported-by: kernel test robot <lkp@...el.com>
> 
> Changes since v3:
> 
> 1. addressing comments from Robin Murphy:
> - add a prepare patch to define pci id in linux/pci_ids.h
> - remove unnecessary 64BIT dependency
> - fix DWC_PCIE_PER_EVENT_OFF/ON macro
> - remove dwc_pcie_pmu struct and move all its fileds into dwc_pcie_rp_info
> - remove unnecessary format field show
> - use sysfs_emit() instead of all the assorted sprintf() and snprintf() calls.
> - remove unnecessary spaces and remove unnecessary cast to follow event show convention
> - remove pcie_pmu_event_attr_is_visible
> - fix a refcout leak on error branch when walk pci device in for_each_pci_dev
> - remove bdf field from dwc_pcie_rp_info and calculate it at runtime
> - finish all the checks before allocating rp_info to avoid hanging wasted memory
> - remove some unused fields
> - warp out control register configuration from sub function to .add()
> - make function return type with a proper signature
> - fix lane event count enable by clear DWC_PCIE_CNT_ENABLE field first
> - pass rp_info directly to the read_*_counter helpers and in start, stop and add callbacks
> - move event type validtion into .event_init()
> - use is_sampling_event() to be consistent with everything else of pmu drivers
> - remove unnecessary dev_err message in .event_init()
> - return EINVAL instead EOPNOTSUPP for not a valid event 
> - finish all the checks before start modifying the event
> - fix sibling event check by comparing event->pmu with sibling->pmu
> - probe PMU for each rootport independently
> - use .update() as .read() directly
> - remove dynamically generating symbolic name of lane event
> - redefine static symbolic name of lane event and leave lane filed to user
> - add CPU hotplug support
> 
> 2. addressing comments from Baolin:
> - add a mask to avoid possible overflow
> 
> Changes since v2 addressing comments from Baolin:
> - remove redundant macro definitions
> - use dev_err to print error message
> - change pmu_is_register to boolean
> - use PLATFORM_DEVID_NONE macro
> - fix module author format
> 
> Changes since v1:
> 
> 1. address comments from Jonathan:
> - drop marco for PMU name and VSEC version
> - simplify code with PCI standard marco
> - simplify code with FIELD_PREP()/FIELD_GET() to replace shift marco
> - name register filed with single _ instead double
> - wrap dwc_pcie_pmu_{write}_dword out and drop meaningless snaity check 
> - check vendor id while matching vesc with pci_find_vsec_capability()
> - remove RP_NUM_MAX and use a list to organize PMU devices for rootports
> - replace DWC_PCIE_CREATE_BDF with standard PCI_DEVID
> - comments on riping register together
> 
> 2. address comments from Bjorn:
> - rename DWC_PCIE_VSEC_ID to DWC_PCIE_VSEC_RAS_DES_ID
> - rename cap_pos to ras_des
> - simplify declare of device_attribute with DEVICE_ATTR_RO
> - simplify code with PCI standard macro and API like pcie_get_width_cap()
> - fix some code style problem and typo
> - drop meaningless snaity check of container_of
> 
> 3. address comments from Yicong:
> - use sysfs_emit() to replace sprintf()
> - simplify iteration of pci device with for_each_pci_dev
> - pick preferred CPUs on a near die and add comments
> - unregister PMU drivers only for failed ones
> - log on behalf PMU device and give more hint
> - fix some code style problem
> 
> (Thanks for all comments and they are very valuable to me)
> 
> This patchset adds the PCIe Performance Monitoring Unit (PMU) driver support
> for T-Head Yitian 710 SoC chip. Yitian 710 is based on the Synopsys PCI Express
> Core controller IP which provides statistics feature.
> 
> Shuai Xue (4):
>   docs: perf: Add description for Synopsys DesignWare PCIe PMU driver
>   PCI: move Alibaba Vendor ID linux/pci_ids.h
>   drivers/perf: add DesignWare PCIe PMU driver
>   MAINTAINERS: add maintainers for DesignWare PCIe PMU driver
> 
>  .../admin-guide/perf/dwc_pcie_pmu.rst         |  97 +++
>  Documentation/admin-guide/perf/index.rst      |   1 +
>  MAINTAINERS                                   |   6 +
>  drivers/infiniband/hw/erdma/erdma_hw.h        |   2 -
>  drivers/perf/Kconfig                          |   7 +
>  drivers/perf/Makefile                         |   1 +
>  drivers/perf/dwc_pcie_pmu.c                   | 701 ++++++++++++++++++
>  include/linux/pci_ids.h                       |   2 +
>  8 files changed, 815 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/admin-guide/perf/dwc_pcie_pmu.rst
>  create mode 100644 drivers/perf/dwc_pcie_pmu.c
> 

Powered by blists - more mailing lists