linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] staging: gasket: fixes and cleanups
@ 2018-07-29 19:36 Todd Poynor
  2018-07-29 19:36 ` [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used Todd Poynor
                   ` (12 more replies)
  0 siblings, 13 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Fixes for device reference counting and root access based on user
namespace for containers, plus cleanups of comments, forward
declarations for static functions (more forthcoming) and multi-line
continuation style (more to come).

Todd Poynor (13):
  staging: gasket: core: hold reference to pci_dev while used
  staging: gasket: sysfs: hold reference to device while in use
  staging: gasket: page table: hold references to device and pci_dev
  staging: gasket: core: allow root access based on user namespace
  staging: gasket: apex: simplify comments for static functions
  staging: gasket: core: simplify comments for static functions
  staging: gasket: ioctl: simplify comments for static functions
  staging: gasket: page table: simplify comments for static functions
  staging: gasket: interrupt: simplify comments for static functions
  staging: gasket: sysfs: simplify comments for static functions
  staging: gasket: TODO: remove entry for static function kernel docs
  staging: gasket: apex: remove static function forward declarations
  staging: gasket: apex: fix function param line continuation style

 drivers/staging/gasket/TODO                |   1 -
 drivers/staging/gasket/apex_driver.c       | 559 +++++++++------------
 drivers/staging/gasket/gasket_core.c       | 165 ++----
 drivers/staging/gasket/gasket_interrupt.c  |  18 +-
 drivers/staging/gasket/gasket_ioctl.c      |  51 +-
 drivers/staging/gasket/gasket_page_table.c | 329 ++----------
 drivers/staging/gasket/gasket_sysfs.c      |  39 +-
 7 files changed, 350 insertions(+), 812 deletions(-)

-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-31  6:18   ` Dmitry Torokhov
  2018-07-29 19:36 ` [PATCH 02/13] staging: gasket: sysfs: hold reference to device while in use Todd Poynor
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Hold a reference on the struct pci_dev while a pointer to it is held in
the gasket data structures.

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

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 2b484d067c38a..b832a4f529f27 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
 	internal_desc->devs[gasket_dev->dev_idx] = NULL;
 	mutex_unlock(&internal_desc->mutex);
 	put_device(gasket_dev->dev);
+	pci_dev_put(gasket_dev->pci_dev);
 	kfree(gasket_dev);
 }
 
@@ -565,6 +566,7 @@ static int gasket_pci_probe(
 	ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev, kobj_name);
 	if (ret)
 		return ret;
+	gasket_dev->pci_dev = pci_dev_get(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,
@@ -572,7 +574,6 @@ static int gasket_pci_probe(
 		ret = -ENODEV;
 		goto fail1;
 	}
-	gasket_dev->pci_dev = pci_dev;
 
 	ret = gasket_setup_pci(pci_dev, gasket_dev);
 	if (ret)
@@ -703,7 +704,6 @@ static int gasket_setup_pci(
 {
 	int i, mapped_bars, ret;
 
-	gasket_dev->pci_dev = pci_dev;
 	ret = pci_enable_device(pci_dev);
 	if (ret) {
 		dev_err(gasket_dev->dev, "cannot enable PCI device\n");
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 02/13] staging: gasket: sysfs: hold reference to device while in use
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
  2018-07-29 19:36 ` [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 03/13] staging: gasket: page table: hold references to device and pci_dev Todd Poynor
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Hold a reference to the struct device while a gasket sysfs mapping
exists for the device and a pointer to the struct is kept in the mapping
data structures.

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

diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
index da972ce0e0db0..fde04658419bc 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -126,6 +126,7 @@ static void put_mapping(struct gasket_sysfs_mapping *mapping)
 		kfree(mapping->attributes);
 		mapping->attributes = NULL;
 		mapping->attribute_count = 0;
+		put_device(mapping->device);
 		mapping->device = NULL;
 		mapping->gasket_dev = NULL;
 	}
@@ -208,22 +209,20 @@ int gasket_sysfs_create_mapping(
 		device->kobj.name);
 
 	mapping = &dev_mappings[map_idx];
-	kref_init(&mapping->refcount);
-	mapping->device = device;
-	mapping->gasket_dev = gasket_dev;
 	mapping->attributes = kcalloc(GASKET_SYSFS_MAX_NODES,
 				      sizeof(*mapping->attributes),
 				      GFP_KERNEL);
-	mapping->attribute_count = 0;
 	if (!mapping->attributes) {
 		dev_dbg(device, "Unable to allocate sysfs attribute array\n");
-		mapping->device = NULL;
-		mapping->gasket_dev = NULL;
 		mutex_unlock(&mapping->mutex);
 		mutex_unlock(&function_mutex);
 		return -ENOMEM;
 	}
 
+	kref_init(&mapping->refcount);
+	mapping->device = get_device(device);
+	mapping->gasket_dev = gasket_dev;
+	mapping->attribute_count = 0;
 	mutex_unlock(&mapping->mutex);
 	mutex_unlock(&function_mutex);
 
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 03/13] staging: gasket: page table: hold references to device and pci_dev
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
  2018-07-29 19:36 ` [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used Todd Poynor
  2018-07-29 19:36 ` [PATCH 02/13] staging: gasket: sysfs: hold reference to device while in use Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 04/13] staging: gasket: core: allow root access based on user namespace Todd Poynor
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Hold references to the struct device and the pci_dev for the page table
while the data structures contian pointers to these.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index b9304d221722b..6b946a155ee3a 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -345,8 +345,8 @@ int gasket_page_table_init(
 		bar_data->virt_base[page_table_config->base_reg]);
 	pg_tbl->extended_offset_reg = (u64 __iomem *)&(
 		bar_data->virt_base[page_table_config->extended_reg]);
-	pg_tbl->device = device;
-	pg_tbl->pci_dev = pci_dev;
+	pg_tbl->device = get_device(device);
+	pg_tbl->pci_dev = pci_dev_get(pci_dev);
 
 	dev_dbg(device, "Page table initialized successfully\n");
 
@@ -364,6 +364,8 @@ void gasket_page_table_cleanup(struct gasket_page_table *pg_tbl)
 	vfree(pg_tbl->entries);
 	pg_tbl->entries = NULL;
 
+	put_device(pg_tbl->device);
+	pci_dev_put(pg_tbl->pci_dev);
 	kfree(pg_tbl);
 }
 
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 04/13] staging: gasket: core: allow root access based on user namespace
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (2 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 03/13] staging: gasket: page table: hold references to device and pci_dev Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-30 17:56   ` Dmitry Torokhov
  2018-07-29 19:36 ` [PATCH 05/13] staging: gasket: apex: simplify comments for static functions Todd Poynor
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Use user namespace to determine whether gasket device file opener is
root, allowing root access to containers, if necessary.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index b832a4f529f27..291cd6d074a2e 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -13,13 +13,16 @@
 #include "gasket_page_table.h"
 #include "gasket_sysfs.h"
 
+#include <linux/capability.h>
 #include <linux/compiler.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/fs.h>
 #include <linux/init.h>
 #include <linux/of.h>
+#include <linux/pid_namespace.h>
 #include <linux/printk.h>
+#include <linux/sched.h>
 
 #ifdef GASKET_KERNEL_TRACE_SUPPORT
 #define CREATE_TRACE_POINTS
@@ -1064,7 +1067,8 @@ static int gasket_open(struct inode *inode, struct file *filp)
 	char task_name[TASK_COMM_LEN];
 	struct gasket_cdev_info *dev_info =
 	    container_of(inode->i_cdev, struct gasket_cdev_info, cdev);
-	int is_root = capable(CAP_SYS_ADMIN);
+	struct pid_namespace *pid_ns = task_active_pid_ns(current);
+	int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);
 
 	gasket_dev = dev_info->gasket_dev_ptr;
 	driver_desc = gasket_dev->internal_desc->driver_desc;
@@ -1147,6 +1151,8 @@ static int gasket_release(struct inode *inode, struct file *file)
 	char task_name[TASK_COMM_LEN];
 	struct gasket_cdev_info *dev_info =
 		container_of(inode->i_cdev, struct gasket_cdev_info, cdev);
+	struct pid_namespace *pid_ns = task_active_pid_ns(current);
+	int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);
 
 	gasket_dev = dev_info->gasket_dev_ptr;
 	driver_desc = gasket_dev->internal_desc->driver_desc;
@@ -1158,7 +1164,7 @@ static int gasket_release(struct inode *inode, struct file *file)
 		"Releasing device node. Call origin: tgid %u (%s) "
 		"(f_mode: 0%03o, fmode_write: %d, is_root: %u)\n",
 		current->tgid, task_name, file->f_mode,
-		(file->f_mode & FMODE_WRITE), capable(CAP_SYS_ADMIN));
+		(file->f_mode & FMODE_WRITE), is_root);
 	dev_dbg(gasket_dev->dev, "Current open count (owning tgid %u): %d\n",
 		ownership->owner, ownership->write_open_count);
 
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 05/13] staging: gasket: apex: simplify comments for static functions
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (3 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 04/13] staging: gasket: core: allow root access based on user namespace Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 06/13] staging: gasket: core: " Todd Poynor
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line.  Remove extraneous text.

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

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index ab466d49608a7..a756764751777 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -378,34 +378,20 @@ static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev)
 		gasket_dev->dev_info.device, apex_sysfs_attrs);
 }
 
-/* On device open, we want to perform a core reinit reset. */
+/* On device open, perform a core reinit reset. */
 static int apex_device_open_cb(struct gasket_dev *gasket_dev)
 {
 	return gasket_reset_nolock(gasket_dev, APEX_CHIP_REINIT_RESET);
 }
 
-/**
- * apex_get_status - Set device status.
- * @dev: Apex device struct.
- *
- * Description: Check the device status registers and set the driver status
- *		to ALIVE or DEAD.
- *
- *		Returns 0 if status is ALIVE, a negative error number otherwise.
- */
+/* Check the device status registers and return device status ALIVE or DEAD. */
 static int apex_get_status(struct gasket_dev *gasket_dev)
 {
 	/* TODO: Check device status. */
 	return GASKET_STATUS_ALIVE;
 }
 
-/**
- * apex_device_cleanup - Clean up Apex HW after close.
- * @gasket_dev: Gasket device pointer.
- *
- * Description: Resets the Apex hardware. Called on final close via
- * device_close_cb.
- */
+/* Reset the Apex hardware. Called on final close via device_close_cb. */
 static int apex_device_cleanup(struct gasket_dev *gasket_dev)
 {
 	u64 scalar_error;
@@ -429,14 +415,7 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
 	return ret;
 }
 
-/**
- * apex_reset - Quits reset.
- * @gasket_dev: Gasket device pointer.
- *
- * Description: Resets the hardware, then quits reset.
- * Called on device open.
- *
- */
+/* Reset the hardware, then quit reset.  Called on device open. */
 static int apex_reset(struct gasket_dev *gasket_dev, uint type)
 {
 	int ret;
@@ -459,9 +438,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type)
 	return ret;
 }
 
-/*
- * Enters GCB reset state.
- */
+/* Enter GCB reset state. */
 static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
 {
 	if (bypass_top_level)
@@ -516,9 +493,7 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
 	return 0;
 }
 
-/*
- * Quits GCB reset state.
- */
+/* Quit GCB reset state. */
 static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
 {
 	u32 val0, val1;
@@ -601,9 +576,7 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
 	return 0;
 }
 
-/*
- * Determines if GCB is in reset state.
- */
+/* Determine if GCB is in reset state. */
 static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
 {
 	u32 val = gasket_dev_read_32(
@@ -615,9 +588,6 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
 
 /*
  * Check permissions for Apex ioctls.
- * @file: File pointer from ioctl.
- * @cmd: ioctl command.
- *
  * Returns true if the current user may execute this ioctl, and false otherwise.
  */
 static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
@@ -625,9 +595,7 @@ static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
 	return !!(filp->f_mode & FMODE_WRITE);
 }
 
-/*
- * Apex-specific ioctl handler.
- */
+/* Apex-specific ioctl handler. */
 static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
 {
 	struct gasket_dev *gasket_dev = filp->private_data;
@@ -643,11 +611,7 @@ static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
 	}
 }
 
-/*
- * Gates or un-gates Apex clock.
- * @gasket_dev: Gasket device pointer.
- * @argp: User ioctl arg, pointer to a apex_gate_clock_ioctl struct.
- */
+/* Gates or un-gates Apex clock. */
 static long apex_clock_gating(struct gasket_dev *gasket_dev,
 			      struct apex_gate_clock_ioctl __user *argp)
 {
@@ -681,15 +645,7 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
 	return 0;
 }
 
-/*
- * Display driver sysfs entries.
- * @device: Kernel device structure.
- * @attr: Attribute to display.
- * @buf: Buffer to which to write output.
- *
- * Description: Looks up the driver data and file-specific attribute data (the
- * type of the attribute), then fills "buf" accordingly.
- */
+/* Display driver sysfs entries. */
 static ssize_t sysfs_show(
 	struct device *device, struct device_attribute *attr, char *buf)
 {
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 06/13] staging: gasket: core: simplify comments for static functions
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (4 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 05/13] staging: gasket: apex: simplify comments for static functions Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 07/13] staging: gasket: ioctl: " Todd Poynor
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line.  Remove extraneous text.

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

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 291cd6d074a2e..c00774059f9eb 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -199,11 +199,7 @@ MODULE_AUTHOR("Rob Springer <rspringer@google.com>");
 module_init(gasket_init);
 module_exit(gasket_exit);
 
-/*
- * Perform a standard Gasket callback.
- * @gasket_dev: Device specific pointer to forward.
- * @cb_function: Standard callback to perform.
- */
+/* Perform a standard Gasket callback. */
 static inline int check_and_invoke_callback(
 	struct gasket_dev *gasket_dev, int (*cb_function)(struct gasket_dev *))
 {
@@ -219,13 +215,7 @@ static inline int check_and_invoke_callback(
 	return ret;
 }
 
-/*
- * Perform a standard Gasket callback
- * without grabbing gasket_dev->mutex.
- * @gasket_dev: Device specific pointer to forward.
- * @cb_function: Standard callback to perform.
- *
- */
+/* Perform a standard Gasket callback without grabbing gasket_dev->mutex. */
 static inline int gasket_check_and_invoke_callback_nolock(
 	struct gasket_dev *gasket_dev, int (*cb_function)(struct gasket_dev *))
 {
@@ -240,9 +230,8 @@ static inline int gasket_check_and_invoke_callback_nolock(
 }
 
 /*
- * Returns nonzero if the gasket_cdev_info is owned by the current thread group
+ * Return nonzero if the gasket_cdev_info is owned by the current thread group
  * ID.
- * @info: Device node info.
  */
 static int gasket_owned_by_current_tgid(struct gasket_cdev_info *info)
 {
@@ -410,14 +399,9 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
 }
 EXPORT_SYMBOL(gasket_unregister_device);
 
-/**
- * Allocate a Gasket device.
- * @internal_desc: Pointer to the internal data for the device driver.
- * @pdev: Pointer to the Gasket device pointer, the allocated device.
- * @kobj_name: PCIe name for the device
- *
- * Description: Allocates and initializes a Gasket device structure.
- *              Adds the device to the device list.
+/*
+ * Allocate and initialize a Gasket device structure, add the device to the
+ * device list.
  *
  * Returns 0 if successful, a negative error code otherwise.
  */
@@ -476,13 +460,7 @@ static int gasket_alloc_dev(
 	return 0;
 }
 
-/*
- * Free a Gasket device.
- * @internal_dev: Gasket device pointer; the device to unregister and free.
- *
- * Description: Removes the device from the device list and frees
- *              the Gasket device structure.
- */
+/* Free a Gasket device. */
 static void gasket_free_dev(struct gasket_dev *gasket_dev)
 {
 	struct gasket_internal_desc *internal_desc = gasket_dev->internal_desc;
@@ -496,7 +474,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
 }
 
 /*
- * Finds the next free gasket_internal_dev slot.
+ * Find the next free gasket_internal_dev slot.
  *
  * Returns the located slot number on success or a negative number on failure.
  */
@@ -533,10 +511,8 @@ static int gasket_find_dev_slot(
 	return i;
 }
 
-/**
+/*
  * PCI subsystem probe function.
- * @pci_dev: PCI device pointer to the new device.
- * @id: PCI device id structure pointer, the vendor and device ids.
  *
  * 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.
@@ -641,7 +617,6 @@ static int gasket_pci_probe(
 
 /*
  * PCI subsystem remove function.
- * @pci_dev: PCI device pointer; the device to remove.
  *
  * Called to remove a Gasket device. Finds the device in the device list and
  * cleans up metadata.
@@ -694,8 +669,6 @@ static void gasket_pci_remove(struct pci_dev *pci_dev)
 
 /*
  * Setup PCI & set up memory mapping for the specified device.
- * @pci_dev: pointer to the particular PCI device.
- * @internal_dev: Corresponding Gasket device pointer.
  *
  * Enables the PCI device, reads the BAR registers and sets up pointers to the
  * device's memory mapped IO space.
@@ -746,8 +719,6 @@ static void gasket_cleanup_pci(struct gasket_dev *gasket_dev)
 
 /*
  * Maps the specified bar into kernel space.
- * @internal_dev: Device possessing the BAR to map.
- * @bar_num: The BAR to map.
  *
  * Returns 0 on success, a negative error code otherwise.
  * A zero-sized BAR will not be mapped, but is not an error.
@@ -824,7 +795,6 @@ static int gasket_map_pci_bar(struct gasket_dev *gasket_dev, int bar_num)
 
 /*
  * Releases PCI BAR mapping.
- * @internal_dev: Device possessing the BAR to unmap.
  *
  * A zero-sized or not-mapped BAR will not be unmapped, but is not an error.
  */
@@ -856,12 +826,7 @@ static void gasket_unmap_pci_bar(struct gasket_dev *dev, int bar_num)
 	release_mem_region(base, bytes);
 }
 
-/*
- * Handle adding a char device and related info.
- * @dev_info: Pointer to the dev_info struct for this device.
- * @file_ops: The file operations for this device.
- * @owner: The owning module for this device.
- */
+/* Add a char device and related info. */
 static int gasket_add_cdev(
 	struct gasket_cdev_info *dev_info,
 	const struct file_operations *file_ops, struct module *owner)
@@ -881,14 +846,7 @@ static int gasket_add_cdev(
 	return 0;
 }
 
-/*
- * Performs final init and marks the device as active.
- * @internal_desc: Pointer to Gasket [internal] driver descriptor structure.
- * @internal_dev: Pointer to Gasket [internal] device structure.
- *
- * Currently forwards all work to device-specific callback; a future phase will
- * extract elements of character device registration here.
- */
+/* 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)
@@ -966,13 +924,7 @@ static int gasket_enable_dev(
 	return 0;
 }
 
-/*
- * Disable device operations.
- * @gasket_dev: Pointer to Gasket device structure.
- *
- * Currently forwards all work to device-specific callback; a future phase will
- * extract elements of character device unregistration here.
- */
+/* Disable device operations. */
 static void gasket_disable_dev(struct gasket_dev *gasket_dev)
 {
 	const struct gasket_driver_desc *driver_desc =
@@ -997,7 +949,7 @@ static void gasket_disable_dev(struct gasket_dev *gasket_dev)
 	check_and_invoke_callback(gasket_dev, driver_desc->disable_dev_cb);
 }
 
-/**
+/*
  * Registered descriptor lookup.
  *
  * Precondition: Called with g_mutex held (to avoid a race on return).
@@ -1045,18 +997,15 @@ const char *gasket_num_name_lookup(
 }
 EXPORT_SYMBOL(gasket_num_name_lookup);
 
-/**
- * Opens the char device file.
- * @inode: Inode structure pointer of the device file.
- * @file: File structure pointer.
+/*
+ * Open the char device file.
  *
- * Description: Called on an open of the device file.  If the open is for
- *              writing, and the device is not owned, this process becomes
- *              the owner.  If the open is for writing and the device is
- *              already owned by some other process, it is an error.  If
- *              this process is the owner, increment the open count.
+ * If the open is for writing, and the device is not owned, this process becomes
+ * the owner.  If the open is for writing and the device is already owned by
+ * some other process, it is an error.  If this process is the owner, increment
+ * the open count.
  *
- *              Returns 0 if successful, a negative error number otherwise.
+ * Returns 0 if successful, a negative error number otherwise.
  */
 static int gasket_open(struct inode *inode, struct file *filp)
 {
@@ -1130,17 +1079,12 @@ static int gasket_open(struct inode *inode, struct file *filp)
 	return 0;
 }
 
-/**
- * gasket_release - Close of the char device file.
- * @inode: Inode structure pointer of the device file.
- * @file: File structure pointer.
- *
- * Description: Called on a close of the device file.  If this process
- *              is the owner, decrement the open count.  On last close
- *              by the owner, free up buffers and eventfd contexts, and
- *              release ownership.
+/*
+ * Called on a close of the device file.  If this process is the owner,
+ * decrement the open count.  On last close by the owner, free up buffers and
+ * eventfd contexts, and release ownership.
  *
- *              Returns 0 if successful, a negative error number otherwise.
+ * Returns 0 if successful, a negative error number otherwise.
  */
 static int gasket_release(struct inode *inode, struct file *file)
 {
@@ -1199,10 +1143,6 @@ static int gasket_release(struct inode *inode, struct file *file)
 }
 
 /*
- * Permission and validity checking for mmap ops.
- * @gasket_dev: Gasket device information structure.
- * @vma: Standard virtual memory area descriptor.
- *
  * Verifies that the user has permissions to perform the requested mapping and
  * that the provided descriptor/range is of adequate size to hold the range to
  * be mapped.
@@ -1246,11 +1186,6 @@ static bool gasket_mmap_has_permissions(
 }
 
 /*
- * Checks if an address is within the region
- * allocated for coherent buffer.
- * @driver_desc: driver description.
- * @address: offset of address to check.
- *
  * Verifies that the input address is within the region allocated to coherent
  * buffer.
  */
@@ -1502,11 +1437,7 @@ static int gasket_mm_vma_bar_offset(
 	return 0;
 }
 
-/*
- * Map a region of coherent memory.
- * @gasket_dev: Gasket device handle.
- * @vma: Virtual memory area descriptor with region to map.
- */
+/* Map a region of coherent memory. */
 static int gasket_mmap_coherent(
 	struct gasket_dev *gasket_dev, struct vm_area_struct *vma)
 {
@@ -1551,16 +1482,7 @@ static int gasket_mmap_coherent(
 	return 0;
 }
 
-/*
- * Maps a device's BARs into user space.
- * @filp: File structure pointer describing this node usage session.
- * @vma: Standard virtual memory area descriptor.
- *
- * Maps the entirety of each of the device's BAR ranges into the user memory
- * range specified by vma.
- *
- * Returns 0 on success, a negative errno on error.
- */
+/* Map a device's BARs into user space. */
 static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
 {
 	int i, ret;
@@ -1704,14 +1626,7 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
 	return ret;
 }
 
-/*
- * Determine the health of the Gasket device.
- * @gasket_dev: Gasket device structure.
- *
- * Checks the underlying device health (via the device_status_cb)
- * and the status of initialized Gasket code systems (currently
- * only interrupts), then returns a gasket_status appropriately.
- */
+/* Determine the health of the Gasket device. */
 static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
 {
 	int status;
@@ -1750,14 +1665,10 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
 
 /*
  * Gasket ioctl dispatch function.
- * @filp: File structure pointer describing this node usage session.
- * @cmd: ioctl number to handle.
- * @arg: ioctl-specific data pointer.
  *
- * First, checks if the ioctl is a generic ioctl. If not, it passes
- * the ioctl to the ioctl_handler_cb registered in the driver description.
- * If the ioctl is a generic ioctl, the function passes it to the
- * gasket_ioctl_handler in gasket_ioctl.c.
+ * Check if the ioctl is a generic ioctl. If not, pass the ioctl to the
+ * ioctl_handler_cb registered in the driver description.
+ * If the ioctl is a generic ioctl, pass it to gasket_ioctl_handler.
  */
 static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
 {
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 07/13] staging: gasket: ioctl: simplify comments for static functions
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (5 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 06/13] staging: gasket: core: " Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 08/13] staging: gasket: page table: " Todd Poynor
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line.  Remove extraneous text.

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

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 78a132a60cc4f..55bdd7bfac866 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -170,14 +170,7 @@ long gasket_is_supported_ioctl(uint cmd)
 	}
 }
 
-/*
- * Permission checker for Gasket ioctls.
- * @filp: File structure pointer describing this node usage session.
- * @cmd: ioctl number to handle.
- *
- * Check permissions for Gasket ioctls.
- * Returns true if the file opener may execute this ioctl, or false otherwise.
- */
+/* Check permissions for Gasket ioctls. */
 static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 {
 	bool alive;
@@ -218,11 +211,7 @@ static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 	return false; /* unknown permissions */
 }
 
-/*
- * Associate an eventfd with an interrupt.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_interrupt_eventfd struct in userspace.
- */
+/* Associate an eventfd with an interrupt. */
 static int gasket_set_event_fd(struct gasket_dev *gasket_dev,
 			       struct gasket_interrupt_eventfd __user *argp)
 {
@@ -237,11 +226,7 @@ static int gasket_set_event_fd(struct gasket_dev *gasket_dev,
 		gasket_dev->interrupt_data, die.interrupt, die.event_fd);
 }
 
-/*
- * Reads the size of the page table.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
- */
+/* Read the size of the page table. */
 static int gasket_read_page_table_size(
 	struct gasket_dev *gasket_dev,
 	struct gasket_page_table_ioctl __user *argp)
@@ -268,11 +253,7 @@ static int gasket_read_page_table_size(
 	return ret;
 }
 
-/*
- * Reads the size of the simple page table.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
- */
+/* Read the size of the simple page table. */
 static int gasket_read_simple_page_table_size(
 	struct gasket_dev *gasket_dev,
 	struct gasket_page_table_ioctl __user *argp)
@@ -299,11 +280,7 @@ static int gasket_read_simple_page_table_size(
 	return ret;
 }
 
-/*
- * Sets the boundary between the simple and extended page tables.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
- */
+/* Set the boundary between the simple and extended page tables. */
 static int gasket_partition_page_table(
 	struct gasket_dev *gasket_dev,
 	struct gasket_page_table_ioctl __user *argp)
@@ -340,11 +317,7 @@ static int gasket_partition_page_table(
 	return ret;
 }
 
-/*
- * Maps a userspace buffer to a device virtual address.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
- */
+/* Map a userspace buffer to a device virtual address. */
 static int gasket_map_buffers(struct gasket_dev *gasket_dev,
 			      struct gasket_page_table_ioctl __user *argp)
 {
@@ -370,11 +343,7 @@ static int gasket_map_buffers(struct gasket_dev *gasket_dev,
 		ibuf.host_address, ibuf.device_address, ibuf.size / PAGE_SIZE);
 }
 
-/*
- * Unmaps a userspace buffer from a device virtual address.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
- */
+/* Unmap a userspace buffer from a device virtual address. */
 static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
 				struct gasket_page_table_ioctl __user *argp)
 {
@@ -402,10 +371,8 @@ static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
 }
 
 /*
- * Tell the driver to reserve structures for coherent allocation, and allocate
- * or free the corresponding memory.
- * @dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to a gasket_coherent_alloc_config_ioctl struct in userspace.
+ * Reserve structures for coherent allocation, and allocate or free the
+ * corresponding memory.
  */
 static int gasket_config_coherent_allocator(
 	struct gasket_dev *gasket_dev,
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 08/13] staging: gasket: page table: simplify comments for static functions
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (6 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 07/13] staging: gasket: ioctl: " Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 09/13] staging: gasket: interrupt: " Todd Poynor
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line.  Remove extraneous text.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 6b946a155ee3a..b42f6637b909b 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -635,27 +635,9 @@ int gasket_page_table_system_status(struct gasket_page_table *page_table)
 	return GASKET_STATUS_ALIVE;
 }
 
-/* Internal functions */
-
-/* Mapping functions */
 /*
  * Allocate and map pages to simple addresses.
- * @pg_tbl: Gasket page table pointer.
- * @host_addr: Starting host virtual memory address of the pages.
- * @dev_addr: Starting device address of the pages.
- * @cnt: Count of the number of device pages to map.
- *
- * Description: gasket_map_simple_pages calls gasket_simple_alloc_pages() to
- *		allocate the page table slots, then calls
- *		gasket_perform_mapping() to actually do the work of mapping the
- *		pages into the the simple page table (device translation table
- *		registers).
- *
- *		The sd_mutex must be held when gasket_map_simple_pages() is
- *		called.
- *
- *		Returns 0 if successful or a non-zero error number otherwise.
- *		If there is an error, no pages are mapped.
+ * If there is an error, no pages are mapped.
  */
 static int gasket_map_simple_pages(
 	struct gasket_page_table *pg_tbl, ulong host_addr, ulong dev_addr,
@@ -685,22 +667,7 @@ static int gasket_map_simple_pages(
 
 /*
  * gasket_map_extended_pages - Get and map buffers to extended addresses.
- * @pg_tbl: Gasket page table pointer.
- * @host_addr: Starting host virtual memory address of the pages.
- * @dev_addr: Starting device address of the pages.
- * @num_pages: The number of device pages to map.
- *
- * Description: gasket_map_extended_buffers calls
- *		gasket_alloc_extended_entries() to allocate the page table
- *		slots, then loops over the level 0 page table entries, and for
- *		each calls gasket_perform_mapping() to map the buffers into the
- *		level 1 page table for that level 0 entry.
- *
- *		The page table mutex must be held when
- *		gasket_map_extended_pages() is called.
- *
- *		Returns 0 if successful or a non-zero error number otherwise.
- *		If there is an error, no pages are mapped.
+ * If there is an error, no pages are mapped.
  */
 static int gasket_map_extended_pages(
 	struct gasket_page_table *pg_tbl, ulong host_addr, ulong dev_addr,
@@ -756,32 +723,11 @@ static int gasket_map_extended_pages(
 
 /*
  * Get and map last level page table buffers.
- * @pg_tbl: Gasket page table pointer.
- * @ptes: Array of page table entries to describe this mapping, one per
- *        page to map.
- * @slots: Location(s) to write device-mapped page address. If this is a simple
- *	   mapping, these will be address translation registers. If this is
- *	   an extended mapping, these will be within a second-level page table
- *	   allocated by the host and so must have their __iomem attribute
- *	   casted away.
- * @host_addr: Starting [host] virtual memory address of the buffers.
- * @num_pages: The number of device pages to map.
- * @is_simple_mapping: 1 if this is a simple mapping, 0 otherwise.
- *
- * Description: gasket_perform_mapping calls get_user_pages() to get pages
- *		of user memory and pin them.  It then calls dma_map_page() to
- *		map them for DMA.  Finally, the mapped DMA addresses are written
- *		into the page table.
  *
- *		This function expects that the page table entries are
- *		already allocated.  The level argument determines how the
- *		final page table entries are written: either into PCIe memory
- *		mapped space for a level 0 page table or into kernel memory
- *		for a level 1 page table.
- *
- *		The page pointers are saved for later releasing the pages.
- *
- *		Returns 0 if successful or a non-zero error number otherwise.
+ * slots is the location(s) to write device-mapped page address. If this is a
+ * simple mapping, these will be address translation registers. If this is
+ * an extended mapping, these will be within a second-level page table
+ * allocated by the host and so must have their __iomem attribute casted away.
  */
 static int gasket_perform_mapping(
 	struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *ptes,
@@ -866,21 +812,9 @@ static int gasket_perform_mapping(
 	return 0;
 }
 
-/**
+/*
  * Allocate page table entries in a simple table.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address for the (eventual) mappings.
- * @num_pages: Count of pages to be mapped.
- *
- * Description: gasket_alloc_simple_entries checks to see if a range of page
- *		table slots are available.  As long as the sd_mutex is
- *		held, the slots will be available.
- *
- *		The page table mutex must be held when
- *		gasket_alloc_simple entries() is called.
- *
- *		Returns 0 if successful, or non-zero if the requested device
- *		addresses are not available.
+ * The page table mutex must be held by the caller.
  */
 static int gasket_alloc_simple_entries(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -893,29 +827,19 @@ static int gasket_alloc_simple_entries(
 	return 0;
 }
 
-/**
- * Allocate slots in an extended page table.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address for the (eventual) mappings.
- * @num_pages: Count of pages to be mapped.
- *
- * Description: gasket_alloc_extended_entries checks to see if a range of page
- *		table slots are available. If necessary, memory is allocated for
- *		second level page tables.
- *
- *		Note that memory for second level page tables is allocated
- *		as needed, but that memory is only freed on the final close
- *		of the device file, when the page tables are repartitioned,
- *		or the the device is removed.  If there is an error or if
- *		the full range of slots is not available, any memory
- *		allocated for second level page tables remains allocated
- *		until final close, repartition, or device removal.
+/*
+ * Allocate slots in an extended page table.  Check to see if a range of page
+ * table slots are available. If necessary, memory is allocated for second level
+ * page tables.
  *
- *		The page table mutex must be held when
- *		gasket_alloc_extended_entries() is called.
+ * Note that memory for second level page tables is allocated as needed, but
+ * that memory is only freed on the final close	of the device file, when the
+ * page tables are repartitioned, or the the device is removed.  If there is an
+ * error or if the full range of slots is not available, any memory
+ * allocated for second level page tables remains allocated until final close,
+ * repartition, or device removal.
  *
- *		Returns 0 if successful, or non-zero if the slots are
- *		not available.
+ * The page table mutex must be held by the caller.
  */
 static int gasket_alloc_extended_entries(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_entries)
@@ -958,21 +882,9 @@ static int gasket_alloc_extended_entries(
 	return 0;
 }
 
-/**
+/*
  * Allocate a second level page table.
- * @pg_tbl: Gasket page table pointer.
- * @pte: Extended page table entry under/for which to allocate a second level.
- * @slot: [Device] slot corresponding to pte.
- *
- * Description: Allocate the memory for a second level page table (subtable) at
- *	        the given level 0 entry.  Then call dma_map_page() to map the
- *		second level page table for DMA.  Finally, write the
- *		mapped DMA address into the device page table.
- *
- *		The page table mutex must be held when
- *		gasket_alloc_extended_subtable() is called.
- *
- *		Returns 0 if successful, or a non-zero error otherwise.
+ * The page table mutex must be held by the caller.
  */
 static int gasket_alloc_extended_subtable(
 	struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *pte,
@@ -1017,15 +929,9 @@ static int gasket_alloc_extended_subtable(
 	return 0;
 }
 
-/* Unmapping functions */
 /*
  * Non-locking entry to unmapping routines.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: Starting device address of the pages to unmap.
- * @num_pages: The number of device pages to unmap.
- *
- * Description: Version of gasket_unmap_pages that assumes the page table lock
- *              is held.
+ * The page table mutex must be held by the caller.
  */
 static void gasket_page_table_unmap_nolock(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1041,14 +947,7 @@ static void gasket_page_table_unmap_nolock(
 
 /*
  * Unmap and release pages mapped to simple addresses.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address of the buffers.
- * @num_pages: The number of device pages to unmap.
- *
- * Description: gasket_simple_unmap_pages calls gasket_perform_unmapping() to
- * unmap and release the buffers in the level 0 page table.
- *
- * The sd_mutex must be held when gasket_unmap_simple_pages() is called.
+ * The page table mutex must be held by the caller.
  */
 static void gasket_unmap_simple_pages(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1059,20 +958,9 @@ static void gasket_unmap_simple_pages(
 				 pg_tbl->base_slot + slot, num_pages, 1);
 }
 
-/**
+/*
  * Unmap and release buffers to extended addresses.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address of the pages to unmap.
- * @addr: Starting device address of the buffers.
- * @num_pages: The number of device pages to unmap.
- *
- * Description: gasket_extended_unmap_pages loops over the level 0 page table
- *		entries, and for each calls gasket_perform_unmapping() to unmap
- *		the buffers from the level 1 page [sub]table for that level 0
- *		entry.
- *
- *		The page table mutex must be held when
- *		gasket_unmap_extended_pages() is called.
+ * The page table mutex must be held by the caller.
  */
 static void gasket_unmap_extended_pages(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1106,28 +994,7 @@ static void gasket_unmap_extended_pages(
 
 /*
  * Unmap and release mapped pages.
- * @pg_tbl: Gasket page table pointer.
- * @ptes: Array of page table entries to describe the mapped range, one per
- *        page to unmap.
- * @slots: Device slots corresponding to the mappings described by "ptes".
- *         As with ptes, one element per page to unmap.
- *         If these are simple mappings, these will be address translation
- *         registers. If these are extended mappings, these will be witin a
- *         second-level page table allocated on the host, and so must have
- *	   their __iomem attribute casted away.
- * @num_pages: Number of pages to unmap.
- * @is_simple_mapping: 1 if this is a simple mapping, 0 otherwise.
- *
- * Description: gasket_perform_unmapping() loops through the metadata entries
- *		in a last level page table (simple table or extended subtable),
- *		and for each page:
- *		 - Unmaps the page from DMA space (dma_unmap_page),
- *		 - Returns the page to the OS (gasket_release_page),
- *		The entry in the page table is written to 0. The metadata
- *		type is set to PTE_FREE and the metadata is all reset
- *		to 0.
- *
- *		The page table mutex must be held when this function is called.
+ * The page table mutex must be held by the caller.
  */
 static void gasket_perform_unmapping(
 	struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *ptes,
@@ -1165,17 +1032,6 @@ static void gasket_perform_unmapping(
 
 /*
  * Free a second level page [sub]table.
- * @pg_tbl: Gasket page table pointer.
- * @pte: Page table entry _pointing_to_ the subtable to free.
- * @slot: Device slot holding a pointer to the sublevel's contents.
- *
- * Description: Safely deallocates a second-level [sub]table by:
- *  - Marking the containing first-level PTE as free
- *  - Setting the corresponding [extended] device slot as NULL
- *  - Unmapping the PTE from DMA space.
- *  - Freeing the subtable's memory.
- *  - Deallocating the page and clearing out the PTE.
- *
  * The page table mutex must be held before this call.
  */
 static void gasket_free_extended_subtable(
@@ -1202,12 +1058,7 @@ static void gasket_free_extended_subtable(
 	memset(pte, 0, sizeof(struct gasket_page_table_entry));
 }
 
-/*
- * Safely return a page to the OS.
- * @page: The page to return to the OS.
- * Returns true if the page was released, false if it was
- * ignored.
- */
+/* Safely return a page to the OS. */
 static bool gasket_release_page(struct page *page)
 {
 	if (!page)
@@ -1229,13 +1080,10 @@ static inline bool gasket_addr_is_simple(
 
 /*
  * Validity checking for simple addresses.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: The device address to which the pages will be mapped.
- * @num_pages: The number of pages in the range to consider.
  *
- * Description: This call verifies that address translation commutes (from
- * address to/from page + offset) and that the requested page range starts and
- * ends within the set of currently-partitioned simple pages.
+ * Verify that address translation commutes (from address to/from page + offset)
+ * and that the requested page range starts and ends within the set of
+ * currently-partitioned simple pages.
  */
 static bool gasket_is_simple_dev_addr_bad(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1269,13 +1117,11 @@ static bool gasket_is_simple_dev_addr_bad(
 }
 
 /*
- * Verifies that address translation commutes (from address to/from page +
- * offset) and that the requested page range starts and ends within the set of
- * currently-partitioned simple pages.
+ * Validity checking for extended addresses.
  *
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: The device address to which the pages will be mapped.
- * @num_pages: The number of second-level/sub pages in the range to consider.
+ * Verify that address translation commutes (from address to/from page +
+ * offset) and that the requested page range starts and ends within the set of
+ * currently-partitioned extended pages.
  */
 static bool gasket_is_extended_dev_addr_bad(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1331,15 +1177,8 @@ static bool gasket_is_extended_dev_addr_bad(
 }
 
 /*
- * Checks if a range of PTEs is free.
- * @ptes: The set of PTEs to check.
- * @num_entries: The number of PTEs to check.
- *
- * Description: Iterates over the input PTEs to determine if all have been
- * marked as FREE or if any are INUSE. In the former case, 1/true is returned.
- * Otherwise, 0/false is returned.
- *
- * The page table mutex must be held before this call.
+ * Check if a range of PTEs is free.
+ * The page table mutex must be held by the caller.
  */
 static bool gasket_is_pte_range_free(
 	struct gasket_page_table_entry *ptes, uint num_entries)
@@ -1356,10 +1195,7 @@ static bool gasket_is_pte_range_free(
 
 /*
  * Actually perform collection.
- * @pg_tbl: Gasket page table structure.
- *
- * Description: Version of gasket_page_table_garbage_collect that assumes the
- *		page table lock is held.
+ * The page table mutex must be held by the caller.
  */
 static void gasket_page_table_garbage_collect_nolock(
 	struct gasket_page_table *pg_tbl)
@@ -1384,14 +1220,7 @@ static void gasket_page_table_garbage_collect_nolock(
 }
 
 /*
- * Converts components to a device address.
- * @pg_tbl: Gasket page table structure.
- * @is_simple: nonzero if this should be a simple entry, zero otherwise.
- * @page_index: The page index into the respective table.
- * @offset: The offset within the requested page.
- *
- * Simple utility function to convert (simple, page, offset) into a device
- * address.
+ * Convert (simple, page, offset) into a device address.
  * Examples:
  * Simple page 0, offset 32:
  *  Input (0, 0, 32), Output 0x20
@@ -1429,14 +1258,7 @@ static ulong gasket_components_to_dev_address(
 }
 
 /*
- * Gets the index of the address' page in the simple table.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: The address whose page index to retrieve.
- *
- * Description: Treats the input address as a simple address and determines the
- * index of its underlying page in the simple page table (i.e., device address
- * translation registers.
- *
+ * Return the index of the page for the address in the simple table.
  * Does not perform validity checking.
  */
 static int gasket_simple_page_idx(
@@ -1447,14 +1269,7 @@ static int gasket_simple_page_idx(
 }
 
 /*
- * Gets the level 0 page index for the given address.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: The address whose page index to retrieve.
- *
- * Description: Treats the input address as an extended address and determines
- * the index of its underlying page in the first-level extended page table
- * (i.e., device extended address translation registers).
- *
+ * Return the level 0 page index for the given address.
  * Does not perform validity checking.
  */
 static ulong gasket_extended_lvl0_page_idx(
@@ -1465,14 +1280,7 @@ static ulong gasket_extended_lvl0_page_idx(
 }
 
 /*
- * Gets the level 1 page index for the given address.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: The address whose page index to retrieve.
- *
- * Description: Treats the input address as an extended address and determines
- * the index of its underlying page in the second-level extended page table
- * (i.e., host memory pointed to by a first-level page table entry).
- *
+ * Return the level 1 page index for the given address.
  * Does not perform validity checking.
  */
 static ulong gasket_extended_lvl1_page_idx(
@@ -1483,13 +1291,10 @@ static ulong gasket_extended_lvl1_page_idx(
 }
 
 /*
- * Determines whether a host buffer was mapped as coherent memory.
- * @pg_tbl: gasket_page_table structure tracking the host buffer mapping
- * @host_addr: user virtual address within a host buffer
+ * Return whether a host buffer was mapped as coherent memory.
  *
- * Description: A Gasket page_table currently support one contiguous
- * dma range, mapped to one contiguous virtual memory range. Check if the
- * host_addr is within start of page 0, and end of last page, for that range.
+ * A Gasket page_table currently support one contiguous dma range, mapped to one
+ * contiguous virtual memory range. Check if the host_addr is within that range.
  */
 static int is_coherent(struct gasket_page_table *pg_tbl, ulong host_addr)
 {
@@ -1505,16 +1310,7 @@ static int is_coherent(struct gasket_page_table *pg_tbl, ulong host_addr)
 	return min <= host_addr && host_addr < max;
 }
 
-/*
- * Records the host_addr to coherent dma memory mapping.
- * @gasket_dev: Gasket Device.
- * @size: Size of the virtual address range to map.
- * @dma_address: Dma address within the coherent memory range.
- * @vma: Virtual address we wish to map to coherent memory.
- *
- * Description: For each page in the virtual address range, record the
- * coherent page mgasket_pretapping.
- */
+/* Record the host_addr to coherent dma memory mapping. */
 int gasket_set_user_virt(
 	struct gasket_dev *gasket_dev, u64 size, dma_addr_t dma_address,
 	ulong vma)
@@ -1541,16 +1337,7 @@ int gasket_set_user_virt(
 	return 0;
 }
 
-/*
- * Allocate a block of coherent memory.
- * @gasket_dev: Gasket Device.
- * @size: Size of the memory block.
- * @dma_address: Dma address allocated by the kernel.
- * @index: Index of the gasket_page_table within this Gasket device
- *
- * Description: Allocate a contiguous coherent memory block, DMA'ble
- * by this device.
- */
+/* Allocate a block of coherent memory. */
 int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 				 dma_addr_t *dma_address, u64 index)
 {
@@ -1613,15 +1400,7 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 	return -ENOMEM;
 }
 
-/*
- * Free a block of coherent memory.
- * @gasket_dev: Gasket Device.
- * @size: Size of the memory block.
- * @dma_address: Dma address allocated by the kernel.
- * @index: Index of the gasket_page_table within this Gasket device
- *
- * Description: Release memory allocated thru gasket_alloc_coherent_memory.
- */
+/* Free a block of coherent memory. */
 int gasket_free_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 				dma_addr_t dma_address, u64 index)
 {
@@ -1647,13 +1426,7 @@ int gasket_free_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 	return 0;
 }
 
-/*
- * Release all coherent memory.
- * @gasket_dev: Gasket Device.
- * @index: Index of the gasket_page_table within this Gasket device
- *
- * Description: Release all memory allocated thru gasket_alloc_coherent_memory.
- */
+/* Release all coherent memory. */
 void gasket_free_coherent_memory_all(
 	struct gasket_dev *gasket_dev, u64 index)
 {
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 09/13] staging: gasket: interrupt: simplify comments for static functions
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (7 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 08/13] staging: gasket: page table: " Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 10/13] staging: gasket: sysfs: " Todd Poynor
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line.  Remove extraneous text.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_interrupt.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index 3be8e24c126d0..27fde991edc6b 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -87,13 +87,6 @@ static struct gasket_sysfs_attribute interrupt_sysfs_attrs[] = {
 	GASKET_END_OF_ATTR_ARRAY,
 };
 
-/*
- * Set up device registers for interrupt handling.
- * @gasket_dev: The Gasket information structure for this device.
- *
- * Sets up the device registers with the correct indices for the relevant
- * interrupts.
- */
 static void gasket_interrupt_setup(struct gasket_dev *gasket_dev);
 
 /* MSIX init and cleanup. */
@@ -334,13 +327,7 @@ int gasket_interrupt_reset_counts(struct gasket_dev *gasket_dev)
 	return 0;
 }
 
-/*
- * Set up device registers for interrupt handling.
- * @gasket_dev: The Gasket information structure for this device.
- *
- * Sets up the device registers with the correct indices for the relevant
- * interrupts.
- */
+/* Set up device registers for interrupt handling. */
 static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
 {
 	int i;
@@ -553,9 +540,6 @@ static ssize_t interrupt_sysfs_show(
 	return ret;
 }
 
-/*
- * MSIX interrupt handler, used with PCI driver.
- */
 static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id)
 {
 	struct eventfd_ctx *ctx;
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 10/13] staging: gasket: sysfs: simplify comments for static functions
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (8 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 09/13] staging: gasket: interrupt: " Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 11/13] staging: gasket: TODO: remove entry for static function kernel docs Todd Poynor
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line.  Remove extraneous text.

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

diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
index fde04658419bc..ef4eca02afa63 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -47,22 +47,13 @@ struct gasket_sysfs_mapping {
  */
 static struct gasket_sysfs_mapping dev_mappings[GASKET_SYSFS_NUM_MAPPINGS];
 
-/*
- * Callback when a mapping's refcount goes to zero.
- * @ref: The reference count of the containing sysfs mapping.
- */
+/* Callback when a mapping's refcount goes to zero. */
 static void release_entry(struct kref *ref)
 {
 	/* All work is done after the return from kref_put. */
 }
 
-/*
- * Looks up mapping information for the given device.
- * @device: The device whose mapping to look for.
- *
- * Looks up the requested device and takes a reference and returns it if found,
- * and returns NULL otherwise.
- */
+/* Look up mapping information for the given device. */
 static struct gasket_sysfs_mapping *get_mapping(struct device *device)
 {
 	int i;
@@ -82,17 +73,7 @@ static struct gasket_sysfs_mapping *get_mapping(struct device *device)
 	return NULL;
 }
 
-/*
- * Returns a reference to a mapping.
- * @mapping: The mapping we're returning.
- *
- * Decrements the refcount for the given mapping (if valid). If the refcount is
- * zero, then it cleans up the mapping - in this function as opposed to the
- * kref_put callback, due to a potential deadlock.
- *
- * Although put_mapping_n exists, this function is left here (as an implicit
- * put_mapping_n(..., 1) for convenience.
- */
+/* Put a reference to a mapping. */
 static void put_mapping(struct gasket_sysfs_mapping *mapping)
 {
 	int i;
@@ -140,8 +121,7 @@ static void put_mapping(struct gasket_sysfs_mapping *mapping)
 }
 
 /*
- * Returns a reference N times.
- * @mapping: The mapping to return.
+ * Put a reference to a mapping N times.
  *
  * In higher-level resource acquire/release function pairs, the release function
  * will need to release a mapping 2x - once for the refcount taken in the
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 11/13] staging: gasket: TODO: remove entry for static function kernel docs
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (9 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 10/13] staging: gasket: sysfs: " Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 12/13] staging: gasket: apex: remove static function forward declarations Todd Poynor
  2018-07-29 19:36 ` [PATCH 13/13] staging: gasket: apex: fix function param line continuation style Todd Poynor
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Remove the TODO entry for simplifying kernel doc style comments for
static functions, now that this has been addressed.

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

diff --git a/drivers/staging/gasket/TODO b/drivers/staging/gasket/TODO
index fb71997fb5612..7f4c13ce021ba 100644
--- a/drivers/staging/gasket/TODO
+++ b/drivers/staging/gasket/TODO
@@ -5,7 +5,6 @@ staging directory.
 - Use misc interface instead of major number for driver version description.
 - Add descriptions of module_param's
 - apex_get_status() should actually check status.
-- Static functions don't need kernel doc formatting, can be simplified.
 - Fix multi-line alignment formatting to look like:
       int ret = long_function_name(device, VARIABLE1, VARIABLE2,
                                    VARIABLE3, VARIABLE4);
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 12/13] staging: gasket: apex: remove static function forward declarations
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (10 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 11/13] staging: gasket: TODO: remove entry for static function kernel docs Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  2018-07-29 19:36 ` [PATCH 13/13] staging: gasket: apex: fix function param line continuation style Todd Poynor
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Remove forward declarations of static functions, move code to avoid
forward references, for kernel style.

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

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index a756764751777..f70fea0d80ecf 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -123,55 +123,6 @@ static struct gasket_page_table_config apex_page_table_configs[NUM_NODES] = {
 	},
 };
 
-/* Function declarations */
-static int __init apex_init(void);
-static void apex_exit(void);
-
-static int apex_add_dev_cb(struct gasket_dev *gasket_dev);
-
-static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev);
-
-static int apex_device_cleanup(struct gasket_dev *gasket_dev);
-
-static int apex_device_open_cb(struct gasket_dev *gasket_dev);
-
-static ssize_t sysfs_show(
-	struct device *device, struct device_attribute *attr, char *buf);
-
-static int apex_reset(struct gasket_dev *gasket_dev, uint type);
-
-static int apex_get_status(struct gasket_dev *gasket_dev);
-
-static bool apex_ioctl_check_permissions(struct file *file, uint cmd);
-
-static long apex_ioctl(struct file *file, uint cmd, void __user *argp);
-
-static long apex_clock_gating(struct gasket_dev *gasket_dev,
-			      struct apex_gate_clock_ioctl __user *argp);
-
-static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type);
-
-static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type);
-
-static bool is_gcb_in_reset(struct gasket_dev *gasket_dev);
-
-/* Data definitions */
-
-/* The data necessary to display this file's sysfs entries. */
-static struct gasket_sysfs_attribute apex_sysfs_attrs[] = {
-	GASKET_SYSFS_RO(node_0_page_table_entries, sysfs_show,
-			ATTR_KERNEL_HIB_PAGE_TABLE_SIZE),
-	GASKET_SYSFS_RO(node_0_simple_page_table_entries, sysfs_show,
-			ATTR_KERNEL_HIB_SIMPLE_PAGE_TABLE_SIZE),
-	GASKET_SYSFS_RO(node_0_num_mapped_pages, sysfs_show,
-			ATTR_KERNEL_HIB_NUM_ACTIVE_PAGES),
-	GASKET_END_OF_ATTR_ARRAY
-};
-
-static const struct pci_device_id apex_pci_ids[] = {
-	{ PCI_DEVICE(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID) }, { 0 }
-};
-
 /* The regions in the BAR2 space that can be mapped into user space. */
 static const struct gasket_mappable_region mappable_regions[NUM_REGIONS] = {
 	{ 0x40000, 0x1000 },
@@ -251,65 +202,6 @@ static struct gasket_interrupt_desc apex_interrupts[] = {
 	},
 };
 
-static struct gasket_driver_desc apex_desc = {
-	.name = "apex",
-	.driver_version = APEX_DRIVER_VERSION,
-	.major = 120,
-	.minor = 0,
-	.module = THIS_MODULE,
-	.pci_id_table = apex_pci_ids,
-
-	.num_page_tables = NUM_NODES,
-	.page_table_bar_index = APEX_BAR_INDEX,
-	.page_table_configs = apex_page_table_configs,
-	.page_table_extended_bit = APEX_EXTENDED_SHIFT,
-
-	.bar_descriptions = {
-		GASKET_UNUSED_BAR,
-		GASKET_UNUSED_BAR,
-		{ APEX_BAR_BYTES, (VM_WRITE | VM_READ), APEX_BAR_OFFSET,
-			NUM_REGIONS, mappable_regions, PCI_BAR },
-		GASKET_UNUSED_BAR,
-		GASKET_UNUSED_BAR,
-		GASKET_UNUSED_BAR,
-	},
-	.coherent_buffer_description = {
-		APEX_CH_MEM_BYTES,
-		(VM_WRITE | VM_READ),
-		APEX_CM_OFFSET,
-	},
-	.interrupt_type = PCI_MSIX,
-	.interrupt_bar_index = APEX_BAR_INDEX,
-	.num_interrupts = APEX_INTERRUPT_COUNT,
-	.interrupts = apex_interrupts,
-	.interrupt_pack_width = 7,
-
-	.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,
-
-	.device_open_cb = apex_device_open_cb,
-	.device_close_cb = apex_device_cleanup,
-
-	.ioctl_handler_cb = apex_ioctl,
-	.device_status_cb = apex_get_status,
-	.hardware_revision_cb = NULL,
-	.device_reset_cb = apex_reset,
-};
-
-/* Module registration boilerplate */
-MODULE_DESCRIPTION("Google Apex driver");
-MODULE_VERSION(APEX_DRIVER_VERSION);
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("John Joseph <jnjoseph@google.com>");
-MODULE_DEVICE_TABLE(pci, apex_pci_ids);
-module_init(apex_init);
-module_exit(apex_exit);
 
 /* Allows device to enter power save upon driver close(). */
 static int allow_power_save;
@@ -329,61 +221,6 @@ module_param(allow_sw_clock_gating, int, 0644);
 module_param(allow_hw_clock_gating, int, 0644);
 module_param(bypass_top_level, int, 0644);
 
-static int __init apex_init(void)
-{
-	return gasket_register_device(&apex_desc);
-}
-
-static void apex_exit(void)
-{
-	gasket_unregister_device(&apex_desc);
-}
-
-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, 0);
-
-	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;
-}
-
-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)
-{
-	return gasket_reset_nolock(gasket_dev, APEX_CHIP_REINIT_RESET);
-}
-
 /* Check the device status registers and return device status ALIVE or DEAD. */
 static int apex_get_status(struct gasket_dev *gasket_dev)
 {
@@ -391,53 +228,6 @@ static int apex_get_status(struct gasket_dev *gasket_dev)
 	return GASKET_STATUS_ALIVE;
 }
 
-/* Reset the Apex hardware. Called on final close via device_close_cb. */
-static int apex_device_cleanup(struct gasket_dev *gasket_dev)
-{
-	u64 scalar_error;
-	u64 hib_error;
-	int ret = 0;
-
-	hib_error = gasket_dev_read_64(
-		gasket_dev, APEX_BAR_INDEX,
-		APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
-	scalar_error = gasket_dev_read_64(
-		gasket_dev, APEX_BAR_INDEX,
-		APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
-
-	dev_dbg(gasket_dev->dev,
-		"%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
-		__func__, gasket_dev, hib_error, scalar_error);
-
-	if (allow_power_save)
-		ret = apex_enter_reset(gasket_dev, APEX_CHIP_REINIT_RESET);
-
-	return ret;
-}
-
-/* Reset the hardware, then quit reset.  Called on device open. */
-static int apex_reset(struct gasket_dev *gasket_dev, uint type)
-{
-	int ret;
-
-	if (bypass_top_level)
-		return 0;
-
-	if (!is_gcb_in_reset(gasket_dev)) {
-		/* We are not in reset - toggle the reset bit so as to force
-		 * re-init of custom block
-		 */
-		dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__);
-
-		ret = apex_enter_reset(gasket_dev, type);
-		if (ret)
-			return ret;
-	}
-	ret = apex_quit_reset(gasket_dev, type);
-
-	return ret;
-}
-
 /* Enter GCB reset state. */
 static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
 {
@@ -576,6 +366,30 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
 	return 0;
 }
 
+/* Reset the Apex hardware. Called on final close via device_close_cb. */
+static int apex_device_cleanup(struct gasket_dev *gasket_dev)
+{
+	u64 scalar_error;
+	u64 hib_error;
+	int ret = 0;
+
+	hib_error = gasket_dev_read_64(
+		gasket_dev, APEX_BAR_INDEX,
+		APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
+	scalar_error = gasket_dev_read_64(
+		gasket_dev, APEX_BAR_INDEX,
+		APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
+
+	dev_dbg(gasket_dev->dev,
+		"%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
+		__func__, gasket_dev, hib_error, scalar_error);
+
+	if (allow_power_save)
+		ret = apex_enter_reset(gasket_dev, APEX_CHIP_REINIT_RESET);
+
+	return ret;
+}
+
 /* Determine if GCB is in reset state. */
 static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
 {
@@ -586,29 +400,69 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
 	return (val & SCU3_CUR_RST_GCB_BIT_MASK);
 }
 
-/*
- * Check permissions for Apex ioctls.
- * Returns true if the current user may execute this ioctl, and false otherwise.
- */
-static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
+/* Reset the hardware, then quit reset.  Called on device open. */
+static int apex_reset(struct gasket_dev *gasket_dev, uint type)
 {
-	return !!(filp->f_mode & FMODE_WRITE);
+	int ret;
+
+	if (bypass_top_level)
+		return 0;
+
+	if (!is_gcb_in_reset(gasket_dev)) {
+		/* We are not in reset - toggle the reset bit so as to force
+		 * re-init of custom block
+		 */
+		dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__);
+
+		ret = apex_enter_reset(gasket_dev, type);
+		if (ret)
+			return ret;
+	}
+	ret = apex_quit_reset(gasket_dev, type);
+
+	return ret;
 }
 
-/* Apex-specific ioctl handler. */
-static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
+static int apex_add_dev_cb(struct gasket_dev *gasket_dev)
 {
-	struct gasket_dev *gasket_dev = filp->private_data;
+	ulong page_table_ready, msix_table_ready;
+	int retries = 0;
 
-	if (!apex_ioctl_check_permissions(filp, cmd))
-		return -EPERM;
+	apex_reset(gasket_dev, 0);
 
-	switch (cmd) {
-	case APEX_IOCTL_GATE_CLOCK:
-		return apex_clock_gating(gasket_dev, argp);
-	default:
-		return -ENOTTY; /* unknown command */
+	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.
+ */
+static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
+{
+	return !!(filp->f_mode & FMODE_WRITE);
 }
 
 /* Gates or un-gates Apex clock. */
@@ -645,6 +499,22 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
 	return 0;
 }
 
+/* Apex-specific ioctl handler. */
+static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
+{
+	struct gasket_dev *gasket_dev = filp->private_data;
+
+	if (!apex_ioctl_check_permissions(filp, cmd))
+		return -EPERM;
+
+	switch (cmd) {
+	case APEX_IOCTL_GATE_CLOCK:
+		return apex_clock_gating(gasket_dev, argp);
+	default:
+		return -ENOTTY; /* unknown command */
+	}
+}
+
 /* Display driver sysfs entries. */
 static ssize_t sysfs_show(
 	struct device *device, struct device_attribute *attr, char *buf)
@@ -696,9 +566,103 @@ static ssize_t sysfs_show(
 	return ret;
 }
 
+static struct gasket_sysfs_attribute apex_sysfs_attrs[] = {
+	GASKET_SYSFS_RO(node_0_page_table_entries, sysfs_show,
+			ATTR_KERNEL_HIB_PAGE_TABLE_SIZE),
+	GASKET_SYSFS_RO(node_0_simple_page_table_entries, sysfs_show,
+			ATTR_KERNEL_HIB_SIMPLE_PAGE_TABLE_SIZE),
+	GASKET_SYSFS_RO(node_0_num_mapped_pages, sysfs_show,
+			ATTR_KERNEL_HIB_NUM_ACTIVE_PAGES),
+	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)
+{
+	return gasket_reset_nolock(gasket_dev, APEX_CHIP_REINIT_RESET);
+}
+
+static const struct pci_device_id apex_pci_ids[] = {
+	{ PCI_DEVICE(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID) }, { 0 }
+};
+
 static void apex_pci_fixup_class(struct pci_dev *pdev)
 {
 	pdev->class = (PCI_CLASS_SYSTEM_OTHER << 8) | pdev->class;
 }
 DECLARE_PCI_FIXUP_CLASS_HEADER(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID,
 			       PCI_CLASS_NOT_DEFINED, 8, apex_pci_fixup_class);
+
+static struct gasket_driver_desc apex_desc = {
+	.name = "apex",
+	.driver_version = APEX_DRIVER_VERSION,
+	.major = 120,
+	.minor = 0,
+	.module = THIS_MODULE,
+	.pci_id_table = apex_pci_ids,
+
+	.num_page_tables = NUM_NODES,
+	.page_table_bar_index = APEX_BAR_INDEX,
+	.page_table_configs = apex_page_table_configs,
+	.page_table_extended_bit = APEX_EXTENDED_SHIFT,
+
+	.bar_descriptions = {
+		GASKET_UNUSED_BAR,
+		GASKET_UNUSED_BAR,
+		{ APEX_BAR_BYTES, (VM_WRITE | VM_READ), APEX_BAR_OFFSET,
+			NUM_REGIONS, mappable_regions, PCI_BAR },
+		GASKET_UNUSED_BAR,
+		GASKET_UNUSED_BAR,
+		GASKET_UNUSED_BAR,
+	},
+	.coherent_buffer_description = {
+		APEX_CH_MEM_BYTES,
+		(VM_WRITE | VM_READ),
+		APEX_CM_OFFSET,
+	},
+	.interrupt_type = PCI_MSIX,
+	.interrupt_bar_index = APEX_BAR_INDEX,
+	.num_interrupts = APEX_INTERRUPT_COUNT,
+	.interrupts = apex_interrupts,
+	.interrupt_pack_width = 7,
+
+	.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,
+
+	.device_open_cb = apex_device_open_cb,
+	.device_close_cb = apex_device_cleanup,
+
+	.ioctl_handler_cb = apex_ioctl,
+	.device_status_cb = apex_get_status,
+	.hardware_revision_cb = NULL,
+	.device_reset_cb = apex_reset,
+};
+
+static int __init apex_init(void)
+{
+	return gasket_register_device(&apex_desc);
+}
+
+static void apex_exit(void)
+{
+	gasket_unregister_device(&apex_desc);
+}
+MODULE_DESCRIPTION("Google Apex driver");
+MODULE_VERSION(APEX_DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("John Joseph <jnjoseph@google.com>");
+MODULE_DEVICE_TABLE(pci, apex_pci_ids);
+module_init(apex_init);
+module_exit(apex_exit);
-- 
2.18.0.345.g5c9ce644c3-goog


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

* [PATCH 13/13] staging: gasket: apex: fix function param line continuation style
  2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
                   ` (11 preceding siblings ...)
  2018-07-29 19:36 ` [PATCH 12/13] staging: gasket: apex: remove static function forward declarations Todd Poynor
@ 2018-07-29 19:36 ` Todd Poynor
  12 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-29 19:36 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Dmitry Torokhov, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Fix multi-line alignment formatting to look like:
      int ret = long_function_name(device, VARIABLE1, VARIABLE2,
                                   VARIABLE3, VARIABLE4);

Many of these TODO items were previously cleaned up during the conversion
to standard logging functions.

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

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index f70fea0d80ecf..c0d3922e1d7c3 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -240,9 +240,9 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
 	 *  - Software force GCB idle
 	 *    - Enable GCB idle
 	 */
-	gasket_read_modify_write_64(
-		gasket_dev, APEX_BAR_INDEX,
-		APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER, 0x0, 1, 32);
+	gasket_read_modify_write_64(gasket_dev, APEX_BAR_INDEX,
+				    APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER,
+				    0x0, 1, 32);
 
 	/*    - Initiate DMA pause */
 	gasket_dev_write_64(gasket_dev, 1, APEX_BAR_INDEX,
@@ -259,16 +259,16 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
 	}
 
 	/*  - Enable GCB reset (0x1 to rg_rst_gcb) */
-	gasket_read_modify_write_32(
-		gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x1, 2, 2);
+	gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+				    APEX_BAR2_REG_SCU_2, 0x1, 2, 2);
 
 	/*  - Enable GCB clock Gate (0x1 to rg_gated_gcb) */
-	gasket_read_modify_write_32(
-		gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x1, 2, 18);
+	gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+				    APEX_BAR2_REG_SCU_2, 0x1, 2, 18);
 
 	/*  - Enable GCB memory shut down (0x3 to rg_force_ram_sd) */
-	gasket_read_modify_write_32(
-		gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 0x3, 2, 14);
+	gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+				    APEX_BAR2_REG_SCU_3, 0x3, 2, 14);
 
 	/*    - Wait for RAM shutdown. */
 	if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
@@ -297,24 +297,24 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
 	 *    - b00: Not forced (HW controlled)
 	 *    - b1x: Force disable
 	 */
-	gasket_read_modify_write_32(
-		gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 0x0, 2, 14);
+	gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+				    APEX_BAR2_REG_SCU_3, 0x0, 2, 14);
 
 	/*
 	 *  - Disable software clock gate:
 	 *    - b00: Not forced (HW controlled)
 	 *    - b1x: Force disable
 	 */
-	gasket_read_modify_write_32(
-		gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x0, 2, 18);
+	gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+				    APEX_BAR2_REG_SCU_2, 0x0, 2, 18);
 
 	/*
 	 *  - Disable GCB reset (rg_rst_gcb):
 	 *    - b00: Not forced (HW controlled)
 	 *    - b1x: Force disable = Force not Reset
 	 */
-	gasket_read_modify_write_32(
-		gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x2, 2, 2);
+	gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+				    APEX_BAR2_REG_SCU_2, 0x2, 2, 2);
 
 	/*    - Wait for RAM enable. */
 	if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
@@ -338,27 +338,28 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
 	}
 
 	if (!allow_hw_clock_gating) {
-		val0 = gasket_dev_read_32(
-			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+		val0 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+					  APEX_BAR2_REG_SCU_3);
 		/* Inactive and Sleep mode are disabled. */
-		gasket_read_modify_write_32(
-			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 0x3,
-			SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
-			SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
-		val1 = gasket_dev_read_32(
-			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+		gasket_read_modify_write_32(gasket_dev,
+					    APEX_BAR_INDEX,
+					    APEX_BAR2_REG_SCU_3, 0x3,
+					    SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
+					    SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
+		val1 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+					  APEX_BAR2_REG_SCU_3);
 		dev_dbg(gasket_dev->dev,
 			"Disallow HW clock gating 0x%x -> 0x%x\n", val0, val1);
 	} else {
-		val0 = gasket_dev_read_32(
-			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+		val0 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+					  APEX_BAR2_REG_SCU_3);
 		/* Inactive mode enabled - Sleep mode disabled. */
-		gasket_read_modify_write_32(
-			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 2,
-			SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
-			SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
-		val1 = gasket_dev_read_32(
-			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+		gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+					    APEX_BAR2_REG_SCU_3, 2,
+					    SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
+					    SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
+		val1 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+					  APEX_BAR2_REG_SCU_3);
 		dev_dbg(gasket_dev->dev, "Allow HW clock gating 0x%x -> 0x%x\n",
 			val0, val1);
 	}
@@ -373,12 +374,10 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
 	u64 hib_error;
 	int ret = 0;
 
-	hib_error = gasket_dev_read_64(
-		gasket_dev, APEX_BAR_INDEX,
-		APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
-	scalar_error = gasket_dev_read_64(
-		gasket_dev, APEX_BAR_INDEX,
-		APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
+	hib_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+				       APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
+	scalar_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+					  APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
 
 	dev_dbg(gasket_dev->dev,
 		"%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
@@ -393,8 +392,8 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
 /* Determine if GCB is in reset state. */
 static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
 {
-	u32 val = gasket_dev_read_32(
-		gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+	u32 val = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+				     APEX_BAR2_REG_SCU_3);
 
 	/* Masks rg_rst_gcb bit of SCU_CTRL_2 */
 	return (val & SCU3_CUR_RST_GCB_BIT_MASK);
@@ -432,13 +431,11 @@ static int apex_add_dev_cb(struct gasket_dev *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);
+			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);
+			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));
@@ -481,20 +478,20 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
 
 	if (ibuf.enable) {
 		/* Quiesce AXI, gate GCB clock. */
-		gasket_read_modify_write_32(
-		    gasket_dev, APEX_BAR_INDEX,
-		    APEX_BAR2_REG_AXI_QUIESCE, 0x1, 1, 16);
-		gasket_read_modify_write_32(
-		    gasket_dev, APEX_BAR_INDEX,
-		    APEX_BAR2_REG_GCB_CLOCK_GATE, 0x1, 2, 18);
+		gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+					    APEX_BAR2_REG_AXI_QUIESCE, 0x1, 1,
+					    16);
+		gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+					    APEX_BAR2_REG_GCB_CLOCK_GATE, 0x1,
+					    2, 18);
 	} else {
 		/* Un-gate GCB clock, un-quiesce AXI. */
-		gasket_read_modify_write_32(
-		    gasket_dev, APEX_BAR_INDEX,
-		    APEX_BAR2_REG_GCB_CLOCK_GATE, 0x0, 2, 18);
-		gasket_read_modify_write_32(
-		    gasket_dev, APEX_BAR_INDEX,
-		    APEX_BAR2_REG_AXI_QUIESCE, 0x0, 1, 16);
+		gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+					    APEX_BAR2_REG_GCB_CLOCK_GATE, 0x0,
+					    2, 18);
+		gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+					    APEX_BAR2_REG_AXI_QUIESCE, 0x0, 1,
+					    16);
 	}
 	return 0;
 }
@@ -516,8 +513,8 @@ static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
 }
 
 /* Display driver sysfs entries. */
-static ssize_t sysfs_show(
-	struct device *device, struct device_attribute *attr, char *buf)
+static ssize_t sysfs_show(struct device *device, struct device_attribute *attr,
+			  char *buf)
 {
 	int ret;
 	struct gasket_dev *gasket_dev;
@@ -578,8 +575,8 @@ static struct gasket_sysfs_attribute apex_sysfs_attrs[] = {
 
 static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev)
 {
-	return gasket_sysfs_create_entries(
-		gasket_dev->dev_info.device, apex_sysfs_attrs);
+	return gasket_sysfs_create_entries(gasket_dev->dev_info.device,
+					   apex_sysfs_attrs);
 }
 
 /* On device open, perform a core reinit reset. */
-- 
2.18.0.345.g5c9ce644c3-goog


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

* Re: [PATCH 04/13] staging: gasket: core: allow root access based on user namespace
  2018-07-29 19:36 ` [PATCH 04/13] staging: gasket: core: allow root access based on user namespace Todd Poynor
@ 2018-07-30 17:56   ` Dmitry Torokhov
  2018-07-30 18:02     ` Todd Poynor
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2018-07-30 17:56 UTC (permalink / raw)
  To: toddpoynor
  Cc: rspringer, jnjoseph, benchan, Greg Kroah-Hartman, devel, lkml,
	toddpoynor

Hi Todd,

On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <toddpoynor@gmail.com> wrote:
> @@ -1064,7 +1067,8 @@ static int gasket_open(struct inode *inode, struct file *filp)
>         char task_name[TASK_COMM_LEN];
>         struct gasket_cdev_info *dev_info =
>             container_of(inode->i_cdev, struct gasket_cdev_info, cdev);
> -       int is_root = capable(CAP_SYS_ADMIN);
> +       struct pid_namespace *pid_ns = task_active_pid_ns(current);
> +       int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);

ns_capable() returns bool, why did you make is_root an integer?

Thanks,
Dmitry

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

* Re: [PATCH 04/13] staging: gasket: core: allow root access based on user namespace
  2018-07-30 17:56   ` Dmitry Torokhov
@ 2018-07-30 18:02     ` Todd Poynor
  0 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-30 18:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: toddpoynor, Rob Springer, John Joseph, benchan, Greg KH, devel, LKML

Hi Dmitry,
On Mon, Jul 30, 2018 at 10:57 AM Dmitry Torokhov <dtor@chromium.org> wrote:
>
> Hi Todd,
>
> On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <toddpoynor@gmail.com> wrote:
> > @@ -1064,7 +1067,8 @@ static int gasket_open(struct inode *inode, struct file *filp)
> >         char task_name[TASK_COMM_LEN];
> >         struct gasket_cdev_info *dev_info =
> >             container_of(inode->i_cdev, struct gasket_cdev_info, cdev);
> > -       int is_root = capable(CAP_SYS_ADMIN);
> > +       struct pid_namespace *pid_ns = task_active_pid_ns(current);
> > +       int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);
>
> ns_capable() returns bool, why did you make is_root an integer?

Gaah, I forgot to change the type of the existing var.  Will fix, thanks -- Todd

>
> Thanks,
> Dmitry

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

* Re: [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used
  2018-07-29 19:36 ` [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used Todd Poynor
@ 2018-07-31  6:18   ` Dmitry Torokhov
  2018-07-31  6:29     ` Todd Poynor
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2018-07-31  6:18 UTC (permalink / raw)
  To: toddpoynor
  Cc: rspringer, jnjoseph, benchan, Greg Kroah-Hartman, devel, lkml,
	toddpoynor

On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <toddpoynor@gmail.com> wrote:
>
> From: Todd Poynor <toddpoynor@google.com>
>
> Hold a reference on the struct pci_dev while a pointer to it is held in
> the gasket data structures.
>
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> ---
>  drivers/staging/gasket/gasket_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
> index 2b484d067c38a..b832a4f529f27 100644
> --- a/drivers/staging/gasket/gasket_core.c
> +++ b/drivers/staging/gasket/gasket_core.c
> @@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
>         internal_desc->devs[gasket_dev->dev_idx] = NULL;
>         mutex_unlock(&internal_desc->mutex);
>         put_device(gasket_dev->dev);
> +       pci_dev_put(gasket_dev->pci_dev);

gasket_free_dev() is called only from driver PCI probe and remove
function. I can assure you that that pci_dev structure is not going
anywhere, there is no need to take this additional reference.

Thanks,
Dmitry

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

* Re: [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used
  2018-07-31  6:18   ` Dmitry Torokhov
@ 2018-07-31  6:29     ` Todd Poynor
  0 siblings, 0 replies; 18+ messages in thread
From: Todd Poynor @ 2018-07-31  6:29 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: toddpoynor, Rob Springer, John Joseph, benchan, Greg KH, devel, LKML

On Mon, Jul 30, 2018 at 11:19 PM Dmitry Torokhov <dtor@chromium.org> wrote:
>
> On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <toddpoynor@gmail.com> wrote:
> >
> > From: Todd Poynor <toddpoynor@google.com>
> >
> > Hold a reference on the struct pci_dev while a pointer to it is held in
> > the gasket data structures.
> >
> > Signed-off-by: Todd Poynor <toddpoynor@google.com>
> > ---
> >  drivers/staging/gasket/gasket_core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
> > index 2b484d067c38a..b832a4f529f27 100644
> > --- a/drivers/staging/gasket/gasket_core.c
> > +++ b/drivers/staging/gasket/gasket_core.c
> > @@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
> >         internal_desc->devs[gasket_dev->dev_idx] = NULL;
> >         mutex_unlock(&internal_desc->mutex);
> >         put_device(gasket_dev->dev);
> > +       pci_dev_put(gasket_dev->pci_dev);
>
> gasket_free_dev() is called only from driver PCI probe and remove
> function. I can assure you that that pci_dev structure is not going
> anywhere, there is no need to take this additional reference.

WIll fix, thanks.

>
> Thanks,
> Dmitry

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

end of thread, other threads:[~2018-07-31  6:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-29 19:36 [PATCH 00/13] staging: gasket: fixes and cleanups Todd Poynor
2018-07-29 19:36 ` [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used Todd Poynor
2018-07-31  6:18   ` Dmitry Torokhov
2018-07-31  6:29     ` Todd Poynor
2018-07-29 19:36 ` [PATCH 02/13] staging: gasket: sysfs: hold reference to device while in use Todd Poynor
2018-07-29 19:36 ` [PATCH 03/13] staging: gasket: page table: hold references to device and pci_dev Todd Poynor
2018-07-29 19:36 ` [PATCH 04/13] staging: gasket: core: allow root access based on user namespace Todd Poynor
2018-07-30 17:56   ` Dmitry Torokhov
2018-07-30 18:02     ` Todd Poynor
2018-07-29 19:36 ` [PATCH 05/13] staging: gasket: apex: simplify comments for static functions Todd Poynor
2018-07-29 19:36 ` [PATCH 06/13] staging: gasket: core: " Todd Poynor
2018-07-29 19:36 ` [PATCH 07/13] staging: gasket: ioctl: " Todd Poynor
2018-07-29 19:36 ` [PATCH 08/13] staging: gasket: page table: " Todd Poynor
2018-07-29 19:36 ` [PATCH 09/13] staging: gasket: interrupt: " Todd Poynor
2018-07-29 19:36 ` [PATCH 10/13] staging: gasket: sysfs: " Todd Poynor
2018-07-29 19:36 ` [PATCH 11/13] staging: gasket: TODO: remove entry for static function kernel docs Todd Poynor
2018-07-29 19:36 ` [PATCH 12/13] staging: gasket: apex: remove static function forward declarations Todd Poynor
2018-07-29 19:36 ` [PATCH 13/13] staging: gasket: apex: fix function param line continuation style Todd Poynor

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).