[PATCH v6 0/6] staging: kpc2000: another batch of fixes

[PATCH v6 0/6] staging: kpc2000: another batch of fixes

[Jeremy Sowden]

There are a number relating to device attributes, one formatting patch,
and another that changes how card numbers are assigned.

Greg reckoned that the changes to the code in the attribute call-backs
that gets the struct kpc2000 object from the struct device object were
broken.  I've reviewed them and split them into two patches because I
was doing two different things in the previous patch.  I *think* they
are correct, but I've moved them to the end of the series in case I
really have just got the wrong end of the stick, so they can easily be
dropped.

Jeremy Sowden (6):
  staging: kpc2000: improved formatting of core.c.
  staging: kpc2000: added a helper to get struct kp2000_device from
    struct device.
  staging: kpc2000: added separate show functions for readable kp device
    attributes, defined them as read-only, and declared them static.
  staging: kpc2000: use IDA to assign card numbers.
  staging: kpc2000: simplified kp2000_device retrieval in device
    attribute call-backs.
  staging: kpc2000: removed superfluous NULL checks from device
    attribute call-backs.

 drivers/staging/kpc2000/TODO           |   1 -
 drivers/staging/kpc2000/kpc2000/core.c | 818 ++++++++++++++-----------
 2 files changed, 452 insertions(+), 367 deletions(-)

Since v5:

  * dropped two patches that Greg has applied;
  * rebased on to staging-testing.

Since v4:

  * rebased on to staging-next to pick up Greg's most recent changes.
  * made a few changes to some commit messages.
  * sent out the right versions of the patches.

Since v3:

  * added the formatting patch for core.c and folded the kp device
    attribute formatting fixes into it;
  * added the patch that introduces get_pcard();
  * added missing clean-up of IDA and reworded the commit message;
  * split the patch that simplified the retrieval of struct
    kp2000_device from struct dev in the show call-backs into two, and
    moved them to the end of the series.

Since v2:

  * dropped the white-space patch since Greg has applied it;
  * added a reported-by tag to patch that drops two attributes;
  * merged the patches that declared attributes static with the ones
    that split up the show call-backs;
  * moved the attribute definitions next to their call-backs;
  * moved the patch that fixed the card-number race to the end of the
  * series;
  * use an IDA to fix the card-number race, not an atomic_t.

Since v1:

  * merged the DEVICE_ATTR_RO patches with the ones that split up the
    show call-backs;
  * converted the show call-backs to use sprintf, instead of scnprintf.

-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v6 1/6] staging: kpc2000: improved formatting of core.c.

[Jeremy Sowden]

  * Indented with tabs.
  * Broke lines over 80 columns where possible.
  * Removed braces from one-statement blocks.
  * Tidied up some comments.
  * Removed multiple blank lines.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 drivers/staging/kpc2000/kpc2000/core.c | 769 ++++++++++++++-----------
 1 file changed, 422 insertions(+), 347 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 8e6db806f260..f1735237cfb6 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -26,28 +26,51 @@
 #include "uapi.h"
 
 /*******************************************************
-  * SysFS Attributes
-  ******************************************************/
-static ssize_t  show_attr(struct device *dev, struct device_attribute *attr, char *buf)
+ * SysFS Attributes
+ ******************************************************/
+
+static ssize_t show_attr(struct device *dev, struct device_attribute *attr,
+			 char *buf)
 {
-    struct pci_dev *pdev = to_pci_dev(dev);
-    struct kp2000_device *pcard;
-
-    if (!pdev)  return -ENXIO;
-    pcard = pci_get_drvdata(pdev);
-    if (!pcard)  return -ENXIO;
-
-    if (strcmp("ssid", attr->attr.name) == 0){         return scnprintf(buf, PAGE_SIZE, "%016llx\n", pcard->ssid);  } else
-    if (strcmp("ddna", attr->attr.name) == 0){         return scnprintf(buf, PAGE_SIZE, "%016llx\n", pcard->ddna);  } else
-    if (strcmp("card_id", attr->attr.name) == 0){      return scnprintf(buf, PAGE_SIZE, "%08x\n", pcard->card_id);  } else
-    if (strcmp("hw_rev", attr->attr.name) == 0){       return scnprintf(buf, PAGE_SIZE, "%08x\n", pcard->hardware_revision);  } else
-    if (strcmp("build", attr->attr.name) == 0){        return scnprintf(buf, PAGE_SIZE, "%08x\n", pcard->build_version);  } else
-    if (strcmp("build_date", attr->attr.name) == 0){   return scnprintf(buf, PAGE_SIZE, "%08x\n", pcard->build_datestamp);  } else
-    if (strcmp("build_time", attr->attr.name) == 0){   return scnprintf(buf, PAGE_SIZE, "%08x\n", pcard->build_timestamp);  } else
-    { return -ENXIO; }
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct kp2000_device *pcard;
+
+	if (!pdev)
+		return -ENXIO;
+	pcard = pci_get_drvdata(pdev);
+	if (!pcard)
+		return -ENXIO;
+
+	if (strcmp("ssid", attr->attr.name) == 0)
+		return scnprintf(buf, PAGE_SIZE, "%016llx\n", pcard->ssid);
+
+	if (strcmp("ddna", attr->attr.name) == 0)
+		return scnprintf(buf, PAGE_SIZE, "%016llx\n", pcard->ddna);
+
+	if (strcmp("card_id", attr->attr.name) == 0)
+		return scnprintf(buf, PAGE_SIZE, "%08x\n", pcard->card_id);
+
+	if (strcmp("hw_rev", attr->attr.name) == 0)
+		return scnprintf(buf, PAGE_SIZE, "%08x\n",
+				 pcard->hardware_revision);
+
+	if (strcmp("build", attr->attr.name) == 0)
+		return scnprintf(buf, PAGE_SIZE, "%08x\n",
+				 pcard->build_version);
+
+	if (strcmp("build_date", attr->attr.name) == 0)
+		return scnprintf(buf, PAGE_SIZE, "%08x\n",
+				 pcard->build_datestamp);
+
+	if (strcmp("build_time", attr->attr.name) == 0)
+		return scnprintf(buf, PAGE_SIZE, "%08x\n",
+				 pcard->build_timestamp);
+
+	return -ENXIO;
 }
 
-static ssize_t  show_cpld_config_reg(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_cpld_config_reg(struct device *dev,
+				    struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct kp2000_device *pcard;
@@ -63,27 +86,33 @@ static ssize_t  show_cpld_config_reg(struct device *dev, struct device_attribute
 	val = readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
 	return scnprintf(buf, PAGE_SIZE, "%016llx\n", val);
 }
-static ssize_t cpld_reconfigure(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+
+static ssize_t cpld_reconfigure(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
 {
-    struct pci_dev *pdev = to_pci_dev(dev);
-    long wr_val;
-    struct kp2000_device *pcard;
-    int rv;
-
-    if (!pdev)  return -ENXIO;
-    pcard = pci_get_drvdata(pdev);
-    if (!pcard)  return -ENXIO;
-
-    rv = kstrtol(buf, 0, &wr_val);
-    if (rv < 0)  return rv;
-    if (wr_val > 7)  return -EINVAL;
-
-    wr_val = wr_val << 8;
-    wr_val |= 0x1; // Set the "Configure Go" bit
-    writeq(wr_val, pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
-    return count;
-}
+	struct pci_dev *pdev = to_pci_dev(dev);
+	long wr_val;
+	struct kp2000_device *pcard;
+	int rv;
+
+	if (!pdev)
+		return -ENXIO;
+	pcard = pci_get_drvdata(pdev);
+	if (!pcard)
+		return -ENXIO;
+
+	rv = kstrtol(buf, 0, &wr_val);
+	if (rv < 0)
+		return rv;
+	if (wr_val > 7)
+		return -EINVAL;
 
+	wr_val = wr_val << 8;
+	wr_val |= 0x1; // Set the "Configure Go" bit
+	writeq(wr_val, pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
+	return count;
+}
 
 DEVICE_ATTR(ssid,       0444, show_attr, NULL);
 DEVICE_ATTR(ddna,       0444, show_attr, NULL);
@@ -95,106 +124,109 @@ DEVICE_ATTR(build_time, 0444, show_attr, NULL);
 DEVICE_ATTR(cpld_reg,   0444, show_cpld_config_reg, NULL);
 DEVICE_ATTR(cpld_reconfigure,   0220, NULL, cpld_reconfigure);
 
-static const struct attribute *  kp_attr_list[] = {
-    &dev_attr_ssid.attr,
-    &dev_attr_ddna.attr,
-    &dev_attr_card_id.attr,
-    &dev_attr_hw_rev.attr,
-    &dev_attr_build.attr,
-    &dev_attr_build_date.attr,
-    &dev_attr_build_time.attr,
-    &dev_attr_cpld_reg.attr,
-    &dev_attr_cpld_reconfigure.attr,
-    NULL,
+static const struct attribute *kp_attr_list[] = {
+	&dev_attr_ssid.attr,
+	&dev_attr_ddna.attr,
+	&dev_attr_card_id.attr,
+	&dev_attr_hw_rev.attr,
+	&dev_attr_build.attr,
+	&dev_attr_build_date.attr,
+	&dev_attr_build_time.attr,
+	&dev_attr_cpld_reg.attr,
+	&dev_attr_cpld_reconfigure.attr,
+	NULL,
 };
 
-
 /*******************************************************
-  * Functions
-  ******************************************************/
+ * Functions
+ ******************************************************/
 
 static void wait_and_read_ssid(struct kp2000_device *pcard)
 {
-    u64 read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_SSID);
-    unsigned long timeout;
-
-    if (read_val & 0x8000000000000000){
-        pcard->ssid = read_val;
-        return;
-    }
-
-    timeout = jiffies + (HZ * 2);
-    do {
-        read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_SSID);
-        if (read_val & 0x8000000000000000){
-            pcard->ssid = read_val;
-            return;
-        }
-        cpu_relax();
-        //schedule();
-    } while (time_before(jiffies, timeout));
-
-    dev_notice(&pcard->pdev->dev, "SSID didn't show up!\n");
-
-    #if 0
-    // Timed out waiting for the SSID to show up, just use the DDNA instead?
-    read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_DDNA);
-    pcard->ssid = read_val;
-    #else
-    // Timed out waiting for the SSID to show up, stick all zeros in the value
-    pcard->ssid = 0;
-    #endif
+	u64 read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_SSID);
+	unsigned long timeout;
+
+	if (read_val & 0x8000000000000000) {
+		pcard->ssid = read_val;
+		return;
+	}
+
+	timeout = jiffies + (HZ * 2);
+	do {
+		read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_SSID);
+		if (read_val & 0x8000000000000000) {
+			pcard->ssid = read_val;
+			return;
+		}
+		cpu_relax();
+		//schedule();
+	} while (time_before(jiffies, timeout));
+
+	dev_notice(&pcard->pdev->dev, "SSID didn't show up!\n");
+
+#if 0
+	// Timed out waiting for the SSID to show up, just use the DDNA instead?
+	read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_DDNA);
+	pcard->ssid = read_val;
+#else
+	// Timed out waiting for the SSID to show up, stick all zeros in the
+	// value
+	pcard->ssid = 0;
+#endif
 }
 
 static int  read_system_regs(struct kp2000_device *pcard)
 {
-    u64 read_val;
-
-    read_val = readq(pcard->sysinfo_regs_base + REG_MAGIC_NUMBER);
-    if (read_val != KP2000_MAGIC_VALUE){
-        dev_err(&pcard->pdev->dev, "Invalid magic!  Got: 0x%016llx  Want: 0x%016lx\n", read_val, KP2000_MAGIC_VALUE);
-        return -EILSEQ;
-    }
-
-    read_val = readq(pcard->sysinfo_regs_base + REG_CARD_ID_AND_BUILD);
-    pcard->card_id = (read_val & 0xFFFFFFFF00000000) >> 32;
-    pcard->build_version = (read_val & 0x00000000FFFFFFFF) >> 0;
-
-    read_val = readq(pcard->sysinfo_regs_base + REG_DATE_AND_TIME_STAMPS);
-    pcard->build_datestamp = (read_val & 0xFFFFFFFF00000000) >> 32;
-    pcard->build_timestamp = (read_val & 0x00000000FFFFFFFF) >> 0;
-
-    read_val = readq(pcard->sysinfo_regs_base + REG_CORE_TABLE_OFFSET);
-    pcard->core_table_length = (read_val & 0xFFFFFFFF00000000) >> 32;
-    pcard->core_table_offset = (read_val & 0x00000000FFFFFFFF) >> 0;
-
-    wait_and_read_ssid(pcard);
-
-    read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_HW_ID);
-    pcard->core_table_rev    = (read_val & 0x0000000000000F00) >> 8;
-    pcard->hardware_revision = (read_val & 0x000000000000001F);
-
-    read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_DDNA);
-    pcard->ddna = read_val;
-
-    dev_info(&pcard->pdev->dev, "system_regs: %08x %08x %08x %08x  %02x  %d %d  %016llx  %016llx\n",
-        pcard->card_id,
-        pcard->build_version,
-        pcard->build_datestamp,
-        pcard->build_timestamp,
-        pcard->hardware_revision,
-        pcard->core_table_rev,
-        pcard->core_table_length,
-        pcard->ssid,
-        pcard->ddna
-    );
-
-    if (pcard->core_table_rev > 1){
-        dev_err(&pcard->pdev->dev, "core table entry revision is higher than we can deal with, cannot continue with this card!\n");
-        return 1;
-    }
-
-    return 0;
+	u64 read_val;
+
+	read_val = readq(pcard->sysinfo_regs_base + REG_MAGIC_NUMBER);
+	if (read_val != KP2000_MAGIC_VALUE) {
+		dev_err(&pcard->pdev->dev,
+			"Invalid magic!  Got: 0x%016llx  Want: 0x%016lx\n",
+			read_val, KP2000_MAGIC_VALUE);
+		return -EILSEQ;
+	}
+
+	read_val = readq(pcard->sysinfo_regs_base + REG_CARD_ID_AND_BUILD);
+	pcard->card_id = (read_val & 0xFFFFFFFF00000000) >> 32;
+	pcard->build_version = (read_val & 0x00000000FFFFFFFF) >> 0;
+
+	read_val = readq(pcard->sysinfo_regs_base + REG_DATE_AND_TIME_STAMPS);
+	pcard->build_datestamp = (read_val & 0xFFFFFFFF00000000) >> 32;
+	pcard->build_timestamp = (read_val & 0x00000000FFFFFFFF) >> 0;
+
+	read_val = readq(pcard->sysinfo_regs_base + REG_CORE_TABLE_OFFSET);
+	pcard->core_table_length = (read_val & 0xFFFFFFFF00000000) >> 32;
+	pcard->core_table_offset = (read_val & 0x00000000FFFFFFFF) >> 0;
+
+	wait_and_read_ssid(pcard);
+
+	read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_HW_ID);
+	pcard->core_table_rev    = (read_val & 0x0000000000000F00) >> 8;
+	pcard->hardware_revision = (read_val & 0x000000000000001F);
+
+	read_val = readq(pcard->sysinfo_regs_base + REG_FPGA_DDNA);
+	pcard->ddna = read_val;
+
+	dev_info(&pcard->pdev->dev,
+		 "system_regs: %08x %08x %08x %08x  %02x  %d %d  %016llx  %016llx\n",
+		 pcard->card_id,
+		 pcard->build_version,
+		 pcard->build_datestamp,
+		 pcard->build_timestamp,
+		 pcard->hardware_revision,
+		 pcard->core_table_rev,
+		 pcard->core_table_length,
+		 pcard->ssid,
+		 pcard->ddna);
+
+	if (pcard->core_table_rev > 1) {
+		dev_err(&pcard->pdev->dev,
+			"core table entry revision is higher than we can deal with, cannot continue with this card!\n");
+		return 1;
+	}
+
+	return 0;
 }
 
 static irqreturn_t kp2000_irq_handler(int irq, void *dev_id)
@@ -317,241 +349,284 @@ static struct file_operations kp2000_fops = {
 static int kp2000_pcie_probe(struct pci_dev *pdev,
 			     const struct pci_device_id *id)
 {
-    int err = 0;
-    struct kp2000_device *pcard;
-    static int card_count = 1;
-    int rv;
-    unsigned long reg_bar_phys_addr;
-    unsigned long reg_bar_phys_len;
-    unsigned long dma_bar_phys_addr;
-    unsigned long dma_bar_phys_len;
-    u16 regval;
-
-    dev_dbg(&pdev->dev, "kp2000_pcie_probe(pdev = [%p], id = [%p])\n", pdev, id);
-
-    //{ Step 1: Allocate a struct for the pcard
-    pcard = kzalloc(sizeof(struct kp2000_device), GFP_KERNEL);
-    if (NULL == pcard){
-        dev_err(&pdev->dev, "probe: failed to allocate private card data\n");
-        return -ENOMEM;
-    }
-    dev_dbg(&pdev->dev, "probe: allocated struct kp2000_device @ %p\n", pcard);
-    //}
-
-    //{ Step 2: Initialize trivial pcard elements
-    pcard->card_num = card_count;
-    card_count++;
-    scnprintf(pcard->name, 16, "kpcard%d", pcard->card_num);
-
-    mutex_init(&pcard->sem);
+	int err = 0;
+	struct kp2000_device *pcard;
+	static int card_count = 1;
+	int rv;
+	unsigned long reg_bar_phys_addr;
+	unsigned long reg_bar_phys_len;
+	unsigned long dma_bar_phys_addr;
+	unsigned long dma_bar_phys_len;
+	u16 regval;
+
+	dev_dbg(&pdev->dev, "kp2000_pcie_probe(pdev = [%p], id = [%p])\n",
+		pdev, id);
+
+	/*
+	 * Step 1: Allocate a struct for the pcard
+	 */
+	pcard = kzalloc(sizeof(struct kp2000_device), GFP_KERNEL);
+	if (NULL == pcard) {
+		dev_err(&pdev->dev,
+			"probe: failed to allocate private card data\n");
+		return -ENOMEM;
+	}
+	dev_dbg(&pdev->dev, "probe: allocated struct kp2000_device @ %p\n",
+		pcard);
+
+	/*
+	 * Step 2: Initialize trivial pcard elements
+	 */
+	pcard->card_num = card_count;
+	card_count++;
+	scnprintf(pcard->name, 16, "kpcard%d", pcard->card_num);
+
+	mutex_init(&pcard->sem);
 	mutex_lock(&pcard->sem);
 
-    pcard->pdev = pdev;
-    pci_set_drvdata(pdev, pcard);
-    //}
-
-    //{ Step 3: Enable PCI device
-    err = pci_enable_device(pcard->pdev);
-    if (err){
-        dev_err(&pcard->pdev->dev, "probe: failed to enable PCIE2000 PCIe device (%d)\n", err);
-        goto out3;
-    }
-    //}
-
-    //{ Step 4: Setup the Register BAR
-    reg_bar_phys_addr = pci_resource_start(pcard->pdev, REG_BAR);
-    reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
-
-    pcard->regs_bar_base = ioremap_nocache(reg_bar_phys_addr, PAGE_SIZE);
-    if (NULL == pcard->regs_bar_base){
-        dev_err(&pcard->pdev->dev, "probe: REG_BAR could not remap memory to virtual space\n");
-        err = -ENODEV;
-        goto out4;
-    }
-    dev_dbg(&pcard->pdev->dev, "probe: REG_BAR virt hardware address start [%p]\n", pcard->regs_bar_base);
-
-    err = pci_request_region(pcard->pdev, REG_BAR, KP_DRIVER_NAME_KP2000);
-    if (err){
-        iounmap(pcard->regs_bar_base);
-        dev_err(&pcard->pdev->dev, "probe: failed to acquire PCI region (%d)\n", err);
-        err = -ENODEV;
-        goto out4;
-    }
-
-    pcard->regs_base_resource.start = reg_bar_phys_addr;
-    pcard->regs_base_resource.end   = reg_bar_phys_addr + reg_bar_phys_len - 1;
-    pcard->regs_base_resource.flags = IORESOURCE_MEM;
-    //}
-
-    //{ Step 5: Setup the DMA BAR
-    dma_bar_phys_addr = pci_resource_start(pcard->pdev, DMA_BAR);
-    dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
-
-    pcard->dma_bar_base = ioremap_nocache(dma_bar_phys_addr, dma_bar_phys_len);
-    if (NULL == pcard->dma_bar_base){
-        dev_err(&pcard->pdev->dev, "probe: DMA_BAR could not remap memory to virtual space\n");
-        err = -ENODEV;
-        goto out5;
-    }
-    dev_dbg(&pcard->pdev->dev, "probe: DMA_BAR virt hardware address start [%p]\n", pcard->dma_bar_base);
-
-    pcard->dma_common_regs = pcard->dma_bar_base + KPC_DMA_COMMON_OFFSET;
-
-    err = pci_request_region(pcard->pdev, DMA_BAR, "kp2000_pcie");
-    if (err){
-        iounmap(pcard->dma_bar_base);
-        dev_err(&pcard->pdev->dev, "probe: failed to acquire PCI region (%d)\n", err);
-        err = -ENODEV;
-        goto out5;
-    }
-
-    pcard->dma_base_resource.start = dma_bar_phys_addr;
-    pcard->dma_base_resource.end   = dma_bar_phys_addr + dma_bar_phys_len - 1;
-    pcard->dma_base_resource.flags = IORESOURCE_MEM;
-    //}
-
-    //{ Step 6: System Regs
-    pcard->sysinfo_regs_base = pcard->regs_bar_base;
-    err = read_system_regs(pcard);
-    if (err)
-        goto out6;
-
-    // Disable all "user" interrupts because they're not used yet.
-    writeq(0xFFFFFFFFFFFFFFFF, pcard->sysinfo_regs_base + REG_INTERRUPT_MASK);
-    //}
-
-    //{ Step 7: Configure PCI thingies
-    // let the card master PCIe
-    pci_set_master(pcard->pdev);
-    // enable IO and mem if not already done
-    pci_read_config_word(pcard->pdev, PCI_COMMAND, &regval);
-    regval |= (PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
-    pci_write_config_word(pcard->pdev, PCI_COMMAND, regval);
-
-    // Clear relaxed ordering bit
-    pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN, 0);
-
-    // Set Max_Payload_Size and Max_Read_Request_Size
-    regval = (0x0) << 5; // Max_Payload_Size = 128 B
-    pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_PAYLOAD, regval);
-    regval = (0x0) << 12; // Max_Read_Request_Size = 128 B
-    pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_READRQ, regval);
-
-    // Enable error reporting for: Correctable Errors, Non-Fatal Errors, Fatal Errors, Unsupported Requests
-    pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL, 0, PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE);
-
-    err = dma_set_mask(PCARD_TO_DEV(pcard), DMA_BIT_MASK(64));
-    if (err){
-        dev_err(&pcard->pdev->dev, "CANNOT use DMA mask %0llx\n", DMA_BIT_MASK(64));
-        goto out7;
-    }
-    dev_dbg(&pcard->pdev->dev, "Using DMA mask %0llx\n", dma_get_mask(PCARD_TO_DEV(pcard)));
-    //}
-
-    //{ Step 8: Configure IRQs
-    err = pci_enable_msi(pcard->pdev);
-    if (err < 0)
-        goto out8a;
-
-    rv = request_irq(pcard->pdev->irq, kp2000_irq_handler, IRQF_SHARED, pcard->name, pcard);
-    if (rv){
-        dev_err(&pcard->pdev->dev, "kp2000_pcie_probe: failed to request_irq: %d\n", rv);
-        goto out8b;
-    }
-    //}
-
-    //{ Step 9: Setup sysfs attributes
-    err = sysfs_create_files(&(pdev->dev.kobj), kp_attr_list);
-    if (err){
-        dev_err(&pdev->dev, "Failed to add sysfs files: %d\n", err);
-        goto out9;
-    }
-    //}
-
-    //{ Step 10: Setup misc device
-    pcard->miscdev.minor = MISC_DYNAMIC_MINOR;
-    pcard->miscdev.fops = &kp2000_fops;
-    pcard->miscdev.parent = &pcard->pdev->dev;
-    pcard->miscdev.name = pcard->name;
-
-    err = misc_register(&pcard->miscdev);
-    if (err){
-        dev_err(&pcard->pdev->dev, "kp2000_pcie_probe: misc_register failed: %d\n", err);
-        goto out10;
-    }
-    //}
-
-    //{ Step 11: Probe cores
-    err = kp2000_probe_cores(pcard);
-    if (err)
-        goto out11;
-    //}
-
-    //{ Step 12: Enable IRQs in HW
+	pcard->pdev = pdev;
+	pci_set_drvdata(pdev, pcard);
+
+	/*
+	 * Step 3: Enable PCI device
+	 */
+	err = pci_enable_device(pcard->pdev);
+	if (err) {
+		dev_err(&pcard->pdev->dev,
+			"probe: failed to enable PCIE2000 PCIe device (%d)\n",
+			err);
+		goto out3;
+	}
+
+	/*
+	 * Step 4: Setup the Register BAR
+	 */
+	reg_bar_phys_addr = pci_resource_start(pcard->pdev, REG_BAR);
+	reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
+
+	pcard->regs_bar_base = ioremap_nocache(reg_bar_phys_addr, PAGE_SIZE);
+	if (NULL == pcard->regs_bar_base) {
+		dev_err(&pcard->pdev->dev,
+			"probe: REG_BAR could not remap memory to virtual space\n");
+		err = -ENODEV;
+		goto out4;
+	}
+	dev_dbg(&pcard->pdev->dev,
+		"probe: REG_BAR virt hardware address start [%p]\n",
+		pcard->regs_bar_base);
+
+	err = pci_request_region(pcard->pdev, REG_BAR, KP_DRIVER_NAME_KP2000);
+	if (err) {
+		iounmap(pcard->regs_bar_base);
+		dev_err(&pcard->pdev->dev,
+			"probe: failed to acquire PCI region (%d)\n",
+			err);
+		err = -ENODEV;
+		goto out4;
+	}
+
+	pcard->regs_base_resource.start = reg_bar_phys_addr;
+	pcard->regs_base_resource.end   = reg_bar_phys_addr +
+					  reg_bar_phys_len - 1;
+	pcard->regs_base_resource.flags = IORESOURCE_MEM;
+
+	/*
+	 * Step 5: Setup the DMA BAR
+	 */
+	dma_bar_phys_addr = pci_resource_start(pcard->pdev, DMA_BAR);
+	dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
+
+	pcard->dma_bar_base = ioremap_nocache(dma_bar_phys_addr,
+					      dma_bar_phys_len);
+	if (NULL == pcard->dma_bar_base) {
+		dev_err(&pcard->pdev->dev,
+			"probe: DMA_BAR could not remap memory to virtual space\n");
+		err = -ENODEV;
+		goto out5;
+	}
+	dev_dbg(&pcard->pdev->dev,
+		"probe: DMA_BAR virt hardware address start [%p]\n",
+		pcard->dma_bar_base);
+
+	pcard->dma_common_regs = pcard->dma_bar_base + KPC_DMA_COMMON_OFFSET;
+
+	err = pci_request_region(pcard->pdev, DMA_BAR, "kp2000_pcie");
+	if (err) {
+		iounmap(pcard->dma_bar_base);
+		dev_err(&pcard->pdev->dev,
+			"probe: failed to acquire PCI region (%d)\n", err);
+		err = -ENODEV;
+		goto out5;
+	}
+
+	pcard->dma_base_resource.start = dma_bar_phys_addr;
+	pcard->dma_base_resource.end   = dma_bar_phys_addr +
+					 dma_bar_phys_len - 1;
+	pcard->dma_base_resource.flags = IORESOURCE_MEM;
+
+	/*
+	 * Step 6: System Regs
+	 */
+	pcard->sysinfo_regs_base = pcard->regs_bar_base;
+	err = read_system_regs(pcard);
+	if (err)
+		goto out6;
+
+	// Disable all "user" interrupts because they're not used yet.
+	writeq(0xFFFFFFFFFFFFFFFF,
+	       pcard->sysinfo_regs_base + REG_INTERRUPT_MASK);
+
+	/*
+	 * Step 7: Configure PCI thingies
+	 */
+	// let the card master PCIe
+	pci_set_master(pcard->pdev);
+	// enable IO and mem if not already done
+	pci_read_config_word(pcard->pdev, PCI_COMMAND, &regval);
+	regval |= (PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
+	pci_write_config_word(pcard->pdev, PCI_COMMAND, regval);
+
+	// Clear relaxed ordering bit
+	pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_RELAX_EN, 0);
+
+	// Set Max_Payload_Size and Max_Read_Request_Size
+	regval = (0x0) << 5; // Max_Payload_Size = 128 B
+	pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_PAYLOAD, regval);
+	regval = (0x0) << 12; // Max_Read_Request_Size = 128 B
+	pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_READRQ, regval);
+
+	// Enable error reporting for: Correctable Errors, Non-Fatal Errors,
+	// Fatal Errors, Unsupported Requests
+	pcie_capability_clear_and_set_word(pcard->pdev, PCI_EXP_DEVCTL, 0,
+					   PCI_EXP_DEVCTL_CERE |
+					   PCI_EXP_DEVCTL_NFERE |
+					   PCI_EXP_DEVCTL_FERE |
+					   PCI_EXP_DEVCTL_URRE);
+
+	err = dma_set_mask(PCARD_TO_DEV(pcard), DMA_BIT_MASK(64));
+	if (err) {
+		dev_err(&pcard->pdev->dev,
+			"CANNOT use DMA mask %0llx\n", DMA_BIT_MASK(64));
+		goto out7;
+	}
+	dev_dbg(&pcard->pdev->dev,
+		"Using DMA mask %0llx\n", dma_get_mask(PCARD_TO_DEV(pcard)));
+
+	/*
+	 * Step 8: Configure IRQs
+	 */
+	err = pci_enable_msi(pcard->pdev);
+	if (err < 0)
+		goto out8a;
+
+	rv = request_irq(pcard->pdev->irq, kp2000_irq_handler, IRQF_SHARED,
+			 pcard->name, pcard);
+	if (rv) {
+		dev_err(&pcard->pdev->dev,
+			"kp2000_pcie_probe: failed to request_irq: %d\n", rv);
+		goto out8b;
+	}
+
+	/*
+	 * Step 9: Setup sysfs attributes
+	 */
+	err = sysfs_create_files(&(pdev->dev.kobj), kp_attr_list);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to add sysfs files: %d\n", err);
+		goto out9;
+	}
+
+	/*
+	 * Step 10: Setup misc device
+	 */
+	pcard->miscdev.minor = MISC_DYNAMIC_MINOR;
+	pcard->miscdev.fops = &kp2000_fops;
+	pcard->miscdev.parent = &pcard->pdev->dev;
+	pcard->miscdev.name = pcard->name;
+
+	err = misc_register(&pcard->miscdev);
+	if (err) {
+		dev_err(&pcard->pdev->dev,
+			"kp2000_pcie_probe: misc_register failed: %d\n", err);
+		goto out10;
+	}
+
+	/*
+	 * Step 11: Probe cores
+	 */
+	err = kp2000_probe_cores(pcard);
+	if (err)
+		goto out11;
+
+	/*
+	 * Step 12: Enable IRQs in HW
+	 */
 	writel(KPC_DMA_CARD_IRQ_ENABLE | KPC_DMA_CARD_USER_INTERRUPT_MODE,
 	       pcard->dma_common_regs);
-    //}
 
-    dev_dbg(&pcard->pdev->dev, "kp2000_pcie_probe() complete!\n");
+	dev_dbg(&pcard->pdev->dev, "kp2000_pcie_probe() complete!\n");
 	mutex_unlock(&pcard->sem);
-    return 0;
-
-  out11:
-    misc_deregister(&pcard->miscdev);
-  out10:
-    sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
-  out9:
-    free_irq(pcard->pdev->irq, pcard);
-  out8b:
-    pci_disable_msi(pcard->pdev);
-  out8a:
-  out7:
-  out6:
-    iounmap(pcard->dma_bar_base);
-    pci_release_region(pdev, DMA_BAR);
-    pcard->dma_bar_base = NULL;
-  out5:
-    iounmap(pcard->regs_bar_base);
-    pci_release_region(pdev, REG_BAR);
-    pcard->regs_bar_base = NULL;
-  out4:
-    pci_disable_device(pcard->pdev);
-  out3:
+	return 0;
+
+out11:
+	misc_deregister(&pcard->miscdev);
+out10:
+	sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
+out9:
+	free_irq(pcard->pdev->irq, pcard);
+out8b:
+	pci_disable_msi(pcard->pdev);
+out8a:
+out7:
+out6:
+	iounmap(pcard->dma_bar_base);
+	pci_release_region(pdev, DMA_BAR);
+	pcard->dma_bar_base = NULL;
+out5:
+	iounmap(pcard->regs_bar_base);
+	pci_release_region(pdev, REG_BAR);
+	pcard->regs_bar_base = NULL;
+out4:
+	pci_disable_device(pcard->pdev);
+out3:
 	mutex_unlock(&pcard->sem);
-    kfree(pcard);
-    return err;
+	kfree(pcard);
+	return err;
 }
 
-
 static void kp2000_pcie_remove(struct pci_dev *pdev)
 {
-    struct kp2000_device *pcard = pci_get_drvdata(pdev);
+	struct kp2000_device *pcard = pci_get_drvdata(pdev);
 
-    dev_dbg(&pdev->dev, "kp2000_pcie_remove(pdev=%p)\n", pdev);
+	dev_dbg(&pdev->dev, "kp2000_pcie_remove(pdev=%p)\n", pdev);
 
-    if (pcard == NULL)  return;
+	if (pcard == NULL)
+		return;
 
 	mutex_lock(&pcard->sem);
-    kp2000_remove_cores(pcard);
-    mfd_remove_devices(PCARD_TO_DEV(pcard));
-    misc_deregister(&pcard->miscdev);
-    sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
-    free_irq(pcard->pdev->irq, pcard);
-    pci_disable_msi(pcard->pdev);
-    if (pcard->dma_bar_base != NULL){
-        iounmap(pcard->dma_bar_base);
-        pci_release_region(pdev, DMA_BAR);
-        pcard->dma_bar_base = NULL;
-    }
-    if (pcard->regs_bar_base != NULL){
-        iounmap(pcard->regs_bar_base);
-        pci_release_region(pdev, REG_BAR);
-        pcard->regs_bar_base = NULL;
-    }
-    pci_disable_device(pcard->pdev);
-    pci_set_drvdata(pdev, NULL);
+	kp2000_remove_cores(pcard);
+	mfd_remove_devices(PCARD_TO_DEV(pcard));
+	misc_deregister(&pcard->miscdev);
+	sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
+	free_irq(pcard->pdev->irq, pcard);
+	pci_disable_msi(pcard->pdev);
+	if (pcard->dma_bar_base != NULL) {
+		iounmap(pcard->dma_bar_base);
+		pci_release_region(pdev, DMA_BAR);
+		pcard->dma_bar_base = NULL;
+	}
+	if (pcard->regs_bar_base != NULL) {
+		iounmap(pcard->regs_bar_base);
+		pci_release_region(pdev, REG_BAR);
+		pcard->regs_bar_base = NULL;
+	}
+	pci_disable_device(pcard->pdev);
+	pci_set_drvdata(pdev, NULL);
 	mutex_unlock(&pcard->sem);
-    kfree(pcard);
+	kfree(pcard);
 }
 
 struct class *kpc_uio_class;
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v6 2/6] staging: kpc2000: added a helper to get struct kp2000_device from struct device.

[Jeremy Sowden]

The attribute call-backs all use the same formula to get the pcard from
dev:

  struct pci_dev *pdev = to_pci_dev(dev);
  struct kp2000_device *pcard;

  if (!pdev)
    return -ENXIO;
  pcard = pci_get_drvdata(pdev);
  if (!pcard)
    return -ENXIO;

Added a function to reduce the duplicated code.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 drivers/staging/kpc2000/kpc2000/core.c | 29 ++++++++++++--------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index f1735237cfb6..e58bddec87ee 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -29,15 +29,21 @@
  * SysFS Attributes
  ******************************************************/
 
-static ssize_t show_attr(struct device *dev, struct device_attribute *attr,
-			 char *buf)
+static struct kp2000_device *get_pcard(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	struct kp2000_device *pcard;
 
 	if (!pdev)
-		return -ENXIO;
-	pcard = pci_get_drvdata(pdev);
+		return NULL;
+
+	return pci_get_drvdata(pdev);
+}
+
+static ssize_t show_attr(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct kp2000_device *pcard = get_pcard(dev);
+
 	if (!pcard)
 		return -ENXIO;
 
@@ -72,14 +78,9 @@ static ssize_t show_attr(struct device *dev, struct device_attribute *attr,
 static ssize_t show_cpld_config_reg(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct kp2000_device *pcard;
+	struct kp2000_device *pcard = get_pcard(dev);
 	u64 val;
 
-	if (!pdev)
-		return -ENXIO;
-
-	pcard = pci_get_drvdata(pdev);
 	if (!pcard)
 		return -ENXIO;
 
@@ -91,14 +92,10 @@ static ssize_t cpld_reconfigure(struct device *dev,
 				struct device_attribute *attr,
 				const char *buf, size_t count)
 {
-	struct pci_dev *pdev = to_pci_dev(dev);
+	struct kp2000_device *pcard = get_pcard(dev);
 	long wr_val;
-	struct kp2000_device *pcard;
 	int rv;
 
-	if (!pdev)
-		return -ENXIO;
-	pcard = pci_get_drvdata(pdev);
 	if (!pcard)
 		return -ENXIO;
 
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v6 3/6] staging: kpc2000: added separate show functions for readable kp device attributes, defined them as read-only, and declared them static.

[Jeremy Sowden]

Defined separate simple show functions for each attribute instead of
having a one big one containing a chain of conditionals.

Replaced calls to scnprintf with sprintf since all the outputs are
single integers.

All the readable device attributes are read-only, so used DEVICE_ATTR_RO
to define them.

The definitions are only used to populate the kp_attr_list attribute
array, so declared them as static.

Fixes the following sparse warnings:

  drivers/staging/kpc2000/kpc2000/core.c:152:1: warning: symbol 'dev_attr_ssid' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:153:1: warning: symbol 'dev_attr_ddna' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:154:1: warning: symbol 'dev_attr_card_id' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:155:1: warning: symbol 'dev_attr_hw_rev' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:156:1: warning: symbol 'dev_attr_build' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:157:1: warning: symbol 'dev_attr_build_date' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:158:1: warning: symbol 'dev_attr_build_time' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:159:1: warning: symbol 'dev_attr_cpld_reg' was not declared. Should it be static?
  drivers/staging/kpc2000/kpc2000/core.c:161:1: warning: symbol 'dev_attr_cpld_reconfigure' was not declared. Should it be static?

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 drivers/staging/kpc2000/kpc2000/core.c | 106 +++++++++++++++++--------
 1 file changed, 73 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index e58bddec87ee..6b56ddcc03fa 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -39,7 +39,7 @@ static struct kp2000_device *get_pcard(struct device *dev)
 	return pci_get_drvdata(pdev);
 }
 
-static ssize_t show_attr(struct device *dev, struct device_attribute *attr,
+static ssize_t ssid_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct kp2000_device *pcard = get_pcard(dev);
@@ -47,36 +47,84 @@ static ssize_t show_attr(struct device *dev, struct device_attribute *attr,
 	if (!pcard)
 		return -ENXIO;
 
-	if (strcmp("ssid", attr->attr.name) == 0)
-		return scnprintf(buf, PAGE_SIZE, "%016llx\n", pcard->ssid);
+	return sprintf(buf, "%016llx\n", pcard->ssid);
+}
+static DEVICE_ATTR_RO(ssid);
 
-	if (strcmp("ddna", attr->attr.name) == 0)
-		return scnprintf(buf, PAGE_SIZE, "%016llx\n", pcard->ddna);
+static ssize_t ddna_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct kp2000_device *pcard = get_pcard(dev);
 
-	if (strcmp("card_id", attr->attr.name) == 0)
-		return scnprintf(buf, PAGE_SIZE, "%08x\n", pcard->card_id);
+	if (!pcard)
+		return -ENXIO;
+
+	return sprintf(buf, "%016llx\n", pcard->ddna);
+}
+static DEVICE_ATTR_RO(ddna);
+
+static ssize_t card_id_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct kp2000_device *pcard = get_pcard(dev);
 
-	if (strcmp("hw_rev", attr->attr.name) == 0)
-		return scnprintf(buf, PAGE_SIZE, "%08x\n",
-				 pcard->hardware_revision);
+	if (!pcard)
+		return -ENXIO;
 
-	if (strcmp("build", attr->attr.name) == 0)
-		return scnprintf(buf, PAGE_SIZE, "%08x\n",
-				 pcard->build_version);
+	return sprintf(buf, "%08x\n", pcard->card_id);
+}
+static DEVICE_ATTR_RO(card_id);
 
-	if (strcmp("build_date", attr->attr.name) == 0)
-		return scnprintf(buf, PAGE_SIZE, "%08x\n",
-				 pcard->build_datestamp);
+static ssize_t hw_rev_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct kp2000_device *pcard = get_pcard(dev);
 
-	if (strcmp("build_time", attr->attr.name) == 0)
-		return scnprintf(buf, PAGE_SIZE, "%08x\n",
-				 pcard->build_timestamp);
+	if (!pcard)
+		return -ENXIO;
 
-	return -ENXIO;
+	return sprintf(buf, "%08x\n", pcard->hardware_revision);
 }
+static DEVICE_ATTR_RO(hw_rev);
 
-static ssize_t show_cpld_config_reg(struct device *dev,
-				    struct device_attribute *attr, char *buf)
+static ssize_t build_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct kp2000_device *pcard = get_pcard(dev);
+
+	if (!pcard)
+		return -ENXIO;
+
+	return sprintf(buf, "%08x\n", pcard->build_version);
+}
+static DEVICE_ATTR_RO(build);
+
+static ssize_t build_date_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct kp2000_device *pcard = get_pcard(dev);
+
+	if (!pcard)
+		return -ENXIO;
+
+	return sprintf(buf, "%08x\n", pcard->build_datestamp);
+}
+static DEVICE_ATTR_RO(build_date);
+
+static ssize_t build_time_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct kp2000_device *pcard = get_pcard(dev);
+
+	if (!pcard)
+		return -ENXIO;
+
+	return sprintf(buf, "%08x\n", pcard->build_timestamp);
+}
+static DEVICE_ATTR_RO(build_time);
+
+static ssize_t cpld_reg_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
 {
 	struct kp2000_device *pcard = get_pcard(dev);
 	u64 val;
@@ -85,8 +133,9 @@ static ssize_t show_cpld_config_reg(struct device *dev,
 		return -ENXIO;
 
 	val = readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
-	return scnprintf(buf, PAGE_SIZE, "%016llx\n", val);
+	return sprintf(buf, "%016llx\n", val);
 }
+static DEVICE_ATTR_RO(cpld_reg);
 
 static ssize_t cpld_reconfigure(struct device *dev,
 				struct device_attribute *attr,
@@ -110,16 +159,7 @@ static ssize_t cpld_reconfigure(struct device *dev,
 	writeq(wr_val, pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
 	return count;
 }
-
-DEVICE_ATTR(ssid,       0444, show_attr, NULL);
-DEVICE_ATTR(ddna,       0444, show_attr, NULL);
-DEVICE_ATTR(card_id,    0444, show_attr, NULL);
-DEVICE_ATTR(hw_rev,     0444, show_attr, NULL);
-DEVICE_ATTR(build,      0444, show_attr, NULL);
-DEVICE_ATTR(build_date, 0444, show_attr, NULL);
-DEVICE_ATTR(build_time, 0444, show_attr, NULL);
-DEVICE_ATTR(cpld_reg,   0444, show_cpld_config_reg, NULL);
-DEVICE_ATTR(cpld_reconfigure,   0220, NULL, cpld_reconfigure);
+static DEVICE_ATTR(cpld_reconfigure, 0220, NULL, cpld_reconfigure);
 
 static const struct attribute *kp_attr_list[] = {
 	&dev_attr_ssid.attr,
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v6 4/6] staging: kpc2000: use IDA to assign card numbers.

[Jeremy Sowden]

Previously the next card number was assigned from a static int local
variable.  Replaced it with an IDA.  Avoids the assignment of ever-
increasing card-numbers by allowing them to be reused.

Updated TODO.

Corrected format-specifier for unsigned pcard->card_num.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 drivers/staging/kpc2000/TODO           |  1 -
 drivers/staging/kpc2000/kpc2000/core.c | 19 +++++++++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/kpc2000/TODO b/drivers/staging/kpc2000/TODO
index 669fe5bf9637..47530e23e940 100644
--- a/drivers/staging/kpc2000/TODO
+++ b/drivers/staging/kpc2000/TODO
@@ -1,6 +1,5 @@
 - the kpc_spi driver doesn't seem to let multiple transactions (to different instances of the core) happen in parallel...
 - The kpc_i2c driver is a hot mess, it should probably be cleaned up a ton.  It functions against current hardware though.
-- pcard->card_num in kp2000_pcie_probe() is a global variable and needs atomic / locking / something better.
 - would be nice if the AIO fileops in kpc_dma could be made to work
     - probably want to add a CONFIG_ option to control compilation of the AIO functions
 - if the AIO fileops in kpc_dma start working, next would be making iov_count > 1 work too
diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 6b56ddcc03fa..7d6b99fcd2bd 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 #include <linux/kernel.h>
+#include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -25,6 +26,8 @@
 #include "pcie.h"
 #include "uapi.h"
 
+static DEFINE_IDA(card_num_ida);
+
 /*******************************************************
  * SysFS Attributes
  ******************************************************/
@@ -388,7 +391,6 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 {
 	int err = 0;
 	struct kp2000_device *pcard;
-	static int card_count = 1;
 	int rv;
 	unsigned long reg_bar_phys_addr;
 	unsigned long reg_bar_phys_len;
@@ -414,9 +416,14 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	/*
 	 * Step 2: Initialize trivial pcard elements
 	 */
-	pcard->card_num = card_count;
-	card_count++;
-	scnprintf(pcard->name, 16, "kpcard%d", pcard->card_num);
+	err = ida_simple_get(&card_num_ida, 1, INT_MAX, GFP_KERNEL);
+	if (err < 0) {
+		dev_err(&pdev->dev, "probe: failed to get card number (%d)\n",
+			err);
+		goto out2;
+	}
+	pcard->card_num = err;
+	scnprintf(pcard->name, 16, "kpcard%u", pcard->card_num);
 
 	mutex_init(&pcard->sem);
 	mutex_lock(&pcard->sem);
@@ -630,6 +637,8 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	pci_disable_device(pcard->pdev);
 out3:
 	mutex_unlock(&pcard->sem);
+	ida_simple_remove(&card_num_ida, pcard->card_num);
+out2:
 	kfree(pcard);
 	return err;
 }
@@ -663,6 +672,7 @@ static void kp2000_pcie_remove(struct pci_dev *pdev)
 	pci_disable_device(pcard->pdev);
 	pci_set_drvdata(pdev, NULL);
 	mutex_unlock(&pcard->sem);
+	ida_simple_remove(&card_num_ida, pcard->card_num);
 	kfree(pcard);
 }
 
@@ -698,6 +708,7 @@ static void __exit  kp2000_pcie_exit(void)
 {
 	pci_unregister_driver(&kp2000_driver_inst);
 	class_destroy(kpc_uio_class);
+	ida_destroy(&card_num_ida);
 }
 module_exit(kp2000_pcie_exit);
 
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v6 5/6] staging: kpc2000: simplified kp2000_device retrieval in device attribute call-backs.

[Jeremy Sowden]

All the call-backs used the same formula to retrieve the pcard from dev:

  struct pci_dev *pdev = to_pci_dev(dev);
  struct kp2000_device *pcard;

  if (!pdev)
    return NULL;

  pcard = pci_get_drvdata(pdev);

Since to_pci_dev is a wrapper for container_of, it will not return NULL,
and since pci_get_drvdata just calls dev_get_drvdata on the dev member
of pdev, this is equivalent to:

  struct kp2000_device *pcard = dev_get_drvdata(&(container_of(dev, struct pci_dev, dev)->dev));

and we can simplify it to:

  struct kp2000_device *pcard = dev_get_drvdata(dev);

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 drivers/staging/kpc2000/kpc2000/core.c | 28 +++++++++-----------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 7d6b99fcd2bd..2af4170a0d68 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -32,20 +32,10 @@ static DEFINE_IDA(card_num_ida);
  * SysFS Attributes
  ******************************************************/
 
-static struct kp2000_device *get_pcard(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-
-	if (!pdev)
-		return NULL;
-
-	return pci_get_drvdata(pdev);
-}
-
 static ssize_t ssid_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
 	if (!pcard)
 		return -ENXIO;
@@ -57,7 +47,7 @@ static DEVICE_ATTR_RO(ssid);
 static ssize_t ddna_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
 	if (!pcard)
 		return -ENXIO;
@@ -69,7 +59,7 @@ static DEVICE_ATTR_RO(ddna);
 static ssize_t card_id_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
 	if (!pcard)
 		return -ENXIO;
@@ -81,7 +71,7 @@ static DEVICE_ATTR_RO(card_id);
 static ssize_t hw_rev_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
 	if (!pcard)
 		return -ENXIO;
@@ -93,7 +83,7 @@ static DEVICE_ATTR_RO(hw_rev);
 static ssize_t build_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
 	if (!pcard)
 		return -ENXIO;
@@ -105,7 +95,7 @@ static DEVICE_ATTR_RO(build);
 static ssize_t build_date_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
 	if (!pcard)
 		return -ENXIO;
@@ -117,7 +107,7 @@ static DEVICE_ATTR_RO(build_date);
 static ssize_t build_time_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
 	if (!pcard)
 		return -ENXIO;
@@ -129,7 +119,7 @@ static DEVICE_ATTR_RO(build_time);
 static ssize_t cpld_reg_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 	u64 val;
 
 	if (!pcard)
@@ -144,7 +134,7 @@ static ssize_t cpld_reconfigure(struct device *dev,
 				struct device_attribute *attr,
 				const char *buf, size_t count)
 {
-	struct kp2000_device *pcard = get_pcard(dev);
+	struct kp2000_device *pcard = dev_get_drvdata(dev);
 	long wr_val;
 	int rv;
 
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v6 6/6] staging: kpc2000: removed superfluous NULL checks from device attribute call-backs.

[Jeremy Sowden]

All the attribute show call-backs check whether pcard is NULL.  However,
pci_set_drvdata(pdev, pcard) is called before the sysfs files are
created during probe, and pci_set_drvdata(pdev, NULL) is not called
until after they are destroyed during remove; therefore, pcard will not
be NULL, and we can drop the checks.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 drivers/staging/kpc2000/kpc2000/core.c | 27 --------------------------
 1 file changed, 27 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 2af4170a0d68..4110032d0cbb 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -37,9 +37,6 @@ static ssize_t ssid_show(struct device *dev, struct device_attribute *attr,
 {
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
-	if (!pcard)
-		return -ENXIO;
-
 	return sprintf(buf, "%016llx\n", pcard->ssid);
 }
 static DEVICE_ATTR_RO(ssid);
@@ -49,9 +46,6 @@ static ssize_t ddna_show(struct device *dev, struct device_attribute *attr,
 {
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
-	if (!pcard)
-		return -ENXIO;
-
 	return sprintf(buf, "%016llx\n", pcard->ddna);
 }
 static DEVICE_ATTR_RO(ddna);
@@ -61,9 +55,6 @@ static ssize_t card_id_show(struct device *dev, struct device_attribute *attr,
 {
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
-	if (!pcard)
-		return -ENXIO;
-
 	return sprintf(buf, "%08x\n", pcard->card_id);
 }
 static DEVICE_ATTR_RO(card_id);
@@ -73,9 +64,6 @@ static ssize_t hw_rev_show(struct device *dev, struct device_attribute *attr,
 {
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
-	if (!pcard)
-		return -ENXIO;
-
 	return sprintf(buf, "%08x\n", pcard->hardware_revision);
 }
 static DEVICE_ATTR_RO(hw_rev);
@@ -85,9 +73,6 @@ static ssize_t build_show(struct device *dev, struct device_attribute *attr,
 {
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
-	if (!pcard)
-		return -ENXIO;
-
 	return sprintf(buf, "%08x\n", pcard->build_version);
 }
 static DEVICE_ATTR_RO(build);
@@ -97,9 +82,6 @@ static ssize_t build_date_show(struct device *dev,
 {
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
-	if (!pcard)
-		return -ENXIO;
-
 	return sprintf(buf, "%08x\n", pcard->build_datestamp);
 }
 static DEVICE_ATTR_RO(build_date);
@@ -109,9 +91,6 @@ static ssize_t build_time_show(struct device *dev,
 {
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 
-	if (!pcard)
-		return -ENXIO;
-
 	return sprintf(buf, "%08x\n", pcard->build_timestamp);
 }
 static DEVICE_ATTR_RO(build_time);
@@ -122,9 +101,6 @@ static ssize_t cpld_reg_show(struct device *dev, struct device_attribute *attr,
 	struct kp2000_device *pcard = dev_get_drvdata(dev);
 	u64 val;
 
-	if (!pcard)
-		return -ENXIO;
-
 	val = readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
 	return sprintf(buf, "%016llx\n", val);
 }
@@ -138,9 +114,6 @@ static ssize_t cpld_reconfigure(struct device *dev,
 	long wr_val;
 	int rv;
 
-	if (!pcard)
-		return -ENXIO;
-
 	rv = kstrtol(buf, 0, &wr_val);
 	if (rv < 0)
 		return rv;
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 0/6] staging: kpc2000: another batch of fixes

[Greg KH]

On Tue, May 21, 2019 at 11:35:18AM +0100, Jeremy Sowden wrote:
> There are a number relating to device attributes, one formatting patch,
> and another that changes how card numbers are assigned.
> 
> Greg reckoned that the changes to the code in the attribute call-backs
> that gets the struct kpc2000 object from the struct device object were
> broken.  I've reviewed them and split them into two patches because I
> was doing two different things in the previous patch.  I *think* they
> are correct, but I've moved them to the end of the series in case I
> really have just got the wrong end of the stick, so they can easily be
> dropped.

Thanks for sticking with this, now applied.

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 0/6] staging: kpc2000: another batch of fixes

[Jeremy Sowden]

[-- Attachment #1.1: Type: text/plain, Size: 776 bytes --]

On 2019-05-22, at 14:45:51 +0200, Greg KH wrote:
> On Tue, May 21, 2019 at 11:35:18AM +0100, Jeremy Sowden wrote:
> > There are a number relating to device attributes, one formatting
> > patch, and another that changes how card numbers are assigned.
> >
> > Greg reckoned that the changes to the code in the attribute
> > call-backs that gets the struct kpc2000 object from the struct
> > device object were broken.  I've reviewed them and split them into
> > two patches because I was doing two different things in the previous
> > patch.  I *think* they are correct, but I've moved them to the end
> > of the series in case I really have just got the wrong end of the
> > stick, so they can easily be dropped.
>
> Thanks for sticking with this, now applied.

Cheers. :)

J.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel