Add support for Marvell's RD-DXBC2
Subsystems (6)
cover Add support for Marvell's RD-DXBC2
This series allows me to boot v3.18 on Marvell's reference board and start up a basic userspace. I've sent 1/4 and 2/4 to the list individually before but hopefully they will be more meaningful when viewed in context.
Thanks,
Chris
Chris Packham (4):
ARM: mvebu: use dt_fixup to provide fallback for enable-method
clk: mvebu: armada-xp: Support for 98DX4251
ARM: mvebu: Initial support for rd-dxbc2
ARM: mvebu: Custom smp_ops for 98DX4251
.../devicetree/bindings/clock/mvebu-core-clock.txt | 4 + arch/arm/boot/dts/rd-dxbc2.dts | 118 +++++++++++++++++++++ arch/arm/mach-mvebu/Makefile | 1 + arch/arm/mach-mvebu/board-v7.c | 7 +- arch/arm/mach-mvebu/common.h | 2 + arch/arm/mach-mvebu/mvebu-soc-id.h | 3 + arch/arm/mach-mvebu/platsmp.c | 28 +++++ arch/arm/mach-mvebu/pmsu-98dx4251.c | 68 ++++++++++++ drivers/clk/mvebu/armada-xp.c | 35 ++++++ 9 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/dts/rd-dxbc2.dts create mode 100644 arch/arm/mach-mvebu/pmsu-98dx4251.c
Signature · 1 line
2.2.0.rc0
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
We need to maintain backwards compatibility with device trees that don't define an enable method. At the same time we want the device tree to be able to specify an enable-method and have it stick.
Previously by having smp assigned in the DT_MACHINE definition this would be picked up by setup_arch() and override whatever arm_dt_init_cpu_maps() had configured. Now we move the initial assignment of default smp_ops to a dt_fixup and let arm_dt_init_cpu_maps() override that if the device tree defines an enable-method.
---
This didn't get much attention last time and is unchanged from v4. I'm pretty happy with this incarnation. It doesn't touch core code. It provides a fallback for old device trees and it achieves my original goal of allowing the device tree to configure the smp_ops via the enable-method property.
v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300182.html
v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300480.html
v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302945.html
v4: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/303899.html
arch/arm/mach-mvebu/board-v7.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/arm/mach-mvebu/board-v7.c +6 -1
@@ -198,6 +198,11 @@ static void __init mvebu_dt_init(void)of_platform_populate (NULL , of_default_bus_match_table,NULL ,NULL ); } +static void __init armada_370_xp_dt_fixup (void ) +{ +smp_set_ops (smp_ops (armada_xp_smp_ops)); +} +static const char *const armada_370_xp_dt_compat[] = {"marvell,armada-370-xp" ,NULL ,
@@ -206,11 +211,11 @@ static const char * const armada_370_xp_dt_compat[] = {DT_MACHINE_START (ARMADA_370_XP_DT ,"Marvell Armada 370/XP (Device Tree)" ) .l2c_aux_val = 0, .l2c_aux_mask = ~0, - .smp = smp_ops(armada_xp_smp_ops), .init_machine = mvebu_dt_init, .init_irq = mvebu_init_irq, .restart = mvebu_restart, .dt_compat = armada_370_xp_dt_compat, + .dt_fixup = armada_370_xp_dt_fixup, MACHINE_ENDstatic const char *const armada_375_dt_compat[] = {
Signature · 1 line
2.2.0.rc0
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
arch/arm/mach-mvebu/board-v7.c static void __init mvebu_dt_init(void) · line 205
Quoted 7 lines · On Wednesday 10 December 2014 15:39:44 Chris Packham wrote:
arch/arm/mach-mvebu/board-v7.c +5
-0+static void __init armada_370_xp_dt_fixup (void ) +{ +smp_set_ops (smp_ops (armada_xp_smp_ops)); +} +
The dt_fixup callback pointer is meant to fix up a legacy dtb file in memory. I think this would be fairly easy in this case, just add in the missing enable-method property here to make the normal boot path work for old dtbs.
Arnd
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Hi Arnd,
Quoted 14 lines · Arnd Bergmann
arch/arm/mach-mvebu/board-v7.c static void __init mvebu_dt_init(void) · line 205
Quoted 7 lines · On Wednesday 10 December 2014 15:39:44 Chris Packham wrote:
arch/arm/mach-mvebu/board-v7.c +5
-0+static void __init armada_370_xp_dt_fixup (void ) +{ +smp_set_ops (smp_ops (armada_xp_smp_ops)); +} +The
dt_fixupcallback pointer is meant to fix up a legacy dtb file in memory. I think this would be fairly easy in this case, just add in the missing enable-method property here to make the normal boot path work for old dtbs.
Quoted 2 lines
Arnd
I briefly explored that approach here[1]. The tricky part would be handling the fact that the enable method can be attached to either the /cpus node or and individual /cpu entry (or is that something I can ignore?).
In the end I thought that the unconditional setting of smp_ops was easier to implement and would achieve the same result.
--
[1] -
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/303465.html
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Hi Arnd,
Quoted 18 lines · Arnd Bergmann
Quoted 14 lines · On Wednesday 10 December 2014 15:39:44 Chris Packham wrote:
Review
Quoted 7 lines
(quoted diff) +5
-0+static void __init armada_370_xp_dt_fixup(void) +{ + smp_set_ops(smp_ops(armada_xp_smp_ops)); +} +The
dt_fixupcallback pointer is meant to fix up a legacy dtb file in memory. I think this would be fairly easy in this case, just add in the missing enable-method property here to make the normal boot path work for old dtbs.Quoted 2 lines
Arnd
Quoted 7 lines
I briefly explored that approach here[1]. The tricky part would be handling the fact that the enable method can be attached to either the /cpus node or and individual /cpu entry (or is that something I can ignore?).
In the end I thought that the unconditional setting of
smp_opswas easier to implement and would achieve the same result.
Actually as it turns out it's not that hard to implement something that checks both /cpus and /cpus/cpu at n. I'll include this with the next round after I've waited for anymore feedback.
---8<---
Subject: [PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method
When the device tree doesn't define an enable-method insert a property into the flattened device tree. arm_dt_init_cpu_maps() will then parse this an set smp_ops appropriately. Now that we have this fallback it is no longer necessary to set .smp in the DT_MACHINE definition.
--- arch/arm/mach-mvebu/Makefile | 2 ++ arch/arm/mach-mvebu/board-v7.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-)
arch/arm/mach-mvebu/Makefile +2 -0
@@ -14,3 +14,5 @@ endif obj-$(CONFIG_MACH_DOVE) += dove.o obj-$(CONFIG_MACH_KIRKWOOD) += kirkwood.o kirkwood-pm.o obj-$(CONFIG_MACH_NETXBIG) += netxbig.o + +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdt
arch/arm/mach-mvebu/board-v7.c +36 -1
@@ -17,6 +17,8 @@#include <linux/clk-provider.h> #include <linux/of_address.h> #include <linux/of_platform.h> +#include <linux/of_fdt.h> +#include <linux/libfdt.h> #include <linux/io.h> #include <linux/clocksource.h> #include <linux/dma-mapping.h>
@@ -198,6 +200,39 @@ static void __init mvebu_dt_init(void)of_platform_populate (NULL , of_default_bus_match_table,NULL ,NULL ); } +static void __init armada_370_xp_dt_fixup (void ) +{ +int offset, node; +int i, len; +void *prop; +char buffer[20 ]; + + offset =fdt_path_offset (initial_boot_params,"/cpus" ); +if (offset <0 ) +return ; + + prop =fdt_getprop (initial_boot_params, offset,"enable-method" , &len); +if (prop) +return ; + +for (i =0 ; i <NR_CPUS ; i++) { +snprintf (buffer,sizeof (buffer),"cpu@%d" , i); + node =fdt_subnode_offset (initial_boot_params, offset, buffer); +if (node <0 ) +break ; + prop =fdt_getprop (initial_boot_params, node, +"enable-method" , &len); +if (prop) +return ; + } + +pr_info ("No enable-method defined. " +"Falling back to \"marvell,armada-xp-smp\"\n" ); + +fdt_setprop (initial_boot_params, offset,"enable-method" , +"marvell,armada-xp-smp" ,sizeof ("marvell,armada-xp-smp" )); +} +static const char *const armada_370_xp_dt_compat[] = {"marvell,armada-370-xp" ,NULL ,
@@ -206,11 +241,11 @@ static const char * const armada_370_xp_dt_compat[] = {DT_MACHINE_START (ARMADA_370_XP_DT ,"Marvell Armada 370/XP (Device Tree)" ) .l2c_aux_val = 0, .l2c_aux_mask = ~0, - .smp = smp_ops(armada_xp_smp_ops), .init_machine = mvebu_dt_init, .init_irq = mvebu_init_irq, .restart = mvebu_restart, .dt_compat = armada_370_xp_dt_compat, + .dt_fixup = armada_370_xp_dt_fixup, MACHINE_ENDstatic const char *const armada_375_dt_compat[] = {
Signature · 1 line
2.2.0.rc0
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Quoted 11 lines · On Friday 12 December 2014 17:53:55 Chris Packham wrote:
Quoted 7 lines
I briefly explored that approach here[1]. The tricky part would be handling the fact that the enable method can be attached to either the /cpus node or and individual /cpu entry (or is that something I can ignore?).
In the end I thought that the unconditional setting of
smp_opswas easier to implement and would achieve the same result.Actually as it turns out it's not that hard to implement something that checks both /cpus and /cpus/cpu at n. I'll include this with the next round after I've waited for anymore feedback.
Ah, very nice!
Quoted 19 lines
---8<---
Subject: [PATCH] ARM: mvebu: usedt_fixupto provide fallback for enable-methodWhen the device tree doesn't define an enable-method insert a property into the flattened device tree.
arm_dt_init_cpu_maps()will then parse this an setsmp_opsappropriately. Now that we have this fallback it is no longer necessary to set .smp in theDT_MACHINEdefinition.Signed-off-by Chris Packham <[email protected]>--- arch/arm/mach-mvebu/Makefile | 2 ++ arch/arm/mach-mvebu/board-v7.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-)arch/arm/mach-mvebu/Makefile +0
-0
arch/arm/mach-mvebu/Makefile endif · line 18
Quoted 6 lines
arch/arm/mach-mvebu/Makefile +2
-0@@ -14,3 +14,5 @@ endif obj-$(CONFIG_MACH_DOVE) += dove.o obj-$(CONFIG_MACH_KIRKWOOD) += kirkwood.o kirkwood-pm.o obj-$(CONFIG_MACH_NETXBIG) += netxbig.o + +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdt
Why is this needed? Can't you just include <linux/libfdt.h> ?
Quoted 22 lines
arch/arm/mach-mvebu/Makefile +22
-0+static void __init armada_370_xp_dt_fixup(void) +{ + int offset, node; + int i, len; + void *prop; + char buffer[20]; + + offset = fdt_path_offset(initial_boot_params, "/cpus"); + if (offset < 0) + return; + + prop = fdt_getprop(initial_boot_params, offset, "enable-method", &len); + if (prop) + return; + + for (i = 0; i < NR_CPUS; i++) { + snprintf(buffer, sizeof(buffer), "cpu@%d", i); + node = fdt_subnode_offset(initial_boot_params, offset, buffer); + if (node < 0) + break; + prop = fdt_getprop(initial_boot_params, node, + "enable-method", &len);
arch/arm/mach-mvebu/Makefile
Quoted 10 lines
arch/arm/mach-mvebu/Makefile +10
-0+ if (prop) + return; + } + + pr_info("No enable-method defined. " + "Falling back to \"marvell,armada-xp-smp\"\n"); + + fdt_setprop(initial_boot_params, offset, "enable-method", + "marvell,armada-xp-smp", sizeof("marvell,armada-xp-smp")); +}
I think it would be good to first check whether you are running on Armada XP or Armada 370, because the latter does not require this.
Arnd
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Hi Arnd,
Quoted 29 lines · Arnd Bergmann
Quoted 11 lines · On Friday 12 December 2014 17:53:55 Chris Packham wrote:
Quoted 7 lines
I briefly explored that approach here[1]. The tricky part would be handling the fact that the enable method can be attached to either the /cpus node or and individual /cpu entry (or is that something I can ignore?).
In the end I thought that the unconditional setting of
smp_opswas easier to implement and would achieve the same result.Actually as it turns out it's not that hard to implement something that checks both /cpus and /cpus/cpu at n. I'll include this with the next round after I've waited for anymore feedback.
Ah, very nice!
Quoted 14 lines
---8<---
Subject: [PATCH] ARM: mvebu: usedt_fixupto provide fallback for enable-methodWhen the device tree doesn't define an enable-method insert a property into the flattened device tree.
arm_dt_init_cpu_maps()will then parse this an setsmp_opsappropriately. Now that we have this fallback it is no longer necessary to set .smp in theDT_MACHINEdefinition.Signed-off-by Chris Packham <[email protected]>--- arch/arm/mach-mvebu/Makefile | 2 ++ arch/arm/mach-mvebu/board-v7.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-)
Quoted 14 lines
arch/arm/mach-mvebu/Makefile endif · line 18
Quoted 11 lines
arch/arm/mach-mvebu/Makefile +2
-0@@ -14,3 +14,5 @@ endif obj-$(CONFIG_MACH_DOVE) += dove.o obj-$(CONFIG_MACH_KIRKWOOD) += kirkwood.o kirkwood-pm.o obj-$(CONFIG_MACH_NETXBIG) += netxbig.o + +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdtWhy is this needed? Can't you just include <
linux/libfdt.h> ?
I am already including linux/libfdt.h. Without the CFLAGS change I get the following compile error.
In file included from include/linux/libfdt.h:6:0,
from arch/arm/mach-mvebu/board-v7.c:21:
include/linux/../../scripts/dtc/libfdt/libfdt.h:54:24: fatal error:libfdt_env.h: No such file or directory
There seems to be precedence in other architectures/drivers
$ git grep -e '-I.*libfdt'
arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o =
-I$(src)/../../../scripts/dtc/libfdt
arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o =
-I$(src)/../../../scripts/dtc/libfdt
arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o =
-I$(src)/../../../scripts/dtc/libfdt
arch/powerpc/kernel/Makefile:CFLAGS_prom.o =
-I$(src)/../../../scripts/dtc/libfdt
drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o
+= -I$(srctree)/scripts/dtc/libfdt/
drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt
drivers/of/Makefile:CFLAGS_fdt_address.o =
-I$(src)/../../scripts/dtc/libfdtQuoted 36 lines
Quoted 22 lines
arch/arm/mach-mvebu/Makefile +22
-0+static void __init armada_370_xp_dt_fixup(void) +{ + int offset, node; + int i, len; + void *prop; + char buffer[20]; + + offset = fdt_path_offset(initial_boot_params, "/cpus"); + if (offset < 0) + return; + + prop = fdt_getprop(initial_boot_params, offset, "enable-method", &len); + if (prop) + return; + + for (i = 0; i < NR_CPUS; i++) { + snprintf(buffer, sizeof(buffer), "cpu@%d", i); + node = fdt_subnode_offset(initial_boot_params, offset, buffer); + if (node < 0) + break; + prop = fdt_getprop(initial_boot_params, node, + "enable-method", &len);arch/arm/mach-mvebu/Makefile
Quoted 10 lines
arch/arm/mach-mvebu/Makefile +10
-0+ if (prop) + return; + } + + pr_info("No enable-method defined. " + "Falling back to \"marvell,armada-xp-smp\"\n"); + + fdt_setprop(initial_boot_params, offset, "enable-method", + "marvell,armada-xp-smp", sizeof("marvell,armada-xp-smp")); +}I think it would be good to first check whether you are running on Armada XP or Armada 370, because the latter does not require this.
Quoted 2 lines
Arnd
Any suggestion as to what to check for. Based on the dts files in the current source seem compatible == "marvell,armada370" seems like a good choice. But I don't know for sure that there isn't a armada-xp variant out using a .dts with this set.
Another option might be just to count the /cpu at n nodes.
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Answering my own question
Quoted 112 lines · Chris Packham
Hi Arnd,
Quoted 31 lines · Arnd Bergmann
Quoted 11 lines · On Friday 12 December 2014 17:53:55 Chris Packham wrote:
Quoted 7 lines
I briefly explored that approach here[1]. The tricky part would be handling the fact that the enable method can be attached to either the /cpus node or and individual /cpu entry (or is that something I can ignore?).
In the end I thought that the unconditional setting of
smp_opswas easier to implement and would achieve the same result.Actually as it turns out it's not that hard to implement something that checks both /cpus and /cpus/cpu at n. I'll include this with the next round after I've waited for anymore feedback.
Ah, very nice!
Quoted 16 lines
---8<---
Subject: [PATCH] ARM: mvebu: usedt_fixupto provide fallback for enable-methodWhen the device tree doesn't define an enable-method insert a property into the flattened device tree.
arm_dt_init_cpu_maps()will then parse this an setsmp_opsappropriately. Now that we have this fallback it is no longer necessary to set .smp in theDT_MACHINEdefinition.Signed-off-by Chris Packham <[email protected]>--- arch/arm/mach-mvebu/Makefile | 2 ++ arch/arm/mach-mvebu/board-v7.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-)Quoted 14 lines
arch/arm/mach-mvebu/Makefile endif · line 18
Quoted 11 lines
arch/arm/mach-mvebu/Makefile +2
-0@@ -14,3 +14,5 @@ endif obj-$(CONFIG_MACH_DOVE) += dove.o obj-$(CONFIG_MACH_KIRKWOOD) += kirkwood.o kirkwood-pm.o obj-$(CONFIG_MACH_NETXBIG) += netxbig.o + +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdtWhy is this needed? Can't you just include <
linux/libfdt.h> ?I am already including
linux/libfdt.h. Without the CFLAGS change I get the following compile error.In file included from include/linux/libfdt.h:6:0, from arch/arm/mach-mvebu/board-v7.c:21: include/linux/../../scripts/dtc/libfdt/libfdt.h:54:24: fatal error:
libfdt_env.h: No such file or directoryThere seems to be precedence in other architectures/drivers
$ git grep -e '-I.*libfdt' arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o =(quoted diff) +0
-6-I$(src)/../../../scripts/dtc/libfdt arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/powerpc/kernel/Makefile:CFLAGS_prom.o = -I$(src)/../../../scripts/dtc/libfdt drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt drivers/of/Makefile:CFLAGS_fdt_address.o = -I$(src)/../../scripts/dtc/libfdtQuoted 39 lines
Quoted 24 lines
arch/arm/mach-mvebu/Makefile +12
-0+static void __init armada_370_xp_dt_fixup(void) +{ + int offset, node; + int i, len; + void *prop; + char buffer[20]; + + offset = fdt_path_offset(initial_boot_params, "/cpus"); + if (offset < 0) + return; + + prop = fdt_getprop(initial_boot_params, offset, "enable-method",&len);
arch/arm/mach-mvebu/Makefile +10
-0+ if (prop) + return; + + for (i = 0; i < NR_CPUS; i++) { + snprintf(buffer, sizeof(buffer), "cpu@%d", i); + node = fdt_subnode_offset(initial_boot_params, offset, buffer); + if (node < 0) + break; + prop = fdt_getprop(initial_boot_params, node, + "enable-method", &len);arch/arm/mach-mvebu/Makefile
Quoted 10 lines
arch/arm/mach-mvebu/Makefile +10
-0+ if (prop) + return; + } + + pr_info("No enable-method defined. " + "Falling back to \"marvell,armada-xp-smp\"\n"); + + fdt_setprop(initial_boot_params, offset, "enable-method", + "marvell,armada-xp-smp", sizeof("marvell,armada-xp-smp")); +}I think it would be good to first check whether you are running on Armada XP
or Armada 370, because the latter does not require this.
Quoted 7 lines
Quoted 2 lines
Arnd
Any suggestion as to what to check for. Based on the dts files in the current source seem compatible == "marvell,armada370" seems like a good choice. But I don't know for sure that there isn't a armada-xp variant out using a .dts with this set.
Adding the following at the top will do the trick (assuming "marvell,armada370" is the right compatible string to use).
unsigned long root = of_get_flat_dt_root();
if (of_flat_dt_is_compatible(root, "marvell,armada370"))
return;
Quoted 2 lines
Another option might be just to count the /cpu at n nodes.
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Quoted 18 lines · On Monday 15 December 2014 00:57:49 Chris Packham wrote:
Quoted 17 lines · Chris Packham
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +2
-0+ +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdtWhy is this needed? Can't you just include <
linux/libfdt.h> ?I am already including
linux/libfdt.h. Without the CFLAGS change I get the following compile error.In file included from include/linux/libfdt.h:6:0, from arch/arm/mach-mvebu/board-v7.c:21: include/linux/../../scripts/dtc/libfdt/libfdt.h:54:24: fatal error:
libfdt_env.h: No such file or directoryThere seems to be precedence in other architectures/drivers
Quoted 15 lines
Quoted 5 lines
$ git grep -e '-I.*libfdt' arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o =(quoted diff) +0
-1-I$(src)/../../../scripts/dtc/libfdt arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o =Review
Quoted 10 lines
(quoted diff) +0
-5-I$(src)/../../../scripts/dtc/libfdt arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/powerpc/kernel/Makefile:CFLAGS_prom.o = -I$(src)/../../../scripts/dtc/libfdt drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt drivers/of/Makefile:CFLAGS_fdt_address.o = -I$(src)/../../scripts/dtc/libfdt
I still think we should not duplicate this but instead change the header files to make the include hack unnecessary in all those places.
Looking at the header file, it only contains
#include <linux/libfdt_env.h>
#include "../../scripts/dtc/libfdt/fdt.h"
#include "../../scripts/dtc/libfdt/libfdt.h"
so two of them get the headers from exactly that place, while the libfdt_env.h file contains the linux-specific implementation of a couple of functions instead of the baremetal implementation.
It looks like the intention was that when you already include the linux version of the header, you no longer need the other one, but that assumption is broken by current implementation.
One way to solve this would be
scripts/dtc/libfdt/libfdt.h +1 -1
@@ -51,7 +51,7 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <libfdt_env.h> +#include "libfdt_env.h" #include <fdt.h> #define FDT_FIRST_SUPPORTED_VERSION 0x10
but I'm not sure if that is the best way.
Quoted 12 lines
Quoted 11 lines
Quoted 6 lines
I think it would be good to first check whether you are running on Armada XP
or Armada 370, because the latter does not require this.Arnd
Any suggestion as to what to check for. Based on the dts files in the current source seem compatible == "marvell,armada370" seems like a good choice. But I don't know for sure that there isn't a armada-xp variant out using a .dts with this set.
Quoted 7 lines
Adding the following at the top will do the trick (assuming "marvell,armada370" is the right compatible string to use).
unsigned long root = of_get_flat_dt_root(); if (of_flat_dt_is_compatible(root, "marvell,armada370")) return;
Right. You could also add
if (!IS_ENABLED(CONFIG_SMP) || !of_flat_dt_is_compatible(root, "marvell,armadaxp")))
to again eliminate that check for uniprocessor kernels, and to not propagate the fixup to future machines but make it clear that it is only needed for armadaxp.
Arnd
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Quoted 69 lines · Arnd Bergmann
Quoted 18 lines · On Monday 15 December 2014 00:57:49 Chris Packham wrote:
Quoted 17 lines · Chris Packham
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +2
-0+ +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdtWhy is this needed? Can't you just include <
linux/libfdt.h> ?I am already including
linux/libfdt.h. Without the CFLAGS change I get the following compile error.In file included from include/linux/libfdt.h:6:0, from arch/arm/mach-mvebu/board-v7.c:21: include/linux/../../scripts/dtc/libfdt/libfdt.h:54:24: fatal error:
libfdt_env.h: No such file or directoryThere seems to be precedence in other architectures/drivers
Quoted 15 lines
Quoted 5 lines
$ git grep -e '-I.*libfdt' arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o =(quoted diff) +0
-1-I$(src)/../../../scripts/dtc/libfdt arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o =Review
Quoted 10 lines
(quoted diff) +0
-5-I$(src)/../../../scripts/dtc/libfdt arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/powerpc/kernel/Makefile:CFLAGS_prom.o = -I$(src)/../../../scripts/dtc/libfdt drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt drivers/of/Makefile:CFLAGS_fdt_address.o = -I$(src)/../../scripts/dtc/libfdtI still think we should not duplicate this but instead change the header files to make the include hack unnecessary in all those places.
Looking at the header file, it only contains
#include <
linux/libfdt_env.h>
#include "../../scripts/dtc/libfdt/fdt.h"
#include "../../scripts/dtc/libfdt/libfdt.h"so two of them get the headers from exactly that place, while the
libfdt_env.hfile contains the linux-specific implementation of a couple of functions instead of the baremetal implementation.It looks like the intention was that when you already include the linux version of the header, you no longer need the other one, but that assumption is broken by current implementation.
One way to solve this would be
scripts/dtc/libfdt/libfdt.h +1
-1@@ -51,7 +51,7 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <libfdt_env.h> +#include "libfdt_env.h" #include <fdt.h> #define FDT_FIRST_SUPPORTED_VERSION 0x10
Quoted 2 lines
but I'm not sure if that is the best way.
I was hoping to limit the scope of my changes to mvebu code. But I can give it a whirl. I agree that propagating the hack isn't ideal.
Quoted 20 lines
Quoted 12 lines
Quoted 11 lines
Quoted 6 lines
I think it would be good to first check whether you are running on Armada XP
or Armada 370, because the latter does not require this.Arnd
Any suggestion as to what to check for. Based on the dts files in the current source seem compatible == "marvell,armada370" seems like a good choice. But I don't know for sure that there isn't a armada-xp variant out using a .dts with this set.
Quoted 7 lines
Adding the following at the top will do the trick (assuming "marvell,armada370" is the right compatible string to use).
unsigned long root = of_get_flat_dt_root(); if (of_flat_dt_is_compatible(root, "marvell,armada370")) return;
Quoted 7 lines
Right. You could also add
if (!IS_ENABLED(CONFIG_SMP) || !of_flat_dt_is_compatible(root, "marvell,armadaxp")))to again eliminate that check for uniprocessor kernels, and to not propagate the fixup to future machines but make it clear that it is only needed for armadaxp.
OK makes sense.
Quoted 3 lines
Arnd
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Quoted 70 lines · Chris Packham
Quoted 67 lines · Arnd Bergmann
Quoted 18 lines · On Monday 15 December 2014 00:57:49 Chris Packham wrote:
Quoted 17 lines · Chris Packham
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +2
-0+ +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdtWhy is this needed? Can't you just include <
linux/libfdt.h> ?I am already including
linux/libfdt.h. Without the CFLAGS change I get the following compile error.In file included from include/linux/libfdt.h:6:0, from arch/arm/mach-mvebu/board-v7.c:21: include/linux/../../scripts/dtc/libfdt/libfdt.h:54:24: fatal error:
libfdt_env.h: No such file or directoryThere seems to be precedence in other architectures/drivers
Quoted 16 lines
Quoted 6 lines
$ git grep -e '-I.*libfdt' arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o =(quoted diff) +0
-2-I$(src)/../../../scripts/dtc/libfdt arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdtReview
Quoted 10 lines
(quoted diff) +0
-5arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/powerpc/kernel/Makefile:CFLAGS_prom.o = -I$(src)/../../../scripts/dtc/libfdt drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt drivers/of/Makefile:CFLAGS_fdt_address.o = -I$(src)/../../scripts/dtc/libfdtI still think we should not duplicate this but instead change the header files to make the include hack unnecessary in all those places.
Looking at the header file, it only contains
#include <
linux/libfdt_env.h>
#include "../../scripts/dtc/libfdt/fdt.h"
#include "../../scripts/dtc/libfdt/libfdt.h"so two of them get the headers from exactly that place, while the
libfdt_env.hfile contains the linux-specific implementation of a couple of functions instead of the baremetal implementation.It looks like the intention was that when you already include the linux version of the header, you no longer need the other one, but that assumption is broken by current implementation.
One way to solve this would be
scripts/dtc/libfdt/libfdt.h +1
-1@@ -51,7 +51,7 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <libfdt_env.h> +#include "libfdt_env.h" #include <fdt.h>
Quoted 9 lines
Quoted 5 lines
scripts/dtc/libfdt/libfdt.h +0
-0#define FDT_FIRST_SUPPORTED_VERSION 0x10but I'm not sure if that is the best way.
I was hoping to limit the scope of my changes to mvebu code. But I can give it a whirl. I agree that propagating the hack isn't ideal.
Well I've sent something up to the devicetree/LKML so we'll see where that goes.
Another alternative I that is not without precedence is to add a of_fdt_xxx function (like of_fdt_limit_memory) to drivers/of/fdt.c which already uses a custom CFLAGS.
Quoted 34 lines
Quoted 20 lines
Quoted 12 lines
Quoted 11 lines
Quoted 6 lines
I think it would be good to first check whether you are running on Armada XP
or Armada 370, because the latter does not require this.Arnd
Any suggestion as to what to check for. Based on the dts files in the current source seem compatible == "marvell,armada370" seems like a good choice. But I don't know for sure that there isn't a armada-xp variant out using a .dts with this set.
Quoted 7 lines
Adding the following at the top will do the trick (assuming "marvell,armada370" is the right compatible string to use).
unsigned long root = of_get_flat_dt_root(); if (of_flat_dt_is_compatible(root, "marvell,armada370")) return;Quoted 8 lines
Right. You could also add
if (!IS_ENABLED(CONFIG_SMP) || !of_flat_dt_is_compatible(root,"marvell,armadaxp")))
to again eliminate that check for uniprocessor kernels, and to not propagate the fixup to future machines but make it clear that it is only needed for armadaxp.
OK makes sense.
Quoted 3 lines
Arnd
PATCH 1/4 ARM: mvebu: use dt_fixup to provide fallback for enable-method
Quoted 3 lines · On Tuesday 16 December 2014 02:21:16 Chris Packham wrote:
Well I've sent something up to the devicetree/LKML so we'll see where that goes.
Ok, thanks!
Quoted 3 lines
Another alternative I that is not without precedence is to add a
of_fdt_xxxfunction (likeof_fdt_limit_memory) todrivers/of/fdt.cwhich already uses a custom CFLAGS.
Agreed, that would work too.
Arnd
PATCH 1/1 dtc: Use quotes to include header files
In reply to Arnd BergmannCurrently in arch and driver code that needs early access to the flattened device tree it is necessary to add specific CFLAGS so that when scripts/dtc/libfdt/libfdt.h is included the C preprocessor is able to locate the libfdt versions of libfdt_env.h and fdt.h without generating an error.
We already provide an alternative linux-specific
version of libfdt_env.h and directly include scripts/dtc/libfdt/fdt.h so the inclusion by scripts/dtc/libfdt/libfdt.h is a no-op thanks to the inclusion guards.
By using quotes in scripts/dtc/libfdt/libfdt.h it picks up fdt.h and libfdt_env.h from the source directory without needing to add CFLAGS for the sources that happen to include linux/libfdt.h.
---
Hi,
This probably should come via git://git.jdl.com/software/dtc.git however this appears to be inaccessible at the moment. Is this still the canonical source for the device tree compiler and libfdt or has it been moved? How much deviation from the canonical source are we prepared to live with in the kernel?
This came up on the arm-LKML[1]. Basically in the name of backwards compatibility I need to add a .dt_fixup to add some required nodes to the flattened device tree prior to the tree being un-flattened and processed. This is a trick powerpc makes use of fairly extensively and there are a few other instances of this.
For the files that include linux/libfdt.h we currently also have to specify additional CFLAGS to satisfy the CPP.
$ git grep '<linux/libfdt.h>' arch/mips/cavium-octeon/octeon-platform.c:#include <linux/libfdt.h> arch/mips/cavium-octeon/setup.c:#include <linux/libfdt.h> arch/mips/mti-sead3/sead3-setup.c:#include <linux/libfdt.h> arch/powerpc/kernel/prom.c:#include <linux/libfdt.h> drivers/firmware/efi/libstub/fdt.c:#include <linux/libfdt.h> drivers/of/fdt.c:#include <linux/libfdt.h> drivers/of/fdt_address.c:#include <linux/libfdt.h>
$ git grep -e '-I.*dtc/libfdt' arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o = -I$(src)/../../../scripts/dtc/libfdt arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/powerpc/kernel/Makefile:CFLAGS_prom.o = -I$(src)/../../../scripts/dtc/libfdt drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt drivers/of/Makefile:CFLAGS_fdt_address.o = -I$(src)/../../scripts/dtc/libfdt lib/Makefile: $(eval CFLAGS_$(file) = -I$(src)/../scripts/dtc/libfdt))
Simply by switching to using quotes we can avoid having this extra step.
Assuming that this patch is acceptable a follow on clean up would be to remove the instances of CFLAGS_... listed above.
Regards,
Chris
--
[1] - http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310840.html
scripts/dtc/libfdt/libfdt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
scripts/dtc/libfdt/libfdt.h +2 -2
@@ -51,8 +51,8 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <libfdt_env.h> -#include <fdt.h> +#include "libfdt_env.h" +#include "fdt.h" #define FDT_FIRST_SUPPORTED_VERSION 0x10#define FDT_LAST_SUPPORTED_VERSION 0x11
Signature · 1 line
2.2.0.rc0
Re: [PATCH] dtc: Use quotes to include header files
Quoted 7 lines · Chris Packham
Hi,
This probably should come via
git://git.jdl.com/software/dtc.githowever this appears to be inaccessible at the moment. Is this still the canonical source for the device tree compiler and libfdt or has it been moved? How much deviation from the canonical source are we prepared to live with in the kernel?
That is because the official DTC repository has moved to git.kernel.org!
Quoted 11 lines
Currently in arch and driver code that needs early access to the flattened device tree it is necessary to add specific CFLAGS so that when
scripts/dtc/libfdt/libfdt.his included the C preprocessor is able to locate the libfdt versions oflibfdt_env.handfdt.hwithout generating an error.We already provide an alternative linux-specific
version oflibfdt_env.hand directly includescripts/dtc/libfdt/fdt.hso the inclusion byscripts/dtc/libfdt/libfdt.his a no-op thanks to the inclusion guards.
Quoted 6 lines
By using quotes in
scripts/dtc/libfdt/libfdt.hit picks upfdt.handlibfdt_env.hfrom the source directory without needing to add CFLAGS for the sources that happen to includelinux/libfdt.h.Signed-off-by Chris Packham <[email protected]>---
and:
Quoted 5 lines
scripts/dtc/libfdt/libfdt.h +0
-0
scripts/dtc/libfdt/libfdt.h line 58
Quoted 10 lines
scripts/dtc/libfdt/libfdt.h +2
-2@@ -51,8 +51,8 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <libfdt_env.h> -#include <fdt.h> +#include "libfdt_env.h" +#include "fdt.h" #define FDT_FIRST_SUPPORTED_VERSION 0x10#define FDT_LAST_SUPPORTED_VERSION 0x11
Hmm. I seem to recall that the use of angle brackets was intentional. I believe that was predicated on installing the FDT library on some target machine and needing it to be able to find the other header files in an installation, not source, directory.
Minor repository spelunking suggests that the angle brackets have been around since Day Zero:
commit 3da0f9a10dfa9b615d06c350c7b9fe29f360a6e0
David?
HTH?,
jdl
Re: [PATCH] dtc: Use quotes to include header files
In reply to Chris PackhamQuoted 71 lines · Chris Packham
Currently in arch and driver code that needs early access to the flattened device tree it is necessary to add specific CFLAGS so that when
scripts/dtc/libfdt/libfdt.his included the C preprocessor is able to locate the libfdt versions oflibfdt_env.handfdt.hwithout generating an error.We already provide an alternative linux-specific
version oflibfdt_env.hand directly includescripts/dtc/libfdt/fdt.hso the inclusion byscripts/dtc/libfdt/libfdt.his a no-op thanks to the inclusion guards.By using quotes in
scripts/dtc/libfdt/libfdt.hit picks upfdt.handlibfdt_env.hfrom the source directory without needing to add CFLAGS for the sources that happen to includelinux/libfdt.h.Signed-off-by Chris Packham <[email protected]>---
Hi,This probably should come via
git://git.jdl.com/software/dtc.githowever this appears to be inaccessible at the moment. Is this still the canonical source for the device tree compiler and libfdt or has it been moved? How much deviation from the canonical source are we prepared to live with in the kernel?This came up on the arm-LKML[1]. Basically in the name of backwards compatibility I need to add a .dt_fixup to add some required nodes to the flattened device tree prior to the tree being un-flattened and processed. This is a trick powerpc makes use of fairly extensively and there are a few other instances of this.
For the files that include
linux/libfdt.hwe currently also have to specify additional CFLAGS to satisfy the CPP.$ git grep '<linux/libfdt.h>' arch/mips/cavium-octeon/octeon-platform.c:#include <linux/libfdt.h> arch/mips/cavium-octeon/setup.c:#include <linux/libfdt.h> arch/mips/mti-sead3/sead3-setup.c:#include <linux/libfdt.h> arch/powerpc/kernel/prom.c:#include <linux/libfdt.h> drivers/firmware/efi/libstub/fdt.c:#include <linux/libfdt.h> drivers/of/fdt.c:#include <linux/libfdt.h> drivers/of/fdt_address.c:#include <linux/libfdt.h>$ git grep -e '-I.*dtc/libfdt' arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o = -I$(src)/../../../scripts/dtc/libfdt arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/powerpc/kernel/Makefile:CFLAGS_prom.o = -I$(src)/../../../scripts/dtc/libfdt drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt drivers/of/Makefile:CFLAGS_fdt_address.o = -I$(src)/../../scripts/dtc/libfdt lib/Makefile: $(eval CFLAGS_$(file) = -I$(src)/../scripts/dtc/libfdt))Simply by switching to using quotes we can avoid having this extra step.
Assuming that this patch is acceptable a follow on clean up would be to remove the instances of
CFLAGS_... listed above.Regards,
Chris
--
[1] - http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310840.htmlscripts/dtc/libfdt/libfdt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)scripts/dtc/libfdt/libfdt.h +0
-0
scripts/dtc/libfdt/libfdt.h line 55
Quoted 7 lines
scripts/dtc/libfdt/libfdt.h +2
-2@@ -51,8 +51,8 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <libfdt_env.h> -#include <fdt.h> +#include "libfdt_env.h" +#include "fdt.h"
Until this is resolved with upstream DTC, what about adding dummy libfdt.h, libfdt_env.h and fdt.h to include/linux? That would get this out of the blocker path for merging this code.
g.
Re: [PATCH] dtc: Use quotes to include header files
Quoted 8 lines · Grant Likely
Quoted 5 lines · Chris Packham
Currently in arch and driver code that needs early access to the flattened device tree it is necessary to add specific CFLAGS so that when
scripts/dtc/libfdt/libfdt.his included the C preprocessor is able to locate the libfdt versions oflibfdt_env.handfdt.hwithout generating an error.
[...]
Quoted 8 lines
Quoted 8 lines
scripts/dtc/libfdt/libfdt.h +0
-0@@ -51,8 +51,8 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
Quoted 8 lines
scripts/dtc/libfdt/libfdt.h line 55
Quoted 4 lines
scripts/dtc/libfdt/libfdt.h +2
-2-#include <libfdt_env.h> -#include <fdt.h> +#include "libfdt_env.h" +#include "fdt.h" Until this is resolved with upstream DTC, what about adding dummy
libfdt.h,libfdt_env.handfdt.htoinclude/linux? That would get this out of the blocker path for merging this code.
2 of those already exist. Is it only a matter of adding fdt.h or is there more to it?
This could also be carried as part of the import script to fix up.
Rob
Re: [PATCH] dtc: Use quotes to include header files
In reply to Grant LikelyQuoted 86 lines · Grant Likely
Quoted 71 lines · Chris Packham
Currently in arch and driver code that needs early access to the flattened device tree it is necessary to add specific CFLAGS so that when
scripts/dtc/libfdt/libfdt.his included the C preprocessor is able to locate the libfdt versions oflibfdt_env.handfdt.hwithout generating an error.We already provide an alternative linux-specific
version oflibfdt_env.hand directly includescripts/dtc/libfdt/fdt.hso the inclusion byscripts/dtc/libfdt/libfdt.his a no-op thanks to the inclusion guards.By using quotes in
scripts/dtc/libfdt/libfdt.hit picks upfdt.handlibfdt_env.hfrom the source directory without needing to add CFLAGS for the sources that happen to includelinux/libfdt.h.Signed-off-by Chris Packham <[email protected]>---
Hi,This probably should come via
git://git.jdl.com/software/dtc.githowever this appears to be inaccessible at the moment. Is this still the canonical source for the device tree compiler and libfdt or has it been moved? How much deviation from the canonical source are we prepared to live with in the kernel?This came up on the arm-LKML[1]. Basically in the name of backwards compatibility I need to add a .dt_fixup to add some required nodes to the flattened device tree prior to the tree being un-flattened and processed. This is a trick powerpc makes use of fairly extensively and there are a few other instances of this.
For the files that include
linux/libfdt.hwe currently also have to specify additional CFLAGS to satisfy the CPP.$ git grep '<linux/libfdt.h>' arch/mips/cavium-octeon/octeon-platform.c:#include <linux/libfdt.h> arch/mips/cavium-octeon/setup.c:#include <linux/libfdt.h> arch/mips/mti-sead3/sead3-setup.c:#include <linux/libfdt.h> arch/powerpc/kernel/prom.c:#include <linux/libfdt.h> drivers/firmware/efi/libstub/fdt.c:#include <linux/libfdt.h> drivers/of/fdt.c:#include <linux/libfdt.h> drivers/of/fdt_address.c:#include <linux/libfdt.h>$ git grep -e '-I.*dtc/libfdt' arch/mips/cavium-octeon/Makefile:CFLAGS_octeon-platform.o = -I$(src)/../../../scripts/dtc/libfdt arch/mips/cavium-octeon/Makefile:CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/mips/mti-sead3/Makefile:CFLAGS_sead3-setup.o = -I$(src)/../../../scripts/dtc/libfdt arch/powerpc/kernel/Makefile:CFLAGS_prom.o = -I$(src)/../../../scripts/dtc/libfdt drivers/firmware/efi/libstub/Makefile:CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ drivers/of/Makefile:CFLAGS_fdt.o = -I$(src)/../../scripts/dtc/libfdt drivers/of/Makefile:CFLAGS_fdt_address.o = -I$(src)/../../scripts/dtc/libfdt lib/Makefile: $(eval CFLAGS_$(file) = -I$(src)/../scripts/dtc/libfdt))Simply by switching to using quotes we can avoid having this extra step.
Assuming that this patch is acceptable a follow on clean up would be to remove the instances of
CFLAGS_... listed above.Regards,
Chris
--
[1] - http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310840.htmlscripts/dtc/libfdt/libfdt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)scripts/dtc/libfdt/libfdt.h +0
-0scripts/dtc/libfdt/libfdt.h line 55
Quoted 7 lines
scripts/dtc/libfdt/libfdt.h +2
-2@@ -51,8 +51,8 @@* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <libfdt_env.h> -#include <fdt.h> +#include "libfdt_env.h" +#include "fdt.h" Until this is resolved with upstream DTC, what about adding dummy
libfdt.h,libfdt_env.handfdt.htoinclude/linux? That would get this out of the blocker path for merging this code.
Quoted 2 lines
g.
I'm traveling right now but I could work up an example patch next week.
Quoted 1 line
From the other discussions I think there are arguments in favor of keeping
the angle brackets for the userspace libfdt usage. Any change in the kernel either adding dummy files in the system include path or performing a subsititution in an import script would probably be permanent.
PATCH 2/4 clk: mvebu: armada-xp: Support for 98DX4251
In reply to Chris PackhamThe 98DX4251 is one of a range of packet processors with integrated CPUs based on armada-xp. One difference is that the TCLK frequency is fixed at 200MHz as opposed to the fixed 250MHz used on armada-xp. Additionally the clock-gating options are a subset of what's available on the armada-xp.
---
As per Thomas' and Andrews comments I've made this specific to the 98DX4251 and separated out the gating definitions. It should be good for other variants of what Marvell call "MSYS" but I don't have access to any to test with.
v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/304387.html
.../devicetree/bindings/clock/mvebu-core-clock.txt | 4 +++ drivers/clk/mvebu/armada-xp.c | 35 ++++++++++++++++++++++ 2 files changed, 39 insertions(+)
Documentation/devicetree/bindings/clock/mvebu-core-clock.txt +4 -0
@@ -34,6 +34,9 @@ The following is a list of provided IDs and clock names on Orion5x: 1 = cpuclk (CPU0 clock) 2 = ddrclk (DDR controller clock derived from CPU0 clock) +The following is a list of provided IDs and clock names on MV98DX4251: + 0 = tclk (Internal Bus clock) + Required properties: - compatible : shall be one of the following: "marvell,armada-370-core-clock" - For Armada 370 SoC core clocks
@@ -46,6 +49,7 @@ Required properties: "marvell,mv88f5182-core-clock" - for Orion MV88F5182 SoC "marvell,mv88f5281-core-clock" - for Orion MV88F5281 SoC "marvell,mv88f6183-core-clock" - for Orion MV88F6183 SoC + "marvell,mv98dx4251-core-clock" - for MV98DX4251 SoC - reg : shall be the register address of the Sample-At-Reset (SAR) register - #clock-cells : from common clock binding; shall be set to 1
drivers/clk/mvebu/armada-xp.c +35 -0
@@ -52,6 +52,12 @@ static u32 __init axp_get_tclk_freq(void __iomem *sar)return 250000000 ; } +/* MV98DX4251 TCLK frequency is fixed to 200MHz */ +static u32 __init mv98dx4251_get_tclk_freq (void __iomem *sar) +{ +return 200000000 ; +} +static const u32 axp_cpu_freqs[]__initconst = {1000000000 ,1066000000 ,
@@ -158,6 +164,14 @@ static const struct coreclk_soc_desc axp_coreclks = { .num_ratios = ARRAY_SIZE(axp_coreclk_ratios), }; +static const struct coreclk_soc_desc mv98dx4251_coreclks = { + .get_tclk_freq = mv98dx4251_get_tclk_freq, + .get_cpu_freq = axp_get_cpu_freq, + .get_clk_ratio = axp_get_clk_ratio, + .ratios = axp_coreclk_ratios, + .num_ratios =ARRAY_SIZE (axp_coreclk_ratios), +}; +/* * Clock Gating Control */
@@ -195,6 +209,15 @@ static const struct clk_gating_soc_desc axp_gating_desc[] __initconst = { { } }; +static const struct clk_gating_soc_desc mv98dx4251_gating_desc[]__initconst = { + {"ge1" ,NULL ,3 ,0 }, + {"ge0" ,NULL ,4 ,0 }, + {"pex00" ,NULL ,5 ,0 }, + {"sdio" ,NULL ,17 ,0 }, + {"xor0" ,NULL ,22 ,0 }, + { } +}; +static void __init axp_clk_init (struct device_node *np) { struct device_node *cgnp =
@@ -206,3 +229,15 @@ static void __init axp_clk_init(struct device_node *np)mvebu_clk_gating_setup (cgnp, axp_gating_desc); }CLK_OF_DECLARE (axp_clk,"marvell,armada-xp-core-clock" , axp_clk_init); + +static void __init mv98dx4251_clk_init (struct device_node *np) +{ +struct device_node *cgnp = +of_find_compatible_node (NULL ,NULL ,"marvell,armada-xp-gating-clock" ); + +mvebu_coreclk_setup (np, &mv98dx4251_coreclks); + +if (cgnp) +mvebu_clk_gating_setup (cgnp, mv98dx4251_gating_desc); +} +CLK_OF_DECLARE (mv98dx4251_clk,"marvell,mv98dx4251-core-clock" , mv98dx4251_clk_init);
Signature · 1 line
2.2.0.rc0
PATCH 3/4 ARM: mvebu: Initial support for rd-dxbc2
In reply to Chris PackhamThe 98DX4251 Control and Management subsystem is a feature reduction derivative of the ARMADA XP with some functional changes.
The following table highlights differences between the ARMADA XP MV78230 and the Control and Management subsystem. There is a complete table in the "Control and Management Subsystem Functional Specification" .
Feature Bobcat2 MV78230 ------- ------- ------- CPU Core (ARMv7 compliant Dual CPU @ up Dual CPU @ up with FPU) to 800 MHz to 1.6 GHz
L2 Cache 2MB 1MB
PCIe 1 x1 1 x4 or 4 x1
1 x1
XOR DMA 2 Channels 4 Channels
SPI interface 1 Port 2 Ports
---
I'm not sure how much of a blurb people want about the new platform. I'd just be repeating what's in Marvell's datasheets anyway.
I deliberately left the copyright notice in rd-dxbc2.dts since it was created by copying armada-xp-db.dts. And I don't think anything I did is worth asserting a different copyright. At the moment the mvebu-soc-id.h definition is not strictly needed for anything but hopefully Marvell will come to the party and add the other SoCs.
arch/arm/boot/dts/rd-dxbc2.dts | 109 +++++++++++++++++++++++++++++++++++++ arch/arm/mach-mvebu/mvebu-soc-id.h | 3 + 2 files changed, 112 insertions(+) create mode 100644 arch/arm/boot/dts/rd-dxbc2.dts
arch/arm/boot/dts/rd-dxbc2.dts new file +109 -0
@@ -0,0 +1,109 @@ +/* + * Device Tree file for RD-DXBC2 board + * + * Copyright (C) 2012-2014 Marvell + * + * Lior Amsalem <[email protected]> + * Gregory CLEMENT <[email protected]> + * Thomas Petazzoni <[email protected]> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + * + * Note: this Device Tree assumes that the bootloader has remapped the + * internal registers to 0xf1000000 (instead of the default + * 0xd0000000). The 0xf1000000 is the default used by the recent, + * DT-capable, U-Boot bootloaders provided by Marvell. Some earlier + * boards were delivered with an older version of the bootloader that + * left internal registers mapped at 0xd0000000. If you are in this + * situation, you should either update your bootloader (preferred + * solution) or the below Device Tree should be adjusted. + */ + +/dts-v1/; +#include "armada-xp-mv78260.dtsi" + +/ { + model = "Marvell Bobcat2 Evaluation Board"; + compatible = "marvell,axp-db", "marvell,armadaxp-mv78260", "marvell,armadaxp", "marvell,armada-370-xp"; + + chosen { + bootargs = "console=ttyS0,115200 earlyprintk"; + }; + + memory { + device_type = "memory"; + reg = <0 0x00000000 0 0x20000000>; /* 512 MB */ + }; + + soc { + ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 + MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000 + MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000>; + + devbus-bootcs { + status = "okay"; + + /* Device Bus parameters are required */ + + /* Read parameters */ + devbus,bus-width = <16>; + devbus,turn-off-ps = <60000>; + devbus,badr-skew-ps = <0>; + devbus,acc-first-ps = <124000>; + devbus,acc-next-ps = <248000>; + devbus,rd-setup-ps = <0>; + devbus,rd-hold-ps = <0>; + + /* Write parameters */ + devbus,sync-enable = <0>; + devbus,wr-high-ps = <60000>; + devbus,wr-low-ps = <60000>; + devbus,ale-wr-ps = <60000>; + }; + + internal-regs { + serial at 12000 { + status = "okay"; + }; + serial at 12100 { + status = "okay"; + }; + + coreclk: mvebu-sar at 18230 { + compatible = "marvell,mv98dx4251-core-clock"; + }; + + i2c at 11000 { + compatible = "marvell,mv64xxx-i2c"; + clock-frequency = <400000>; + status = "okay"; + }; + + mvsdio at d4000 { + pinctrl-0 = <&sdio_pins>; + pinctrl-names = "default"; + status = "okay"; + /* No CD or WP GPIOs */ + broken-cd; + }; + + spi0: spi at 10600 { + status = "okay"; + + spi-flash at 0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "m25p64"; + reg = <0>; /* Chip select 0 */ + spi-max-frequency = <20000000>; + }; + }; + + xor at f0900 { + status = "disabled"; + }; + }; + }; +};
arch/arm/mach-mvebu/mvebu-soc-id.h +3 -0
@@ -24,6 +24,9 @@#define ARMADA_375_Z1_REV 0x0#define ARMADA_375_A0_REV 0x3 +/* Packet Processors with integrated CPU */ +#define MV98DX4251 0xFC00 +#ifdef CONFIG_ARCH_MVEBU int mvebu_get_soc_id (u32 *dev,u32 *rev);#else
Signature · 1 line
2.2.0.rc0
PATCH 4/4 ARM: mvebu: Custom smp_ops for 98DX4251
In reply to Chris PackhamCompared to the armada-xp the 98DX4251 uses different registers to set the boot address for the secondary CPU. This seems to contradict the datasheet which lists the same PMSU registers. This could be an error in the datasheet since it is likely to reproduce much of the armada-xp information. Or it could be an artifact of the early silicon revision being used.
---
I wasn't sure about the name "pmsu-98dx4251" originally I called this "msys" but following previous conversations on the subject I've opted to go with something specific to the chip I'm dealing with. I could fold this into pmsu.c so it was alongside mvebu_pmsu_set_cpu_boot_addr if people think that would be better.
arch/arm/boot/dts/rd-dxbc2.dts | 9 +++++ arch/arm/mach-mvebu/Makefile | 1 + arch/arm/mach-mvebu/common.h | 2 ++ arch/arm/mach-mvebu/platsmp.c | 28 +++++++++++++++ arch/arm/mach-mvebu/pmsu-98dx4251.c | 68 +++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+) create mode 100644 arch/arm/mach-mvebu/pmsu-98dx4251.c
arch/arm/boot/dts/rd-dxbc2.dts +9 -0
@@ -28,6 +28,10 @@ model = "Marvell Bobcat2 Evaluation Board"; compatible = "marvell,axp-db", "marvell,armadaxp-mv78260", "marvell,armadaxp", "marvell,armada-370-xp"; + cpus { + enable-method = "marvell,98dx4251-smp"; + }; + chosen { bootargs = "console=ttyS0,115200 earlyprintk"; };
@@ -104,6 +108,11 @@ xor at f0900 { status = "disabled"; }; + + resume at 20980 { + compatible = "marvell,98dx4251-resume-ctrl"; + reg = <0x20980 0x10>; + }; }; }; };
arch/arm/mach-mvebu/Makefile +1 -0
@@ -8,6 +8,7 @@ obj-$(CONFIG_MACH_MVEBU_ANY) += system-controller.o mvebu-soc-id.o ifeq ($(CONFIG_MACH_MVEBU_V7),y) obj-y += cpu-reset.o board-v7.o coherency.o coherency_ll.o pmsu.o pmsu_ll.o +obj-y += pmsu-98dx4251.o obj-$(CONFIG_SMP) += platsmp.o headsmp.o platsmp-a9.o headsmp-a9.o endif
arch/arm/mach-mvebu/common.h +2 -0
@@ -25,4 +25,6 @@ int mvebu_system_controller_get_soc_id(u32 *dev, u32 *rev);void __iomem *mvebu_get_scu_base (void ); +void mv98dx4251_resume_set_cpu_boot_addr (int hw_cpu,void *boot_addr); +#endif
arch/arm/mach-mvebu/platsmp.c +28 -0
@@ -180,5 +180,33 @@ struct smp_operations armada_xp_smp_ops __initdata = {#endif }; +static int mv98dx4251_boot_secondary (unsigned int cpu,struct task_struct *idle) +{ +int ret, hw_cpu; + +pr_info ("Booting CPU %d\n" , cpu); + + hw_cpu =cpu_logical_map (cpu); +mv98dx4251_resume_set_cpu_boot_addr (hw_cpu, armada_xp_secondary_startup); + ret =mvebu_cpu_reset_deassert (hw_cpu); +if (ret) { +pr_warn ("unable to boot CPU: %d\n" , ret); +return ret; + } + +return 0 ; +} + +struct smp_operations mv98dx4251_smp_ops__initdata = { + .smp_init_cpus = armada_xp_smp_init_cpus, + .smp_prepare_cpus = armada_xp_smp_prepare_cpus, + .smp_boot_secondary = mv98dx4251_boot_secondary, +#ifdef CONFIG_HOTPLUG_CPU + .cpu_die = armada_xp_cpu_die, +#endif +}; +CPU_METHOD_OF_DECLARE (armada_xp_smp,"marvell,armada-xp-smp" , &armada_xp_smp_ops); +CPU_METHOD_OF_DECLARE (mv98dx4251_smp,"marvell,98dx4251-smp" , + &mv98dx4251_smp_ops);
arch/arm/mach-mvebu/pmsu-98dx4251.c new file +68 -0
68 lines not on this page · raw message
Signature · 1 line
2.2.0.rc0