linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] staging: gasket: fixes
@ 2018-10-15  4:59 Todd Poynor
  2018-10-15  4:59 ` [PATCH 01/11] staging: gasket: core: debug log updates Todd Poynor
                   ` (12 more replies)
  0 siblings, 13 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Various fixes for gasket/apex drivers.

Nick Ewalt (8):
  staging: gasket: page_table: remove unnecessary PTE status set to free
  staging: gasket: page_table: rearrange gasket_page_table_entry
  staging: gasket: page table: fixup error path allocating coherent mem
  staging: gasket: page_table: fix comment in components_to_dev_address
  staging: gasket: page_table: simplify gasket_components_to_dev_address
  staging: gasket: apex: fix sysfs_show
  staging: gasket: sysfs: fix attribute release comment
  staging: gasket: Update device virtual address comment

Todd Poynor (3):
  staging: gasket: core: debug log updates
  staging: gasket: page table: return valid error code on map fail
  staging: gasket: page table: remove dead code in coherent mem alloc

 drivers/staging/gasket/apex_driver.c       |  2 +-
 drivers/staging/gasket/gasket_core.c       |  9 +--
 drivers/staging/gasket/gasket_page_table.c | 65 +++++++++-------------
 drivers/staging/gasket/gasket_sysfs.h      |  4 +-
 4 files changed, 33 insertions(+), 47 deletions(-)

-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 01/11] staging: gasket: core: debug log updates
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  7:34   ` Greg Kroah-Hartman
  2018-10-15  4:59 ` [PATCH 02/11] staging: gasket: page table: return valid error code on map fail Todd Poynor
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Add debug logs for device enable/disable events, remove logs for
callbacks (the called functions can generate their own logs if needed).

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

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index f230bec76ae4e..62a7515915e59 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -109,8 +109,6 @@ check_and_invoke_callback(struct gasket_dev *gasket_dev,
 {
 	int ret = 0;
 
-	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);
@@ -126,11 +124,8 @@ gasket_check_and_invoke_callback_nolock(struct gasket_dev *gasket_dev,
 {
 	int ret = 0;
 
-	if (cb_function) {
-		dev_dbg(gasket_dev->dev,
-			"Invoking device-specific callback.\n");
+	if (cb_function)
 		ret = cb_function(gasket_dev);
-	}
 	return ret;
 }
 
@@ -633,6 +628,7 @@ void gasket_disable_device(struct gasket_dev *gasket_dev)
 		gasket_dev->internal_desc->driver_desc;
 	int i;
 
+	dev_dbg(gasket_dev->dev, "disabling device\n");
 	/* Only delete the device if it has been successfully added. */
 	if (gasket_dev->dev_info.cdev_added)
 		cdev_del(&gasket_dev->dev_info.cdev);
@@ -1357,6 +1353,7 @@ int gasket_enable_device(struct gasket_dev *gasket_dev)
 	const struct gasket_driver_desc *driver_desc =
 		gasket_dev->internal_desc->driver_desc;
 
+	dev_dbg(gasket_dev->dev, "enabling device\n");
 	ret = gasket_interrupt_init(gasket_dev);
 	if (ret) {
 		dev_err(gasket_dev->dev,
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 02/11] staging: gasket: page table: return valid error code on map fail
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
  2018-10-15  4:59 ` [PATCH 01/11] staging: gasket: core: debug log updates Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 03/11] staging: gasket: page table: remove dead code in coherent mem alloc Todd Poynor
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Return -EINVAL on mapping failures, instead of -1, which triggers a
checkpatch error.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 964146f0df526..2e1de8ad4a2c6 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -514,13 +514,12 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl,
 					(void *)page_to_pfn(page),
 					(void *)page_to_phys(page));
 
-				/* clean up */
 				if (gasket_release_page(ptes[i].page))
 					--pg_tbl->num_active_pages;
 
 				memset(&ptes[i], 0,
 				       sizeof(struct gasket_page_table_entry));
-				return -1;
+				return -EINVAL;
 			}
 		}
 
@@ -1165,7 +1164,7 @@ int gasket_page_table_lookup_page(
 	*ppage = NULL;
 	*poffset = 0;
 	mutex_unlock(&pg_tbl->mutex);
-	return -1;
+	return -EINVAL;
 }
 
 /* See gasket_page_table.h for description. */
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 03/11] staging: gasket: page table: remove dead code in coherent mem alloc
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
  2018-10-15  4:59 ` [PATCH 01/11] staging: gasket: core: debug log updates Todd Poynor
  2018-10-15  4:59 ` [PATCH 02/11] staging: gasket: page table: return valid error code on map fail Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 04/11] staging: gasket: page_table: remove unnecessary PTE status set to free Todd Poynor
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

gasket_alloc_coherent_memory() has some unnecessary code related to out
of memory checking that will never hit the condition checked, remove.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 2e1de8ad4a2c6..985a3a93499d5 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -1316,7 +1316,6 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 			GFP_KERNEL);
 	if (!gasket_dev->page_table[index]->coherent_pages)
 		goto nomem;
-	*dma_address = 0;
 
 	gasket_dev->coherent_buffer.length_bytes =
 		PAGE_SIZE * (num_pages);
@@ -1331,15 +1330,12 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 			(u64)mem + j * PAGE_SIZE;
 	}
 
-	if (*dma_address == 0)
-		goto nomem;
 	return 0;
 
 nomem:
-	if (mem) {
+	if (mem)
 		dma_free_coherent(gasket_get_device(gasket_dev),
 				  num_pages * PAGE_SIZE, mem, handle);
-	}
 
 	kfree(gasket_dev->page_table[index]->coherent_pages);
 	gasket_dev->page_table[index]->coherent_pages = NULL;
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 04/11] staging: gasket: page_table: remove unnecessary PTE status set to free
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (2 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 03/11] staging: gasket: page table: remove dead code in coherent mem alloc Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 05/11] staging: gasket: page_table: rearrange gasket_page_table_entry Todd Poynor
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

Remove unnecessary ptes[i].status update in gasket_perform_unmapping.
The vaaue will be cleared in the following memset.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 985a3a93499d5..d2e115d2dba30 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -623,7 +623,6 @@ static void gasket_perform_unmapping(struct gasket_page_table *pg_tbl,
 			if (gasket_release_page(ptes[i].page))
 				--pg_tbl->num_active_pages;
 		}
-		ptes[i].status = PTE_FREE;
 
 		/* and clear the PTE. */
 		memset(&ptes[i], 0, sizeof(struct gasket_page_table_entry));
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 05/11] staging: gasket: page_table: rearrange gasket_page_table_entry
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (3 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 04/11] staging: gasket: page_table: remove unnecessary PTE status set to free Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  7:37   ` Greg Kroah-Hartman
  2018-10-15  4:59 ` [PATCH 06/11] staging: gasket: page table: fixup error path allocating coherent mem Todd Poynor
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

Rearrange gasket_page_table entry to reduce padding slop.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index d2e115d2dba30..9c2f8671216b5 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -103,12 +103,6 @@ struct gasket_page_table_entry {
 	/* The status of this entry/slot: free or in use. */
 	enum pte_status status;
 
-	/* Address of the page in DMA space. */
-	dma_addr_t dma_addr;
-
-	/* Linux page descriptor for the page described by this structure. */
-	struct page *page;
-
 	/*
 	 * Index for alignment into host vaddrs.
 	 * When a user specifies a host address for a mapping, that address may
@@ -119,6 +113,12 @@ struct gasket_page_table_entry {
 	 */
 	int offset;
 
+	/* Address of the page in DMA space. */
+	dma_addr_t dma_addr;
+
+	/* Linux page descriptor for the page described by this structure. */
+	struct page *page;
+
 	/*
 	 * If this is an extended and first-level entry, sublevel points
 	 * to the second-level entries underneath this entry.
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 06/11] staging: gasket: page table: fixup error path allocating coherent mem
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (4 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 05/11] staging: gasket: page_table: rearrange gasket_page_table_entry Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 07/11] staging: gasket: page_table: fix comment in components_to_dev_address Todd Poynor
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

Correctly clean up data structure state in gasket_alloc_coherent_memory
error path, to ensure no double free on the stale pointer value.

Signed-off-by: Nick Ewalt <nicholasewalt@google.com>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_page_table.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 9c2f8671216b5..a88f2ae0cee8b 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -1332,9 +1332,13 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 	return 0;
 
 nomem:
-	if (mem)
+	if (mem) {
 		dma_free_coherent(gasket_get_device(gasket_dev),
 				  num_pages * PAGE_SIZE, mem, handle);
+		gasket_dev->coherent_buffer.length_bytes = 0;
+		gasket_dev->coherent_buffer.virt_base = NULL;
+		gasket_dev->coherent_buffer.phys_base = 0;
+	}
 
 	kfree(gasket_dev->page_table[index]->coherent_pages);
 	gasket_dev->page_table[index]->coherent_pages = NULL;
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 07/11] staging: gasket: page_table: fix comment in components_to_dev_address
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (5 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 06/11] staging: gasket: page table: fixup error path allocating coherent mem Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 08/11] staging: gasket: page_table: simplify gasket_components_to_dev_address Todd Poynor
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

Comments in components_to_dev_address() describing examples are
inconsistent, fix these to be accurate.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index a88f2ae0cee8b..ec9359576ea7c 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -687,13 +687,13 @@ static inline bool gasket_addr_is_simple(struct gasket_page_table *pg_tbl,
  * Convert (simple, page, offset) into a device address.
  * Examples:
  * Simple page 0, offset 32:
- *  Input (0, 0, 32), Output 0x20
+ *  Input (1, 0, 32), Output 0x20
  * Simple page 1000, offset 511:
- *  Input (0, 1000, 512), Output 0x3E81FF
+ *  Input (1, 1000, 511), Output 0x3E81FF
  * Extended page 0, offset 32:
  *  Input (0, 0, 32), Output 0x8000000020
  * Extended page 1000, offset 511:
- *  Input (1, 1000, 512), Output 0x8003E81FF
+ *  Input (0, 1000, 511), Output 0x8003E81FF
  */
 static ulong gasket_components_to_dev_address(struct gasket_page_table *pg_tbl,
 					      int is_simple, uint page_index,
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 08/11] staging: gasket: page_table: simplify gasket_components_to_dev_address
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (6 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 07/11] staging: gasket: page_table: fix comment in components_to_dev_address Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 09/11] staging: gasket: apex: fix sysfs_show Todd Poynor
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

Refactor gasket_components_to_dev_address to be faster and easier to
understand. The old implementation was unnecessarily complex and masked
the page_index for simple addresses but not extended ones. It makes the
most sense for this function to perform no such masking.

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

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index ec9359576ea7c..c2fbab74194f8 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -699,26 +699,9 @@ static ulong gasket_components_to_dev_address(struct gasket_page_table *pg_tbl,
 					      int is_simple, uint page_index,
 					      uint offset)
 {
-	ulong lvl0_index, lvl1_index;
+	ulong dev_addr = (page_index << GASKET_SIMPLE_PAGE_SHIFT) | offset;
 
-	if (is_simple) {
-		/* Return simple addresses directly. */
-		lvl0_index = page_index & (pg_tbl->config.total_entries - 1);
-		return (lvl0_index << GASKET_SIMPLE_PAGE_SHIFT) | offset;
-	}
-
-	/*
-	 * This could be compressed into fewer statements, but
-	 * A) the compiler should optimize it
-	 * B) this is not slow
-	 * C) this is an uncommon operation
-	 * D) this is actually readable this way.
-	 */
-	lvl0_index = page_index / GASKET_PAGES_PER_SUBTABLE;
-	lvl1_index = page_index & (GASKET_PAGES_PER_SUBTABLE - 1);
-	return (pg_tbl)->extended_flag |
-	       (lvl0_index << GASKET_EXTENDED_LVL0_SHIFT) |
-	       (lvl1_index << GASKET_EXTENDED_LVL1_SHIFT) | offset;
+	return is_simple ? dev_addr : (pg_tbl->extended_flag | dev_addr);
 }
 
 /*
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 09/11] staging: gasket: apex: fix sysfs_show
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (7 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 08/11] staging: gasket: page_table: simplify gasket_components_to_dev_address Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 10/11] staging: gasket: sysfs: fix attribute release comment Todd Poynor
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

sysfs_show was incorrectly extracting the sysfs_attribute_type from the
gasket_sysfs_attribute. This prevented dispatch from working properly.

Signed-off-by: Nick Ewalt <nicholasewalt@google.com>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 6dca3b177863c..3c7a13a4798ef 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -526,7 +526,7 @@ static ssize_t sysfs_show(struct device *device, struct device_attribute *attr,
 		return -ENODEV;
 	}
 
-	type = (enum sysfs_attribute_type)gasket_sysfs_get_attr(device, attr);
+	type = (enum sysfs_attribute_type)gasket_attr->data.attr_type;
 	switch (type) {
 	case ATTR_KERNEL_HIB_PAGE_TABLE_SIZE:
 		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 10/11] staging: gasket: sysfs: fix attribute release comment
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (8 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 09/11] staging: gasket: apex: fix sysfs_show Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  4:59 ` [PATCH 11/11] staging: gasket: Update device virtual address comment Todd Poynor
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

Comments for gasket_sysfs_get_attr() incorrectly describe reference
release procedure.

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

diff --git a/drivers/staging/gasket/gasket_sysfs.h b/drivers/staging/gasket/gasket_sysfs.h
index f32eaf89e056b..151e8edd28ea6 100644
--- a/drivers/staging/gasket/gasket_sysfs.h
+++ b/drivers/staging/gasket/gasket_sysfs.h
@@ -152,8 +152,8 @@ void gasket_sysfs_put_device_data(struct device *device,
  * Returns the Gasket sysfs attribute associated with the kernel device
  * attribute and device structure itself. Upon success, this call will take a
  * reference to internal sysfs data that must be released with a call to
- * gasket_sysfs_get_device_data. While this reference is held, the underlying
- * device sysfs information/structure will remain valid/will not be deleted.
+ * gasket_sysfs_put_attr. While this reference is held, the underlying device
+ * sysfs information/structure will remain valid/will not be deleted.
  */
 struct gasket_sysfs_attribute *
 gasket_sysfs_get_attr(struct device *device, struct device_attribute *attr);
-- 
2.19.0.605.g01d371f741-goog


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

* [PATCH 11/11] staging: gasket: Update device virtual address comment
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (9 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 10/11] staging: gasket: sysfs: fix attribute release comment Todd Poynor
@ 2018-10-15  4:59 ` Todd Poynor
  2018-10-15  7:33 ` [PATCH 00/11] staging: gasket: fixes Greg Kroah-Hartman
  2018-10-15 10:29 ` Christoph Hellwig
  12 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15  4:59 UTC (permalink / raw)
  To: Rob Springer, Ben Chan, Greg Kroah-Hartman
  Cc: devel, linux-kernel, Nick Ewalt, Todd Poynor

From: Nick Ewalt <nicholasewalt@google.com>

Add that number of page table entries and extended address bit offset
are configurable. Update example virtual address format to be more
consistent with typical usage.

Signed-off-by: Nick Ewalt <nicholasewalt@google.com>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_page_table.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index c2fbab74194f8..5b398b7ba81d3 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -10,10 +10,18 @@
  *
  * This file assumes 4kB pages throughout; can be factored out when necessary.
  *
- * Address format is as follows:
+ * There is a configurable number of page table entries, as well as a
+ * configurable bit index for the extended address flag. Both of these are
+ * specified in gasket_page_table_init through the page_table_config parameter.
+ *
+ * The following example assumes:
+ *   page_table_config->total_entries = 8192
+ *   page_table_config->extended_bit = 63
+ *
+ * Address format:
  * Simple addresses - those whose containing pages are directly placed in the
  * device's address translation registers - are laid out as:
- * [ 63 - 40: Unused | 39 - 28: 0 | 27 - 12: page index | 11 - 0: page offset ]
+ * [ 63 - 25: 0 | 24 - 12: page index | 11 - 0: page offset ]
  * page index:  The index of the containing page in the device's address
  *              translation registers.
  * page offset: The index of the address into the containing page.
@@ -21,7 +29,7 @@
  * Extended address - those whose containing pages are contained in a second-
  * level page table whose address is present in the device's address translation
  * registers - are laid out as:
- * [ 63 - 40: Unused | 39: flag | 38 - 37: 0 | 36 - 21: dev/level 0 index |
+ * [ 63: flag | 62 - 34: 0 | 33 - 21: dev/level 0 index |
  *   20 - 12: host/level 1 index | 11 - 0: page offset ]
  * flag:        Marker indicating that this is an extended address. Always 1.
  * dev index:   The index of the first-level page in the device's extended
-- 
2.19.0.605.g01d371f741-goog


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

* Re: [PATCH 00/11] staging: gasket: fixes
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (10 preceding siblings ...)
  2018-10-15  4:59 ` [PATCH 11/11] staging: gasket: Update device virtual address comment Todd Poynor
@ 2018-10-15  7:33 ` Greg Kroah-Hartman
  2018-10-15 10:19   ` Todd Poynor
  2018-10-15 10:29 ` Christoph Hellwig
  12 siblings, 1 reply; 19+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-15  7:33 UTC (permalink / raw)
  To: Todd Poynor; +Cc: Rob Springer, Ben Chan, devel, linux-kernel, Todd Poynor

On Sun, Oct 14, 2018 at 09:59:16PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> Various fixes for gasket/apex drivers.

For some reason you seem to have ignored/missed this patch:
	Subject: [PATCH v3] staging: gasket: Fix sparse "incorrect type in assignment" warnings.
that was sent last week.

Any specific reason?

thanks,

greg k-h

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

* Re: [PATCH 01/11] staging: gasket: core: debug log updates
  2018-10-15  4:59 ` [PATCH 01/11] staging: gasket: core: debug log updates Todd Poynor
@ 2018-10-15  7:34   ` Greg Kroah-Hartman
  2018-10-15 20:33     ` Todd Poynor
  0 siblings, 1 reply; 19+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-15  7:34 UTC (permalink / raw)
  To: Todd Poynor; +Cc: Rob Springer, Ben Chan, devel, linux-kernel, Todd Poynor

On Sun, Oct 14, 2018 at 09:59:17PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> Add debug logs for device enable/disable events,

Why?

What is going to need this?

> remove logs for
> callbacks (the called functions can generate their own logs if needed).

That's a different thing than "adding" them, so shouldn't this really be
2 patches?  If it was, I could have accepted this patch already, and
ignored the "add new logs" one :)

thanks,

greg k-h

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

* Re: [PATCH 05/11] staging: gasket: page_table: rearrange gasket_page_table_entry
  2018-10-15  4:59 ` [PATCH 05/11] staging: gasket: page_table: rearrange gasket_page_table_entry Todd Poynor
@ 2018-10-15  7:37   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 19+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-15  7:37 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Rob Springer, Ben Chan, devel, linux-kernel, Nick Ewalt, Todd Poynor

On Sun, Oct 14, 2018 at 09:59:21PM -0700, Todd Poynor wrote:
> From: Nick Ewalt <nicholasewalt@google.com>
> 
> Rearrange gasket_page_table entry to reduce padding slop.

In the future, defining exactly what "slop" is here would be nice :)

thanks,

greg k-h

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

* Re: [PATCH 00/11] staging: gasket: fixes
  2018-10-15  7:33 ` [PATCH 00/11] staging: gasket: fixes Greg Kroah-Hartman
@ 2018-10-15 10:19   ` Todd Poynor
  0 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15 10:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Rob Springer, Ben Chan, devel, lkml, Todd Poynor

On Mon, Oct 15, 2018 at 12:33 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Oct 14, 2018 at 09:59:16PM -0700, Todd Poynor wrote:
> > From: Todd Poynor <toddpoynor@google.com>
> >
> > Various fixes for gasket/apex drivers.
>
> For some reason you seem to have ignored/missed this patch:
>         Subject: [PATCH v3] staging: gasket: Fix sparse "incorrect type in assignment" warnings.
> that was sent last week.
>
> Any specific reason?

Coming off a 2 week vacation, I'm behind on everything.  I saw you had
commented already, but I'll take a look as well, sure.

>
> thanks,
>
> greg k-h



-- 
Todd

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

* Re: [PATCH 00/11] staging: gasket: fixes
  2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
                   ` (11 preceding siblings ...)
  2018-10-15  7:33 ` [PATCH 00/11] staging: gasket: fixes Greg Kroah-Hartman
@ 2018-10-15 10:29 ` Christoph Hellwig
  2018-10-15 10:35   ` Greg Kroah-Hartman
  12 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2018-10-15 10:29 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Rob Springer, Ben Chan, Greg Kroah-Hartman, devel, linux-kernel,
	Todd Poynor

On Sun, Oct 14, 2018 at 09:59:16PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> Various fixes for gasket/apex drivers.

I still can't find any explanation in the staging tree or in your
comments what gasket even is.

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

* Re: [PATCH 00/11] staging: gasket: fixes
  2018-10-15 10:29 ` Christoph Hellwig
@ 2018-10-15 10:35   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 19+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-15 10:35 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Todd Poynor, devel, linux-kernel, Rob Springer, Todd Poynor

On Mon, Oct 15, 2018 at 03:29:49AM -0700, Christoph Hellwig wrote:
> On Sun, Oct 14, 2018 at 09:59:16PM -0700, Todd Poynor wrote:
> > From: Todd Poynor <toddpoynor@google.com>
> > 
> > Various fixes for gasket/apex drivers.
> 
> I still can't find any explanation in the staging tree or in your
> comments what gasket even is.

No one really knows, it will all eventually be deleted and they will
just use the UIO interface in the end, but it's fun to watch them
work themselves toward that over time :)

greg k-h

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

* Re: [PATCH 01/11] staging: gasket: core: debug log updates
  2018-10-15  7:34   ` Greg Kroah-Hartman
@ 2018-10-15 20:33     ` Todd Poynor
  0 siblings, 0 replies; 19+ messages in thread
From: Todd Poynor @ 2018-10-15 20:33 UTC (permalink / raw)
  To: Greg KH; +Cc: toddpoynor, Rob Springer, benchan, devel, LKML

On Mon, Oct 15, 2018 at 12:34 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Oct 14, 2018 at 09:59:17PM -0700, Todd Poynor wrote:
> > From: Todd Poynor <toddpoynor@google.com>
> >
> > Add debug logs for device enable/disable events,
>
> Why?
>
> What is going to need this?

As one of the few people actually developing for Apex chips (but this
may be more soon), I ran into a situation where I cared about this.
But I usually cobble together custom debugging for development
situations, and generally don't get debug logs for
released-in-the-wild drivers, so I'm fine not including these or any
other debug logs.

It sounds like debug logs face a pretty high bar for acceptance.  I'm
happy to send a patch removing all of 'em from gasket/apex if that's
preferred.

> > remove logs for
> > callbacks (the called functions can generate their own logs if needed).
>
> That's a different thing than "adding" them, so shouldn't this really be
> 2 patches?  If it was, I could have accepted this patch already, and
> ignored the "add new logs" one :)

Sure, the callbacks most frequently occur during disable/enable events
and were linked in my brain, but will send a new patch to just remove
the useless callback logs.

>
> thanks,
>
> greg k-h

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

end of thread, other threads:[~2018-10-15 20:33 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-15  4:59 [PATCH 00/11] staging: gasket: fixes Todd Poynor
2018-10-15  4:59 ` [PATCH 01/11] staging: gasket: core: debug log updates Todd Poynor
2018-10-15  7:34   ` Greg Kroah-Hartman
2018-10-15 20:33     ` Todd Poynor
2018-10-15  4:59 ` [PATCH 02/11] staging: gasket: page table: return valid error code on map fail Todd Poynor
2018-10-15  4:59 ` [PATCH 03/11] staging: gasket: page table: remove dead code in coherent mem alloc Todd Poynor
2018-10-15  4:59 ` [PATCH 04/11] staging: gasket: page_table: remove unnecessary PTE status set to free Todd Poynor
2018-10-15  4:59 ` [PATCH 05/11] staging: gasket: page_table: rearrange gasket_page_table_entry Todd Poynor
2018-10-15  7:37   ` Greg Kroah-Hartman
2018-10-15  4:59 ` [PATCH 06/11] staging: gasket: page table: fixup error path allocating coherent mem Todd Poynor
2018-10-15  4:59 ` [PATCH 07/11] staging: gasket: page_table: fix comment in components_to_dev_address Todd Poynor
2018-10-15  4:59 ` [PATCH 08/11] staging: gasket: page_table: simplify gasket_components_to_dev_address Todd Poynor
2018-10-15  4:59 ` [PATCH 09/11] staging: gasket: apex: fix sysfs_show Todd Poynor
2018-10-15  4:59 ` [PATCH 10/11] staging: gasket: sysfs: fix attribute release comment Todd Poynor
2018-10-15  4:59 ` [PATCH 11/11] staging: gasket: Update device virtual address comment Todd Poynor
2018-10-15  7:33 ` [PATCH 00/11] staging: gasket: fixes Greg Kroah-Hartman
2018-10-15 10:19   ` Todd Poynor
2018-10-15 10:29 ` Christoph Hellwig
2018-10-15 10:35   ` Greg Kroah-Hartman

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