linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/15] staging: gasket: unwrap pci core and more
@ 2018-08-05 20:07 Todd Poynor
  2018-08-05 20:07 ` [PATCH 01/15] staging: gasket: sysfs: clean up state if ENOMEM removing mapping Todd Poynor
                   ` (15 more replies)
  0 siblings, 16 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Stop wrapping PCI core calls like probe, enable, remove, etc. in the
gasket framework, move these calls to the device driver instead.  Have
gasket drivers call into framework on init, enable, disable,
etc. sequences, rather than the other way around.  Remove the
gasket-to-device callbacks associated with these sequences.

Plus a few other fixes and cleanups.

Todd Poynor (15):
  staging: gasket: sysfs: clean up state if ENOMEM removing mapping
  staging: gasket: core: move core PCI calls to device drivers
  staging: gasket: apex: move PCI core calls to apex driver
  staging: gasket: core: convert remaining info logs to debug
  staging: gasket: core: remove device enable and disable callbacks
  staging: gasket: apex: remove device enable and disable callbacks
  staging: gasket: core: let device driver enable/disable gasket device
  staging: gasket: apex: enable/disable gasket device from apex
  staging: gasket: core: delete device add and remove callbacks
  staging: gasket: apex: fold device add/remove logic inline
  staging: gasket: core: remove sysfs setup and cleanup callbacks
  staging: gasket: apex: move sysfs setup code to probe function
  staging: gasket: core: protect against races during unregister
  staging: gasket: apex: place in low power reset until opened
  staging: gasket: core: remove incorrect extraneous comment

 drivers/staging/gasket/apex_driver.c  | 145 +++++++++++++++++---------
 drivers/staging/gasket/gasket_core.c  | 140 ++++++-------------------
 drivers/staging/gasket/gasket_core.h  |  82 +++------------
 drivers/staging/gasket/gasket_sysfs.c |  13 ++-
 4 files changed, 148 insertions(+), 232 deletions(-)

-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 01/15] staging: gasket: sysfs: clean up state if ENOMEM removing mapping
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 02/15] staging: gasket: core: move core PCI calls to device drivers Todd Poynor
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

If kcalloc() returns NULL in put_mapping(), continue to clean up state,
including dropping the reference on the struct device and free attribute
memory.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_sysfs.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
index 56d62aea51118..fc45f0d13e87d 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -101,13 +101,12 @@ static void put_mapping(struct gasket_sysfs_mapping *mapping)
 		files_to_remove = kcalloc(num_files_to_remove,
 					  sizeof(*files_to_remove),
 					  GFP_KERNEL);
-		if (!files_to_remove) {
-			mutex_unlock(&mapping->mutex);
-			return;
-		}
-
-		for (i = 0; i < num_files_to_remove; i++)
-			files_to_remove[i] = mapping->attributes[i].attr;
+		if (files_to_remove)
+			for (i = 0; i < num_files_to_remove; i++)
+				files_to_remove[i] =
+				    mapping->attributes[i].attr;
+		else
+			num_files_to_remove = 0;
 
 		kfree(mapping->attributes);
 		mapping->attributes = NULL;
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 02/15] staging: gasket: core: move core PCI calls to device drivers
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
  2018-08-05 20:07 ` [PATCH 01/15] staging: gasket: sysfs: clean up state if ENOMEM removing mapping Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 03/15] staging: gasket: apex: move PCI core calls to apex driver Todd Poynor
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Remove gasket wrapping of PCI probe, enable, disable, and remove
functions.  Replace with calls to add and remove PCI gasket devices,
to be called by the gasket device drivers.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 82 ++++++++--------------------
 drivers/staging/gasket/gasket_core.h |  6 ++
 2 files changed, 28 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 2d209e36cf372..01cafe1ff6605 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -51,9 +51,6 @@ struct gasket_internal_desc {
 	/* Kernel-internal device class. */
 	struct class *class;
 
-	/* PCI subsystem metadata associated with this driver. */
-	struct pci_driver pci;
-
 	/* Instantiated / present devices of this type. */
 	struct gasket_dev *devs[GASKET_DEV_MAX];
 };
@@ -368,10 +365,10 @@ static void gasket_unmap_pci_bar(struct gasket_dev *dev, int bar_num)
 }
 
 /*
- * Setup PCI & set up memory mapping for the specified device.
+ * Setup PCI memory mapping for the specified device.
  *
- * Enables the PCI device, reads the BAR registers and sets up pointers to the
- * device's memory mapped IO space.
+ * Reads the BAR registers and sets up pointers to the device's memory mapped
+ * IO space.
  *
  * Returns 0 on success and a negative value otherwise.
  */
@@ -380,14 +377,6 @@ static int gasket_setup_pci(struct pci_dev *pci_dev,
 {
 	int i, mapped_bars, ret;
 
-	ret = pci_enable_device(pci_dev);
-	if (ret) {
-		dev_err(gasket_dev->dev, "cannot enable PCI device\n");
-		return ret;
-	}
-
-	pci_set_master(pci_dev);
-
 	for (i = 0; i < GASKET_NUM_BARS; i++) {
 		ret = gasket_map_pci_bar(gasket_dev, i);
 		if (ret) {
@@ -402,19 +391,16 @@ static int gasket_setup_pci(struct pci_dev *pci_dev,
 	for (i = 0; i < mapped_bars; i++)
 		gasket_unmap_pci_bar(gasket_dev, i);
 
-	pci_disable_device(pci_dev);
 	return -ENOMEM;
 }
 
-/* Unmaps memory and cleans up PCI for the specified device. */
+/* Unmaps memory for the specified device. */
 static void gasket_cleanup_pci(struct gasket_dev *gasket_dev)
 {
 	int i;
 
 	for (i = 0; i < GASKET_NUM_BARS; i++)
 		gasket_unmap_pci_bar(gasket_dev, i);
-
-	pci_disable_device(gasket_dev->pci_dev);
 }
 
 /* Determine the health of the Gasket device. */
@@ -1443,15 +1429,14 @@ static int gasket_enable_dev(struct gasket_internal_desc *internal_desc,
 }
 
 /*
- * PCI subsystem probe function.
- *
- * Called when a Gasket device is found. Allocates device metadata, maps device
- * memory, and calls gasket_enable_dev to prepare the device for active use.
+ * Add PCI gasket device.
  *
- * Returns 0 if successful and a negative value otherwise.
+ * Called by Gasket device probe function.
+ * Allocates device metadata, maps device memory, and calls gasket_enable_dev
+ * to prepare the device for active use.
  */
-static int gasket_pci_probe(struct pci_dev *pci_dev,
-			    const struct pci_device_id *id)
+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);
@@ -1460,13 +1445,14 @@ static int gasket_pci_probe(struct pci_dev *pci_dev,
 	const struct gasket_driver_desc *driver_desc;
 	struct device *parent;
 
-	pr_info("Add Gasket device %s\n", kobj_name);
+	pr_debug("add PCI device %s\n", kobj_name);
 
 	mutex_lock(&g_mutex);
 	internal_desc = lookup_internal_desc(pci_dev);
 	mutex_unlock(&g_mutex);
 	if (!internal_desc) {
-		pr_err("PCI probe called for unknown driver type\n");
+		dev_err(&pci_dev->dev,
+			"PCI add device called for unknown driver type\n");
 		return -ENODEV;
 	}
 
@@ -1530,6 +1516,7 @@ static int gasket_pci_probe(struct pci_dev *pci_dev,
 		goto fail5;
 	}
 
+	*gasket_devp = gasket_dev;
 	return 0;
 
 fail5:
@@ -1545,14 +1532,10 @@ static int gasket_pci_probe(struct pci_dev *pci_dev,
 	gasket_free_dev(gasket_dev);
 	return ret;
 }
+EXPORT_SYMBOL(gasket_pci_add_device);
 
-/*
- * PCI subsystem remove function.
- *
- * Called to remove a Gasket device. Finds the device in the device list and
- * cleans up metadata.
- */
-static void gasket_pci_remove(struct pci_dev *pci_dev)
+/* Remove a PCI gasket device. */
+void gasket_pci_remove_device(struct pci_dev *pci_dev)
 {
 	int i;
 	struct gasket_internal_desc *internal_desc;
@@ -1583,8 +1566,8 @@ static void gasket_pci_remove(struct pci_dev *pci_dev)
 	if (!gasket_dev)
 		return;
 
-	pr_info("remove %s device %s\n", internal_desc->driver_desc->name,
-		gasket_dev->kobj_name);
+	dev_dbg(gasket_dev->dev, "remove %s PCI gasket device\n",
+		internal_desc->driver_desc->name);
 
 	gasket_disable_dev(gasket_dev);
 	gasket_cleanup_pci(gasket_dev);
@@ -1597,6 +1580,7 @@ static void gasket_pci_remove(struct pci_dev *pci_dev)
 	device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
 	gasket_free_dev(gasket_dev);
 }
+EXPORT_SYMBOL(gasket_pci_remove_device);
 
 /**
  * Lookup a name by number in a num_name table.
@@ -1770,11 +1754,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
 	internal = &g_descs[desc_idx];
 	mutex_init(&internal->mutex);
 	memset(internal->devs, 0, sizeof(struct gasket_dev *) * GASKET_DEV_MAX);
-	memset(&internal->pci, 0, sizeof(internal->pci));
-	internal->pci.name = driver_desc->name;
-	internal->pci.id_table = driver_desc->pci_id_table;
-	internal->pci.probe = gasket_pci_probe;
-	internal->pci.remove = gasket_pci_remove;
 	internal->class =
 		class_create(driver_desc->module, driver_desc->name);
 
@@ -1785,33 +1764,18 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
 		goto unregister_gasket_driver;
 	}
 
-	/*
-	 * Not using pci_register_driver() (without underscores), as it
-	 * depends on KBUILD_MODNAME, and this is a shared file.
-	 */
-	ret = __pci_register_driver(&internal->pci, driver_desc->module,
-				    driver_desc->name);
-	if (ret) {
-		pr_err("cannot register %s pci driver [ret=%d]\n",
-		       driver_desc->name, ret);
-		goto fail1;
-	}
-
 	ret = register_chrdev_region(MKDEV(driver_desc->major,
 					   driver_desc->minor), GASKET_DEV_MAX,
 				     driver_desc->name);
 	if (ret) {
 		pr_err("cannot register %s char driver [ret=%d]\n",
 		       driver_desc->name, ret);
-		goto fail2;
+		goto destroy_class;
 	}
 
 	return 0;
 
-fail2:
-	pci_unregister_driver(&internal->pci);
-
-fail1:
+destroy_class:
 	class_destroy(internal->class);
 
 unregister_gasket_driver:
@@ -1848,8 +1812,6 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
 	unregister_chrdev_region(MKDEV(driver_desc->major, driver_desc->minor),
 				 GASKET_DEV_MAX);
 
-	pci_unregister_driver(&internal_desc->pci);
-
 	class_destroy(internal_desc->class);
 
 	/* Finally, effectively "remove" the driver. */
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 67f5960943a8a..9f9bc66a0daa0 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -607,6 +607,12 @@ int gasket_register_device(const struct gasket_driver_desc *desc);
  */
 void gasket_unregister_device(const struct gasket_driver_desc *desc);
 
+/* Add a PCI gasket device. */
+int gasket_pci_add_device(struct pci_dev *pci_dev,
+			  struct gasket_dev **gasket_devp);
+/* Remove a PCI gasket device. */
+void gasket_pci_remove_device(struct pci_dev *pci_dev);
+
 /*
  * Reset the Gasket device.
  * @gasket_dev: Gasket device struct.
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 03/15] staging: gasket: apex: move PCI core calls to apex driver
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
  2018-08-05 20:07 ` [PATCH 01/15] staging: gasket: sysfs: clean up state if ENOMEM removing mapping Todd Poynor
  2018-08-05 20:07 ` [PATCH 02/15] staging: gasket: core: move core PCI calls to device drivers Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 04/15] staging: gasket: core: convert remaining info logs to debug Todd Poynor
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Apex driver moves PCI core calls like probe, enable, and remove from
gasket to apex.  Call new functions in gasket to register apex as a PCI
device to the gasket framework.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 49 +++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 42cef68eb4c19..b47661442009d 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -13,6 +13,7 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/pci.h>
 #include <linux/printk.h>
 #include <linux/sched.h>
 #include <linux/uaccess.h>
@@ -621,6 +622,36 @@ static void apex_pci_fixup_class(struct pci_dev *pdev)
 DECLARE_PCI_FIXUP_CLASS_HEADER(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID,
 			       PCI_CLASS_NOT_DEFINED, 8, apex_pci_fixup_class);
 
+static int apex_pci_probe(struct pci_dev *pci_dev,
+			  const struct pci_device_id *id)
+{
+	int ret;
+	struct gasket_dev *gasket_dev;
+
+	ret = pci_enable_device(pci_dev);
+	if (ret) {
+		dev_err(&pci_dev->dev, "error enabling PCI device\n");
+		return ret;
+	}
+
+	pci_set_master(pci_dev);
+
+	ret = gasket_pci_add_device(pci_dev, &gasket_dev);
+	if (ret) {
+		dev_err(&pci_dev->dev, "error adding gasket device\n");
+		pci_disable_device(pci_dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void apex_pci_remove(struct pci_dev *pci_dev)
+{
+	gasket_pci_remove_device(pci_dev);
+	pci_disable_device(pci_dev);
+}
+
 static struct gasket_driver_desc apex_desc = {
 	.name = "apex",
 	.driver_version = APEX_DRIVER_VERSION,
@@ -672,13 +703,29 @@ static struct gasket_driver_desc apex_desc = {
 	.device_reset_cb = apex_reset,
 };
 
+static struct pci_driver apex_pci_driver = {
+	.name = "apex",
+	.probe = apex_pci_probe,
+	.remove = apex_pci_remove,
+	.id_table = apex_pci_ids,
+};
+
 static int __init apex_init(void)
 {
-	return gasket_register_device(&apex_desc);
+	int ret;
+
+	ret = gasket_register_device(&apex_desc);
+	if (ret)
+		return ret;
+	ret = pci_register_driver(&apex_pci_driver);
+	if (ret)
+		gasket_unregister_device(&apex_desc);
+	return ret;
 }
 
 static void apex_exit(void)
 {
+	pci_unregister_driver(&apex_pci_driver);
 	gasket_unregister_device(&apex_desc);
 }
 MODULE_DESCRIPTION("Google Apex driver");
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 04/15] staging: gasket: core: convert remaining info logs to debug
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (2 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 03/15] staging: gasket: apex: move PCI core calls to apex driver Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-08  9:17   ` Greg Kroah-Hartman
  2018-08-05 20:07 ` [PATCH 05/15] staging: gasket: core: remove device enable and disable callbacks Todd Poynor
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Remaining info-level logs in gasket core converted to debug-level; the
information is not needed during normal system operation.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 01cafe1ff6605..2741256eacfe8 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1819,7 +1819,7 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
 	g_descs[desc_idx].driver_desc = NULL;
 	mutex_unlock(&g_mutex);
 
-	pr_info("removed %s driver\n", driver_desc->name);
+	pr_debug("removed %s driver\n", driver_desc->name);
 }
 EXPORT_SYMBOL(gasket_unregister_device);
 
@@ -1827,7 +1827,7 @@ static int __init gasket_init(void)
 {
 	int i;
 
-	pr_info("Performing one-time init of the Gasket framework.\n");
+	pr_debug("%s\n", __func__);
 	/* Check for duplicates and find a free slot. */
 	mutex_lock(&g_mutex);
 	for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
@@ -1843,8 +1843,7 @@ static int __init gasket_init(void)
 
 static void __exit gasket_exit(void)
 {
-	/* No deinit/dealloc needed at present. */
-	pr_info("Removing Gasket framework module.\n");
+	pr_debug("%s\n", __func__);
 }
 MODULE_DESCRIPTION("Google Gasket driver framework");
 MODULE_VERSION(GASKET_FRAMEWORK_VERSION);
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 05/15] staging: gasket: core: remove device enable and disable callbacks
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (3 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 04/15] staging: gasket: core: convert remaining info logs to debug Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 06/15] staging: gasket: apex: " Todd Poynor
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Device enable/disable operations are moving from being initiated through
the gasket framework to being initiated by the gasket device driver.
The driver can perform any processing needed for these operations before
or after the calls into the framework.  Neither of these callbacks are
implemented for the only gasket driver upstream today, apex.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c |  9 ---------
 drivers/staging/gasket/gasket_core.h | 27 ++-------------------------
 2 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 2741256eacfe8..b070efaf0d41c 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -648,8 +648,6 @@ static void gasket_disable_dev(struct gasket_dev *gasket_dev)
 			gasket_page_table_cleanup(gasket_dev->page_table[i]);
 		}
 	}
-
-	check_and_invoke_callback(gasket_dev, driver_desc->disable_dev_cb);
 }
 
 /*
@@ -1408,13 +1406,6 @@ static int gasket_enable_dev(struct gasket_internal_desc *internal_desc,
 	}
 	gasket_dev->hardware_revision = ret;
 
-	ret = check_and_invoke_callback(gasket_dev, driver_desc->enable_dev_cb);
-	if (ret) {
-		dev_err(gasket_dev->dev, "Error in enable device cb: %d\n",
-			ret);
-		return ret;
-	}
-
 	/* device_status_cb returns a device status, not an error code. */
 	gasket_dev->status = gasket_get_hw_status(gasket_dev);
 	if (gasket_dev->status == GASKET_STATUS_DEAD)
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 9f9bc66a0daa0..5d40bc7f52e91 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -473,34 +473,11 @@ struct gasket_driver_desc {
 	 */
 	int (*device_close_cb)(struct gasket_dev *dev);
 
-	/*
-	 * enable_dev_cb: Callback immediately before enabling the device.
-	 * @dev: Pointer to the gasket_dev struct for this driver instance.
-	 *
-	 * This callback is invoked after the device has been added and all BAR
-	 * spaces mapped, immediately before registering and enabling the
-	 * [character] device via cdev_add. If this call fails (returns
-	 * nonzero), disable_dev_cb will be called.
-	 *
-	 * Note that cdev are initialized but not active
-	 * (cdev_add has not yet been called) when this callback is invoked.
-	 */
-	int (*enable_dev_cb)(struct gasket_dev *dev);
-
-	/*
-	 * disable_dev_cb: Callback immediately after disabling the device.
-	 * @dev: Pointer to the gasket_dev struct for this driver instance.
-	 *
-	 * Called during device shutdown, immediately after disabling device
-	 * operations via cdev_del.
-	 */
-	int (*disable_dev_cb)(struct gasket_dev *dev);
-
 	/*
 	 * sysfs_setup_cb: Callback to set up driver-specific sysfs nodes.
 	 * @dev: Pointer to the gasket_dev struct for this device.
 	 *
-	 * Called just before enable_dev_cb.
+	 * Called during the add gasket device call.
 	 *
 	 */
 	int (*sysfs_setup_cb)(struct gasket_dev *dev);
@@ -509,7 +486,7 @@ struct gasket_driver_desc {
 	 * sysfs_cleanup_cb: Callback to clean up driver-specific sysfs nodes.
 	 * @dev: Pointer to the gasket_dev struct for this device.
 	 *
-	 * Called just before disable_dev_cb.
+	 * Called during device disable processing.
 	 *
 	 */
 	int (*sysfs_cleanup_cb)(struct gasket_dev *dev);
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 06/15] staging: gasket: apex: remove device enable and disable callbacks
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (4 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 05/15] staging: gasket: core: remove device enable and disable callbacks Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 07/15] staging: gasket: core: let device driver enable/disable gasket device Todd Poynor
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

These are not implemented for apex, and are now being removed from the
gasket framework.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index b47661442009d..e2bc06b5244f7 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -688,9 +688,6 @@ static struct gasket_driver_desc apex_desc = {
 	.add_dev_cb = apex_add_dev_cb,
 	.remove_dev_cb = NULL,
 
-	.enable_dev_cb = NULL,
-	.disable_dev_cb = NULL,
-
 	.sysfs_setup_cb = apex_sysfs_setup_cb,
 	.sysfs_cleanup_cb = NULL,
 
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 07/15] staging: gasket: core: let device driver enable/disable gasket device
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (5 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 06/15] staging: gasket: apex: " Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 08/15] staging: gasket: apex: enable/disable gasket device from apex Todd Poynor
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Move gasket device enable/disable functions from internal calls to
external calls from the gasket device drivers.  The device driver will
call these functions at appropriate times in its processing, placing
the device driver in control of this sequence and reducing the need for
callbacks from framework back to the device drivers during the
enable/disable sequences.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 22 ++++++++--------------
 drivers/staging/gasket/gasket_core.h |  6 ++++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index b070efaf0d41c..fad4883e6332c 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -628,7 +628,7 @@ static int gasket_add_cdev(struct gasket_cdev_info *dev_info,
 }
 
 /* Disable device operations. */
-static void gasket_disable_dev(struct gasket_dev *gasket_dev)
+void gasket_disable_device(struct gasket_dev *gasket_dev)
 {
 	const struct gasket_driver_desc *driver_desc =
 		gasket_dev->internal_desc->driver_desc;
@@ -649,6 +649,7 @@ static void gasket_disable_dev(struct gasket_dev *gasket_dev)
 		}
 	}
 }
+EXPORT_SYMBOL(gasket_disable_device);
 
 /*
  * Registered descriptor lookup.
@@ -1350,13 +1351,12 @@ static const struct file_operations gasket_file_ops = {
 };
 
 /* Perform final init and marks the device as active. */
-static int gasket_enable_dev(struct gasket_internal_desc *internal_desc,
-			     struct gasket_dev *gasket_dev)
+int gasket_enable_device(struct gasket_dev *gasket_dev)
 {
 	int tbl_idx;
 	int ret;
 	const struct gasket_driver_desc *driver_desc =
-		internal_desc->driver_desc;
+		gasket_dev->internal_desc->driver_desc;
 
 	ret = gasket_interrupt_init(gasket_dev, driver_desc->name,
 				    driver_desc->interrupt_type,
@@ -1418,13 +1418,15 @@ static int gasket_enable_dev(struct gasket_internal_desc *internal_desc,
 
 	return 0;
 }
+EXPORT_SYMBOL(gasket_enable_device);
 
 /*
  * Add PCI gasket device.
  *
  * Called by Gasket device probe function.
- * Allocates device metadata, maps device memory, and calls gasket_enable_dev
- * to prepare the device for active use.
+ * Allocates device metadata and maps device memory.  The device driver must
+ * call gasket_enable_device after driver init is complete to place the device
+ * in active use.
  */
 int gasket_pci_add_device(struct pci_dev *pci_dev,
 			  struct gasket_dev **gasket_devp)
@@ -1500,13 +1502,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
 		goto fail5;
 	}
 
-	ret = gasket_enable_dev(internal_desc, gasket_dev);
-	if (ret) {
-		pr_err("cannot setup %s device\n", driver_desc->name);
-		gasket_disable_dev(gasket_dev);
-		goto fail5;
-	}
-
 	*gasket_devp = gasket_dev;
 	return 0;
 
@@ -1560,7 +1555,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev)
 	dev_dbg(gasket_dev->dev, "remove %s PCI gasket device\n",
 		internal_desc->driver_desc->name);
 
-	gasket_disable_dev(gasket_dev);
 	gasket_cleanup_pci(gasket_dev);
 
 	check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb);
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 5d40bc7f52e91..9c143ebeba452 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -590,6 +590,12 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
 /* Remove a PCI gasket device. */
 void gasket_pci_remove_device(struct pci_dev *pci_dev);
 
+/* Enable a Gasket device. */
+int gasket_enable_device(struct gasket_dev *gasket_dev);
+
+/* Disable a Gasket device. */
+void gasket_disable_device(struct gasket_dev *gasket_dev);
+
 /*
  * Reset the Gasket device.
  * @gasket_dev: Gasket device struct.
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 08/15] staging: gasket: apex: enable/disable gasket device from apex
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (6 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 07/15] staging: gasket: core: let device driver enable/disable gasket device Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 09/15] staging: gasket: core: delete device add and remove callbacks Todd Poynor
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Gasket framework now places device drivers in charge of calling APIs to
enable and disable gasket device operations.  Make the appropriate calls
from the apex driver.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index e2bc06b5244f7..1d8a100c52885 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -643,11 +643,23 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
 		return ret;
 	}
 
+	pci_set_drvdata(pci_dev, gasket_dev);
+	ret = gasket_enable_device(gasket_dev);
+	if (ret) {
+		dev_err(&pci_dev->dev, "error enabling gasket device\n");
+		gasket_pci_remove_device(pci_dev);
+		pci_disable_device(pci_dev);
+		return ret;
+	}
+
 	return 0;
 }
 
 static void apex_pci_remove(struct pci_dev *pci_dev)
 {
+	struct gasket_dev *gasket_dev = pci_get_drvdata(pci_dev);
+
+	gasket_disable_device(gasket_dev);
 	gasket_pci_remove_device(pci_dev);
 	pci_disable_device(pci_dev);
 }
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 09/15] staging: gasket: core: delete device add and remove callbacks
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (7 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 08/15] staging: gasket: apex: enable/disable gasket device from apex Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 10/15] staging: gasket: apex: fold device add/remove logic inline Todd Poynor
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Gasket device drivers are now in charge of orchestrating the device add
and removal sequences, so the callbacks from the framework to the device
drivers for these events are no longer needed.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 10 ----------
 drivers/staging/gasket/gasket_core.h | 29 ----------------------------
 2 files changed, 39 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index fad4883e6332c..0d76e18fcde5b 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1468,12 +1468,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
 	if (ret)
 		goto fail2;
 
-	ret = check_and_invoke_callback(gasket_dev, driver_desc->add_dev_cb);
-	if (ret) {
-		dev_err(gasket_dev->dev, "Error in add device cb: %d\n", ret);
-		goto fail2;
-	}
-
 	ret = gasket_sysfs_create_mapping(gasket_dev->dev_info.device,
 					  gasket_dev);
 	if (ret)
@@ -1512,7 +1506,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
 	gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
 fail2:
 	gasket_cleanup_pci(gasket_dev);
-	check_and_invoke_callback(gasket_dev, driver_desc->remove_dev_cb);
 	device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
 fail1:
 	gasket_free_dev(gasket_dev);
@@ -1559,9 +1552,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev)
 
 	check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb);
 	gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
-
-	check_and_invoke_callback(gasket_dev, driver_desc->remove_dev_cb);
-
 	device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
 	gasket_free_dev(gasket_dev);
 }
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 9c143ebeba452..0ef0a2640f0fe 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -302,12 +302,6 @@ struct gasket_dev {
 	/* Hardware revision value for this device. */
 	int hardware_revision;
 
-	/*
-	 * Device-specific data; allocated in gasket_driver_desc.add_dev_cb()
-	 * and freed in gasket_driver_desc.remove_dev_cb().
-	 */
-	void *cb_data;
-
 	/* Protects access to per-device data (i.e. this structure). */
 	struct mutex mutex;
 
@@ -415,29 +409,6 @@ struct gasket_driver_desc {
 	int interrupt_pack_width;
 
 	/* Driver callback functions - all may be NULL */
-	/*
-	 * add_dev_cb: Callback when a device is found.
-	 * @dev: The gasket_dev struct for this driver instance.
-	 *
-	 * This callback should initialize the device-specific cb_data.
-	 * Called when a device is found by the driver,
-	 * before any BAR ranges have been mapped. If this call fails (returns
-	 * nonzero), remove_dev_cb will be called.
-	 *
-	 */
-	int (*add_dev_cb)(struct gasket_dev *dev);
-
-	/*
-	 * remove_dev_cb: Callback for when a device is removed from the system.
-	 * @dev: The gasket_dev struct for this driver instance.
-	 *
-	 * This callback should free data allocated in add_dev_cb.
-	 * Called immediately before a device is unregistered by the driver.
-	 * All framework-managed resources will have been cleaned up by the time
-	 * this callback is invoked (PCI BARs, character devices, ...).
-	 */
-	int (*remove_dev_cb)(struct gasket_dev *dev);
-
 	/*
 	 * device_open_cb: Callback for when a device node is opened in write
 	 * mode.
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 10/15] staging: gasket: apex: fold device add/remove logic inline
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (8 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 09/15] staging: gasket: core: delete device add and remove callbacks Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 11/15] staging: gasket: core: remove sysfs setup and cleanup callbacks Todd Poynor
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Gasket device drivers are now in charge of the device add and remove
sequences; the framework callbacks for these are deleted.  Move the
apex device add callback code to the probe function.  Apex did not
implement the removal callback.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 69 +++++++++++++---------------
 1 file changed, 32 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 1d8a100c52885..69ca7fb10eddc 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -448,37 +448,6 @@ static int apex_reset(struct gasket_dev *gasket_dev)
 	return ret;
 }
 
-static int apex_add_dev_cb(struct gasket_dev *gasket_dev)
-{
-	ulong page_table_ready, msix_table_ready;
-	int retries = 0;
-
-	apex_reset(gasket_dev);
-
-	while (retries < APEX_RESET_RETRY) {
-		page_table_ready =
-			gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
-					   APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
-		msix_table_ready =
-			gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
-					   APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
-		if (page_table_ready && msix_table_ready)
-			break;
-		schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY));
-		retries++;
-	}
-
-	if (retries == APEX_RESET_RETRY) {
-		if (!page_table_ready)
-			dev_err(gasket_dev->dev, "Page table init timed out\n");
-		if (!msix_table_ready)
-			dev_err(gasket_dev->dev, "MSI-X table init timed out\n");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
 /*
  * Check permissions for Apex ioctls.
  * Returns true if the current user may execute this ioctl, and false otherwise.
@@ -626,6 +595,8 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
 			  const struct pci_device_id *id)
 {
 	int ret;
+	ulong page_table_ready, msix_table_ready;
+	int retries = 0;
 	struct gasket_dev *gasket_dev;
 
 	ret = pci_enable_device(pci_dev);
@@ -644,15 +615,42 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
 	}
 
 	pci_set_drvdata(pci_dev, gasket_dev);
+	apex_reset(gasket_dev);
+
+	while (retries < APEX_RESET_RETRY) {
+		page_table_ready =
+			gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+					   APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
+		msix_table_ready =
+			gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+					   APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
+		if (page_table_ready && msix_table_ready)
+			break;
+		schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY));
+		retries++;
+	}
+
+	if (retries == APEX_RESET_RETRY) {
+		if (!page_table_ready)
+			dev_err(gasket_dev->dev, "Page table init timed out\n");
+		if (!msix_table_ready)
+			dev_err(gasket_dev->dev, "MSI-X table init timed out\n");
+		ret = -ETIMEDOUT;
+		goto remove_device;
+	}
+
 	ret = gasket_enable_device(gasket_dev);
 	if (ret) {
 		dev_err(&pci_dev->dev, "error enabling gasket device\n");
-		gasket_pci_remove_device(pci_dev);
-		pci_disable_device(pci_dev);
-		return ret;
+		goto remove_device;
 	}
 
 	return 0;
+
+remove_device:
+	gasket_pci_remove_device(pci_dev);
+	pci_disable_device(pci_dev);
+	return ret;
 }
 
 static void apex_pci_remove(struct pci_dev *pci_dev)
@@ -697,9 +695,6 @@ static struct gasket_driver_desc apex_desc = {
 	.interrupts = apex_interrupts,
 	.interrupt_pack_width = 7,
 
-	.add_dev_cb = apex_add_dev_cb,
-	.remove_dev_cb = NULL,
-
 	.sysfs_setup_cb = apex_sysfs_setup_cb,
 	.sysfs_cleanup_cb = NULL,
 
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 11/15] staging: gasket: core: remove sysfs setup and cleanup callbacks
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (9 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 10/15] staging: gasket: apex: fold device add/remove logic inline Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 12/15] staging: gasket: apex: move sysfs setup code to probe function Todd Poynor
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Gasket device drivers now call into the gasket framework to initialize
and de-initialize, rather than the other way around.  The calling code
can perform sysfs setup and cleanup actions without callbacks from the
framework.  Remove the sysfs setup and cleanup callbacks.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 10 ----------
 drivers/staging/gasket/gasket_core.h | 18 ------------------
 2 files changed, 28 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 0d76e18fcde5b..ace92f107ed58 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1489,18 +1489,9 @@ int gasket_pci_add_device(struct pci_dev *pci_dev,
 	if (ret)
 		goto fail4;
 
-	ret = check_and_invoke_callback(gasket_dev,
-					driver_desc->sysfs_setup_cb);
-	if (ret) {
-		dev_err(gasket_dev->dev, "Error in sysfs setup cb: %d\n", ret);
-		goto fail5;
-	}
-
 	*gasket_devp = gasket_dev;
 	return 0;
 
-fail5:
-	check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb);
 fail4:
 fail3:
 	gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
@@ -1550,7 +1541,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev)
 
 	gasket_cleanup_pci(gasket_dev);
 
-	check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb);
 	gasket_sysfs_remove_mapping(gasket_dev->dev_info.device);
 	device_destroy(internal_desc->class, gasket_dev->dev_info.devt);
 	gasket_free_dev(gasket_dev);
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 0ef0a2640f0fe..275fd0b345b6e 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -444,24 +444,6 @@ struct gasket_driver_desc {
 	 */
 	int (*device_close_cb)(struct gasket_dev *dev);
 
-	/*
-	 * sysfs_setup_cb: Callback to set up driver-specific sysfs nodes.
-	 * @dev: Pointer to the gasket_dev struct for this device.
-	 *
-	 * Called during the add gasket device call.
-	 *
-	 */
-	int (*sysfs_setup_cb)(struct gasket_dev *dev);
-
-	/*
-	 * sysfs_cleanup_cb: Callback to clean up driver-specific sysfs nodes.
-	 * @dev: Pointer to the gasket_dev struct for this device.
-	 *
-	 * Called during device disable processing.
-	 *
-	 */
-	int (*sysfs_cleanup_cb)(struct gasket_dev *dev);
-
 	/*
 	 * get_mappable_regions_cb: Get descriptors of mappable device memory.
 	 * @gasket_dev: Pointer to the struct gasket_dev for this device.
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 12/15] staging: gasket: apex: move sysfs setup code to probe function
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (10 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 11/15] staging: gasket: core: remove sysfs setup and cleanup callbacks Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 13/15] staging: gasket: core: protect against races during unregister Todd Poynor
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

The gasket framework no longer provides callbacks to the device driver
for sysfs setup and teardown.  Move the sysfs setup code to the device
probe function.  Apex does not implement sysfs cleanup code.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 69ca7fb10eddc..55319619b2e66 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -568,12 +568,6 @@ static struct gasket_sysfs_attribute apex_sysfs_attrs[] = {
 	GASKET_END_OF_ATTR_ARRAY
 };
 
-static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev)
-{
-	return gasket_sysfs_create_entries(gasket_dev->dev_info.device,
-					   apex_sysfs_attrs);
-}
-
 /* On device open, perform a core reinit reset. */
 static int apex_device_open_cb(struct gasket_dev *gasket_dev)
 {
@@ -639,6 +633,11 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
 		goto remove_device;
 	}
 
+	ret = gasket_sysfs_create_entries(gasket_dev->dev_info.device,
+					  apex_sysfs_attrs);
+	if (ret)
+		dev_err(&pci_dev->dev, "error creating device sysfs entries\n");
+
 	ret = gasket_enable_device(gasket_dev);
 	if (ret) {
 		dev_err(&pci_dev->dev, "error enabling gasket device\n");
@@ -695,9 +694,6 @@ static struct gasket_driver_desc apex_desc = {
 	.interrupts = apex_interrupts,
 	.interrupt_pack_width = 7,
 
-	.sysfs_setup_cb = apex_sysfs_setup_cb,
-	.sysfs_cleanup_cb = NULL,
-
 	.device_open_cb = apex_device_open_cb,
 	.device_close_cb = apex_device_cleanup,
 
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 13/15] staging: gasket: core: protect against races during unregister
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (11 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 12/15] staging: gasket: apex: move sysfs setup code to probe function Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 14/15] staging: gasket: apex: place in low power reset until opened Todd Poynor
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Keep mutex held across the unregistration operation, until the
driver_desc field of the global table is removed, to prevent a
concurrent accessor from looking up the driver_desc while
gasket_unregister_device() is in the processing of removing it.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index ace92f107ed58..a6462b6d702f1 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1765,9 +1765,9 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
 			break;
 		}
 	}
-	mutex_unlock(&g_mutex);
 
 	if (!internal_desc) {
+		mutex_unlock(&g_mutex);
 		pr_err("request to unregister unknown desc: %s, %d:%d\n",
 		       driver_desc->name, driver_desc->major,
 		       driver_desc->minor);
@@ -1780,7 +1780,6 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
 	class_destroy(internal_desc->class);
 
 	/* Finally, effectively "remove" the driver. */
-	mutex_lock(&g_mutex);
 	g_descs[desc_idx].driver_desc = NULL;
 	mutex_unlock(&g_mutex);
 
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 14/15] staging: gasket: apex: place in low power reset until opened
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (12 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 13/15] staging: gasket: core: protect against races during unregister Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-05 20:07 ` [PATCH 15/15] staging: gasket: core: remove incorrect extraneous comment Todd Poynor
  2018-08-08  9:20 ` [PATCH 00/15] staging: gasket: unwrap pci core and more Greg Kroah-Hartman
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

The apex device is left out of reset mode at the end of device
probe/initialize processing.  Add a call to enter reset at the end of
the sequence, triggering power gating and other low power features.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 55319619b2e66..c747e9ca45186 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -644,6 +644,10 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
 		goto remove_device;
 	}
 
+	/* Place device in low power mode until opened */
+	if (allow_power_save)
+		apex_enter_reset(gasket_dev);
+
 	return 0;
 
 remove_device:
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 15/15] staging: gasket: core: remove incorrect extraneous comment
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (13 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 14/15] staging: gasket: apex: place in low power reset until opened Todd Poynor
@ 2018-08-05 20:07 ` Todd Poynor
  2018-08-08  9:20 ` [PATCH 00/15] staging: gasket: unwrap pci core and more Greg Kroah-Hartman
  15 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-08-05 20:07 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

A copy-and-pasted comment from another code sequence is removed from
gasket core init sequence.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index a6462b6d702f1..d12ab560411f7 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1792,7 +1792,6 @@ static int __init gasket_init(void)
 	int i;
 
 	pr_debug("%s\n", __func__);
-	/* Check for duplicates and find a free slot. */
 	mutex_lock(&g_mutex);
 	for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
 		g_descs[i].driver_desc = NULL;
-- 
2.18.0.597.ga71716f1ad-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 04/15] staging: gasket: core: convert remaining info logs to debug
  2018-08-05 20:07 ` [PATCH 04/15] staging: gasket: core: convert remaining info logs to debug Todd Poynor
@ 2018-08-08  9:17   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2018-08-08  9:17 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Rob Springer, John Joseph, Ben Chan, devel, Todd Poynor, linux-kernel

On Sun, Aug 05, 2018 at 01:07:38PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> Remaining info-level logs in gasket core converted to debug-level; the
> information is not needed during normal system operation.
> 
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> ---
>  drivers/staging/gasket/gasket_core.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
> index 01cafe1ff6605..2741256eacfe8 100644
> --- a/drivers/staging/gasket/gasket_core.c
> +++ b/drivers/staging/gasket/gasket_core.c
> @@ -1819,7 +1819,7 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
>  	g_descs[desc_idx].driver_desc = NULL;
>  	mutex_unlock(&g_mutex);
>  
> -	pr_info("removed %s driver\n", driver_desc->name);
> +	pr_debug("removed %s driver\n", driver_desc->name);
>  }
>  EXPORT_SYMBOL(gasket_unregister_device);
>  
> @@ -1827,7 +1827,7 @@ static int __init gasket_init(void)
>  {
>  	int i;
>  
> -	pr_info("Performing one-time init of the Gasket framework.\n");
> +	pr_debug("%s\n", __func__);

Lines like this should just be deleted, that is what ftrace is for :)

>  	/* Check for duplicates and find a free slot. */
>  	mutex_lock(&g_mutex);
>  	for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
> @@ -1843,8 +1843,7 @@ static int __init gasket_init(void)
>  
>  static void __exit gasket_exit(void)
>  {
> -	/* No deinit/dealloc needed at present. */
> -	pr_info("Removing Gasket framework module.\n");
> +	pr_debug("%s\n", __func__);

No need to have an exit function at all, right?

I'll take this for now, as this is the last day for me to take patches
for 4.19-rc1, but keep this in mind for the future.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/15] staging: gasket: unwrap pci core and more
  2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
                   ` (14 preceding siblings ...)
  2018-08-05 20:07 ` [PATCH 15/15] staging: gasket: core: remove incorrect extraneous comment Todd Poynor
@ 2018-08-08  9:20 ` Greg Kroah-Hartman
  15 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2018-08-08  9:20 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Rob Springer, John Joseph, Ben Chan, devel, Todd Poynor, linux-kernel

On Sun, Aug 05, 2018 at 01:07:34PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> Stop wrapping PCI core calls like probe, enable, remove, etc. in the
> gasket framework, move these calls to the device driver instead.  Have
> gasket drivers call into framework on init, enable, disable,
> etc. sequences, rather than the other way around.  Remove the
> gasket-to-device callbacks associated with these sequences.
> 
> Plus a few other fixes and cleanups.
> 
> Todd Poynor (15):
>   staging: gasket: sysfs: clean up state if ENOMEM removing mapping
>   staging: gasket: core: move core PCI calls to device drivers
>   staging: gasket: apex: move PCI core calls to apex driver
>   staging: gasket: core: convert remaining info logs to debug
>   staging: gasket: core: remove device enable and disable callbacks
>   staging: gasket: apex: remove device enable and disable callbacks
>   staging: gasket: core: let device driver enable/disable gasket device
>   staging: gasket: apex: enable/disable gasket device from apex
>   staging: gasket: core: delete device add and remove callbacks
>   staging: gasket: apex: fold device add/remove logic inline
>   staging: gasket: core: remove sysfs setup and cleanup callbacks
>   staging: gasket: apex: move sysfs setup code to probe function
>   staging: gasket: core: protect against races during unregister
>   staging: gasket: apex: place in low power reset until opened
>   staging: gasket: core: remove incorrect extraneous comment
> 
>  drivers/staging/gasket/apex_driver.c  | 145 +++++++++++++++++---------
>  drivers/staging/gasket/gasket_core.c  | 140 ++++++-------------------
>  drivers/staging/gasket/gasket_core.h  |  82 +++------------
>  drivers/staging/gasket/gasket_sysfs.c |  13 ++-
>  4 files changed, 148 insertions(+), 232 deletions(-)

Nice cleanups!

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-08-08  9:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-05 20:07 [PATCH 00/15] staging: gasket: unwrap pci core and more Todd Poynor
2018-08-05 20:07 ` [PATCH 01/15] staging: gasket: sysfs: clean up state if ENOMEM removing mapping Todd Poynor
2018-08-05 20:07 ` [PATCH 02/15] staging: gasket: core: move core PCI calls to device drivers Todd Poynor
2018-08-05 20:07 ` [PATCH 03/15] staging: gasket: apex: move PCI core calls to apex driver Todd Poynor
2018-08-05 20:07 ` [PATCH 04/15] staging: gasket: core: convert remaining info logs to debug Todd Poynor
2018-08-08  9:17   ` Greg Kroah-Hartman
2018-08-05 20:07 ` [PATCH 05/15] staging: gasket: core: remove device enable and disable callbacks Todd Poynor
2018-08-05 20:07 ` [PATCH 06/15] staging: gasket: apex: " Todd Poynor
2018-08-05 20:07 ` [PATCH 07/15] staging: gasket: core: let device driver enable/disable gasket device Todd Poynor
2018-08-05 20:07 ` [PATCH 08/15] staging: gasket: apex: enable/disable gasket device from apex Todd Poynor
2018-08-05 20:07 ` [PATCH 09/15] staging: gasket: core: delete device add and remove callbacks Todd Poynor
2018-08-05 20:07 ` [PATCH 10/15] staging: gasket: apex: fold device add/remove logic inline Todd Poynor
2018-08-05 20:07 ` [PATCH 11/15] staging: gasket: core: remove sysfs setup and cleanup callbacks Todd Poynor
2018-08-05 20:07 ` [PATCH 12/15] staging: gasket: apex: move sysfs setup code to probe function Todd Poynor
2018-08-05 20:07 ` [PATCH 13/15] staging: gasket: core: protect against races during unregister Todd Poynor
2018-08-05 20:07 ` [PATCH 14/15] staging: gasket: apex: place in low power reset until opened Todd Poynor
2018-08-05 20:07 ` [PATCH 15/15] staging: gasket: core: remove incorrect extraneous comment Todd Poynor
2018-08-08  9:20 ` [PATCH 00/15] staging: gasket: unwrap pci core and more Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).