Re: [PATCH 1/4] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device
Hi Yicong,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc6 next-20210406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Yicong-Yang/Add-support-for-HiSilicon-PCIe-Tune-and-Trace-device/20210406-204959
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
0a50438c84363bd37fe18fe432888ae9a074dcab
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/8d755179573b25c8c165509321a32c3c04b10ab5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Yicong-Yang/Add-support-for-HiSilicon-PCIe-Tune-and-Trace-device/20210406-204959
git checkout 8d755179573b25c8c165509321a32c3c04b10ab5
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_irq_register':
>> drivers/hwtracing/hisilicon/hisi_ptt.c:1067:3: error: implicit declaration
>> of function 'pci_free_irq_vectors'; did you mean 'pci_alloc_irq_vectors'?
>> [-Werror=implicit-function-declaration]
1067 | pci_free_irq_vectors(pdev);
| ^~~~
| pci_alloc_irq_vectors
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_init_ctrls':
>> drivers/hwtracing/hisilicon/hisi_ptt.c:1231:8: error: implicit declaration
>> of function 'pci_find_bus'; did you mean 'pci_find_next_bus'?
>> [-Werror=implicit-function-declaration]
1231 | bus = pci_find_bus(pci_domain_nr(hisi_ptt->pdev->bus),
|^~~~
|pci_find_next_bus
drivers/hwtracing/hisilicon/hisi_ptt.c:1231:6: warning: assignment to
'struct pci_bus *' from 'int' makes pointer from integer without a cast
[-Wint-conversion]
1231 | bus = pci_find_bus(pci_domain_nr(hisi_ptt->pdev->bus),
| ^
>> drivers/hwtracing/hisilicon/hisi_ptt.c:1234:3: error: implicit declaration
>> of function 'pci_walk_bus' [-Werror=implicit-function-declaration]
1234 | pci_walk_bus(bus, hisi_ptt_init_filters, hisi_ptt);
| ^~~~
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_probe':
>> drivers/hwtracing/hisilicon/hisi_ptt.c:1359:31: error: 'pci_bus_type'
>> undeclared (first use in this function); did you mean 'pci_pcie_type'?
1359 | ret = bus_register_notifier(_bus_type, _ptt->hisi_ptt_nb);
| ^~~~
| pci_pcie_type
drivers/hwtracing/hisilicon/hisi_ptt.c:1359:31: note: each undeclared
identifier is reported only once for each function it appears in
drivers/hwtracing/hisilicon/hisi_ptt.c: At top level:
drivers/hwtracing/hisilicon/hisi_ptt.c:1366:6: warning: no previous
prototype for 'hisi_ptt_remove' [-Wmissing-prototypes]
1366 | void hisi_ptt_remove(struct pci_dev *pdev)
| ^~~
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_remove':
drivers/hwtracing/hisilicon/hisi_ptt.c:1370:27: error: 'pci_bus_type'
undeclared (first use in this function); did you mean 'pci_pcie_type'?
1370 | bus_unregister_notifier(_bus_type, _ptt->hisi_ptt_nb);
| ^~~~
| pci_pcie_type
cc1: some warnings being treated as errors
vim +1067 drivers/hwtracing/hisilicon/hisi_ptt.c
1048
1049 static int hisi_ptt_irq_register(struct hisi_ptt *hisi_ptt)
1050 {
1051 struct pci_dev *pdev = hisi_ptt->pdev;
1052 int ret;
1053
1054 ret = pci_alloc_irq_vectors(pdev, HISI_PTT_IRQ_NUMS,
HISI_PTT_IRQ_NUMS,
1055 PCI_IRQ_MSI);
1056 if (ret < 0) {
1057 pci_err(pdev, "failed to allocate irq vector, ret =
%d\n", ret);
1058 return ret;
1059 }
1060
1061 ret = request_threaded_irq(pci_irq_vector(pdev,
HISI_PTT_DMA_IRQ),
1062 hisi_ptt_irq, hisi_ptt_isr,
IRQF_SHARED,
1063 "hisi-ptt", hisi_ptt);
1064 if (ret) {
1065 pci_err(pdev, "failed to request irq %d, ret = %d\n",
1066 pci_irq_vector(pdev, HISI_PTT_DMA_IRQ), ret);
> 1067
Re: [PATCH 1/4] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device
Hi Yicong,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.12-rc6 next-20210406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Yicong-Yang/Add-support-for-HiSilicon-PCIe-Tune-and-Trace-device/20210406-204959
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
0a50438c84363bd37fe18fe432888ae9a074dcab
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/8d755179573b25c8c165509321a32c3c04b10ab5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Yicong-Yang/Add-support-for-HiSilicon-PCIe-Tune-and-Trace-device/20210406-204959
git checkout 8d755179573b25c8c165509321a32c3c04b10ab5
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_irq_register':
drivers/hwtracing/hisilicon/hisi_ptt.c:1067:3: error: implicit declaration
of function 'pci_free_irq_vectors'; did you mean 'pci_alloc_irq_vectors'?
[-Werror=implicit-function-declaration]
1067 | pci_free_irq_vectors(pdev);
| ^~~~
| pci_alloc_irq_vectors
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_init_ctrls':
drivers/hwtracing/hisilicon/hisi_ptt.c:1231:8: error: implicit declaration
of function 'pci_find_bus'; did you mean 'pci_find_next_bus'?
[-Werror=implicit-function-declaration]
1231 | bus = pci_find_bus(pci_domain_nr(hisi_ptt->pdev->bus),
|^~~~
|pci_find_next_bus
>> drivers/hwtracing/hisilicon/hisi_ptt.c:1231:6: warning: assignment to
>> 'struct pci_bus *' from 'int' makes pointer from integer without a cast
>> [-Wint-conversion]
1231 | bus = pci_find_bus(pci_domain_nr(hisi_ptt->pdev->bus),
| ^
drivers/hwtracing/hisilicon/hisi_ptt.c:1234:3: error: implicit declaration
of function 'pci_walk_bus' [-Werror=implicit-function-declaration]
1234 | pci_walk_bus(bus, hisi_ptt_init_filters, hisi_ptt);
| ^~~~
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_probe':
drivers/hwtracing/hisilicon/hisi_ptt.c:1359:31: error: 'pci_bus_type'
undeclared (first use in this function); did you mean 'pci_pcie_type'?
1359 | ret = bus_register_notifier(_bus_type, _ptt->hisi_ptt_nb);
| ^~~~
| pci_pcie_type
drivers/hwtracing/hisilicon/hisi_ptt.c:1359:31: note: each undeclared
identifier is reported only once for each function it appears in
drivers/hwtracing/hisilicon/hisi_ptt.c: At top level:
drivers/hwtracing/hisilicon/hisi_ptt.c:1366:6: warning: no previous
prototype for 'hisi_ptt_remove' [-Wmissing-prototypes]
1366 | void hisi_ptt_remove(struct pci_dev *pdev)
| ^~~
drivers/hwtracing/hisilicon/hisi_ptt.c: In function 'hisi_ptt_remove':
drivers/hwtracing/hisilicon/hisi_ptt.c:1370:27: error: 'pci_bus_type'
undeclared (first use in this function); did you mean 'pci_pcie_type'?
1370 | bus_unregister_notifier(_bus_type, _ptt->hisi_ptt_nb);
| ^~~~
| pci_pcie_type
cc1: some warnings being treated as errors
vim +1231 drivers/hwtracing/hisilicon/hisi_ptt.c
1209
1210 static void hisi_ptt_init_ctrls(struct hisi_ptt *hisi_ptt)
1211 {
1212 struct pci_bus *bus;
1213 u32 reg;
1214
1215 INIT_LIST_HEAD(_ptt->port_filters);
1216 INIT_LIST_HEAD(_ptt->req_filters);
1217
1218 /*
1219 * The device range register provides the information about the
1220 * root ports which the RCiEP can control and trace. The RCiEP
1221 * and the root ports it support are on the same PCIe core, with
1222 * same domain number but maybe different bus number. The device
1223 * range register will tell us which root ports we can support,
1224 * Bit[31:16] indicates the upper BDF numbers of the root port,
1225 * while Bit[15:0] indicates the lower.
1226 */
1227 reg = readl(hisi_ptt->iobase + HISI_PTT_DEVICE_RANGE);
1228 hisi_ptt->upper =
Re: [PATCH 1/4] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device
On Tue, Apr 06, 2021 at 08:45:51PM +0800, Yicong Yang wrote:
> +static int hisi_ptt_create_trace_entries(struct hisi_ptt *hisi_ptt)
> +{
> + struct hisi_ptt_debugfs_file_desc *trace_files;
> + struct dentry *dir;
> + int i, ret = 0;
> +
> + dir = debugfs_create_dir("trace", hisi_ptt->debugfs_dir);
> + if (IS_ERR(dir))
> + return PTR_ERR(dir);
No need to care about this, please do not check, code should not do
different things based on if debugfs is working or not.
> +
> + trace_files = devm_kmemdup(_ptt->pdev->dev, trace_entries,
> +sizeof(trace_entries), GFP_KERNEL);
> + if (IS_ERR(trace_files)) {
> + ret = PTR_ERR(trace_files);
> + goto err;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(trace_entries); ++i) {
> + struct dentry *file;
> +
> + trace_files[i].hisi_ptt = hisi_ptt;
> + file = debugfs_create_file(trace_files[i].name, 0600,
> +dir, _files[i],
> +trace_files[i].fops);
> + if (IS_ERR(file)) {
> + ret = PTR_ERR(file);
Same here, why check?
> +static int hisi_ptt_register_debugfs(void)
> +{
> + if (!debugfs_initialized()) {
> + pr_err("failed to create debugfs directory: debugfs
> uninitialized\n");
Why do you care? How can this happen?
> + return -ENOENT;
> + }
> +
> + hisi_ptt_debugfs_root = debugfs_create_dir("hisi_ptt", NULL);
> + if (IS_ERR(hisi_ptt_debugfs_root)) {
Again, no need to check.
If you are building the whole functionality of your code on if debugfs
is working or not, that feels really wrong. Debugfs is for random
kernel debug type things, not a whole driver infrastructure that somehow
relies on it working or not.
thanks,
greg k-h
[PATCH 1/4] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device
HiSilicon PCIe tune and trace device(PTT) is a PCIe Root Complex integrated Endpoint(RCiEP) device, providing the capability to dynamically monitor and tune the PCIe traffic(tune), and trace the TLP headers(trace). Add the driver for the device to enable the trace function. The driver will create debugfs directory for each PTT device, and users can operate the device through the files under its directory. Signed-off-by: Yicong Yang --- drivers/Makefile |1 + drivers/hwtracing/Kconfig |2 + drivers/hwtracing/hisilicon/Kconfig|8 + drivers/hwtracing/hisilicon/Makefile |2 + drivers/hwtracing/hisilicon/hisi_ptt.c | 1449 5 files changed, 1462 insertions(+) create mode 100644 drivers/hwtracing/hisilicon/Kconfig create mode 100644 drivers/hwtracing/hisilicon/Makefile create mode 100644 drivers/hwtracing/hisilicon/hisi_ptt.c diff --git a/drivers/Makefile b/drivers/Makefile index 6fba7da..9a0f76d 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -174,6 +174,7 @@ obj-$(CONFIG_PERF_EVENTS) += perf/ obj-$(CONFIG_RAS) += ras/ obj-$(CONFIG_USB4) += thunderbolt/ obj-$(CONFIG_CORESIGHT)+= hwtracing/coresight/ +obj-$(CONFIG_HISI_PTT) += hwtracing/hisilicon/ obj-y += hwtracing/intel_th/ obj-$(CONFIG_STM) += hwtracing/stm/ obj-$(CONFIG_ANDROID) += android/ diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig index 1308583..e3796b1 100644 --- a/drivers/hwtracing/Kconfig +++ b/drivers/hwtracing/Kconfig @@ -5,4 +5,6 @@ source "drivers/hwtracing/stm/Kconfig" source "drivers/hwtracing/intel_th/Kconfig" +source "drivers/hwtracing/hisilicon/Kconfig" + endmenu diff --git a/drivers/hwtracing/hisilicon/Kconfig b/drivers/hwtracing/hisilicon/Kconfig new file mode 100644 index 000..e4c2379 --- /dev/null +++ b/drivers/hwtracing/hisilicon/Kconfig @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0-only +config HISI_PTT + tristate "HiSilicon PCIe Tune and Trace Device" + depends on (ARM64 && PCI && HAS_DMA && HAS_IOMEM && DEBUG_FS) || COMPILE_TEST + help + HiSilicon PCIe Tune and Trace Device exist as a PCIe iEP + device, provides support for PCIe traffic tuning and + tracing TLP headers to the memory. diff --git a/drivers/hwtracing/hisilicon/Makefile b/drivers/hwtracing/hisilicon/Makefile new file mode 100644 index 000..908c09a --- /dev/null +++ b/drivers/hwtracing/hisilicon/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_HISI_PTT) += hisi_ptt.o diff --git a/drivers/hwtracing/hisilicon/hisi_ptt.c b/drivers/hwtracing/hisilicon/hisi_ptt.c new file mode 100644 index 000..a1feece --- /dev/null +++ b/drivers/hwtracing/hisilicon/hisi_ptt.c @@ -0,0 +1,1449 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for HiSilicon PCIe tune and trace device + * + * Copyright (c) 2021 HiSilicon Limited. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define HISI_PTT_CTRL_STR_LEN 40 +#define HISI_PTT_DEFAULT_TRACE_BUF_CNT 64 +#define HISI_PTT_FILTER_UPDATE_FIFO_SIZE 16 + +#define HISI_PTT_RESET_WAIT_MS 1000UL +#define HISI_PTT_WORK_DELAY_MS 100UL +#define HISI_PTT_WAIT_TIMEOUT_US 100UL +#define HISI_PTT_WAIT_POLL_INTERVAL_US 100UL + +#define HISI_PTT_IRQ_NUMS 1 +#define HISI_PTT_DMA_IRQ 0 +#define HISI_PTT_DMA_NUMS 4 + +#define HISI_PTT_TUNING_CTRL 0x +#define HISI_PTT_TUNING_CTRL_CODEGENMASK(15, 0) +#define HISI_PTT_TUNING_CTRL_SUB GENMASK(23, 16) +#define HISI_PTT_TUNING_CTRL_CHN GENMASK(31, 24) +#define HISI_PTT_TUNING_DATA 0x0004 +#define HISI_PTT_TUNING_DATA_VAL_MASKGENMASK(15, 0) +#define HISI_PTT_TRACE_ADDR_SIZE 0x0800 +#define HISI_PTT_TRACE_ADDR_BASE_LO_0 0x0810 +#define HISI_PTT_TRACE_ADDR_BASE_HI_0 0x0814 +#define HISI_PTT_TRACE_ADDR_STRIDE 0x8 +#define HISI_PTT_TRACE_CTRL0x0850 +#define HISI_PTT_TRACE_CTRL_EN BIT(0) +#define HISI_PTT_TRACE_CTRL_RST BIT(1) +#define HISI_PTT_TRACE_CTRL_RXTX_SEL GENMASK(3, 2) +#define HISI_PTT_TRACE_CTRL_TYPE_SEL GENMASK(7, 4) +#define HISI_PTT_TRACE_CTRL_DATA_FORMAT BIT(14) +#define HISI_PTT_TRACE_CTRL_FILTER_MODE BIT(15) +#define HISI_PTT_TRACE_CTRL_TARGET_SEL GENMASK(31, 16) +#define HISI_PTT_TRACE_INT_STAT0x0890 +#define HISI_PTT_TRACE_INT_STAT_MASK GENMASK(3, 0) +#define HISI_PTT_TRACE_INT_MASK_REG0x0894 +#define HISI_PTT_TUNING_INT_STAT 0x0898 +#define HISI_PTT_TUNING_INT_STAT_MASKBIT(0) +#define HISI_PTT_TUNING_INT_MASK 0x089c +#define HISI_PTT_TRACE_WR_STS 0x08a0 +#define HISI_PTT_TRACE_WR_STS_WRITE GENMASK(27, 0) +#define
