linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 1/3] of: introduce for_each_matching_node_and_match()
@ 2012-11-20 23:12 Stephen Warren
  2012-11-20 23:12 ` [PATCH V4 2/3] clocksource: add common of_clksrc_init() function Stephen Warren
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Stephen Warren @ 2012-11-20 23:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Warren <swarren@nvidia.com>

The following pattern of code is tempting:

    for_each_matching_node(np, table) {
        match = of_match_node(table, np);

However, this results in iterating over table twice; the second time
inside of_match_node(). The implementation of for_each_matching_node()
already found the match, so this is redundant. Invent new function
of_find_matching_node_and_match() and macro
for_each_matching_node_and_match() to remove the double iteration,
thus transforming the above code to:

    for_each_matching_node_and_match(np, table, &match)

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v4: New patch.

This series is based on the ARM sys_timer rework that I posted yesterday,
although patch 1/3 should be completely independant of that, and could
perhaps even be applied for 3.8 right now.

I hope that these patches can be taken through arm-soc tree for 3.9;
I will repost them based on 3.8-rc1 at the appropriate time.
---
 drivers/of/base.c  |   18 +++++++++++++-----
 include/linux/of.h |   15 +++++++++++++--
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f2f63c8..094271e 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -594,27 +594,35 @@ const struct of_device_id *of_match_node(const struct of_device_id *matches,
 EXPORT_SYMBOL(of_match_node);
 
 /**
- *	of_find_matching_node - Find a node based on an of_device_id match
- *				table.
+ *	of_find_matching_node_and_match - Find a node based on an of_device_id
+ *					  match table.
  *	@from:		The node to start searching from or NULL, the node
  *			you pass will not be searched, only the next one
  *			will; typically, you pass what the previous call
  *			returned. of_node_put() will be called on it
  *	@matches:	array of of device match structures to search in
+ *	@match		Updated to point at the matches entry which matched
  *
  *	Returns a node pointer with refcount incremented, use
  *	of_node_put() on it when done.
  */
-struct device_node *of_find_matching_node(struct device_node *from,
-					  const struct of_device_id *matches)
+struct device_node *of_find_matching_node_and_match(struct device_node *from,
+					const struct of_device_id *matches,
+					const struct of_device_id **match)
 {
 	struct device_node *np;
 
+	if (match)
+		*match = NULL;
+
 	read_lock(&devtree_lock);
 	np = from ? from->allnext : allnodes;
 	for (; np; np = np->allnext) {
-		if (of_match_node(matches, np) && of_node_get(np))
+		if (of_match_node(matches, np) && of_node_get(np)) {
+			if (match)
+				*match = matches;
 			break;
+		}
 	}
 	of_node_put(from);
 	read_unlock(&devtree_lock);
diff --git a/include/linux/of.h b/include/linux/of.h
index 681a6c8..1000496 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -180,11 +180,22 @@ extern struct device_node *of_find_compatible_node(struct device_node *from,
 #define for_each_compatible_node(dn, type, compatible) \
 	for (dn = of_find_compatible_node(NULL, type, compatible); dn; \
 	     dn = of_find_compatible_node(dn, type, compatible))
-extern struct device_node *of_find_matching_node(struct device_node *from,
-	const struct of_device_id *matches);
+extern struct device_node *of_find_matching_node_and_match(
+	struct device_node *from,
+	const struct of_device_id *matches,
+	const struct of_device_id **match);
+static inline struct device_node *of_find_matching_node(
+	struct device_node *from,
+	const struct of_device_id *matches)
+{
+	return of_find_matching_node_and_match(from, matches, NULL);
+}
 #define for_each_matching_node(dn, matches) \
 	for (dn = of_find_matching_node(NULL, matches); dn; \
 	     dn = of_find_matching_node(dn, matches))
+#define for_each_matching_node_and_match(dn, matches, match) \
+	for (dn = of_find_matching_node_and_match(NULL, matches, match); \
+	     dn; dn = of_find_matching_node_and_match(dn, matches, match))
 extern struct device_node *of_find_node_by_path(const char *path);
 extern struct device_node *of_find_node_by_phandle(phandle handle);
 extern struct device_node *of_get_parent(const struct device_node *node);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V4 2/3] clocksource: add common of_clksrc_init() function
  2012-11-20 23:12 [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Stephen Warren
@ 2012-11-20 23:12 ` Stephen Warren
  2013-01-02 18:16   ` Stephen Warren
  2012-11-20 23:12 ` [PATCH V4 3/3] ARM: tegra: move timer.c to drivers/clocksource/ Stephen Warren
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2012-11-20 23:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Warren <swarren@nvidia.com>

It is desirable to move all clocksource drivers to drivers/clocksource,
yet each requires its own initialization function. We'd rather not
pollute <linux/> with a header for each function. Instead, create a
single of_clksrc_init() function which will determine which clocksource
driver to initialize based on device tree.

Based on a similar patch for drivers/irqchip by Thomas Petazzoni.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v4:
* Use for_each_matching_node_and_match().
* Build a single of_device_id table, rather than a table of pointers to
  partial tables.
v3: Use a linker section to replace manually maintained table.
v2: New patch.
---
 drivers/clocksource/Kconfig       |    3 +++
 drivers/clocksource/Makefile      |    1 +
 drivers/clocksource/clksrc-of.c   |   35 +++++++++++++++++++++++++++++++++++
 include/asm-generic/vmlinux.lds.h |    9 +++++++++
 include/linux/clocksource.h       |    9 +++++++++
 5 files changed, 57 insertions(+)
 create mode 100644 drivers/clocksource/clksrc-of.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index c9f67de..4f300c5 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -1,3 +1,6 @@
+config CLKSRC_OF
+	bool
+
 config CLKSRC_I8253
 	bool
 
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 24fb888..29017a3 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_CLKSRC_OF)	+= clksrc-of.o
 obj-$(CONFIG_ATMEL_TCB_CLKSRC)	+= tcb_clksrc.o
 obj-$(CONFIG_X86_CYCLONE_TIMER)	+= cyclone.o
 obj-$(CONFIG_X86_PM_TIMER)	+= acpi_pm.o
diff --git a/drivers/clocksource/clksrc-of.c b/drivers/clocksource/clksrc-of.c
new file mode 100644
index 0000000..bdabdaa
--- /dev/null
+++ b/drivers/clocksource/clksrc-of.c
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/init.h>
+#include <linux/of.h>
+
+extern struct of_device_id __clksrc_of_table[];
+
+static const struct of_device_id __clksrc_of_table_sentinel
+	__used __section(__clksrc_of_table_end);
+
+void __init clocksource_of_init(void)
+{
+	struct device_node *np;
+	const struct of_device_id *match;
+	void (*init_func)(void);
+
+	for_each_matching_node_and_match(np, __clksrc_of_table, &match) {
+		init_func = match->data;
+		init_func();
+	}
+}
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index d1ea7ce..1e744c5 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -149,6 +149,14 @@
 #define TRACE_SYSCALLS()
 #endif
 
+#ifdef CONFIG_CLKSRC_OF
+#define CLKSRC_OF_TABLES() . = ALIGN(8);				\
+			   VMLINUX_SYMBOL(__clksrc_of_table) = .;	\
+			   *(__clksrc_of_table)				\
+			   *(__clksrc_of_table_end)
+#else
+#define CLKSRC_OF_TABLES()
+#endif
 
 #define KERNEL_DTB()							\
 	STRUCT_ALIGN();							\
@@ -493,6 +501,7 @@
 	DEV_DISCARD(init.rodata)					\
 	CPU_DISCARD(init.rodata)					\
 	MEM_DISCARD(init.rodata)					\
+	CLKSRC_OF_TABLES()						\
 	KERNEL_DTB()
 
 #define INIT_TEXT							\
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 4dceaf8..7944f14 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -332,4 +332,13 @@ extern int clocksource_mmio_init(void __iomem *, const char *,
 
 extern int clocksource_i8253_init(void);
 
+#ifdef CONFIG_CLKSRC_OF
+extern void clocksource_of_init(void);
+
+#define CLOCKSOURCE_OF_DECLARE(name, compat, fn)			\
+	static const struct of_device_id __clksrc_of_table_##name	\
+		__used __section(__clksrc_of_table)			\
+		 = { .compatible = compat, .data = fn };
+#endif
+
 #endif /* _LINUX_CLOCKSOURCE_H */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V4 3/3] ARM: tegra: move timer.c to drivers/clocksource/
  2012-11-20 23:12 [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Stephen Warren
  2012-11-20 23:12 ` [PATCH V4 2/3] clocksource: add common of_clksrc_init() function Stephen Warren
@ 2012-11-20 23:12 ` Stephen Warren
  2013-01-02 18:21   ` Stephen Warren
  2012-11-20 23:52 ` [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Rob Herring
  2012-11-21  9:53 ` Arnd Bergmann
  3 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2012-11-20 23:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Warren <swarren@nvidia.com>

Move arch/arm/mach-tegra/timer.c to drivers/clocksource/tegra20_timer.c
so that the code is co-located with other clocksource drivers, and to
reduce the size of the mach-tegra directory.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v4: Adjust to changes in the rest of the patch series.
v3: Rework using CLKSRC_OF_MATCH_TABLE() for registration, rather than a
    manually maintained table in clksrc-of.c.
v2: Rebase on ARM sys_timer rework, and addition of clocksource_of_init().
---
 arch/arm/Kconfig                                                |    1 +
 arch/arm/mach-tegra/Makefile                                    |    1 -
 arch/arm/mach-tegra/board-dt-tegra20.c                          |    3 ++-
 arch/arm/mach-tegra/board-dt-tegra30.c                          |    3 ++-
 arch/arm/mach-tegra/board.h                                     |    1 -
 drivers/clocksource/Makefile                                    |    1 +
 .../mach-tegra/timer.c => drivers/clocksource/tegra20_timer.c   |    7 ++-----
 7 files changed, 8 insertions(+), 9 deletions(-)
 rename arch/arm/mach-tegra/timer.c => drivers/clocksource/tegra20_timer.c (98%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 14f8160..9eb54f6 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -633,6 +633,7 @@ config ARCH_TEGRA
 	select ARCH_HAS_CPUFREQ
 	select CLKDEV_LOOKUP
 	select CLKSRC_MMIO
+	select CLKSRC_OF
 	select COMMON_CLK
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_GPIO
diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index 6f224f7..6ab3ed5 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -2,7 +2,6 @@ obj-y                                   += common.o
 obj-y                                   += io.o
 obj-y                                   += irq.o
 obj-y                                   += clock.o
-obj-y                                   += timer.o
 obj-y					+= fuse.o
 obj-y					+= pmc.o
 obj-y					+= flowctrl.o
diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c
index 794a7bf..0bee85a 100644
--- a/arch/arm/mach-tegra/board-dt-tegra20.c
+++ b/arch/arm/mach-tegra/board-dt-tegra20.c
@@ -15,6 +15,7 @@
  *
  */
 
+#include <linux/clocksource.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
@@ -194,7 +195,7 @@ DT_MACHINE_START(TEGRA_DT, "nVidia Tegra20 (Flattened Device Tree)")
 	.init_early	= tegra20_init_early,
 	.init_irq	= tegra_dt_init_irq,
 	.handle_irq	= gic_handle_irq,
-	.init_time	= tegra_init_timer,
+	.init_time	= clocksource_of_init,
 	.init_machine	= tegra_dt_init,
 	.init_late	= tegra_dt_init_late,
 	.restart	= tegra_assert_system_reset,
diff --git a/arch/arm/mach-tegra/board-dt-tegra30.c b/arch/arm/mach-tegra/board-dt-tegra30.c
index 08d3b19..a2b6cf1 100644
--- a/arch/arm/mach-tegra/board-dt-tegra30.c
+++ b/arch/arm/mach-tegra/board-dt-tegra30.c
@@ -23,6 +23,7 @@
  *
  */
 
+#include <linux/clocksource.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
@@ -104,7 +105,7 @@ DT_MACHINE_START(TEGRA30_DT, "NVIDIA Tegra30 (Flattened Device Tree)")
 	.init_early	= tegra30_init_early,
 	.init_irq	= tegra_dt_init_irq,
 	.handle_irq	= gic_handle_irq,
-	.init_time	= tegra_init_timer,
+	.init_time	= clocksource_of_init,
 	.init_machine	= tegra30_dt_init,
 	.init_late	= tegra_init_late,
 	.restart	= tegra_assert_system_reset,
diff --git a/arch/arm/mach-tegra/board.h b/arch/arm/mach-tegra/board.h
index 744cdd2..da8f5a3 100644
--- a/arch/arm/mach-tegra/board.h
+++ b/arch/arm/mach-tegra/board.h
@@ -55,5 +55,4 @@ static inline int harmony_pcie_init(void) { return 0; }
 
 void __init tegra_paz00_wifikill_init(void);
 
-extern void tegra_init_timer(void);
 #endif
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 29017a3..ecf37f3 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -16,5 +16,6 @@ obj-$(CONFIG_CLKSRC_NOMADIK_MTU)	+= nomadik-mtu.o
 obj-$(CONFIG_CLKSRC_DBX500_PRCMU)	+= clksrc-dbx500-prcmu.o
 obj-$(CONFIG_ARMADA_370_XP_TIMER)	+= time-armada-370-xp.o
 obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835_timer.o
+obj-$(CONFIG_ARCH_TEGRA)	+= tegra20_timer.o
 
 obj-$(CONFIG_CLKSRC_ARM_GENERIC)	+= arm_generic.o
diff --git a/arch/arm/mach-tegra/timer.c b/drivers/clocksource/tegra20_timer.c
similarity index 98%
rename from arch/arm/mach-tegra/timer.c
rename to drivers/clocksource/tegra20_timer.c
index b0036e5..3b2f947 100644
--- a/arch/arm/mach-tegra/timer.c
+++ b/drivers/clocksource/tegra20_timer.c
@@ -1,6 +1,4 @@
 /*
- * arch/arch/mach-tegra/timer.c
- *
  * Copyright (C) 2010 Google, Inc.
  *
  * Author:
@@ -33,8 +31,6 @@
 #include <asm/smp_twd.h>
 #include <asm/sched_clock.h>
 
-#include "board.h"
-
 #define RTC_SECONDS            0x08
 #define RTC_SHADOW_SECONDS     0x0c
 #define RTC_MILLISECONDS       0x10
@@ -168,7 +164,7 @@ static const struct of_device_id rtc_match[] __initconst = {
 	{}
 };
 
-void __init tegra_init_timer(void)
+static void __init tegra20_init_timer(void)
 {
 	struct device_node *np;
 	struct clk *clk;
@@ -272,6 +268,7 @@ void __init tegra_init_timer(void)
 #endif
 	register_persistent_clock(NULL, tegra_read_persistent_clock);
 }
+CLOCKSOURCE_OF_DECLARE(tegra20, "nvidia,tegra20-timer", tegra20_init_timer);
 
 #ifdef CONFIG_PM
 static u32 usec_config;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V4 1/3] of: introduce for_each_matching_node_and_match()
  2012-11-20 23:12 [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Stephen Warren
  2012-11-20 23:12 ` [PATCH V4 2/3] clocksource: add common of_clksrc_init() function Stephen Warren
  2012-11-20 23:12 ` [PATCH V4 3/3] ARM: tegra: move timer.c to drivers/clocksource/ Stephen Warren
@ 2012-11-20 23:52 ` Rob Herring
  2012-11-21  9:53 ` Arnd Bergmann
  3 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2012-11-20 23:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/20/2012 05:12 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> The following pattern of code is tempting:
> 
>     for_each_matching_node(np, table) {
>         match = of_match_node(table, np);
> 
> However, this results in iterating over table twice; the second time
> inside of_match_node(). The implementation of for_each_matching_node()
> already found the match, so this is redundant. Invent new function
> of_find_matching_node_and_match() and macro
> for_each_matching_node_and_match() to remove the double iteration,
> thus transforming the above code to:
> 
>     for_each_matching_node_and_match(np, table, &match)
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v4: New patch.
> 
> This series is based on the ARM sys_timer rework that I posted yesterday,
> although patch 1/3 should be completely independant of that, and could
> perhaps even be applied for 3.8 right now.

Agreed. I will apply for 3.8 assuming no further comments in the next
few days.

Rob

> I hope that these patches can be taken through arm-soc tree for 3.9;
> I will repost them based on 3.8-rc1 at the appropriate time.
> ---
>  drivers/of/base.c  |   18 +++++++++++++-----
>  include/linux/of.h |   15 +++++++++++++--
>  2 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f2f63c8..094271e 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -594,27 +594,35 @@ const struct of_device_id *of_match_node(const struct of_device_id *matches,
>  EXPORT_SYMBOL(of_match_node);
>  
>  /**
> - *	of_find_matching_node - Find a node based on an of_device_id match
> - *				table.
> + *	of_find_matching_node_and_match - Find a node based on an of_device_id
> + *					  match table.
>   *	@from:		The node to start searching from or NULL, the node
>   *			you pass will not be searched, only the next one
>   *			will; typically, you pass what the previous call
>   *			returned. of_node_put() will be called on it
>   *	@matches:	array of of device match structures to search in
> + *	@match		Updated to point at the matches entry which matched
>   *
>   *	Returns a node pointer with refcount incremented, use
>   *	of_node_put() on it when done.
>   */
> -struct device_node *of_find_matching_node(struct device_node *from,
> -					  const struct of_device_id *matches)
> +struct device_node *of_find_matching_node_and_match(struct device_node *from,
> +					const struct of_device_id *matches,
> +					const struct of_device_id **match)
>  {
>  	struct device_node *np;
>  
> +	if (match)
> +		*match = NULL;
> +
>  	read_lock(&devtree_lock);
>  	np = from ? from->allnext : allnodes;
>  	for (; np; np = np->allnext) {
> -		if (of_match_node(matches, np) && of_node_get(np))
> +		if (of_match_node(matches, np) && of_node_get(np)) {
> +			if (match)
> +				*match = matches;
>  			break;
> +		}
>  	}
>  	of_node_put(from);
>  	read_unlock(&devtree_lock);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 681a6c8..1000496 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -180,11 +180,22 @@ extern struct device_node *of_find_compatible_node(struct device_node *from,
>  #define for_each_compatible_node(dn, type, compatible) \
>  	for (dn = of_find_compatible_node(NULL, type, compatible); dn; \
>  	     dn = of_find_compatible_node(dn, type, compatible))
> -extern struct device_node *of_find_matching_node(struct device_node *from,
> -	const struct of_device_id *matches);
> +extern struct device_node *of_find_matching_node_and_match(
> +	struct device_node *from,
> +	const struct of_device_id *matches,
> +	const struct of_device_id **match);
> +static inline struct device_node *of_find_matching_node(
> +	struct device_node *from,
> +	const struct of_device_id *matches)
> +{
> +	return of_find_matching_node_and_match(from, matches, NULL);
> +}
>  #define for_each_matching_node(dn, matches) \
>  	for (dn = of_find_matching_node(NULL, matches); dn; \
>  	     dn = of_find_matching_node(dn, matches))
> +#define for_each_matching_node_and_match(dn, matches, match) \
> +	for (dn = of_find_matching_node_and_match(NULL, matches, match); \
> +	     dn; dn = of_find_matching_node_and_match(dn, matches, match))
>  extern struct device_node *of_find_node_by_path(const char *path);
>  extern struct device_node *of_find_node_by_phandle(phandle handle);
>  extern struct device_node *of_get_parent(const struct device_node *node);
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V4 1/3] of: introduce for_each_matching_node_and_match()
  2012-11-20 23:12 [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Stephen Warren
                   ` (2 preceding siblings ...)
  2012-11-20 23:52 ` [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Rob Herring
@ 2012-11-21  9:53 ` Arnd Bergmann
  2012-11-21 10:06   ` Grant Likely
  3 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2012-11-21  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 20 November 2012, Stephen Warren wrote:
> However, this results in iterating over table twice; the second time
> inside of_match_node(). The implementation of for_each_matching_node()
> already found the match, so this is redundant. Invent new function
> of_find_matching_node_and_match() and macro
> for_each_matching_node_and_match() to remove the double iteration,
> thus transforming the above code to:
> 
>     for_each_matching_node_and_match(np, table, &match)
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

This look useful, but I wonder if the interface would make more sense if you
make the last argument to the macro a normal pointer, rather than a
pointer-to-pointer. You can take the reference as part of the macro.

	Arnd

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V4 1/3] of: introduce for_each_matching_node_and_match()
  2012-11-21  9:53 ` Arnd Bergmann
@ 2012-11-21 10:06   ` Grant Likely
  2012-11-21 10:09     ` Thomas Petazzoni
  0 siblings, 1 reply; 9+ messages in thread
From: Grant Likely @ 2012-11-21 10:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 21, 2012 at 9:53 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 20 November 2012, Stephen Warren wrote:
>> However, this results in iterating over table twice; the second time
>> inside of_match_node(). The implementation of for_each_matching_node()
>> already found the match, so this is redundant. Invent new function
>> of_find_matching_node_and_match() and macro
>> for_each_matching_node_and_match() to remove the double iteration,
>> thus transforming the above code to:
>>
>>     for_each_matching_node_and_match(np, table, &match)
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>
> This look useful, but I wonder if the interface would make more sense if you
> make the last argument to the macro a normal pointer, rather than a
> pointer-to-pointer. You can take the reference as part of the macro.

To me that makes for harder to understand code. It *looks* like an
argument to a normal function call, but it gets changed by the caller.

g.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V4 1/3] of: introduce for_each_matching_node_and_match()
  2012-11-21 10:06   ` Grant Likely
@ 2012-11-21 10:09     ` Thomas Petazzoni
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Petazzoni @ 2012-11-21 10:09 UTC (permalink / raw)
  To: linux-arm-kernel


On Wed, 21 Nov 2012 10:06:16 +0000, Grant Likely wrote:
> On Wed, Nov 21, 2012 at 9:53 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 20 November 2012, Stephen Warren wrote:
> >> However, this results in iterating over table twice; the second time
> >> inside of_match_node(). The implementation of for_each_matching_node()
> >> already found the match, so this is redundant. Invent new function
> >> of_find_matching_node_and_match() and macro
> >> for_each_matching_node_and_match() to remove the double iteration,
> >> thus transforming the above code to:
> >>
> >>     for_each_matching_node_and_match(np, table, &match)
> >>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >
> > This look useful, but I wonder if the interface would make more sense if you
> > make the last argument to the macro a normal pointer, rather than a
> > pointer-to-pointer. You can take the reference as part of the macro.
> 
> To me that makes for harder to understand code. It *looks* like an
> argument to a normal function call, but it gets changed by the caller.

Agreed. Too much magic is too much.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V4 2/3] clocksource: add common of_clksrc_init() function
  2012-11-20 23:12 ` [PATCH V4 2/3] clocksource: add common of_clksrc_init() function Stephen Warren
@ 2013-01-02 18:16   ` Stephen Warren
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Warren @ 2013-01-02 18:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/20/2012 04:12 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> It is desirable to move all clocksource drivers to drivers/clocksource,
> yet each requires its own initialization function. We'd rather not
> pollute <linux/> with a header for each function. Instead, create a
> single of_clksrc_init() function which will determine which clocksource
> driver to initialize based on device tree.
> 
> Based on a similar patch for drivers/irqchip by Thomas Petazzoni.

I have applied this one patch to Tegra's for-3.9/arm-timer-rework branch
since. I'll send a pull request to include this is arm-soc in the next
day or two, so that others can build changes on top of it.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V4 3/3] ARM: tegra: move timer.c to drivers/clocksource/
  2012-11-20 23:12 ` [PATCH V4 3/3] ARM: tegra: move timer.c to drivers/clocksource/ Stephen Warren
@ 2013-01-02 18:21   ` Stephen Warren
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Warren @ 2013-01-02 18:21 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/20/2012 04:12 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Move arch/arm/mach-tegra/timer.c to drivers/clocksource/tegra20_timer.c
> so that the code is co-located with other clocksource drivers, and to
> reduce the size of the mach-tegra directory.

I have applied this to Tegra's for-3.9/cleanup branch.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-01-02 18:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-20 23:12 [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Stephen Warren
2012-11-20 23:12 ` [PATCH V4 2/3] clocksource: add common of_clksrc_init() function Stephen Warren
2013-01-02 18:16   ` Stephen Warren
2012-11-20 23:12 ` [PATCH V4 3/3] ARM: tegra: move timer.c to drivers/clocksource/ Stephen Warren
2013-01-02 18:21   ` Stephen Warren
2012-11-20 23:52 ` [PATCH V4 1/3] of: introduce for_each_matching_node_and_match() Rob Herring
2012-11-21  9:53 ` Arnd Bergmann
2012-11-21 10:06   ` Grant Likely
2012-11-21 10:09     ` Thomas Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).