* [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver @ 2021-02-11 9:08 Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 1/6] misc: " Gustavo Pimentel ` (5 more replies) 0 siblings, 6 replies; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:08 UTC (permalink / raw) To: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas Cc: Gustavo Pimentel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=n, Size: 2203 bytes --] 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 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's feedback Rework driver accordingly to Krzysztof Wilczyński's feedback Cc: Derek Kiernan <derek.kiernan@xilinx.com> Cc: Dragan Cvetic <dragan.cvetic@xilinx.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jonathan Corbet <corbet@lwn.net> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: linux-pci@vger.kernel.org Cc: linux-doc@vger.kernel.org Cc: linux-kernel@vger.kernel.org Gustavo Pimentel (6): misc: Add Synopsys DesignWare xData IP driver misc: Add Synopsys DesignWare xData IP driver to Makefile misc: Add Synopsys DesignWare xData IP driver to Kconfig 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 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 | 394 +++++++++++++++++++++++++++ 6 files changed, 498 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 9:08 [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver Gustavo Pimentel @ 2021-02-11 9:08 ` Gustavo Pimentel 2021-02-11 9:30 ` Greg Kroah-Hartman 2021-02-11 11:42 ` Leon Romanovsky 2021-02-11 9:08 ` [PATCH v5 2/6] misc: Add Synopsys DesignWare xData IP driver to Makefile Gustavo Pimentel ` (4 subsequent siblings) 5 siblings, 2 replies; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:08 UTC (permalink / raw) To: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas, Gustavo Pimentel 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 <gustavo.pimentel@synopsys.com> --- drivers/misc/dw-xdata-pcie.c | 394 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 394 insertions(+) create mode 100644 drivers/misc/dw-xdata-pcie.c diff --git a/drivers/misc/dw-xdata-pcie.c b/drivers/misc/dw-xdata-pcie.c new file mode 100644 index 00000000..2e023ba --- /dev/null +++ b/drivers/misc/dw-xdata-pcie.c @@ -0,0 +1,394 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates. + * Synopsys DesignWare xData driver + * + * Author: Gustavo Pimentel <gustavo.pimentel@synopsys.com> + */ + +#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; +}; + +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 = readl(&(__dw_regs(dw)->burst_cnt)); + + if (burst & BURST_REPEAT) { + burst &= ~(u32)BURST_REPEAT; + writel(burst, &(__dw_regs(dw)->burst_cnt)); + } +} + +static void dw_xdata_start(struct dw_xdata *dw, bool write) +{ + u32 control, status; + + /* Stop first if xfer in progress */ + dw_xdata_stop(dw); + + /* 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)); + 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; + + /* 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); + + pci_dbg(dw->pdev, "xData: time=%llu us, %s=%llu MB/s\n", + diff, write ? "write" : "read", *rate); +} + +static ssize_t write_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct dw_xdata *dw = pci_get_drvdata(pdev); + u64 rate; + + mutex_lock(&dw->mutex); + dw_xdata_perf(dw, &rate, true); + mutex_unlock(&dw->mutex); + + return sysfs_emit(buf, "%llu MB/s\n", rate); +} + +static ssize_t write_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t size) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct dw_xdata *dw = pci_get_drvdata(pdev); + + pci_dbg(pdev, "xData: requested write transfer\n"); + + mutex_lock(&dw->mutex); + dw_xdata_start(dw, true); + mutex_unlock(&dw->mutex); + + return size; +} + +static DEVICE_ATTR_RW(write); + +static ssize_t read_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct dw_xdata *dw = pci_get_drvdata(pdev); + u64 rate; + + mutex_lock(&dw->mutex); + dw_xdata_perf(dw, &rate, false); + mutex_unlock(&dw->mutex); + + return sysfs_emit(buf, "%llu MB/s\n", rate); +} + +static ssize_t read_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t size) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct dw_xdata *dw = pci_get_drvdata(pdev); + + pci_dbg(pdev, "xData: requested read transfer\n"); + + mutex_lock(&dw->mutex); + dw_xdata_start(dw, false); + mutex_unlock(&dw->mutex); + + 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 pci_dev *pdev = to_pci_dev(dev); + struct dw_xdata *dw = pci_get_drvdata(pdev); + + pci_dbg(pdev, "xData: requested stop any transfer\n"); + + mutex_lock(&dw->mutex); + dw_xdata_stop(dw); + mutex_unlock(&dw->mutex); + + return size; +} + +static DEVICE_ATTR_WO(stop); + +static struct attribute *default_attrs[] = { + &dev_attr_write.attr, + &dev_attr_read.attr, + &dev_attr_stop.attr, + NULL, +}; + +static const struct attribute_group xdata_attr_group = { + .attrs = default_attrs, + .name = DW_XDATA_DRIVER_NAME, +}; + +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; + + 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); + + /* Sysfs */ + err = sysfs_create_group(&pdev->dev.kobj, &xdata_attr_group); + if (err) + return err; + + err = sysfs_create_link(kernel_kobj, &pdev->dev.kobj, + DW_XDATA_DRIVER_NAME); + 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) { + mutex_lock(&dw->mutex); + dw_xdata_stop(dw); + mutex_unlock(&dw->mutex); + } + + sysfs_remove_link(kernel_kobj, DW_XDATA_DRIVER_NAME); + sysfs_remove_group(&pdev->dev.kobj, &xdata_attr_group); +} + +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 <gustavo.pimentel@synopsys.com>"); + -- 2.7.4 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 9:08 ` [PATCH v5 1/6] misc: " Gustavo Pimentel @ 2021-02-11 9:30 ` Greg Kroah-Hartman 2021-02-11 9:50 ` Gustavo Pimentel 2021-02-11 11:42 ` Leon Romanovsky 1 sibling, 1 reply; 24+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 9:30 UTC (permalink / raw) To: Gustavo Pimentel Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > +static ssize_t write_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + struct dw_xdata *dw = pci_get_drvdata(pdev); > + u64 rate; > + > + mutex_lock(&dw->mutex); > + dw_xdata_perf(dw, &rate, true); > + mutex_unlock(&dw->mutex); > + > + return sysfs_emit(buf, "%llu MB/s\n", rate); Do not put units in a sysfs file, that should be in the documentation, otherwise this forces userspace to "parse" the units which is a mess. Same for the other sysfs file. And why do you need a lock for this show function? thanks, greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 9:30 ` Greg Kroah-Hartman @ 2021-02-11 9:50 ` Gustavo Pimentel 2021-02-11 9:59 ` Greg Kroah-Hartman 0 siblings, 1 reply; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:50 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 9:30:16, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > +static ssize_t write_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct pci_dev *pdev = to_pci_dev(dev); > > + struct dw_xdata *dw = pci_get_drvdata(pdev); > > + u64 rate; > > + > > + mutex_lock(&dw->mutex); > > + dw_xdata_perf(dw, &rate, true); > > + mutex_unlock(&dw->mutex); > > + > > + return sysfs_emit(buf, "%llu MB/s\n", rate); > > Do not put units in a sysfs file, that should be in the documentation, > otherwise this forces userspace to "parse" the units which is a mess. Okay. > > Same for the other sysfs file. > > And why do you need a lock for this show function? Maybe I understood it wrongly, please correct me in that case. The dw_xdata_perf() is called on the write_show() and read_show(), to avoid a possible race condition between those calls, I have added this mutex. Thanks. -Gustavo > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 9:50 ` Gustavo Pimentel @ 2021-02-11 9:59 ` Greg Kroah-Hartman 2021-02-11 10:21 ` Gustavo Pimentel 0 siblings, 1 reply; 24+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 9:59 UTC (permalink / raw) To: Gustavo Pimentel Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 09:50:33AM +0000, Gustavo Pimentel wrote: > On Thu, Feb 11, 2021 at 9:30:16, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > > +static ssize_t write_show(struct device *dev, struct device_attribute *attr, > > > + char *buf) > > > +{ > > > + struct pci_dev *pdev = to_pci_dev(dev); > > > + struct dw_xdata *dw = pci_get_drvdata(pdev); > > > + u64 rate; > > > + > > > + mutex_lock(&dw->mutex); > > > + dw_xdata_perf(dw, &rate, true); > > > + mutex_unlock(&dw->mutex); > > > + > > > + return sysfs_emit(buf, "%llu MB/s\n", rate); > > > > Do not put units in a sysfs file, that should be in the documentation, > > otherwise this forces userspace to "parse" the units which is a mess. > > Okay. > > > > > Same for the other sysfs file. > > > > And why do you need a lock for this show function? > > Maybe I understood it wrongly, please correct me in that case. The > dw_xdata_perf() is called on the write_show() and read_show(), to avoid a > possible race condition between those calls, I have added this mutex. What race? If the value changes with a write right after a read, what does it matter? What exactly are you trying to protect with this lock? thanks, greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 9:59 ` Greg Kroah-Hartman @ 2021-02-11 10:21 ` Gustavo Pimentel 2021-02-11 10:33 ` Greg Kroah-Hartman 0 siblings, 1 reply; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 10:21 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 9:59:26, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Feb 11, 2021 at 09:50:33AM +0000, Gustavo Pimentel wrote: > > On Thu, Feb 11, 2021 at 9:30:16, Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > > > +static ssize_t write_show(struct device *dev, struct device_attribute *attr, > > > > + char *buf) > > > > +{ > > > > + struct pci_dev *pdev = to_pci_dev(dev); > > > > + struct dw_xdata *dw = pci_get_drvdata(pdev); > > > > + u64 rate; > > > > + > > > > + mutex_lock(&dw->mutex); > > > > + dw_xdata_perf(dw, &rate, true); > > > > + mutex_unlock(&dw->mutex); > > > > + > > > > + return sysfs_emit(buf, "%llu MB/s\n", rate); > > > > > > Do not put units in a sysfs file, that should be in the documentation, > > > otherwise this forces userspace to "parse" the units which is a mess. > > > > Okay. > > > > > > > > Same for the other sysfs file. > > > > > > And why do you need a lock for this show function? > > > > Maybe I understood it wrongly, please correct me in that case. The > > dw_xdata_perf() is called on the write_show() and read_show(), to avoid a > > possible race condition between those calls, I have added this mutex. > > What race? If the value changes with a write right after a read, what > does it matter? > > What exactly are you trying to protect with this lock? The write_store() does a procedure to enable the traffic on the write direction, however, the write_show() does a different procedure to calculate the link throughput speed, which uses a different set of registers on the HW. Similar happens on the read_store() (which enable the traffic on the read direction) and on the read_show() To summarize write_store() follows the same approach of read_store() and the write_show() of the read_show(). I added the mutex on those functions for instance to avoid while during the write_show() call the possibility of been called the read_show() messing up the link throughput speed calculation. Or while during the write_store() call to be called the read_store or even the write_show() for the same reasons. This is the reason why I added those mutexes, maybe this isn't necessary and it's overkill. Please advise me if a different approach can be done. -Gustavo > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 10:21 ` Gustavo Pimentel @ 2021-02-11 10:33 ` Greg Kroah-Hartman 2021-02-11 10:51 ` Gustavo Pimentel 0 siblings, 1 reply; 24+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 10:33 UTC (permalink / raw) To: Gustavo Pimentel Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 10:21:07AM +0000, Gustavo Pimentel wrote: > On Thu, Feb 11, 2021 at 9:59:26, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > On Thu, Feb 11, 2021 at 09:50:33AM +0000, Gustavo Pimentel wrote: > > > On Thu, Feb 11, 2021 at 9:30:16, Greg Kroah-Hartman > > > <gregkh@linuxfoundation.org> wrote: > > > > > > > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > > > > +static ssize_t write_show(struct device *dev, struct device_attribute *attr, > > > > > + char *buf) > > > > > +{ > > > > > + struct pci_dev *pdev = to_pci_dev(dev); > > > > > + struct dw_xdata *dw = pci_get_drvdata(pdev); > > > > > + u64 rate; > > > > > + > > > > > + mutex_lock(&dw->mutex); > > > > > + dw_xdata_perf(dw, &rate, true); > > > > > + mutex_unlock(&dw->mutex); > > > > > + > > > > > + return sysfs_emit(buf, "%llu MB/s\n", rate); > > > > > > > > Do not put units in a sysfs file, that should be in the documentation, > > > > otherwise this forces userspace to "parse" the units which is a mess. > > > > > > Okay. > > > > > > > > > > > Same for the other sysfs file. > > > > > > > > And why do you need a lock for this show function? > > > > > > Maybe I understood it wrongly, please correct me in that case. The > > > dw_xdata_perf() is called on the write_show() and read_show(), to avoid a > > > possible race condition between those calls, I have added this mutex. > > > > What race? If the value changes with a write right after a read, what > > does it matter? > > > > What exactly are you trying to protect with this lock? > > The write_store() does a procedure to enable the traffic on the write > direction, however, the write_show() does a different procedure to > calculate the link throughput speed, which uses a different set of > registers on the HW. > > Similar happens on the read_store() (which enable the traffic on the read > direction) and on the read_show() > > To summarize write_store() follows the same approach of read_store() and > the write_show() of the read_show(). I added the mutex on those functions > for instance to avoid while during the write_show() call the possibility > of been called the read_show() messing up the link throughput speed > calculation. > Or while during the write_store() call to be called the read_store or > even the write_show() for the same reasons. If you need to protect these types of things, but the lock down in the function that does this, not above it which forces people to audit everything and manually try to determine what lock is doing what for what. Make it impossible to get wrong, as it is, you have to do extra work here to keep things working properly, always a bad idea in an api. thanks, greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 10:33 ` Greg Kroah-Hartman @ 2021-02-11 10:51 ` Gustavo Pimentel 0 siblings, 0 replies; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 10:51 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 10:33:25, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Feb 11, 2021 at 10:21:07AM +0000, Gustavo Pimentel wrote: > > On Thu, Feb 11, 2021 at 9:59:26, Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > On Thu, Feb 11, 2021 at 09:50:33AM +0000, Gustavo Pimentel wrote: > > > > On Thu, Feb 11, 2021 at 9:30:16, Greg Kroah-Hartman > > > > <gregkh@linuxfoundation.org> wrote: > > > > > > > > > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > > > > > +static ssize_t write_show(struct device *dev, struct device_attribute *attr, > > > > > > + char *buf) > > > > > > +{ > > > > > > + struct pci_dev *pdev = to_pci_dev(dev); > > > > > > + struct dw_xdata *dw = pci_get_drvdata(pdev); > > > > > > + u64 rate; > > > > > > + > > > > > > + mutex_lock(&dw->mutex); > > > > > > + dw_xdata_perf(dw, &rate, true); > > > > > > + mutex_unlock(&dw->mutex); > > > > > > + > > > > > > + return sysfs_emit(buf, "%llu MB/s\n", rate); > > > > > > > > > > Do not put units in a sysfs file, that should be in the documentation, > > > > > otherwise this forces userspace to "parse" the units which is a mess. > > > > > > > > Okay. > > > > > > > > > > > > > > Same for the other sysfs file. > > > > > > > > > > And why do you need a lock for this show function? > > > > > > > > Maybe I understood it wrongly, please correct me in that case. The > > > > dw_xdata_perf() is called on the write_show() and read_show(), to avoid a > > > > possible race condition between those calls, I have added this mutex. > > > > > > What race? If the value changes with a write right after a read, what > > > does it matter? > > > > > > What exactly are you trying to protect with this lock? > > > > The write_store() does a procedure to enable the traffic on the write > > direction, however, the write_show() does a different procedure to > > calculate the link throughput speed, which uses a different set of > > registers on the HW. > > > > Similar happens on the read_store() (which enable the traffic on the read > > direction) and on the read_show() > > > > To summarize write_store() follows the same approach of read_store() and > > the write_show() of the read_show(). I added the mutex on those functions > > for instance to avoid while during the write_show() call the possibility > > of been called the read_show() messing up the link throughput speed > > calculation. > > Or while during the write_store() call to be called the read_store or > > even the write_show() for the same reasons. > > If you need to protect these types of things, but the lock down in the > function that does this, not above it which forces people to audit > everything and manually try to determine what lock is doing what for > what. > > Make it impossible to get wrong, as it is, you have to do extra work > here to keep things working properly, always a bad idea in an api. I think I understood what you mean, I will *reduce* the mutex scope to the basic functions that are called by the sysfs *_store() and *_show(). -Gustavo > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 9:08 ` [PATCH v5 1/6] misc: " Gustavo Pimentel 2021-02-11 9:30 ` Greg Kroah-Hartman @ 2021-02-11 11:42 ` Leon Romanovsky 2021-02-11 12:32 ` Greg Kroah-Hartman 1 sibling, 1 reply; 24+ messages in thread From: Leon Romanovsky @ 2021-02-11 11:42 UTC (permalink / raw) To: Gustavo Pimentel Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > 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 <gustavo.pimentel@synopsys.com> > --- > drivers/misc/dw-xdata-pcie.c | 394 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 394 insertions(+) > create mode 100644 drivers/misc/dw-xdata-pcie.c <...> > +MODULE_LICENSE("GPL v2"); "GPL" and not "GPL v2". Thanks > +MODULE_DESCRIPTION("Synopsys DesignWare xData PCIe driver"); > +MODULE_AUTHOR("Gustavo Pimentel <gustavo.pimentel@synopsys.com>"); > + > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 11:42 ` Leon Romanovsky @ 2021-02-11 12:32 ` Greg Kroah-Hartman 2021-02-11 13:50 ` Leon Romanovsky 0 siblings, 1 reply; 24+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 12:32 UTC (permalink / raw) To: Leon Romanovsky Cc: Gustavo Pimentel, linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 01:42:43PM +0200, Leon Romanovsky wrote: > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > 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 <gustavo.pimentel@synopsys.com> > > --- > > drivers/misc/dw-xdata-pcie.c | 394 +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 394 insertions(+) > > create mode 100644 drivers/misc/dw-xdata-pcie.c > > <...> > > > +MODULE_LICENSE("GPL v2"); > > "GPL" and not "GPL v2". There is no difference, please go read module.h. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 12:32 ` Greg Kroah-Hartman @ 2021-02-11 13:50 ` Leon Romanovsky 2021-02-11 14:02 ` Greg Kroah-Hartman 0 siblings, 1 reply; 24+ messages in thread From: Leon Romanovsky @ 2021-02-11 13:50 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Gustavo Pimentel, linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 01:32:13PM +0100, Greg Kroah-Hartman wrote: > On Thu, Feb 11, 2021 at 01:42:43PM +0200, Leon Romanovsky wrote: > > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > > 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 <gustavo.pimentel@synopsys.com> > > > --- > > > drivers/misc/dw-xdata-pcie.c | 394 +++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 394 insertions(+) > > > create mode 100644 drivers/misc/dw-xdata-pcie.c > > > > <...> > > > > > +MODULE_LICENSE("GPL v2"); > > > > "GPL" and not "GPL v2". > > There is no difference, please go read module.h. I read and this is why I said it. Documentation/process/license-rules.rst: "It exists for historic reasons." Historic, for me, means that new code is better do not use this. Thanks ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 13:50 ` Leon Romanovsky @ 2021-02-11 14:02 ` Greg Kroah-Hartman 2021-02-11 14:13 ` Leon Romanovsky 0 siblings, 1 reply; 24+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 14:02 UTC (permalink / raw) To: Leon Romanovsky Cc: Gustavo Pimentel, linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 03:50:19PM +0200, Leon Romanovsky wrote: > On Thu, Feb 11, 2021 at 01:32:13PM +0100, Greg Kroah-Hartman wrote: > > On Thu, Feb 11, 2021 at 01:42:43PM +0200, Leon Romanovsky wrote: > > > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > > > 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 <gustavo.pimentel@synopsys.com> > > > > --- > > > > drivers/misc/dw-xdata-pcie.c | 394 +++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 394 insertions(+) > > > > create mode 100644 drivers/misc/dw-xdata-pcie.c > > > > > > <...> > > > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > "GPL" and not "GPL v2". > > > > There is no difference, please go read module.h. > > I read and this is why I said it. > Documentation/process/license-rules.rst: "It exists for historic reasons." > > Historic, for me, means that new code is better do not use this. Nope, either is fine for new code, author gets to pick what they want. Personally, I like the explicitness of "GPL v2" for a variety of reasons. thanks, greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/6] misc: Add Synopsys DesignWare xData IP driver 2021-02-11 14:02 ` Greg Kroah-Hartman @ 2021-02-11 14:13 ` Leon Romanovsky 0 siblings, 0 replies; 24+ messages in thread From: Leon Romanovsky @ 2021-02-11 14:13 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Gustavo Pimentel, linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 03:02:03PM +0100, Greg Kroah-Hartman wrote: > On Thu, Feb 11, 2021 at 03:50:19PM +0200, Leon Romanovsky wrote: > > On Thu, Feb 11, 2021 at 01:32:13PM +0100, Greg Kroah-Hartman wrote: > > > On Thu, Feb 11, 2021 at 01:42:43PM +0200, Leon Romanovsky wrote: > > > > On Thu, Feb 11, 2021 at 10:08:38AM +0100, Gustavo Pimentel wrote: > > > > > 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 <gustavo.pimentel@synopsys.com> > > > > > --- > > > > > drivers/misc/dw-xdata-pcie.c | 394 +++++++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 394 insertions(+) > > > > > create mode 100644 drivers/misc/dw-xdata-pcie.c > > > > > > > > <...> > > > > > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > > > "GPL" and not "GPL v2". > > > > > > There is no difference, please go read module.h. > > > > I read and this is why I said it. > > Documentation/process/license-rules.rst: "It exists for historic reasons." > > > > Historic, for me, means that new code is better do not use this. > > Nope, either is fine for new code, author gets to pick what they want. > Personally, I like the explicitness of "GPL v2" for a variety of > reasons. Feel free to update the documentation. Personally, I don't like two names for the same license. This is one of the reasons why SPFX clearly marks the code license, It is why we have "SPDX-License-Identifier: GPL-2.0" and not "SPDX-License-Identifier: GPL". https://spdx.org/licenses/preview/ GNU General Public License v2.0 only GPL-2.0-only GNU General Public License v2.0 or later GPL-2.0-or-later Thanks > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 2/6] misc: Add Synopsys DesignWare xData IP driver to Makefile 2021-02-11 9:08 [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 1/6] misc: " Gustavo Pimentel @ 2021-02-11 9:08 ` Gustavo Pimentel 2021-02-11 9:28 ` Greg Kroah-Hartman 2021-02-11 9:08 ` [PATCH v5 3/6] misc: Add Synopsys DesignWare xData IP driver to Kconfig Gustavo Pimentel ` (3 subsequent siblings) 5 siblings, 1 reply; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:08 UTC (permalink / raw) To: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas Cc: Gustavo Pimentel Add Synopsys DesignWare xData IP driver to Makefile. This driver enables/disables the PCIe traffic generator module pertain to the Synopsys DesignWare prototype. Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> --- drivers/misc/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index d23231e..bf22021 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -49,6 +49,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-y += cardreader/ -- 2.7.4 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5 2/6] misc: Add Synopsys DesignWare xData IP driver to Makefile 2021-02-11 9:08 ` [PATCH v5 2/6] misc: Add Synopsys DesignWare xData IP driver to Makefile Gustavo Pimentel @ 2021-02-11 9:28 ` Greg Kroah-Hartman 2021-02-11 9:40 ` Gustavo Pimentel 0 siblings, 1 reply; 24+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 9:28 UTC (permalink / raw) To: Gustavo Pimentel Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 10:08:39AM +0100, Gustavo Pimentel wrote: > Add Synopsys DesignWare xData IP driver to Makefile. > > This driver enables/disables the PCIe traffic generator module > pertain to the Synopsys DesignWare prototype. > > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > --- > drivers/misc/Makefile | 1 + > 1 file changed, 1 insertion(+) I said patch 2 and 3 should be merged into 1 before, right? Why ignore that suggestion? Otherwise build errors/issues will get attributed to the wrong commit... greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v5 2/6] misc: Add Synopsys DesignWare xData IP driver to Makefile 2021-02-11 9:28 ` Greg Kroah-Hartman @ 2021-02-11 9:40 ` Gustavo Pimentel 0 siblings, 0 replies; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:40 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 9:28:9, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Feb 11, 2021 at 10:08:39AM +0100, Gustavo Pimentel wrote: > > Add Synopsys DesignWare xData IP driver to Makefile. > > > > This driver enables/disables the PCIe traffic generator module > > pertain to the Synopsys DesignWare prototype. > > > > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > --- > > drivers/misc/Makefile | 1 + > > 1 file changed, 1 insertion(+) > > I said patch 2 and 3 should be merged into 1 before, right? Why ignore > that suggestion? Otherwise build errors/issues will get attributed to > the wrong commit... I didn't saw that suggestion, sorry. I can do that, no problem. > > greg k-h ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 3/6] misc: Add Synopsys DesignWare xData IP driver to Kconfig 2021-02-11 9:08 [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 1/6] misc: " Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 2/6] misc: Add Synopsys DesignWare xData IP driver to Makefile Gustavo Pimentel @ 2021-02-11 9:08 ` Gustavo Pimentel 2021-02-11 11:29 ` Krzysztof Wilczyński 2021-02-11 9:08 ` [PATCH v5 4/6] Documentation: misc-devices: Add Documentation for dw-xdata-pcie driver Gustavo Pimentel ` (2 subsequent siblings) 5 siblings, 1 reply; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:08 UTC (permalink / raw) To: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas Cc: Gustavo Pimentel Add Synopsys DesignWare xData IP driver to Kconfig. This driver enables/disables the PCIe traffic generator module pertain to the Synopsys DesignWare prototype. Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> --- drivers/misc/Kconfig | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index fafa8b0..41684fe 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -423,6 +423,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 -- 2.7.4 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5 3/6] misc: Add Synopsys DesignWare xData IP driver to Kconfig 2021-02-11 9:08 ` [PATCH v5 3/6] misc: Add Synopsys DesignWare xData IP driver to Kconfig Gustavo Pimentel @ 2021-02-11 11:29 ` Krzysztof Wilczyński 2021-02-11 13:33 ` Gustavo Pimentel 0 siblings, 1 reply; 24+ messages in thread From: Krzysztof Wilczyński @ 2021-02-11 11:29 UTC (permalink / raw) To: Gustavo Pimentel Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas Hi Gustavo, [...] > +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. [...] To be consistent. It would be "DesignWare" in the sentence above. Krzysztof ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v5 3/6] misc: Add Synopsys DesignWare xData IP driver to Kconfig 2021-02-11 11:29 ` Krzysztof Wilczyński @ 2021-02-11 13:33 ` Gustavo Pimentel 0 siblings, 0 replies; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 13:33 UTC (permalink / raw) To: Krzysztof Wilczyński Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 11:29:43, Krzysztof Wilczyński <kw@linux.com> wrote: > Hi Gustavo, > > [...] > > +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. > [...] > > To be consistent. It would be "DesignWare" in the sentence above. Nicely catch. Thanks. > > Krzysztof ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 4/6] Documentation: misc-devices: Add Documentation for dw-xdata-pcie driver 2021-02-11 9:08 [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver Gustavo Pimentel ` (2 preceding siblings ...) 2021-02-11 9:08 ` [PATCH v5 3/6] misc: Add Synopsys DesignWare xData IP driver to Kconfig Gustavo Pimentel @ 2021-02-11 9:08 ` Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 5/6] MAINTAINERS: Add Synopsys xData IP driver maintainer Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 6/6] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver Gustavo Pimentel 5 siblings, 0 replies; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:08 UTC (permalink / raw) To: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas, Gustavo Pimentel Add Documentation for dw-xdata-pcie driver. Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> --- 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..3af9fad --- /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/kernel/dw-xdata-pcie/write + +Get write TLPs traffic link throughput +- Command: + cat /sys/kernel/dw-xdata-pcie/write +- Output example: + 204 MB/s + +Request read TLPs traffic generation - Endpoint to Root Complex direction: +- Command: + echo 1 > /sys/kernel/dw-xdata-pcie/read + +Get read TLPs traffic link throughput +- Command: + cat /sys/kernel/dw-xdata-pcie/read +- Output example: + 199 MB/s + +Request to stop any current TLP transfer: +- Command: + echo 1 > /sys/kernel/dw-xdata-pcie/stop -- 2.7.4 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 5/6] MAINTAINERS: Add Synopsys xData IP driver maintainer 2021-02-11 9:08 [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver Gustavo Pimentel ` (3 preceding siblings ...) 2021-02-11 9:08 ` [PATCH v5 4/6] Documentation: misc-devices: Add Documentation for dw-xdata-pcie driver Gustavo Pimentel @ 2021-02-11 9:08 ` Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 6/6] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver Gustavo Pimentel 5 siblings, 0 replies; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:08 UTC (permalink / raw) To: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas Cc: Gustavo Pimentel 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 <gustavo.pimentel@synopsys.com> --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 546aa66..f9d681b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5061,6 +5061,13 @@ S: Maintained F: drivers/dma/dw-edma/ F: include/linux/dma/edma.h +DESIGNWARE XDATA IP DRIVER +M: Gustavo Pimentel <gustavo.pimentel@synopsys.com> +L: linux-pci@vger.kernel.org +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 <hminas@synopsys.com> L: linux-usb@vger.kernel.org -- 2.7.4 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 6/6] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver 2021-02-11 9:08 [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver Gustavo Pimentel ` (4 preceding siblings ...) 2021-02-11 9:08 ` [PATCH v5 5/6] MAINTAINERS: Add Synopsys xData IP driver maintainer Gustavo Pimentel @ 2021-02-11 9:08 ` Gustavo Pimentel 2021-02-11 9:29 ` Leon Romanovsky 5 siblings, 1 reply; 24+ messages in thread From: Gustavo Pimentel @ 2021-02-11 9:08 UTC (permalink / raw) To: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas Cc: Gustavo Pimentel This patch describes the sysfs interface implemented on the dw-xdata-pcie driver. Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> --- 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..a7bb44b --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-xdata @@ -0,0 +1,46 @@ +What: /sys/kernel/dw-xdata-pcie/write +Date: February 2021 +KernelVersion: 5.12 +Contact: Gustavo Pimentel <gustavo.pimentel@synopsys.com> +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/kernel/dw-xdata-pcie/write + + The user can read the current PCIe link throughput generated + through this generator. + Usage e.g. + cat /sys/kernel/dw-xdata-pcie/write + 204 MB/s + + The file is read and write. + +What: /sys/kernel/dw-xdata-pcie/read +Date: February 2021 +KernelVersion: 5.12 +Contact: Gustavo Pimentel <gustavo.pimentel@synopsys.com> +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/kernel/dw-xdata-pcie/read + + The user can read the current PCIe link throughput generated + through this generator. + Usage e.g. + cat /sys/kernel/dw-xdata-pcie/read + 199 MB/s + + The file is read and write. + +What: /sys/kernel/dw-xdata-pcie/stop +Date: February 2021 +KernelVersion: 5.12 +Contact: Gustavo Pimentel <gustavo.pimentel@synopsys.com> +Description: Allows the user to disable the PCIe traffic generator in all + directions. + Usage e.g. + echo 1 > /sys/kernel/dw-xdata-pcie/stop + + The file is write only. -- 2.7.4 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5 6/6] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver 2021-02-11 9:08 ` [PATCH v5 6/6] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver Gustavo Pimentel @ 2021-02-11 9:29 ` Leon Romanovsky 2021-02-11 9:36 ` Greg Kroah-Hartman 0 siblings, 1 reply; 24+ messages in thread From: Leon Romanovsky @ 2021-02-11 9:29 UTC (permalink / raw) To: Gustavo Pimentel Cc: linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Greg Kroah-Hartman, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 10:08:43AM +0100, Gustavo Pimentel wrote: > This patch describes the sysfs interface implemented on the dw-xdata-pcie > driver. > > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > --- > Documentation/ABI/testing/sysfs-driver-xdata | 46 ++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-driver-xdata I don't know and maybe this is how modern drivers are developed now, but my laptop doesn't have anything driver and vendor specific related under plain /sys/kernel/* directory. ➜ kernel git:(rdma-next) ls -l /sys/kernel total 0 drwxr-xr-x. 2 root root 0 Feb 11 11:20 boot_params drwxr-xr-x. 2 root root 0 Feb 11 11:20 btf drwxr-xr-x. 2 root root 0 Feb 11 11:20 cgroup drwxr-xr-x. 2 root root 0 Feb 8 07:45 config drwx------. 51 root root 0 Feb 8 07:45 debug -r--r--r--. 1 root root 4096 Feb 11 11:20 fscaps drwxr-xr-x. 2 root root 0 Feb 8 10:16 iommu_groups drwxr-xr-x. 94 root root 0 Feb 11 11:20 irq -r--r--r--. 1 root root 4096 Feb 11 11:20 kexec_crash_loaded -rw-r--r--. 1 root root 4096 Feb 11 11:20 kexec_crash_size -r--r--r--. 1 root root 4096 Feb 11 11:20 kexec_loaded drwxr-xr-x. 2 root root 0 Feb 11 11:20 livepatch drwxr-xr-x. 6 root root 0 Feb 8 07:45 mm -r--r--r--. 1 root root 512 Feb 11 11:20 notes -rw-r--r--. 1 root root 4096 Feb 11 11:20 profiling -rw-r--r--. 1 root root 4096 Feb 11 11:20 rcu_expedited -rw-r--r--. 1 root root 4096 Feb 11 11:20 rcu_normal drwxr-xr-x. 4 root root 0 Feb 8 07:45 security drwxr-xr-x. 163 root root 0 Feb 11 11:20 slab drwxr-xr-x. 4 root root 0 Feb 11 11:20 software_nodes drwx------. 8 root root 0 Feb 8 07:45 tracing -r--r--r--. 1 root root 4096 Feb 11 11:20 uevent_seqnum -r--r--r--. 1 root root 4096 Feb 11 11:20 vmcoreinfo Also it is very uncommon in large subsystems to see custom sysfs entries for the specific driver. I wonder why this dw-xdata-pcie driver is different. Thanks > > diff --git a/Documentation/ABI/testing/sysfs-driver-xdata b/Documentation/ABI/testing/sysfs-driver-xdata > new file mode 100644 > index 00000000..a7bb44b > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-xdata > @@ -0,0 +1,46 @@ > +What: /sys/kernel/dw-xdata-pcie/write > +Date: February 2021 > +KernelVersion: 5.12 > +Contact: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > +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/kernel/dw-xdata-pcie/write > + > + The user can read the current PCIe link throughput generated > + through this generator. > + Usage e.g. > + cat /sys/kernel/dw-xdata-pcie/write > + 204 MB/s > + > + The file is read and write. > + > +What: /sys/kernel/dw-xdata-pcie/read > +Date: February 2021 > +KernelVersion: 5.12 > +Contact: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > +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/kernel/dw-xdata-pcie/read > + > + The user can read the current PCIe link throughput generated > + through this generator. > + Usage e.g. > + cat /sys/kernel/dw-xdata-pcie/read > + 199 MB/s > + > + The file is read and write. > + > +What: /sys/kernel/dw-xdata-pcie/stop > +Date: February 2021 > +KernelVersion: 5.12 > +Contact: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > +Description: Allows the user to disable the PCIe traffic generator in all > + directions. > + Usage e.g. > + echo 1 > /sys/kernel/dw-xdata-pcie/stop > + > + The file is write only. > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 6/6] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver 2021-02-11 9:29 ` Leon Romanovsky @ 2021-02-11 9:36 ` Greg Kroah-Hartman 0 siblings, 0 replies; 24+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 9:36 UTC (permalink / raw) To: Leon Romanovsky Cc: Gustavo Pimentel, linux-doc, linux-pci, linux-kernel, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Andrew Morton, Jonathan Corbet, Bjorn Helgaas On Thu, Feb 11, 2021 at 11:29:14AM +0200, Leon Romanovsky wrote: > On Thu, Feb 11, 2021 at 10:08:43AM +0100, Gustavo Pimentel wrote: > > This patch describes the sysfs interface implemented on the dw-xdata-pcie > > driver. > > > > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > --- > > Documentation/ABI/testing/sysfs-driver-xdata | 46 ++++++++++++++++++++++++++++ > > 1 file changed, 46 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-driver-xdata > > I don't know and maybe this is how modern drivers are developed now, but my > laptop doesn't have anything driver and vendor specific related under plain > /sys/kernel/* directory. > > ➜ kernel git:(rdma-next) ls -l /sys/kernel > total 0 > drwxr-xr-x. 2 root root 0 Feb 11 11:20 boot_params > drwxr-xr-x. 2 root root 0 Feb 11 11:20 btf > drwxr-xr-x. 2 root root 0 Feb 11 11:20 cgroup > drwxr-xr-x. 2 root root 0 Feb 8 07:45 config > drwx------. 51 root root 0 Feb 8 07:45 debug > -r--r--r--. 1 root root 4096 Feb 11 11:20 fscaps > drwxr-xr-x. 2 root root 0 Feb 8 10:16 iommu_groups > drwxr-xr-x. 94 root root 0 Feb 11 11:20 irq > -r--r--r--. 1 root root 4096 Feb 11 11:20 kexec_crash_loaded > -rw-r--r--. 1 root root 4096 Feb 11 11:20 kexec_crash_size > -r--r--r--. 1 root root 4096 Feb 11 11:20 kexec_loaded > drwxr-xr-x. 2 root root 0 Feb 11 11:20 livepatch > drwxr-xr-x. 6 root root 0 Feb 8 07:45 mm > -r--r--r--. 1 root root 512 Feb 11 11:20 notes > -rw-r--r--. 1 root root 4096 Feb 11 11:20 profiling > -rw-r--r--. 1 root root 4096 Feb 11 11:20 rcu_expedited > -rw-r--r--. 1 root root 4096 Feb 11 11:20 rcu_normal > drwxr-xr-x. 4 root root 0 Feb 8 07:45 security > drwxr-xr-x. 163 root root 0 Feb 11 11:20 slab > drwxr-xr-x. 4 root root 0 Feb 11 11:20 software_nodes > drwx------. 8 root root 0 Feb 8 07:45 tracing > -r--r--r--. 1 root root 4096 Feb 11 11:20 uevent_seqnum > -r--r--r--. 1 root root 4096 Feb 11 11:20 vmcoreinfo > > Also it is very uncommon in large subsystems to see custom sysfs entries > for the specific driver. I wonder why this dw-xdata-pcie driver is different. It is not, and should not do that at all. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-02-11 14:16 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-11 9:08 [PATCH v5 0/6] misc: Add Add Synopsys DesignWare xData IP driver Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 1/6] misc: " Gustavo Pimentel 2021-02-11 9:30 ` Greg Kroah-Hartman 2021-02-11 9:50 ` Gustavo Pimentel 2021-02-11 9:59 ` Greg Kroah-Hartman 2021-02-11 10:21 ` Gustavo Pimentel 2021-02-11 10:33 ` Greg Kroah-Hartman 2021-02-11 10:51 ` Gustavo Pimentel 2021-02-11 11:42 ` Leon Romanovsky 2021-02-11 12:32 ` Greg Kroah-Hartman 2021-02-11 13:50 ` Leon Romanovsky 2021-02-11 14:02 ` Greg Kroah-Hartman 2021-02-11 14:13 ` Leon Romanovsky 2021-02-11 9:08 ` [PATCH v5 2/6] misc: Add Synopsys DesignWare xData IP driver to Makefile Gustavo Pimentel 2021-02-11 9:28 ` Greg Kroah-Hartman 2021-02-11 9:40 ` Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 3/6] misc: Add Synopsys DesignWare xData IP driver to Kconfig Gustavo Pimentel 2021-02-11 11:29 ` Krzysztof Wilczyński 2021-02-11 13:33 ` Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 4/6] Documentation: misc-devices: Add Documentation for dw-xdata-pcie driver Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 5/6] MAINTAINERS: Add Synopsys xData IP driver maintainer Gustavo Pimentel 2021-02-11 9:08 ` [PATCH v5 6/6] docs: ABI: Add sysfs documentation interface of dw-xdata-pcie driver Gustavo Pimentel 2021-02-11 9:29 ` Leon Romanovsky 2021-02-11 9:36 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).