From: Todd Poynor <[email protected]>
Remove extraneous memory barriers, refactor PCI-specific code in prep
for platform devices in the near future, general cleanups, and make de
facto maintainership official.
Todd Poynor (16):
MAINTAINERS: Switch a maintainer for drivers/staging/gasket
staging: gasket: core: remove debug log that could crash
staging: gasket: core: fix line continuation indent in
gasket_alloc_dev
staging: gasket: core: remove kobj_name param from gasket_alloc_dev
staging: gasket: core: remove ftrace-style debug logs
staging: gasket: remove gasket_exit()
staging: gasket: page table: remove unnecessary NULL check
staging: gasket: page table: use dma_mapping_error for error detection
staging: gasket: core: switch to relaxed memory-mapped I/O
staging: gasket: page table: remove extraneous memory barriers
staging: gasket: core: factor out generic device add code from PCI
code
staging: gasket: core: factor out generic device remove code from PCI
staging: gasket: core: rename lookup_internal_desc to be PCI-specific
staging: gasket: interrupt: refactor PCI MSIX-specific handler code
staging: gasket: interrupt: simplify interrupt init parameters
staging: gasket: interrupt: remove unimplemented interrupt types
MAINTAINERS | 2 +-
drivers/staging/gasket/gasket_core.c | 138 +++++++++++----------
drivers/staging/gasket/gasket_core.h | 19 +--
drivers/staging/gasket/gasket_interrupt.c | 105 ++++++----------
drivers/staging/gasket/gasket_interrupt.h | 24 +---
drivers/staging/gasket/gasket_page_table.c | 24 ++--
6 files changed, 124 insertions(+), 188 deletions(-)
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Some explicit memory barriers in the page table code are not necessary,
either because:
(a) The barrier follows a non-relaxed MMIO access that already performs
a read or write memory barrier.
(b) The barrier follows DMA API calls for which the device-visible
effects of IOMMU programming are guaranteed to be flushed to the IOMMU
prior to the call returning, and doesn't need to sync with normal memory
access.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_page_table.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 4d2499269499b..53492f4fad6aa 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -317,8 +317,6 @@ static void gasket_free_extended_subtable(struct gasket_page_table *pg_tbl,
/* Release the page table from the device */
writeq(0, slot);
- /* Force sync around the address release. */
- mb();
if (pte->dma_addr)
dma_unmap_page(pg_tbl->device, pte->dma_addr, PAGE_SIZE,
@@ -504,8 +502,6 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl,
(void *)page_to_phys(page));
return -1;
}
- /* Wait until the page is mapped. */
- mb();
}
/* Make the DMA-space address available to the device. */
@@ -604,12 +600,13 @@ static void gasket_perform_unmapping(struct gasket_page_table *pg_tbl,
*/
for (i = 0; i < num_pages; i++) {
/* release the address from the device, */
- if (is_simple_mapping || ptes[i].status == PTE_INUSE)
+ if (is_simple_mapping || ptes[i].status == PTE_INUSE) {
writeq(0, &slots[i]);
- else
+ } else {
((u64 __force *)slots)[i] = 0;
- /* Force sync around the address release. */
- mb();
+ /* sync above PTE update before updating mappings */
+ wmb();
+ }
/* release the address from the driver, */
if (ptes[i].status == PTE_INUSE) {
@@ -898,8 +895,6 @@ static int gasket_alloc_extended_subtable(struct gasket_page_table *pg_tbl,
/* Map the page into DMA space. */
pte->dma_addr = dma_map_page(pg_tbl->device, pte->page, 0, PAGE_SIZE,
DMA_BIDIRECTIONAL);
- /* Wait until the page is mapped. */
- mb();
/* make the addresses available to the device */
dma_addr = (pte->dma_addr + pte->offset) | GASKET_VALID_SLOT_FLAG;
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Split out generic gasket device add code from the code for adding a PCI
gasket device, in prep for other gasket device types in the future.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 76 ++++++++++++++++++----------
1 file changed, 48 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index aee819f379e9a..ce8ae226f82d9 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1419,6 +1419,48 @@ int gasket_enable_device(struct gasket_dev *gasket_dev)
}
EXPORT_SYMBOL(gasket_enable_device);
+static int __gasket_add_device(struct device *parent_dev,
+ struct gasket_internal_desc *internal_desc,
+ struct gasket_dev **gasket_devp)
+{
+ int ret;
+ struct gasket_dev *gasket_dev;
+ const struct gasket_driver_desc *driver_desc =
+ internal_desc->driver_desc;
+
+ ret = gasket_alloc_dev(internal_desc, parent_dev, &gasket_dev);
+ if (ret)
+ return ret;
+ if (IS_ERR(gasket_dev->dev_info.device)) {
+ dev_err(parent_dev, "Cannot create %s device %s [ret = %ld]\n",
+ driver_desc->name, gasket_dev->dev_info.name,
+ PTR_ERR(gasket_dev->dev_info.device));
+ ret = -ENODEV;
+ goto free_gasket_dev;
+ }
+
+ ret = gasket_sysfs_create_mapping(gasket_dev->dev_info.device,
+ gasket_dev);
+ if (ret)
+ goto remove_device;
+
+ ret = gasket_sysfs_create_entries(gasket_dev->dev_info.device,
+ gasket_sysfs_generic_attrs);
+ if (ret)
+ goto remove_sysfs_mapping;
+
+ *gasket_devp = gasket_dev;
+ return 0;
+
+remove_sysfs_mapping:
+ gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
+remove_device:
+ device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
+free_gasket_dev:
+ gasket_free_dev(gasket_dev);
+ return ret;
+}
+
/*
* Add PCI gasket device.
*
@@ -1433,7 +1475,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
int ret;
struct gasket_internal_desc *internal_desc;
struct gasket_dev *gasket_dev;
- const struct gasket_driver_desc *driver_desc;
struct device *parent;
dev_dbg(&pci_dev->dev, "add PCI gasket device\n");
@@ -1447,29 +1488,15 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
return -ENODEV;
}
- driver_desc = internal_desc->driver_desc;
-
parent = &pci_dev->dev;
- ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev);
+ ret = __gasket_add_device(parent, internal_desc, &gasket_dev);
if (ret)
return ret;
- gasket_dev->pci_dev = pci_dev;
- if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) {
- pr_err("Cannot create %s device %s [ret = %ld]\n",
- driver_desc->name, gasket_dev->dev_info.name,
- PTR_ERR(gasket_dev->dev_info.device));
- ret = -ENODEV;
- goto fail1;
- }
+ gasket_dev->pci_dev = pci_dev;
ret = gasket_setup_pci(pci_dev, gasket_dev);
if (ret)
- goto fail2;
-
- ret = gasket_sysfs_create_mapping(gasket_dev->dev_info.device,
- gasket_dev);
- if (ret)
- goto fail3;
+ goto cleanup_pci;
/*
* Once we've created the mapping structures successfully, attempt to
@@ -1480,23 +1507,16 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
if (ret) {
dev_err(gasket_dev->dev,
"Cannot create sysfs pci link: %d\n", ret);
- goto fail3;
+ goto cleanup_pci;
}
- ret = gasket_sysfs_create_entries(gasket_dev->dev_info.device,
- gasket_sysfs_generic_attrs);
- if (ret)
- goto fail4;
*gasket_devp = gasket_dev;
return 0;
-fail4:
-fail3:
- gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
-fail2:
+cleanup_pci:
gasket_cleanup_pci(gasket_dev);
+ gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
-fail1:
gasket_free_dev(gasket_dev);
return ret;
}
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Separate code for generic parts of gasket device removal sequence from
the PCI device removal code, in prep for non-PCI devices later.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index ce8ae226f82d9..5e048f6e16e12 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1461,6 +1461,14 @@ static int __gasket_add_device(struct device *parent_dev,
return ret;
}
+static void __gasket_remove_device(struct gasket_internal_desc *internal_desc,
+ struct gasket_dev *gasket_dev)
+{
+ gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
+ device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
+ gasket_free_dev(gasket_dev);
+}
+
/*
* Add PCI gasket device.
*
@@ -1515,9 +1523,7 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
cleanup_pci:
gasket_cleanup_pci(gasket_dev);
- gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
- device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
- gasket_free_dev(gasket_dev);
+ __gasket_remove_device(internal_desc, gasket_dev);
return ret;
}
EXPORT_SYMBOL(gasket_pci_add_device);
@@ -1528,7 +1534,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev)
int i;
struct gasket_internal_desc *internal_desc;
struct gasket_dev *gasket_dev = NULL;
- const struct gasket_driver_desc *driver_desc;
/* Find the device desc. */
mutex_lock(&g_mutex);
internal_desc = lookup_internal_desc(pci_dev);
@@ -1538,8 +1543,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev)
}
mutex_unlock(&g_mutex);
- driver_desc = internal_desc->driver_desc;
-
/* Now find the specific device */
mutex_lock(&internal_desc->mutex);
for (i = 0; i < GASKET_DEV_MAX; i++) {
@@ -1558,10 +1561,7 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev)
internal_desc->driver_desc->name);
gasket_cleanup_pci(gasket_dev);
-
- gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
- device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
- gasket_free_dev(gasket_dev);
+ __gasket_remove_device(internal_desc, gasket_dev);
}
EXPORT_SYMBOL(gasket_pci_remove_device);
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Todd Poynor takes over for John Joseph.
Signed-off-by: John Joseph <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index af64fe0f0b41f..f3466b5c50482 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5939,7 +5939,7 @@ F: Documentation/gcc-plugins.txt
GASKET DRIVER FRAMEWORK
M: Rob Springer <[email protected]>
-M: John Joseph <[email protected]>
+M: Todd Poynor <[email protected]>
M: Ben Chan <[email protected]>
S: Maintained
F: drivers/staging/gasket/
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
gasket_alloc_dev can retrieve the device name from the parent parameter,
a separate parameter isn't needed for this. Rename the variable to
better reflect its meaning, as the name of the parent device for which a
gasket device is being allocated.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 3fb805204d700..5f54b3615f67c 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -189,26 +189,26 @@ static int gasket_find_dev_slot(struct gasket_internal_desc *internal_desc,
* Returns 0 if successful, a negative error code otherwise.
*/
static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc,
- struct device *parent, struct gasket_dev **pdev,
- const char *kobj_name)
+ struct device *parent, struct gasket_dev **pdev)
{
int dev_idx;
const struct gasket_driver_desc *driver_desc =
internal_desc->driver_desc;
struct gasket_dev *gasket_dev;
struct gasket_cdev_info *dev_info;
+ const char *parent_name = dev_name(parent);
- pr_debug("Allocating a Gasket device %s.\n", kobj_name);
+ pr_debug("Allocating a Gasket device, parent %s.\n", parent_name);
*pdev = NULL;
- dev_idx = gasket_find_dev_slot(internal_desc, kobj_name);
+ dev_idx = gasket_find_dev_slot(internal_desc, parent_name);
if (dev_idx < 0)
return dev_idx;
gasket_dev = *pdev = kzalloc(sizeof(*gasket_dev), GFP_KERNEL);
if (!gasket_dev) {
- pr_err("no memory for device %s\n", kobj_name);
+ pr_err("no memory for device, parent %s\n", parent_name);
return -ENOMEM;
}
internal_desc->devs[dev_idx] = gasket_dev;
@@ -217,7 +217,7 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc,
gasket_dev->internal_desc = internal_desc;
gasket_dev->dev_idx = dev_idx;
- snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", kobj_name);
+ snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", parent_name);
gasket_dev->dev = get_device(parent);
/* gasket_bar_data is uninitialized. */
gasket_dev->num_page_tables = driver_desc->num_page_tables;
@@ -1431,13 +1431,12 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
struct gasket_dev **gasket_devp)
{
int ret;
- const char *kobj_name = dev_name(&pci_dev->dev);
struct gasket_internal_desc *internal_desc;
struct gasket_dev *gasket_dev;
const struct gasket_driver_desc *driver_desc;
struct device *parent;
- pr_debug("add PCI device %s\n", kobj_name);
+ dev_dbg(&pci_dev->dev, "add PCI gasket device\n");
mutex_lock(&g_mutex);
internal_desc = lookup_internal_desc(pci_dev);
@@ -1451,7 +1450,7 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
driver_desc = internal_desc->driver_desc;
parent = &pci_dev->dev;
- ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev, kobj_name);
+ ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev);
if (ret)
return ret;
gasket_dev->pci_dev = pci_dev;
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
gasket_perform_mapping() call dma_mapping_error() to determine if
mapping failed.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_page_table.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index bd921dc6094de..4d2499269499b 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -493,7 +493,8 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl,
(void *)page_to_pfn(page),
(unsigned long long)ptes[i].dma_addr);
- if (ptes[i].dma_addr == -1) {
+ if (dma_mapping_error(pg_tbl->device,
+ ptes[i].dma_addr)) {
dev_dbg(pg_tbl->device,
"%s i %d -> fail to map page %llx "
"[pfn %p ohys %p]\n",
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Use of readl() is deprecated; readl_relaxed() with appropriate memory
barriers is preferred. Switch to relaxed reads and writes for better
performance as well. Memory barriers required for I/O vs. normal
memory access on Apex devices have already been explicitly coded in the
page table routines.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 275fd0b345b6e..fd7e75b765a6d 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -590,25 +590,25 @@ const char *gasket_num_name_lookup(uint num,
static inline ulong gasket_dev_read_64(struct gasket_dev *gasket_dev, int bar,
ulong location)
{
- return readq(&gasket_dev->bar_data[bar].virt_base[location]);
+ return readq_relaxed(&gasket_dev->bar_data[bar].virt_base[location]);
}
static inline void gasket_dev_write_64(struct gasket_dev *dev, u64 value,
int bar, ulong location)
{
- writeq(value, &dev->bar_data[bar].virt_base[location]);
+ writeq_relaxed(value, &dev->bar_data[bar].virt_base[location]);
}
static inline void gasket_dev_write_32(struct gasket_dev *dev, u32 value,
int bar, ulong location)
{
- writel(value, &dev->bar_data[bar].virt_base[location]);
+ writel_relaxed(value, &dev->bar_data[bar].virt_base[location]);
}
static inline u32 gasket_dev_read_32(struct gasket_dev *dev, int bar,
ulong location)
{
- return readl(&dev->bar_data[bar].virt_base[location]);
+ return readl_relaxed(&dev->bar_data[bar].virt_base[location]);
}
static inline void gasket_read_modify_write_64(struct gasket_dev *dev, int bar,
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Interrupt types PCI_MSI and PLATFORM_WIRE are unused and unimplemented.
Remove these.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.h | 11 --------
drivers/staging/gasket/gasket_interrupt.c | 34 +----------------------
2 files changed, 1 insertion(+), 44 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index fd7e75b765a6d..0203460f48957 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -50,8 +50,6 @@ enum gasket_interrupt_packing {
/* Type of the interrupt supported by the device. */
enum gasket_interrupt_type {
PCI_MSIX = 0,
- PCI_MSI = 1,
- PLATFORM_WIRE = 2,
};
/*
@@ -69,12 +67,6 @@ struct gasket_interrupt_desc {
int packing;
};
-/* Offsets to the wire interrupt handling registers */
-struct gasket_wire_interrupt_offsets {
- u64 pending_bit_array;
- u64 mask_array;
-};
-
/*
* This enum is used to identify memory regions being part of the physical
* memory that belongs to a device.
@@ -384,9 +376,6 @@ struct gasket_driver_desc {
*/
struct gasket_coherent_buffer_desc coherent_buffer_description;
- /* Offset of wire interrupt registers. */
- const struct gasket_wire_interrupt_offsets *wire_interrupt_offsets;
-
/* Interrupt type. (One of gasket_interrupt_type). */
int interrupt_type;
diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index eb5dfbe08e214..2cd262be65ca0 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -45,9 +45,6 @@ struct gasket_interrupt_data {
/* The width of a single interrupt in a packed interrupt register. */
int pack_width;
- /* offset of wire interrupt registers */
- const struct gasket_wire_interrupt_offsets *wire_interrupt_offsets;
-
/*
* Design-wise, these elements should be bundled together, but
* pci_enable_msix's interface requires that they be managed
@@ -92,19 +89,6 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
dev_dbg(gasket_dev->dev, "Running interrupt setup\n");
- if (interrupt_data->type == PLATFORM_WIRE ||
- interrupt_data->type == PCI_MSI) {
- /* Nothing needs to be done for platform or PCI devices. */
- return;
- }
-
- if (interrupt_data->type != PCI_MSIX) {
- dev_dbg(gasket_dev->dev,
- "Cannot handle unsupported interrupt type %d\n",
- interrupt_data->type);
- return;
- }
-
/* Setup the MSIX table. */
for (i = 0; i < interrupt_data->num_interrupts; i++) {
@@ -351,8 +335,6 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
interrupt_data->interrupt_bar_index = driver_desc->interrupt_bar_index;
interrupt_data->pack_width = driver_desc->interrupt_pack_width;
interrupt_data->num_configured = 0;
- interrupt_data->wire_interrupt_offsets =
- driver_desc->wire_interrupt_offsets;
interrupt_data->eventfd_ctxs = kcalloc(driver_desc->num_interrupts,
sizeof(struct eventfd_ctx *),
@@ -379,12 +361,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
force_msix_interrupt_unmasking(gasket_dev);
break;
- case PCI_MSI:
- case PLATFORM_WIRE:
default:
- dev_err(gasket_dev->dev,
- "Cannot handle unsupported interrupt type %d\n",
- interrupt_data->type);
ret = -EINVAL;
}
@@ -439,12 +416,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
force_msix_interrupt_unmasking(gasket_dev);
break;
- case PCI_MSI:
- case PLATFORM_WIRE:
default:
- dev_dbg(gasket_dev->dev,
- "Cannot handle unsupported interrupt type %d\n",
- gasket_dev->interrupt_data->type);
ret = -EINVAL;
}
@@ -489,12 +461,8 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
gasket_interrupt_msix_cleanup(interrupt_data);
break;
- case PCI_MSI:
- case PLATFORM_WIRE:
default:
- dev_dbg(gasket_dev->dev,
- "Cannot handle unsupported interrupt type %d\n",
- interrupt_data->type);
+ break;
}
kfree(interrupt_data->interrupt_counts);
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Pass the gasket driver descriptor to the interrupt init function, rather
than exploding out separate parameters from various fields of that
structure. This allows us to make more localized changes to the types
of interrupts supported (MSIX vs. wire, etc.) without affecting the
calling sequence, and seems nicer for simplification purposes.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 8 +------
drivers/staging/gasket/gasket_interrupt.c | 27 +++++++++++------------
drivers/staging/gasket/gasket_interrupt.h | 24 +-------------------
3 files changed, 15 insertions(+), 44 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 99f3f11d75ce2..f230bec76ae4e 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1357,13 +1357,7 @@ int gasket_enable_device(struct gasket_dev *gasket_dev)
const struct gasket_driver_desc *driver_desc =
gasket_dev->internal_desc->driver_desc;
- ret = gasket_interrupt_init(gasket_dev, driver_desc->name,
- driver_desc->interrupt_type,
- driver_desc->interrupts,
- driver_desc->num_interrupts,
- driver_desc->interrupt_pack_width,
- driver_desc->interrupt_bar_index,
- driver_desc->wire_interrupt_offsets);
+ ret = gasket_interrupt_init(gasket_dev);
if (ret) {
dev_err(gasket_dev->dev,
"Critical failure to allocate interrupts: %d\n", ret);
diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index f94e4ea9a7ded..eb5dfbe08e214 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -331,31 +331,30 @@ static struct gasket_sysfs_attribute interrupt_sysfs_attrs[] = {
GASKET_END_OF_ATTR_ARRAY,
};
-int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
- int type,
- const struct gasket_interrupt_desc *interrupts,
- int num_interrupts, int pack_width, int bar_index,
- const struct gasket_wire_interrupt_offsets *wire_int_offsets)
+int gasket_interrupt_init(struct gasket_dev *gasket_dev)
{
int ret;
struct gasket_interrupt_data *interrupt_data;
+ const struct gasket_driver_desc *driver_desc =
+ gasket_get_driver_desc(gasket_dev);
interrupt_data = kzalloc(sizeof(struct gasket_interrupt_data),
GFP_KERNEL);
if (!interrupt_data)
return -ENOMEM;
gasket_dev->interrupt_data = interrupt_data;
- interrupt_data->name = name;
- interrupt_data->type = type;
+ interrupt_data->name = driver_desc->name;
+ interrupt_data->type = driver_desc->interrupt_type;
interrupt_data->pci_dev = gasket_dev->pci_dev;
- interrupt_data->num_interrupts = num_interrupts;
- interrupt_data->interrupts = interrupts;
- interrupt_data->interrupt_bar_index = bar_index;
- interrupt_data->pack_width = pack_width;
+ interrupt_data->num_interrupts = driver_desc->num_interrupts;
+ interrupt_data->interrupts = driver_desc->interrupts;
+ interrupt_data->interrupt_bar_index = driver_desc->interrupt_bar_index;
+ interrupt_data->pack_width = driver_desc->interrupt_pack_width;
interrupt_data->num_configured = 0;
- interrupt_data->wire_interrupt_offsets = wire_int_offsets;
+ interrupt_data->wire_interrupt_offsets =
+ driver_desc->wire_interrupt_offsets;
- interrupt_data->eventfd_ctxs = kcalloc(num_interrupts,
+ interrupt_data->eventfd_ctxs = kcalloc(driver_desc->num_interrupts,
sizeof(struct eventfd_ctx *),
GFP_KERNEL);
if (!interrupt_data->eventfd_ctxs) {
@@ -363,7 +362,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
return -ENOMEM;
}
- interrupt_data->interrupt_counts = kcalloc(num_interrupts,
+ interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts,
sizeof(ulong),
GFP_KERNEL);
if (!interrupt_data->interrupt_counts) {
diff --git a/drivers/staging/gasket/gasket_interrupt.h b/drivers/staging/gasket/gasket_interrupt.h
index 835af439e96a9..85526a1374a1a 100644
--- a/drivers/staging/gasket/gasket_interrupt.h
+++ b/drivers/staging/gasket/gasket_interrupt.h
@@ -24,30 +24,8 @@ struct gasket_interrupt_data;
/*
* Initialize the interrupt module.
* @gasket_dev: The Gasket device structure for the device to be initted.
- * @type: Type of the interrupt. (See gasket_interrupt_type).
- * @name: The name to associate with these interrupts.
- * @interrupts: An array of all interrupt descriptions for this device.
- * @num_interrupts: The length of the @interrupts array.
- * @pack_width: The width, in bits, of a single field in a packed interrupt reg.
- * @bar_index: The bar containing all interrupt registers.
- *
- * Allocates and initializes data to track interrupt state for a device.
- * After this call, no interrupts will be configured/delivered; call
- * gasket_interrupt_set_vector[_packed] to associate each interrupt with an
- * __iomem location, then gasket_interrupt_set_eventfd to associate an eventfd
- * with an interrupt.
- *
- * If num_interrupts interrupts are not available, this call will return a
- * negative error code. In that case, gasket_interrupt_cleanup should still be
- * called. Returns 0 on success (which can include a device where interrupts
- * are not possible to set up, but is otherwise OK; that device will report
- * status LAMED.)
*/
-int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
- int type,
- const struct gasket_interrupt_desc *interrupts,
- int num_interrupts, int pack_width, int bar_index,
- const struct gasket_wire_interrupt_offsets *wire_int_offsets);
+int gasket_interrupt_init(struct gasket_dev *gasket_dev);
/*
* Clean up a device's interrupt structure.
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Remove now-empty gasket_exit() function.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 0fe5b86b294c8..aee819f379e9a 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1801,12 +1801,8 @@ static int __init gasket_init(void)
return 0;
}
-static void __exit gasket_exit(void)
-{
-}
MODULE_DESCRIPTION("Google Gasket driver framework");
MODULE_VERSION(GASKET_FRAMEWORK_VERSION);
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Rob Springer <[email protected]>");
module_init(gasket_init);
-module_exit(gasket_exit);
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Split interrupt handler into PCI MSIX-specific and generic functions,
for adding non-MSIX handlers in the future. Move MSIX init code
together,, out of generic init path.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_interrupt.c | 48 ++++++++++++-----------
1 file changed, 25 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index 1cfbc120f2284..f94e4ea9a7ded 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -157,9 +157,22 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
}
}
-static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id)
+static void
+gasket_handle_interrupt(struct gasket_interrupt_data *interrupt_data,
+ int interrupt_index)
{
struct eventfd_ctx *ctx;
+
+ trace_gasket_interrupt_event(interrupt_data->name, interrupt_index);
+ ctx = interrupt_data->eventfd_ctxs[interrupt_index];
+ if (ctx)
+ eventfd_signal(ctx, 1);
+
+ ++(interrupt_data->interrupt_counts[interrupt_index]);
+}
+
+static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id)
+{
struct gasket_interrupt_data *interrupt_data = dev_id;
int interrupt = -1;
int i;
@@ -175,14 +188,7 @@ static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id)
pr_err("Received unknown irq %d\n", irq);
return IRQ_HANDLED;
}
- trace_gasket_interrupt_event(interrupt_data->name, interrupt);
-
- ctx = interrupt_data->eventfd_ctxs[interrupt];
- if (ctx)
- eventfd_signal(ctx, 1);
-
- ++(interrupt_data->interrupt_counts[interrupt]);
-
+ gasket_handle_interrupt(interrupt_data, interrupt);
return IRQ_HANDLED;
}
@@ -192,6 +198,12 @@ gasket_interrupt_msix_init(struct gasket_interrupt_data *interrupt_data)
int ret = 1;
int i;
+ interrupt_data->msix_entries =
+ kcalloc(interrupt_data->num_interrupts,
+ sizeof(struct msix_entry), GFP_KERNEL);
+ if (!interrupt_data->msix_entries)
+ return -ENOMEM;
+
for (i = 0; i < interrupt_data->num_interrupts; i++) {
interrupt_data->msix_entries[i].entry = i;
interrupt_data->msix_entries[i].vector = 0;
@@ -343,20 +355,10 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
interrupt_data->num_configured = 0;
interrupt_data->wire_interrupt_offsets = wire_int_offsets;
- /* Allocate all dynamic structures. */
- interrupt_data->msix_entries = kcalloc(num_interrupts,
- sizeof(struct msix_entry),
- GFP_KERNEL);
- if (!interrupt_data->msix_entries) {
- kfree(interrupt_data);
- return -ENOMEM;
- }
-
interrupt_data->eventfd_ctxs = kcalloc(num_interrupts,
sizeof(struct eventfd_ctx *),
GFP_KERNEL);
if (!interrupt_data->eventfd_ctxs) {
- kfree(interrupt_data->msix_entries);
kfree(interrupt_data);
return -ENOMEM;
}
@@ -366,7 +368,6 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
GFP_KERNEL);
if (!interrupt_data->interrupt_counts) {
kfree(interrupt_data->eventfd_ctxs);
- kfree(interrupt_data->msix_entries);
kfree(interrupt_data);
return -ENOMEM;
}
@@ -417,6 +418,7 @@ gasket_interrupt_msix_cleanup(struct gasket_interrupt_data *interrupt_data)
if (interrupt_data->msix_configured)
pci_disable_msix(interrupt_data->pci_dev);
interrupt_data->msix_configured = 0;
+ kfree(interrupt_data->msix_entries);
}
int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
@@ -448,10 +450,11 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
}
if (ret) {
- /* Failing to setup MSIx will cause the device
+ /* Failing to setup interrupts will cause the device
* to report GASKET_STATUS_LAMED, but is not fatal.
*/
- dev_warn(gasket_dev->dev, "Couldn't init msix: %d\n", ret);
+ dev_warn(gasket_dev->dev, "Couldn't reinit interrupts: %d\n",
+ ret);
return 0;
}
@@ -497,7 +500,6 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
kfree(interrupt_data->interrupt_counts);
kfree(interrupt_data->eventfd_ctxs);
- kfree(interrupt_data->msix_entries);
kfree(interrupt_data);
gasket_dev->interrupt_data = NULL;
}
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Rename lookup_internal_desc() to lookup_pci_internal_desc() to reflect
use for PCI devices only, in prep for non-PCI devices in the future.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 5e048f6e16e12..99f3f11d75ce2 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -651,13 +651,13 @@ void gasket_disable_device(struct gasket_dev *gasket_dev)
EXPORT_SYMBOL(gasket_disable_device);
/*
- * Registered descriptor lookup.
+ * Registered driver descriptor lookup for PCI devices.
*
* Precondition: Called with g_mutex held (to avoid a race on return).
* Returns NULL if no matching device was found.
*/
static struct gasket_internal_desc *
-lookup_internal_desc(struct pci_dev *pci_dev)
+lookup_pci_internal_desc(struct pci_dev *pci_dev)
{
int i;
@@ -1488,7 +1488,7 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
dev_dbg(&pci_dev->dev, "add PCI gasket device\n");
mutex_lock(&g_mutex);
- internal_desc = lookup_internal_desc(pci_dev);
+ internal_desc = lookup_pci_internal_desc(pci_dev);
mutex_unlock(&g_mutex);
if (!internal_desc) {
dev_err(&pci_dev->dev,
@@ -1536,7 +1536,7 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev)
struct gasket_dev *gasket_dev = NULL;
/* Find the device desc. */
mutex_lock(&g_mutex);
- internal_desc = lookup_internal_desc(pci_dev);
+ internal_desc = lookup_pci_internal_desc(pci_dev);
if (!internal_desc) {
mutex_unlock(&g_mutex);
return;
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
A debug log in gasket_alloc_dev() is issued regardless of whether the
device pointer used returned success or error. The log isn't that
useful anyway, remove it.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index d12ab560411f7..37d14e30ffa21 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -234,8 +234,6 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc,
dev_info->device = device_create(internal_desc->class, parent,
dev_info->devt, gasket_dev, dev_info->name);
- dev_dbg(dev_info->device, "Gasket device allocated.\n");
-
/* cdev has not yet been added; cdev_added is 0 */
dev_info->gasket_dev_ptr = gasket_dev;
/* ownership is all 0, indicating no owner or opens. */
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Previous cleanups missed a case of multi-line function call with line
continuation parameters not aligned per kernel style.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 37d14e30ffa21..3fb805204d700 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -231,8 +231,9 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc,
dev_info->devt =
MKDEV(driver_desc->major, driver_desc->minor +
gasket_dev->dev_idx);
- dev_info->device = device_create(internal_desc->class, parent,
- dev_info->devt, gasket_dev, dev_info->name);
+ dev_info->device =
+ device_create(internal_desc->class, parent, dev_info->devt,
+ gasket_dev, dev_info->name);
/* cdev has not yet been added; cdev_added is 0 */
dev_info->gasket_dev_ptr = gasket_dev;
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
gasket_alloc_coherent_memory remove unnecessary NULL check for
coherent_pages.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_page_table.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index d4c5f8aa7dd34..bd921dc6094de 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -1328,10 +1328,8 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
num_pages * PAGE_SIZE, mem, handle);
}
- if (gasket_dev->page_table[index]->coherent_pages) {
- kfree(gasket_dev->page_table[index]->coherent_pages);
- gasket_dev->page_table[index]->coherent_pages = NULL;
- }
+ kfree(gasket_dev->page_table[index]->coherent_pages);
+ gasket_dev->page_table[index]->coherent_pages = NULL;
gasket_dev->page_table[index]->num_coherent_pages = 0;
return -ENOMEM;
}
--
2.18.0.597.ga71716f1ad-goog
From: Todd Poynor <[email protected]>
Remove debug logs that only indicate the name of the entered function,
in favor of using ftrace for function tracing style logs.
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 5f54b3615f67c..0fe5b86b294c8 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1789,7 +1789,6 @@ static int __init gasket_init(void)
{
int i;
- pr_debug("%s\n", __func__);
mutex_lock(&g_mutex);
for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
g_descs[i].driver_desc = NULL;
@@ -1804,7 +1803,6 @@ static int __init gasket_init(void)
static void __exit gasket_exit(void)
{
- pr_debug("%s\n", __func__);
}
MODULE_DESCRIPTION("Google Gasket driver framework");
MODULE_VERSION(GASKET_FRAMEWORK_VERSION);
--
2.18.0.597.ga71716f1ad-goog
Acked-by: John Joseph <[email protected]>
On Thu, Aug 9, 2018 at 8:20 PM, Todd Poynor <[email protected]> wrote:
> From: Todd Poynor <[email protected]>
>
> Todd Poynor takes over for John Joseph.
>
> Signed-off-by: John Joseph <[email protected]>
> Signed-off-by: Todd Poynor <[email protected]>
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index af64fe0f0b41f..f3466b5c50482 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5939,7 +5939,7 @@ F: Documentation/gcc-plugins.txt
>
> GASKET DRIVER FRAMEWORK
> M: Rob Springer <[email protected]>
> -M: John Joseph <[email protected]>
> +M: Todd Poynor <[email protected]>
> M: Ben Chan <[email protected]>
> S: Maintained
> F: drivers/staging/gasket/
> --
> 2.18.0.597.ga71716f1ad-goog
>
On Thu, Aug 09, 2018 at 08:40:06PM -0700, John Joseph wrote:
> Acked-by: John Joseph <[email protected]>
Why are you acking something you supposidly already signed-off on?
> On Thu, Aug 9, 2018 at 8:20 PM, Todd Poynor <[email protected]> wrote:
> > From: Todd Poynor <[email protected]>
> >
> > Todd Poynor takes over for John Joseph.
> >
> > Signed-off-by: John Joseph <[email protected]>
> > Signed-off-by: Todd Poynor <[email protected]>
Did you not really provide your signed-off-by here?
totally confused,
greg k-h
On Thu, Aug 9, 2018 at 11:14 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Thu, Aug 09, 2018 at 08:40:06PM -0700, John Joseph wrote:
> > Acked-by: John Joseph <[email protected]>
>
> Why are you acking something you supposidly already signed-off on?
Sorry, my fault, wasn't sure what the protocol was for these so I
suggested he send an Acked-by as well, can drop that.
>
> > On Thu, Aug 9, 2018 at 8:20 PM, Todd Poynor <[email protected]> wrote:
> > > From: Todd Poynor <[email protected]>
> > >
> > > Todd Poynor takes over for John Joseph.
> > >
> > > Signed-off-by: John Joseph <[email protected]>
> > > Signed-off-by: Todd Poynor <[email protected]>
>
> Did you not really provide your signed-off-by here?
We generated this together, the S-o-b is valid,
>
> totally confused,
>
> greg k-h
Reviewed-by: Rob Springer <[email protected]>
On Fri, Aug 10, 2018 at 9:56 AM Rob Springer <[email protected]> wrote:
>
> Revewed-by: Rob Springer <[email protected]>
>
> On Thu, Aug 9, 2018 at 8:21 PM Todd Poynor <[email protected]> wrote:
>>
>> From: Todd Poynor <[email protected]>
>>
>> A debug log in gasket_alloc_dev() is issued regardless of whether the
>> device pointer used returned success or error. The log isn't that
>> useful anyway, remove it.
>>
>> Signed-off-by: Todd Poynor <[email protected]>
>> ---
>> drivers/staging/gasket/gasket_core.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
>> index d12ab560411f7..37d14e30ffa21 100644
>> --- a/drivers/staging/gasket/gasket_core.c
>> +++ b/drivers/staging/gasket/gasket_core.c
>> @@ -234,8 +234,6 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc,
>> dev_info->device = device_create(internal_desc->class, parent,
>> dev_info->devt, gasket_dev, dev_info->name);
>>
>> - dev_dbg(dev_info->device, "Gasket device allocated.\n");
>> -
>> /* cdev has not yet been added; cdev_added is 0 */
>> dev_info->gasket_dev_ptr = gasket_dev;
>> /* ownership is all 0, indicating no owner or opens. */
>> --
>> 2.18.0.597.ga71716f1ad-goog
>>
Btw, can someone explain what 'gasket' even is? A quick look through
the directory didn't provide any answers. And please don't just send
that anser to me, put it in the source tree (Kconfig, and if needed
another documentation file).