All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.