Rob Herring wrote:
>
> On Wed, Apr 16, 2014 at 6:50 AM, Sachin Kamat <sachin.kamat@xxxxxxxxxx>
> wrote:
> > Instead of hardcoding the SYSRAM details for each SoC,
> > pass this information through device tree (DT) and make
> > the code SoC agnostic.
> >
> > Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx>
> > ---
> > Rebased on latest linux-next.
> > ---
> > .../devicetree/bindings/arm/samsung-boards.txt | 11 +++
> > arch/arm/boot/dts/exynos4210-universal_c210.dts | 9 ++
> > arch/arm/boot/dts/exynos4210.dtsi | 10 ++
> > arch/arm/boot/dts/exynos4x12.dtsi | 10 ++
> > arch/arm/boot/dts/exynos5.dtsi | 5 +
> > arch/arm/boot/dts/exynos5250.dtsi | 5 +
> > arch/arm/boot/dts/exynos5420.dtsi | 5 +
> > arch/arm/mach-exynos/exynos.c | 104 ++++++++-----------
> -
> > arch/arm/mach-exynos/include/mach/map.h | 7 --
> > 9 files changed, 95 insertions(+), 71 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/samsung-boards.txt
> b/Documentation/devicetree/bindings/arm/samsung-boards.txt
> > index 2168ed31e1b0..f79710eb7e79 100644
> > --- a/Documentation/devicetree/bindings/arm/samsung-boards.txt
> > +++ b/Documentation/devicetree/bindings/arm/samsung-boards.txt
> > @@ -7,6 +7,17 @@ Required root node properties:
> > (a) "samsung,smdkv310" - for Samsung's SMDKV310 eval board.
> > (b) "samsung,exynos4210" - for boards based on Exynos4210 SoC.
> >
> > + - sysram node, specifying the type (secure or non-secure) of SYSRAM
> > + - compatible: following types are supported
> > + "samsung,exynos4210-sysram" : Secure SYSRAM
> > + "samsung,exynos4210-sysram-ns" : Non-secure SYSRAM
>
> Base this on mmio-sram binding please.
>
> > + - reg: address of SYSRAM bank
> > +
> > + sysram@02020000 {
> > + compatible = "samsung,exynos4210-sysram";
> > + reg = <0x02020000 0x1000>;
> > + };
> > +
> > Optional:
> > - firmware node, specifying presence and type of secure firmware:
> > - compatible: only "samsung,secure-firmware" is currently
> supported
> > diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts
> b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> > index 63e34b24b04f..cf4158728108 100644
> > --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
> > +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> > @@ -28,6 +28,15 @@
> > bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw
> rootwait earlyprintk panic=5 maxcpus=1";
> > };
> >
> > + sysram@02020000 {
> > + status = "disabled";
> > + };
> > +
> > + sysram@02025000 {
> > + compatible = "samsung,exynos4210-sysram";
> > + reg = <0x02025000 0x1000>;
> > + };
> > +
> > mct@10050000 {
> > compatible = "none";
> > };
> > diff --git a/arch/arm/boot/dts/exynos4210.dtsi
> b/arch/arm/boot/dts/exynos4210.dtsi
> > index cacf6140dd2f..a3f4bba099e6 100644
> > --- a/arch/arm/boot/dts/exynos4210.dtsi
> > +++ b/arch/arm/boot/dts/exynos4210.dtsi
> > @@ -31,6 +31,16 @@
> > pinctrl2 = &pinctrl_2;
> > };
> >
> > + sysram@02020000 {
> > + compatible = "samsung,exynos4210-sysram";
> > + reg = <0x02020000 0x1000>;
> > + };
> > +
> > + sysram-ns@0203F000 {
> > + compatible = "samsung,exynos4210-sysram-ns";
> > + reg = <0x0203F000 0x1000>;
>
> hex should be lower case.
>
>
> [...]
>
> > @@ -268,6 +218,44 @@ static int __init exynos_fdt_map_chipid(unsigned
> long node, const char *uname,
> > return 1;
> > }
> >
> > +struct __sysram_desc {
> > + char name[32];
> > + unsigned long addr;
> > +};
> > +
> > +static struct __sysram_desc sysram_desc[] __initdata = {
> > + {
> > + .name = "samsung,exynos4210-sysram",
> > + .addr = (unsigned long)S5P_VA_SYSRAM,
> > + }, {
> > + .name = "samsung,exynos4210-sysram-ns",
> > + .addr = (unsigned long)S5P_VA_SYSRAM_NS,
> > + },
> > +};
> > +
> > +static int __init exynos_fdt_map_sysram(unsigned long node, const char
> *uname,
> > + int depth, void *data)
> > +{
> > + struct map_desc iodesc;
> > + __be32 *reg;
> > + unsigned long len;
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(sysram_desc); i++) {
> > + if (of_flat_dt_is_compatible(node, sysram_desc[i].name)) {
> > + reg = of_get_flat_dt_prop(node, "reg", &len);
> > + if (!reg || len != (sizeof(unsigned long) * 2))
> > + return -ENODEV;
> > + iodesc.virtual = sysram_desc[i].addr;
> > + iodesc.pfn = __phys_to_pfn(be32_to_cpu(reg[0]));
> > + iodesc.length = be32_to_cpu(reg[1]);
> > + iodesc.type = MT_DEVICE;
> > + iotable_init(&iodesc, 1);
>
> I don't think this needs to be a static mapping at all. Fix your SMP
> code. Move the code setting the boot address in prepare_cpus to
> boot_secondary.
>
> Also, the pen code is all unnecessary if you can properly reset a core
> on hotplug. Given this code:
>
> if (!(__raw_readl(S5P_ARM_CORE1_STATUS) & S5P_CORE_LOCAL_PWR_EN)) {
> __raw_writel(S5P_CORE_LOCAL_PWR_EN,
> S5P_ARM_CORE1_CONFIGURATION);
>
> and:
>
> /* make cpu1 to be turned off at next WFI command */
> if (cpu == 1)
> __raw_writel(0, S5P_ARM_CORE1_CONFIGURATION);
>
> It would appear you probably have this capability and the exynos smp
> code is pure mindless cut n' paste.
>
Basically, I have no strong objection on Rob's suggestion but I'd like to pick this one after small addressing because of other patches' dependency. Then how about addressing comments?
- Kukjin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
