[PATCH v6 0/6] ARM: rockchip: add smp functionality

[PATCH v6 0/6] ARM: rockchip: add smp functionality

[Heiko Stübner]

This series enables the use of the additional cores on Rockchip
Cortex-A9 SoCs.

To achieve this, add the scu, the needed sram and power-management-unit.

Tested on both a BQ Curie2 (rk3066a / dual core) and
on a Radxa Rock (rk3188 / quad core).

changes since v5:
- Grant Likely liked it better to "specify reserved regions of the
  memory instead of the valid ranges", so go back to the mmio-sram-reserved
  property originally suggested by Rob Herring

changes since v4:
- rebase on top of the recent rk3188 board support
- implement suggestion from Matt Sealey in moving the sram-limit from
  marking reserved regions to marking available regions - hopefully
  I got the usage right 
- remove __CPUINIT as suggested by Fabio Estevam

changes since v3:
- address comments from Rob Herring:
  - split the gathering of the reserve-data into a separate loop
  - spelling and style fixes
- first patch only included for reference, already part of the
  char-misc git tree

changes since v2:
- rework the sram allocation following the suggestion from Philipp Zabel

changes since v1:
- add reserved block feature for mmio-sram, to not use two logical
  sram nodes
- the sram content is kept intact while the device is running, so
  copying the trampoline is only needed once


Heiko Stuebner (6):
  dt-bindings: sram: describe option to reserve parts of the memory
  misc: sram: implement mmio-sram-reserved option
  ARM: rockchip: add snoop-control-unit
  ARM: rockchip: add sram dt nodes and documentation
  ARM: rockchip: add power-management-unit
  ARM: rockchip: add smp bringup code

 .../devicetree/bindings/arm/rockchip/pmu.txt       |   16 ++
 .../devicetree/bindings/arm/rockchip/smp-sram.txt  |   23 +++
 Documentation/devicetree/bindings/misc/sram.txt    |    8 +
 arch/arm/boot/dts/rk3066a.dtsi                     |    6 +
 arch/arm/boot/dts/rk3188.dtsi                      |    6 +
 arch/arm/boot/dts/rk3xxx.dtsi                      |   10 +
 arch/arm/mach-rockchip/Kconfig                     |    1 +
 arch/arm/mach-rockchip/Makefile                    |    1 +
 arch/arm/mach-rockchip/core.h                      |   22 +++
 arch/arm/mach-rockchip/headsmp.S                   |   30 +++
 arch/arm/mach-rockchip/platsmp.c                   |  208 ++++++++++++++++++++
 arch/arm/mach-rockchip/rockchip.c                  |    2 +
 drivers/misc/sram.c                                |  118 ++++++++++-
 13 files changed, 443 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/pmu.txt
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt
 create mode 100644 arch/arm/mach-rockchip/core.h
 create mode 100644 arch/arm/mach-rockchip/headsmp.S
 create mode 100644 arch/arm/mach-rockchip/platsmp.c

-- 
1.7.10.4

[PATCH v6 1/6] dt-bindings: sram: describe option to reserve parts of the memory

[Heiko Stübner]

Some SoCs need parts of their sram for special purposes. So while being part
of the peripheral, it should not be part of the genpool controlling the sram.

Therefore add an option mmio-sram-reserved to keep arbitrary portions of the
sram from general usage.

Suggested-by: Rob Herring <robherring2@gmail.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>
---
 Documentation/devicetree/bindings/misc/sram.txt |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt
index 4d0a00e..09ee7a3 100644
--- a/Documentation/devicetree/bindings/misc/sram.txt
+++ b/Documentation/devicetree/bindings/misc/sram.txt
@@ -8,9 +8,17 @@ Required properties:
 
 - reg : SRAM iomem address range
 
+Optional properties:
+
+- mmio-sram-reserved: ordered list of reserved chunks inside the sram that
+  should not be used by the operating system.
+  Format is <base size>, <base size>, ...; with base being relative to the
+  reg property base.
+
 Example:
 
 sram: sram@5c000000 {
 	compatible = "mmio-sram";
 	reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */
+	mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */
 };
-- 
1.7.10.4

[PATCH v6 2/6] misc: sram: implement mmio-sram-reserved option

[Heiko Stübner]

This implements support for the mmio-sram-reserved option to keep the
genpool from using these areas.

Suggested-by: Rob Herring <robherring2@gmail.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>
---
 drivers/misc/sram.c |  118 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 110 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index afe66571..7fb60f3 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -36,13 +36,23 @@ struct sram_dev {
 	struct clk *clk;
 };
 
+struct sram_reserve {
+	unsigned long start;
+	unsigned long size;
+};
+
 static int sram_probe(struct platform_device *pdev)
 {
 	void __iomem *virt_base;
 	struct sram_dev *sram;
 	struct resource *res;
-	unsigned long size;
-	int ret;
+	unsigned long size, cur_start, cur_size;
+	const __be32 *reserved_list = NULL;
+	int reserved_size = 0;
+	struct sram_reserve *rblocks;
+	unsigned int nblocks;
+	int ret = 0;
+	int i;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	virt_base = devm_ioremap_resource(&pdev->dev, res);
@@ -65,19 +75,111 @@ static int sram_probe(struct platform_device *pdev)
 	if (!sram->pool)
 		return -ENOMEM;
 
-	ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base,
-				res->start, size, -1);
-	if (ret < 0) {
-		if (sram->clk)
-			clk_disable_unprepare(sram->clk);
-		return ret;
+	if (pdev->dev.of_node) {
+		reserved_list = of_get_property(pdev->dev.of_node,
+						"mmio-sram-reserved",
+						&reserved_size);
+		if (reserved_list) {
+			reserved_size /= sizeof(*reserved_list);
+			if (!reserved_size || reserved_size % 2) {
+				dev_warn(&pdev->dev, "wrong number of arguments in mmio-sram-reserved\n");
+				reserved_list = NULL;
+				reserved_size = 0;
+			}
+		}
+	}
+
+	/*
+	 * We need an additional block to mark the end of the memory region
+	 * after the reserved blocks from the dt are processed.
+	 */
+	nblocks = reserved_size / 2 + 1;
+	rblocks = kmalloc((nblocks) * sizeof(*rblocks), GFP_KERNEL);
+	if (!rblocks) {
+		ret = -ENOMEM;
+		goto err_alloc;
+	}
+
+	cur_start = 0;
+	for (i = 0; i < nblocks - 1; i++) {
+		rblocks[i].start = be32_to_cpu(*reserved_list++);
+		rblocks[i].size = be32_to_cpu(*reserved_list++);
+
+		if (rblocks[i].start < cur_start) {
+			dev_err(&pdev->dev,
+				"unsorted reserved list (0x%lx before current 0x%lx)\n",
+				rblocks[i].start, cur_start);
+			ret = -EINVAL;
+			goto err_chunks;
+		}
+
+		if (rblocks[i].start >= size) {
+			dev_err(&pdev->dev,
+				"reserved block at 0x%lx outside the sram size 0x%lx\n",
+				rblocks[i].start, size);
+			ret = -EINVAL;
+			goto err_chunks;
+		}
+
+		if (rblocks[i].start + rblocks[i].size > size) {
+			dev_warn(&pdev->dev,
+				 "reserved block at 0x%lx to large, trimming\n",
+				 rblocks[i].start);
+			rblocks[i].size = size - rblocks[i].start;
+		}
+
+		cur_start = rblocks[i].start + rblocks[i].size;
+
+		dev_dbg(&pdev->dev, "found reserved block 0x%lx-0x%lx\n",
+			rblocks[i].start,
+			rblocks[i].start + rblocks[i].size);
+	}
+
+	/* the last chunk marks the end of the region */
+	rblocks[nblocks - 1].start = size;
+	rblocks[nblocks - 1].size = 0;
+
+	cur_start = 0;
+	for (i = 0; i < nblocks; i++) {
+		/* current start is in a reserved block, so continue after it */
+		if (rblocks[i].start == cur_start) {
+			cur_start = rblocks[i].start + rblocks[i].size;
+			continue;
+		}
+
+		/*
+		 * allocate the space between the current starting
+		 * address and the following reserved block, or the
+		 * end of the region.
+		 */
+		cur_size = rblocks[i].start - cur_start;
+
+		dev_dbg(&pdev->dev, "adding chunk 0x%lx-0x%lx\n",
+			cur_start, cur_start + cur_size);
+		ret = gen_pool_add_virt(sram->pool,
+				(unsigned long)virt_base + cur_start,
+				res->start + cur_start, cur_size, -1);
+		if (ret < 0)
+			goto err_chunks;
+
+		/* next allocation after this reserved block */
+		cur_start = rblocks[i].start + rblocks[i].size;
 	}
 
+	kfree(rblocks);
+
 	platform_set_drvdata(pdev, sram);
 
 	dev_dbg(&pdev->dev, "SRAM pool: %ld KiB @ 0x%p\n", size / 1024, virt_base);
 
 	return 0;
+
+err_chunks:
+	kfree(rblocks);
+err_alloc:
+	if (sram->clk)
+		clk_disable_unprepare(sram->clk);
+	return ret;
 }
 
 static int sram_remove(struct platform_device *pdev)
-- 
1.7.10.4

[PATCH v6 3/6] ARM: rockchip: add snoop-control-unit

[Heiko Stübner]

This adds the device-node and config select to enable the
scu in all Rockchip Cortex-A9 SoCs.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>
---
 arch/arm/boot/dts/rk3xxx.dtsi  |    5 +++++
 arch/arm/mach-rockchip/Kconfig |    1 +
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index 0fcbcfd..0a3d5b1 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -26,6 +26,11 @@
 		compatible = "simple-bus";
 		ranges;
 
+		scu@1013c000 {
+			compatible = "arm,cortex-a9-scu";
+			reg = <0x1013c000 0x100>;
+		};
+
 		gic: interrupt-controller@1013d000 {
 			compatible = "arm,cortex-a9-gic";
 			interrupt-controller;
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index cf073de..412cbcf 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -5,6 +5,7 @@ config ARCH_ROCKCHIP
 	select ARCH_REQUIRE_GPIOLIB
 	select ARM_GIC
 	select CACHE_L2X0
+	select HAVE_ARM_SCU
 	select HAVE_ARM_TWD if SMP
 	select HAVE_SMP
 	select COMMON_CLK
-- 
1.7.10.4

[PATCH v6 4/6] ARM: rockchip: add sram dt nodes and documentation

[Heiko Stübner]

Add dt-nodes for the sram on rk3066 and rk3188 including the reserved section
needed for smp bringup.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>
---
 .../devicetree/bindings/arm/rockchip/smp-sram.txt  |   23 ++++++++++++++++++++
 arch/arm/boot/dts/rk3066a.dtsi                     |    6 +++++
 arch/arm/boot/dts/rk3188.dtsi                      |    6 +++++
 3 files changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt

diff --git a/Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt b/Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt
new file mode 100644
index 0000000..80c878e
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/rockchip/smp-sram.txt
@@ -0,0 +1,23 @@
+Rockchip SRAM for smp bringup:
+------------------------------
+
+Rockchip's smp-capable SoCs use the first part of the sram for the bringup
+of the cores. Once the core gets powered up it executes the code that is
+residing at the very beginning of the sram.
+
+Therefore a reserved section has to be added to the mmio-sram declaration.
+
+Required node properties:
+- compatible : should contain both "rockchip,rk3066-sram", "mmio-sram"
+  so that the smp code can select the correct sram node.
+
+The rest of the properties should follow the generic mmio-sram discription
+found in ../../misc/sram.txt
+
+Example:
+
+	sram: sram@10080000 {
+		compatible = "rockchip,rk3066-sram", "mmio-sram";
+		reg = <0x10080000 0x10000>;
+		mmio-sram-reserved = <0x0 0x50>;
+	};
diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
index be5d2b0..0dfb680 100644
--- a/arch/arm/boot/dts/rk3066a.dtsi
+++ b/arch/arm/boot/dts/rk3066a.dtsi
@@ -64,6 +64,12 @@
 			clock-names = "timer", "pclk";
 		};
 
+		sram: sram@10080000 {
+			compatible = "rockchip,rk3066-sram", "mmio-sram";
+			reg = <0x10080000 0x10000>;
+			mmio-sram-reserved = <0x0 0x50>;
+		};
+
 		pinctrl@20008000 {
 			compatible = "rockchip,rk3066a-pinctrl";
 			reg = <0x20008000 0x150>;
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 1a26b03..1fc7eda 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -60,6 +60,12 @@
 			interrupts = <GIC_PPI 13 0xf04>;
 		};
 
+		sram: sram@10080000 {
+			compatible = "rockchip,rk3066-sram", "mmio-sram";
+			reg = <0x10080000 0x8000>;
+			mmio-sram-reserved = <0x0 0x50>;
+		};
+
 		pinctrl@20008000 {
 			compatible = "rockchip,rk3188-pinctrl";
 			reg = <0x20008000 0xa0>,
-- 
1.7.10.4

[PATCH v6 5/6] ARM: rockchip: add power-management-unit

[Heiko Stübner]

The pmu is needed to bring up the cores during smp operations and later
also other system parts. Therefore add a node and documentation for it.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>
---
 Documentation/devicetree/bindings/arm/rockchip/pmu.txt |   16 ++++++++++++++++
 arch/arm/boot/dts/rk3xxx.dtsi                          |    5 +++++
 2 files changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/pmu.txt

diff --git a/Documentation/devicetree/bindings/arm/rockchip/pmu.txt b/Documentation/devicetree/bindings/arm/rockchip/pmu.txt
new file mode 100644
index 0000000..3ee9b42
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/rockchip/pmu.txt
@@ -0,0 +1,16 @@
+Rockchip power-management-unit:
+-------------------------------
+
+The pmu is used to turn off and on different power domains of the SoCs
+This includes the power to the CPU cores.
+
+Required node properties:
+- compatible value : = "rockchip,rk3066-pmu";
+- reg : physical base address and the size of the registers window
+
+Example:
+
+	pmu@20004000 {
+		compatible = "rockchip,rk3066-pmu";
+		reg = <0x20004000 0x100>;
+	};
diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index 0a3d5b1..26e5a96 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -31,6 +31,11 @@
 			reg = <0x1013c000 0x100>;
 		};
 
+		pmu@20004000 {
+			compatible = "rockchip,rk3066-pmu";
+			reg = <0x20004000 0x100>;
+		};
+
 		gic: interrupt-controller@1013d000 {
 			compatible = "arm,cortex-a9-gic";
 			interrupt-controller;
-- 
1.7.10.4

[PATCH v6 6/6] ARM: rockchip: add smp bringup code

[Heiko Stübner]

This adds the necessary smp-operations and startup code to use
additional cores on Rockchip SoCs.

We currently hog the power management unit in the smp code, as it is
necessary to control the power to the cpu core and nothing else is
currently using it, so a generic implementation can be done later.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>
---
 arch/arm/mach-rockchip/Makefile   |    1 +
 arch/arm/mach-rockchip/core.h     |   22 ++++
 arch/arm/mach-rockchip/headsmp.S  |   30 ++++++
 arch/arm/mach-rockchip/platsmp.c  |  208 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-rockchip/rockchip.c |    2 +
 5 files changed, 263 insertions(+)
 create mode 100644 arch/arm/mach-rockchip/core.h
 create mode 100644 arch/arm/mach-rockchip/headsmp.S
 create mode 100644 arch/arm/mach-rockchip/platsmp.c

diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index 1547d4f..4377a14 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
+obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/mach-rockchip/core.h b/arch/arm/mach-rockchip/core.h
new file mode 100644
index 0000000..e2e7c9d
--- /dev/null
+++ b/arch/arm/mach-rockchip/core.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2013 MundoReader S.L.
+ * Author: Heiko Stuebner <heiko@sntech.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+extern char rockchip_secondary_trampoline;
+extern char rockchip_secondary_trampoline_end;
+
+extern unsigned long rockchip_boot_fn;
+extern void rockchip_secondary_startup(void);
+
+extern struct smp_operations rockchip_smp_ops;
diff --git a/arch/arm/mach-rockchip/headsmp.S b/arch/arm/mach-rockchip/headsmp.S
new file mode 100644
index 0000000..73206e3
--- /dev/null
+++ b/arch/arm/mach-rockchip/headsmp.S
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2013 MundoReader S.L.
+ * Author: Heiko Stuebner <heiko@sntech.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+ENTRY(rockchip_secondary_startup)
+	bl	v7_invalidate_l1
+	b	secondary_startup
+ENDPROC(rockchip_secondary_startup)
+
+ENTRY(rockchip_secondary_trampoline)
+	ldr	pc, 1f
+ENDPROC(rockchip_secondary_trampoline)
+	.globl	rockchip_boot_fn
+rockchip_boot_fn:
+1:	.space	4
+
+ENTRY(rockchip_secondary_trampoline_end)
diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
new file mode 100644
index 0000000..3459b17
--- /dev/null
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright (c) 2013 MundoReader S.L.
+ * Author: Heiko Stuebner <heiko@sntech.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/smp.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include <asm/cacheflush.h>
+#include <asm/smp_scu.h>
+#include <asm/smp_plat.h>
+#include <asm/mach/map.h>
+
+#include "core.h"
+
+static void __iomem *scu_base_addr;
+static void __iomem *sram_base_addr;
+static int ncores;
+
+#define PMU_PWRDN_CON		0x08
+#define PMU_PWRDN_ST		0x0c
+
+#define PMU_PWRDN_SCU		4
+
+static void __iomem *pmu_base_addr;
+
+static inline bool pmu_power_domain_is_on(int pd)
+{
+	return !(readl_relaxed(pmu_base_addr + PMU_PWRDN_ST) & BIT(pd));
+}
+
+static void pmu_set_power_domain(int pd, bool on)
+{
+	u32 val = readl_relaxed(pmu_base_addr + PMU_PWRDN_CON);
+	if (on)
+		val &= ~BIT(pd);
+	else
+		val |=  BIT(pd);
+	writel(val, pmu_base_addr + PMU_PWRDN_CON);
+
+	while (pmu_power_domain_is_on(pd) != on) { }
+}
+
+/*
+ * Handling of CPU cores
+ */
+
+static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
+					     struct task_struct *idle)
+{
+	if (!sram_base_addr || !pmu_base_addr) {
+		pr_err("%s: sram or pmu missing for cpu boot\n", __func__);
+		return -ENXIO;
+	}
+
+	if (cpu >= ncores) {
+		pr_err("%s: cpu %d outside maximum number of cpus %d\n",
+							__func__, cpu, ncores);
+		return -ENXIO;
+	}
+
+	/* start the core */
+	pmu_set_power_domain(0 + cpu, true);
+
+	return 0;
+}
+
+/**
+ * rockchip_smp_prepare_sram - populate necessary sram block
+ * Starting cores execute the code residing at the start of the on-chip sram
+ * after power-on. Therefore make sure, this sram region is reserved and
+ * big enough. After this check, copy the trampoline code that directs the
+ * core to the real startup code in ram into the sram-region.
+ * @node: mmio-sram device node
+ */
+static int __init rockchip_smp_prepare_sram(struct device_node *node)
+{
+	unsigned int trampoline_sz = &rockchip_secondary_trampoline_end -
+					    &rockchip_secondary_trampoline;
+	const __be32 *reserved_list = NULL;
+	int reserved_size;
+	int rstart = -1;
+	unsigned int rsize;
+	unsigned int i;
+
+	reserved_list = of_get_property(node, "mmio-sram-reserved",
+					&reserved_size);
+	if (!reserved_list) {
+		pr_err("%s: wrong number of arguments in mmio-sram-reserved\n",
+		       __func__);
+		return -ENOENT;
+	}
+
+	reserved_size /= sizeof(*reserved_list);
+	if (!reserved_size || reserved_size % 2) {
+		pr_err("%s: wrong number of arguments in mmio-sram-reserved\n",
+		       __func__);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < reserved_size; i += 2) {
+		/* get the next reserved block */
+		rstart = be32_to_cpu(*reserved_list++);
+		rsize = be32_to_cpu(*reserved_list++);
+
+		if (!rstart)
+			break;
+	}
+
+	if (rstart) {
+		pr_err("%s: start of sram is not reserved from mmio-sram\n",
+		       __func__);
+		return -EINVAL;
+	}
+
+	if (rsize < trampoline_sz) {
+		pr_err("%s: reserved block with size 0x%x is to small for trampoline size 0x%x\n",
+		       __func__, rsize, trampoline_sz);
+		return -EINVAL;
+	}
+
+	sram_base_addr = of_iomap(node, 0);
+
+	/* set the boot function for the sram code */
+	rockchip_boot_fn = virt_to_phys(rockchip_secondary_startup);
+
+	/* copy the trampoline to sram, that runs during startup of the core */
+	memcpy(sram_base_addr, &rockchip_secondary_trampoline, trampoline_sz);
+	flush_cache_all();
+	outer_clean_range(0, trampoline_sz);
+
+	dsb_sev();
+
+	return 0;
+}
+
+static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *node;
+	unsigned int i;
+
+	node = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+	if (!node) {
+		pr_err("%s: missing scu\n", __func__);
+		return;
+	}
+
+	scu_base_addr = of_iomap(node, 0);
+	if (!scu_base_addr) {
+		pr_err("%s: could not map scu registers\n", __func__);
+		return;
+	}
+
+	node = of_find_compatible_node(NULL, NULL, "rockchip,rk3066-sram");
+	if (!node) {
+		pr_err("%s: could not find sram dt node\n", __func__);
+		return;
+	}
+
+	if (rockchip_smp_prepare_sram(node))
+		return;
+
+	node = of_find_compatible_node(NULL, NULL, "rockchip,rk3066-pmu");
+	if (!node) {
+		pr_err("%s: could not find sram dt node\n", __func__);
+		return;
+	}
+
+	pmu_base_addr = of_iomap(node, 0);
+	if (!pmu_base_addr) {
+		pr_err("%s: could not map pmu registers\n", __func__);
+		return;
+	}
+
+	/* enable the SCU power domain */
+	pmu_set_power_domain(PMU_PWRDN_SCU, true);
+
+	/*
+	 * While the number of cpus is gathered from dt, also get the number
+	 * of cores from the scu to verify this value when booting the cores.
+	 */
+	ncores = scu_get_core_count(scu_base_addr);
+
+	scu_enable(scu_base_addr);
+
+	/* Make sure that all cores except the first are really off */
+	for (i = 1; i < ncores; i++)
+		pmu_set_power_domain(0 + i, false);
+}
+
+struct smp_operations rockchip_smp_ops __initdata = {
+	.smp_prepare_cpus	= rockchip_smp_prepare_cpus,
+	.smp_boot_secondary	= rockchip_boot_secondary,
+};
diff --git a/arch/arm/mach-rockchip/rockchip.c b/arch/arm/mach-rockchip/rockchip.c
index 82c0b07..d211d6f 100644
--- a/arch/arm/mach-rockchip/rockchip.c
+++ b/arch/arm/mach-rockchip/rockchip.c
@@ -22,6 +22,7 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 #include <asm/hardware/cache-l2x0.h>
+#include "core.h"
 
 static void __init rockchip_dt_init(void)
 {
@@ -38,6 +39,7 @@ static const char * const rockchip_board_dt_compat[] = {
 };
 
 DT_MACHINE_START(ROCKCHIP_DT, "Rockchip Cortex-A9 (Device Tree)")
+	.smp		= smp_ops(rockchip_smp_ops),
 	.init_machine	= rockchip_dt_init,
 	.dt_compat	= rockchip_board_dt_compat,
 MACHINE_END
-- 
1.7.10.4

Re: [PATCH v6 2/6] misc: sram: implement mmio-sram-reserved option

[Mark Rutland]

On Wed, Jan 15, 2014 at 09:41:28PM +0000, Heiko Stübner wrote:
> This implements support for the mmio-sram-reserved option to keep the
> genpool from using these areas.
> 
> Suggested-by: Rob Herring <robherring2@gmail.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>
> ---
>  drivers/misc/sram.c |  118 +++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 110 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
> index afe66571..7fb60f3 100644
> --- a/drivers/misc/sram.c
> +++ b/drivers/misc/sram.c
> @@ -36,13 +36,23 @@ struct sram_dev {
>  	struct clk *clk;
>  };
>  
> +struct sram_reserve {
> +	unsigned long start;
> +	unsigned long size;
> +};
> +
>  static int sram_probe(struct platform_device *pdev)
>  {
>  	void __iomem *virt_base;
>  	struct sram_dev *sram;
>  	struct resource *res;
> -	unsigned long size;
> -	int ret;
> +	unsigned long size, cur_start, cur_size;
> +	const __be32 *reserved_list = NULL;
> +	int reserved_size = 0;
> +	struct sram_reserve *rblocks;
> +	unsigned int nblocks;
> +	int ret = 0;
> +	int i;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	virt_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -65,19 +75,111 @@ static int sram_probe(struct platform_device *pdev)
>  	if (!sram->pool)
>  		return -ENOMEM;
>  
> -	ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base,
> -				res->start, size, -1);
> -	if (ret < 0) {
> -		if (sram->clk)
> -			clk_disable_unprepare(sram->clk);
> -		return ret;
> +	if (pdev->dev.of_node) {
> +		reserved_list = of_get_property(pdev->dev.of_node,
> +						"mmio-sram-reserved",
> +						&reserved_size);
> +		if (reserved_list) {
> +			reserved_size /= sizeof(*reserved_list);

As a general observation, It looks like a lot of people need to know how
many array elements a property might hold (for datastructure
allocation), and are open-coding element counting.

I think it would be nice if we had a helper function to count how many
elements a property can hold (of_property_count_u32_elems?) that would
centralise strict sanity checking (e.g. prop->len % elem_size == 0) and
DTB format details (so you don't have to care about endianness, and it
becomes possible to add DTB metadata later and get runtime type
checking).

That can all come later and shouldn't block this patch.

> +			if (!reserved_size || reserved_size % 2) {
> +				dev_warn(&pdev->dev, "wrong number of arguments in mmio-sram-reserved\n");
> +				reserved_list = NULL;
> +				reserved_size = 0;
> +			}
> +		}
> +	}
> +
> +	/*
> +	 * We need an additional block to mark the end of the memory region
> +	 * after the reserved blocks from the dt are processed.
> +	 */
> +	nblocks = reserved_size / 2 + 1;
> +	rblocks = kmalloc((nblocks) * sizeof(*rblocks), GFP_KERNEL);
> +	if (!rblocks) {
> +		ret = -ENOMEM;
> +		goto err_alloc;
> +	}
> +
> +	cur_start = 0;
> +	for (i = 0; i < nblocks - 1; i++) {
> +		rblocks[i].start = be32_to_cpu(*reserved_list++);
> +		rblocks[i].size = be32_to_cpu(*reserved_list++);

It feels a little odd to have to have to care about the format of the
dtb and do endianness conversion here. It would be nice to limit the
scope of DTB format details to the of_ helper functions.

Could you use of_property_read_u32_index here instead please?

Otherwise this looks fine to me.

Cheers,
Mark.

Re: [PATCH v6 1/6] dt-bindings: sram: describe option to reserve parts of the memory

[Rob Herring]

On Wed, Jan 15, 2014 at 3:40 PM, Heiko Stübner <heiko@sntech.de> wrote:
> Some SoCs need parts of their sram for special purposes. So while being part
> of the peripheral, it should not be part of the genpool controlling the sram.
>
> Therefore add an option mmio-sram-reserved to keep arbitrary portions of the
> sram from general usage.
>
> Suggested-by: Rob Herring <robherring2@gmail.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Ulrich Prinz <ulrich.prinz@googlemail.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  Documentation/devicetree/bindings/misc/sram.txt |    8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt
> index 4d0a00e..09ee7a3 100644
> --- a/Documentation/devicetree/bindings/misc/sram.txt
> +++ b/Documentation/devicetree/bindings/misc/sram.txt
> @@ -8,9 +8,17 @@ Required properties:
>
>  - reg : SRAM iomem address range
>
> +Optional properties:
> +
> +- mmio-sram-reserved: ordered list of reserved chunks inside the sram that
> +  should not be used by the operating system.
> +  Format is <base size>, <base size>, ...; with base being relative to the
> +  reg property base.
> +
>  Example:
>
>  sram: sram@5c000000 {
>         compatible = "mmio-sram";
>         reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */
> +       mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */
>  };
> --
> 1.7.10.4
>
>

[PATCH] of: add function to count number of u32 elements in a property

[Heiko Stübner]

The need to know the number of array elements in a property is
a common pattern. To prevent duplication of open-coded implementations
add a helper function that also centralises strict sanity checking
and DTB format details.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
Hi Mark,
did you mean it like this? I've tested it with the sram-reserve change and
it made the part of the determining the number elements a lot nicer :-)

 drivers/of/base.c  |   32 ++++++++++++++++++++++++++++++++
 include/linux/of.h |    8 ++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f807d0e..0f40ea5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -920,6 +920,38 @@ int of_property_read_u32_index(const struct device_node *np,
 EXPORT_SYMBOL_GPL(of_property_read_u32_index);
 
 /**
+ * of_property_count_u32_elems - Count the number of u32 values in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u32 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u32 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u32_elems(const struct device_node *np,
+				const char *propname)
+{
+	int elem_size = sizeof(u32);
+	int len;
+	struct property *prop = of_find_property(np, propname, &len);
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->value)
+		return -ENODATA;
+
+	if (prop->length % elem_size != 0) {
+		pr_err("size of %s is not a multiple of u32\n", propname);
+		return -EINVAL;
+	}
+
+	return len / elem_size;
+}
+EXPORT_SYMBOL_GPL(of_property_count_u32_elems);
+
+/**
  * of_property_read_u8_array - Find and read an array of u8 from a property.
  *
  * @np:		device node from which the property value is to be read.
diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..5794942 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -253,6 +253,8 @@ extern struct property *of_find_property(const struct device_node *np,
 extern int of_property_read_u32_index(const struct device_node *np,
 				       const char *propname,
 				       u32 index, u32 *out_value);
+extern int of_property_count_u32_elems(const struct device_node *np,
+				       const char *propname);
 extern int of_property_read_u8_array(const struct device_node *np,
 			const char *propname, u8 *out_values, size_t sz);
 extern int of_property_read_u16_array(const struct device_node *np,
@@ -432,6 +434,12 @@ static inline int of_property_read_u32_index(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_property_count_u32_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u8_array(const struct device_node *np,
 			const char *propname, u8 *out_values, size_t sz)
 {
-- 
1.7.10.4

Re: [PATCH] of: add function to count number of u32 elements in a property

[Rob Herring]

On Thu, Jan 16, 2014 at 12:04 PM, Heiko Stübner <heiko@sntech.de> wrote:
> The need to know the number of array elements in a property is
> a common pattern. To prevent duplication of open-coded implementations
> add a helper function that also centralises strict sanity checking
> and DTB format details.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Hi Mark,
> did you mean it like this? I've tested it with the sram-reserve change and
> it made the part of the determining the number elements a lot nicer :-)
>
>  drivers/of/base.c  |   32 ++++++++++++++++++++++++++++++++
>  include/linux/of.h |    8 ++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f807d0e..0f40ea5 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -920,6 +920,38 @@ int of_property_read_u32_index(const struct device_node *np,
>  EXPORT_SYMBOL_GPL(of_property_read_u32_index);
>
>  /**
> + * of_property_count_u32_elems - Count the number of u32 values in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + *
> + * Search for a property in a device node and count the number of u32 elements
> + * in it. Returns number of elements on sucess, -EINVAL if the property does
> + * not exist or its length does not match a multiple of u32 and -ENODATA if the
> + * property does not have a value.
> + */
> +int of_property_count_u32_elems(const struct device_node *np,
> +                               const char *propname)
> +{
> +       int elem_size = sizeof(u32);

This should be a parameter, so functions for different sized
properties can be a simple wrapper function.

> +       int len;
> +       struct property *prop = of_find_property(np, propname, &len);
> +
> +       if (!prop)
> +               return -EINVAL;
> +       if (!prop->value)
> +               return -ENODATA;
> +
> +       if (prop->length % elem_size != 0) {
> +               pr_err("size of %s is not a multiple of u32\n", propname);

The node name would be useful here too.

> +               return -EINVAL;
> +       }
> +
> +       return len / elem_size;
> +}
> +EXPORT_SYMBOL_GPL(of_property_count_u32_elems);
> +
> +/**
>   * of_property_read_u8_array - Find and read an array of u8 from a property.
>   *
>   * @np:                device node from which the property value is to be read.
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 276c546..5794942 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -253,6 +253,8 @@ extern struct property *of_find_property(const struct device_node *np,
>  extern int of_property_read_u32_index(const struct device_node *np,
>                                        const char *propname,
>                                        u32 index, u32 *out_value);
> +extern int of_property_count_u32_elems(const struct device_node *np,
> +                                      const char *propname);
>  extern int of_property_read_u8_array(const struct device_node *np,
>                         const char *propname, u8 *out_values, size_t sz);
>  extern int of_property_read_u16_array(const struct device_node *np,
> @@ -432,6 +434,12 @@ static inline int of_property_read_u32_index(const struct device_node *np,
>         return -ENOSYS;
>  }
>
> +static inline int of_property_count_u32_elems(const struct device_node *np,
> +                       const char *propname)
> +{
> +       return -ENOSYS;
> +}
> +
>  static inline int of_property_read_u8_array(const struct device_node *np,
>                         const char *propname, u8 *out_values, size_t sz)
>  {
> --
> 1.7.10.4
>
>

Re: [PATCH] of: add function to count number of u32 elements in a property

[Mark Rutland]

On Thu, Jan 16, 2014 at 06:04:42PM +0000, Heiko Stübner wrote:
> The need to know the number of array elements in a property is
> a common pattern. To prevent duplication of open-coded implementations
> add a helper function that also centralises strict sanity checking
> and DTB format details.
> 
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Hi Mark,
> did you mean it like this? I've tested it with the sram-reserve change and
> it made the part of the determining the number elements a lot nicer :-)

Yes! This looks pretty nice! :)

> 
>  drivers/of/base.c  |   32 ++++++++++++++++++++++++++++++++
>  include/linux/of.h |    8 ++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f807d0e..0f40ea5 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -920,6 +920,38 @@ int of_property_read_u32_index(const struct device_node *np,
>  EXPORT_SYMBOL_GPL(of_property_read_u32_index);
>  
>  /**
> + * of_property_count_u32_elems - Count the number of u32 values in a property
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + *
> + * Search for a property in a device node and count the number of u32 elements
> + * in it. Returns number of elements on sucess, -EINVAL if the property does
> + * not exist or its length does not match a multiple of u32 and -ENODATA if the
> + * property does not have a value.
> + */
> +int of_property_count_u32_elems(const struct device_node *np,
> +				const char *propname)
> +{
> +	int elem_size = sizeof(u32);
> +	int len;
> +	struct property *prop = of_find_property(np, propname, &len);
> +
> +	if (!prop)
> +		return -EINVAL;
> +	if (!prop->value)
> +		return -ENODATA;
> +
> +	if (prop->length % elem_size != 0) {
> +		pr_err("size of %s is not a multiple of u32\n", propname);
> +		return -EINVAL;
> +	}
> +
> +	return len / elem_size;
> +}
> +EXPORT_SYMBOL_GPL(of_property_count_u32_elems);

As Rob said in his reply, it would be nice to split this into a static
helper that took elem size as a parameter, so we can have the full suite
of of_property_count_{u8,u16,u32,u64}_elems.

Also, I think you can get rid of len and always use prop->length, as
other helpers seem to do.

Cheers for putting this together!

Mark.

[PATCH v2] of: add functions to count number of elements in a property

[Heiko Stübner]

The need to know the number of array elements in a property is
a common pattern. To prevent duplication of open-coded implementations
add a helper static function that also centralises strict sanity
checking and DTB format details, as well as a set of wrapper functions
for u8, u16, u32 and u64.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
changes since v1:
address comments from Rob Herring and Mark Rutland:
- provide a helper and a set of wrappers for u8-u64
- get rid of extra len variable, prop->length is enough
- include node name in error message

 drivers/of/base.c  |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h |   32 +++++++++++++++++
 2 files changed, 130 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f807d0e..b6e6d4a 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -862,6 +862,104 @@ struct device_node *of_find_node_by_phandle(phandle handle)
 EXPORT_SYMBOL(of_find_node_by_phandle);
 
 /**
+ * of_count_property_elems_of_size - Count the number of elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @elem_size:	size of the individual element
+ */
+static int of_count_property_elems_of_size(const struct device_node *np,
+				const char *propname, int elem_size)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->value)
+		return -ENODATA;
+
+	if (prop->length % elem_size != 0) {
+		pr_err("size of %s in node %s is not a multiple of %d\n",
+		       propname, np->name, elem_size);
+		return -EINVAL;
+	}
+
+	return prop->length / elem_size;
+}
+
+/**
+ * of_property_count_u8_elems - Count the number of u8 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u8 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u8 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u8_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_count_property_elems_of_size(np, propname, sizeof(u8));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u8_elems);
+
+/**
+ * of_property_count_u16_elems - Count the number of u16 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u16 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u16 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u16_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_count_property_elems_of_size(np, propname, sizeof(u16));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u16_elems);
+
+/**
+ * of_property_count_u32_elems - Count the number of u32 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u32 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u32 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u32_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_count_property_elems_of_size(np, propname, sizeof(u32));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u32_elems);
+
+/**
+ * of_property_count_u64_elems - Count the number of u64 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u64 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u64 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u64_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_count_property_elems_of_size(np, propname, sizeof(u64));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u64_elems);
+
+/**
  * of_find_property_value_of_size
  *
  * @np:		device node from which the property value is to be read.
diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..06fe898 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -250,6 +250,14 @@ extern struct device_node *of_find_node_with_property(
 extern struct property *of_find_property(const struct device_node *np,
 					 const char *name,
 					 int *lenp);
+extern int of_property_count_u8_elems(const struct device_node *np,
+				       const char *propname);
+extern int of_property_count_u16_elems(const struct device_node *np,
+				       const char *propname);
+extern int of_property_count_u32_elems(const struct device_node *np,
+				       const char *propname);
+extern int of_property_count_u64_elems(const struct device_node *np,
+				       const char *propname);
 extern int of_property_read_u32_index(const struct device_node *np,
 				       const char *propname,
 				       u32 index, u32 *out_value);
@@ -426,6 +434,30 @@ static inline struct device_node *of_find_compatible_node(
 	return NULL;
 }
 
+static inline int of_property_count_u8_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_count_u16_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_count_u32_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_count_u64_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u32_index(const struct device_node *np,
 			const char *propname, u32 index, u32 *out_value)
 {
-- 
1.7.10.4

Re: [PATCH v2] of: add functions to count number of elements in a property

[Mark Rutland]

On Fri, Jan 17, 2014 at 03:44:14PM +0000, Heiko Stübner wrote:
> The need to know the number of array elements in a property is
> a common pattern. To prevent duplication of open-coded implementations
> add a helper static function that also centralises strict sanity
> checking and DTB format details, as well as a set of wrapper functions
> for u8, u16, u32 and u64.
> 
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> changes since v1:
> address comments from Rob Herring and Mark Rutland:
> - provide a helper and a set of wrappers for u8-u64
> - get rid of extra len variable, prop->length is enough
> - include node name in error message
> 
>  drivers/of/base.c  |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |   32 +++++++++++++++++
>  2 files changed, 130 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f807d0e..b6e6d4a 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -862,6 +862,104 @@ struct device_node *of_find_node_by_phandle(phandle handle)
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>  
>  /**
> + * of_count_property_elems_of_size - Count the number of elements in a property
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @elem_size:	size of the individual element
> + */
> +static int of_count_property_elems_of_size(const struct device_node *np,
> +				const char *propname, int elem_size)

As a minor nit, it would be nicer to have 'count' and 'property' switch
places in the name (i.e. call this of_property_count_elems_of_size).
That way it's concistent with the naming of the wrappers.

> +{
> +	struct property *prop = of_find_property(np, propname, NULL);
> +
> +	if (!prop)
> +		return -EINVAL;
> +	if (!prop->value)
> +		return -ENODATA;
> +
> +	if (prop->length % elem_size != 0) {
> +		pr_err("size of %s in node %s is not a multiple of %d\n",
> +		       propname, np->name, elem_size);

It would be nice to use np->full_name so you get the absolute path of
the node -- it makes finding them easier later.

Otherwise, the patch looks good to me, thanks for implementing it!

With those changes:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Cheers,
Mark.

[PATCH v3] of: add functions to count number of elements in a property

[Heiko Stübner]

The need to know the number of array elements in a property is
a common pattern. To prevent duplication of open-coded implementations
add a helper static function that also centralises strict sanity
checking and DTB format details, as well as a set of wrapper functions
for u8, u16, u32 and u64.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
---
changes since v2:
address more comments from Mark Rutland
- switch to of_property_count_elems_of_size
- use full_name instead of name in error message
changes since v1:
address comments from Rob Herring and Mark Rutland:
- provide a helper and a set of wrappers for u8-u64
- get rid of extra len variable, prop->length is enough
- include node name in error message

Posted for completenes sake. If nobody complains,
I'll simply make it part of my Rockchip-SMP series.

 drivers/of/base.c  |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h |   32 +++++++++++++++++
 2 files changed, 131 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f807d0e..1cd6dc9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -862,6 +862,105 @@ struct device_node *of_find_node_by_phandle(phandle handle)
 EXPORT_SYMBOL(of_find_node_by_phandle);
 
 /**
+ * of_property_count_elems_of_size - Count the number of elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @elem_size:	size of the individual element
+ */
+static int of_property_count_elems_of_size(const struct device_node *np,
+				const char *propname, int elem_size)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->value)
+		return -ENODATA;
+
+pr_err("size of %s in node %s is not a multiple of %d\n", propname, np->full_name, elem_size);
+	if (prop->length % elem_size != 0) {
+		pr_err("size of %s in node %s is not a multiple of %d\n",
+		       propname, np->full_name, elem_size);
+		return -EINVAL;
+	}
+
+	return prop->length / elem_size;
+}
+
+/**
+ * of_property_count_u8_elems - Count the number of u8 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u8 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u8 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u8_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u8));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u8_elems);
+
+/**
+ * of_property_count_u16_elems - Count the number of u16 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u16 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u16 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u16_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u16));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u16_elems);
+
+/**
+ * of_property_count_u32_elems - Count the number of u32 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u32 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u32 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u32_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u32));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u32_elems);
+
+/**
+ * of_property_count_u64_elems - Count the number of u64 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u64 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u64 and -ENODATA if the
+ * property does not have a value.
+ */
+int of_property_count_u64_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u64));
+}
+EXPORT_SYMBOL_GPL(of_property_count_u64_elems);
+
+/**
  * of_find_property_value_of_size
  *
  * @np:		device node from which the property value is to be read.
diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..06fe898 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -250,6 +250,14 @@ extern struct device_node *of_find_node_with_property(
 extern struct property *of_find_property(const struct device_node *np,
 					 const char *name,
 					 int *lenp);
+extern int of_property_count_u8_elems(const struct device_node *np,
+				       const char *propname);
+extern int of_property_count_u16_elems(const struct device_node *np,
+				       const char *propname);
+extern int of_property_count_u32_elems(const struct device_node *np,
+				       const char *propname);
+extern int of_property_count_u64_elems(const struct device_node *np,
+				       const char *propname);
 extern int of_property_read_u32_index(const struct device_node *np,
 				       const char *propname,
 				       u32 index, u32 *out_value);
@@ -426,6 +434,30 @@ static inline struct device_node *of_find_compatible_node(
 	return NULL;
 }
 
+static inline int of_property_count_u8_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_count_u16_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_count_u32_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_count_u64_elems(const struct device_node *np,
+			const char *propname)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u32_index(const struct device_node *np,
 			const char *propname, u32 index, u32 *out_value)
 {
-- 
1.7.10.4

Re: [PATCH v2] of: add functions to count number of elements in a property

[Rob Herring]

On Fri, Jan 17, 2014 at 9:44 AM, Heiko Stübner <heiko@sntech.de> wrote:
> The need to know the number of array elements in a property is
> a common pattern. To prevent duplication of open-coded implementations
> add a helper static function that also centralises strict sanity
> checking and DTB format details, as well as a set of wrapper functions
> for u8, u16, u32 and u64.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> changes since v1:
> address comments from Rob Herring and Mark Rutland:
> - provide a helper and a set of wrappers for u8-u64
> - get rid of extra len variable, prop->length is enough
> - include node name in error message
>
>  drivers/of/base.c  |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |   32 +++++++++++++++++
>  2 files changed, 130 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f807d0e..b6e6d4a 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -862,6 +862,104 @@ struct device_node *of_find_node_by_phandle(phandle handle)
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>
>  /**
> + * of_count_property_elems_of_size - Count the number of elements in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + * @elem_size: size of the individual element
> + */
> +static int of_count_property_elems_of_size(const struct device_node *np,
> +                               const char *propname, int elem_size)
> +{
> +       struct property *prop = of_find_property(np, propname, NULL);
> +
> +       if (!prop)
> +               return -EINVAL;
> +       if (!prop->value)
> +               return -ENODATA;
> +
> +       if (prop->length % elem_size != 0) {
> +               pr_err("size of %s in node %s is not a multiple of %d\n",
> +                      propname, np->name, elem_size);
> +               return -EINVAL;
> +       }
> +
> +       return prop->length / elem_size;
> +}

Export this one.

> +
> +/**
> + * of_property_count_u8_elems - Count the number of u8 elements in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + *
> + * Search for a property in a device node and count the number of u8 elements
> + * in it. Returns number of elements on sucess, -EINVAL if the property does
> + * not exist or its length does not match a multiple of u8 and -ENODATA if the
> + * property does not have a value.
> + */
> +int of_property_count_u8_elems(const struct device_node *np,
> +                               const char *propname)
> +{
> +       return of_count_property_elems_of_size(np, propname, sizeof(u8));
> +}
> +EXPORT_SYMBOL_GPL(of_property_count_u8_elems);

And make all these static inline.

Then you only need a single empty function for !OF.

Rob

[PATCH v4] of: add functions to count number of elements in a property

[Heiko Stübner]

The need to know the number of array elements in a property is
a common pattern. To prevent duplication of open-coded implementations
add a helper static function that also centralises strict sanity
checking and DTB format details, as well as a set of wrapper functions
for u8, u16, u32 and u64.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
changes since v3:
address more comments from Rob Herring
- export the base function and inline the type-specific wrappers
changes since v2:
address more comments from Mark Rutland
- switch to of_property_count_elems_of_size
- use full_name instead of name in error message
changes since v1:
address comments from Rob Herring and Mark Rutland:
- provide a helper and a set of wrappers for u8-u64
- get rid of extra len variable, prop->length is enough
- include node name in error message

Mark, does your Reviewed-by holds for this variant too?

 drivers/of/base.c  |   32 ++++++++++++++++++++++
 include/linux/of.h |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f807d0e..21646c0 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -862,6 +862,38 @@ struct device_node *of_find_node_by_phandle(phandle handle)
 EXPORT_SYMBOL(of_find_node_by_phandle);
 
 /**
+ * of_property_count_elems_of_size - Count the number of elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @elem_size:	size of the individual element
+ *
+ * Search for a property in a device node and count the number of elements of
+ * size elem_size in it. Returns number of elements on sucess, -EINVAL if the
+ * property does not exist or its length does not match a multiple of u16 and
+ * -ENODATA if the property does not have a value.
+ */
+int of_property_count_elems_of_size(const struct device_node *np,
+				const char *propname, int elem_size)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->value)
+		return -ENODATA;
+
+	if (prop->length % elem_size != 0) {
+		pr_err("size of %s in node %s is not a multiple of %d\n",
+		       propname, np->full_name, elem_size);
+		return -EINVAL;
+	}
+
+	return prop->length / elem_size;
+}
+EXPORT_SYMBOL_GPL(of_property_count_elems_of_size);
+
+/**
  * of_find_property_value_of_size
  *
  * @np:		device node from which the property value is to be read.
diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..293920d 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -250,6 +250,8 @@ extern struct device_node *of_find_node_with_property(
 extern struct property *of_find_property(const struct device_node *np,
 					 const char *name,
 					 int *lenp);
+extern int of_property_count_elems_of_size(const struct device_node *np,
+				const char *propname, int elem_size);
 extern int of_property_read_u32_index(const struct device_node *np,
 				       const char *propname,
 				       u32 index, u32 *out_value);
@@ -426,6 +428,12 @@ static inline struct device_node *of_find_compatible_node(
 	return NULL;
 }
 
+static inline int of_property_count_elems_of_size(const struct device_node *np,
+			const char *propname, int elem_size)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u32_index(const struct device_node *np,
 			const char *propname, u32 index, u32 *out_value)
 {
@@ -565,6 +573,74 @@ static inline int of_node_to_nid(struct device_node *device) { return 0; }
 #endif
 
 /**
+ * of_property_count_u8_elems - Count the number of u8 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u8 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u8 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u8_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u8));
+}
+
+/**
+ * of_property_count_u16_elems - Count the number of u16 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u16 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u16 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u16_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u16));
+}
+
+/**
+ * of_property_count_u32_elems - Count the number of u32 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u32 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u32 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u32_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u32));
+}
+
+/**
+ * of_property_count_u64_elems - Count the number of u64 elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u64 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u64 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u64_elems(const struct device_node *np,
+				const char *propname)
+{
+	return of_property_count_elems_of_size(np, propname, sizeof(u64));
+}
+
+/**
  * of_property_read_bool - Findfrom a property
  * @np:		device node from which the property value is to be read.
  * @propname:	name of the property to be searched.
-- 
1.7.10.4

Re: [PATCH v4] of: add functions to count number of elements in a property

[Rob Herring]

On Sat, Jan 18, 2014 at 6:02 AM, Heiko Stübner <heiko@sntech.de> wrote:
> The need to know the number of array elements in a property is
> a common pattern. To prevent duplication of open-coded implementations
> add a helper static function that also centralises strict sanity
> checking and DTB format details, as well as a set of wrapper functions
> for u8, u16, u32 and u64.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---

Looks good. Do you plan to convert some users to use this?

Rob

> changes since v3:
> address more comments from Rob Herring
> - export the base function and inline the type-specific wrappers
> changes since v2:
> address more comments from Mark Rutland
> - switch to of_property_count_elems_of_size
> - use full_name instead of name in error message
> changes since v1:
> address comments from Rob Herring and Mark Rutland:
> - provide a helper and a set of wrappers for u8-u64
> - get rid of extra len variable, prop->length is enough
> - include node name in error message
>
> Mark, does your Reviewed-by holds for this variant too?
>
>  drivers/of/base.c  |   32 ++++++++++++++++++++++
>  include/linux/of.h |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 108 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f807d0e..21646c0 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -862,6 +862,38 @@ struct device_node *of_find_node_by_phandle(phandle handle)
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>
>  /**
> + * of_property_count_elems_of_size - Count the number of elements in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + * @elem_size: size of the individual element
> + *
> + * Search for a property in a device node and count the number of elements of
> + * size elem_size in it. Returns number of elements on sucess, -EINVAL if the
> + * property does not exist or its length does not match a multiple of u16 and
> + * -ENODATA if the property does not have a value.
> + */
> +int of_property_count_elems_of_size(const struct device_node *np,
> +                               const char *propname, int elem_size)
> +{
> +       struct property *prop = of_find_property(np, propname, NULL);
> +
> +       if (!prop)
> +               return -EINVAL;
> +       if (!prop->value)
> +               return -ENODATA;
> +
> +       if (prop->length % elem_size != 0) {
> +               pr_err("size of %s in node %s is not a multiple of %d\n",
> +                      propname, np->full_name, elem_size);
> +               return -EINVAL;
> +       }
> +
> +       return prop->length / elem_size;
> +}
> +EXPORT_SYMBOL_GPL(of_property_count_elems_of_size);
> +
> +/**
>   * of_find_property_value_of_size
>   *
>   * @np:                device node from which the property value is to be read.
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 276c546..293920d 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -250,6 +250,8 @@ extern struct device_node *of_find_node_with_property(
>  extern struct property *of_find_property(const struct device_node *np,
>                                          const char *name,
>                                          int *lenp);
> +extern int of_property_count_elems_of_size(const struct device_node *np,
> +                               const char *propname, int elem_size);
>  extern int of_property_read_u32_index(const struct device_node *np,
>                                        const char *propname,
>                                        u32 index, u32 *out_value);
> @@ -426,6 +428,12 @@ static inline struct device_node *of_find_compatible_node(
>         return NULL;
>  }
>
> +static inline int of_property_count_elems_of_size(const struct device_node *np,
> +                       const char *propname, int elem_size)
> +{
> +       return -ENOSYS;
> +}
> +
>  static inline int of_property_read_u32_index(const struct device_node *np,
>                         const char *propname, u32 index, u32 *out_value)
>  {
> @@ -565,6 +573,74 @@ static inline int of_node_to_nid(struct device_node *device) { return 0; }
>  #endif
>
>  /**
> + * of_property_count_u8_elems - Count the number of u8 elements in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + *
> + * Search for a property in a device node and count the number of u8 elements
> + * in it. Returns number of elements on sucess, -EINVAL if the property does
> + * not exist or its length does not match a multiple of u8 and -ENODATA if the
> + * property does not have a value.
> + */
> +static inline int of_property_count_u8_elems(const struct device_node *np,
> +                               const char *propname)
> +{
> +       return of_property_count_elems_of_size(np, propname, sizeof(u8));
> +}
> +
> +/**
> + * of_property_count_u16_elems - Count the number of u16 elements in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + *
> + * Search for a property in a device node and count the number of u16 elements
> + * in it. Returns number of elements on sucess, -EINVAL if the property does
> + * not exist or its length does not match a multiple of u16 and -ENODATA if the
> + * property does not have a value.
> + */
> +static inline int of_property_count_u16_elems(const struct device_node *np,
> +                               const char *propname)
> +{
> +       return of_property_count_elems_of_size(np, propname, sizeof(u16));
> +}
> +
> +/**
> + * of_property_count_u32_elems - Count the number of u32 elements in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + *
> + * Search for a property in a device node and count the number of u32 elements
> + * in it. Returns number of elements on sucess, -EINVAL if the property does
> + * not exist or its length does not match a multiple of u32 and -ENODATA if the
> + * property does not have a value.
> + */
> +static inline int of_property_count_u32_elems(const struct device_node *np,
> +                               const char *propname)
> +{
> +       return of_property_count_elems_of_size(np, propname, sizeof(u32));
> +}
> +
> +/**
> + * of_property_count_u64_elems - Count the number of u64 elements in a property
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + *
> + * Search for a property in a device node and count the number of u64 elements
> + * in it. Returns number of elements on sucess, -EINVAL if the property does
> + * not exist or its length does not match a multiple of u64 and -ENODATA if the
> + * property does not have a value.
> + */
> +static inline int of_property_count_u64_elems(const struct device_node *np,
> +                               const char *propname)
> +{
> +       return of_property_count_elems_of_size(np, propname, sizeof(u64));
> +}
> +
> +/**
>   * of_property_read_bool - Findfrom a property
>   * @np:                device node from which the property value is to be read.
>   * @propname:  name of the property to be searched.
> --
> 1.7.10.4
>
>

Re: [PATCH v4] of: add functions to count number of elements in a property

[Heiko Stübner]

Am Samstag, 18. Januar 2014, 09:07:30 schrieb Rob Herring:
> On Sat, Jan 18, 2014 at 6:02 AM, Heiko Stübner <heiko@sntech.de> wrote:
> > The need to know the number of array elements in a property is
> > a common pattern. To prevent duplication of open-coded implementations
> > add a helper static function that also centralises strict sanity
> > checking and DTB format details, as well as a set of wrapper functions
> > for u8, u16, u32 and u64.
> > 
> > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> 
> Looks good. Do you plan to convert some users to use this?

My plan at the moment was to "just" use it for my mmio-sram-reserve stuff - 
just wanted to make sure that this change is sane, before having to sent the 
whole thing for each iteration.

I haven't yet looked where the other users, that Mark mentioned, are at :-)


Heiko


> > changes since v3:
> > address more comments from Rob Herring
> > - export the base function and inline the type-specific wrappers
> > changes since v2:
> > address more comments from Mark Rutland
> > - switch to of_property_count_elems_of_size
> > - use full_name instead of name in error message
> > changes since v1:
> > address comments from Rob Herring and Mark Rutland:
> > - provide a helper and a set of wrappers for u8-u64
> > - get rid of extra len variable, prop->length is enough
> > - include node name in error message
> > 
> > Mark, does your Reviewed-by holds for this variant too?
> > 
> >  drivers/of/base.c  |   32 ++++++++++++++++++++++
> >  include/linux/of.h |   76
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
> >  108 insertions(+)
> > 
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index f807d0e..21646c0 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -862,6 +862,38 @@ struct device_node *of_find_node_by_phandle(phandle
> > handle)> 
> >  EXPORT_SYMBOL(of_find_node_by_phandle);
> >  
> >  /**
> > 
> > + * of_property_count_elems_of_size - Count the number of elements in a
> > property + *
> > + * @np:                device node from which the property value is to be
> > read. + * @propname:  name of the property to be searched.
> > + * @elem_size: size of the individual element
> > + *
> > + * Search for a property in a device node and count the number of
> > elements of + * size elem_size in it. Returns number of elements on
> > sucess, -EINVAL if the + * property does not exist or its length does not
> > match a multiple of u16 and + * -ENODATA if the property does not have a
> > value.
> > + */
> > +int of_property_count_elems_of_size(const struct device_node *np,
> > +                               const char *propname, int elem_size)
> > +{
> > +       struct property *prop = of_find_property(np, propname, NULL);
> > +
> > +       if (!prop)
> > +               return -EINVAL;
> > +       if (!prop->value)
> > +               return -ENODATA;
> > +
> > +       if (prop->length % elem_size != 0) {
> > +               pr_err("size of %s in node %s is not a multiple of %d\n",
> > +                      propname, np->full_name, elem_size);
> > +               return -EINVAL;
> > +       }
> > +
> > +       return prop->length / elem_size;
> > +}
> > +EXPORT_SYMBOL_GPL(of_property_count_elems_of_size);
> > +
> > +/**
> > 
> >   * of_find_property_value_of_size
> >   *
> >   * @np:                device node from which the property value is to be
> >   read.> 
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 276c546..293920d 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -250,6 +250,8 @@ extern struct device_node *of_find_node_with_property(
> > 
> >  extern struct property *of_find_property(const struct device_node *np,
> >  
> >                                          const char *name,
> >                                          int *lenp);
> > 
> > +extern int of_property_count_elems_of_size(const struct device_node *np,
> > +                               const char *propname, int elem_size);
> > 
> >  extern int of_property_read_u32_index(const struct device_node *np,
> >  
> >                                        const char *propname,
> >                                        u32 index, u32 *out_value);
> > 
> > @@ -426,6 +428,12 @@ static inline struct device_node
> > *of_find_compatible_node(> 
> >         return NULL;
> >  
> >  }
> > 
> > +static inline int of_property_count_elems_of_size(const struct
> > device_node *np, +                       const char *propname, int
> > elem_size)
> > +{
> > +       return -ENOSYS;
> > +}
> > +
> > 
> >  static inline int of_property_read_u32_index(const struct device_node
> >  *np,
> >  
> >                         const char *propname, u32 index, u32 *out_value)
> >  
> >  {
> > 
> > @@ -565,6 +573,74 @@ static inline int of_node_to_nid(struct device_node
> > *device) { return 0; }> 
> >  #endif
> >  
> >  /**
> > 
> > + * of_property_count_u8_elems - Count the number of u8 elements in a
> > property + *
> > + * @np:                device node from which the property value is to be
> > read. + * @propname:  name of the property to be searched.
> > + *
> > + * Search for a property in a device node and count the number of u8
> > elements + * in it. Returns number of elements on sucess, -EINVAL if the
> > property does + * not exist or its length does not match a multiple of u8
> > and -ENODATA if the + * property does not have a value.
> > + */
> > +static inline int of_property_count_u8_elems(const struct device_node
> > *np,
> > +                               const char *propname)
> > +{
> > +       return of_property_count_elems_of_size(np, propname, sizeof(u8));
> > +}
> > +
> > +/**
> > + * of_property_count_u16_elems - Count the number of u16 elements in a
> > property + *
> > + * @np:                device node from which the property value is to be
> > read. + * @propname:  name of the property to be searched.
> > + *
> > + * Search for a property in a device node and count the number of u16
> > elements + * in it. Returns number of elements on sucess, -EINVAL if the
> > property does + * not exist or its length does not match a multiple of
> > u16 and -ENODATA if the + * property does not have a value.
> > + */
> > +static inline int of_property_count_u16_elems(const struct device_node
> > *np, +                               const char *propname)
> > +{
> > +       return of_property_count_elems_of_size(np, propname, sizeof(u16));
> > +}
> > +
> > +/**
> > + * of_property_count_u32_elems - Count the number of u32 elements in a
> > property + *
> > + * @np:                device node from which the property value is to be
> > read. + * @propname:  name of the property to be searched.
> > + *
> > + * Search for a property in a device node and count the number of u32
> > elements + * in it. Returns number of elements on sucess, -EINVAL if the
> > property does + * not exist or its length does not match a multiple of
> > u32 and -ENODATA if the + * property does not have a value.
> > + */
> > +static inline int of_property_count_u32_elems(const struct device_node
> > *np, +                               const char *propname)
> > +{
> > +       return of_property_count_elems_of_size(np, propname, sizeof(u32));
> > +}
> > +
> > +/**
> > + * of_property_count_u64_elems - Count the number of u64 elements in a
> > property + *
> > + * @np:                device node from which the property value is to be
> > read. + * @propname:  name of the property to be searched.
> > + *
> > + * Search for a property in a device node and count the number of u64
> > elements + * in it. Returns number of elements on sucess, -EINVAL if the
> > property does + * not exist or its length does not match a multiple of
> > u64 and -ENODATA if the + * property does not have a value.
> > + */
> > +static inline int of_property_count_u64_elems(const struct device_node
> > *np, +                               const char *propname)
> > +{
> > +       return of_property_count_elems_of_size(np, propname, sizeof(u64));
> > +}
> > +
> > +/**
> > 
> >   * of_property_read_bool - Findfrom a property
> >   * @np:                device node from which the property value is to be
> >   read. * @propname:  name of the property to be searched.
> > 
> > --
> > 1.7.10.4

Re: [PATCH v4] of: add functions to count number of elements in a property

[Heiko Stübner]

Hi Grant,

On Tuesday, 4. February 2014 17:30:34 Grant Likely wrote:
> On Sat, 18 Jan 2014 09:07:30 -0600, Rob Herring <robherring2@gmail.com> 
wrote:
> > On Sat, Jan 18, 2014 at 6:02 AM, Heiko Stübner <heiko@sntech.de> wrote:
> > > The need to know the number of array elements in a property is
> > > a common pattern. To prevent duplication of open-coded implementations
> > > add a helper static function that also centralises strict sanity
> > > checking and DTB format details, as well as a set of wrapper functions
> > > for u8, u16, u32 and u64.
> > > 
> > > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > 
> > Looks good. Do you plan to convert some users to use this?
> 
> I'll take that as an acked-by. Merged, thanks.

before you taking this patch, I was planning on simply sending this as part of 
my rockchip-smp series - as I'm currently the only user of it :-) .

This going through your tree is most likely the better way, but now I need it 
to somehow make its way into arm-soc too ... I guess some sort of stable 
branch arm-soc could pull?


Thanks
Heiko

Re: [PATCH v4] of: add functions to count number of elements in a property

[Heiko Stübner]

Am Mittwoch, 5. Februar 2014, 12:06:52 schrieb Grant Likely:
> On Tue, 04 Feb 2014 19:48:17 +0100, Heiko Stübner <heiko@sntech.de> wrote:
> > Hi Grant,
> > 
> > On Tuesday, 4. February 2014 17:30:34 Grant Likely wrote:
> > > On Sat, 18 Jan 2014 09:07:30 -0600, Rob Herring <robherring2@gmail.com>
> > 
> > wrote:
> > > > On Sat, Jan 18, 2014 at 6:02 AM, Heiko Stübner <heiko@sntech.de> 
wrote:
> > > > > The need to know the number of array elements in a property is
> > > > > a common pattern. To prevent duplication of open-coded
> > > > > implementations
> > > > > add a helper static function that also centralises strict sanity
> > > > > checking and DTB format details, as well as a set of wrapper
> > > > > functions
> > > > > for u8, u16, u32 and u64.
> > > > > 
> > > > > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > > > ---
> > > > 
> > > > Looks good. Do you plan to convert some users to use this?
> > > 
> > > I'll take that as an acked-by. Merged, thanks.
> > 
> > before you taking this patch, I was planning on simply sending this as
> > part of my rockchip-smp series - as I'm currently the only user of it :-)
> > .
> > 
> > This going through your tree is most likely the better way, but now I need
> > it to somehow make its way into arm-soc too ... I guess some sort of
> > stable branch arm-soc could pull?
> 
> Nah, I'll drop it from my tree. Add my acked-by and merge it via
> arm-soc.

As I said on IRC, now it seems like you can keep this patch in your tree :-)

If we're really going with reserved-memory like you suggested in the rockchip-
smp series it removes the need to count u32-elements in a property for me, as 
the reserved blocks move into individual subnodes.


Heiko