[RFC/PATCH 0/4] Add support for Marvell's RD-DXBC2

[RFC/PATCH 0/4] Add support for Marvell's RD-DXBC2

[Chris Packham]

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

-- 
2.2.0.rc0

[RFC/PATCH 1/4] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Chris Packham]

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.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

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(-)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index d0d39f1..ce23c6a 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -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_END
 
 static const char * const armada_375_dt_compat[] = {
-- 
2.2.0.rc0

[RFC/PATCH 2/4] clk: mvebu: armada-xp: Support for 98DX4251

[Chris Packham]

The 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.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

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(+)

diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index dc5ea5b..503849e 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -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
 
diff --git a/drivers/clk/mvebu/armada-xp.c b/drivers/clk/mvebu/armada-xp.c
index b309431..eb17817 100644
--- a/drivers/clk/mvebu/armada-xp.c
+++ b/drivers/clk/mvebu/armada-xp.c
@@ -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);
-- 
2.2.0.rc0

[RFC/PATCH 3/4] ARM: mvebu: Initial support for rd-dxbc2

[Chris Packham]

The 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

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

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

diff --git a/arch/arm/boot/dts/rd-dxbc2.dts b/arch/arm/boot/dts/rd-dxbc2.dts
new file mode 100644
index 0000000..97a72d4
--- /dev/null
+++ b/arch/arm/boot/dts/rd-dxbc2.dts
@@ -0,0 +1,109 @@
+/*
+ * Device Tree file for RD-DXBC2 board
+ *
+ * Copyright (C) 2012-2014 Marvell
+ *
+ * Lior Amsalem <alior@marvell.com>
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * 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";
+			};
+		};
+	};
+};
diff --git a/arch/arm/mach-mvebu/mvebu-soc-id.h b/arch/arm/mach-mvebu/mvebu-soc-id.h
index c16bb68..98a9dca 100644
--- a/arch/arm/mach-mvebu/mvebu-soc-id.h
+++ b/arch/arm/mach-mvebu/mvebu-soc-id.h
@@ -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
-- 
2.2.0.rc0

[RFC/PATCH 4/4] ARM: mvebu: Custom smp_ops for 98DX4251

[Chris Packham]

Compared 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.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

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

diff --git a/arch/arm/boot/dts/rd-dxbc2.dts b/arch/arm/boot/dts/rd-dxbc2.dts
index 97a72d4..5763c4f 100644
--- a/arch/arm/boot/dts/rd-dxbc2.dts
+++ b/arch/arm/boot/dts/rd-dxbc2.dts
@@ -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>;
+			};
 		};
 	};
 };
diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
index e24136b..866822e 100644
--- a/arch/arm/mach-mvebu/Makefile
+++ b/arch/arm/mach-mvebu/Makefile
@@ -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
 
diff --git a/arch/arm/mach-mvebu/common.h b/arch/arm/mach-mvebu/common.h
index 3ccb40c..5064ac5 100644
--- a/arch/arm/mach-mvebu/common.h
+++ b/arch/arm/mach-mvebu/common.h
@@ -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
diff --git a/arch/arm/mach-mvebu/platsmp.c b/arch/arm/mach-mvebu/platsmp.c
index 895dc37..2a2ed65d 100644
--- a/arch/arm/mach-mvebu/platsmp.c
+++ b/arch/arm/mach-mvebu/platsmp.c
@@ -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);
diff --git a/arch/arm/mach-mvebu/pmsu-98dx4251.c b/arch/arm/mach-mvebu/pmsu-98dx4251.c
new file mode 100644
index 0000000..e4d3ad46
--- /dev/null
+++ b/arch/arm/mach-mvebu/pmsu-98dx4251.c
@@ -0,0 +1,68 @@
+/**
+ * CPU resume support for 98DX4521 internal CPU (a.k.a. MSYS).
+ */
+
+#define pr_fmt(fmt) "mv98dx4251-resume: " fmt
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/of_address.h>
+#include <linux/io.h>
+#include "common.h"
+
+static void __iomem *mv98dx4251_resume_base;
+#define MV98DX4251_CPU_RESUME_CTRL_OFFSET	0x08
+#define MV98DX4251_CPU_RESUME_ADDR_OFFSET	0x04
+
+static struct of_device_id of_mv98dx4251_resume_table[] = {
+	{.compatible = "marvell,98dx4251-resume-ctrl",},
+	{ /* end of list */ },
+};
+
+void mv98dx4251_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr)
+{
+	WARN_ON(hw_cpu != 1);
+
+	writel(0, mv98dx4251_resume_base + MV98DX4251_CPU_RESUME_CTRL_OFFSET);
+	writel(virt_to_phys(boot_addr), mv98dx4251_resume_base +
+	       MV98DX4251_CPU_RESUME_ADDR_OFFSET);
+}
+
+static int __init mv98dx4251_resume_init(void)
+{
+	struct device_node *np;
+	struct resource res;
+	int ret = 0;
+
+	np = of_find_matching_node(NULL, of_mv98dx4251_resume_table);
+	if (!np)
+		return 0;
+
+	pr_info("Initializing 98DX4521 Resume\n");
+
+	if (of_address_to_resource(np, 0, &res)) {
+		pr_err("unable to get resource\n");
+		ret = -ENOENT;
+		goto out;
+	}
+
+	if (!request_mem_region(res.start, resource_size(&res), np->full_name)) {
+		pr_err("unable to request region\n");
+		ret = -EBUSY;
+		goto out;
+	}
+
+	mv98dx4251_resume_base = ioremap(res.start, resource_size(&res));
+	if (!mv98dx4251_resume_base) {
+		pr_err("unable to map registers\n");
+		release_mem_region(res.start, resource_size(&res));
+		ret = -ENOMEM;
+		goto out;
+	}
+
+out:
+	of_node_put(np);
+	return ret;
+}
+
+early_initcall(mv98dx4251_resume_init);
-- 
2.2.0.rc0

[RFC/PATCH 1/4] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Arnd Bergmann]

On Wednesday 10 December 2014 15:39:44 Chris Packham wrote:
> 
> +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

[RFC/PATCH 1/4] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Chris Packham]

Hi Arnd,

On 12/10/2014 10:44 PM, Arnd Bergmann wrote:
> On Wednesday 10 December 2014 15:39:44 Chris Packham wrote:
>>
>> +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
>

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

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Chris Packham]

Hi Arnd,

> On 12/10/2014 10:44 PM, Arnd Bergmann wrote:
>> On Wednesday 10 December 2014 15:39:44 Chris Packham wrote:
>>>
>>> +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
>>
> 
> 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.

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.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/mach-mvebu/Makefile   |  2 ++
 arch/arm/mach-mvebu/board-v7.c | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
index e24136b..68310f8 100644
--- a/arch/arm/mach-mvebu/Makefile
+++ b/arch/arm/mach-mvebu/Makefile
@@ -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
\ No newline at end of file
diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index d0d39f1..0592c76 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -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_END
 
 static const char * const armada_375_dt_compat[] = {
-- 
2.2.0.rc0

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Arnd Bergmann]

On Friday 12 December 2014 17:53:55 Chris Packham wrote:
> > 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.
> 
> 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!

> ---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.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/arm/mach-mvebu/Makefile   |  2 ++
>  arch/arm/mach-mvebu/board-v7.c | 37 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
> index e24136b..68310f8 100644
> --- a/arch/arm/mach-mvebu/Makefile
> +++ b/arch/arm/mach-mvebu/Makefile
> @@ -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
> \ No newline at end of file

Why is this needed? Can't you just include <linux/libfdt.h> ?

> +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"));
> +}

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

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Chris Packham]

Hi Arnd,

On 12/13/2014 12:35 AM, Arnd Bergmann wrote:
> On Friday 12 December 2014 17:53:55 Chris Packham wrote:
>>> 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.
>>
>> 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!
>
>> ---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.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   arch/arm/mach-mvebu/Makefile   |  2 ++
>>   arch/arm/mach-mvebu/board-v7.c | 37 ++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
>> index e24136b..68310f8 100644
>> --- a/arch/arm/mach-mvebu/Makefile
>> +++ b/arch/arm/mach-mvebu/Makefile
>> @@ -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
>> \ No newline at end of file
>
> Why 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/libfdt

>> +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"));
>> +}
>
> 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.

Another option might be just to count the /cpu at n nodes.

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Chris Packham]

Answering my own question

On 12/15/2014 10:11 AM, Chris Packham wrote:
> Hi Arnd,
>
> On 12/13/2014 12:35 AM, Arnd Bergmann wrote:
>> On Friday 12 December 2014 17:53:55 Chris Packham wrote:
>>>> 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.
>>>
>>> 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!
>>
>>> ---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.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>   arch/arm/mach-mvebu/Makefile   |  2 ++
>>>   arch/arm/mach-mvebu/board-v7.c | 37
>>> ++++++++++++++++++++++++++++++++++++-
>>>   2 files changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
>>> index e24136b..68310f8 100644
>>> --- a/arch/arm/mach-mvebu/Makefile
>>> +++ b/arch/arm/mach-mvebu/Makefile
>>> @@ -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
>>> \ No newline at end of file
>>
>> Why 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/libfdt
>
>>> +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"));
>>> +}
>>
>> 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.

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;

>
> Another option might be just to count the /cpu at n nodes.

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Arnd Bergmann]

On Monday 15 December 2014 00:57:49 Chris Packham wrote:
> On 12/15/2014 10:11 AM, Chris Packham wrote:
> >>> +
> >>> +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdt
> >>> \ No newline at end of file
> >>
> >> Why 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/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

diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
index 73f49759a5e7..23c55baa8e18 100644
--- a/scripts/dtc/libfdt/libfdt.h
+++ b/scripts/dtc/libfdt/libfdt.h
@@ -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.

> >> 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.
> 
> 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

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Chris Packham]

On 12/15/2014 11:37 PM, Arnd Bergmann wrote:
> On Monday 15 December 2014 00:57:49 Chris Packham wrote:
>> On 12/15/2014 10:11 AM, Chris Packham wrote:
>>>>> +
>>>>> +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdt
>>>>> \ No newline at end of file
>>>>
>>>> Why 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/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
>
> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
> index 73f49759a5e7..23c55baa8e18 100644
> --- a/scripts/dtc/libfdt/libfdt.h
> +++ b/scripts/dtc/libfdt/libfdt.h
> @@ -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.
>

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.

>>>> 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.
>>
>> 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.

OK makes sense.

>
> 	Arnd
>

[PATCH] dtc: Use quotes to include header files

[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.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.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
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(-)

diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
index 73f4975..ea1ddcd 100644
--- a/scripts/dtc/libfdt/libfdt.h
+++ b/scripts/dtc/libfdt/libfdt.h
@@ -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
-- 
2.2.0.rc0

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Chris Packham]

On 12/16/2014 09:11 AM, Chris Packham wrote:
>
>
> On 12/15/2014 11:37 PM, Arnd Bergmann wrote:
>> On Monday 15 December 2014 00:57:49 Chris Packham wrote:
>>> On 12/15/2014 10:11 AM, Chris Packham wrote:
>>>>>> +
>>>>>> +CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdt
>>>>>> \ No newline at end of file
>>>>>
>>>>> Why 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/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
>>
>> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
>> index 73f49759a5e7..23c55baa8e18 100644
>> --- a/scripts/dtc/libfdt/libfdt.h
>> +++ b/scripts/dtc/libfdt/libfdt.h
>> @@ -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.
>>
>
> 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.

>>>>> 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.
>>>
>>> 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.
>
> OK makes sense.
>
>>
>>     Arnd
>>

[RFC/PATCH 1/4 PATCH] ARM: mvebu: use dt_fixup to provide fallback for enable-method

[Arnd Bergmann]

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!

> 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.

Agreed, that would work too.

	Arnd

[PATCH] dtc: Use quotes to include header files

[Jon Loeliger]

So, like, Chris Packham said:
> 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?

That is because the official DTC repository has moved to git.kernel.org!



> 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.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.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---

and:

> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
> index 73f4975..ea1ddcd 100644
> --- a/scripts/dtc/libfdt/libfdt.h
> +++ b/scripts/dtc/libfdt/libfdt.h
> @@ -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

[PATCH] dtc: Use quotes to include header files

[Grant Likely]

On Tue, 16 Dec 2014 15:13:24 +1300
, Chris Packham <chris.packham@alliedtelesis.co.nz>
 wrote:
> 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.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.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 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(-)
> 
> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
> index 73f4975..ea1ddcd 100644
> --- a/scripts/dtc/libfdt/libfdt.h
> +++ b/scripts/dtc/libfdt/libfdt.h
> @@ -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.

[PATCH] dtc: Use quotes to include header files

[Rob Herring]

On Thu, Jan 29, 2015 at 9:56 AM, Grant Likely <grant.likely@linaro.org> wrote:
> On Tue, 16 Dec 2014 15:13:24 +1300
> , Chris Packham <chris.packham@alliedtelesis.co.nz>
>  wrote:
>> 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.h is included the C preprocessor is able
>> to locate the libfdt versions of libfdt_env.h and fdt.h without
>> generating an error.

[...]

>> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
>> index 73f4975..ea1ddcd 100644
>> --- a/scripts/dtc/libfdt/libfdt.h
>> +++ b/scripts/dtc/libfdt/libfdt.h
>> @@ -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.

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

[PATCH] dtc: Use quotes to include header files

[Chris Packham]

On Thu, 29 Jan 2015, Grant Likely wrote:

> On Tue, 16 Dec 2014 15:13:24 +1300
> , Chris Packham <chris.packham@alliedtelesis.co.nz>
> wrote:
>> 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.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.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>> 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(-)
>>
>> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
>> index 73f4975..ea1ddcd 100644
>> --- a/scripts/dtc/libfdt/libfdt.h
>> +++ b/scripts/dtc/libfdt/libfdt.h
>> @@ -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.
>

I'm traveling right now but I could work up an example patch next week.

>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.