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

2022-02-24 Thread Yicong Yang via iommu
On 2022/2/24 20:32, John Garry wrote:
> On 24/02/2022 03:53, Yicong Yang wrote:
>> On 2022/2/22 19:06, John Garry wrote:
>>> On 21/02/2022 08:43, 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.

>>>
>>> Fill commit message lines upto 75 characters
>>
>> Hi John,
>>
>> Thanks for the comments.
>>
>> The commit message is within 75 characters. I checked again and checkpatch
>> didn't warning for this.
> 
> I mean to fill the lines up as much as possible, upto 75 char max, if not 
> already done so. I am not sure if you are doing this already, but it looks 
> like you were not.
> 
> Checkpatch
> will
> no
> warn
> about
> a
> commit
> message
> like
> this
> :)
> 

Uh I got it. Thanks for the vivid clarification!
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2022-02-24 Thread John Garry via iommu

On 24/02/2022 03:53, Yicong Yang wrote:

On 2022/2/22 19:06, John Garry wrote:

On 21/02/2022 08:43, 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.



Fill commit message lines upto 75 characters


Hi John,

Thanks for the comments.

The commit message is within 75 characters. I checked again and checkpatch
didn't warning for this.


I mean to fill the lines up as much as possible, upto 75 char max, if 
not already done so. I am not sure if you are doing this already, but it 
looks like you were not.


Checkpatch
will
no
warn
about
a
commit
message
like
this
:)


Thanks,
john
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2022-02-23 Thread Yicong Yang via iommu
On 2022/2/22 19:06, John Garry wrote:
> On 21/02/2022 08:43, 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.
>>
> 
> Fill commit message lines upto 75 characters

Hi John,

Thanks for the comments.

The commit message is within 75 characters. I checked again and checkpatch
didn't warning for this.

> 
>> 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 | 370 +++
>>   drivers/hwtracing/ptt/hisi_ptt.h | 149 +
>>   6 files changed, 535 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 ..41fa83921a07
>> --- /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
> 
> why no compile test support?
> 

I'll add compile test on ARM64.

>> +    help
>> +  HiSilicon PCIe Tune and Trace Device exists as a PCIe RCiEP
>> +  device, and it 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 ..a5b4f09ccd1e
>> --- /dev/null
>> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
>> @@ -0,0 +1,370 @@
>> +// 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_atomic(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 int hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
>> +{
>> +    u32 val;
>> +
>> +    return readl_poll_timeout_atomic(hisi_ptt->iobase + 
>> HISI_PTT_TRACE_WR_STS,
>> + val, !val, HISI_PTT_RESET_POLL_INTERVAL_US,
>> + 

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

2022-02-22 Thread John Garry via iommu

On 21/02/2022 08:43, 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.



Fill commit message lines upto 75 characters


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 | 370 +++
  drivers/hwtracing/ptt/hisi_ptt.h | 149 +
  6 files changed, 535 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 ..41fa83921a07
--- /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


why no compile test support?


+   help
+ HiSilicon PCIe Tune and Trace Device exists as a PCIe RCiEP
+ device, and it 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 ..a5b4f09ccd1e
--- /dev/null
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -0,0 +1,370 @@
+// 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_atomic(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 int hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
+{
+   u32 val;
+
+   return readl_poll_timeout_atomic(hisi_ptt->iobase + 
HISI_PTT_TRACE_WR_STS,
+val, !val, 
HISI_PTT_RESET_POLL_INTERVAL_US,
+HISI_PTT_RESET_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;
+   int i;
+
+   if (!ctrl->trace_buf)
+   return;
+
+   for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)


it's good practice to use {} for if-else or similar in the loop


+   if 

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

2022-02-21 Thread Jonathan Cameron via iommu
On Mon, 21 Feb 2022 21:13:45 +0800
Yicong Yang  wrote:

> Hi Jonathan,
> 
> On 2022/2/21 19:18, Jonathan Cameron wrote:
> > On Mon, 21 Feb 2022 16:43:01 +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,
> > 
> > A few really minor things inline, particularly one place
> > where you can improve the error handling.
> > It's always fiddly to handle errors in a pci_walk_bus() but
> > in this case it's not too difficult as you just need to store
> > the retval somewhere in the private data then retrieve it
> > after the pci_walk_bus() call.
> >   
> 
> Thanks for the quick reply!
> 
> The pci_walk_bus() in this patch will fail only if the memory allocation
> of filter struct fails. We won't allocate memory in the pci_bus_walk()
> after Patch 4 so it will never fail. Maybe I can add some comments
> mentioning this.
Great. Given that answers my only significant question.

Reviewed-by: Jonathan Cameron 

> 
> I also expressed this inline.
> 
> > 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 | 370 +++
> >>  drivers/hwtracing/ptt/hisi_ptt.h | 149 +
> >>  6 files changed, 535 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 ..41fa83921a07
> >> --- /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 exists as a PCIe RCiEP
> >> +device, and it 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 ..a5b4f09ccd1e
> >> --- /dev/null
> >> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
> >> @@ -0,0 +1,370 @@  
> > 
> > ...
> >   
> >> +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;
> >> +  int i;
> >> +
> >> +  if (!ctrl->trace_buf)
> >> +  return;
> >> +
> >> +  for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)
> >> +  if (ctrl->trace_buf[i].addr)
> >> +  dma_free_coherent(dev, HISI_PTT_TRACE_BUF_SIZE,
> >> +  

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

2022-02-21 Thread Yicong Yang via iommu
Hi Jonathan,

On 2022/2/21 19:18, Jonathan Cameron wrote:
> On Mon, 21 Feb 2022 16:43:01 +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,
> 
> A few really minor things inline, particularly one place
> where you can improve the error handling.
> It's always fiddly to handle errors in a pci_walk_bus() but
> in this case it's not too difficult as you just need to store
> the retval somewhere in the private data then retrieve it
> after the pci_walk_bus() call.
> 

Thanks for the quick reply!

The pci_walk_bus() in this patch will fail only if the memory allocation
of filter struct fails. We won't allocate memory in the pci_bus_walk()
after Patch 4 so it will never fail. Maybe I can add some comments
mentioning this.

I also expressed this inline.

> 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 | 370 +++
>>  drivers/hwtracing/ptt/hisi_ptt.h | 149 +
>>  6 files changed, 535 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 ..41fa83921a07
>> --- /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 exists as a PCIe RCiEP
>> +  device, and it 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 ..a5b4f09ccd1e
>> --- /dev/null
>> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
>> @@ -0,0 +1,370 @@
> 
> ...
> 
>> +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;
>> +int i;
>> +
>> +if (!ctrl->trace_buf)
>> +return;
>> +
>> +for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)
>> +if (ctrl->trace_buf[i].addr)
>> +dma_free_coherent(dev, HISI_PTT_TRACE_BUF_SIZE,
>> +  ctrl->trace_buf[i].addr,
>> +  ctrl->trace_buf[i].dma);
>> +
>> +kfree(ctrl->trace_buf);
>> +ctrl->trace_buf = NULL;
>> +}
>> +
>> +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;
>> +

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

2022-02-21 Thread Jonathan Cameron via iommu
On Mon, 21 Feb 2022 16:43:01 +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,

A few really minor things inline, particularly one place
where you can improve the error handling.
It's always fiddly to handle errors in a pci_walk_bus() but
in this case it's not too difficult as you just need to store
the retval somewhere in the private data then retrieve it
after the pci_walk_bus() call.

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 | 370 +++
>  drivers/hwtracing/ptt/hisi_ptt.h | 149 +
>  6 files changed, 535 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 ..41fa83921a07
> --- /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 exists as a PCIe RCiEP
> +   device, and it 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 ..a5b4f09ccd1e
> --- /dev/null
> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
> @@ -0,0 +1,370 @@

...

> +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;
> + int i;
> +
> + if (!ctrl->trace_buf)
> + return;
> +
> + for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)
> + if (ctrl->trace_buf[i].addr)
> + dma_free_coherent(dev, HISI_PTT_TRACE_BUF_SIZE,
> +   ctrl->trace_buf[i].addr,
> +   ctrl->trace_buf[i].dma);
> +
> + kfree(ctrl->trace_buf);
> + ctrl->trace_buf = NULL;
> +}
> +
> +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;
> + int i;
> +
> + hisi_ptt->trace_ctrl.buf_index = 0;
> +
> + /* If the trace buffer has already been allocated, zero it. */
> + if (ctrl->trace_buf) {
> + for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)
> + memset(ctrl->trace_buf[i].addr, 0, 
> HISI_PTT_TRACE_BUF_SIZE);
> + return 0;
> + }
> +
> + ctrl->trace_buf = kcalloc(HISI_PTT_TRACE_BUF_CNT, sizeof(struct 
> hisi_ptt_dma_buffer),

Slight 

[PATCH v4 2/8] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

2022-02-21 Thread Yicong Yang via iommu
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 | 370 +++
 drivers/hwtracing/ptt/hisi_ptt.h | 149 +
 6 files changed, 535 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 ..41fa83921a07
--- /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 exists as a PCIe RCiEP
+ device, and it 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 ..a5b4f09ccd1e
--- /dev/null
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -0,0 +1,370 @@
+// 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_atomic(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 int hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
+{
+   u32 val;
+
+   return readl_poll_timeout_atomic(hisi_ptt->iobase + 
HISI_PTT_TRACE_WR_STS,
+val, !val, 
HISI_PTT_RESET_POLL_INTERVAL_US,
+HISI_PTT_RESET_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;
+   int i;
+
+   if (!ctrl->trace_buf)
+   return;
+
+   for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)
+   if (ctrl->trace_buf[i].addr)
+   dma_free_coherent(dev, HISI_PTT_TRACE_BUF_SIZE,
+ ctrl->trace_buf[i].addr,
+