2021-03-29 09:53:26

by Gustavo Pimentel

[permalink] [raw]
Subject: [PATCH v8 0/5] misc: Add Add Synopsys DesignWare xData IP driver

This patch series adds a new driver called xData-pcie for the Synopsys
DesignWare PCIe prototype.

The driver configures and enables the Synopsys DesignWare PCIe traffic
generator IP inside of prototype Endpoint which will generate upstream
and downstream PCIe traffic. This allows to quickly test the PCIe link
throughput speed and check is the prototype solution has some limitation
or not.

Changes:
V2: Rework driver according to Greg Kroah-Hartman' feedback
- Replace module parameter by sysfs use.
- Replace bit fields structure with macros and masks use.
- Removed SET() and GET() macros by the writel() and readl().
- Removed some noisy info messages.
V3: Fixed issues detected while running on 64 bits platforms
Rebased patches on top of v5.11-rc1 version
V4: Rework driver according to Greg Kroah-Hartman' feedback
- Add the ABI doc related to the sysfs implemented on this driver
V5: Rework driver accordingly to Leon Romanovsky' feedback
- Removed "default n" on Kconfig
Rework driver accordingly to Krzysztof Wilczyński' feedback
- Added some explanatory comments for some steps
- Added some bit defines instead of magic numbers
V6: Rework driver according to Greg Kroah-Hartman' feedback
- Squashed patches 2 and 3
- Removed units (MB/s) on the sys file
- Reduced mutex scope on the functions called by sysfs
Rework driver accordingly to Krzysztof Wilczyński' feedback
- Fix typo "DesignWare"
V7: Rework driver according to Greg Kroah-Hartman' feedback
- Created a sub device (misc device) that will be associated with the PCI driver
- sysfs group is now associated with the misc drivers instead of the PCI driver
V8: Rework driver according to Greg Kroah-Hartman' feedback
- Added more detail to the version changes on the cover letter
- Squashed patches 1 and 2
- Removed struct device from the dw_xdata_pcie structure
- Replaced the pci_*() use by dev_*()
- Added free call for the misc driver name allocation
- Added reference counting
- Removed snps_edda_data structure and their usage
Rebased patches on top of v5.12-rc4 version

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Derek Kiernan <[email protected]>
Cc: Dragan Cvetic <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Krzysztof Wilczyński <[email protected]>

Gustavo Pimentel (5):
misc: Add Synopsys DesignWare xData IP driver
Documentation: misc-devices: Add Documentation for dw-xdata-pcie
driver
MAINTAINERS: Add Synopsys xData IP driver maintainer
docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver
FIX driver

Documentation/ABI/testing/sysfs-driver-xdata | 46 +++
Documentation/misc-devices/dw-xdata-pcie.rst | 40 +++
MAINTAINERS | 7 +
drivers/misc/Kconfig | 10 +
drivers/misc/Makefile | 1 +
drivers/misc/dw-xdata-pcie.c | 416 +++++++++++++++++++++++++++
6 files changed, 520 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-xdata
create mode 100644 Documentation/misc-devices/dw-xdata-pcie.rst
create mode 100644 drivers/misc/dw-xdata-pcie.c

--
2.7.4


2021-03-29 09:53:26

by Gustavo Pimentel

[permalink] [raw]
Subject: [PATCH v8 3/5] MAINTAINERS: Add Synopsys xData IP driver maintainer

Add Synopsys xData IP driver maintainer.

This driver aims to support Synopsys xData IP and is normally distributed
along with Synopsys PCIe EndPoint IP as a PCIe traffic generator (depends
of the use and licensing agreement).

Signed-off-by: Gustavo Pimentel <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e87692..36387b3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5099,6 +5099,13 @@ S: Maintained
F: drivers/dma/dw-edma/
F: include/linux/dma/edma.h

+DESIGNWARE XDATA IP DRIVER
+M: Gustavo Pimentel <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/misc-devices/dw-xdata-pcie.rst
+F: drivers/misc/dw-xdata-pcie.c
+
DESIGNWARE USB2 DRD IP DRIVER
M: Minas Harutyunyan <[email protected]>
L: [email protected]
--
2.7.4

2021-03-29 09:53:28

by Gustavo Pimentel

[permalink] [raw]
Subject: [PATCH v8 1/5] misc: Add Synopsys DesignWare xData IP driver

Add Synopsys DesignWare xData IP driver. This driver enables/disables
the PCI traffic generator module pertain to the Synopsys DesignWare
prototype.

Signed-off-by: Gustavo Pimentel <[email protected]>
---
drivers/misc/Kconfig | 10 ++
drivers/misc/Makefile | 1 +
drivers/misc/dw-xdata-pcie.c | 401 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 412 insertions(+)
create mode 100644 drivers/misc/dw-xdata-pcie.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f532c59..e6af9ff 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -402,6 +402,16 @@ config SRAM
config SRAM_EXEC
bool

+config DW_XDATA_PCIE
+ depends on PCI
+ tristate "Synopsys DesignWare xData PCIe driver"
+ help
+ This driver allows controlling Synopsys DesignWare PCIe traffic
+ generator IP also known as xData, present in Synopsys DesignWare
+ PCIe Endpoint prototype.
+
+ If unsure, say N.
+
config PCI_ENDPOINT_TEST
depends on PCI
select CRC32
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 99b6f15..5411996 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
obj-$(CONFIG_GENWQE) += genwqe/
obj-$(CONFIG_ECHO) += echo/
obj-$(CONFIG_CXL_BASE) += cxl/
+obj-$(CONFIG_DW_XDATA_PCIE) += dw-xdata-pcie.o
obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
obj-$(CONFIG_OCXL) += ocxl/
obj-$(CONFIG_BCM_VK) += bcm-vk/
diff --git a/drivers/misc/dw-xdata-pcie.c b/drivers/misc/dw-xdata-pcie.c
new file mode 100644
index 00000000..43fdd35
--- /dev/null
+++ b/drivers/misc/dw-xdata-pcie.c
@@ -0,0 +1,401 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
+ * Synopsys DesignWare xData driver
+ *
+ * Author: Gustavo Pimentel <[email protected]>
+ */
+
+#include <linux/miscdevice.h>
+#include <linux/bitfield.h>
+#include <linux/pci-epf.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/bitops.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+
+#define DW_XDATA_DRIVER_NAME "dw-xdata-pcie"
+
+#define DW_XDATA_EP_MEM_OFFSET 0x8000000
+
+struct dw_xdata_pcie_data {
+ /* xData registers location */
+ enum pci_barno rg_bar;
+ off_t rg_off;
+ size_t rg_sz;
+};
+
+static const struct dw_xdata_pcie_data snps_edda_data = {
+ /* xData registers location */
+ .rg_bar = BAR_0,
+ .rg_off = 0x00000000, /* 0 Kbytes */
+ .rg_sz = 0x0000012c, /* 300 bytes */
+};
+
+#define STATUS_DONE BIT(0)
+
+#define CONTROL_DOORBELL BIT(0)
+#define CONTROL_IS_WRITE BIT(1)
+#define CONTROL_LENGTH(a) FIELD_PREP(GENMASK(13, 2), a)
+#define CONTROL_PATTERN_INC BIT(16)
+#define CONTROL_NO_ADDR_INC BIT(18)
+
+#define XPERF_CONTROL_ENABLE BIT(5)
+
+#define BURST_REPEAT BIT(31)
+#define BURST_VALUE 0x1001
+
+#define PATTERN_VALUE 0x0
+
+struct dw_xdata_regs {
+ u32 addr_lsb; /* 0x000 */
+ u32 addr_msb; /* 0x004 */
+ u32 burst_cnt; /* 0x008 */
+ u32 control; /* 0x00c */
+ u32 pattern; /* 0x010 */
+ u32 status; /* 0x014 */
+ u32 RAM_addr; /* 0x018 */
+ u32 RAM_port; /* 0x01c */
+ u32 _reserved0[14]; /* 0x020..0x054 */
+ u32 perf_control; /* 0x058 */
+ u32 _reserved1[41]; /* 0x05c..0x0fc */
+ u32 wr_cnt_lsb; /* 0x100 */
+ u32 wr_cnt_msb; /* 0x104 */
+ u32 rd_cnt_lsb; /* 0x108 */
+ u32 rd_cnt_msb; /* 0x10c */
+} __packed;
+
+struct dw_xdata_region {
+ phys_addr_t paddr; /* physical address */
+ void __iomem *vaddr; /* virtual address */
+ size_t sz; /* size */
+};
+
+struct dw_xdata {
+ struct dw_xdata_region rg_region; /* registers */
+ size_t max_wr_len; /* max wr xfer len */
+ size_t max_rd_len; /* max rd xfer len */
+ struct mutex mutex;
+ struct pci_dev *pdev;
+ struct device *dev;
+ struct miscdevice misc_dev;
+};
+
+static inline struct dw_xdata_regs __iomem *__dw_regs(struct dw_xdata *dw)
+{
+ return dw->rg_region.vaddr;
+}
+
+static void dw_xdata_stop(struct dw_xdata *dw)
+{
+ u32 burst;
+
+ mutex_lock(&dw->mutex);
+
+ burst = readl(&(__dw_regs(dw)->burst_cnt));
+
+ if (burst & BURST_REPEAT) {
+ burst &= ~(u32)BURST_REPEAT;
+ writel(burst, &(__dw_regs(dw)->burst_cnt));
+ }
+
+ mutex_unlock(&dw->mutex);
+}
+
+static void dw_xdata_start(struct dw_xdata *dw, bool write)
+{
+ u32 control, status;
+
+ /* Stop first if xfer in progress */
+ dw_xdata_stop(dw);
+
+ mutex_lock(&dw->mutex);
+
+ /* Clear status register */
+ writel(0x0, &(__dw_regs(dw)->status));
+
+ /* Burst count register set for continuous until stopped */
+ writel(BURST_REPEAT | BURST_VALUE, &(__dw_regs(dw)->burst_cnt));
+
+ /* Pattern register */
+ writel(PATTERN_VALUE, &(__dw_regs(dw)->pattern));
+
+ /* Control register */
+ control = CONTROL_DOORBELL | CONTROL_PATTERN_INC | CONTROL_NO_ADDR_INC;
+ if (write) {
+ control |= CONTROL_IS_WRITE;
+ control |= CONTROL_LENGTH(dw->max_wr_len);
+ } else {
+ control |= CONTROL_LENGTH(dw->max_rd_len);
+ }
+ writel(control, &(__dw_regs(dw)->control));
+
+ /*
+ * The xData HW block needs about 100 ms to initiate the traffic
+ * generation according this HW block datasheet.
+ */
+ usleep_range(100, 150);
+
+ status = readl(&(__dw_regs(dw)->status));
+
+ mutex_unlock(&dw->mutex);
+
+ if (!(status & STATUS_DONE))
+ pci_dbg(dw->pdev, "xData: started %s direction\n",
+ write ? "write" : "read");
+}
+
+static void dw_xdata_perf_meas(struct dw_xdata *dw, u64 *data, bool write)
+{
+ if (write) {
+ *data = readl(&(__dw_regs(dw)->wr_cnt_msb));
+ *data <<= 32;
+ *data |= readl(&(__dw_regs(dw)->wr_cnt_lsb));
+ } else {
+ *data = readl(&(__dw_regs(dw)->rd_cnt_msb));
+ *data <<= 32;
+ *data |= readl(&(__dw_regs(dw)->rd_cnt_lsb));
+ }
+}
+
+static u64 dw_xdata_perf_diff(u64 *m1, u64 *m2, u64 time)
+{
+ u64 rate = (*m1 - *m2);
+
+ rate *= (1000 * 1000 * 1000);
+ rate >>= 20;
+ rate = DIV_ROUND_CLOSEST_ULL(rate, time);
+
+ return rate;
+}
+
+static void dw_xdata_perf(struct dw_xdata *dw, u64 *rate, bool write)
+{
+ u64 data[2], time[2], diff;
+
+ mutex_lock(&dw->mutex);
+
+ /* First acquisition of current count frames */
+ writel(0x0, &(__dw_regs(dw)->perf_control));
+ dw_xdata_perf_meas(dw, &data[0], write);
+ time[0] = jiffies;
+ writel((u32)XPERF_CONTROL_ENABLE, &(__dw_regs(dw)->perf_control));
+
+ /*
+ * Wait 100ms between the 1st count frame acquisition and the 2nd
+ * count frame acquisition, in order to calculate the speed later
+ */
+ mdelay(100);
+
+ /* Second acquisition of current count frames */
+ writel(0x0, &(__dw_regs(dw)->perf_control));
+ dw_xdata_perf_meas(dw, &data[1], write);
+ time[1] = jiffies;
+ writel((u32)XPERF_CONTROL_ENABLE, &(__dw_regs(dw)->perf_control));
+
+ /*
+ * Speed calculation
+ *
+ * rate = (2nd count frames - 1st count frames) / (time elapsed)
+ */
+ diff = jiffies_to_nsecs(time[1] - time[0]);
+ *rate = dw_xdata_perf_diff(&data[1], &data[0], diff);
+
+ mutex_unlock(&dw->mutex);
+
+ pci_dbg(dw->pdev, "xData: time=%llu us, %s=%llu MB/s\n",
+ diff, write ? "write" : "read", *rate);
+}
+
+static struct dw_xdata *misc_dev_to_dw(struct miscdevice *misc_dev)
+{
+ return container_of(misc_dev, struct dw_xdata, misc_dev);
+}
+
+static ssize_t write_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct miscdevice *misc_dev = dev_get_drvdata(dev);
+ struct dw_xdata *dw = misc_dev_to_dw(misc_dev);
+ u64 rate;
+
+ dw_xdata_perf(dw, &rate, true);
+
+ return sysfs_emit(buf, "%llu\n", rate);
+}
+
+static ssize_t write_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct miscdevice *misc_dev = dev_get_drvdata(dev);
+ struct dw_xdata *dw = misc_dev_to_dw(misc_dev);
+
+ pci_dbg(dw->pdev, "xData: requested write transfer\n");
+
+ dw_xdata_start(dw, true);
+
+ return size;
+}
+
+static DEVICE_ATTR_RW(write);
+
+static ssize_t read_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct miscdevice *misc_dev = dev_get_drvdata(dev);
+ struct dw_xdata *dw = misc_dev_to_dw(misc_dev);
+ u64 rate;
+
+ dw_xdata_perf(dw, &rate, false);
+
+ return sysfs_emit(buf, "%llu\n", rate);
+}
+
+static ssize_t read_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct miscdevice *misc_dev = dev_get_drvdata(dev);
+ struct dw_xdata *dw = misc_dev_to_dw(misc_dev);
+
+ pci_dbg(dw->pdev, "xData: requested read transfer\n");
+
+ dw_xdata_start(dw, false);
+
+ return size;
+}
+
+static DEVICE_ATTR_RW(read);
+
+static ssize_t stop_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct miscdevice *misc_dev = dev_get_drvdata(dev);
+ struct dw_xdata *dw = misc_dev_to_dw(misc_dev);
+
+ pci_dbg(dw->pdev, "xData: requested stop any transfer\n");
+
+ dw_xdata_stop(dw);
+
+ return size;
+}
+
+static DEVICE_ATTR_WO(stop);
+
+static struct attribute *xdata_attrs[] = {
+ &dev_attr_write.attr,
+ &dev_attr_read.attr,
+ &dev_attr_stop.attr,
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(xdata);
+
+static int dw_xdata_pcie_probe(struct pci_dev *pdev,
+ const struct pci_device_id *pid)
+{
+ const struct dw_xdata_pcie_data *pdata = (void *)pid->driver_data;
+ struct dw_xdata *dw;
+ u64 addr;
+ int err;
+
+ /* Enable PCI device */
+ err = pcim_enable_device(pdev);
+ if (err) {
+ pci_err(pdev, "enabling device failed\n");
+ return err;
+ }
+
+ /* Mapping PCI BAR regions */
+ err = pcim_iomap_regions(pdev, BIT(pdata->rg_bar), pci_name(pdev));
+ if (err) {
+ pci_err(pdev, "xData BAR I/O remapping failed\n");
+ return err;
+ }
+
+ pci_set_master(pdev);
+
+ /* Allocate memory */
+ dw = devm_kzalloc(&pdev->dev, sizeof(*dw), GFP_KERNEL);
+ if (!dw)
+ return -ENOMEM;
+
+ /* Data structure initialization */
+ mutex_init(&dw->mutex);
+
+ dw->rg_region.vaddr = pcim_iomap_table(pdev)[pdata->rg_bar];
+ if (!dw->rg_region.vaddr)
+ return -ENOMEM;
+
+ dw->rg_region.vaddr += pdata->rg_off;
+ dw->rg_region.paddr = pdev->resource[pdata->rg_bar].start;
+ dw->rg_region.paddr += pdata->rg_off;
+ dw->rg_region.sz = pdata->rg_sz;
+
+ dw->max_wr_len = pcie_get_mps(pdev);
+ dw->max_wr_len >>= 2;
+
+ dw->max_rd_len = pcie_get_readrq(pdev);
+ dw->max_rd_len >>= 2;
+
+ dw->pdev = pdev;
+ dw->dev = &pdev->dev;
+
+ dw->misc_dev.minor = MISC_DYNAMIC_MINOR;
+ dw->misc_dev.name = kstrdup(DW_XDATA_DRIVER_NAME, GFP_KERNEL);
+ dw->misc_dev.parent = &pdev->dev;
+ dw->misc_dev.groups = xdata_groups;
+
+ writel(0x0, &(__dw_regs(dw)->RAM_addr));
+ writel(0x0, &(__dw_regs(dw)->RAM_port));
+
+ addr = dw->rg_region.paddr + DW_XDATA_EP_MEM_OFFSET;
+ writel(lower_32_bits(addr), &(__dw_regs(dw)->addr_lsb));
+ writel(upper_32_bits(addr), &(__dw_regs(dw)->addr_msb));
+ pci_dbg(pdev, "xData: target address = 0x%.16llx\n", addr);
+
+ pci_dbg(pdev, "xData: wr_len = %zu, rd_len = %zu\n",
+ dw->max_wr_len * 4, dw->max_rd_len * 4);
+
+ /* Saving data structure reference */
+ pci_set_drvdata(pdev, dw);
+
+ /* Register misc device */
+ err = misc_register(&dw->misc_dev);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static void dw_xdata_pcie_remove(struct pci_dev *pdev)
+{
+ struct dw_xdata *dw = pci_get_drvdata(pdev);
+
+ if (dw) {
+ dw_xdata_stop(dw);
+ misc_deregister(&dw->misc_dev);
+ }
+}
+
+static const struct pci_device_id dw_xdata_pcie_id_table[] = {
+ { PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
+ { }
+};
+MODULE_DEVICE_TABLE(pci, dw_xdata_pcie_id_table);
+
+static struct pci_driver dw_xdata_pcie_driver = {
+ .name = DW_XDATA_DRIVER_NAME,
+ .id_table = dw_xdata_pcie_id_table,
+ .probe = dw_xdata_pcie_probe,
+ .remove = dw_xdata_pcie_remove,
+};
+
+module_pci_driver(dw_xdata_pcie_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Synopsys DesignWare xData PCIe driver");
+MODULE_AUTHOR("Gustavo Pimentel <[email protected]>");
+
--
2.7.4

2021-03-29 09:53:36

by Gustavo Pimentel

[permalink] [raw]
Subject: [PATCH v8 2/5] Documentation: misc-devices: Add Documentation for dw-xdata-pcie driver

Add Documentation for dw-xdata-pcie driver.

Signed-off-by: Gustavo Pimentel <[email protected]>
---
Documentation/misc-devices/dw-xdata-pcie.rst | 40 ++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/misc-devices/dw-xdata-pcie.rst

diff --git a/Documentation/misc-devices/dw-xdata-pcie.rst b/Documentation/misc-devices/dw-xdata-pcie.rst
new file mode 100644
index 00000000..fd75c93
--- /dev/null
+++ b/Documentation/misc-devices/dw-xdata-pcie.rst
@@ -0,0 +1,40 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================================================================
+Driver for Synopsys DesignWare PCIe traffic generator (also known as xData)
+===========================================================================
+
+This driver should be used as a host-side (Root Complex) driver and Synopsys
+DesignWare prototype that includes this IP.
+
+The "dw-xdata-pcie" driver can be used to enable/disable PCIe traffic
+generator in either direction (mutual exclusion) besides allowing the
+PCIe link performance analysis.
+
+The interaction with this driver is done through the module parameter and
+can be changed in runtime. The driver outputs the requested command state
+information to /var/log/kern.log or dmesg.
+
+Request write TLPs traffic generation - Root Complex to Endpoint direction
+- Command:
+ echo 1 > /sys/class/misc/dw-xdata-pcie/write
+
+Get write TLPs traffic link throughput in MB/s
+- Command:
+ cat /sys/class/misc/dw-xdata-pcie/write
+- Output example:
+ 204
+
+Request read TLPs traffic generation - Endpoint to Root Complex direction:
+- Command:
+ echo 1 > /sys/class/misc/dw-xdata-pcie/read
+
+Get read TLPs traffic link throughput in MB/s
+- Command:
+ cat /sys/class/misc/dw-xdata-pcie/read
+- Output example:
+ 199
+
+Request to stop any current TLP transfer:
+- Command:
+ echo 1 > /sys/class/misc/dw-xdata-pcie/stop
--
2.7.4

2021-03-29 09:53:57

by Gustavo Pimentel

[permalink] [raw]
Subject: [PATCH v8 4/5] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver

This patch describes the sysfs interface implemented on the dw-xdata-pcie
driver.

Signed-off-by: Gustavo Pimentel <[email protected]>
---
Documentation/ABI/testing/sysfs-driver-xdata | 46 ++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-xdata

diff --git a/Documentation/ABI/testing/sysfs-driver-xdata b/Documentation/ABI/testing/sysfs-driver-xdata
new file mode 100644
index 00000000..66af19a
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-xdata
@@ -0,0 +1,46 @@
+What: /sys/class/misc/drivers/dw-xdata-pcie.<device>/write
+Date: April 2021
+KernelVersion: 5.13
+Contact: Gustavo Pimentel <[email protected]>
+Description: Allows the user to enable the PCIe traffic generator which
+ will create write TLPs frames - from the Root Complex to the
+ Endpoint direction.
+ Usage e.g.
+ echo 1 > /sys/class/misc/dw-xdata-pcie.<device>/write
+
+ The user can read the current PCIe link throughput generated
+ through this generator in MB/s.
+ Usage e.g.
+ cat /sys/class/misc/dw-xdata-pcie.<device>/write
+ 204
+
+ The file is read and write.
+
+What: /sys/class/misc/dw-xdata-pcie.<device>/read
+Date: April 2021
+KernelVersion: 5.13
+Contact: Gustavo Pimentel <[email protected]>
+Description: Allows the user to enable the PCIe traffic generator which
+ will create read TLPs frames - from the Endpoint to the Root
+ Complex direction.
+ Usage e.g.
+ echo 1 > /sys/class/misc/dw-xdata-pcie.<device>/read
+
+ The user can read the current PCIe link throughput generated
+ through this generator in MB/s.
+ Usage e.g.
+ cat /sys/class/misc/dw-xdata-pcie.<device>/read
+ 199
+
+ The file is read and write.
+
+What: /sys/class/misc/dw-xdata-pcie.<device>/stop
+Date: April 2021
+KernelVersion: 5.13
+Contact: Gustavo Pimentel <[email protected]>
+Description: Allows the user to disable the PCIe traffic generator in all
+ directions.
+ Usage e.g.
+ echo 1 > /sys/class/misc/dw-xdata-pcie.<device>/stop
+
+ The file is write only.
--
2.7.4

2021-03-29 09:53:57

by Gustavo Pimentel

[permalink] [raw]
Subject: [PATCH v8 5/5] FIX driver

Signed-off-by: Gustavo Pimentel <[email protected]>
---
drivers/misc/dw-xdata-pcie.c | 99 +++++++++++++++++++++++++-------------------
1 file changed, 57 insertions(+), 42 deletions(-)

diff --git a/drivers/misc/dw-xdata-pcie.c b/drivers/misc/dw-xdata-pcie.c
index 43fdd35..011516b 100644
--- a/drivers/misc/dw-xdata-pcie.c
+++ b/drivers/misc/dw-xdata-pcie.c
@@ -21,19 +21,7 @@

#define DW_XDATA_EP_MEM_OFFSET 0x8000000

-struct dw_xdata_pcie_data {
- /* xData registers location */
- enum pci_barno rg_bar;
- off_t rg_off;
- size_t rg_sz;
-};
-
-static const struct dw_xdata_pcie_data snps_edda_data = {
- /* xData registers location */
- .rg_bar = BAR_0,
- .rg_off = 0x00000000, /* 0 Kbytes */
- .rg_sz = 0x0000012c, /* 300 bytes */
-};
+static DEFINE_IDA(xdata_ida);

#define STATUS_DONE BIT(0)

@@ -71,7 +59,6 @@ struct dw_xdata_regs {
struct dw_xdata_region {
phys_addr_t paddr; /* physical address */
void __iomem *vaddr; /* virtual address */
- size_t sz; /* size */
};

struct dw_xdata {
@@ -80,7 +67,6 @@ struct dw_xdata {
size_t max_rd_len; /* max rd xfer len */
struct mutex mutex;
struct pci_dev *pdev;
- struct device *dev;
struct miscdevice misc_dev;
};

@@ -107,6 +93,7 @@ static void dw_xdata_stop(struct dw_xdata *dw)

static void dw_xdata_start(struct dw_xdata *dw, bool write)
{
+ struct device *dev = &dw->pdev->dev;
u32 control, status;

/* Stop first if xfer in progress */
@@ -144,7 +131,7 @@ static void dw_xdata_start(struct dw_xdata *dw, bool write)
mutex_unlock(&dw->mutex);

if (!(status & STATUS_DONE))
- pci_dbg(dw->pdev, "xData: started %s direction\n",
+ dev_dbg(dev, "xData: started %s direction\n",
write ? "write" : "read");
}

@@ -174,6 +161,7 @@ static u64 dw_xdata_perf_diff(u64 *m1, u64 *m2, u64 time)

static void dw_xdata_perf(struct dw_xdata *dw, u64 *rate, bool write)
{
+ struct device *dev = &dw->pdev->dev;
u64 data[2], time[2], diff;

mutex_lock(&dw->mutex);
@@ -206,7 +194,7 @@ static void dw_xdata_perf(struct dw_xdata *dw, u64 *rate, bool write)

mutex_unlock(&dw->mutex);

- pci_dbg(dw->pdev, "xData: time=%llu us, %s=%llu MB/s\n",
+ dev_dbg(dev, "xData: time=%llu us, %s=%llu MB/s\n",
diff, write ? "write" : "read", *rate);
}

@@ -233,7 +221,7 @@ static ssize_t write_store(struct device *dev, struct device_attribute *attr,
struct miscdevice *misc_dev = dev_get_drvdata(dev);
struct dw_xdata *dw = misc_dev_to_dw(misc_dev);

- pci_dbg(dw->pdev, "xData: requested write transfer\n");
+ dev_dbg(dev, "xData: requested write transfer\n");

dw_xdata_start(dw, true);

@@ -260,7 +248,7 @@ static ssize_t read_store(struct device *dev, struct device_attribute *attr,
struct miscdevice *misc_dev = dev_get_drvdata(dev);
struct dw_xdata *dw = misc_dev_to_dw(misc_dev);

- pci_dbg(dw->pdev, "xData: requested read transfer\n");
+ dev_dbg(dev, "xData: requested read transfer\n");

dw_xdata_start(dw, false);

@@ -275,7 +263,7 @@ static ssize_t stop_store(struct device *dev, struct device_attribute *attr,
struct miscdevice *misc_dev = dev_get_drvdata(dev);
struct dw_xdata *dw = misc_dev_to_dw(misc_dev);

- pci_dbg(dw->pdev, "xData: requested stop any transfer\n");
+ dev_dbg(dev, "xData: requested stop any transfer\n");

dw_xdata_stop(dw);

@@ -296,43 +284,42 @@ ATTRIBUTE_GROUPS(xdata);
static int dw_xdata_pcie_probe(struct pci_dev *pdev,
const struct pci_device_id *pid)
{
- const struct dw_xdata_pcie_data *pdata = (void *)pid->driver_data;
+ struct device *dev = &pdev->dev;
struct dw_xdata *dw;
+ char name[24];
u64 addr;
int err;
+ int id;

/* Enable PCI device */
err = pcim_enable_device(pdev);
if (err) {
- pci_err(pdev, "enabling device failed\n");
+ dev_err(dev, "enabling device failed\n");
return err;
}

/* Mapping PCI BAR regions */
- err = pcim_iomap_regions(pdev, BIT(pdata->rg_bar), pci_name(pdev));
+ err = pcim_iomap_regions(pdev, BIT(BAR_0), pci_name(pdev));
if (err) {
- pci_err(pdev, "xData BAR I/O remapping failed\n");
+ dev_err(dev, "xData BAR I/O remapping failed\n");
return err;
}

pci_set_master(pdev);

/* Allocate memory */
- dw = devm_kzalloc(&pdev->dev, sizeof(*dw), GFP_KERNEL);
+ dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
if (!dw)
return -ENOMEM;

/* Data structure initialization */
mutex_init(&dw->mutex);

- dw->rg_region.vaddr = pcim_iomap_table(pdev)[pdata->rg_bar];
+ dw->rg_region.vaddr = pcim_iomap_table(pdev)[BAR_0];
if (!dw->rg_region.vaddr)
return -ENOMEM;

- dw->rg_region.vaddr += pdata->rg_off;
- dw->rg_region.paddr = pdev->resource[pdata->rg_bar].start;
- dw->rg_region.paddr += pdata->rg_off;
- dw->rg_region.sz = pdata->rg_sz;
+ dw->rg_region.paddr = pdev->resource[BAR_0].start;

dw->max_wr_len = pcie_get_mps(pdev);
dw->max_wr_len >>= 2;
@@ -341,11 +328,22 @@ static int dw_xdata_pcie_probe(struct pci_dev *pdev,
dw->max_rd_len >>= 2;

dw->pdev = pdev;
- dw->dev = &pdev->dev;
+
+ id = ida_simple_get(&xdata_ida, 0, 0, GFP_KERNEL);
+ if (id < 0) {
+ dev_err(dev, "xData: unable to get id\n");
+ return id;
+ }
+
+ snprintf(name, sizeof(name), DW_XDATA_DRIVER_NAME ".%d", id);
+ dw->misc_dev.name = kstrdup(name, GFP_KERNEL);
+ if (!dw->misc_dev.name) {
+ err = -ENOMEM;
+ goto err_ida_remove;
+ }

dw->misc_dev.minor = MISC_DYNAMIC_MINOR;
- dw->misc_dev.name = kstrdup(DW_XDATA_DRIVER_NAME, GFP_KERNEL);
- dw->misc_dev.parent = &pdev->dev;
+ dw->misc_dev.parent = dev;
dw->misc_dev.groups = xdata_groups;

writel(0x0, &(__dw_regs(dw)->RAM_addr));
@@ -354,9 +352,9 @@ static int dw_xdata_pcie_probe(struct pci_dev *pdev,
addr = dw->rg_region.paddr + DW_XDATA_EP_MEM_OFFSET;
writel(lower_32_bits(addr), &(__dw_regs(dw)->addr_lsb));
writel(upper_32_bits(addr), &(__dw_regs(dw)->addr_msb));
- pci_dbg(pdev, "xData: target address = 0x%.16llx\n", addr);
+ dev_dbg(dev, "xData: target address = 0x%.16llx\n", addr);

- pci_dbg(pdev, "xData: wr_len = %zu, rd_len = %zu\n",
+ dev_dbg(dev, "xData: wr_len = %zu, rd_len = %zu\n",
dw->max_wr_len * 4, dw->max_rd_len * 4);

/* Saving data structure reference */
@@ -364,24 +362,41 @@ static int dw_xdata_pcie_probe(struct pci_dev *pdev,

/* Register misc device */
err = misc_register(&dw->misc_dev);
- if (err)
- return err;
+ if (err) {
+ dev_err(dev, "xData: failed to register device\n");
+ goto err_kfree_name;
+ }

return 0;
+
+err_kfree_name:
+ kfree(dw->misc_dev.name);
+
+err_ida_remove:
+ ida_simple_remove(&xdata_ida, id);
+
+ return err;
}

static void dw_xdata_pcie_remove(struct pci_dev *pdev)
{
struct dw_xdata *dw = pci_get_drvdata(pdev);
+ int id;

- if (dw) {
- dw_xdata_stop(dw);
- misc_deregister(&dw->misc_dev);
- }
+ if (sscanf(dw->misc_dev.name, DW_XDATA_DRIVER_NAME ".%d", &id) != 1)
+ return;
+
+ if (id < 0)
+ return;
+
+ dw_xdata_stop(dw);
+ misc_deregister(&dw->misc_dev);
+ kfree(dw->misc_dev.name);
+ ida_simple_remove(&xdata_ida, id);
}

static const struct pci_device_id dw_xdata_pcie_id_table[] = {
- { PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
+ { PCI_DEVICE_DATA(SYNOPSYS, EDDA, NULL) },
{ }
};
MODULE_DEVICE_TABLE(pci, dw_xdata_pcie_id_table);
--
2.7.4

2021-03-29 10:05:18

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v8 5/5] FIX driver

On Mon, Mar 29, 2021 at 11:51:38AM +0200, Gustavo Pimentel wrote:
> Signed-off-by: Gustavo Pimentel <[email protected]>

Please make an effort, this is in no way an acceptable commit description
for a patch. The subject is already extremely vague "FIX driver" with no
context at all, and there is no description of the intent here. What is
someone supposed to think about the risk of keeping or reverting it if a
bisect section would end on this one ?

Thanks,
Willy

2021-03-29 10:08:11

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v8 5/5] FIX driver

On Mon, Mar 29, 2021 at 11:51:38AM +0200, Gustavo Pimentel wrote:
> Signed-off-by: Gustavo Pimentel <[email protected]>
> ---
> drivers/misc/dw-xdata-pcie.c | 99 +++++++++++++++++++++++++-------------------
> 1 file changed, 57 insertions(+), 42 deletions(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch did many different things all at once, making it difficult
to review. All Linux kernel patches need to only do one thing at a
time. If you need to do multiple things (such as clean up all coding
style issues in a file/driver), do it in a sequence of patches, each
one doing only one thing. This will make it easier to review the
patches to ensure that they are correct, and to help alleviate any
merge issues that larger patches can cause.

- You did not specify a description of why the patch is needed, or
possibly, any description at all, in the email body. Please read the
section entitled "The canonical patch format" in the kernel file,
Documentation/SubmittingPatches for what is needed in order to
properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
and everyone else, to know what this patch is all about. Please read
the section entitled "The canonical patch format" in the kernel file,
Documentation/SubmittingPatches for what a proper Subject: line should
look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2021-03-29 10:13:23

by Gustavo Pimentel

[permalink] [raw]
Subject: RE: [PATCH v8 5/5] FIX driver

On Mon, Mar 29, 2021 at 11:3:3, Willy Tarreau <[email protected]> wrote:

> On Mon, Mar 29, 2021 at 11:51:38AM +0200, Gustavo Pimentel wrote:
> > Signed-off-by: Gustavo Pimentel <[email protected]>
>
> Please make an effort, this is in no way an acceptable commit description
> for a patch. The subject is already extremely vague "FIX driver" with no
> context at all, and there is no description of the intent here. What is
> someone supposed to think about the risk of keeping or reverting it if a
> bisect section would end on this one ?

Hi Willy,

this patch was sent by mistake, it was already squashed on patch #1 on
v9.
Please check your inbox for the patch series v9, that was sent a few
minutes ago.

-Gustavo

>
> Thanks,
> Willy