The series contains a number of minor fixes (report correct pool
size, fix ups on error path etc.) and overall driver clean up,
no functional change.
Changes from v4 to v5, thanks to Philipp:
- removed patch v4 3/8 "use phys_addr_t instead of u32 for physical
address" from the series and rebased the rest of the changes;
the patch was removed, because it is helpful only in case,
when SRAM size is larger than 4GiB, which is unlikely
Changes from v3 to v4, thanks to Philipp's review:
- squashed patches v3 1/9 (fix) and 7/9 (clean-up) into v4 1/8
- replaced "0x%llx" with "0x%pa" to display phys_addr_t values
Changes from v2 to v3:
- immediately return -ENOMEM on kmalloc() failure after probe
error path simplification (7/9)
- fixes a variable may be uninitialized warning
Changes from v1 to v2:
- report size of SRAM in decimal format '%zu' instead of '%zx'
- replacement of denominator '1024' to SZ_1K requires explicit
include of linux/sizes.h on some platforms, keep it as a number
Vladimir Zapolskiy (7):
misc: sram: fix enabled clock leak on error path
misc: sram: fix device node reference leak on error
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: move reserved block logic out of probe function
misc: sram: sort and clean up included headers
drivers/misc/sram.c | 137 +++++++++++++++++++++++++++-------------------------
1 file changed, 72 insertions(+), 65 deletions(-)
--
2.1.4
If devm_gen_pool_create() fails, the previously enabled sram->clk is
not disabled on probe() exit.
Because 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, so it is
safe to place devm_clk_get() at the end of probe(). No functional
change.
Signed-off-by: Vladimir Zapolskiy <[email protected]>
---
Changes from v3 to v4:
- squashed patches v3 1/9 (fix) and 7/9 (clean-up) into v4 1/8
drivers/misc/sram.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index eeaaf5f..76a23f9 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -90,12 +90,6 @@ static int sram_probe(struct platform_device *pdev)
if (!sram)
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;
@@ -106,10 +100,8 @@ static int sram_probe(struct platform_device *pdev)
*/
nblocks = (np) ? of_get_available_child_count(np) + 1 : 1;
rblocks = kmalloc((nblocks) * sizeof(*rblocks), GFP_KERNEL);
- if (!rblocks) {
- ret = -ENOMEM;
- goto err_alloc;
- }
+ if (!rblocks)
+ return -ENOMEM;
block = &rblocks[0];
for_each_available_child_of_node(np, child) {
@@ -188,6 +180,12 @@ static int sram_probe(struct platform_device *pdev)
kfree(rblocks);
+ sram->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(sram->clk))
+ sram->clk = NULL;
+ else
+ clk_prepare_enable(sram->clk);
+
platform_set_drvdata(pdev, sram);
dev_dbg(&pdev->dev, "SRAM pool: %ld KiB @ 0x%p\n", size / 1024, virt_base);
@@ -196,9 +194,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;
}
--
2.1.4
A pointer device node reference should be decremented on manual exit
from for_each_available_child_of_node() loop.
Signed-off-by: Vladimir Zapolskiy <[email protected]>
---
drivers/misc/sram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 76a23f9..0bfdfac 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -112,6 +112,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;
}
@@ -120,6 +121,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;
}
--
2.1.4
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 <[email protected]>
---
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 0bfdfac..baada47 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -205,7 +205,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);
--
2.1.4
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 <[email protected]>
---
Changes from v4 to v5:
- rebased on top of v5 series
Changes from v1 to v2:
- rebased on top of v2 3/9 misc: sram: use phys_addr_t instead of u32
for physical address
drivers/misc/sram.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index baada47..dd66d5f 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -190,7 +190,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: %zu KiB @ 0x%p\n",
+ gen_pool_size(sram->pool) / 1024, virt_base);
return 0;
--
2.1.4
No functional change, this is a preceding change to simplify
separation of reserved partition handling logic from probe()
function.
Signed-off-by: Vladimir Zapolskiy <[email protected]>
---
Changes from v4 to v5:
- rebased on top of v5 series
Changes from v3 to v4:
- rebased on top of v4 3/8 misc: sram: use phys_addr_t instead of u32
for physical address
Changes from v1 to v2:
- rebased on top of v2 3/9 misc: sram: use phys_addr_t instead of u32
for physical address
drivers/misc/sram.c | 54 ++++++++++++++++++++++++++++-------------------------
1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index dd66d5f..9ecc7d5 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;
@@ -68,29 +70,31 @@ 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,
- res->start, size, pdev->name)) {
- dev_err(&pdev->dev, "could not request region for resource\n");
+ 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;
}
- 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, ilog2(SRAM_GRANULARITY), -1);
+ sram->pool = devm_gen_pool_create(sram->dev,
+ ilog2(SRAM_GRANULARITY), -1);
if (!sram->pool)
return -ENOMEM;
@@ -109,7 +113,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);
@@ -117,7 +121,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;
@@ -129,9 +133,8 @@ 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(sram->dev, "found reserved block 0x%x-0x%x\n",
+ block->start, block->start + block->size);
block++;
}
@@ -148,7 +151,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%x starts after current offset 0x%lx\n",
block->start, cur_start);
ret = -EINVAL;
@@ -168,10 +171,11 @@ 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",
+ dev_dbg(sram->dev, "adding chunk 0x%lx-0x%lx\n",
cur_start, 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;
@@ -182,7 +186,7 @@ static int sram_probe(struct platform_device *pdev)
kfree(rblocks);
- 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
@@ -190,8 +194,8 @@ static int sram_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, sram);
- dev_dbg(&pdev->dev, "SRAM pool: %zu KiB @ 0x%p\n",
- gen_pool_size(sram->pool) / 1024, virt_base);
+ dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n",
+ gen_pool_size(sram->pool) / 1024, sram->virt_base);
return 0;
@@ -206,7 +210,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);
--
2.1.4
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 <[email protected]>
---
Changes from v4 to v5:
- rebased on top of v5 series
Changes from v2 to v3:
- fixes a variable may be uninitialized warning
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 9ecc7d5..7502b6c 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -57,47 +57,19 @@ 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;
unsigned long size, cur_start, cur_size;
struct sram_reserve *rblocks, *block;
struct list_head reserve_list;
unsigned int nblocks;
- int ret;
+ int ret = 0;
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.
@@ -184,8 +156,51 @@ 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;
@@ -198,11 +213,6 @@ static int sram_probe(struct platform_device *pdev)
gen_pool_size(sram->pool) / 1024, sram->virt_base);
return 0;
-
-err_chunks:
- kfree(rblocks);
-
- return ret;
}
static int sram_remove(struct platform_device *pdev)
--
2.1.4
Most of the included header files are already included as
dependencies.
Signed-off-by: Vladimir Zapolskiy <[email protected]>
---
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 7502b6c..15c33cc 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
--
2.1.4