Re: [PATCH v3 1/8] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

2022-02-14 Thread Yicong Yang via iommu
On 2022/2/8 19:07, Yicong Yang wrote:
> On 2022/2/7 19:42, Jonathan Cameron wrote:
>> On Mon, 24 Jan 2022 21:11:11 +0800
>> Yicong Yang  wrote:
>>
>>> 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, and trace
>>> the TLP headers.
>>>
>>> Add the driver for the device to enable the trace function.
>>> This patch adds basic function of trace, including the device's
>>> probe and initialization, functions for trace buffer allocation
>>> and trace enable/disable, register an interrupt handler to
>>> simply response to the DMA events. The user interface of trace
>>> will be added in the following patch.
>>>
>>> Signed-off-by: Yicong Yang 
>> Hi Yicong,
>>
>> I've not been following all the earlier discussion on this driver closely
>> so I may well raise something that has already been addressed. If so
>> just ignore the comment.
> 
> Thanks for the comments. It's ok for me to clarify it :).
> Part replies inline and I need to do some test on the others.
> 
>>
>> Thanks,
>>
>> Jonathan
>>
>>> ---
>>>  drivers/Makefile |   1 +
>>>  drivers/hwtracing/Kconfig|   2 +
>>>  drivers/hwtracing/ptt/Kconfig|  11 +
>>>  drivers/hwtracing/ptt/Makefile   |   2 +
>>>  drivers/hwtracing/ptt/hisi_ptt.c | 398 +++
>>>  drivers/hwtracing/ptt/hisi_ptt.h | 159 
>>>  6 files changed, 573 insertions(+)
>>>  create mode 100644 drivers/hwtracing/ptt/Kconfig
>>>  create mode 100644 drivers/hwtracing/ptt/Makefile
>>>  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.c
>>>  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.h
>>>
> [...]
>>> +
>>> +static int hisi_ptt_alloc_trace_buf(struct hisi_ptt *hisi_ptt)
>>> +{
>>> +   struct hisi_ptt_trace_ctrl *ctrl = _ptt->trace_ctrl;
>>> +   struct device *dev = _ptt->pdev->dev;
>>> +   struct hisi_ptt_dma_buffer *buffer;
>>> +   int i, ret;
>>> +
>>> +   hisi_ptt->trace_ctrl.buf_index = 0;
>>> +
>>> +   /* Make sure the trace buffer is empty before allocating */
>>
>> This comment is misleading as it suggests it not being empty is
>> a bad thing but the code handles it as an acceptable path.
>> Perhaps:
>>  /*
>>   * If the trace buffer has already been allocated, zero the
>>   * memory.
>>   */
>>
> 
> will make it less misleading. thanks.
> 
>>> +   if (!list_empty(>trace_buf)) {
>>> +   list_for_each_entry(buffer, >trace_buf, list)
>>> +   memset(buffer->addr, 0, buffer->size);
>>> +   return 0;
>>> +   }
>>> +
>>> +   for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; ++i) {
>>> +   buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>>> +   if (!buffer) {
>>> +   ret = -ENOMEM;
>>> +   goto err;
>>> +   }
>>> +
>>> +   buffer->addr = dma_alloc_coherent(dev, ctrl->buffer_size,
>>> + >dma, GFP_KERNEL);
>>> +   if (!buffer->addr) {
>>> +   kfree(buffer);
>>> +   ret = -ENOMEM;
>>> +   goto err;
>>> +   }
>>> +
>>> +   memset(buffer->addr, 0, buffer->size);
>> See:
>> https://lore.kernel.org/lkml/20190108130701.14161-4-...@lst.de/
>> dma_alloc_coherent() always zeros the memory for us hence there
>> is no longer a dma_kzalloc_coherent()
>>
> 
> thanks for the information. Then the memset here is redundant and will drop 
> it.
> 
>>> +
>>> +   buffer->index = i;
>>
>> Carrying an index inside a list which corresponds directly
>> to the position in the list is not particularly nice.
>> Why can't we compute this index on the fly where the list
>> is walked?  Or am I misunderstanding and the order of the buffers
>> is changed in a later patch?
>>
> 
> The index is fixed once allocated and I stored it to avoid later
> computing. But seems it's highly recommended to compute these sort
> of things on the fly when necessary. John recommends the same things
> on some other places so I think I can get these addressed.
> 
>> As a side note, is a list actually appropriate when we always
>> have 4 of these buffers?  Feels like an array of buffer
>> structures might be cheaper.
>>

As suggested here and below, I tried to maintianed the buffers with
an array instead of a list and it looks more straightforward and some
fields of buffer structure can also be dropped. So I think I can change
to use an array.

Thanks for the suggestion!

Yicong

>>> +   buffer->size = ctrl->buffer_size;
>>> +   list_add_tail(>list, >trace_buf);
>>> +   }
>>> +
>>> +   return 0;
>>> +err:
>>> +   hisi_ptt_free_trace_buf(hisi_ptt);
>>> +   return ret;
>>> +}
>>> +
>>> +static void hisi_ptt_trace_end(struct hisi_ptt *hisi_ptt)
>>> +{
>>> +   writel(0, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>>> +   hisi_ptt->trace_ctrl.status = HISI_PTT_TRACE_STATUS_OFF;
>>> +}
>>> +
>>> +static int 

Re: [PATCH v3 1/8] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

2022-02-08 Thread Yicong Yang via iommu
On 2022/2/7 19:42, Jonathan Cameron wrote:
> On Mon, 24 Jan 2022 21:11:11 +0800
> Yicong Yang  wrote:
> 
>> 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, and trace
>> the TLP headers.
>>
>> Add the driver for the device to enable the trace function.
>> This patch adds basic function of trace, including the device's
>> probe and initialization, functions for trace buffer allocation
>> and trace enable/disable, register an interrupt handler to
>> simply response to the DMA events. The user interface of trace
>> will be added in the following patch.
>>
>> Signed-off-by: Yicong Yang 
> Hi Yicong,
> 
> I've not been following all the earlier discussion on this driver closely
> so I may well raise something that has already been addressed. If so
> just ignore the comment.

Thanks for the comments. It's ok for me to clarify it :).
Part replies inline and I need to do some test on the others.

> 
> Thanks,
> 
> Jonathan
> 
>> ---
>>  drivers/Makefile |   1 +
>>  drivers/hwtracing/Kconfig|   2 +
>>  drivers/hwtracing/ptt/Kconfig|  11 +
>>  drivers/hwtracing/ptt/Makefile   |   2 +
>>  drivers/hwtracing/ptt/hisi_ptt.c | 398 +++
>>  drivers/hwtracing/ptt/hisi_ptt.h | 159 
>>  6 files changed, 573 insertions(+)
>>  create mode 100644 drivers/hwtracing/ptt/Kconfig
>>  create mode 100644 drivers/hwtracing/ptt/Makefile
>>  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.c
>>  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.h
>>
[...]
>> +
>> +static int hisi_ptt_alloc_trace_buf(struct hisi_ptt *hisi_ptt)
>> +{
>> +struct hisi_ptt_trace_ctrl *ctrl = _ptt->trace_ctrl;
>> +struct device *dev = _ptt->pdev->dev;
>> +struct hisi_ptt_dma_buffer *buffer;
>> +int i, ret;
>> +
>> +hisi_ptt->trace_ctrl.buf_index = 0;
>> +
>> +/* Make sure the trace buffer is empty before allocating */
> 
> This comment is misleading as it suggests it not being empty is
> a bad thing but the code handles it as an acceptable path.
> Perhaps:
>   /*
>* If the trace buffer has already been allocated, zero the
>* memory.
>*/
> 

will make it less misleading. thanks.

>> +if (!list_empty(>trace_buf)) {
>> +list_for_each_entry(buffer, >trace_buf, list)
>> +memset(buffer->addr, 0, buffer->size);
>> +return 0;
>> +}
>> +
>> +for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; ++i) {
>> +buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>> +if (!buffer) {
>> +ret = -ENOMEM;
>> +goto err;
>> +}
>> +
>> +buffer->addr = dma_alloc_coherent(dev, ctrl->buffer_size,
>> +  >dma, GFP_KERNEL);
>> +if (!buffer->addr) {
>> +kfree(buffer);
>> +ret = -ENOMEM;
>> +goto err;
>> +}
>> +
>> +memset(buffer->addr, 0, buffer->size);
> See:
> https://lore.kernel.org/lkml/20190108130701.14161-4-...@lst.de/
> dma_alloc_coherent() always zeros the memory for us hence there
> is no longer a dma_kzalloc_coherent()
> 

thanks for the information. Then the memset here is redundant and will drop it.

>> +
>> +buffer->index = i;
> 
> Carrying an index inside a list which corresponds directly
> to the position in the list is not particularly nice.
> Why can't we compute this index on the fly where the list
> is walked?  Or am I misunderstanding and the order of the buffers
> is changed in a later patch?
> 

The index is fixed once allocated and I stored it to avoid later
computing. But seems it's highly recommended to compute these sort
of things on the fly when necessary. John recommends the same things
on some other places so I think I can get these addressed.

> As a side note, is a list actually appropriate when we always
> have 4 of these buffers?  Feels like an array of buffer
> structures might be cheaper.
> 
>> +buffer->size = ctrl->buffer_size;
>> +list_add_tail(>list, >trace_buf);
>> +}
>> +
>> +return 0;
>> +err:
>> +hisi_ptt_free_trace_buf(hisi_ptt);
>> +return ret;
>> +}
>> +
>> +static void hisi_ptt_trace_end(struct hisi_ptt *hisi_ptt)
>> +{
>> +writel(0, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>> +hisi_ptt->trace_ctrl.status = HISI_PTT_TRACE_STATUS_OFF;
>> +}
>> +
>> +static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>> +{
>> +struct hisi_ptt_trace_ctrl *ctrl = _ptt->trace_ctrl;
>> +struct hisi_ptt_dma_buffer *cur;
>> +u32 val;
>> +
>> +/* Check device idle before start trace */
>> +if (hisi_ptt_wait_trace_hw_idle(hisi_ptt)) {
>> +pci_err(hisi_ptt->pdev, "Failed to start trace, the device is 
>> still busy.\n");
>> +return -EBUSY;
>> +}
>> +

Re: [PATCH v3 1/8] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

2022-02-08 Thread Yicong Yang via iommu
Hi John,

Thanks for the comments. some replies inline.

On 2022/2/8 2:11, John Garry wrote:
> On 24/01/2022 13:11, Yicong Yang wrote:
>> 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, and trace
>> the TLP headers.
>>
>> Add the driver for the device to enable the trace function.
>> This patch adds basic function of trace, including the device's
>> probe and initialization, functions for trace buffer allocation
>> and trace enable/disable, register an interrupt handler to
>> simply response to the DMA events. The user interface of trace
>> will be added in the following patch.
>>
>> Signed-off-by: Yicong Yang 
>> ---
>>   drivers/Makefile |   1 +
>>   drivers/hwtracing/Kconfig    |   2 +
>>   drivers/hwtracing/ptt/Kconfig    |  11 +
>>   drivers/hwtracing/ptt/Makefile   |   2 +
>>   drivers/hwtracing/ptt/hisi_ptt.c | 398 +++
>>   drivers/hwtracing/ptt/hisi_ptt.h | 159 
>>   6 files changed, 573 insertions(+)
>>   create mode 100644 drivers/hwtracing/ptt/Kconfig
>>   create mode 100644 drivers/hwtracing/ptt/Makefile
>>   create mode 100644 drivers/hwtracing/ptt/hisi_ptt.c
>>   create mode 100644 drivers/hwtracing/ptt/hisi_ptt.h
>>
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index a110338c860c..ab3411e4eba5 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -175,6 +175,7 @@ obj-$(CONFIG_USB4)    += thunderbolt/
>>   obj-$(CONFIG_CORESIGHT)    += hwtracing/coresight/
>>   obj-y    += hwtracing/intel_th/
>>   obj-$(CONFIG_STM)    += hwtracing/stm/
>> +obj-$(CONFIG_HISI_PTT)    += hwtracing/ptt/
>>   obj-$(CONFIG_ANDROID)    += android/
>>   obj-$(CONFIG_NVMEM)    += nvmem/
>>   obj-$(CONFIG_FPGA)    += fpga/
>> diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
>> index 13085835a636..911ee977103c 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/ptt/Kconfig"
>> +
>>   endmenu
>> diff --git a/drivers/hwtracing/ptt/Kconfig b/drivers/hwtracing/ptt/Kconfig
>> new file mode 100644
>> index ..4f4f2459ac47
>> --- /dev/null
>> +++ b/drivers/hwtracing/ptt/Kconfig
>> @@ -0,0 +1,11 @@
>> +# 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
>> +    help
>> +  HiSilicon PCIe Tune and Trace Device exist as a PCIe RCiEP
> 
> exists
> 
>> +  device, provides support for PCIe traffic tuning and
> 
> and it provides support...
> 

will fix, thanks.

>> +  tracing TLP headers to the memory.
>> +
>> +  This driver can also be built as a module. If so, the module
>> +  will be called hisi_ptt.
>> diff --git a/drivers/hwtracing/ptt/Makefile b/drivers/hwtracing/ptt/Makefile
>> new file mode 100644
>> index ..908c09a98161
>> --- /dev/null
>> +++ b/drivers/hwtracing/ptt/Makefile
>> @@ -0,0 +1,2 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +obj-$(CONFIG_HISI_PTT) += hisi_ptt.o
>> diff --git a/drivers/hwtracing/ptt/hisi_ptt.c 
>> b/drivers/hwtracing/ptt/hisi_ptt.c
>> new file mode 100644
>> index ..6d0a0ca5c0a9
>> --- /dev/null
>> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
>> @@ -0,0 +1,398 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for HiSilicon PCIe tune and trace device
>> + *
>> + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
>> + * Author: Yicong Yang 
>> + */
>> +
[...]
>> +static int hisi_ptt_alloc_trace_buf(struct hisi_ptt *hisi_ptt)
>> +{
>> +    struct hisi_ptt_trace_ctrl *ctrl = _ptt->trace_ctrl;
>> +    struct device *dev = _ptt->pdev->dev;
>> +    struct hisi_ptt_dma_buffer *buffer;
>> +    int i, ret;
>> +
>> +    hisi_ptt->trace_ctrl.buf_index = 0;
>> +
>> +    /* Make sure the trace buffer is empty before allocating */
>> +    if (!list_empty(>trace_buf)) {
>> +    list_for_each_entry(buffer, >trace_buf, list)
>> +    memset(buffer->addr, 0, buffer->size);
>> +    return 0;
>> +    }
>> +
>> +    for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; ++i) {
>> +    buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
> 
> I may have asked this before: why no devm usage?
> 

I remembered I was suggested for not using devm where we may need to manually
free it as it intends to be freed automically after the driver detachment.

>> +    if (!buffer) {
>> +    ret = -ENOMEM;
>> +    goto err;
>> +    }
>> +
>> +    buffer->addr = dma_alloc_coherent(dev, ctrl->buffer_size,
>> +  >dma, GFP_KERNEL);
>> +    if (!buffer->addr) {
>> +    kfree(buffer);
>> +    ret = -ENOMEM;
>> +    goto err;
>> +    }
>> +
>> +  

Re: [PATCH v3 1/8] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

2022-02-07 Thread John Garry via iommu

On 24/01/2022 13:11, Yicong Yang wrote:

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, and trace
the TLP headers.

Add the driver for the device to enable the trace function.
This patch adds basic function of trace, including the device's
probe and initialization, functions for trace buffer allocation
and trace enable/disable, register an interrupt handler to
simply response to the DMA events. The user interface of trace
will be added in the following patch.

Signed-off-by: Yicong Yang 
---
  drivers/Makefile |   1 +
  drivers/hwtracing/Kconfig|   2 +
  drivers/hwtracing/ptt/Kconfig|  11 +
  drivers/hwtracing/ptt/Makefile   |   2 +
  drivers/hwtracing/ptt/hisi_ptt.c | 398 +++
  drivers/hwtracing/ptt/hisi_ptt.h | 159 
  6 files changed, 573 insertions(+)
  create mode 100644 drivers/hwtracing/ptt/Kconfig
  create mode 100644 drivers/hwtracing/ptt/Makefile
  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.c
  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.h

diff --git a/drivers/Makefile b/drivers/Makefile
index a110338c860c..ab3411e4eba5 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_USB4)  += thunderbolt/
  obj-$(CONFIG_CORESIGHT)   += hwtracing/coresight/
  obj-y += hwtracing/intel_th/
  obj-$(CONFIG_STM) += hwtracing/stm/
+obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/
  obj-$(CONFIG_ANDROID) += android/
  obj-$(CONFIG_NVMEM)   += nvmem/
  obj-$(CONFIG_FPGA)+= fpga/
diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
index 13085835a636..911ee977103c 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/ptt/Kconfig"

+
  endmenu
diff --git a/drivers/hwtracing/ptt/Kconfig b/drivers/hwtracing/ptt/Kconfig
new file mode 100644
index ..4f4f2459ac47
--- /dev/null
+++ b/drivers/hwtracing/ptt/Kconfig
@@ -0,0 +1,11 @@
+# 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
+   help
+ HiSilicon PCIe Tune and Trace Device exist as a PCIe RCiEP


exists


+ device, provides support for PCIe traffic tuning and


and it provides support...


+ tracing TLP headers to the memory.
+
+ This driver can also be built as a module. If so, the module
+ will be called hisi_ptt.
diff --git a/drivers/hwtracing/ptt/Makefile b/drivers/hwtracing/ptt/Makefile
new file mode 100644
index ..908c09a98161
--- /dev/null
+++ b/drivers/hwtracing/ptt/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_HISI_PTT) += hisi_ptt.o
diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
new file mode 100644
index ..6d0a0ca5c0a9
--- /dev/null
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -0,0 +1,398 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for HiSilicon PCIe tune and trace device
+ *
+ * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
+ * Author: Yicong Yang 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "hisi_ptt.h"
+
+static u16 hisi_ptt_get_filter_val(struct pci_dev *pdev)
+{
+   if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
+   return BIT(HISI_PCIE_CORE_PORT_ID(PCI_SLOT(pdev->devfn)));
+
+   return PCI_DEVID(pdev->bus->number, pdev->devfn);
+}
+
+static int hisi_ptt_wait_trace_hw_idle(struct hisi_ptt *hisi_ptt)
+{
+   u32 val;
+
+   return readl_poll_timeout(hisi_ptt->iobase + HISI_PTT_TRACE_STS, val,
+ val & HISI_PTT_TRACE_IDLE,
+ HISI_PTT_WAIT_POLL_INTERVAL_US,
+ HISI_PTT_WAIT_TIMEOUT_US);
+}
+
+static void hisi_ptt_free_trace_buf(struct hisi_ptt *hisi_ptt)
+{
+   struct hisi_ptt_trace_ctrl *ctrl = _ptt->trace_ctrl;
+   struct device *dev = _ptt->pdev->dev;
+   struct hisi_ptt_dma_buffer *buffer, *tbuffer;
+
+   list_for_each_entry_safe(buffer, tbuffer, >trace_buf, list) {
+   list_del(>list);
+   dma_free_coherent(dev, buffer->size, buffer->addr,
+ buffer->dma);
+   kfree(buffer);
+   }
+}
+
+static int hisi_ptt_alloc_trace_buf(struct hisi_ptt *hisi_ptt)
+{
+   struct hisi_ptt_trace_ctrl *ctrl = _ptt->trace_ctrl;
+   struct device *dev = _ptt->pdev->dev;
+   struct hisi_ptt_dma_buffer *buffer;
+   int i, ret;
+
+   hisi_ptt->trace_ctrl.buf_index = 0;
+
+   /* Make sure the trace 

Re: [PATCH v3 1/8] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

2022-02-07 Thread Jonathan Cameron via iommu
On Mon, 24 Jan 2022 21:11:11 +0800
Yicong Yang  wrote:

> 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, and trace
> the TLP headers.
> 
> Add the driver for the device to enable the trace function.
> This patch adds basic function of trace, including the device's
> probe and initialization, functions for trace buffer allocation
> and trace enable/disable, register an interrupt handler to
> simply response to the DMA events. The user interface of trace
> will be added in the following patch.
> 
> Signed-off-by: Yicong Yang 
Hi Yicong,

I've not been following all the earlier discussion on this driver closely
so I may well raise something that has already been addressed. If so
just ignore the comment.

Thanks,

Jonathan

> ---
>  drivers/Makefile |   1 +
>  drivers/hwtracing/Kconfig|   2 +
>  drivers/hwtracing/ptt/Kconfig|  11 +
>  drivers/hwtracing/ptt/Makefile   |   2 +
>  drivers/hwtracing/ptt/hisi_ptt.c | 398 +++
>  drivers/hwtracing/ptt/hisi_ptt.h | 159 
>  6 files changed, 573 insertions(+)
>  create mode 100644 drivers/hwtracing/ptt/Kconfig
>  create mode 100644 drivers/hwtracing/ptt/Makefile
>  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.c
>  create mode 100644 drivers/hwtracing/ptt/hisi_ptt.h
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index a110338c860c..ab3411e4eba5 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -175,6 +175,7 @@ obj-$(CONFIG_USB4)+= thunderbolt/
>  obj-$(CONFIG_CORESIGHT)  += hwtracing/coresight/
>  obj-y+= hwtracing/intel_th/
>  obj-$(CONFIG_STM)+= hwtracing/stm/
> +obj-$(CONFIG_HISI_PTT)   += hwtracing/ptt/
>  obj-$(CONFIG_ANDROID)+= android/
>  obj-$(CONFIG_NVMEM)  += nvmem/
>  obj-$(CONFIG_FPGA)   += fpga/
> diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
> index 13085835a636..911ee977103c 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/ptt/Kconfig"
> +
>  endmenu
> diff --git a/drivers/hwtracing/ptt/Kconfig b/drivers/hwtracing/ptt/Kconfig
> new file mode 100644
> index ..4f4f2459ac47
> --- /dev/null
> +++ b/drivers/hwtracing/ptt/Kconfig
> @@ -0,0 +1,11 @@
> +# 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
> + help
> +   HiSilicon PCIe Tune and Trace Device exist as a PCIe RCiEP
> +   device, provides support for PCIe traffic tuning and
> +   tracing TLP headers to the memory.
> +
> +   This driver can also be built as a module. If so, the module
> +   will be called hisi_ptt.
> diff --git a/drivers/hwtracing/ptt/Makefile b/drivers/hwtracing/ptt/Makefile
> new file mode 100644
> index ..908c09a98161
> --- /dev/null
> +++ b/drivers/hwtracing/ptt/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_HISI_PTT) += hisi_ptt.o
> diff --git a/drivers/hwtracing/ptt/hisi_ptt.c 
> b/drivers/hwtracing/ptt/hisi_ptt.c
> new file mode 100644
> index ..6d0a0ca5c0a9
> --- /dev/null
> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
> @@ -0,0 +1,398 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for HiSilicon PCIe tune and trace device
> + *
> + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
> + * Author: Yicong Yang 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "hisi_ptt.h"
> +
> +static u16 hisi_ptt_get_filter_val(struct pci_dev *pdev)
> +{
> + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> + return BIT(HISI_PCIE_CORE_PORT_ID(PCI_SLOT(pdev->devfn)));
> +
> + return PCI_DEVID(pdev->bus->number, pdev->devfn);
> +}
> +
> +static int hisi_ptt_wait_trace_hw_idle(struct hisi_ptt *hisi_ptt)
> +{
> + u32 val;
> +
> + return readl_poll_timeout(hisi_ptt->iobase + HISI_PTT_TRACE_STS, val,
> +   val & HISI_PTT_TRACE_IDLE,
> +   HISI_PTT_WAIT_POLL_INTERVAL_US,
> +   HISI_PTT_WAIT_TIMEOUT_US);
> +}
> +
> +static void hisi_ptt_free_trace_buf(struct hisi_ptt *hisi_ptt)
> +{
> + struct hisi_ptt_trace_ctrl *ctrl = _ptt->trace_ctrl;
> + struct device *dev = _ptt->pdev->dev;
> + struct hisi_ptt_dma_buffer *buffer, *tbuffer;
> +
> + list_for_each_entry_safe(buffer, tbuffer, >trace_buf, list) {
> + list_del(>list);
> + dma_free_coherent(dev, buffer->size, buffer->addr,
> +