* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-23 12:57 [PATCH 0/3] ARM: zynq: add SMP support for zynq7000 Steffen Trumtrar
@ 2013-03-23 12:57 ` Steffen Trumtrar
2013-03-25 14:01 ` Michal Simek
2013-03-23 12:57 ` [PATCH 2/3] ARM: zynq: get slcr base earlier Steffen Trumtrar
2013-03-23 12:57 ` [PATCH 3/3] ARM: zynq: add SMP support Steffen Trumtrar
2 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-23 12:57 UTC (permalink / raw)
To: linux-arm-kernel
Instead of hardcoding the base address of the SCU get it from the device.
While at it, add the SCU to the DT.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Josh Cartwright <josh.cartwright@ni.com>
---
arch/arm/boot/dts/zynq-7000.dtsi | 5 +++++
arch/arm/mach-zynq/common.c | 34 ++++++++++++++++++++++------------
2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 88564fa..c103082 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -199,6 +199,11 @@
};
};
+ scu: scu at f8f000000 {
+ compatible = "arm,cortex-a9-scu";
+ reg = <0xf8f00000 0x58>;
+ };
+
timer: timer at f8f00600 {
compatible = "arm,cortex-a9-twd-timer";
reg = <0xf8f00600 0x20>;
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 920e20a..014131c 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -32,11 +32,14 @@
#include <asm/mach-types.h>
#include <asm/page.h>
#include <asm/pgtable.h>
+#include <asm/smp_scu.h>
#include <asm/smp_twd.h>
#include <asm/hardware/cache-l2x0.h>
#include "common.h"
+void __iomem *scu_base;
+
static struct of_device_id zynq_of_bus_ids[] __initdata = {
{ .compatible = "simple-bus", },
{}
@@ -56,17 +59,6 @@ static void __init xilinx_init_machine(void)
of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
}
-#define SCU_PERIPH_PHYS 0xF8F00000
-#define SCU_PERIPH_SIZE SZ_8K
-#define SCU_PERIPH_VIRT (VMALLOC_END - SCU_PERIPH_SIZE)
-
-static struct map_desc scu_desc __initdata = {
- .virtual = SCU_PERIPH_VIRT,
- .pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
- .length = SCU_PERIPH_SIZE,
- .type = MT_DEVICE,
-};
-
static void __init xilinx_zynq_timer_init(void)
{
struct device_node *np;
@@ -82,13 +74,31 @@ static void __init xilinx_zynq_timer_init(void)
xttcps_timer_init();
}
+static struct map_desc zynq_cortex_a9_scu_map __initdata = {
+ .length = SZ_8K,
+ .type = MT_DEVICE,
+};
+
+void __init zynq_scu_map_io(void)
+{
+ if (scu_a9_has_base()) {
+ unsigned long base;
+
+ base = scu_a9_get_base();
+ zynq_cortex_a9_scu_map.pfn = __phys_to_pfn(base);
+ zynq_cortex_a9_scu_map.virtual = base;
+ iotable_init(&zynq_cortex_a9_scu_map, 1);
+ scu_base = ioremap(base, zynq_cortex_a9_scu_map.length);
+ }
+}
+
/**
* xilinx_map_io() - Create memory mappings needed for early I/O.
*/
static void __init xilinx_map_io(void)
{
debug_ll_io_init();
- iotable_init(&scu_desc, 1);
+ zynq_scu_map_io();
}
static const char *xilinx_dt_match[] = {
--
1.8.2.rc2
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-23 12:57 ` [PATCH 1/3] ARM: zynq: read scu base from SoC Steffen Trumtrar
@ 2013-03-25 14:01 ` Michal Simek
2013-03-25 14:25 ` Steffen Trumtrar
0 siblings, 1 reply; 18+ messages in thread
From: Michal Simek @ 2013-03-25 14:01 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> Instead of hardcoding the base address of the SCU get it from the device.
> While at it, add the SCU to the DT.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Josh Cartwright <josh.cartwright@ni.com>
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 5 +++++
> arch/arm/mach-zynq/common.c | 34 ++++++++++++++++++++++------------
> 2 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index 88564fa..c103082 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -199,6 +199,11 @@
> };
> };
>
> + scu: scu at f8f000000 {
> + compatible = "arm,cortex-a9-scu";
> + reg = <0xf8f00000 0x58>;
> + };
> +
> timer: timer at f8f00600 {
> compatible = "arm,cortex-a9-twd-timer";
> reg = <0xf8f00600 0x20>;
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index 920e20a..014131c 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -32,11 +32,14 @@
> #include <asm/mach-types.h>
> #include <asm/page.h>
> #include <asm/pgtable.h>
> +#include <asm/smp_scu.h>
> #include <asm/smp_twd.h>
> #include <asm/hardware/cache-l2x0.h>
>
> #include "common.h"
>
> +void __iomem *scu_base;
This must be defined in header - will produce sparse warning.
> +
> static struct of_device_id zynq_of_bus_ids[] __initdata = {
> { .compatible = "simple-bus", },
> {}
> @@ -56,17 +59,6 @@ static void __init xilinx_init_machine(void)
> of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
> }
>
> -#define SCU_PERIPH_PHYS 0xF8F00000
> -#define SCU_PERIPH_SIZE SZ_8K
> -#define SCU_PERIPH_VIRT (VMALLOC_END - SCU_PERIPH_SIZE)
> -
> -static struct map_desc scu_desc __initdata = {
> - .virtual = SCU_PERIPH_VIRT,
> - .pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
> - .length = SCU_PERIPH_SIZE,
> - .type = MT_DEVICE,
> -};
> -
> static void __init xilinx_zynq_timer_init(void)
> {
> struct device_node *np;
> @@ -82,13 +74,31 @@ static void __init xilinx_zynq_timer_init(void)
> xttcps_timer_init();
> }
>
> +static struct map_desc zynq_cortex_a9_scu_map __initdata = {
> + .length = SZ_8K,
That's bogus. Size 8k is too big because it also cover gic which is wrong.
As you can see from xilinx git repo correct size is 256 or less.
> + .type = MT_DEVICE,
> +};
> +
> +void __init zynq_scu_map_io(void)
Should be static.
> +{
> + if (scu_a9_has_base()) {
I am not calling this function because I think it is not necessary to do so
because it is run only on a9 where scu_a9_get_base will just work.
Of did I miss anything?
> + unsigned long base;
> +
> + base = scu_a9_get_base();
> + zynq_cortex_a9_scu_map.pfn = __phys_to_pfn(base);
> + zynq_cortex_a9_scu_map.virtual = base;
> + iotable_init(&zynq_cortex_a9_scu_map, 1);
> + scu_base = ioremap(base, zynq_cortex_a9_scu_map.length);
> + }
> +}
> +
> /**
> * xilinx_map_io() - Create memory mappings needed for early I/O.
> */
> static void __init xilinx_map_io(void)
> {
> debug_ll_io_init();
> - iotable_init(&scu_desc, 1);
> + zynq_scu_map_io();
> }
You are using a little bit different names than we have in xilinx git tree
but maybe worth to call it as you.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-25 14:01 ` Michal Simek
@ 2013-03-25 14:25 ` Steffen Trumtrar
2013-03-25 14:59 ` Michal Simek
0 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-25 14:25 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 03:01:59PM +0100, Michal Simek wrote:
> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> > Instead of hardcoding the base address of the SCU get it from the device.
> > While at it, add the SCU to the DT.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > Cc: Michal Simek <michal.simek@xilinx.com>
> > Cc: Josh Cartwright <josh.cartwright@ni.com>
> > ---
> > arch/arm/boot/dts/zynq-7000.dtsi | 5 +++++
> > arch/arm/mach-zynq/common.c | 34 ++++++++++++++++++++++------------
> > 2 files changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> > index 88564fa..c103082 100644
> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> > @@ -199,6 +199,11 @@
> > };
> > };
> >
> > + scu: scu at f8f000000 {
> > + compatible = "arm,cortex-a9-scu";
> > + reg = <0xf8f00000 0x58>;
> > + };
> > +
> > timer: timer at f8f00600 {
> > compatible = "arm,cortex-a9-twd-timer";
> > reg = <0xf8f00600 0x20>;
> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> > index 920e20a..014131c 100644
> > --- a/arch/arm/mach-zynq/common.c
> > +++ b/arch/arm/mach-zynq/common.c
> > @@ -32,11 +32,14 @@
> > #include <asm/mach-types.h>
> > #include <asm/page.h>
> > #include <asm/pgtable.h>
> > +#include <asm/smp_scu.h>
> > #include <asm/smp_twd.h>
> > #include <asm/hardware/cache-l2x0.h>
> >
> > #include "common.h"
> >
> > +void __iomem *scu_base;
>
> This must be defined in header - will produce sparse warning.
>
Okay. No problem. I can change that.
> > +
> > static struct of_device_id zynq_of_bus_ids[] __initdata = {
> > { .compatible = "simple-bus", },
> > {}
> > @@ -56,17 +59,6 @@ static void __init xilinx_init_machine(void)
> > of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
> > }
> >
> > -#define SCU_PERIPH_PHYS 0xF8F00000
> > -#define SCU_PERIPH_SIZE SZ_8K
> > -#define SCU_PERIPH_VIRT (VMALLOC_END - SCU_PERIPH_SIZE)
> > -
> > -static struct map_desc scu_desc __initdata = {
> > - .virtual = SCU_PERIPH_VIRT,
> > - .pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
> > - .length = SCU_PERIPH_SIZE,
> > - .type = MT_DEVICE,
> > -};
> > -
> > static void __init xilinx_zynq_timer_init(void)
> > {
> > struct device_node *np;
> > @@ -82,13 +74,31 @@ static void __init xilinx_zynq_timer_init(void)
> > xttcps_timer_init();
> > }
> >
> > +static struct map_desc zynq_cortex_a9_scu_map __initdata = {
> > + .length = SZ_8K,
>
> That's bogus. Size 8k is too big because it also cover gic which is wrong.
> As you can see from xilinx git repo correct size is 256 or less.
>
Hm,... you are obviously correct. 256 it is.
> > + .type = MT_DEVICE,
> > +};
> > +
> > +void __init zynq_scu_map_io(void)
>
> Should be static.
>
Yes.
> > +{
> > + if (scu_a9_has_base()) {
>
> I am not calling this function because I think it is not necessary to do so
> because it is run only on a9 where scu_a9_get_base will just work.
> Of did I miss anything?
>
As long as this file is only used for zynqs with A9 this call is not needed.
Right.
>
> > + unsigned long base;
> > +
> > + base = scu_a9_get_base();
> > + zynq_cortex_a9_scu_map.pfn = __phys_to_pfn(base);
> > + zynq_cortex_a9_scu_map.virtual = base;
> > + iotable_init(&zynq_cortex_a9_scu_map, 1);
> > + scu_base = ioremap(base, zynq_cortex_a9_scu_map.length);
> > + }
> > +}
> > +
> > /**
> > * xilinx_map_io() - Create memory mappings needed for early I/O.
> > */
> > static void __init xilinx_map_io(void)
> > {
> > debug_ll_io_init();
> > - iotable_init(&scu_desc, 1);
> > + zynq_scu_map_io();
> > }
>
> You are using a little bit different names than we have in xilinx git tree
> but maybe worth to call it as you.
>
I propose using the common mainline way of calling this functions.
When there maybe will be more xilinx SoCs in mainline it will be easier to find
what one is looking for.
I actually wanted to make an RFC patch naming everything to zynq_* instead of
xilinx_* and replace the "MACHINE_START(XILINX_EP107,...".
Didn't get around to is though.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-25 14:25 ` Steffen Trumtrar
@ 2013-03-25 14:59 ` Michal Simek
2013-03-26 11:08 ` Steffen Trumtrar
0 siblings, 1 reply; 18+ messages in thread
From: Michal Simek @ 2013-03-25 14:59 UTC (permalink / raw)
To: linux-arm-kernel
Hi Steffen,
2013/3/25 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> On Mon, Mar 25, 2013 at 03:01:59PM +0100, Michal Simek wrote:
>> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> > Instead of hardcoding the base address of the SCU get it from the device.
>> > While at it, add the SCU to the DT.
>> >
>> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> > Cc: Michal Simek <michal.simek@xilinx.com>
>> > Cc: Josh Cartwright <josh.cartwright@ni.com>
>> > ---
>> > arch/arm/boot/dts/zynq-7000.dtsi | 5 +++++
>> > arch/arm/mach-zynq/common.c | 34 ++++++++++++++++++++++------------
>> > 2 files changed, 27 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> > index 88564fa..c103082 100644
>> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> > @@ -199,6 +199,11 @@
>> > };
>> > };
>> >
>> > + scu: scu at f8f000000 {
>> > + compatible = "arm,cortex-a9-scu";
>> > + reg = <0xf8f00000 0x58>;
>> > + };
>> > +
>> > timer: timer at f8f00600 {
>> > compatible = "arm,cortex-a9-twd-timer";
>> > reg = <0xf8f00600 0x20>;
>> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
>> > index 920e20a..014131c 100644
>> > --- a/arch/arm/mach-zynq/common.c
>> > +++ b/arch/arm/mach-zynq/common.c
>> > @@ -32,11 +32,14 @@
>> > #include <asm/mach-types.h>
>> > #include <asm/page.h>
>> > #include <asm/pgtable.h>
>> > +#include <asm/smp_scu.h>
>> > #include <asm/smp_twd.h>
>> > #include <asm/hardware/cache-l2x0.h>
>> >
>> > #include "common.h"
>> >
>> > +void __iomem *scu_base;
>>
>> This must be defined in header - will produce sparse warning.
>>
>
> Okay. No problem. I can change that.
>
>> > +
>> > static struct of_device_id zynq_of_bus_ids[] __initdata = {
>> > { .compatible = "simple-bus", },
>> > {}
>> > @@ -56,17 +59,6 @@ static void __init xilinx_init_machine(void)
>> > of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
>> > }
>> >
>> > -#define SCU_PERIPH_PHYS 0xF8F00000
>> > -#define SCU_PERIPH_SIZE SZ_8K
>> > -#define SCU_PERIPH_VIRT (VMALLOC_END - SCU_PERIPH_SIZE)
>> > -
>> > -static struct map_desc scu_desc __initdata = {
>> > - .virtual = SCU_PERIPH_VIRT,
>> > - .pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
>> > - .length = SCU_PERIPH_SIZE,
>> > - .type = MT_DEVICE,
>> > -};
>> > -
>> > static void __init xilinx_zynq_timer_init(void)
>> > {
>> > struct device_node *np;
>> > @@ -82,13 +74,31 @@ static void __init xilinx_zynq_timer_init(void)
>> > xttcps_timer_init();
>> > }
>> >
>> > +static struct map_desc zynq_cortex_a9_scu_map __initdata = {
>> > + .length = SZ_8K,
>>
>> That's bogus. Size 8k is too big because it also cover gic which is wrong.
>> As you can see from xilinx git repo correct size is 256 or less.
>>
>
> Hm,... you are obviously correct. 256 it is.
>
>> > + .type = MT_DEVICE,
>> > +};
>> > +
>> > +void __init zynq_scu_map_io(void)
>>
>> Should be static.
>>
>
> Yes.
>
>> > +{
>> > + if (scu_a9_has_base()) {
>>
>> I am not calling this function because I think it is not necessary to do so
>> because it is run only on a9 where scu_a9_get_base will just work.
>> Of did I miss anything?
>>
>
> As long as this file is only used for zynqs with A9 this call is not needed.
> Right.
>
>>
>> > + unsigned long base;
>> > +
>> > + base = scu_a9_get_base();
>> > + zynq_cortex_a9_scu_map.pfn = __phys_to_pfn(base);
>> > + zynq_cortex_a9_scu_map.virtual = base;
>> > + iotable_init(&zynq_cortex_a9_scu_map, 1);
>> > + scu_base = ioremap(base, zynq_cortex_a9_scu_map.length);
>> > + }
>> > +}
>> > +
>> > /**
>> > * xilinx_map_io() - Create memory mappings needed for early I/O.
>> > */
>> > static void __init xilinx_map_io(void)
>> > {
>> > debug_ll_io_init();
>> > - iotable_init(&scu_desc, 1);
>> > + zynq_scu_map_io();
>> > }
>>
>> You are using a little bit different names than we have in xilinx git tree
>> but maybe worth to call it as you.
>>
>
> I propose using the common mainline way of calling this functions.
> When there maybe will be more xilinx SoCs in mainline it will be easier to find
> what one is looking for.
yes. Let's wait for finishing this discussion and I also like your names.
I will change my patches to it and I will also add you as the author.
> I actually wanted to make an RFC patch naming everything to zynq_* instead of
> xilinx_* and replace the "MACHINE_START(XILINX_EP107,...".
> Didn't get around to is though.
I haven't started with it but yes, I would like to do this change too.
IRC: The original post was done by John Linn and it has to go through
Russel I think.
EP107 was emulating platform and none is using it right now.
Also I hope we have removed all PSS prefix from all files.
It was caused because we didn't know what will be commercial name for
this product.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-25 14:59 ` Michal Simek
@ 2013-03-26 11:08 ` Steffen Trumtrar
2013-03-26 12:35 ` Michal Simek
0 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-26 11:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi Michal,
On Mon, Mar 25, 2013 at 03:59:14PM +0100, Michal Simek wrote:
> Hi Steffen,
>
> 2013/3/25 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> > On Mon, Mar 25, 2013 at 03:01:59PM +0100, Michal Simek wrote:
> >> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> >> > Instead of hardcoding the base address of the SCU get it from the device.
> >> > While at it, add the SCU to the DT.
> >> >
> >> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> >> > Cc: Michal Simek <michal.simek@xilinx.com>
> >> > Cc: Josh Cartwright <josh.cartwright@ni.com>
> >> > ---
> >> > arch/arm/boot/dts/zynq-7000.dtsi | 5 +++++
> >> > arch/arm/mach-zynq/common.c | 34 ++++++++++++++++++++++------------
> >> > 2 files changed, 27 insertions(+), 12 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> >> > index 88564fa..c103082 100644
> >> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
> >> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> >> > @@ -199,6 +199,11 @@
> >> > };
> >> > };
> >> >
> >> > + scu: scu at f8f000000 {
> >> > + compatible = "arm,cortex-a9-scu";
> >> > + reg = <0xf8f00000 0x58>;
> >> > + };
> >> > +
> >> > timer: timer at f8f00600 {
> >> > compatible = "arm,cortex-a9-twd-timer";
> >> > reg = <0xf8f00600 0x20>;
> >> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> >> > index 920e20a..014131c 100644
> >> > --- a/arch/arm/mach-zynq/common.c
> >> > +++ b/arch/arm/mach-zynq/common.c
> >> > @@ -32,11 +32,14 @@
> >> > #include <asm/mach-types.h>
> >> > #include <asm/page.h>
> >> > #include <asm/pgtable.h>
> >> > +#include <asm/smp_scu.h>
> >> > #include <asm/smp_twd.h>
> >> > #include <asm/hardware/cache-l2x0.h>
> >> >
> >> > #include "common.h"
> >> >
> >> > +void __iomem *scu_base;
> >>
> >> This must be defined in header - will produce sparse warning.
> >>
> >
> > Okay. No problem. I can change that.
> >
> >> > +
> >> > static struct of_device_id zynq_of_bus_ids[] __initdata = {
> >> > { .compatible = "simple-bus", },
> >> > {}
> >> > @@ -56,17 +59,6 @@ static void __init xilinx_init_machine(void)
> >> > of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
> >> > }
> >> >
> >> > -#define SCU_PERIPH_PHYS 0xF8F00000
> >> > -#define SCU_PERIPH_SIZE SZ_8K
> >> > -#define SCU_PERIPH_VIRT (VMALLOC_END - SCU_PERIPH_SIZE)
> >> > -
> >> > -static struct map_desc scu_desc __initdata = {
> >> > - .virtual = SCU_PERIPH_VIRT,
> >> > - .pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
> >> > - .length = SCU_PERIPH_SIZE,
> >> > - .type = MT_DEVICE,
> >> > -};
> >> > -
> >> > static void __init xilinx_zynq_timer_init(void)
> >> > {
> >> > struct device_node *np;
> >> > @@ -82,13 +74,31 @@ static void __init xilinx_zynq_timer_init(void)
> >> > xttcps_timer_init();
> >> > }
> >> >
> >> > +static struct map_desc zynq_cortex_a9_scu_map __initdata = {
> >> > + .length = SZ_8K,
> >>
> >> That's bogus. Size 8k is too big because it also cover gic which is wrong.
> >> As you can see from xilinx git repo correct size is 256 or less.
> >>
> >
> > Hm,... you are obviously correct. 256 it is.
> >
> >> > + .type = MT_DEVICE,
> >> > +};
> >> > +
> >> > +void __init zynq_scu_map_io(void)
> >>
> >> Should be static.
> >>
> >
> > Yes.
> >
> >> > +{
> >> > + if (scu_a9_has_base()) {
> >>
> >> I am not calling this function because I think it is not necessary to do so
> >> because it is run only on a9 where scu_a9_get_base will just work.
> >> Of did I miss anything?
> >>
> >
> > As long as this file is only used for zynqs with A9 this call is not needed.
> > Right.
> >
> >>
> >> > + unsigned long base;
> >> > +
> >> > + base = scu_a9_get_base();
> >> > + zynq_cortex_a9_scu_map.pfn = __phys_to_pfn(base);
> >> > + zynq_cortex_a9_scu_map.virtual = base;
> >> > + iotable_init(&zynq_cortex_a9_scu_map, 1);
> >> > + scu_base = ioremap(base, zynq_cortex_a9_scu_map.length);
> >> > + }
> >> > +}
> >> > +
> >> > /**
> >> > * xilinx_map_io() - Create memory mappings needed for early I/O.
> >> > */
> >> > static void __init xilinx_map_io(void)
> >> > {
> >> > debug_ll_io_init();
> >> > - iotable_init(&scu_desc, 1);
> >> > + zynq_scu_map_io();
> >> > }
> >>
> >> You are using a little bit different names than we have in xilinx git tree
> >> but maybe worth to call it as you.
> >>
> >
> > I propose using the common mainline way of calling this functions.
> > When there maybe will be more xilinx SoCs in mainline it will be easier to find
> > what one is looking for.
>
> yes. Let's wait for finishing this discussion and I also like your names.
> I will change my patches to it and I will also add you as the author.
>
> > I actually wanted to make an RFC patch naming everything to zynq_* instead of
> > xilinx_* and replace the "MACHINE_START(XILINX_EP107,...".
> > Didn't get around to is though.
>
> I haven't started with it but yes, I would like to do this change too.
> IRC: The original post was done by John Linn and it has to go through
> Russel I think.
> EP107 was emulating platform and none is using it right now.
> Also I hope we have removed all PSS prefix from all files.
> It was caused because we didn't know what will be commercial name for
> this product.
>
Okay, I'm confused now. What/Where is the official for-next branch for
Zynq? MAINTAINERS says arch/arm/mach-zynq goes via
git://git.xilinx.com/linux-xlnx.git
But the arm-next branch in there is behind mainline.
Are the patches collected in arm-soc/for-next instead?
If I'm doing this patch I need to have the correct base for it.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-26 11:08 ` Steffen Trumtrar
@ 2013-03-26 12:35 ` Michal Simek
2013-03-26 12:46 ` Steffen Trumtrar
0 siblings, 1 reply; 18+ messages in thread
From: Michal Simek @ 2013-03-26 12:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi Steffen
2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> Hi Michal,
>
> On Mon, Mar 25, 2013 at 03:59:14PM +0100, Michal Simek wrote:
>> Hi Steffen,
>>
>> 2013/3/25 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> > On Mon, Mar 25, 2013 at 03:01:59PM +0100, Michal Simek wrote:
>> >> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> >> > Instead of hardcoding the base address of the SCU get it from the device.
>> >> > While at it, add the SCU to the DT.
>> >> >
>> >> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> >> > Cc: Michal Simek <michal.simek@xilinx.com>
>> >> > Cc: Josh Cartwright <josh.cartwright@ni.com>
>> >> > ---
>> >> > arch/arm/boot/dts/zynq-7000.dtsi | 5 +++++
>> >> > arch/arm/mach-zynq/common.c | 34 ++++++++++++++++++++++------------
>> >> > 2 files changed, 27 insertions(+), 12 deletions(-)
>> >> >
>> >> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> >> > index 88564fa..c103082 100644
>> >> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> >> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> >> > @@ -199,6 +199,11 @@
>> >> > };
>> >> > };
>> >> >
>> >> > + scu: scu at f8f000000 {
>> >> > + compatible = "arm,cortex-a9-scu";
>> >> > + reg = <0xf8f00000 0x58>;
>> >> > + };
>> >> > +
>> >> > timer: timer at f8f00600 {
>> >> > compatible = "arm,cortex-a9-twd-timer";
>> >> > reg = <0xf8f00600 0x20>;
>> >> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
>> >> > index 920e20a..014131c 100644
>> >> > --- a/arch/arm/mach-zynq/common.c
>> >> > +++ b/arch/arm/mach-zynq/common.c
>> >> > @@ -32,11 +32,14 @@
>> >> > #include <asm/mach-types.h>
>> >> > #include <asm/page.h>
>> >> > #include <asm/pgtable.h>
>> >> > +#include <asm/smp_scu.h>
>> >> > #include <asm/smp_twd.h>
>> >> > #include <asm/hardware/cache-l2x0.h>
>> >> >
>> >> > #include "common.h"
>> >> >
>> >> > +void __iomem *scu_base;
>> >>
>> >> This must be defined in header - will produce sparse warning.
>> >>
>> >
>> > Okay. No problem. I can change that.
>> >
>> >> > +
>> >> > static struct of_device_id zynq_of_bus_ids[] __initdata = {
>> >> > { .compatible = "simple-bus", },
>> >> > {}
>> >> > @@ -56,17 +59,6 @@ static void __init xilinx_init_machine(void)
>> >> > of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
>> >> > }
>> >> >
>> >> > -#define SCU_PERIPH_PHYS 0xF8F00000
>> >> > -#define SCU_PERIPH_SIZE SZ_8K
>> >> > -#define SCU_PERIPH_VIRT (VMALLOC_END - SCU_PERIPH_SIZE)
>> >> > -
>> >> > -static struct map_desc scu_desc __initdata = {
>> >> > - .virtual = SCU_PERIPH_VIRT,
>> >> > - .pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
>> >> > - .length = SCU_PERIPH_SIZE,
>> >> > - .type = MT_DEVICE,
>> >> > -};
>> >> > -
>> >> > static void __init xilinx_zynq_timer_init(void)
>> >> > {
>> >> > struct device_node *np;
>> >> > @@ -82,13 +74,31 @@ static void __init xilinx_zynq_timer_init(void)
>> >> > xttcps_timer_init();
>> >> > }
>> >> >
>> >> > +static struct map_desc zynq_cortex_a9_scu_map __initdata = {
>> >> > + .length = SZ_8K,
>> >>
>> >> That's bogus. Size 8k is too big because it also cover gic which is wrong.
>> >> As you can see from xilinx git repo correct size is 256 or less.
>> >>
>> >
>> > Hm,... you are obviously correct. 256 it is.
>> >
>> >> > + .type = MT_DEVICE,
>> >> > +};
>> >> > +
>> >> > +void __init zynq_scu_map_io(void)
>> >>
>> >> Should be static.
>> >>
>> >
>> > Yes.
>> >
>> >> > +{
>> >> > + if (scu_a9_has_base()) {
>> >>
>> >> I am not calling this function because I think it is not necessary to do so
>> >> because it is run only on a9 where scu_a9_get_base will just work.
>> >> Of did I miss anything?
>> >>
>> >
>> > As long as this file is only used for zynqs with A9 this call is not needed.
>> > Right.
>> >
>> >>
>> >> > + unsigned long base;
>> >> > +
>> >> > + base = scu_a9_get_base();
>> >> > + zynq_cortex_a9_scu_map.pfn = __phys_to_pfn(base);
>> >> > + zynq_cortex_a9_scu_map.virtual = base;
>> >> > + iotable_init(&zynq_cortex_a9_scu_map, 1);
>> >> > + scu_base = ioremap(base, zynq_cortex_a9_scu_map.length);
>> >> > + }
>> >> > +}
>> >> > +
>> >> > /**
>> >> > * xilinx_map_io() - Create memory mappings needed for early I/O.
>> >> > */
>> >> > static void __init xilinx_map_io(void)
>> >> > {
>> >> > debug_ll_io_init();
>> >> > - iotable_init(&scu_desc, 1);
>> >> > + zynq_scu_map_io();
>> >> > }
>> >>
>> >> You are using a little bit different names than we have in xilinx git tree
>> >> but maybe worth to call it as you.
>> >>
>> >
>> > I propose using the common mainline way of calling this functions.
>> > When there maybe will be more xilinx SoCs in mainline it will be easier to find
>> > what one is looking for.
>>
>> yes. Let's wait for finishing this discussion and I also like your names.
>> I will change my patches to it and I will also add you as the author.
>>
>> > I actually wanted to make an RFC patch naming everything to zynq_* instead of
>> > xilinx_* and replace the "MACHINE_START(XILINX_EP107,...".
>> > Didn't get around to is though.
>>
>> I haven't started with it but yes, I would like to do this change too.
>> IRC: The original post was done by John Linn and it has to go through
>> Russel I think.
>> EP107 was emulating platform and none is using it right now.
>> Also I hope we have removed all PSS prefix from all files.
>> It was caused because we didn't know what will be commercial name for
>> this product.
>>
>
> Okay, I'm confused now. What/Where is the official for-next branch for
> Zynq? MAINTAINERS says arch/arm/mach-zynq goes via
> git://git.xilinx.com/linux-xlnx.git
> But the arm-next branch in there is behind mainline.
That's correct. All reviewed patches go to this branch.
Currently there are just old patches which were already merged via
arm-soc tree to mainline to v3.9
because we don't have new patches. I will add there all patches which
we are discussing right now.
Also this branch is merged to linux-next.
> Are the patches collected in arm-soc/for-next instead?
No. I am asking Arnd and Olof for merging that branch above.
> If I'm doing this patch I need to have the correct base for it.
Give me some time. I am incorporating all changes from your series and
other comments to v2 which I will send.
I will also add your Signed-off-by lines to that patches.
I hope that you are ok with it. Let me know if not.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-26 12:35 ` Michal Simek
@ 2013-03-26 12:46 ` Steffen Trumtrar
2013-03-26 12:53 ` Michal Simek
0 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-26 12:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Michal!
On Tue, Mar 26, 2013 at 01:35:31PM +0100, Michal Simek wrote:
> >> Hi Steffen,
> >> I haven't started with it but yes, I would like to do this change too.
> >> IRC: The original post was done by John Linn and it has to go through
> >> Russel I think.
> >> EP107 was emulating platform and none is using it right now.
> >> Also I hope we have removed all PSS prefix from all files.
> >> It was caused because we didn't know what will be commercial name for
> >> this product.
> >>
> >
> > Okay, I'm confused now. What/Where is the official for-next branch for
> > Zynq? MAINTAINERS says arch/arm/mach-zynq goes via
> > git://git.xilinx.com/linux-xlnx.git
> > But the arm-next branch in there is behind mainline.
>
> That's correct. All reviewed patches go to this branch.
> Currently there are just old patches which were already merged via
> arm-soc tree to mainline to v3.9
> because we don't have new patches. I will add there all patches which
> we are discussing right now.
> Also this branch is merged to linux-next.
>
> > Are the patches collected in arm-soc/for-next instead?
>
> No. I am asking Arnd and Olof for merging that branch above.
>
> > If I'm doing this patch I need to have the correct base for it.
>
> Give me some time. I am incorporating all changes from your series and
> other comments to v2 which I will send.
>
Very good. That is what I was expecting, but as you mentioned Russel,
I got confused, as he doesn't care for such stuff.
> I will also add your Signed-off-by lines to that patches.
> I hope that you are ok with it. Let me know if not.
>
I'm definitely okay with that.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-26 12:46 ` Steffen Trumtrar
@ 2013-03-26 12:53 ` Michal Simek
2013-03-26 13:01 ` Steffen Trumtrar
0 siblings, 1 reply; 18+ messages in thread
From: Michal Simek @ 2013-03-26 12:53 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> Hi Michal!
>
> On Tue, Mar 26, 2013 at 01:35:31PM +0100, Michal Simek wrote:
>> >> Hi Steffen,
>> >> I haven't started with it but yes, I would like to do this change too.
>> >> IRC: The original post was done by John Linn and it has to go through
>> >> Russel I think.
>> >> EP107 was emulating platform and none is using it right now.
>> >> Also I hope we have removed all PSS prefix from all files.
>> >> It was caused because we didn't know what will be commercial name for
>> >> this product.
>> >>
>> >
>> > Okay, I'm confused now. What/Where is the official for-next branch for
>> > Zynq? MAINTAINERS says arch/arm/mach-zynq goes via
>> > git://git.xilinx.com/linux-xlnx.git
>> > But the arm-next branch in there is behind mainline.
>>
>> That's correct. All reviewed patches go to this branch.
>> Currently there are just old patches which were already merged via
>> arm-soc tree to mainline to v3.9
>> because we don't have new patches. I will add there all patches which
>> we are discussing right now.
>> Also this branch is merged to linux-next.
>>
>> > Are the patches collected in arm-soc/for-next instead?
>>
>> No. I am asking Arnd and Olof for merging that branch above.
>>
>> > If I'm doing this patch I need to have the correct base for it.
>>
>> Give me some time. I am incorporating all changes from your series and
>> other comments to v2 which I will send.
>>
>
> Very good. That is what I was expecting, but as you mentioned Russel,
> I got confused, as he doesn't care for such stuff.
I mentioned Russel in connect to change machine name in
arch/arm/tools/mach-types.
Not about the handling zynq patches and provide the path to mainline.
>
>> I will also add your Signed-off-by lines to that patches.
>> I hope that you are ok with it. Let me know if not.
>>
>
> I'm definitely okay with that.
ok.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] ARM: zynq: read scu base from SoC
2013-03-26 12:53 ` Michal Simek
@ 2013-03-26 13:01 ` Steffen Trumtrar
0 siblings, 0 replies; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-26 13:01 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 26, 2013 at 01:53:06PM +0100, Michal Simek wrote:
> 2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> > Hi Michal!
> >
> > On Tue, Mar 26, 2013 at 01:35:31PM +0100, Michal Simek wrote:
> >> >> Hi Steffen,
> >> >> I haven't started with it but yes, I would like to do this change too.
> >> >> IRC: The original post was done by John Linn and it has to go through
> >> >> Russel I think.
> >> >> EP107 was emulating platform and none is using it right now.
> >> >> Also I hope we have removed all PSS prefix from all files.
> >> >> It was caused because we didn't know what will be commercial name for
> >> >> this product.
> >> >>
> >> >
> >> > Okay, I'm confused now. What/Where is the official for-next branch for
> >> > Zynq? MAINTAINERS says arch/arm/mach-zynq goes via
> >> > git://git.xilinx.com/linux-xlnx.git
> >> > But the arm-next branch in there is behind mainline.
> >>
> >> That's correct. All reviewed patches go to this branch.
> >> Currently there are just old patches which were already merged via
> >> arm-soc tree to mainline to v3.9
> >> because we don't have new patches. I will add there all patches which
> >> we are discussing right now.
> >> Also this branch is merged to linux-next.
> >>
> >> > Are the patches collected in arm-soc/for-next instead?
> >>
> >> No. I am asking Arnd and Olof for merging that branch above.
> >>
> >> > If I'm doing this patch I need to have the correct base for it.
> >>
> >> Give me some time. I am incorporating all changes from your series and
> >> other comments to v2 which I will send.
> >>
> >
> > Very good. That is what I was expecting, but as you mentioned Russel,
> > I got confused, as he doesn't care for such stuff.
>
> I mentioned Russel in connect to change machine name in
> arch/arm/tools/mach-types.
> Not about the handling zynq patches and provide the path to mainline.
>
Ah, okay. Yes. That is right. Now we are on common ground. You should be
right there.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] ARM: zynq: get slcr base earlier
2013-03-23 12:57 [PATCH 0/3] ARM: zynq: add SMP support for zynq7000 Steffen Trumtrar
2013-03-23 12:57 ` [PATCH 1/3] ARM: zynq: read scu base from SoC Steffen Trumtrar
@ 2013-03-23 12:57 ` Steffen Trumtrar
2013-03-25 14:04 ` Michal Simek
2013-03-23 12:57 ` [PATCH 3/3] ARM: zynq: add SMP support Steffen Trumtrar
2 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-23 12:57 UTC (permalink / raw)
To: linux-arm-kernel
The slcr is needed for pinctrl, clocks and reset. Therefore we want it as early
as possible. As there is no driver that handles it and instead a pointer needs
to be passed around, rename the variable to something a little more obvious.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Cc: Michal Simek <michal.simek@xilinx.com>
---
arch/arm/mach-zynq/common.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 014131c..1b9bb3d 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -38,6 +38,7 @@
#include "common.h"
+void __iomem *slcr_base_addr;
void __iomem *scu_base;
static struct of_device_id zynq_of_bus_ids[] __initdata = {
@@ -61,19 +62,21 @@ static void __init xilinx_init_machine(void)
static void __init xilinx_zynq_timer_init(void)
{
- struct device_node *np;
- void __iomem *slcr;
-
- np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
- slcr = of_iomap(np, 0);
- WARN_ON(!slcr);
-
- xilinx_zynq_clocks_init(slcr);
+ xilinx_zynq_clocks_init(slcr_base_addr);
twd_local_timer_of_register();
xttcps_timer_init();
}
+static void zynq_slcr_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
+ slcr_base_addr = of_iomap(np, 0);
+ WARN_ON(!slcr_base_addr);
+}
+
static struct map_desc zynq_cortex_a9_scu_map __initdata = {
.length = SZ_8K,
.type = MT_DEVICE,
@@ -101,6 +104,12 @@ static void __init xilinx_map_io(void)
zynq_scu_map_io();
}
+static void __init xilinx_irqchip_init(void)
+{
+ irqchip_init();
+ zynq_slcr_init();
+}
+
static const char *xilinx_dt_match[] = {
"xlnx,zynq-zc702",
"xlnx,zynq-7000",
@@ -109,7 +118,7 @@ static const char *xilinx_dt_match[] = {
MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
.map_io = xilinx_map_io,
- .init_irq = irqchip_init,
+ .init_irq = xilinx_irqchip_init,
.init_machine = xilinx_init_machine,
.init_time = xilinx_zynq_timer_init,
.dt_compat = xilinx_dt_match,
--
1.8.2.rc2
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/3] ARM: zynq: get slcr base earlier
2013-03-23 12:57 ` [PATCH 2/3] ARM: zynq: get slcr base earlier Steffen Trumtrar
@ 2013-03-25 14:04 ` Michal Simek
2013-03-25 14:39 ` Steffen Trumtrar
0 siblings, 1 reply; 18+ messages in thread
From: Michal Simek @ 2013-03-25 14:04 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> The slcr is needed for pinctrl, clocks and reset. Therefore we want it as early
> as possible. As there is no driver that handles it and instead a pointer needs
> to be passed around, rename the variable to something a little more obvious.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> ---
> arch/arm/mach-zynq/common.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index 014131c..1b9bb3d 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -38,6 +38,7 @@
>
> #include "common.h"
>
> +void __iomem *slcr_base_addr;
> void __iomem *scu_base;
>
> static struct of_device_id zynq_of_bus_ids[] __initdata = {
> @@ -61,19 +62,21 @@ static void __init xilinx_init_machine(void)
>
> static void __init xilinx_zynq_timer_init(void)
> {
> - struct device_node *np;
> - void __iomem *slcr;
> -
> - np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
> - slcr = of_iomap(np, 0);
> - WARN_ON(!slcr);
> -
> - xilinx_zynq_clocks_init(slcr);
> + xilinx_zynq_clocks_init(slcr_base_addr);
>
> twd_local_timer_of_register();
> xttcps_timer_init();
> }
>
> +static void zynq_slcr_init(void)
> +{
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
> + slcr_base_addr = of_iomap(np, 0);
> + WARN_ON(!slcr_base_addr);
> +}
Xilinx is using separate driver for slcr and IMHO make sense to have it
like that because this IP can handle more things which will be just messy
to have it in one file.
What do you think?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 2/3] ARM: zynq: get slcr base earlier
2013-03-25 14:04 ` Michal Simek
@ 2013-03-25 14:39 ` Steffen Trumtrar
2013-03-25 14:55 ` Michal Simek
0 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-25 14:39 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 03:04:36PM +0100, Michal Simek wrote:
> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> > The slcr is needed for pinctrl, clocks and reset. Therefore we want it as early
> > as possible. As there is no driver that handles it and instead a pointer needs
> > to be passed around, rename the variable to something a little more obvious.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > Cc: Michal Simek <michal.simek@xilinx.com>
> > ---
> > arch/arm/mach-zynq/common.c | 27 ++++++++++++++++++---------
> > 1 file changed, 18 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> > index 014131c..1b9bb3d 100644
> > --- a/arch/arm/mach-zynq/common.c
> > +++ b/arch/arm/mach-zynq/common.c
> > @@ -38,6 +38,7 @@
> >
> > #include "common.h"
> >
> > +void __iomem *slcr_base_addr;
> > void __iomem *scu_base;
> >
> > static struct of_device_id zynq_of_bus_ids[] __initdata = {
> > @@ -61,19 +62,21 @@ static void __init xilinx_init_machine(void)
> >
> > static void __init xilinx_zynq_timer_init(void)
> > {
> > - struct device_node *np;
> > - void __iomem *slcr;
> > -
> > - np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
> > - slcr = of_iomap(np, 0);
> > - WARN_ON(!slcr);
> > -
> > - xilinx_zynq_clocks_init(slcr);
> > + xilinx_zynq_clocks_init(slcr_base_addr);
> >
> > twd_local_timer_of_register();
> > xttcps_timer_init();
> > }
> >
> > +static void zynq_slcr_init(void)
> > +{
> > + struct device_node *np;
> > +
> > + np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
> > + slcr_base_addr = of_iomap(np, 0);
> > + WARN_ON(!slcr_base_addr);
> > +}
>
> Xilinx is using separate driver for slcr and IMHO make sense to have it
> like that because this IP can handle more things which will be just messy
> to have it in one file.
> What do you think?
>
Definitely. I think we should have a main slcr driver for locking/unlocking
etc. and the clock/reset/mio-drivers should be "clients" of this.
Then for example the clockdriver would request a write to a register.
The slcr can then unlock and make the write.
But maybe this is overengineering. I haven't found time to look at the
xilinx driver. And I'm actually not sure why I would want to lock the
slcr. But as Xilinx opted for this feature, it should be handeled correctly.
At the moment I was trying to make do with what is there.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 2/3] ARM: zynq: get slcr base earlier
2013-03-25 14:39 ` Steffen Trumtrar
@ 2013-03-25 14:55 ` Michal Simek
0 siblings, 0 replies; 18+ messages in thread
From: Michal Simek @ 2013-03-25 14:55 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/25 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> On Mon, Mar 25, 2013 at 03:04:36PM +0100, Michal Simek wrote:
>> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> > The slcr is needed for pinctrl, clocks and reset. Therefore we want it as early
>> > as possible. As there is no driver that handles it and instead a pointer needs
>> > to be passed around, rename the variable to something a little more obvious.
>> >
>> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> > Cc: Michal Simek <michal.simek@xilinx.com>
>> > ---
>> > arch/arm/mach-zynq/common.c | 27 ++++++++++++++++++---------
>> > 1 file changed, 18 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
>> > index 014131c..1b9bb3d 100644
>> > --- a/arch/arm/mach-zynq/common.c
>> > +++ b/arch/arm/mach-zynq/common.c
>> > @@ -38,6 +38,7 @@
>> >
>> > #include "common.h"
>> >
>> > +void __iomem *slcr_base_addr;
>> > void __iomem *scu_base;
>> >
>> > static struct of_device_id zynq_of_bus_ids[] __initdata = {
>> > @@ -61,19 +62,21 @@ static void __init xilinx_init_machine(void)
>> >
>> > static void __init xilinx_zynq_timer_init(void)
>> > {
>> > - struct device_node *np;
>> > - void __iomem *slcr;
>> > -
>> > - np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
>> > - slcr = of_iomap(np, 0);
>> > - WARN_ON(!slcr);
>> > -
>> > - xilinx_zynq_clocks_init(slcr);
>> > + xilinx_zynq_clocks_init(slcr_base_addr);
>> >
>> > twd_local_timer_of_register();
>> > xttcps_timer_init();
>> > }
>> >
>> > +static void zynq_slcr_init(void)
>> > +{
>> > + struct device_node *np;
>> > +
>> > + np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
>> > + slcr_base_addr = of_iomap(np, 0);
>> > + WARN_ON(!slcr_base_addr);
>> > +}
>>
>> Xilinx is using separate driver for slcr and IMHO make sense to have it
>> like that because this IP can handle more things which will be just messy
>> to have it in one file.
>> What do you think?
>>
>
> Definitely. I think we should have a main slcr driver for locking/unlocking
> etc. and the clock/reset/mio-drivers should be "clients" of this.
> Then for example the clockdriver would request a write to a register.
> The slcr can then unlock and make the write.
> But maybe this is overengineering. I haven't found time to look at the
> xilinx driver. And I'm actually not sure why I would want to lock the
> slcr. But as Xilinx opted for this feature, it should be handeled correctly.
> At the moment I was trying to make do with what is there.
I was thinking about locking and unlocking slcr for normal linux runs
and I haven't found a reason why to locking/unlocking them.
That's why we just unlock them and use them.
But for some application locking could make sense because slcr
is too powerful.
We might find out the proper way how to handle it in future.
Currently I am ok to keep it unlock or unlock it in every slcr function.
lock();
do_something()
unlock();
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM: zynq: add SMP support
2013-03-23 12:57 [PATCH 0/3] ARM: zynq: add SMP support for zynq7000 Steffen Trumtrar
2013-03-23 12:57 ` [PATCH 1/3] ARM: zynq: read scu base from SoC Steffen Trumtrar
2013-03-23 12:57 ` [PATCH 2/3] ARM: zynq: get slcr base earlier Steffen Trumtrar
@ 2013-03-23 12:57 ` Steffen Trumtrar
2013-03-25 14:27 ` Michal Simek
2 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-23 12:57 UTC (permalink / raw)
To: linux-arm-kernel
The Zynq7000 has a CortexA9 with at least 2 cores. Add support to boot them all.
To do this a trampoline is needed. At runtime the jump address and two
instructions are copied to RAM by CPU0 and then executed by CPU1.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Cc: Michal Simek <michal.simek@xilinx.com>
---
arch/arm/boot/dts/zynq-7000.dtsi | 14 +++++
arch/arm/mach-zynq/Makefile | 2 +
arch/arm/mach-zynq/common.c | 1 +
arch/arm/mach-zynq/common.h | 14 +++++
arch/arm/mach-zynq/headsmp.S | 20 +++++++
arch/arm/mach-zynq/platsmp.c | 110 +++++++++++++++++++++++++++++++++++++++
6 files changed, 161 insertions(+)
create mode 100644 arch/arm/mach-zynq/headsmp.S
create mode 100644 arch/arm/mach-zynq/platsmp.c
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index c103082..258b6c9 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -15,6 +15,20 @@
/ {
compatible = "xlnx,zynq-7000";
+ cpus {
+ cpu at 0 {
+ compatible = "arm,cortex-a9";
+ next-level-cache = <&L2>;
+ clocks = <&armpll 0>;
+ };
+
+ cpu at 1 {
+ compatible = "arm,cortex-a9";
+ next-level-cache = <&L2>;
+ clocks = <&armpll 0>;
+ };
+ };
+
amba {
compatible = "simple-bus";
#address-cells = <1>;
diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
index 397268c..906d199 100644
--- a/arch/arm/mach-zynq/Makefile
+++ b/arch/arm/mach-zynq/Makefile
@@ -4,3 +4,5 @@
# Common support
obj-y := common.o timer.o
+AFLAGS_headsmp.o :=-Wa,-march=armv7-a
+obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 1b9bb3d..4053962 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -117,6 +117,7 @@ static const char *xilinx_dt_match[] = {
};
MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
+ .smp = smp_ops(zynq_smp_ops),
.map_io = xilinx_map_io,
.init_irq = xilinx_irqchip_init,
.init_machine = xilinx_init_machine,
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index 8b4dbba..451d386 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -19,4 +19,18 @@
void __init xttcps_timer_init(void);
+#ifdef CONFIG_SMP
+extern void secondary_startup(void);
+extern char secondary_trampoline, secondary_trampoline_end;
+#endif
+
+extern struct smp_operations __initdata zynq_smp_ops;
+extern void __iomem *slcr_base_addr;
+extern void __iomem *scu_base;
+
+#define SLCR_UNLOCK_MAGIC 0xdf0d
+
+#define SLCR_UNLOCK 0x8
+#define SLCR_A9_CPU_RST_CTRL 0x244
+
#endif
diff --git a/arch/arm/mach-zynq/headsmp.S b/arch/arm/mach-zynq/headsmp.S
new file mode 100644
index 0000000..145bbae
--- /dev/null
+++ b/arch/arm/mach-zynq/headsmp.S
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * based on mach-socfpga/headsmp.S
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+ __CPUINIT
+ .arch armv7-a
+
+ENTRY(secondary_trampoline)
+ ldr r0, [pc]
+ mov pc, r0
+
+ENTRY(secondary_trampoline_end)
diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
new file mode 100644
index 0000000..1ea3d4f
--- /dev/null
+++ b/arch/arm/mach-zynq/platsmp.c
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * based on
+ * linux/arch/arm/mach-zynq/platsmp.c Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/irqchip/arm-gic.h>
+#include <linux/io.h>
+#include <linux/jiffies.h>
+#include <linux/smp.h>
+
+#include <asm/cacheflush.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/map.h>
+#include <asm/smp_scu.h>
+
+#include "common.h"
+
+#define A9_RST_MASK (1 << 0)
+#define A9_CLKSTOP_MASK (1 << 4)
+
+static DEFINE_SPINLOCK(boot_lock);
+
+static void __cpuinit zynq_smp_secondary_init(unsigned int cpu)
+{
+ /*
+ * if any interrupts are already enabled for the primary
+ * core (e.g. timer irq), then they will not have been enabled
+ * for us: do so
+ */
+ gic_secondary_init(0);
+
+ /*
+ * Synchronise with the boot thread.
+ */
+ spin_lock(&boot_lock);
+ spin_unlock(&boot_lock);
+}
+
+static inline void zynq_set_cpu_jump(int cpu, void *jump_addr)
+{
+ if (jump_addr) {
+ int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
+
+ writel(virt_to_phys(jump_addr), phys_to_virt(8));
+ memcpy(phys_to_virt(0), &secondary_trampoline, trampoline_size);
+
+ flush_cache_all();
+ outer_flush_all();
+ smp_wmb();
+ }
+}
+
+static void zynq_enable_cpu(int cpu, bool enable)
+{
+ writel(A9_CLKSTOP_MASK << cpu, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
+ writel(0, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
+}
+
+int __cpuinit zynq_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ writel(SLCR_UNLOCK_MAGIC, slcr_base_addr + SLCR_UNLOCK);
+ writel((A9_CLKSTOP_MASK | A9_RST_MASK) << cpu,
+ slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
+
+ zynq_set_cpu_jump(cpu, secondary_startup);
+ zynq_enable_cpu(cpu, true);
+
+ return 0;
+}
+
+/*
+ * Initialise the CPU possible map early - this describes the CPUs
+ * which may be present or become present in the system.
+ */
+static void __init zynq_smp_init_cpus(void)
+{
+ unsigned int ncores, i;
+
+ ncores = scu_get_core_count(scu_base);
+
+ for (i = 0; i < ncores; ++i)
+ set_cpu_possible(i, true);
+
+ /* sanity check */
+ if (ncores > num_possible_cpus()) {
+ pr_warn("zynq: no. of cores (%d) greater than configured"
+ "maximum of %d - clipping\n", ncores, num_possible_cpus());
+ ncores = num_possible_cpus();
+ }
+}
+
+static void __init zynq_smp_prepare_cpus(unsigned int max_cpus)
+{
+ scu_enable(scu_base);
+}
+
+struct smp_operations __initdata zynq_smp_ops = {
+ .smp_init_cpus = zynq_smp_init_cpus,
+ .smp_prepare_cpus = zynq_smp_prepare_cpus,
+ .smp_secondary_init = zynq_smp_secondary_init,
+ .smp_boot_secondary = zynq_smp_boot_secondary,
+};
--
1.8.2.rc2
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 3/3] ARM: zynq: add SMP support
2013-03-23 12:57 ` [PATCH 3/3] ARM: zynq: add SMP support Steffen Trumtrar
@ 2013-03-25 14:27 ` Michal Simek
2013-03-25 16:34 ` Steffen Trumtrar
0 siblings, 1 reply; 18+ messages in thread
From: Michal Simek @ 2013-03-25 14:27 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> The Zynq7000 has a CortexA9 with at least 2 cores. Add support to boot them all.
> To do this a trampoline is needed. At runtime the jump address and two
> instructions are copied to RAM by CPU0 and then executed by CPU1.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 14 +++++
> arch/arm/mach-zynq/Makefile | 2 +
> arch/arm/mach-zynq/common.c | 1 +
> arch/arm/mach-zynq/common.h | 14 +++++
> arch/arm/mach-zynq/headsmp.S | 20 +++++++
> arch/arm/mach-zynq/platsmp.c | 110 +++++++++++++++++++++++++++++++++++++++
> 6 files changed, 161 insertions(+)
> create mode 100644 arch/arm/mach-zynq/headsmp.S
> create mode 100644 arch/arm/mach-zynq/platsmp.c
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index c103082..258b6c9 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -15,6 +15,20 @@
> / {
> compatible = "xlnx,zynq-7000";
>
> + cpus {
> + cpu at 0 {
> + compatible = "arm,cortex-a9";
> + next-level-cache = <&L2>;
> + clocks = <&armpll 0>;
> + };
> +
> + cpu at 1 {
> + compatible = "arm,cortex-a9";
> + next-level-cache = <&L2>;
> + clocks = <&armpll 0>;
> + };
> + };
Is this really needed?
> +
> amba {
> compatible = "simple-bus";
> #address-cells = <1>;
> diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
> index 397268c..906d199 100644
> --- a/arch/arm/mach-zynq/Makefile
> +++ b/arch/arm/mach-zynq/Makefile
> @@ -4,3 +4,5 @@
>
> # Common support
> obj-y := common.o timer.o
> +AFLAGS_headsmp.o :=-Wa,-march=armv7-a
> +obj-$(CONFIG_SMP) += headsmp.o platsmp.o
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index 1b9bb3d..4053962 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -117,6 +117,7 @@ static const char *xilinx_dt_match[] = {
> };
>
> MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
> + .smp = smp_ops(zynq_smp_ops),
> .map_io = xilinx_map_io,
> .init_irq = xilinx_irqchip_init,
> .init_machine = xilinx_init_machine,
> diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
> index 8b4dbba..451d386 100644
> --- a/arch/arm/mach-zynq/common.h
> +++ b/arch/arm/mach-zynq/common.h
> @@ -19,4 +19,18 @@
>
> void __init xttcps_timer_init(void);
>
> +#ifdef CONFIG_SMP
> +extern void secondary_startup(void);
> +extern char secondary_trampoline, secondary_trampoline_end;
> +#endif
> +
> +extern struct smp_operations __initdata zynq_smp_ops;
> +extern void __iomem *slcr_base_addr;
> +extern void __iomem *scu_base;
> +
> +#define SLCR_UNLOCK_MAGIC 0xdf0d
> +
> +#define SLCR_UNLOCK 0x8
> +#define SLCR_A9_CPU_RST_CTRL 0x244
As previous code.
> +
> #endif
> diff --git a/arch/arm/mach-zynq/headsmp.S b/arch/arm/mach-zynq/headsmp.S
> new file mode 100644
> index 0000000..145bbae
> --- /dev/null
> +++ b/arch/arm/mach-zynq/headsmp.S
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * based on mach-socfpga/headsmp.S
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/linkage.h>
> +#include <linux/init.h>
> +
> + __CPUINIT
> + .arch armv7-a
> +
> +ENTRY(secondary_trampoline)
> + ldr r0, [pc]
> + mov pc, r0
This code is familiar to me. It looks like my jump trampoline which I
wrote in C. :-)
> +
> +ENTRY(secondary_trampoline_end)
> diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
> new file mode 100644
> index 0000000..1ea3d4f
> --- /dev/null
> +++ b/arch/arm/mach-zynq/platsmp.c
> @@ -0,0 +1,110 @@
> +/*
> + * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * based on
> + * linux/arch/arm/mach-zynq/platsmp.c Copyright (C) 2011 Xilinx
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/irqchip/arm-gic.h>
> +#include <linux/io.h>
> +#include <linux/jiffies.h>
> +#include <linux/smp.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/map.h>
> +#include <asm/smp_scu.h>
> +
> +#include "common.h"
> +
> +#define A9_RST_MASK (1 << 0)
> +#define A9_CLKSTOP_MASK (1 << 4)
as I wrote above moving to separate driver make sense to me.
> +
> +static DEFINE_SPINLOCK(boot_lock);
> +
> +static void __cpuinit zynq_smp_secondary_init(unsigned int cpu)
> +{
> + /*
> + * if any interrupts are already enabled for the primary
> + * core (e.g. timer irq), then they will not have been enabled
> + * for us: do so
> + */
> + gic_secondary_init(0);
> +
> + /*
> + * Synchronise with the boot thread.
> + */
> + spin_lock(&boot_lock);
> + spin_unlock(&boot_lock);
> +}
> +
> +static inline void zynq_set_cpu_jump(int cpu, void *jump_addr)
> +{
> + if (jump_addr) {
This is wrong because if jump_addr is 0x1 - 0xF then you are rewriting.
It is unlikely but it should be captured.
The main reason is AMP solution where Linux can't be added from 0x0
and user amp code can setup any entry point. Better to check it just for sure.
> + int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
> +
> + writel(virt_to_phys(jump_addr), phys_to_virt(8));
This will probably generate sparse warning.
> + memcpy(phys_to_virt(0), &secondary_trampoline, trampoline_size);
This is nice solution. I am hardcoding instruction and this looks like
good solution.
Let me do some experiment with it because maybe this could be
problematic when Linux
kernel starts from different address than 0x0.
> +
> + flush_cache_all();
> + outer_flush_all();
> + smp_wmb();
> + }
> +}
> +
> +static void zynq_enable_cpu(int cpu, bool enable)
> +{
> + writel(A9_CLKSTOP_MASK << cpu, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
> + writel(0, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
> +}
> +
> +int __cpuinit zynq_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + writel(SLCR_UNLOCK_MAGIC, slcr_base_addr + SLCR_UNLOCK);
> + writel((A9_CLKSTOP_MASK | A9_RST_MASK) << cpu,
> + slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
> +
> + zynq_set_cpu_jump(cpu, secondary_startup);
> + zynq_enable_cpu(cpu, true);
> +
> + return 0;
> +}
> +
> +/*
> + * Initialise the CPU possible map early - this describes the CPUs
> + * which may be present or become present in the system.
> + */
> +static void __init zynq_smp_init_cpus(void)
> +{
> + unsigned int ncores, i;
> +
> + ncores = scu_get_core_count(scu_base);
> +
> + for (i = 0; i < ncores; ++i)
> + set_cpu_possible(i, true);
> +
> + /* sanity check */
> + if (ncores > num_possible_cpus()) {
> + pr_warn("zynq: no. of cores (%d) greater than configured"
> + "maximum of %d - clipping\n", ncores, num_possible_cpus());
> + ncores = num_possible_cpus();
> + }
Is this necessary?
IRC: There is checking in smp_prepare_cpus in arch/arm/kernel/smp.c for it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 3/3] ARM: zynq: add SMP support
2013-03-25 14:27 ` Michal Simek
@ 2013-03-25 16:34 ` Steffen Trumtrar
2013-03-25 16:47 ` Michal Simek
0 siblings, 1 reply; 18+ messages in thread
From: Steffen Trumtrar @ 2013-03-25 16:34 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 03:27:57PM +0100, Michal Simek wrote:
> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> > The Zynq7000 has a CortexA9 with at least 2 cores. Add support to boot them all.
> > To do this a trampoline is needed. At runtime the jump address and two
> > instructions are copied to RAM by CPU0 and then executed by CPU1.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > Cc: Michal Simek <michal.simek@xilinx.com>
> > ---
> > arch/arm/boot/dts/zynq-7000.dtsi | 14 +++++
> > arch/arm/mach-zynq/Makefile | 2 +
> > arch/arm/mach-zynq/common.c | 1 +
> > arch/arm/mach-zynq/common.h | 14 +++++
> > arch/arm/mach-zynq/headsmp.S | 20 +++++++
> > arch/arm/mach-zynq/platsmp.c | 110 +++++++++++++++++++++++++++++++++++++++
> > 6 files changed, 161 insertions(+)
> > create mode 100644 arch/arm/mach-zynq/headsmp.S
> > create mode 100644 arch/arm/mach-zynq/platsmp.c
> >
> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> > index c103082..258b6c9 100644
> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> > @@ -15,6 +15,20 @@
> > / {
> > compatible = "xlnx,zynq-7000";
> >
> > + cpus {
> > + cpu at 0 {
> > + compatible = "arm,cortex-a9";
> > + next-level-cache = <&L2>;
> > + clocks = <&armpll 0>;
> > + };
> > +
> > + cpu at 1 {
> > + compatible = "arm,cortex-a9";
> > + next-level-cache = <&L2>;
> > + clocks = <&armpll 0>;
> > + };
> > + };
>
> Is this really needed?
>
I don't know if it is really necessary for the SMP stuff, but my reasoning is:
the DT describes the HW and not only for Linux that is. We have two cores, so
we should specify that. Also, you can add other properties here like
imx6q.dtsi does for the operating-points for example.
So, I think it is valid to have those entries here.
> > +
> > amba {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
> > index 397268c..906d199 100644
> > --- a/arch/arm/mach-zynq/Makefile
> > +++ b/arch/arm/mach-zynq/Makefile
> > @@ -4,3 +4,5 @@
> >
> > # Common support
> > obj-y := common.o timer.o
> > +AFLAGS_headsmp.o :=-Wa,-march=armv7-a
> > +obj-$(CONFIG_SMP) += headsmp.o platsmp.o
> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> > index 1b9bb3d..4053962 100644
> > --- a/arch/arm/mach-zynq/common.c
> > +++ b/arch/arm/mach-zynq/common.c
> > @@ -117,6 +117,7 @@ static const char *xilinx_dt_match[] = {
> > };
> >
> > MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
> > + .smp = smp_ops(zynq_smp_ops),
> > .map_io = xilinx_map_io,
> > .init_irq = xilinx_irqchip_init,
> > .init_machine = xilinx_init_machine,
> > diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
> > index 8b4dbba..451d386 100644
> > --- a/arch/arm/mach-zynq/common.h
> > +++ b/arch/arm/mach-zynq/common.h
> > @@ -19,4 +19,18 @@
> >
> > void __init xttcps_timer_init(void);
> >
> > +#ifdef CONFIG_SMP
> > +extern void secondary_startup(void);
> > +extern char secondary_trampoline, secondary_trampoline_end;
> > +#endif
> > +
> > +extern struct smp_operations __initdata zynq_smp_ops;
> > +extern void __iomem *slcr_base_addr;
> > +extern void __iomem *scu_base;
> > +
> > +#define SLCR_UNLOCK_MAGIC 0xdf0d
> > +
> > +#define SLCR_UNLOCK 0x8
> > +#define SLCR_A9_CPU_RST_CTRL 0x244
>
> As previous code.
>
> > +
> > #endif
> > diff --git a/arch/arm/mach-zynq/headsmp.S b/arch/arm/mach-zynq/headsmp.S
> > new file mode 100644
> > index 0000000..145bbae
> > --- /dev/null
> > +++ b/arch/arm/mach-zynq/headsmp.S
> > @@ -0,0 +1,20 @@
> > +/*
> > + * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > + *
> > + * based on mach-socfpga/headsmp.S
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +#include <linux/linkage.h>
> > +#include <linux/init.h>
> > +
> > + __CPUINIT
> > + .arch armv7-a
> > +
> > +ENTRY(secondary_trampoline)
> > + ldr r0, [pc]
> > + mov pc, r0
>
> This code is familiar to me. It looks like my jump trampoline which I
> wrote in C. :-)
>
Of course. I shouldn't have only mentioned that in the platsmp.c
But I think asm is a little easier on the eyes than machine code ;-)
> > +
> > +ENTRY(secondary_trampoline_end)
> > diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
> > new file mode 100644
> > index 0000000..1ea3d4f
> > --- /dev/null
> > +++ b/arch/arm/mach-zynq/platsmp.c
> > @@ -0,0 +1,110 @@
> > +/*
> > + * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > + *
> > + * based on
> > + * linux/arch/arm/mach-zynq/platsmp.c Copyright (C) 2011 Xilinx
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/irqchip/arm-gic.h>
> > +#include <linux/io.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/smp.h>
> > +
> > +#include <asm/cacheflush.h>
> > +#include <asm/mach/arch.h>
> > +#include <asm/mach/map.h>
> > +#include <asm/smp_scu.h>
> > +
> > +#include "common.h"
> > +
> > +#define A9_RST_MASK (1 << 0)
> > +#define A9_CLKSTOP_MASK (1 << 4)
>
> as I wrote above moving to separate driver make sense to me.
>
> > +
> > +static DEFINE_SPINLOCK(boot_lock);
> > +
> > +static void __cpuinit zynq_smp_secondary_init(unsigned int cpu)
> > +{
> > + /*
> > + * if any interrupts are already enabled for the primary
> > + * core (e.g. timer irq), then they will not have been enabled
> > + * for us: do so
> > + */
> > + gic_secondary_init(0);
> > +
> > + /*
> > + * Synchronise with the boot thread.
> > + */
> > + spin_lock(&boot_lock);
> > + spin_unlock(&boot_lock);
> > +}
> > +
> > +static inline void zynq_set_cpu_jump(int cpu, void *jump_addr)
> > +{
> > + if (jump_addr) {
>
> This is wrong because if jump_addr is 0x1 - 0xF then you are rewriting.
> It is unlikely but it should be captured.
> The main reason is AMP solution where Linux can't be added from 0x0
> and user amp code can setup any entry point. Better to check it just for sure.
>
> > + int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
> > +
> > + writel(virt_to_phys(jump_addr), phys_to_virt(8));
>
> This will probably generate sparse warning.
>
> > + memcpy(phys_to_virt(0), &secondary_trampoline, trampoline_size);
>
> This is nice solution. I am hardcoding instruction and this looks like
> good solution.
> Let me do some experiment with it because maybe this could be
> problematic when Linux
> kernel starts from different address than 0x0.
>
> > +
> > + flush_cache_all();
> > + outer_flush_all();
> > + smp_wmb();
> > + }
> > +}
> > +
> > +static void zynq_enable_cpu(int cpu, bool enable)
> > +{
> > + writel(A9_CLKSTOP_MASK << cpu, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
> > + writel(0, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
> > +}
> > +
> > +int __cpuinit zynq_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> > +{
> > + writel(SLCR_UNLOCK_MAGIC, slcr_base_addr + SLCR_UNLOCK);
> > + writel((A9_CLKSTOP_MASK | A9_RST_MASK) << cpu,
> > + slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
> > +
> > + zynq_set_cpu_jump(cpu, secondary_startup);
> > + zynq_enable_cpu(cpu, true);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Initialise the CPU possible map early - this describes the CPUs
> > + * which may be present or become present in the system.
> > + */
> > +static void __init zynq_smp_init_cpus(void)
> > +{
> > + unsigned int ncores, i;
> > +
> > + ncores = scu_get_core_count(scu_base);
> > +
> > + for (i = 0; i < ncores; ++i)
> > + set_cpu_possible(i, true);
> > +
> > + /* sanity check */
> > + if (ncores > num_possible_cpus()) {
> > + pr_warn("zynq: no. of cores (%d) greater than configured"
> > + "maximum of %d - clipping\n", ncores, num_possible_cpus());
> > + ncores = num_possible_cpus();
> > + }
>
> Is this necessary?
> IRC: There is checking in smp_prepare_cpus in arch/arm/kernel/smp.c for it.
>
As our solutions are pretty much alike, I think I will not work on this patch
in the future. Makes no sense to do everything twice :-)
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 3/3] ARM: zynq: add SMP support
2013-03-25 16:34 ` Steffen Trumtrar
@ 2013-03-25 16:47 ` Michal Simek
0 siblings, 0 replies; 18+ messages in thread
From: Michal Simek @ 2013-03-25 16:47 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/25 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> On Mon, Mar 25, 2013 at 03:27:57PM +0100, Michal Simek wrote:
>> 2013/3/23 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> > The Zynq7000 has a CortexA9 with at least 2 cores. Add support to boot them all.
>> > To do this a trampoline is needed. At runtime the jump address and two
>> > instructions are copied to RAM by CPU0 and then executed by CPU1.
>> >
>> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> > Cc: Michal Simek <michal.simek@xilinx.com>
>> > ---
>> > arch/arm/boot/dts/zynq-7000.dtsi | 14 +++++
>> > arch/arm/mach-zynq/Makefile | 2 +
>> > arch/arm/mach-zynq/common.c | 1 +
>> > arch/arm/mach-zynq/common.h | 14 +++++
>> > arch/arm/mach-zynq/headsmp.S | 20 +++++++
>> > arch/arm/mach-zynq/platsmp.c | 110 +++++++++++++++++++++++++++++++++++++++
>> > 6 files changed, 161 insertions(+)
>> > create mode 100644 arch/arm/mach-zynq/headsmp.S
>> > create mode 100644 arch/arm/mach-zynq/platsmp.c
>> >
>> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> > index c103082..258b6c9 100644
>> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> > @@ -15,6 +15,20 @@
>> > / {
>> > compatible = "xlnx,zynq-7000";
>> >
>> > + cpus {
>> > + cpu at 0 {
>> > + compatible = "arm,cortex-a9";
>> > + next-level-cache = <&L2>;
>> > + clocks = <&armpll 0>;
>> > + };
>> > +
>> > + cpu at 1 {
>> > + compatible = "arm,cortex-a9";
>> > + next-level-cache = <&L2>;
>> > + clocks = <&armpll 0>;
>> > + };
>> > + };
>>
>> Is this really needed?
>>
> I don't know if it is really necessary for the SMP stuff, but my reasoning is:
> the DT describes the HW and not only for Linux that is. We have two cores, so
> we should specify that. Also, you can add other properties here like
> imx6q.dtsi does for the operating-points for example.
> So, I think it is valid to have those entries here.
>From device-tree description point of view we should add that cpus.
Rob also mention in scu patch that we can count number of cpus from dts
that there is support which we can use.
But anyway there are are still some things in description which I really miss
like adding pmu node directly to cpu node because core0 uses different irq
than core1. In our case there is also register access.
The same case is with scu timer/watchdog which we have on the bus
and it is handled via PPI interrupt cpu mask properties.
I am not saying not to have them at all.
The point is why to have this in this patch in smp support if it works
without it.
>> > +
>> > amba {
>> > compatible = "simple-bus";
>> > #address-cells = <1>;
>> > diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
>> > index 397268c..906d199 100644
>> > --- a/arch/arm/mach-zynq/Makefile
>> > +++ b/arch/arm/mach-zynq/Makefile
>> > @@ -4,3 +4,5 @@
>> >
>> > # Common support
>> > obj-y := common.o timer.o
>> > +AFLAGS_headsmp.o :=-Wa,-march=armv7-a
>> > +obj-$(CONFIG_SMP) += headsmp.o platsmp.o
>> > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
>> > index 1b9bb3d..4053962 100644
>> > --- a/arch/arm/mach-zynq/common.c
>> > +++ b/arch/arm/mach-zynq/common.c
>> > @@ -117,6 +117,7 @@ static const char *xilinx_dt_match[] = {
>> > };
>> >
>> > MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
>> > + .smp = smp_ops(zynq_smp_ops),
>> > .map_io = xilinx_map_io,
>> > .init_irq = xilinx_irqchip_init,
>> > .init_machine = xilinx_init_machine,
>> > diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
>> > index 8b4dbba..451d386 100644
>> > --- a/arch/arm/mach-zynq/common.h
>> > +++ b/arch/arm/mach-zynq/common.h
>> > @@ -19,4 +19,18 @@
>> >
>> > void __init xttcps_timer_init(void);
>> >
>> > +#ifdef CONFIG_SMP
>> > +extern void secondary_startup(void);
>> > +extern char secondary_trampoline, secondary_trampoline_end;
>> > +#endif
>> > +
>> > +extern struct smp_operations __initdata zynq_smp_ops;
>> > +extern void __iomem *slcr_base_addr;
>> > +extern void __iomem *scu_base;
>> > +
>> > +#define SLCR_UNLOCK_MAGIC 0xdf0d
>> > +
>> > +#define SLCR_UNLOCK 0x8
>> > +#define SLCR_A9_CPU_RST_CTRL 0x244
>>
>> As previous code.
>>
>> > +
>> > #endif
>> > diff --git a/arch/arm/mach-zynq/headsmp.S b/arch/arm/mach-zynq/headsmp.S
>> > new file mode 100644
>> > index 0000000..145bbae
>> > --- /dev/null
>> > +++ b/arch/arm/mach-zynq/headsmp.S
>> > @@ -0,0 +1,20 @@
>> > +/*
>> > + * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> > + *
>> > + * based on mach-socfpga/headsmp.S
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + */
>> > +#include <linux/linkage.h>
>> > +#include <linux/init.h>
>> > +
>> > + __CPUINIT
>> > + .arch armv7-a
>> > +
>> > +ENTRY(secondary_trampoline)
>> > + ldr r0, [pc]
>> > + mov pc, r0
>>
>> This code is familiar to me. It looks like my jump trampoline which I
>> wrote in C. :-)
>>
> Of course. I shouldn't have only mentioned that in the platsmp.c
> But I think asm is a little easier on the eyes than machine code ;-)
Definitely. I will adopt this solution.
Thanks,
>> > +
>> > +ENTRY(secondary_trampoline_end)
>> > diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
>> > new file mode 100644
>> > index 0000000..1ea3d4f
>> > --- /dev/null
>> > +++ b/arch/arm/mach-zynq/platsmp.c
>> > @@ -0,0 +1,110 @@
>> > +/*
>> > + * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> > + *
>> > + * based on
>> > + * linux/arch/arm/mach-zynq/platsmp.c Copyright (C) 2011 Xilinx
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + */
>> > +#include <linux/delay.h>
>> > +#include <linux/errno.h>
>> > +#include <linux/init.h>
>> > +#include <linux/irqchip/arm-gic.h>
>> > +#include <linux/io.h>
>> > +#include <linux/jiffies.h>
>> > +#include <linux/smp.h>
>> > +
>> > +#include <asm/cacheflush.h>
>> > +#include <asm/mach/arch.h>
>> > +#include <asm/mach/map.h>
>> > +#include <asm/smp_scu.h>
>> > +
>> > +#include "common.h"
>> > +
>> > +#define A9_RST_MASK (1 << 0)
>> > +#define A9_CLKSTOP_MASK (1 << 4)
>>
>> as I wrote above moving to separate driver make sense to me.
>>
>> > +
>> > +static DEFINE_SPINLOCK(boot_lock);
>> > +
>> > +static void __cpuinit zynq_smp_secondary_init(unsigned int cpu)
>> > +{
>> > + /*
>> > + * if any interrupts are already enabled for the primary
>> > + * core (e.g. timer irq), then they will not have been enabled
>> > + * for us: do so
>> > + */
>> > + gic_secondary_init(0);
>> > +
>> > + /*
>> > + * Synchronise with the boot thread.
>> > + */
>> > + spin_lock(&boot_lock);
>> > + spin_unlock(&boot_lock);
>> > +}
>> > +
>> > +static inline void zynq_set_cpu_jump(int cpu, void *jump_addr)
>> > +{
>> > + if (jump_addr) {
>>
>> This is wrong because if jump_addr is 0x1 - 0xF then you are rewriting.
>> It is unlikely but it should be captured.
>> The main reason is AMP solution where Linux can't be added from 0x0
>> and user amp code can setup any entry point. Better to check it just for sure.
>>
>> > + int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
>> > +
>> > + writel(virt_to_phys(jump_addr), phys_to_virt(8));
>>
>> This will probably generate sparse warning.
>>
>> > + memcpy(phys_to_virt(0), &secondary_trampoline, trampoline_size);
>>
>> This is nice solution. I am hardcoding instruction and this looks like
>> good solution.
>> Let me do some experiment with it because maybe this could be
>> problematic when Linux
>> kernel starts from different address than 0x0.
>>
>> > +
>> > + flush_cache_all();
>> > + outer_flush_all();
>> > + smp_wmb();
>> > + }
>> > +}
>> > +
>> > +static void zynq_enable_cpu(int cpu, bool enable)
>> > +{
>> > + writel(A9_CLKSTOP_MASK << cpu, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
>> > + writel(0, slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
>> > +}
>> > +
>> > +int __cpuinit zynq_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
>> > +{
>> > + writel(SLCR_UNLOCK_MAGIC, slcr_base_addr + SLCR_UNLOCK);
>> > + writel((A9_CLKSTOP_MASK | A9_RST_MASK) << cpu,
>> > + slcr_base_addr + SLCR_A9_CPU_RST_CTRL);
>> > +
>> > + zynq_set_cpu_jump(cpu, secondary_startup);
>> > + zynq_enable_cpu(cpu, true);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +/*
>> > + * Initialise the CPU possible map early - this describes the CPUs
>> > + * which may be present or become present in the system.
>> > + */
>> > +static void __init zynq_smp_init_cpus(void)
>> > +{
>> > + unsigned int ncores, i;
>> > +
>> > + ncores = scu_get_core_count(scu_base);
>> > +
>> > + for (i = 0; i < ncores; ++i)
>> > + set_cpu_possible(i, true);
>> > +
>> > + /* sanity check */
>> > + if (ncores > num_possible_cpus()) {
>> > + pr_warn("zynq: no. of cores (%d) greater than configured"
>> > + "maximum of %d - clipping\n", ncores, num_possible_cpus());
>> > + ncores = num_possible_cpus();
>> > + }
>>
>> Is this necessary?
>> IRC: There is checking in smp_prepare_cpus in arch/arm/kernel/smp.c for it.
>>
>
> As our solutions are pretty much alike, I think I will not work on this patch
> in the future. Makes no sense to do everything twice :-)
:-)
M
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 18+ messages in thread