ARM: rockchip: add smp functionality
Subsystems (5)
Heiko Stübner
maintainerv2 cover ARM: rockchip: add smp functionality
This series enables the use of the additional cores on Rockchip Cortex-A9 SoCs.
To achieve this, add the scu, the needed sram and power-management-unit.
changes since v1:
- add reserved block feature for mmio-sram, to not use two logical sram nodes
- the sram content is kept intact while the device is running, so copying the trampoline is only needed once
Heiko Stuebner (6):
misc: sram: fix error path in sram_probe
misc: sram: add ability to mark sram sections as reserved
arm: rockchip: add snoop-control-unit
arm: rockchip: add sram dt nodes and documentation for the smp part
arm: rockchip: add power-management-unit dt node
arm: rockchip: add smp bringup code
.../devicetree/bindings/arm/rockchip/pmu.txt | 16 ++ .../devicetree/bindings/arm/rockchip/smp-sram.txt | 23 +++ Documentation/devicetree/bindings/misc/sram.txt | 8 + arch/arm/boot/dts/rk3066a.dtsi | 16 ++ arch/arm/mach-rockchip/Kconfig | 1 + arch/arm/mach-rockchip/Makefile | 1 + arch/arm/mach-rockchip/core.h | 22 +++ arch/arm/mach-rockchip/headsmp.S | 32 +++ arch/arm/mach-rockchip/platsmp.c | 203 ++++++++++++++++++++ arch/arm/mach-rockchip/rockchip.c | 2 + drivers/misc/sram.c | 85 +++++++- 11 files changed, 404 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/rockchip/pmu.txt create mode 100644 Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt create mode 100644 arch/arm/mach-rockchip/core.h create mode 100644 arch/arm/mach-rockchip/headsmp.S create mode 100644 arch/arm/mach-rockchip/platsmp.c
Signature · 1 line
1.7.10.4
Heiko Stübner
maintainerPATCH v2 1/6 misc: sram: fix error path in sram_probe
The pool is created thru devm_gen_pool_create, so the call to gen_pool_destroy is not necessary.
Instead the sram-clock must be turned off again if it exists.
--- drivers/misc/sram.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/misc/sram.c +2 -1
@@ -68,7 +68,8 @@ static int sram_probe(struct platform_device *pdev) ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, res->start, size,-1 );if (ret <0 ) { -gen_pool_destroy (sram->pool); +if (sram->clk) +clk_disable_unprepare (sram->clk);return ret; }
Signature · 1 line
1.7.10.4
Re: [PATCH v2 1/6] misc: sram: fix error path in sram_probe
Hi Heiko,
Am Dienstag, den 25.06.2013, 10:46 +0200 schrieb Heiko St?bner:
Quoted 14 lines
The pool is created thru
devm_gen_pool_create, so the call togen_pool_destroyis not necessary.
Instead the sram-clock must be turned off again if it exists.Signed-off-by Heiko Stuebner <[email protected]>--- drivers/misc/sram.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/misc/sram.c +0
-0
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 71
Quoted 4 lines
drivers/misc/sram.c +0
-1@@ -68,7 +68,8 @@ static int sram_probe(struct platform_device *pdev) ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, res->start, size,-1 );if (ret <0 ) { -gen_pool_destroy (sram->pool);
Right, thanks.
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 75
Quoted 5 lines
drivers/misc/sram.c +2
-0+if (sram->clk) +clk_disable_unprepare (sram->clk);return ret; }
In light of the following patch, I'd rather move the clk_prepare_enable() call after gen_pool_add_virt() and its error path.
regards
Philipp
Heiko Stübner
maintainerRe: [PATCH v2 1/6] misc: sram: fix error path in sram_probe
Hi Philipp,
Am Dienstag, 25. Juni 2013, 11:04:34 schrieb Philipp Zabel:
Quoted 32 lines
Hi Heiko,
Am Dienstag, den 25.06.2013, 10:46 +0200 schrieb Heiko St?bner:
Quoted 15 lines
The pool is created thru
devm_gen_pool_create, so the call togen_pool_destroyis not necessary.
Instead the sram-clock must be turned off again if it exists.Signed-off-by Heiko Stuebner <[email protected]>---
drivers/misc/sram.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/misc/sram.c +0
-0drivers/misc/sram.c static int sram_probe(struct platform_device *pdev)
Quoted 8 lines
drivers/misc/sram.c +0
-1@@ -68,7 +68,8 @@ static int sram_probe(struct platform_device *pdev) ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, res->start, size,-1 );if (ret <0 ) { -gen_pool_destroy (sram->pool);Right, thanks.
Quoted 3 lines
drivers/misc/sram.c +2
-0+if (sram->clk) +clk_disable_unprepare (sram->clk);
Quoted 6 lines
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev)
Quoted 3 lines
drivers/misc/sram.c +0
-0return ret; }In light of the following patch, I'd rather move the
clk_prepare_enable()call aftergen_pool_add_virt()and its error path.
I'm not sure, but isn't moving the clock enablement below the pool allocation producing a race condition?
I.e. can the case happen that some other part wants to allocate part of the newly generated pool already, while the subsequent gen_pool_add_virt calls from the following patch are still running? ... And what will happen in this case, when the sram clock is still disabled?
Thanks
Heiko
Re: [PATCH v2 1/6] misc: sram: fix error path in sram_probe
Hi Heiko,
Am Donnerstag, den 04.07.2013, 16:34 +0200 schrieb Heiko St?bner:
Quoted 37 lines
Hi Philipp,
Am Dienstag, 25. Juni 2013, 11:04:34 schrieb Philipp Zabel:
Quoted 34 lines
Hi Heiko,
Am Dienstag, den 25.06.2013, 10:46 +0200 schrieb Heiko St?bner:
Quoted 15 lines
The pool is created thru
devm_gen_pool_create, so the call togen_pool_destroyis not necessary.
Instead the sram-clock must be turned off again if it exists.Signed-off-by Heiko Stuebner <[email protected]>---
drivers/misc/sram.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/misc/sram.c +0
-0drivers/misc/sram.c static int sram_probe(struct platform_device *pdev)
Quoted 8 lines
drivers/misc/sram.c +0
-1@@ -68,7 +68,8 @@ static int sram_probe(struct platform_device *pdev) ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, res->start, size,-1 );if (ret <0 ) { -gen_pool_destroy (sram->pool);Right, thanks.
Quoted 5 lines
drivers/misc/sram.c +2
-0+if (sram->clk) +clk_disable_unprepare (sram->clk);return ret;
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
}
In light of the following patch, I'd rather move the
clk_prepare_enable()call aftergen_pool_add_virt()and its error path.I'm not sure, but isn't moving the clock enablement below the pool allocation producing a race condition?
since this is a platform device, and the driver is not allowed to be compiled as a module, it is probed at the time platform_driver_register is called from postcore_initcall.
You'd have to go to quite some lengths to make this race by probing your driver before mmio-sram and then asynchronously trying to allocate memory from the pool some time later, without having made sure that the pool exists during probe.
It would be nice to have callbacks from gen_pool_alloc/free to enable/disable the clock.
For the time being, maybe it's the safest bet to just enable the clock before, especially if you reorder the loop so there is only one gen_pool_add_virt and corresponding error path.
Quoted 4 lines
I.e. can the case happen that some other part wants to allocate part of the newly generated pool already, while the subsequent
gen_pool_add_virtcalls from the following patch are still running? ... And what will happen in this case, when the sram clock is still disabled?
On i.MX6, the system will hang.
regards
Philipp
Heiko Stübner
maintainerPATCH v2 2/6 misc: sram: add ability to mark sram sections as reserved
In reply to Heiko StübnerSome SoCs need parts of their sram for special purposes. So while being part of the periphal, it should not be part of the genpool controlling the sram.
Threfore add an option mmio-sram-reserved to keep arbitary portions of the sram from being part of the pool.
--- Documentation/devicetree/bindings/misc/sram.txt | 8 +++ drivers/misc/sram.c | 86 +++++++++++++++++++++-- 2 files changed, 88 insertions(+), 6 deletions(-)
Documentation/devicetree/bindings/misc/sram.txt +8 -0
@@ -8,9 +8,17 @@ Required properties: - reg : SRAM iomem address range +Optional properties: + +- mmio-sram-reserved: ordered list of reserved chunks inside the sram that + should not become part of the genalloc pool. + Format is <base size>, <base size>, ...; with base being relative to the + reg property base. + Example: sram: sram at 5c000000 { compatible = "mmio-sram"; reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */ + mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */ };
drivers/misc/sram.c +85 -6
103 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Re: [PATCH v2 2/6] misc: sram: add ability to mark sram sections as reserved
Hi Heiko,
Am Dienstag, den 25.06.2013, 10:47 +0200 schrieb Heiko St?bner:
Quoted 18 lines
Some SoCs need parts of their sram for special purposes. So while being part of the periphal, it should not be part of the genpool controlling the sram.
Threfore add an option mmio-sram-reserved to keep arbitary portions of the sram from being part of the pool.
Suggested-by Rob Herring <[email protected]>Signed-off-by Heiko Stuebner <[email protected]>--- Documentation/devicetree/bindings/misc/sram.txt | 8 +++ drivers/misc/sram.c | 86 +++++++++++++++++++++-- 2 files changed, 88 insertions(+), 6 deletions(-)Documentation/devicetree/bindings/misc/sram.txt +0
-0
Documentation/devicetree/bindings/misc/sram.txt Required properties: · line 17
Quoted 10 lines
Documentation/devicetree/bindings/misc/sram.txt +7
-0@@ -8,9 +8,17 @@ Required properties: - reg : SRAM iomem address range +Optional properties: + +- mmio-sram-reserved: ordered list of reserved chunks inside the sram that + should not become part of the genalloc pool. + Format is <base size>, <base size>, ...; with base being relative to the + reg property base. +
the keyword to reserve blocks of ram is /memreserve/ - should this property name be aligned with that?
Quoted 41 lines
Documentation/devicetree/bindings/misc/sram.txt +1
-0Example: sram: sram at 5c000000 { compatible = "mmio-sram"; reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */ + mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */ };drivers/misc/sram.c +13
-6@@ -42,6 +42,8 @@ static int sram_probe(struct platform_device *pdev)struct sram_dev *sram;struct resource *res;unsigned long size; +const __be32 *reserved_list =NULL ; +int reserved_size =0 ;int ret; res =platform_get_resource (pdev,IORESOURCE_MEM ,0 );@@ -65,12 +67,89 @@ static int sram_probe(struct platform_device *pdev)if (!sram->pool)return -ENOMEM ; - ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, - res->start, size,-1 ); -if (ret <0 ) { -if (sram->clk) -clk_disable_unprepare (sram->clk); -return ret; +if (pdev->dev.of_node) { + reserved_list =of_get_property (pdev->dev.of_node, +"mmio-sram-reserved" , + &reserved_size); +if (reserved_list) { + reserved_size /=sizeof (*reserved_list); +if (!reserved_size || reserved_size %2 ) { +dev_warn (&pdev->dev,"wrong number of arguments in mmio-sram-reserved\n" ); + reserved_list =NULL ; + } + }
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 90
Quoted 10 lines
drivers/misc/sram.c +10
-0+ } + +if (!reserved_list) { + ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, + res->start, size,-1 ); +if (ret <0 ) { +if (sram->clk) +clk_disable_unprepare (sram->clk); +return ret; + }
Moving the clk_prepare_enable() further down would allow to avoid the clk_disable_unprepare() in every error path,
Quoted 8 lines
drivers/misc/sram.c +8
-0+ }else { +unsigned int cur_start =0 ; +unsigned int cur_size; +unsigned int rstart; +unsigned int rsize; +int i; + +for (i =0 ; i < reserved_size; i +=2 ) {
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 108
Quoted 10 lines
drivers/misc/sram.c +10
-0+/* get the next reserved block */ + rstart =be32_to_cpu (*reserved_list++); + rsize =be32_to_cpu (*reserved_list++); + +/* catch unsorted list entries */ +if (rstart < cur_start) { +dev_err (&pdev->dev,"unsorted reserved list (0x%x before current 0x%x)\n" , + rstart, cur_start); +if (sram->clk) +clk_disable_unprepare (sram->clk);
like here
Quoted 16 lines
drivers/misc/sram.c +16
-0+return -EINVAL ; + } + +dev_dbg (&pdev->dev,"found reserved block 0x%x-0x%x\n" , + rstart, rstart + rsize); + +/* current start is in a reserved block */ +if (rstart <= cur_start) { + cur_start = rstart + rsize; +continue ; + } + +/* +* allocate the space between the current starting +* address and the following reserved block +*/
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 134
Quoted 10 lines
drivers/misc/sram.c +10
-0+ cur_size = rstart - cur_start; + +dev_dbg (&pdev->dev,"adding chunk 0x%x-0x%x\n" , + cur_start, cur_start + cur_size); + ret =gen_pool_add_virt (sram->pool, + (unsigned long )virt_base + cur_start, + res->start + cur_start, cur_size,-1 ); +if (ret <0 ) { +if (sram->clk) +clk_disable_unprepare (sram->clk);
and here.
Quoted 11 lines
drivers/misc/sram.c +11
-0+return ret; + } + +/* next allocation after this reserved block */ + cur_start = rstart + rsize; + } + +/* allocate the space after the last reserved block */ +if (cur_start < size) { + cur_size = size - cur_start; +
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 155
Quoted 10 lines
drivers/misc/sram.c +7
-0+dev_dbg (&pdev->dev,"adding chunk 0x%x-0x%x\n" , + cur_start, cur_start + cur_size); + ret =gen_pool_add_virt (sram->pool, + (unsigned long )virt_base + cur_start, + res->start + cur_start, cur_size,-1 ); + } + }platform_set_drvdata (pdev, sram);
Also, I think you could reduce the duplication of gen_pool_add_virt() function calls, somehow like this:
unsigned int cur_start = 0;
unsigned int cur_size;
unsigned int rstart;
unsigned int rsize;
int i = 0;
if (!reserved_list)
reserved_size = 0;
for (i = 0; i < (reserved_size + 2); i += 2) {
if (i < reserved_size) {
/* get the next reserved block */
rstart = be32_to_cpu(*reserved_list++);
rsize = be32_to_cpu(*reserved_list++);
/* catch unsorted list entries */
if (rstart < cur_start) {
dev_err(&pdev->dev,
"unsorted reserved list (0x%x before current 0x%x)\n",
rstart, cur_start);
return -EINVAL;
}
dev_dbg(&pdev->dev,
"found reserved block 0x%x-0x%x\n",
rstart, rstart + rsize);
} else {
/* the last chunk extends to the end of the region */
rstart = size;
}
/* current start is in a reserved block */
if (rstart <= cur_start) {
cur_start = rstart + rsize;
continue;
}
/*
* allocate the space between the current starting
* address and the following reserved block, or the
* end of the region.
*/
cur_size = rstart - cur_start;
dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n",
cur_start, cur_start + cur_size);
ret = gen_pool_add_virt(sram->pool,
(unsigned long)virt_base + cur_start,
res->start + cur_start, cur_size, -1);
if (ret < 0)
return ret;
}regards
Philipp
Heiko Stübner
maintainerRe: [PATCH v2 2/6] misc: sram: add ability to mark sram sections as reserved
Hi Philipp,
Am Dienstag, 25. Juni 2013, 12:17:05 schrieb Philipp Zabel:
Quoted 28 lines
Hi Heiko,
Am Dienstag, den 25.06.2013, 10:47 +0200 schrieb Heiko St?bner:
Quoted 25 lines
Some SoCs need parts of their sram for special purposes. So while being part of the periphal, it should not be part of the genpool controlling the sram.
Threfore add an option mmio-sram-reserved to keep arbitary portions of the sram from being part of the pool.
Suggested-by Rob Herring <[email protected]>Signed-off-by Heiko Stuebner <[email protected]>---
Documentation/devicetree/bindings/misc/sram.txt | 8 +++ drivers/misc/sram.c | 86+++++++++++++++++++++-- 2 files changed, 88 insertions(+), 6 deletions(-)
(quoted diff) +0
-0
b/Documentation/devicetree/bindings/misc/sram.txtindex 4d0a00e..eae080e 100644Documentation/devicetree/bindings/misc/sram.txt +0
-0@@ -8,9 +8,17 @@ Required properties: - reg : SRAM iomem address range
Quoted 10 lines
Quoted 7 lines
Documentation/devicetree/bindings/misc/sram.txt +3
-0+Optional properties: + +- mmio-sram-reserved: ordered list of reserved chunks inside the sramthat + should not become part of the genalloc pool.
+ Format is <base size>, <base size>, ...; with base being relative to the + reg property base.
+the keyword to reserve blocks of ram is /memreserve/ - should this property name be aligned with that?
The mmio-sram-reserved name was suggested by Rob Herring, who I suppose has some slight experience with devicetree :-) .
I wasn't able to find real documentation on /memreserve/ but it looks more like it's used to reserve generic memregions, not being node-specific. So reusing this might also cause confusion when the reserve-data now is relative to it's node reg.
Quoted 65 lines
Quoted 51 lines
Example:
sram: sram at
5c000000{Documentation/devicetree/bindings/misc/sram.txt +1
-0compatible = "mmio-sram"; reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */ + mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */ };drivers/misc/sram.c +2
-0@@ -42,6 +42,8 @@ static int sram_probe(struct platform_device *pdev)struct sram_dev *sram;struct resource *res;unsigned long size; +const __be32 *reserved_list =NULL ; +int reserved_size =0 ;drivers/misc/sram.c +10
-6int ret; res =platform_get_resource (pdev,IORESOURCE_MEM ,0 );@@ -65,12 +67,89 @@ static int sram_probe(struct platform_device *pdev) if (!sram->pool) return -ENOMEM; - ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, - res->start, size,-1 ); -if (ret <0 ) { -if (sram->clk) -clk_disable_unprepare (sram->clk); -return ret; + if (pdev->dev.of_node) { + reserved_list = of_get_property(pdev->dev.of_node, + "mmio-sram-reserved", + &reserved_size); + if (reserved_list) { + reserved_size /= sizeof(*reserved_list); + if (!reserved_size || reserved_size % 2) { + dev_warn(&pdev->dev, "wrong number of arguments in mmio-sram-reserved\n"); + reserved_list = NULL; + } + }drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 92
Quoted 10 lines
drivers/misc/sram.c +10
-0+ } + +if (!reserved_list) { + ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, + res->start, size,-1 ); +if (ret <0 ) { +if (sram->clk) +clk_disable_unprepare (sram->clk); +return ret; + }Moving the
clk_prepare_enable()further down would allow to avoid theclk_disable_unprepare()in every error path,
Quoted 15 lines
Quoted 5 lines
drivers/misc/sram.c +5
-0+ }else { +unsigned int cur_start =0 ; +unsigned int cur_size; +unsigned int rstart; +unsigned int rsize;drivers/misc/sram.c static int sram_probe(struct platform_device *pdev)
Quoted 10 lines
drivers/misc/sram.c +10
-0+int i; + +for (i =0 ; i < reserved_size; i +=2 ) { +/* get the next reserved block */ + rstart =be32_to_cpu (*reserved_list++); + rsize =be32_to_cpu (*reserved_list++); + +/* catch unsorted list entries */ +if (rstart < cur_start) { + dev_err(&pdev->dev, "unsorted reserved list (0x%x before
current
Quoted 105 lines
drivers/misc/sram.c
Quoted 3 lines
0x%x)\n", + rstart,
cur_start);drivers/misc/sram.c +2
-0+if (sram->clk) +clk_disable_unprepare (sram->clk);like here
Quoted 16 lines
drivers/misc/sram.c +16
-0+return -EINVAL ; + } + +dev_dbg (&pdev->dev,"found reserved block 0x%x-0x%x\n" , + rstart, rstart + rsize); + +/* current start is in a reserved block */ +if (rstart <= cur_start) { + cur_start = rstart + rsize; +continue ; + } + +/* +* allocate the space between the current starting +* address and the following reserved block +*/ drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 134
Quoted 10 lines
drivers/misc/sram.c +10
-0+ cur_size = rstart - cur_start; + +dev_dbg (&pdev->dev,"adding chunk 0x%x-0x%x\n" , + cur_start, cur_start + cur_size); + ret =gen_pool_add_virt (sram->pool, + (unsigned long )virt_base + cur_start, + res->start + cur_start, cur_size,-1 ); +if (ret <0 ) { +if (sram->clk) +clk_disable_unprepare (sram->clk);and here.
Quoted 12 lines
drivers/misc/sram.c +12
-0+return ret; + } + +/* next allocation after this reserved block */ + cur_start = rstart + rsize; + } + +/* allocate the space after the last reserved block */ +if (cur_start < size) { + cur_size = size - cur_start; + + dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n",drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 155
Quoted 10 lines
drivers/misc/sram.c +6
-0+ cur_start, cur_start + cur_size); + ret =gen_pool_add_virt (sram->pool, + (unsigned long )virt_base + cur_start, + res->start + cur_start, cur_size,-1 ); + } + }platform_set_drvdata (pdev, sram);Also, I think you could reduce the duplication of
gen_pool_add_virt()function calls, somehow like this:unsigned int cur_start = 0; unsigned int cur_size; unsigned int rstart; unsigned int rsize; int i = 0; if (!reserved_list) reserved_size = 0; for (i = 0; i < (reserved_size + 2); i += 2) { if (i < reserved_size) { /* get the next reserved block */ rstart = be32_to_cpu(*reserved_list++); rsize = be32_to_cpu(*reserved_list++); /* catch unsorted list entries */ if (rstart < cur_start) { dev_err(&pdev->dev, "unsorted reserved list (0x%x before current 0x%x)\n", rstart, cur_start); return -EINVAL; } dev_dbg(&pdev->dev, "found reserved block 0x%x-0x%x\n", rstart, rstart + rsize); } else { /* the last chunk extends to the end of the region */ rstart = size; } /* current start is in a reserved block */ if (rstart <= cur_start) { cur_start = rstart + rsize; continue; } /* * allocate the space between the current starting * address and the following reserved block, or the * end of the region. */ cur_size = rstart - cur_start;
Quoted 8 lines
dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", cur_start, cur_start + cur_size); ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base + cur_start, res->start + cur_start, cur_size, -1); if (ret < 0) return ret; }
yep, this looks nicer - same for moving the clk_prepare_enable to below this loop to unclutter the error-path.
So I will incorporate this in v3.
Thanks
Heiko
Re: [PATCH v2 2/6] misc: sram: add ability to mark sram sections as reserved
Am Mittwoch, den 26.06.2013, 11:18 +0200 schrieb Heiko St?bner:
Quoted 42 lines
Hi Philipp,
Am Dienstag, 25. Juni 2013, 12:17:05 schrieb Philipp Zabel:
Quoted 28 lines
Hi Heiko,
Am Dienstag, den 25.06.2013, 10:47 +0200 schrieb Heiko St?bner:
Quoted 25 lines
Some SoCs need parts of their sram for special purposes. So while being part of the periphal, it should not be part of the genpool controlling the sram.
Threfore add an option mmio-sram-reserved to keep arbitary portions of the sram from being part of the pool.
Suggested-by Rob Herring <[email protected]>Signed-off-by Heiko Stuebner <[email protected]>---
Documentation/devicetree/bindings/misc/sram.txt | 8 +++ drivers/misc/sram.c | 86+++++++++++++++++++++-- 2 files changed, 88 insertions(+), 6 deletions(-)
(quoted diff) +0
-0
b/Documentation/devicetree/bindings/misc/sram.txtindex 4d0a00e..eae080e 100644Documentation/devicetree/bindings/misc/sram.txt +0
-0@@ -8,9 +8,17 @@ Required properties: - reg : SRAM iomem address rangeQuoted 10 lines
Quoted 7 lines
Documentation/devicetree/bindings/misc/sram.txt +3
-0+Optional properties: + +- mmio-sram-reserved: ordered list of reserved chunks inside the sramthat + should not become part of the genalloc pool.
+ Format is <base size>, <base size>, ...; with base being relative to the + reg property base.
+the keyword to reserve blocks of ram is /memreserve/ - should this property name be aligned with that?
Quoted 2 lines
The mmio-sram-reserved name was suggested by Rob Herring, who I suppose has some slight experience with devicetree :-) .
That did sound like a suggestion from the top of his head, though. I just wanted to point out the similarity in function, in case it is relevant.
Quoted 4 lines
I wasn't able to find real documentation on /memreserve/ but it looks more like it's used to reserve generic memregions, not being node-specific. So reusing this might also cause confusion when the reserve-data now is relative to it's node reg.
True.
Quoted 195 lines
Quoted 65 lines
Quoted 51 lines
Example:
sram: sram at
5c000000{Documentation/devicetree/bindings/misc/sram.txt +1
-0compatible = "mmio-sram"; reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */ + mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */ };drivers/misc/sram.c +2
-0@@ -42,6 +42,8 @@ static int sram_probe(struct platform_device *pdev)struct sram_dev *sram;struct resource *res;unsigned long size; +const __be32 *reserved_list =NULL ; +int reserved_size =0 ;drivers/misc/sram.c +10
-6int ret; res =platform_get_resource (pdev,IORESOURCE_MEM ,0 );@@ -65,12 +67,89 @@ static int sram_probe(struct platform_device *pdev) if (!sram->pool) return -ENOMEM; - ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, - res->start, size,-1 ); -if (ret <0 ) { -if (sram->clk) -clk_disable_unprepare (sram->clk); -return ret; + if (pdev->dev.of_node) { + reserved_list = of_get_property(pdev->dev.of_node, + "mmio-sram-reserved", + &reserved_size); + if (reserved_list) { + reserved_size /= sizeof(*reserved_list); + if (!reserved_size || reserved_size % 2) { + dev_warn(&pdev->dev, "wrong number of arguments in mmio-sram-reserved\n"); + reserved_list = NULL; + } + }drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 92
Quoted 10 lines
drivers/misc/sram.c +10
-0+ } + +if (!reserved_list) { + ret =gen_pool_add_virt (sram->pool, (unsigned long )virt_base, + res->start, size,-1 ); +if (ret <0 ) { +if (sram->clk) +clk_disable_unprepare (sram->clk); +return ret; + }Moving the
clk_prepare_enable()further down would allow to avoid theclk_disable_unprepare()in every error path,Quoted 15 lines
Quoted 5 lines
drivers/misc/sram.c +5
-0+ }else { +unsigned int cur_start =0 ; +unsigned int cur_size; +unsigned int rstart; +unsigned int rsize;drivers/misc/sram.c static int sram_probe(struct platform_device *pdev)
Quoted 10 lines
drivers/misc/sram.c +10
-0+int i; + +for (i =0 ; i < reserved_size; i +=2 ) { +/* get the next reserved block */ + rstart =be32_to_cpu (*reserved_list++); + rsize =be32_to_cpu (*reserved_list++); + +/* catch unsorted list entries */ +if (rstart < cur_start) { + dev_err(&pdev->dev, "unsorted reserved list (0x%x beforecurrent
Quoted 105 lines
drivers/misc/sram.c
Quoted 3 lines
0x%x)\n", + rstart,
cur_start);drivers/misc/sram.c +2
-0+if (sram->clk) +clk_disable_unprepare (sram->clk);like here
Quoted 16 lines
drivers/misc/sram.c +16
-0+return -EINVAL ; + } + +dev_dbg (&pdev->dev,"found reserved block 0x%x-0x%x\n" , + rstart, rstart + rsize); + +/* current start is in a reserved block */ +if (rstart <= cur_start) { + cur_start = rstart + rsize; +continue ; + } + +/* +* allocate the space between the current starting +* address and the following reserved block +*/ drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 134
Quoted 10 lines
drivers/misc/sram.c +10
-0+ cur_size = rstart - cur_start; + +dev_dbg (&pdev->dev,"adding chunk 0x%x-0x%x\n" , + cur_start, cur_start + cur_size); + ret =gen_pool_add_virt (sram->pool, + (unsigned long )virt_base + cur_start, + res->start + cur_start, cur_size,-1 ); +if (ret <0 ) { +if (sram->clk) +clk_disable_unprepare (sram->clk);and here.
Quoted 12 lines
drivers/misc/sram.c +12
-0+return ret; + } + +/* next allocation after this reserved block */ + cur_start = rstart + rsize; + } + +/* allocate the space after the last reserved block */ +if (cur_start < size) { + cur_size = size - cur_start; + + dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n",drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 155
Quoted 10 lines
drivers/misc/sram.c +6
-0+ cur_start, cur_start + cur_size); + ret =gen_pool_add_virt (sram->pool, + (unsigned long )virt_base + cur_start, + res->start + cur_start, cur_size,-1 ); + } + }platform_set_drvdata (pdev, sram);Also, I think you could reduce the duplication of
gen_pool_add_virt()function calls, somehow like this:unsigned int cur_start = 0; unsigned int cur_size; unsigned int rstart; unsigned int rsize; int i = 0; if (!reserved_list) reserved_size = 0; for (i = 0; i < (reserved_size + 2); i += 2) { if (i < reserved_size) { /* get the next reserved block */ rstart = be32_to_cpu(*reserved_list++); rsize = be32_to_cpu(*reserved_list++); /* catch unsorted list entries */ if (rstart < cur_start) { dev_err(&pdev->dev, "unsorted reserved list (0x%x before current 0x%x)\n", rstart, cur_start); return -EINVAL; } dev_dbg(&pdev->dev, "found reserved block 0x%x-0x%x\n", rstart, rstart + rsize); } else { /* the last chunk extends to the end of the region */ rstart = size; } /* current start is in a reserved block */ if (rstart <= cur_start) { cur_start = rstart + rsize; continue; } /* * allocate the space between the current starting * address and the following reserved block, or the * end of the region. */ cur_size = rstart - cur_start;Quoted 8 lines
dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", cur_start, cur_start + cur_size); ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base + cur_start, res->start + cur_start, cur_size, -1); if (ret < 0) return ret; }
Quoted 4 lines
yep, this looks nicer - same for moving the
clk_prepare_enableto below this loop to unclutter the error-path.So I will incorporate this in v3.
Excellent.
thanks
Philipp
Heiko Stübner
maintainerPATCH v2 3/6 ARM: rockchip: add snoop-control-unit
In reply to Heiko StübnerThis adds the device-node and config select to enable the scu in all Rockchip Cortex-A9 SoCs.
--- arch/arm/boot/dts/rk3066a.dtsi | 5 +++++ arch/arm/mach-rockchip/Kconfig | 1 + 2 files changed, 6 insertions(+)
arch/arm/boot/dts/rk3066a.dtsi +5 -0
@@ -48,6 +48,11 @@ compatible = "simple-bus"; ranges; + scu at 1013c000 { + compatible = "arm,cortex-a9-scu"; + reg = <0x1013c000 0x100>; + }; + gic: interrupt-controller at 1013d000 { compatible = "arm,cortex-a9-gic"; interrupt-controller;
arch/arm/mach-rockchip/Kconfig +1 -0
@@ -5,6 +5,7 @@ config ARCH_ROCKCHIP select ARCH_REQUIRE_GPIOLIB select ARM_GIC select CACHE_L2X0 + select HAVE_ARM_SCU select HAVE_ARM_TWD if LOCAL_TIMERS select HAVE_SMP select LOCAL_TIMERS if SMP
Signature · 1 line
1.7.10.4
Heiko Stübner
maintainerPATCH v2 4/6 ARM: rockchip: add sram dt nodes and documentation
In reply to Heiko StübnerThe Rockchip SoCs need a special part of their sram for bringup of additional cores. Therefore also add a reserved section when adding the mmio-sram node to keep the sram driver from using this space.
--- .../devicetree/bindings/arm/rockchip/smp-sram.txt | 23 ++++++++++++++++++++ arch/arm/boot/dts/rk3066a.dtsi | 6 +++++ 2 files changed, 29 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt
Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt new file +23 -0
@@ -0,0 +1,23 @@ +Rockchip SRAM for smp bringup: +------------------------------ + +Rockchip's smp-capable SoCs use the first part of the sram for the bringup +of the cores. Once the core gets powered up it executes the code that is +residing at the very beginning of the sram. + +Therefore a reserved section has to be added to the mmio-sram declaration. + +Required node properties: +- compatible : should contain both "rockchip,rk3066-sram", "mmio-sram" + so that the smp code can select the correct sram node. + +The rest of the properties should follow the generic mmio-sram discription +found in ../../misc/sram.txt + +Example: + + sram: sram at 10080000 { + compatible = "rockchip,rk3066-sram", "mmio-sram"; + reg = <0x10080000 0x10000>; + mmio-sram-reserved = <0x0 0x50>; + };
arch/arm/boot/dts/rk3066a.dtsi +6 -0
@@ -53,6 +53,12 @@ reg = <0x1013c000 0x100>; }; + sram: sram at 10080000 { + compatible = "rockchip,rk3066-sram", "mmio-sram"; + reg = <0x10080000 0x10000>; + mmio-sram-reserved = <0x0 0x50>; + }; + gic: interrupt-controller at 1013d000 { compatible = "arm,cortex-a9-gic"; interrupt-controller;
Signature · 1 line
1.7.10.4
Heiko Stübner
maintainerPATCH v2 5/6 ARM: rockchip: add power-management-unit dt node
In reply to Heiko StübnerThe pmu is needed to bring up the cores during smp operations. Therefore add a node and documentation for it.
--- Documentation/devicetree/bindings/arm/rockchip/pmu.txt | 16 ++++++++++++++++ arch/arm/boot/dts/rk3066a.dtsi | 5 +++++ 2 files changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/rockchip/pmu.txt
Documentation/devicetree/bindings/arm/rockchip/pmu.txt new file +16 -0
@@ -0,0 +1,16 @@ +Rockchip power-management-unit: +------------------------------- + +The pmu is used to turn off and on different power domains of the SoCs +This includes the power to the CPU cores. + +Required node properties: +- compatible value : = "rockchip,rk3066-pmu"; +- reg : physical base address and the size of the registers window + +Example: + + pmu at 20004000 { + compatible = "rockchip,rk3066-pmu"; + reg = <0x20004000 0x100>; + };
arch/arm/boot/dts/rk3066a.dtsi +5 -0
@@ -59,6 +59,11 @@ mmio-sram-reserved = <0x0 0x50>; }; + pmu at 20004000 { + compatible = "rockchip,rk3066-pmu"; + reg = <0x20004000 0x100>; + }; + gic: interrupt-controller at 1013d000 { compatible = "arm,cortex-a9-gic"; interrupt-controller;
Signature · 1 line
1.7.10.4
Heiko Stübner
maintainerPATCH v2 6/6 ARM: rockchip: add smp bringup code
In reply to Heiko StübnerThis adds the necessary smp-operations and startup code to use additional cores on Rockchip SoCs.
We currently hog the power management unit in the smp code, as it is necessary to control the power to the cpu core and nothing else is currently using it, so a generic implementation can be done later.
--- arch/arm/mach-rockchip/Makefile | 1 + arch/arm/mach-rockchip/core.h | 22 ++++ arch/arm/mach-rockchip/headsmp.S | 32 ++++++ arch/arm/mach-rockchip/platsmp.c | 203 +++++++++++++++++++++++++++++++++++++ arch/arm/mach-rockchip/rockchip.c | 2 + 5 files changed, 260 insertions(+) create mode 100644 arch/arm/mach-rockchip/core.h create mode 100644 arch/arm/mach-rockchip/headsmp.S create mode 100644 arch/arm/mach-rockchip/platsmp.c
arch/arm/mach-rockchip/Makefile +1 -0
@@ -1 +1,2 @@ obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o +obj-$(CONFIG_SMP) += headsmp.o platsmp.o
arch/arm/mach-rockchip/core.h new file +22 -0
@@ -0,0 +1,22 @@ +/* +* Copyright (c) 2013 MundoReader S.L. +* Author: Heiko Stuebner <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ + +extern char rockchip_secondary_trampoline; +extern char rockchip_secondary_trampoline_end; + +extern unsigned long rockchip_boot_fn; +extern void rockchip_secondary_startup (void ); + +extern struct smp_operations rockchip_smp_ops;
arch/arm/mach-rockchip/headsmp.S new file +32 -0
@@ -0,0 +1,32 @@ +/* + * Copyright (c) 2013 MundoReader S.L. + * Author: Heiko Stuebner <[email protected]> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#include <linux/linkage.h> +#include <linux/init.h> + + __CPUINIT + +ENTRY(rockchip_secondary_startup) + bl v7_invalidate_l1 + b secondary_startup +ENDPROC(rockchip_secondary_startup) + +ENTRY(rockchip_secondary_trampoline) + ldr pc, 1f +ENDPROC(rockchip_secondary_trampoline) + .globl rockchip_boot_fn +rockchip_boot_fn: +1: .space 4 + +ENTRY(rockchip_secondary_trampoline_end)
arch/arm/mach-rockchip/platsmp.c new file +203 -0
203 lines not on this page · raw message
arch/arm/mach-rockchip/rockchip.c +2 -0
@@ -24,6 +24,7 @@#include <asm/mach/arch.h> #include <asm/mach/map.h> #include <asm/hardware/cache-l2x0.h> +#include "core.h" static void __init rockchip_timer_init (void ) {
@@ -46,6 +47,7 @@ static const char * const rockchip_board_dt_compat[] = { };DT_MACHINE_START (ROCKCHIP_DT ,"Rockchip Cortex-A9 (Device Tree)" ) + .smp = smp_ops(rockchip_smp_ops), .init_machine = rockchip_dt_init, .init_time = rockchip_timer_init, .dt_compat = rockchip_board_dt_compat,
Signature · 1 line
1.7.10.4