* [PATCH 00/10] staging: gasket: logging cleanups
@ 2018-07-27 3:07 Todd Poynor
2018-07-27 3:07 ` [PATCH 01/10] staging: gasket: save struct device for a gasket device Todd Poynor
` (9 more replies)
0 siblings, 10 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Kill off gasket logging functions, convert to standard.
Fixup a few formatting/style problems in the process.
Todd Poynor (10):
staging: gasket: save struct device for a gasket device
staging: gasket: core: convert to standard logging
staging: gasket: interrupt: convert to standard logging
staging: gasket: ioctl: convert to standard logging
staging: gasket: page table: convert to standard logging
staging: gasket: sysfs: convert to standard logging
staging: gasket: apex: convert to standard logging
staging: gasket: remove gasket logging header
staging: gasket: TODO: remove entry for convert to standard logging
staging: gasket: don't print device addresses as kernel pointers
drivers/staging/gasket/TODO | 1 -
drivers/staging/gasket/apex_driver.c | 61 ++---
drivers/staging/gasket/gasket_core.c | 300 ++++++++++-----------
drivers/staging/gasket/gasket_core.h | 3 +
drivers/staging/gasket/gasket_interrupt.c | 67 +++--
drivers/staging/gasket/gasket_ioctl.c | 23 +-
drivers/staging/gasket/gasket_logging.h | 64 -----
drivers/staging/gasket/gasket_page_table.c | 131 ++++-----
drivers/staging/gasket/gasket_sysfs.c | 73 +++--
9 files changed, 296 insertions(+), 427 deletions(-)
delete mode 100644 drivers/staging/gasket/gasket_logging.h
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 01/10] staging: gasket: save struct device for a gasket device
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 15:08 ` Greg Kroah-Hartman
2018-07-27 3:07 ` [PATCH 02/10] staging: gasket: core: convert to standard logging Todd Poynor
` (8 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Save the struct device pointer to a gasket device in gasket's metadata,
to facilitate use of standard logging calls and in anticipation of
non-PCI gasket devices in the future.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 5 +++--
drivers/staging/gasket/gasket_core.h | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 732218773c3c6..e8f3b021c20d1 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -450,6 +450,7 @@ static int gasket_alloc_dev(
gasket_dev->internal_desc = internal_desc;
gasket_dev->dev_idx = dev_idx;
snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", kobj_name);
+ gasket_dev->dev = parent;
/* gasket_bar_data is uninitialized. */
gasket_dev->num_page_tables = driver_desc->num_page_tables;
/* max_page_table_size and *page table are uninit'ed */
@@ -925,7 +926,7 @@ static int gasket_enable_dev(
&gasket_dev->bar_data[
driver_desc->page_table_bar_index],
&driver_desc->page_table_configs[tbl_idx],
- &gasket_dev->pci_dev->dev, gasket_dev->pci_dev, true);
+ gasket_dev->dev, gasket_dev->pci_dev, true);
if (ret) {
gasket_log_error(
gasket_dev,
@@ -2028,7 +2029,7 @@ const struct gasket_driver_desc *gasket_get_driver_desc(struct gasket_dev *dev)
*/
struct device *gasket_get_device(struct gasket_dev *dev)
{
- return &dev->pci_dev->dev;
+ return dev->dev;
}
/**
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index bf4ed3769efb2..8bd431ad3b58b 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -263,6 +263,9 @@ struct gasket_dev {
/* Pointer to the internal driver description for this device. */
struct gasket_internal_desc *internal_desc;
+ /* Device info */
+ struct device *dev;
+
/* PCI subsystem metadata. */
struct pci_dev *pci_dev;
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 02/10] staging: gasket: core: convert to standard logging
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
2018-07-27 3:07 ` [PATCH 01/10] staging: gasket: save struct device for a gasket device Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 03/10] staging: gasket: interrupt: " Todd Poynor
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Use standard logging functions, drop use of gasket log functions.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 295 ++++++++++++---------------
1 file changed, 134 insertions(+), 161 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index e8f3b021c20d1..f44805c38159b 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -10,15 +10,16 @@
#include "gasket_interrupt.h"
#include "gasket_ioctl.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include "gasket_sysfs.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/printk.h>
#ifdef GASKET_KERNEL_TRACE_SUPPORT
#define CREATE_TRACE_POINTS
@@ -205,8 +206,8 @@ static inline int check_and_invoke_callback(
{
int ret = 0;
- gasket_log_debug(gasket_dev, "check_and_invoke_callback %p",
- cb_function);
+ dev_dbg(gasket_dev->dev, "check_and_invoke_callback %p\n",
+ cb_function);
if (cb_function) {
mutex_lock(&gasket_dev->mutex);
ret = cb_function(gasket_dev);
@@ -228,8 +229,8 @@ static inline int gasket_check_and_invoke_callback_nolock(
int ret = 0;
if (cb_function) {
- gasket_log_debug(
- gasket_dev, "Invoking device-specific callback.");
+ dev_dbg(gasket_dev->dev,
+ "Invoking device-specific callback.\n");
ret = cb_function(gasket_dev);
}
return ret;
@@ -250,7 +251,7 @@ static int __init gasket_init(void)
{
int i;
- gasket_nodev_info("Performing one-time init of the Gasket framework.");
+ pr_info("Performing one-time init of the Gasket framework.\n");
/* Check for duplicates and find a free slot. */
mutex_lock(&g_mutex);
for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
@@ -267,7 +268,7 @@ static int __init gasket_init(void)
static void __exit gasket_exit(void)
{
/* No deinit/dealloc needed at present. */
- gasket_nodev_info("Removing Gasket framework module.");
+ pr_info("Removing Gasket framework module.\n");
}
/* See gasket_core.h for description. */
@@ -277,15 +278,14 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
int desc_idx = -1;
struct gasket_internal_desc *internal;
- gasket_nodev_info("Initializing Gasket framework device");
+ pr_info("Initializing Gasket framework device\n");
/* Check for duplicates and find a free slot. */
mutex_lock(&g_mutex);
for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
if (g_descs[i].driver_desc == driver_desc) {
- gasket_nodev_error(
- "%s driver already loaded/registered",
- driver_desc->name);
+ pr_err("%s driver already loaded/registered\n",
+ driver_desc->name);
mutex_unlock(&g_mutex);
return -EBUSY;
}
@@ -301,17 +301,17 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
}
mutex_unlock(&g_mutex);
- gasket_nodev_info("Loaded %s driver, framework version %s",
- driver_desc->name, GASKET_FRAMEWORK_VERSION);
+ pr_info("Loaded %s driver, framework version %s\n",
+ driver_desc->name, GASKET_FRAMEWORK_VERSION);
if (desc_idx == -1) {
- gasket_nodev_error("Too many Gasket drivers loaded: %d\n",
- GASKET_FRAMEWORK_DESC_MAX);
+ pr_err("Too many Gasket drivers loaded: %d\n",
+ GASKET_FRAMEWORK_DESC_MAX);
return -EBUSY;
}
/* Internal structure setup. */
- gasket_nodev_info("Performing initial internal structure setup.");
+ pr_debug("Performing initial internal structure setup.\n");
internal = &g_descs[desc_idx];
mutex_init(&internal->mutex);
memset(internal->devs, 0, sizeof(struct gasket_dev *) * GASKET_DEV_MAX);
@@ -324,8 +324,8 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
class_create(driver_desc->module, driver_desc->name);
if (IS_ERR(internal->class)) {
- gasket_nodev_error("Cannot register %s class [ret=%ld]",
- driver_desc->name, PTR_ERR(internal->class));
+ pr_err("Cannot register %s class [ret=%ld]\n",
+ driver_desc->name, PTR_ERR(internal->class));
ret = PTR_ERR(internal->class);
goto unregister_gasket_driver;
}
@@ -334,25 +334,24 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
* Not using pci_register_driver() (without underscores), as it
* depends on KBUILD_MODNAME, and this is a shared file.
*/
- gasket_nodev_info("Registering PCI driver.");
+ pr_debug("Registering PCI driver.\n");
ret = __pci_register_driver(
&internal->pci, driver_desc->module, driver_desc->name);
if (ret) {
- gasket_nodev_error(
- "cannot register pci driver [ret=%d]", ret);
+ pr_err("cannot register pci driver [ret=%d]\n", ret);
goto fail1;
}
- gasket_nodev_info("Registering char driver.");
+ pr_debug("Registering char driver.\n");
ret = register_chrdev_region(
MKDEV(driver_desc->major, driver_desc->minor), GASKET_DEV_MAX,
driver_desc->name);
if (ret) {
- gasket_nodev_error("cannot register char driver [ret=%d]", ret);
+ pr_err("cannot register char driver [ret=%d]\n", ret);
goto fail2;
}
- gasket_nodev_info("Driver registered successfully.");
+ pr_info("Driver registered successfully.\n");
return 0;
fail2:
@@ -386,10 +385,9 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
mutex_unlock(&g_mutex);
if (!internal_desc) {
- gasket_nodev_error(
- "request to unregister unknown desc: %s, %d:%d",
- driver_desc->name, driver_desc->major,
- driver_desc->minor);
+ pr_err("request to unregister unknown desc: %s, %d:%d\n",
+ driver_desc->name, driver_desc->major,
+ driver_desc->minor);
return;
}
@@ -405,7 +403,7 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
g_descs[desc_idx].driver_desc = NULL;
mutex_unlock(&g_mutex);
- gasket_nodev_info("removed %s driver", driver_desc->name);
+ pr_info("removed %s driver\n", driver_desc->name);
}
EXPORT_SYMBOL(gasket_unregister_device);
@@ -430,7 +428,7 @@ static int gasket_alloc_dev(
struct gasket_dev *gasket_dev;
struct gasket_cdev_info *dev_info;
- gasket_nodev_info("Allocating a Gasket device %s.", kobj_name);
+ pr_debug("Allocating a Gasket device %s.\n", kobj_name);
*pdev = NULL;
@@ -440,7 +438,7 @@ static int gasket_alloc_dev(
gasket_dev = *pdev = kzalloc(sizeof(*gasket_dev), GFP_KERNEL);
if (!gasket_dev) {
- gasket_nodev_error("no memory for device");
+ pr_err("no memory for device\n");
return -ENOMEM;
}
internal_desc->devs[dev_idx] = gasket_dev;
@@ -466,7 +464,7 @@ static int gasket_alloc_dev(
dev_info->device = device_create(internal_desc->class, parent,
dev_info->devt, gasket_dev, dev_info->name);
- gasket_nodev_info("Gasket device allocated: %p.", dev_info->device);
+ dev_dbg(dev_info->device, "Gasket device allocated.\n");
/* cdev has not yet been added; cdev_added is 0 */
dev_info->gasket_dev_ptr = gasket_dev;
@@ -509,7 +507,7 @@ static int gasket_find_dev_slot(
for (i = 0; i < GASKET_DEV_MAX; i++) {
if (internal_desc->devs[i] &&
strcmp(internal_desc->devs[i]->kobj_name, kobj_name) == 0) {
- gasket_nodev_error("Duplicate device %s", kobj_name);
+ pr_err("Duplicate device %s\n", kobj_name);
mutex_unlock(&internal_desc->mutex);
return -EBUSY;
}
@@ -522,8 +520,7 @@ static int gasket_find_dev_slot(
}
if (i == GASKET_DEV_MAX) {
- gasket_nodev_info(
- "Too many registered devices; max %d", GASKET_DEV_MAX);
+ pr_err("Too many registered devices; max %d\n", GASKET_DEV_MAX);
mutex_unlock(&internal_desc->mutex);
return -EBUSY;
}
@@ -552,13 +549,13 @@ static int gasket_pci_probe(
const struct gasket_driver_desc *driver_desc;
struct device *parent;
- gasket_nodev_info("Add Gasket device %s", kobj_name);
+ pr_info("Add Gasket device %s\n", kobj_name);
mutex_lock(&g_mutex);
internal_desc = lookup_internal_desc(pci_dev);
mutex_unlock(&g_mutex);
if (!internal_desc) {
- gasket_nodev_info("PCI probe called for unknown driver type");
+ pr_err("PCI probe called for unknown driver type\n");
return -ENODEV;
}
@@ -569,9 +566,9 @@ static int gasket_pci_probe(
if (ret)
return ret;
if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) {
- gasket_nodev_error("Cannot create %s device %s [ret = %ld]",
- driver_desc->name, gasket_dev->dev_info.name,
- PTR_ERR(gasket_dev->dev_info.device));
+ pr_err("Cannot create %s device %s [ret = %ld]\n",
+ driver_desc->name, gasket_dev->dev_info.name,
+ PTR_ERR(gasket_dev->dev_info.device));
ret = -ENODEV;
goto fail1;
}
@@ -583,7 +580,7 @@ static int gasket_pci_probe(
ret = check_and_invoke_callback(gasket_dev, driver_desc->add_dev_cb);
if (ret) {
- gasket_log_error(gasket_dev, "Error in add device cb: %d", ret);
+ dev_err(gasket_dev->dev, "Error in add device cb: %d\n", ret);
goto fail2;
}
@@ -599,8 +596,8 @@ static int gasket_pci_probe(
ret = sysfs_create_link(&gasket_dev->dev_info.device->kobj,
&pci_dev->dev.kobj, dev_name(&pci_dev->dev));
if (ret) {
- gasket_log_error(
- gasket_dev, "Cannot create sysfs pci link: %d", ret);
+ dev_err(gasket_dev->dev,
+ "Cannot create sysfs pci link: %d\n", ret);
goto fail3;
}
ret = gasket_sysfs_create_entries(
@@ -611,14 +608,13 @@ static int gasket_pci_probe(
ret = check_and_invoke_callback(
gasket_dev, driver_desc->sysfs_setup_cb);
if (ret) {
- gasket_log_error(
- gasket_dev, "Error in sysfs setup cb: %d", ret);
+ dev_err(gasket_dev->dev, "Error in sysfs setup cb: %d\n", ret);
goto fail5;
}
ret = gasket_enable_dev(internal_desc, gasket_dev);
if (ret) {
- gasket_nodev_error("cannot setup %s device", driver_desc->name);
+ pr_err("cannot setup %s device\n", driver_desc->name);
gasket_disable_dev(gasket_dev);
goto fail5;
}
@@ -677,8 +673,7 @@ static void gasket_pci_remove(struct pci_dev *pci_dev)
if (!gasket_dev)
return;
- gasket_nodev_info(
- "remove %s device %s", internal_desc->driver_desc->name,
+ pr_info("remove %s device %s\n", internal_desc->driver_desc->name,
gasket_dev->kobj_name);
gasket_disable_dev(gasket_dev);
@@ -711,7 +706,7 @@ static int gasket_setup_pci(
gasket_dev->pci_dev = pci_dev;
ret = pci_enable_device(pci_dev);
if (ret) {
- gasket_log_error(gasket_dev, "cannot enable PCI device");
+ dev_err(gasket_dev->dev, "cannot enable PCI device\n");
return ret;
}
@@ -777,17 +772,16 @@ static int gasket_map_pci_bar(struct gasket_dev *gasket_dev, int bar_num)
gasket_dev->bar_data[bar_num].phys_base =
(ulong)pci_resource_start(gasket_dev->pci_dev, bar_num);
if (!gasket_dev->bar_data[bar_num].phys_base) {
- gasket_log_error(gasket_dev, "Cannot get BAR%u base address",
- bar_num);
+ dev_err(gasket_dev->dev, "Cannot get BAR%u base address\n",
+ bar_num);
return -EINVAL;
}
gasket_dev->bar_data[bar_num].length_bytes =
(ulong)pci_resource_len(gasket_dev->pci_dev, bar_num);
if (gasket_dev->bar_data[bar_num].length_bytes < desc_bytes) {
- gasket_log_error(
- gasket_dev,
- "PCI BAR %u space is too small: %lu; expected >= %lu",
+ dev_err(gasket_dev->dev,
+ "PCI BAR %u space is too small: %lu; expected >= %lu\n",
bar_num, gasket_dev->bar_data[bar_num].length_bytes,
desc_bytes);
return -ENOMEM;
@@ -796,9 +790,8 @@ static int gasket_map_pci_bar(struct gasket_dev *gasket_dev, int bar_num)
if (!request_mem_region(gasket_dev->bar_data[bar_num].phys_base,
gasket_dev->bar_data[bar_num].length_bytes,
gasket_dev->dev_info.name)) {
- gasket_log_error(
- gasket_dev,
- "Cannot get BAR %d memory region %p",
+ dev_err(gasket_dev->dev,
+ "Cannot get BAR %d memory region %p\n",
bar_num, &gasket_dev->pci_dev->resource[bar_num]);
return -EINVAL;
}
@@ -807,9 +800,8 @@ static int gasket_map_pci_bar(struct gasket_dev *gasket_dev, int bar_num)
ioremap_nocache(gasket_dev->bar_data[bar_num].phys_base,
gasket_dev->bar_data[bar_num].length_bytes);
if (!gasket_dev->bar_data[bar_num].virt_base) {
- gasket_log_error(
- gasket_dev,
- "Cannot remap BAR %d memory region %p",
+ dev_err(gasket_dev->dev,
+ "Cannot remap BAR %d memory region %p\n",
bar_num, &gasket_dev->pci_dev->resource[bar_num]);
ret = -ENOMEM;
goto fail;
@@ -852,8 +844,8 @@ static void gasket_unmap_pci_bar(struct gasket_dev *dev, int bar_num)
base = pci_resource_start(dev->pci_dev, bar_num);
if (!base) {
- gasket_log_error(
- dev, "cannot get PCI BAR%u base address", bar_num);
+ dev_err(dev->dev, "cannot get PCI BAR%u base address\n",
+ bar_num);
return;
}
@@ -877,9 +869,8 @@ static int gasket_add_cdev(
dev_info->cdev.owner = owner;
ret = cdev_add(&dev_info->cdev, dev_info->devt, 1);
if (ret) {
- gasket_log_error(
- dev_info->gasket_dev_ptr,
- "cannot add char device [ret=%d]", ret);
+ dev_err(dev_info->gasket_dev_ptr->dev,
+ "cannot add char device [ret=%d]\n", ret);
return ret;
}
dev_info->cdev_added = 1;
@@ -911,16 +902,15 @@ static int gasket_enable_dev(
driver_desc->interrupt_bar_index,
driver_desc->wire_interrupt_offsets);
if (ret) {
- gasket_log_error(gasket_dev,
- "Critical failure to allocate interrupts: %d",
- ret);
+ dev_err(gasket_dev->dev,
+ "Critical failure to allocate interrupts: %d\n", ret);
gasket_interrupt_cleanup(gasket_dev);
return ret;
}
for (tbl_idx = 0; tbl_idx < driver_desc->num_page_tables; tbl_idx++) {
- gasket_log_debug(
- gasket_dev, "Initializing page table %d.", tbl_idx);
+ dev_dbg(gasket_dev->dev, "Initializing page table %d.\n",
+ tbl_idx);
ret = gasket_page_table_init(
&gasket_dev->page_table[tbl_idx],
&gasket_dev->bar_data[
@@ -928,9 +918,8 @@ static int gasket_enable_dev(
&driver_desc->page_table_configs[tbl_idx],
gasket_dev->dev, gasket_dev->pci_dev, true);
if (ret) {
- gasket_log_error(
- gasket_dev,
- "Couldn't init page table %d: %d",
+ dev_err(gasket_dev->dev,
+ "Couldn't init page table %d: %d\n",
tbl_idx, ret);
return ret;
}
@@ -948,23 +937,23 @@ static int gasket_enable_dev(
ret = check_and_invoke_callback(
gasket_dev, driver_desc->hardware_revision_cb);
if (ret < 0) {
- gasket_log_error(
- gasket_dev, "Error getting hardware revision: %d", ret);
+ dev_err(gasket_dev->dev,
+ "Error getting hardware revision: %d\n", ret);
return ret;
}
gasket_dev->hardware_revision = ret;
ret = check_and_invoke_callback(gasket_dev, driver_desc->enable_dev_cb);
if (ret) {
- gasket_log_error(
- gasket_dev, "Error in enable device cb: %d", ret);
+ dev_err(gasket_dev->dev, "Error in enable device cb: %d\n",
+ ret);
return ret;
}
/* device_status_cb returns a device status, not an error code. */
gasket_dev->status = gasket_get_hw_status(gasket_dev);
if (gasket_dev->status == GASKET_STATUS_DEAD)
- gasket_log_error(gasket_dev, "Device reported as unhealthy.");
+ dev_err(gasket_dev->dev, "Device reported as unhealthy.\n");
ret = gasket_add_cdev(
&gasket_dev->dev_info, &gasket_file_ops, driver_desc->module);
@@ -1084,31 +1073,29 @@ static int gasket_open(struct inode *inode, struct file *filp)
filp->private_data = gasket_dev;
inode->i_size = 0;
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Attempting to open with tgid %u (%s) (f_mode: 0%03o, "
- "fmode_write: %d is_root: %u)",
+ "fmode_write: %d is_root: %u)\n",
current->tgid, task_name, filp->f_mode,
(filp->f_mode & FMODE_WRITE), is_root);
/* Always allow non-writing accesses. */
if (!(filp->f_mode & FMODE_WRITE)) {
- gasket_log_debug(gasket_dev, "Allowing read-only opening.");
+ dev_dbg(gasket_dev->dev, "Allowing read-only opening.\n");
return 0;
}
mutex_lock(&gasket_dev->mutex);
- gasket_log_debug(
- gasket_dev, "Current owner open count (owning tgid %u): %d.",
+ dev_dbg(gasket_dev->dev,
+ "Current owner open count (owning tgid %u): %d.\n",
ownership->owner, ownership->write_open_count);
/* Opening a node owned by another TGID is an error (unless root) */
if (ownership->is_owned && ownership->owner != current->tgid &&
!is_root) {
- gasket_log_error(
- gasket_dev,
- "Process %u is opening a node held by %u.",
+ dev_err(gasket_dev->dev,
+ "Process %u is opening a node held by %u.\n",
current->tgid, ownership->owner);
mutex_unlock(&gasket_dev->mutex);
return -EPERM;
@@ -1119,21 +1106,21 @@ static int gasket_open(struct inode *inode, struct file *filp)
ret = gasket_check_and_invoke_callback_nolock(
gasket_dev, driver_desc->device_open_cb);
if (ret) {
- gasket_log_error(
- gasket_dev, "Error in device open cb: %d", ret);
+ dev_err(gasket_dev->dev,
+ "Error in device open cb: %d\n", ret);
mutex_unlock(&gasket_dev->mutex);
return ret;
}
ownership->is_owned = 1;
ownership->owner = current->tgid;
- gasket_log_debug(gasket_dev, "Device owner is now tgid %u",
- ownership->owner);
+ dev_dbg(gasket_dev->dev, "Device owner is now tgid %u\n",
+ ownership->owner);
}
ownership->write_open_count++;
- gasket_log_debug(gasket_dev, "New open count (owning tgid %u): %d",
- ownership->owner, ownership->write_open_count);
+ dev_dbg(gasket_dev->dev, "New open count (owning tgid %u): %d\n",
+ ownership->owner, ownership->write_open_count);
mutex_unlock(&gasket_dev->mutex);
return 0;
@@ -1167,19 +1154,18 @@ static int gasket_release(struct inode *inode, struct file *file)
get_task_comm(task_name, current);
mutex_lock(&gasket_dev->mutex);
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Releasing device node. Call origin: tgid %u (%s) "
- "(f_mode: 0%03o, fmode_write: %d, is_root: %u)",
+ "(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));
- gasket_log_debug(gasket_dev, "Current open count (owning tgid %u): %d",
- ownership->owner, ownership->write_open_count);
+ dev_dbg(gasket_dev->dev, "Current open count (owning tgid %u): %d\n",
+ ownership->owner, ownership->write_open_count);
if (file->f_mode & FMODE_WRITE) {
ownership->write_open_count--;
if (ownership->write_open_count == 0) {
- gasket_log_debug(gasket_dev, "Device is now free");
+ dev_dbg(gasket_dev->dev, "Device is now free\n");
ownership->is_owned = 0;
ownership->owner = 0;
@@ -1200,8 +1186,7 @@ static int gasket_release(struct inode *inode, struct file *file)
}
}
- gasket_log_debug(
- gasket_dev, "New open count (owning tgid %u): %d",
+ dev_dbg(gasket_dev->dev, "New open count (owning tgid %u): %d\n",
ownership->owner, ownership->write_open_count);
mutex_unlock(&gasket_dev->mutex);
return 0;
@@ -1227,7 +1212,7 @@ static bool gasket_mmap_has_permissions(
/* Never allow non-sysadmins to access to a dead device. */
if (gasket_dev->status != GASKET_STATUS_ALIVE) {
- gasket_log_debug(gasket_dev, "Device is dead.");
+ dev_dbg(gasket_dev->dev, "Device is dead.\n");
return false;
}
@@ -1235,10 +1220,9 @@ static bool gasket_mmap_has_permissions(
requested_permissions =
(vma->vm_flags & (VM_WRITE | VM_READ | VM_EXEC));
if (requested_permissions & ~(bar_permissions)) {
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Attempting to map a region with requested permissions "
- "0x%x, but region has permissions 0x%x.",
+ "0x%x, but region has permissions 0x%x.\n",
requested_permissions, bar_permissions);
return false;
}
@@ -1246,10 +1230,9 @@ static bool gasket_mmap_has_permissions(
/* Do not allow a non-owner to write. */
if ((vma->vm_flags & VM_WRITE) &&
!gasket_owned_by_current_tgid(&gasket_dev->dev_info)) {
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Attempting to mmap a region for write without owning "
- "device.");
+ "device.\n");
return false;
}
@@ -1462,8 +1445,8 @@ static enum do_map_region_status do_map_region(
(phys_base + mapped_bytes) >> PAGE_SHIFT,
chunk_size, vma->vm_page_prot);
if (ret) {
- gasket_log_error(
- gasket_dev, "Error remapping PFN range.");
+ dev_err(gasket_dev->dev,
+ "Error remapping PFN range.\n");
goto fail;
}
mapped_bytes += chunk_size;
@@ -1475,9 +1458,8 @@ static enum do_map_region_status do_map_region(
/* Unmap the partial chunk we mapped. */
mappable_region->length_bytes = mapped_bytes;
if (gasket_mm_unmap_region(gasket_dev, vma, mappable_region))
- gasket_log_error(
- gasket_dev,
- "Error unmapping partial region 0x%lx (0x%lx bytes)",
+ dev_err(gasket_dev->dev,
+ "Error unmapping partial region 0x%lx (0x%lx bytes)\n",
(ulong)virt_offset,
(ulong)mapped_bytes);
@@ -1502,9 +1484,8 @@ static int gasket_mm_vma_bar_offset(
driver_desc->legacy_mmap_address_offset;
bar_index = gasket_get_bar_index(gasket_dev, raw_offset);
if (bar_index < 0) {
- gasket_log_error(
- gasket_dev,
- "Unable to find matching bar for address 0x%lx",
+ dev_err(gasket_dev->dev,
+ "Unable to find matching bar for address 0x%lx\n",
raw_offset);
trace_gasket_mmap_exit(bar_index);
return bar_index;
@@ -1537,7 +1518,7 @@ static int gasket_mmap_coherent(
permissions = driver_desc->coherent_buffer_description.permissions;
if (!gasket_mmap_has_permissions(gasket_dev, vma, permissions)) {
- gasket_log_error(gasket_dev, "Permission checking failed.");
+ dev_err(gasket_dev->dev, "Permission checking failed.\n");
trace_gasket_mmap_exit(-EPERM);
return -EPERM;
}
@@ -1549,8 +1530,8 @@ static int gasket_mmap_coherent(
(gasket_dev->coherent_buffer.phys_base) >> PAGE_SHIFT,
requested_length, vma->vm_page_prot);
if (ret) {
- gasket_log_error(
- gasket_dev, "Error remapping PFN range err=%d.", ret);
+ dev_err(gasket_dev->dev, "Error remapping PFN range err=%d.\n",
+ ret);
trace_gasket_mmap_exit(ret);
return ret;
}
@@ -1592,8 +1573,8 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
driver_desc = gasket_dev->internal_desc->driver_desc;
if (vma->vm_start & ~PAGE_MASK) {
- gasket_log_error(
- gasket_dev, "Base address not page-aligned: 0x%lx\n",
+ dev_err(gasket_dev->dev,
+ "Base address not page-aligned: 0x%lx\n",
vma->vm_start);
trace_gasket_mmap_exit(-EINVAL);
return -EINVAL;
@@ -1613,18 +1594,16 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
bar_index = gasket_get_bar_index(gasket_dev, raw_offset);
is_coherent_region = gasket_is_coherent_region(driver_desc, raw_offset);
if (bar_index < 0 && !is_coherent_region) {
- gasket_log_error(
- gasket_dev,
- "Unable to find matching bar for address 0x%lx",
+ dev_err(gasket_dev->dev,
+ "Unable to find matching bar for address 0x%lx\n",
raw_offset);
trace_gasket_mmap_exit(bar_index);
return bar_index;
}
if (bar_index > 0 && is_coherent_region) {
- gasket_log_error(
- gasket_dev,
+ dev_err(gasket_dev->dev,
"double matching bar and coherent buffers for address "
- "0x%lx",
+ "0x%lx\n",
raw_offset);
trace_gasket_mmap_exit(bar_index);
return -EINVAL;
@@ -1644,7 +1623,7 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
bar_desc = &driver_desc->bar_descriptions[bar_index];
permissions = bar_desc->permissions;
if (!gasket_mmap_has_permissions(gasket_dev, vma, permissions)) {
- gasket_log_error(gasket_dev, "Permission checking failed.");
+ dev_err(gasket_dev->dev, "Permission checking failed.\n");
trace_gasket_mmap_exit(-EPERM);
return -EPERM;
}
@@ -1657,8 +1636,8 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
} else {
if (!gasket_mmap_has_permissions(gasket_dev, vma,
bar_desc->permissions)) {
- gasket_log_error(
- gasket_dev, "Permission checking failed.");
+ dev_err(gasket_dev->dev,
+ "Permission checking failed.\n");
trace_gasket_mmap_exit(-EPERM);
return -EPERM;
}
@@ -1674,7 +1653,7 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
}
if (!map_regions || num_map_regions == 0) {
- gasket_log_error(gasket_dev, "No mappable regions returned!");
+ dev_err(gasket_dev->dev, "No mappable regions returned!\n");
return -EINVAL;
}
@@ -1697,9 +1676,8 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
/* If we could not map any memory, the request was invalid. */
if (!has_mapped_anything) {
- gasket_log_error(
- gasket_dev,
- "Map request did not contain a valid region.");
+ dev_err(gasket_dev->dev,
+ "Map request did not contain a valid region.\n");
trace_gasket_mmap_exit(-EINVAL);
return -EINVAL;
}
@@ -1713,8 +1691,8 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
for (i = 0; i < num_map_regions; i++)
if (gasket_mm_unmap_region(gasket_dev, vma,
&bar_desc->mappable_regions[i]))
- gasket_log_error(
- gasket_dev, "Error unmapping range %d.", i);
+ dev_err(gasket_dev->dev, "Error unmapping range %d.\n",
+ i);
kfree(map_regions);
return ret;
@@ -1738,16 +1716,15 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
status = gasket_check_and_invoke_callback_nolock(
gasket_dev, driver_desc->device_status_cb);
if (status != GASKET_STATUS_ALIVE) {
- gasket_log_debug(gasket_dev, "Hardware reported status %d.",
- status);
+ dev_dbg(gasket_dev->dev, "Hardware reported status %d.\n",
+ status);
return status;
}
status = gasket_interrupt_system_status(gasket_dev);
if (status != GASKET_STATUS_ALIVE) {
- gasket_log_debug(gasket_dev,
- "Interrupt system reported status %d.",
- status);
+ dev_dbg(gasket_dev->dev,
+ "Interrupt system reported status %d.\n", status);
return status;
}
@@ -1755,8 +1732,8 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
status = gasket_page_table_system_status(
gasket_dev->page_table[i]);
if (status != GASKET_STATUS_ALIVE) {
- gasket_log_debug(
- gasket_dev, "Page table %d reported status %d.",
+ dev_dbg(gasket_dev->dev,
+ "Page table %d reported status %d.\n",
i, status);
return status;
}
@@ -1786,9 +1763,8 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
gasket_dev = (struct gasket_dev *)filp->private_data;
driver_desc = gasket_dev->internal_desc->driver_desc;
if (!driver_desc) {
- gasket_log_debug(
- gasket_dev,
- "Unable to find device descriptor for file %s",
+ dev_dbg(gasket_dev->dev,
+ "Unable to find device descriptor for file %s\n",
d_path(&filp->f_path, path, 256));
return -ENODEV;
}
@@ -1802,8 +1778,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
if (driver_desc->ioctl_handler_cb)
return driver_desc->ioctl_handler_cb(filp, cmd, argp);
- gasket_log_debug(
- gasket_dev, "Received unknown ioctl 0x%x", cmd);
+ dev_dbg(gasket_dev->dev, "Received unknown ioctl 0x%x\n", cmd);
return -EINVAL;
}
@@ -1834,8 +1809,8 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
/* Perform a device reset of the requested type. */
ret = driver_desc->device_reset_cb(gasket_dev, reset_type);
if (ret) {
- gasket_log_debug(
- gasket_dev, "Device reset cb returned %d.", ret);
+ dev_dbg(gasket_dev->dev, "Device reset cb returned %d.\n",
+ ret);
return ret;
}
@@ -1845,15 +1820,15 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
ret = gasket_interrupt_reinit(gasket_dev);
if (ret) {
- gasket_log_debug(
- gasket_dev, "Unable to reinit interrupts: %d.", ret);
+ dev_dbg(gasket_dev->dev, "Unable to reinit interrupts: %d.\n",
+ ret);
return ret;
}
/* Get current device health. */
gasket_dev->status = gasket_get_hw_status(gasket_dev);
if (gasket_dev->status == GASKET_STATUS_DEAD) {
- gasket_log_debug(gasket_dev, "Device reported as dead.");
+ dev_dbg(gasket_dev->dev, "Device reported as dead.\n");
return -EINVAL;
}
@@ -1909,15 +1884,13 @@ static ssize_t gasket_sysfs_data_show(
gasket_dev = gasket_sysfs_get_device_data(device);
if (!gasket_dev) {
- gasket_nodev_error(
- "No sysfs mapping found for device 0x%p", device);
+ dev_err(device, "No sysfs mapping found for device\n");
return 0;
}
gasket_attr = gasket_sysfs_get_attr(device, attr);
if (!gasket_attr) {
- gasket_nodev_error(
- "No sysfs attr found for device 0x%p", device);
+ dev_err(device, "No sysfs attr found for device\n");
gasket_sysfs_put_device_data(device, gasket_dev);
return 0;
}
@@ -2005,8 +1978,8 @@ static ssize_t gasket_sysfs_data_show(
}
break;
default:
- gasket_log_debug(
- gasket_dev, "Unknown attribute: %s", attr->attr.name);
+ dev_dbg(gasket_dev->dev, "Unknown attribute: %s\n",
+ attr->attr.name);
ret = 0;
break;
}
@@ -2059,8 +2032,8 @@ int gasket_wait_with_reschedule(
msleep(delay_ms);
retries++;
}
- gasket_log_debug(gasket_dev, "%s timeout: reg %llx timeout (%llu ms)",
- __func__, offset, max_retries * delay_ms);
+ dev_dbg(gasket_dev->dev, "%s timeout: reg %llx timeout (%llu ms)\n",
+ __func__, offset, max_retries * delay_ms);
return -ETIMEDOUT;
}
EXPORT_SYMBOL(gasket_wait_with_reschedule);
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 03/10] staging: gasket: interrupt: convert to standard logging
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
2018-07-27 3:07 ` [PATCH 01/10] staging: gasket: save struct device for a gasket device Todd Poynor
2018-07-27 3:07 ` [PATCH 02/10] staging: gasket: core: convert to standard logging Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 04/10] staging: gasket: ioctl: " Todd Poynor
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Convert gasket logging calls to standard functions.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_interrupt.c | 67 +++++++++++------------
1 file changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index 2b8c26d065336..3be8e24c126d0 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -5,9 +5,10 @@
#include "gasket_constants.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
#include "gasket_sysfs.h"
+#include <linux/device.h>
#include <linux/interrupt.h>
+#include <linux/printk.h>
#include <linux/version.h>
#ifdef GASKET_KERNEL_TRACE_SUPPORT
#define CREATE_TRACE_POINTS
@@ -165,8 +166,8 @@ int gasket_interrupt_init(
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_error(
- "Cannot handle unsupported interrupt type %d.",
+ dev_err(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
ret = -EINVAL;
};
@@ -175,8 +176,8 @@ int gasket_interrupt_init(
/* Failing to setup interrupts will cause the device to report
* GASKET_STATUS_LAMED. But it is not fatal.
*/
- gasket_log_warn(
- gasket_dev, "Couldn't initialize interrupts: %d", ret);
+ dev_warn(gasket_dev->dev,
+ "Couldn't initialize interrupts: %d\n", ret);
return 0;
}
@@ -216,7 +217,7 @@ static int gasket_interrupt_msix_init(
interrupt_data);
if (ret) {
- gasket_nodev_error(
+ dev_err(&interrupt_data->pci_dev->dev,
"Cannot get IRQ for interrupt %d, vector %d; "
"%d\n",
i, interrupt_data->msix_entries[i].vector, ret);
@@ -287,9 +288,8 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
int ret;
if (!gasket_dev->interrupt_data) {
- gasket_log_debug(
- gasket_dev,
- "Attempted to reinit uninitialized interrupt data.");
+ dev_dbg(gasket_dev->dev,
+ "Attempted to reinit uninitialized interrupt data\n");
return -EINVAL;
}
@@ -305,8 +305,8 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_debug(
- "Cannot handle unsupported interrupt type %d.",
+ dev_dbg(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
gasket_dev->interrupt_data->type);
ret = -EINVAL;
};
@@ -315,7 +315,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
/* Failing to setup MSIx will cause the device
* to report GASKET_STATUS_LAMED, but is not fatal.
*/
- gasket_log_warn(gasket_dev, "Couldn't init msix: %d", ret);
+ dev_warn(gasket_dev->dev, "Couldn't init msix: %d\n", ret);
return 0;
}
@@ -327,7 +327,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
/* See gasket_interrupt.h for description. */
int gasket_interrupt_reset_counts(struct gasket_dev *gasket_dev)
{
- gasket_log_debug(gasket_dev, "Clearing interrupt counts.");
+ dev_dbg(gasket_dev->dev, "Clearing interrupt counts\n");
memset(gasket_dev->interrupt_data->interrupt_counts, 0,
gasket_dev->interrupt_data->num_interrupts *
sizeof(*gasket_dev->interrupt_data->interrupt_counts));
@@ -351,12 +351,11 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
gasket_dev->interrupt_data;
if (!interrupt_data) {
- gasket_log_debug(
- gasket_dev, "Interrupt data is not initialized.");
+ dev_dbg(gasket_dev->dev, "Interrupt data is not initialized\n");
return;
}
- gasket_log_debug(gasket_dev, "Running interrupt setup.");
+ dev_dbg(gasket_dev->dev, "Running interrupt setup\n");
if (interrupt_data->type == PLATFORM_WIRE ||
interrupt_data->type == PCI_MSI) {
@@ -365,8 +364,8 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
}
if (interrupt_data->type != PCI_MSIX) {
- gasket_nodev_debug(
- "Cannot handle unsupported interrupt type %d.",
+ dev_dbg(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
return;
}
@@ -379,10 +378,9 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
* the register directly. If not, we need to deal with a read-
* modify-write and shift based on the packing index.
*/
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Setting up interrupt index %d with index 0x%llx and "
- "packing %d",
+ "packing %d\n",
interrupt_data->interrupts[i].index,
interrupt_data->interrupts[i].reg,
interrupt_data->interrupts[i].packing);
@@ -403,9 +401,9 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
pack_shift = 3 * interrupt_data->pack_width;
break;
default:
- gasket_nodev_debug(
+ dev_dbg(gasket_dev->dev,
"Found interrupt description with "
- "unknown enum %d.",
+ "unknown enum %d\n",
interrupt_data->interrupts[i].packing);
return;
}
@@ -445,8 +443,8 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_debug(
- "Cannot handle unsupported interrupt type %d.",
+ dev_dbg(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
};
@@ -460,18 +458,19 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
int gasket_interrupt_system_status(struct gasket_dev *gasket_dev)
{
if (!gasket_dev->interrupt_data) {
- gasket_nodev_debug("Interrupt data is null.");
+ dev_dbg(gasket_dev->dev, "Interrupt data is null\n");
return GASKET_STATUS_DEAD;
}
if (!gasket_dev->interrupt_data->msix_configured) {
- gasket_nodev_debug("Interrupt not initialized.");
+ dev_dbg(gasket_dev->dev, "Interrupt not initialized\n");
return GASKET_STATUS_LAMED;
}
if (gasket_dev->interrupt_data->num_configured !=
gasket_dev->interrupt_data->num_interrupts) {
- gasket_nodev_debug("Not all interrupts were configured.");
+ dev_dbg(gasket_dev->dev,
+ "Not all interrupts were configured\n");
return GASKET_STATUS_LAMED;
}
@@ -516,15 +515,13 @@ static ssize_t interrupt_sysfs_show(
gasket_dev = gasket_sysfs_get_device_data(device);
if (!gasket_dev) {
- gasket_nodev_debug(
- "No sysfs mapping found for device 0x%p", device);
+ dev_dbg(device, "No sysfs mapping found for device\n");
return 0;
}
gasket_attr = gasket_sysfs_get_attr(device, attr);
if (!gasket_attr) {
- gasket_nodev_debug(
- "No sysfs attr data found for device 0x%p", device);
+ dev_dbg(device, "No sysfs attr data found for device\n");
gasket_sysfs_put_device_data(device, gasket_dev);
return 0;
}
@@ -545,8 +542,8 @@ static ssize_t interrupt_sysfs_show(
ret = total_written;
break;
default:
- gasket_log_debug(
- gasket_dev, "Unknown attribute: %s", attr->attr.name);
+ dev_dbg(gasket_dev->dev, "Unknown attribute: %s\n",
+ attr->attr.name);
ret = 0;
break;
}
@@ -574,7 +571,7 @@ static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id)
}
}
if (interrupt == -1) {
- gasket_nodev_error("Received unknown irq %d", irq);
+ pr_err("Received unknown irq %d\n", irq);
return IRQ_HANDLED;
}
trace_gasket_interrupt_event(interrupt_data->name, interrupt);
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 04/10] staging: gasket: ioctl: convert to standard logging
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
` (2 preceding siblings ...)
2018-07-27 3:07 ` [PATCH 03/10] staging: gasket: interrupt: " Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 05/10] staging: gasket: page table: " Todd Poynor
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Replace gasket logging calls with standard logging calls.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_ioctl.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 63e139ab7ff89..78a132a60cc4f 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -5,9 +5,9 @@
#include "gasket_constants.h"
#include "gasket_core.h"
#include "gasket_interrupt.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include <linux/compiler.h>
+#include <linux/device.h>
#include <linux/fs.h>
#include <linux/uaccess.h>
@@ -73,7 +73,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
}
} else if (!gasket_ioctl_check_permissions(filp, cmd)) {
trace_gasket_ioctl_exit(-EPERM);
- gasket_log_debug(gasket_dev, "ioctl cmd=%x noperm.", cmd);
+ dev_dbg(gasket_dev->dev, "ioctl cmd=%x noperm\n", cmd);
return -EPERM;
}
@@ -132,10 +132,9 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
* the arg.
*/
trace_gasket_ioctl_integer_data(arg);
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Unknown ioctl cmd=0x%x not caught by "
- "gasket_is_supported_ioctl",
+ "gasket_is_supported_ioctl\n",
cmd);
retval = -EINVAL;
break;
@@ -186,12 +185,9 @@ static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
struct gasket_dev *gasket_dev = (struct gasket_dev *)filp->private_data;
alive = (gasket_dev->status == GASKET_STATUS_ALIVE);
- if (!alive) {
- gasket_nodev_error(
- "%s alive %d status %d.",
- __func__,
- alive, gasket_dev->status);
- }
+ if (!alive)
+ dev_dbg(gasket_dev->dev, "%s alive %d status %d\n",
+ __func__, alive, gasket_dev->status);
read = !!(filp->f_mode & FMODE_READ);
write = !!(filp->f_mode & FMODE_WRITE);
@@ -329,9 +325,8 @@ static int gasket_partition_page_table(
gasket_dev->page_table[ibuf.page_table_index]);
if (ibuf.size > max_page_table_size) {
- gasket_log_debug(
- gasket_dev,
- "Partition request 0x%llx too large, max is 0x%x.",
+ dev_dbg(gasket_dev->dev,
+ "Partition request 0x%llx too large, max is 0x%x\n",
ibuf.size, max_page_table_size);
return -EINVAL;
}
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 05/10] staging: gasket: page table: convert to standard logging
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
` (3 preceding siblings ...)
2018-07-27 3:07 ` [PATCH 04/10] staging: gasket: ioctl: " Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 06/10] staging: gasket: sysfs: " Todd Poynor
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Replace gasket logging calls with standard logging calls.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_page_table.c | 131 +++++++++------------
1 file changed, 54 insertions(+), 77 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 55ab59303247a..8ea8ea1c5174c 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -33,6 +33,7 @@
*/
#include "gasket_page_table.h"
+#include <linux/device.h>
#include <linux/file.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -43,7 +44,6 @@
#include "gasket_constants.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
/* Constants & utility macros */
/* The number of pages that can be mapped into each second-level page table. */
@@ -79,11 +79,6 @@
*/
#define GASKET_EXTENDED_LVL1_SHIFT 12
-/* Page-table specific error logging. */
-#define gasket_pg_tbl_error(pg_tbl, format, arg...) \
- gasket_dev_log(err, (pg_tbl)->device, (struct pci_dev *)NULL, format, \
- ##arg)
-
/* Type declarations */
/* Valid states for a struct gasket_page_table_entry. */
enum pte_status {
@@ -306,24 +301,23 @@ int gasket_page_table_init(
* hardware register that contains the page table size.
*/
if (total_entries == ULONG_MAX) {
- gasket_nodev_debug(
- "Error reading page table size. "
- "Initializing page table with size 0.");
+ dev_dbg(device, "Error reading page table size. "
+ "Initializing page table with size 0\n");
total_entries = 0;
}
- gasket_nodev_debug(
- "Attempting to initialize page table of size 0x%lx.",
+ dev_dbg(device,
+ "Attempting to initialize page table of size 0x%lx\n",
total_entries);
- gasket_nodev_debug(
- "Table has base reg 0x%x, extended offset reg 0x%x.",
+ dev_dbg(device,
+ "Table has base reg 0x%x, extended offset reg 0x%x\n",
page_table_config->base_reg,
page_table_config->extended_reg);
*ppg_tbl = kzalloc(sizeof(**ppg_tbl), GFP_KERNEL);
if (!*ppg_tbl) {
- gasket_nodev_debug("No memory for page table.");
+ dev_dbg(device, "No memory for page table\n");
return -ENOMEM;
}
@@ -332,8 +326,8 @@ int gasket_page_table_init(
if (bytes != 0) {
pg_tbl->entries = vzalloc(bytes);
if (!pg_tbl->entries) {
- gasket_nodev_debug(
- "No memory for address translation metadata.");
+ dev_dbg(device,
+ "No memory for address translation metadata\n");
kfree(pg_tbl);
*ppg_tbl = NULL;
return -ENOMEM;
@@ -361,7 +355,7 @@ int gasket_page_table_init(
pg_tbl->pci_dev = pci_dev;
pg_tbl->dma_ops = has_dma_ops;
- gasket_nodev_debug("Page table initialized successfully.");
+ dev_dbg(device, "Page table initialized successfully\n");
return 0;
}
@@ -398,7 +392,7 @@ int gasket_page_table_partition(
for (i = start; i < pg_tbl->config.total_entries; i++) {
if (pg_tbl->entries[i].status != PTE_FREE) {
- gasket_pg_tbl_error(pg_tbl, "entry %d is not free", i);
+ dev_err(pg_tbl->device, "entry %d is not free\n", i);
mutex_unlock(&pg_tbl->mutex);
return -EBUSY;
}
@@ -443,11 +437,9 @@ int gasket_page_table_map(
mutex_unlock(&pg_tbl->mutex);
- gasket_nodev_debug(
- "%s done: ha %llx daddr %llx num %d, "
- "ret %d\n",
- __func__,
- (unsigned long long)host_addr,
+ dev_dbg(pg_tbl->device,
+ "%s done: ha %llx daddr %llx num %d, ret %d\n",
+ __func__, (unsigned long long)host_addr,
(unsigned long long)dev_addr, num_pages, ret);
return ret;
}
@@ -562,9 +554,8 @@ bool gasket_page_table_are_addrs_bad(
ulong bytes)
{
if (host_addr & (PAGE_SIZE - 1)) {
- gasket_pg_tbl_error(
- pg_tbl,
- "host mapping address 0x%lx must be page aligned",
+ dev_err(pg_tbl->device,
+ "host mapping address 0x%lx must be page aligned\n",
host_addr);
return true;
}
@@ -580,16 +571,14 @@ bool gasket_page_table_is_dev_addr_bad(
uint num_pages = bytes / PAGE_SIZE;
if (bytes & (PAGE_SIZE - 1)) {
- gasket_pg_tbl_error(
- pg_tbl,
- "mapping size 0x%lX must be page aligned", bytes);
+ dev_err(pg_tbl->device,
+ "mapping size 0x%lX must be page aligned\n", bytes);
return true;
}
if (num_pages == 0) {
- gasket_pg_tbl_error(
- pg_tbl,
- "requested mapping is less than one page: %lu / %lu",
+ dev_err(pg_tbl->device,
+ "requested mapping is less than one page: %lu / %lu\n",
bytes, PAGE_SIZE);
return true;
}
@@ -644,7 +633,7 @@ int gasket_page_table_system_status(struct gasket_page_table *page_table)
return GASKET_STATUS_LAMED;
if (gasket_page_table_num_entries(page_table) == 0) {
- gasket_nodev_debug("Page table size is 0.");
+ dev_dbg(page_table->device, "Page table size is 0\n");
return GASKET_STATUS_LAMED;
}
@@ -682,9 +671,8 @@ static int gasket_map_simple_pages(
ret = gasket_alloc_simple_entries(pg_tbl, dev_addr, num_pages);
if (ret) {
- gasket_pg_tbl_error(
- pg_tbl,
- "page table slots %u (@ 0x%lx) to %u are not available",
+ dev_err(pg_tbl->device,
+ "page table slots %u (@ 0x%lx) to %u are not available\n",
slot_idx, dev_addr, slot_idx + num_pages - 1);
return ret;
}
@@ -695,7 +683,7 @@ static int gasket_map_simple_pages(
if (ret) {
gasket_page_table_unmap_nolock(pg_tbl, dev_addr, num_pages);
- gasket_pg_tbl_error(pg_tbl, "gasket_perform_mapping %d.", ret);
+ dev_err(pg_tbl->device, "gasket_perform_mapping %d\n", ret);
}
return ret;
}
@@ -732,10 +720,9 @@ static int gasket_map_extended_pages(
ret = gasket_alloc_extended_entries(pg_tbl, dev_addr, num_pages);
if (ret) {
dev_addr_end = dev_addr + (num_pages / PAGE_SIZE) - 1;
- gasket_pg_tbl_error(
- pg_tbl,
+ dev_err(pg_tbl->device,
"page table slots (%lu,%lu) (@ 0x%lx) to (%lu,%lu) are "
- "not available",
+ "not available\n",
gasket_extended_lvl0_page_idx(pg_tbl, dev_addr),
dev_addr,
gasket_extended_lvl1_page_idx(pg_tbl, dev_addr),
@@ -843,7 +830,7 @@ static int gasket_perform_mapping(
for (i = 0; i < num_pages; i++) {
page_addr = host_addr + i * PAGE_SIZE;
offset = page_addr & (PAGE_SIZE - 1);
- gasket_nodev_debug("%s i %d\n", __func__, i);
+ dev_dbg(pg_tbl->device, "%s i %d\n", __func__, i);
if (is_coherent(pg_tbl, host_addr)) {
u64 off =
(u64)host_addr -
@@ -857,10 +844,9 @@ static int gasket_perform_mapping(
page_addr - offset, 1, 1, &page);
if (ret <= 0) {
- gasket_pg_tbl_error(
- pg_tbl,
+ dev_err(pg_tbl->device,
"get user pages failed for addr=0x%lx, "
- "offset=0x%lx [ret=%d]",
+ "offset=0x%lx [ret=%d]\n",
page_addr, offset, ret);
return ret ? ret : -ENOMEM;
}
@@ -880,21 +866,17 @@ static int gasket_perform_mapping(
DMA_BIDIRECTIONAL);
}
- gasket_nodev_debug(
- "%s dev %p "
- "i %d pte %p pfn %p -> mapped %llx\n",
- __func__,
- pg_tbl->device, i, &ptes[i],
+ dev_dbg(pg_tbl->device,
+ "%s i %d pte %p pfn %p -> mapped %llx\n",
+ __func__, i, &ptes[i],
(void *)page_to_pfn(page),
(unsigned long long)ptes[i].dma_addr);
if (ptes[i].dma_addr == -1) {
- gasket_nodev_debug(
- "%s i %d"
- " -> fail to map page %llx "
+ dev_dbg(pg_tbl->device,
+ "%s i %d -> fail to map page %llx "
"[pfn %p ohys %p]\n",
- __func__,
- i,
+ __func__, i,
(unsigned long long)ptes[i].dma_addr,
(void *)page_to_pfn(page),
(void *)page_to_phys(page));
@@ -996,9 +978,8 @@ static int gasket_alloc_extended_entries(
if (pte->status == PTE_FREE) {
ret = gasket_alloc_extended_subtable(pg_tbl, pte, slot);
if (ret) {
- gasket_pg_tbl_error(
- pg_tbl,
- "no memory for extended addr subtable");
+ dev_err(pg_tbl->device,
+ "no memory for extended addr subtable\n");
return ret;
}
} else {
@@ -1309,23 +1290,21 @@ static bool gasket_is_simple_dev_addr_bad(
if (gasket_components_to_dev_address(
pg_tbl, 1, page_index, page_offset) != dev_addr) {
- gasket_pg_tbl_error(
- pg_tbl, "address is invalid, 0x%lX", dev_addr);
+ dev_err(pg_tbl->device, "address is invalid, 0x%lX\n",
+ dev_addr);
return true;
}
if (page_index >= pg_tbl->num_simple_entries) {
- gasket_pg_tbl_error(
- pg_tbl,
- "starting slot at %lu is too large, max is < %u",
+ dev_err(pg_tbl->device,
+ "starting slot at %lu is too large, max is < %u\n",
page_index, pg_tbl->num_simple_entries);
return true;
}
if (page_index + num_pages > pg_tbl->num_simple_entries) {
- gasket_pg_tbl_error(
- pg_tbl,
- "ending slot at %lu is too large, max is <= %u",
+ dev_err(pg_tbl->device,
+ "ending slot at %lu is too large, max is <= %u\n",
page_index + num_pages, pg_tbl->num_simple_entries);
return true;
}
@@ -1354,8 +1333,8 @@ static bool gasket_is_extended_dev_addr_bad(
/* check if the device address is out of bound */
addr = dev_addr & ~((pg_tbl)->extended_flag);
if (addr >> (GASKET_EXTENDED_LVL0_WIDTH + GASKET_EXTENDED_LVL0_SHIFT)) {
- gasket_pg_tbl_error(pg_tbl, "device address out of bound, 0x%p",
- (void *)dev_addr);
+ dev_err(pg_tbl->device, "device address out of bound, 0x%p\n",
+ (void *)dev_addr);
return true;
}
@@ -1372,23 +1351,21 @@ static bool gasket_is_extended_dev_addr_bad(
if (gasket_components_to_dev_address(
pg_tbl, 0, page_global_idx, page_offset) != dev_addr) {
- gasket_pg_tbl_error(
- pg_tbl, "address is invalid, 0x%p", (void *)dev_addr);
+ dev_err(pg_tbl->device, "address is invalid, 0x%p\n",
+ (void *)dev_addr);
return true;
}
if (page_lvl0_idx >= pg_tbl->num_extended_entries) {
- gasket_pg_tbl_error(
- pg_tbl,
+ dev_err(pg_tbl->device,
"starting level 0 slot at %lu is too large, max is < "
- "%u", page_lvl0_idx, pg_tbl->num_extended_entries);
+ "%u\n", page_lvl0_idx, pg_tbl->num_extended_entries);
return true;
}
if (page_lvl0_idx + num_lvl0_pages > pg_tbl->num_extended_entries) {
- gasket_pg_tbl_error(
- pg_tbl,
- "ending level 0 slot at %lu is too large, max is <= %u",
+ dev_err(pg_tbl->device,
+ "ending level 0 slot at %lu is too large, max is <= %u\n",
page_lvl0_idx + num_lvl0_pages,
pg_tbl->num_extended_entries);
return true;
@@ -1597,8 +1574,8 @@ int gasket_set_user_virt(
*/
pg_tbl = gasket_dev->page_table[0];
if (!pg_tbl) {
- gasket_nodev_debug(
- "%s: invalid page table index", __func__);
+ dev_dbg(gasket_dev->dev, "%s: invalid page table index\n",
+ __func__);
return 0;
}
for (j = 0; j < num_pages; j++) {
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 06/10] staging: gasket: sysfs: convert to standard logging
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
` (4 preceding siblings ...)
2018-07-27 3:07 ` [PATCH 05/10] staging: gasket: page table: " Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 15:07 ` Greg Kroah-Hartman
2018-07-27 3:07 ` [PATCH 07/10] staging: gasket: apex: " Todd Poynor
` (3 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Drop gasket logging calls in favor of standard logging.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_sysfs.c | 73 +++++++++++++--------------
1 file changed, 35 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
index 1c5f7502e0d5e..418b81797e638 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -3,7 +3,9 @@
#include "gasket_sysfs.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
+
+#include <linux/device.h>
+#include <linux/printk.h>
/*
* Pair of kernel device and user-specified pointer. Used in lookups in sysfs
@@ -66,7 +68,7 @@ static struct gasket_sysfs_mapping *get_mapping(struct device *device)
int i;
if (!device) {
- gasket_nodev_error("Received NULL device!");
+ pr_debug("%s: Received NULL device\n", __func__);
return NULL;
}
@@ -80,7 +82,8 @@ static struct gasket_sysfs_mapping *get_mapping(struct device *device)
mutex_unlock(&dev_mappings[i].mutex);
}
- gasket_nodev_info("Mapping to device %s not found.", device->kobj.name);
+ dev_dbg(device, "%s: Mapping to device %s not found\n",
+ __func__, device->kobj.name);
return NULL;
}
@@ -103,16 +106,15 @@ static void put_mapping(struct gasket_sysfs_mapping *mapping)
struct device *device;
if (!mapping) {
- gasket_nodev_info("Mapping should not be NULL.");
+ pr_debug("%s: Mapping should not be NULL\n", __func__);
return;
}
mutex_lock(&mapping->mutex);
if (refcount_read(&mapping->refcount.refcount) == 0)
- gasket_nodev_error("Refcount is already 0!");
+ dev_err(mapping->device, "Refcount is already 0\n");
if (kref_put(&mapping->refcount, release_entry)) {
- gasket_nodev_info("Removing Gasket sysfs mapping, device %s",
- mapping->device->kobj.name);
+ dev_dbg(mapping->device, "Removing Gasket sysfs mapping\n");
/*
* We can't remove the sysfs nodes in the kref callback, since
* device_remove_file() blocks until the node is free.
@@ -182,16 +184,13 @@ int gasket_sysfs_create_mapping(
static DEFINE_MUTEX(function_mutex);
mutex_lock(&function_mutex);
-
- gasket_nodev_info(
- "Creating sysfs entries for device pointer 0x%p.", device);
+ dev_dbg(device, "Creating sysfs entries for device\n");
/* Check that the device we're adding hasn't already been added. */
mapping = get_mapping(device);
if (mapping) {
- gasket_nodev_error(
- "Attempting to re-initialize sysfs mapping for device "
- "0x%p.", device);
+ dev_err(device,
+ "Attempting to re-initialize sysfs mapping for device\n");
put_mapping(mapping);
mutex_unlock(&function_mutex);
return -EBUSY;
@@ -207,13 +206,13 @@ int gasket_sysfs_create_mapping(
}
if (map_idx == GASKET_SYSFS_NUM_MAPPINGS) {
- gasket_nodev_error("All mappings have been exhausted!");
+ dev_err(device, "All mappings have been exhausted\n");
mutex_unlock(&function_mutex);
return -ENOMEM;
}
- gasket_nodev_info(
- "Creating sysfs mapping for device %s.", device->kobj.name);
+ dev_dbg(device, "Creating sysfs mapping for device %s\n",
+ device->kobj.name);
mapping = &dev_mappings[map_idx];
kref_init(&mapping->refcount);
@@ -224,7 +223,7 @@ int gasket_sysfs_create_mapping(
GFP_KERNEL);
mapping->attribute_count = 0;
if (!mapping->attributes) {
- gasket_nodev_error("Unable to allocate sysfs attribute array.");
+ dev_dbg(device, "Unable to allocate sysfs attribute array\n");
mapping->device = NULL;
mapping->gasket_dev = NULL;
mutex_unlock(&mapping->mutex);
@@ -247,10 +246,9 @@ int gasket_sysfs_create_entries(
struct gasket_sysfs_mapping *mapping = get_mapping(device);
if (!mapping) {
- gasket_nodev_error(
- "Creating entries for device 0x%p without first "
- "initializing mapping.",
- device);
+ dev_dbg(device,
+ "Creating entries for device without first "
+ "initializing mapping\n");
return -EINVAL;
}
@@ -258,9 +256,9 @@ int gasket_sysfs_create_entries(
for (i = 0; strcmp(attrs[i].attr.attr.name, GASKET_ARRAY_END_MARKER);
i++) {
if (mapping->attribute_count == GASKET_SYSFS_MAX_NODES) {
- gasket_nodev_error(
+ dev_err(device,
"Maximum number of sysfs nodes reached for "
- "device.");
+ "device\n");
mutex_unlock(&mapping->mutex);
put_mapping(mapping);
return -ENOMEM;
@@ -268,7 +266,7 @@ int gasket_sysfs_create_entries(
ret = device_create_file(device, &attrs[i].attr);
if (ret) {
- gasket_nodev_error("Unable to create device entries");
+ dev_dbg(device, "Unable to create device entries\n");
mutex_unlock(&mapping->mutex);
put_mapping(mapping);
return ret;
@@ -289,10 +287,9 @@ void gasket_sysfs_remove_mapping(struct device *device)
struct gasket_sysfs_mapping *mapping = get_mapping(device);
if (!mapping) {
- gasket_nodev_error(
+ dev_err(device,
"Attempted to remove non-existent sysfs mapping to "
- "device 0x%p",
- device);
+ "device\n");
return;
}
@@ -304,7 +301,7 @@ struct gasket_dev *gasket_sysfs_get_device_data(struct device *device)
struct gasket_sysfs_mapping *mapping = get_mapping(device);
if (!mapping) {
- gasket_nodev_error("device %p not registered.", device);
+ dev_err(device, "device not registered\n");
return NULL;
}
@@ -342,8 +339,8 @@ struct gasket_sysfs_attribute *gasket_sysfs_get_attr(
return &attrs[i];
}
- gasket_nodev_error("Unable to find match for device_attribute %s",
- attr->attr.name);
+ dev_err(device, "Unable to find match for device_attribute %s\n",
+ attr->attr.name);
return NULL;
}
EXPORT_SYMBOL(gasket_sysfs_get_attr);
@@ -368,8 +365,8 @@ void gasket_sysfs_put_attr(
}
}
- gasket_nodev_error(
- "Unable to put unknown attribute: %s", attr->attr.attr.name);
+ dev_err(device, "Unable to put unknown attribute: %s\n",
+ attr->attr.attr.name);
}
EXPORT_SYMBOL(gasket_sysfs_put_attr);
@@ -383,26 +380,26 @@ ssize_t gasket_sysfs_register_store(
struct gasket_sysfs_attribute *gasket_attr;
if (count < 3 || buf[0] != '0' || buf[1] != 'x') {
- gasket_nodev_error(
- "sysfs register write format: \"0x<hex value>\".");
+ dev_err(device,
+ "sysfs register write format: \"0x<hex value>\"\n");
return -EINVAL;
}
if (kstrtoul(buf, 16, &parsed_value) != 0) {
- gasket_nodev_error(
- "Unable to parse input as 64-bit hex value: %s.", buf);
+ dev_err(device,
+ "Unable to parse input as 64-bit hex value: %s\n", buf);
return -EINVAL;
}
mapping = get_mapping(device);
if (!mapping) {
- gasket_nodev_info("Device driver may have been removed.");
+ dev_err(device, "Device driver may have been removed\n");
return 0;
}
gasket_dev = mapping->gasket_dev;
if (!gasket_dev) {
- gasket_nodev_info("Device driver may have been removed.");
+ dev_err(device, "Device driver may have been removed\n");
return 0;
}
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 07/10] staging: gasket: apex: convert to standard logging
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
` (5 preceding siblings ...)
2018-07-27 3:07 ` [PATCH 06/10] staging: gasket: sysfs: " Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 08/10] staging: gasket: remove gasket logging header Todd Poynor
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Drop gasket logging calls in favor of standard logging.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/apex_driver.c | 61 ++++++++++++----------------
1 file changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 6396b18b246a5..73fc2683a834d 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -7,11 +7,13 @@
#include <linux/compiler.h>
#include <linux/delay.h>
+#include <linux/device.h>
#include <linux/fs.h>
#include <linux/init.h>
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/printk.h>
#include <linux/sched.h>
#include <linux/uaccess.h>
@@ -19,7 +21,6 @@
#include "gasket_core.h"
#include "gasket_interrupt.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include "gasket_sysfs.h"
@@ -362,11 +363,9 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev)
if (retries == APEX_RESET_RETRY) {
if (!page_table_ready)
- gasket_log_error(
- gasket_dev, "Page table init timed out.");
+ dev_err(gasket_dev->dev, "Page table init timed out\n");
if (!msix_table_ready)
- gasket_log_error(
- gasket_dev, "MSI-X table init timed out.");
+ dev_err(gasket_dev->dev, "MSI-X table init timed out\n");
return -ETIMEDOUT;
}
@@ -420,12 +419,9 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
- gasket_log_debug(
- gasket_dev,
- "%s 0x%p hib_error 0x%llx scalar_error "
- "0x%llx.",
- __func__,
- gasket_dev, hib_error, scalar_error);
+ 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);
@@ -452,7 +448,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type)
/* We are not in reset - toggle the reset bit so as to force
* re-init of custom block
*/
- gasket_log_debug(gasket_dev, "%s: toggle reset.", __func__);
+ dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__);
ret = apex_enter_reset(gasket_dev, type);
if (ret)
@@ -489,9 +485,9 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_USER_HIB_DMA_PAUSED, 1, 1,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(gasket_dev,
- "DMAs did not quiesce within timeout (%d ms)",
- APEX_RESET_RETRY * APEX_RESET_DELAY);
+ dev_err(gasket_dev->dev,
+ "DMAs did not quiesce within timeout (%d ms)\n",
+ APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -511,9 +507,8 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCU_3, 1 << 6, 1 << 6,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
- gasket_dev,
- "RAM did not shut down within timeout (%d ms)",
+ dev_err(gasket_dev->dev,
+ "RAM did not shut down within timeout (%d ms)\n",
APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -560,9 +555,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCU_3, 1 << 6, 0,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
- gasket_dev,
- "RAM did not enable within timeout (%d ms)",
+ dev_err(gasket_dev->dev,
+ "RAM did not enable within timeout (%d ms)\n",
APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -572,9 +566,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
APEX_BAR2_REG_SCU_3,
SCU3_CUR_RST_GCB_BIT_MASK, 0,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
- gasket_dev,
- "GCB did not leave reset within timeout (%d ms)",
+ dev_err(gasket_dev->dev,
+ "GCB did not leave reset within timeout (%d ms)\n",
APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -589,9 +582,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
val1 = gasket_dev_read_32(
gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
- gasket_log_debug(
- gasket_dev, "Disallow HW clock gating 0x%x -> 0x%x",
- val0, val1);
+ 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);
@@ -602,9 +594,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
val1 = gasket_dev_read_32(
gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
- gasket_log_debug(
- gasket_dev, "Allow HW clock gating 0x%x -> 0x%x", val0,
- val1);
+ dev_dbg(gasket_dev->dev, "Allow HW clock gating 0x%x -> 0x%x\n",
+ val0, val1);
}
return 0;
@@ -668,7 +659,7 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
if (copy_from_user(&ibuf, argp, sizeof(ibuf)))
return -EFAULT;
- gasket_log_debug(gasket_dev, "%s %llu", __func__, ibuf.enable);
+ dev_dbg(gasket_dev->dev, "%s %llu\n", __func__, ibuf.enable);
if (ibuf.enable) {
/* Quiesce AXI, gate GCB clock. */
@@ -709,13 +700,13 @@ static ssize_t sysfs_show(
gasket_dev = gasket_sysfs_get_device_data(device);
if (!gasket_dev) {
- gasket_nodev_error("No Apex device sysfs mapping found");
+ dev_err(device, "No Apex device sysfs mapping found\n");
return -ENODEV;
}
gasket_attr = gasket_sysfs_get_attr(device, attr);
if (!gasket_attr) {
- gasket_nodev_error("No Apex device sysfs attr data found");
+ dev_err(device, "No Apex device sysfs attr data found\n");
gasket_sysfs_put_device_data(device, gasket_dev);
return -ENODEV;
}
@@ -738,8 +729,8 @@ static ssize_t sysfs_show(
gasket_dev->page_table[0]));
break;
default:
- gasket_log_debug(
- gasket_dev, "Unknown attribute: %s", attr->attr.name);
+ dev_dbg(gasket_dev->dev, "Unknown attribute: %s\n",
+ attr->attr.name);
ret = 0;
break;
}
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 08/10] staging: gasket: remove gasket logging header
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
` (6 preceding siblings ...)
2018-07-27 3:07 ` [PATCH 07/10] staging: gasket: apex: " Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 09/10] staging: gasket: TODO: remove entry for convert to standard logging Todd Poynor
2018-07-27 3:07 ` [PATCH 10/10] staging: gasket: don't print device addresses as kernel pointers Todd Poynor
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Gasket logging functions no longer used.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_logging.h | 64 -------------------------
1 file changed, 64 deletions(-)
delete mode 100644 drivers/staging/gasket/gasket_logging.h
diff --git a/drivers/staging/gasket/gasket_logging.h b/drivers/staging/gasket/gasket_logging.h
deleted file mode 100644
index 54bbe516b0716..0000000000000
--- a/drivers/staging/gasket/gasket_logging.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Common logging utilities for the Gasket driver framework.
- *
- * Copyright (C) 2018 Google, Inc.
- */
-
-#include <linux/pci.h>
-#include <linux/printk.h>
-
-#ifndef _GASKET_LOGGING_H_
-#define _GASKET_LOGGING_H_
-
-/* Base macro; other logging can/should be built on top of this. */
-#define gasket_dev_log(level, device, pci_dev, format, arg...) \
- if (false) { \
- if (pci_dev) { \
- dev_##level(&(pci_dev)->dev, "%s: " format "\n", \
- __func__, ##arg); \
- } else { \
- gasket_nodev_log(level, format, ##arg); \
- } \
- }
-
-/* "No-device" logging macros. */
-#define gasket_nodev_log(level, format, arg...) \
- if (false) pr_##level("gasket: %s: " format "\n", __func__, ##arg)
-
-#define gasket_nodev_debug(format, arg...) \
- if (false) gasket_nodev_log(debug, format, ##arg)
-
-#define gasket_nodev_info(format, arg...) \
- if (false) gasket_nodev_log(info, format, ##arg)
-
-#define gasket_nodev_warn(format, arg...) \
- if (false) gasket_nodev_log(warn, format, ##arg)
-
-#define gasket_nodev_error(format, arg...) \
- if (false) gasket_nodev_log(err, format, ##arg)
-
-/* gasket_dev logging macros */
-#define gasket_log_info(gasket_dev, format, arg...) \
- if (false) gasket_dev_log(info, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_warn(gasket_dev, format, arg...) \
- if (false) gasket_dev_log(warn, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_error(gasket_dev, format, arg...) \
- if (false) gasket_dev_log(err, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_debug(gasket_dev, format, arg...) \
- if (false){ \
- if ((gasket_dev)->pci_dev) { \
- dev_dbg(&((gasket_dev)->pci_dev)->dev, "%s: " format \
- "\n", __func__, ##arg); \
- } else { \
- gasket_nodev_log(debug, format, ##arg); \
- } \
- }
-
-#endif
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 09/10] staging: gasket: TODO: remove entry for convert to standard logging
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
` (7 preceding siblings ...)
2018-07-27 3:07 ` [PATCH 08/10] staging: gasket: remove gasket logging header Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 10/10] staging: gasket: don't print device addresses as kernel pointers Todd Poynor
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Gasket/apex drivers now use standard logging, remove TODO entry for
this.
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 d3c44ca4fda25..fb71997fb5612 100644
--- a/drivers/staging/gasket/TODO
+++ b/drivers/staging/gasket/TODO
@@ -4,7 +4,6 @@ staging directory.
- Document sysfs files with Documentation/ABI/ entries.
- Use misc interface instead of major number for driver version description.
- Add descriptions of module_param's
-- Remove gasket-specific logging functions.
- 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:
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 10/10] staging: gasket: don't print device addresses as kernel pointers
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
` (8 preceding siblings ...)
2018-07-27 3:07 ` [PATCH 09/10] staging: gasket: TODO: remove entry for convert to standard logging Todd Poynor
@ 2018-07-27 3:07 ` Todd Poynor
9 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 3:07 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: devel, linux-kernel, Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Print device addresses as unsigned long, not as kernel pointers.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_page_table.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 8ea8ea1c5174c..32f1c1e10c7e2 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -1333,8 +1333,8 @@ static bool gasket_is_extended_dev_addr_bad(
/* check if the device address is out of bound */
addr = dev_addr & ~((pg_tbl)->extended_flag);
if (addr >> (GASKET_EXTENDED_LVL0_WIDTH + GASKET_EXTENDED_LVL0_SHIFT)) {
- dev_err(pg_tbl->device, "device address out of bound, 0x%p\n",
- (void *)dev_addr);
+ dev_err(pg_tbl->device, "device address out of bounds: 0x%lx\n",
+ dev_addr);
return true;
}
@@ -1351,8 +1351,8 @@ static bool gasket_is_extended_dev_addr_bad(
if (gasket_components_to_dev_address(
pg_tbl, 0, page_global_idx, page_offset) != dev_addr) {
- dev_err(pg_tbl->device, "address is invalid, 0x%p\n",
- (void *)dev_addr);
+ dev_err(pg_tbl->device, "address is invalid: 0x%lx\n",
+ dev_addr);
return true;
}
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 06/10] staging: gasket: sysfs: convert to standard logging
2018-07-27 3:07 ` [PATCH 06/10] staging: gasket: sysfs: " Todd Poynor
@ 2018-07-27 15:07 ` Greg Kroah-Hartman
2018-07-27 17:00 ` Todd Poynor
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2018-07-27 15:07 UTC (permalink / raw)
To: Todd Poynor
Cc: Rob Springer, John Joseph, Ben Chan, devel, Todd Poynor, linux-kernel
On Thu, Jul 26, 2018 at 08:07:33PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
>
> Drop gasket logging calls in favor of standard logging.
>
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> ---
> drivers/staging/gasket/gasket_sysfs.c | 73 +++++++++++++--------------
> 1 file changed, 35 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
> index 1c5f7502e0d5e..418b81797e638 100644
> --- a/drivers/staging/gasket/gasket_sysfs.c
> +++ b/drivers/staging/gasket/gasket_sysfs.c
> @@ -3,7 +3,9 @@
> #include "gasket_sysfs.h"
>
> #include "gasket_core.h"
> -#include "gasket_logging.h"
> +
> +#include <linux/device.h>
> +#include <linux/printk.h>
>
> /*
> * Pair of kernel device and user-specified pointer. Used in lookups in sysfs
> @@ -66,7 +68,7 @@ static struct gasket_sysfs_mapping *get_mapping(struct device *device)
> int i;
>
> if (!device) {
> - gasket_nodev_error("Received NULL device!");
> + pr_debug("%s: Received NULL device\n", __func__);
I know you are just trying to clean up the logging mess, but this type
of check isn't even needed, as it's impossible.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/10] staging: gasket: save struct device for a gasket device
2018-07-27 3:07 ` [PATCH 01/10] staging: gasket: save struct device for a gasket device Todd Poynor
@ 2018-07-27 15:08 ` Greg Kroah-Hartman
2018-07-27 17:02 ` Todd Poynor
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2018-07-27 15:08 UTC (permalink / raw)
To: Todd Poynor
Cc: Rob Springer, John Joseph, Ben Chan, devel, Todd Poynor, linux-kernel
On Thu, Jul 26, 2018 at 08:07:28PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
>
> Save the struct device pointer to a gasket device in gasket's metadata,
> to facilitate use of standard logging calls and in anticipation of
> non-PCI gasket devices in the future.
>
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> ---
> drivers/staging/gasket/gasket_core.c | 5 +++--
> drivers/staging/gasket/gasket_core.h | 3 +++
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
> index 732218773c3c6..e8f3b021c20d1 100644
> --- a/drivers/staging/gasket/gasket_core.c
> +++ b/drivers/staging/gasket/gasket_core.c
> @@ -450,6 +450,7 @@ static int gasket_alloc_dev(
> gasket_dev->internal_desc = internal_desc;
> gasket_dev->dev_idx = dev_idx;
> snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", kobj_name);
> + gasket_dev->dev = parent;
Normally when saving off a pointer to an object that you reference later
on, and that you rely on, you need to grab a reference to it, otherwise
it may disappear at any point in time.
However this whole "wrap the pci layer" nonsense is a total mess with
the lifetime rules of devices, so it's probably the least of your
worries....
As long as you know you will have to fix that crud up, and what you are
doing here will bite you if you do not do it right, that's fine with
me...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 06/10] staging: gasket: sysfs: convert to standard logging
2018-07-27 15:07 ` Greg Kroah-Hartman
@ 2018-07-27 17:00 ` Todd Poynor
0 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 17:00 UTC (permalink / raw)
To: Greg KH; +Cc: toddpoynor, Rob Springer, John Joseph, benchan, devel, LKML
On Fri, Jul 27, 2018 at 8:07 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 26, 2018 at 08:07:33PM -0700, Todd Poynor wrote:
> > From: Todd Poynor <toddpoynor@google.com>
> >
> > Drop gasket logging calls in favor of standard logging.
> >
> > Signed-off-by: Todd Poynor <toddpoynor@google.com>
> > ---
> > drivers/staging/gasket/gasket_sysfs.c | 73 +++++++++++++--------------
> > 1 file changed, 35 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
> > index 1c5f7502e0d5e..418b81797e638 100644
> > --- a/drivers/staging/gasket/gasket_sysfs.c
> > +++ b/drivers/staging/gasket/gasket_sysfs.c
> > @@ -3,7 +3,9 @@
> > #include "gasket_sysfs.h"
> >
> > #include "gasket_core.h"
> > -#include "gasket_logging.h"
> > +
> > +#include <linux/device.h>
> > +#include <linux/printk.h>
> >
> > /*
> > * Pair of kernel device and user-specified pointer. Used in lookups in sysfs
> > @@ -66,7 +68,7 @@ static struct gasket_sysfs_mapping *get_mapping(struct device *device)
> > int i;
> >
> > if (!device) {
> > - gasket_nodev_error("Received NULL device!");
> > + pr_debug("%s: Received NULL device\n", __func__);
>
> I know you are just trying to clean up the logging mess, but this type
> of check isn't even needed, as it's impossible.
Ah, yeah, I noticed it when I was converting, I'll put this on my list
to clean up in next round, thanks.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/10] staging: gasket: save struct device for a gasket device
2018-07-27 15:08 ` Greg Kroah-Hartman
@ 2018-07-27 17:02 ` Todd Poynor
0 siblings, 0 replies; 15+ messages in thread
From: Todd Poynor @ 2018-07-27 17:02 UTC (permalink / raw)
To: Greg KH; +Cc: toddpoynor, Rob Springer, John Joseph, benchan, devel, LKML
On Fri, Jul 27, 2018 at 8:09 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 26, 2018 at 08:07:28PM -0700, Todd Poynor wrote:
> > From: Todd Poynor <toddpoynor@google.com>
> >
> > Save the struct device pointer to a gasket device in gasket's metadata,
> > to facilitate use of standard logging calls and in anticipation of
> > non-PCI gasket devices in the future.
> >
> > Signed-off-by: Todd Poynor <toddpoynor@google.com>
> > ---
> > drivers/staging/gasket/gasket_core.c | 5 +++--
> > drivers/staging/gasket/gasket_core.h | 3 +++
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
> > index 732218773c3c6..e8f3b021c20d1 100644
> > --- a/drivers/staging/gasket/gasket_core.c
> > +++ b/drivers/staging/gasket/gasket_core.c
> > @@ -450,6 +450,7 @@ static int gasket_alloc_dev(
> > gasket_dev->internal_desc = internal_desc;
> > gasket_dev->dev_idx = dev_idx;
> > snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", kobj_name);
> > + gasket_dev->dev = parent;
>
> Normally when saving off a pointer to an object that you reference later
> on, and that you rely on, you need to grab a reference to it, otherwise
> it may disappear at any point in time.
Got it, I'll clean that up next round, thanks.
>
> However this whole "wrap the pci layer" nonsense is a total mess with
> the lifetime rules of devices, so it's probably the least of your
> worries....
That's high on the list to fix up, too.
>
> As long as you know you will have to fix that crud up, and what you are
> doing here will bite you if you do not do it right, that's fine with
> me...
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-07-27 17:02 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27 3:07 [PATCH 00/10] staging: gasket: logging cleanups Todd Poynor
2018-07-27 3:07 ` [PATCH 01/10] staging: gasket: save struct device for a gasket device Todd Poynor
2018-07-27 15:08 ` Greg Kroah-Hartman
2018-07-27 17:02 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 02/10] staging: gasket: core: convert to standard logging Todd Poynor
2018-07-27 3:07 ` [PATCH 03/10] staging: gasket: interrupt: " Todd Poynor
2018-07-27 3:07 ` [PATCH 04/10] staging: gasket: ioctl: " Todd Poynor
2018-07-27 3:07 ` [PATCH 05/10] staging: gasket: page table: " Todd Poynor
2018-07-27 3:07 ` [PATCH 06/10] staging: gasket: sysfs: " Todd Poynor
2018-07-27 15:07 ` Greg Kroah-Hartman
2018-07-27 17:00 ` Todd Poynor
2018-07-27 3:07 ` [PATCH 07/10] staging: gasket: apex: " Todd Poynor
2018-07-27 3:07 ` [PATCH 08/10] staging: gasket: remove gasket logging header Todd Poynor
2018-07-27 3:07 ` [PATCH 09/10] staging: gasket: TODO: remove entry for convert to standard logging Todd Poynor
2018-07-27 3:07 ` [PATCH 10/10] staging: gasket: don't print device addresses as kernel pointers 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.