[PATCH 0/9] misc: sram: minor fixes and clean up

[PATCH 0/9] misc: sram: minor fixes and clean up

[Vladimir Zapolskiy]

The series contains a number of minor fixes (move to phys_addr_t
from u32, fix ups on error path etc.) and overall driver clean up,
no functional change.

Vladimir Zapolskiy (9):
  misc: sram: fix enabled clock leak on error path
  misc: sram: fix device node reference leak on error
  misc: sram: use phys_addr_t instead of u32 for physical address
  misc: sram: bump error message level on unclean driver unbinding
  misc: sram: report correct SRAM pool size
  misc: sram: add private struct device and virt_base members
  misc: sram: simplify probe error path
  misc: sram: move reserved block logic out of probe function
  misc: sram: sort and clean up included headers

 drivers/misc/sram.c |  149 +++++++++++++++++++++++++++------------------------
 1 file changed, 80 insertions(+), 69 deletions(-)

-- 
1.7.10.4

[PATCH 1/9] misc: sram: fix enabled clock leak on error path

[Vladimir Zapolskiy]

If devm_gen_pool_create() fails, the previously enabled sram->clk is
not disabled on probe() exit.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index eeaaf5f..b44a423 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -90,16 +90,17 @@ static int sram_probe(struct platform_device *pdev)
 	if (!sram)
 		return -ENOMEM;
 
+	sram->pool = devm_gen_pool_create(&pdev->dev,
+					  ilog2(SRAM_GRANULARITY), -1);
+	if (!sram->pool)
+		return -ENOMEM;
+
 	sram->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(sram->clk))
 		sram->clk = NULL;
 	else
 		clk_prepare_enable(sram->clk);
 
-	sram->pool = devm_gen_pool_create(&pdev->dev, ilog2(SRAM_GRANULARITY), -1);
-	if (!sram->pool)
-		return -ENOMEM;
-
 	/*
 	 * We need an additional block to mark the end of the memory region
 	 * after the reserved blocks from the dt are processed.
-- 
1.7.10.4

[PATCH 2/9] misc: sram: fix device node reference leak on error

[Vladimir Zapolskiy]

A pointer device node reference should be decremented on manual exit
from for_each_available_child_of_node() loop.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index b44a423..999684a 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -121,6 +121,7 @@ static int sram_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev,
 				"could not get address for node %s\n",
 				child->full_name);
+			of_node_put(child);
 			goto err_chunks;
 		}
 
@@ -129,6 +130,7 @@ static int sram_probe(struct platform_device *pdev)
 				"reserved block %s outside the sram area\n",
 				child->full_name);
 			ret = -EINVAL;
+			of_node_put(child);
 			goto err_chunks;
 		}
 
-- 
1.7.10.4

[PATCH 3/9] misc: sram: use phys_addr_t instead of u32 for physical address

[Vladimir Zapolskiy]

To avoid any problems on non 32-bit platforms get and store memory
addresses under phys_addr_t type.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |   26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 999684a..a703e37 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -41,8 +41,8 @@ struct sram_dev {
 
 struct sram_reserve {
 	struct list_head list;
-	u32 start;
-	u32 size;
+	phys_addr_t start;
+	size_t size;
 };
 
 static int sram_reserve_cmp(void *priv, struct list_head *a,
@@ -60,7 +60,8 @@ static int sram_probe(struct platform_device *pdev)
 	struct sram_dev *sram;
 	struct resource *res;
 	struct device_node *np = pdev->dev.of_node, *child;
-	unsigned long size, cur_start, cur_size;
+	phys_addr_t cur_start;
+	size_t size, cur_size;
 	struct sram_reserve *rblocks, *block;
 	struct list_head reserve_list;
 	unsigned int nblocks;
@@ -138,9 +139,9 @@ static int sram_probe(struct platform_device *pdev)
 		block->size = resource_size(&child_res);
 		list_add_tail(&block->list, &reserve_list);
 
-		dev_dbg(&pdev->dev, "found reserved block 0x%x-0x%x\n",
-			block->start,
-			block->start + block->size);
+		dev_dbg(&pdev->dev, "found reserved block 0x%llx-0x%llx\n",
+			(unsigned long long)block->start,
+			(unsigned long long)block->start + block->size);
 
 		block++;
 	}
@@ -158,8 +159,9 @@ static int sram_probe(struct platform_device *pdev)
 		/* can only happen if sections overlap */
 		if (block->start < cur_start) {
 			dev_err(&pdev->dev,
-				"block at 0x%x starts after current offset 0x%lx\n",
-				block->start, cur_start);
+				"block at 0x%llx starts after current offset 0x%llx\n",
+				(unsigned long long)block->start,
+				(unsigned long long)cur_start);
 			ret = -EINVAL;
 			goto err_chunks;
 		}
@@ -177,8 +179,9 @@ static int sram_probe(struct platform_device *pdev)
 		 */
 		cur_size = block->start - cur_start;
 
-		dev_dbg(&pdev->dev, "adding chunk 0x%lx-0x%lx\n",
-			cur_start, cur_start + cur_size);
+		dev_dbg(&pdev->dev, "adding chunk 0x%llx-0x%llx\n",
+			(unsigned long long)cur_start,
+			(unsigned long long)cur_start + cur_size);
 		ret = gen_pool_add_virt(sram->pool,
 				(unsigned long)virt_base + cur_start,
 				res->start + cur_start, cur_size, -1);
@@ -193,7 +196,8 @@ static int sram_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, sram);
 
-	dev_dbg(&pdev->dev, "SRAM pool: %ld KiB @ 0x%p\n", size / 1024, virt_base);
+	dev_dbg(&pdev->dev, "SRAM pool: %zx KiB @ 0x%p\n",
+		size / SZ_1K, virt_base);
 
 	return 0;
 
-- 
1.7.10.4

[PATCH 4/9] misc: sram: bump error message level on unclean driver unbinding

[Vladimir Zapolskiy]

Report an error level message to a user, if the driver is unbound
while there are still some pool allocations.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index a703e37..d0abd60 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -214,7 +214,7 @@ static int sram_remove(struct platform_device *pdev)
 	struct sram_dev *sram = platform_get_drvdata(pdev);
 
 	if (gen_pool_avail(sram->pool) < gen_pool_size(sram->pool))
-		dev_dbg(&pdev->dev, "removed while SRAM allocated\n");
+		dev_err(&pdev->dev, "removed while SRAM allocated\n");
 
 	if (sram->clk)
 		clk_disable_unprepare(sram->clk);
-- 
1.7.10.4

[PATCH 5/9] misc: sram: report correct SRAM pool size

[Vladimir Zapolskiy]

Since some space in SRAM may be reserved, report the left free space
in the allocated memory pool instead of total physical size of the
SRAM device.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index d0abd60..9ea265c 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -197,7 +197,7 @@ static int sram_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, sram);
 
 	dev_dbg(&pdev->dev, "SRAM pool: %zx KiB @ 0x%p\n",
-		size / SZ_1K, virt_base);
+		gen_pool_size(sram->pool) / SZ_1K, virt_base);
 
 	return 0;
 
-- 
1.7.10.4

[PATCH 6/9] misc: sram: add private struct device and virt_base members

[Vladimir Zapolskiy]

No functional change, this is a preceding change to simplify
separation of reserved partition handling logic from probe()
function.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |   48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 9ea265c..ac6b587 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -35,6 +35,9 @@
 #define SRAM_GRANULARITY	32
 
 struct sram_dev {
+	struct device *dev;
+	void __iomem *virt_base;
+
 	struct gen_pool *pool;
 	struct clk *clk;
 };
@@ -56,7 +59,6 @@ static int sram_reserve_cmp(void *priv, struct list_head *a,
 
 static int sram_probe(struct platform_device *pdev)
 {
-	void __iomem *virt_base;
 	struct sram_dev *sram;
 	struct resource *res;
 	struct device_node *np = pdev->dev.of_node, *child;
@@ -69,34 +71,36 @@ static int sram_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&reserve_list);
 
+	sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
+	if (!sram)
+		return -ENOMEM;
+
+	sram->dev = &pdev->dev;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
-		dev_err(&pdev->dev, "found no memory resource\n");
+		dev_err(sram->dev, "found no memory resource\n");
 		return -EINVAL;
 	}
 
 	size = resource_size(res);
 
-	if (!devm_request_mem_region(&pdev->dev,
+	if (!devm_request_mem_region(sram->dev,
 			res->start, size, pdev->name)) {
-		dev_err(&pdev->dev, "could not request region for resource\n");
+		dev_err(sram->dev, "could not request region for resource\n");
 		return -EBUSY;
 	}
 
-	virt_base = devm_ioremap_wc(&pdev->dev, res->start, size);
-	if (IS_ERR(virt_base))
-		return PTR_ERR(virt_base);
-
-	sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
-	if (!sram)
-		return -ENOMEM;
+	sram->virt_base = devm_ioremap_wc(sram->dev, res->start, size);
+	if (IS_ERR(sram->virt_base))
+		return PTR_ERR(sram->virt_base);
 
-	sram->pool = devm_gen_pool_create(&pdev->dev,
+	sram->pool = devm_gen_pool_create(sram->dev,
 					  ilog2(SRAM_GRANULARITY), -1);
 	if (!sram->pool)
 		return -ENOMEM;
 
-	sram->clk = devm_clk_get(&pdev->dev, NULL);
+	sram->clk = devm_clk_get(sram->dev, NULL);
 	if (IS_ERR(sram->clk))
 		sram->clk = NULL;
 	else
@@ -119,7 +123,7 @@ static int sram_probe(struct platform_device *pdev)
 
 		ret = of_address_to_resource(child, 0, &child_res);
 		if (ret < 0) {
-			dev_err(&pdev->dev,
+			dev_err(sram->dev,
 				"could not get address for node %s\n",
 				child->full_name);
 			of_node_put(child);
@@ -127,7 +131,7 @@ static int sram_probe(struct platform_device *pdev)
 		}
 
 		if (child_res.start < res->start || child_res.end > res->end) {
-			dev_err(&pdev->dev,
+			dev_err(sram->dev,
 				"reserved block %s outside the sram area\n",
 				child->full_name);
 			ret = -EINVAL;
@@ -139,7 +143,7 @@ static int sram_probe(struct platform_device *pdev)
 		block->size = resource_size(&child_res);
 		list_add_tail(&block->list, &reserve_list);
 
-		dev_dbg(&pdev->dev, "found reserved block 0x%llx-0x%llx\n",
+		dev_dbg(sram->dev, "found reserved block 0x%llx-0x%llx\n",
 			(unsigned long long)block->start,
 			(unsigned long long)block->start + block->size);
 
@@ -158,7 +162,7 @@ static int sram_probe(struct platform_device *pdev)
 	list_for_each_entry(block, &reserve_list, list) {
 		/* can only happen if sections overlap */
 		if (block->start < cur_start) {
-			dev_err(&pdev->dev,
+			dev_err(sram->dev,
 				"block at 0x%llx starts after current offset 0x%llx\n",
 				(unsigned long long)block->start,
 				(unsigned long long)cur_start);
@@ -179,11 +183,11 @@ static int sram_probe(struct platform_device *pdev)
 		 */
 		cur_size = block->start - cur_start;
 
-		dev_dbg(&pdev->dev, "adding chunk 0x%llx-0x%llx\n",
+		dev_dbg(sram->dev, "adding chunk 0x%llx-0x%llx\n",
 			(unsigned long long)cur_start,
 			(unsigned long long)cur_start + cur_size);
 		ret = gen_pool_add_virt(sram->pool,
-				(unsigned long)virt_base + cur_start,
+				(unsigned long)sram->virt_base + cur_start,
 				res->start + cur_start, cur_size, -1);
 		if (ret < 0)
 			goto err_chunks;
@@ -196,8 +200,8 @@ static int sram_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, sram);
 
-	dev_dbg(&pdev->dev, "SRAM pool: %zx KiB @ 0x%p\n",
-		gen_pool_size(sram->pool) / SZ_1K, virt_base);
+	dev_dbg(sram->dev, "SRAM pool: %zx KiB @ 0x%p\n",
+		gen_pool_size(sram->pool) / SZ_1K, sram->virt_base);
 
 	return 0;
 
@@ -214,7 +218,7 @@ static int sram_remove(struct platform_device *pdev)
 	struct sram_dev *sram = platform_get_drvdata(pdev);
 
 	if (gen_pool_avail(sram->pool) < gen_pool_size(sram->pool))
-		dev_err(&pdev->dev, "removed while SRAM allocated\n");
+		dev_err(sram->dev, "removed while SRAM allocated\n");
 
 	if (sram->clk)
 		clk_disable_unprepare(sram->clk);
-- 
1.7.10.4

[PATCH 7/9] misc: sram: simplify probe error path

[Vladimir Zapolskiy]

Reserved block logic relies only on information from device tree,
there is no need to get and enable device clock in advance, especially
because not provided clock is not considered as an error. No
functional change.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |   20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index ac6b587..c1488a8 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -100,22 +100,14 @@ static int sram_probe(struct platform_device *pdev)
 	if (!sram->pool)
 		return -ENOMEM;
 
-	sram->clk = devm_clk_get(sram->dev, NULL);
-	if (IS_ERR(sram->clk))
-		sram->clk = NULL;
-	else
-		clk_prepare_enable(sram->clk);
-
 	/*
 	 * We need an additional block to mark the end of the memory region
 	 * after the reserved blocks from the dt are processed.
 	 */
 	nblocks = (np) ? of_get_available_child_count(np) + 1 : 1;
 	rblocks = kmalloc((nblocks) * sizeof(*rblocks), GFP_KERNEL);
-	if (!rblocks) {
+	if (!rblocks)
 		ret = -ENOMEM;
-		goto err_alloc;
-	}
 
 	block = &rblocks[0];
 	for_each_available_child_of_node(np, child) {
@@ -198,6 +190,12 @@ static int sram_probe(struct platform_device *pdev)
 
 	kfree(rblocks);
 
+	sram->clk = devm_clk_get(sram->dev, NULL);
+	if (IS_ERR(sram->clk))
+		sram->clk = NULL;
+	else
+		clk_prepare_enable(sram->clk);
+
 	platform_set_drvdata(pdev, sram);
 
 	dev_dbg(sram->dev, "SRAM pool: %zx KiB @ 0x%p\n",
@@ -207,9 +205,7 @@ static int sram_probe(struct platform_device *pdev)
 
 err_chunks:
 	kfree(rblocks);
-err_alloc:
-	if (sram->clk)
-		clk_disable_unprepare(sram->clk);
+
 	return ret;
 }
 
-- 
1.7.10.4

[PATCH 8/9] misc: sram: move reserved block logic out of probe function

[Vladimir Zapolskiy]

No functional change, but now previously overloaded sram_probe() is
greatly simplified and perceptible, reserved regions logic also has
its own space.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |   82 +++++++++++++++++++++++++++++----------------------
 1 file changed, 46 insertions(+), 36 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index c1488a8..2036f6d 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -57,11 +57,9 @@ static int sram_reserve_cmp(void *priv, struct list_head *a,
 	return ra->start - rb->start;
 }
 
-static int sram_probe(struct platform_device *pdev)
+static int sram_reserve_regions(struct sram_dev *sram, struct resource *res)
 {
-	struct sram_dev *sram;
-	struct resource *res;
-	struct device_node *np = pdev->dev.of_node, *child;
+	struct device_node *np = sram->dev->of_node, *child;
 	phys_addr_t cur_start;
 	size_t size, cur_size;
 	struct sram_reserve *rblocks, *block;
@@ -71,35 +69,8 @@ static int sram_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&reserve_list);
 
-	sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
-	if (!sram)
-		return -ENOMEM;
-
-	sram->dev = &pdev->dev;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(sram->dev, "found no memory resource\n");
-		return -EINVAL;
-	}
-
 	size = resource_size(res);
 
-	if (!devm_request_mem_region(sram->dev,
-			res->start, size, pdev->name)) {
-		dev_err(sram->dev, "could not request region for resource\n");
-		return -EBUSY;
-	}
-
-	sram->virt_base = devm_ioremap_wc(sram->dev, res->start, size);
-	if (IS_ERR(sram->virt_base))
-		return PTR_ERR(sram->virt_base);
-
-	sram->pool = devm_gen_pool_create(sram->dev,
-					  ilog2(SRAM_GRANULARITY), -1);
-	if (!sram->pool)
-		return -ENOMEM;
-
 	/*
 	 * We need an additional block to mark the end of the memory region
 	 * after the reserved blocks from the dt are processed.
@@ -188,8 +159,52 @@ static int sram_probe(struct platform_device *pdev)
 		cur_start = block->start + block->size;
 	}
 
+ err_chunks:
 	kfree(rblocks);
 
+	return ret;
+}
+
+static int sram_probe(struct platform_device *pdev)
+{
+	struct sram_dev *sram;
+	struct resource *res;
+	size_t size;
+	int ret;
+
+	sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
+	if (!sram)
+		return -ENOMEM;
+
+	sram->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(sram->dev, "found no memory resource\n");
+		return -EINVAL;
+	}
+
+	size = resource_size(res);
+
+	if (!devm_request_mem_region(sram->dev, res->start, size,
+				     pdev->name)) {
+		dev_err(sram->dev, "could not request region for resource\n");
+		return -EBUSY;
+	}
+
+	sram->virt_base = devm_ioremap_wc(sram->dev, res->start, size);
+	if (IS_ERR(sram->virt_base))
+		return PTR_ERR(sram->virt_base);
+
+	sram->pool = devm_gen_pool_create(sram->dev,
+					  ilog2(SRAM_GRANULARITY), -1);
+	if (!sram->pool)
+		return -ENOMEM;
+
+	ret = sram_reserve_regions(sram, res);
+	if (ret)
+		return ret;
+
 	sram->clk = devm_clk_get(sram->dev, NULL);
 	if (IS_ERR(sram->clk))
 		sram->clk = NULL;
@@ -202,11 +217,6 @@ static int sram_probe(struct platform_device *pdev)
 		gen_pool_size(sram->pool) / SZ_1K, sram->virt_base);
 
 	return 0;
-
-err_chunks:
-	kfree(rblocks);
-
-	return ret;
 }
 
 static int sram_remove(struct platform_device *pdev)
-- 
1.7.10.4

[PATCH 9/9] misc: sram: sort and clean up included headers

[Vladimir Zapolskiy]

Most of the included header files are already included as
dependencies.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/misc/sram.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 2036f6d..ed26ce0 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -18,19 +18,13 @@
  * MA 02110-1301, USA.
  */
 
-#include <linux/kernel.h>
-#include <linux/init.h>
 #include <linux/clk.h>
-#include <linux/err.h>
+#include <linux/genalloc.h>
 #include <linux/io.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/list.h>
 #include <linux/list_sort.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/genalloc.h>
 
 #define SRAM_GRANULARITY	32
 
-- 
1.7.10.4