Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/6] ARM: zImage: Allow the appending of a device tree binary
From: Dave Martin @ 2011-09-14 13:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315978906-15829-3-git-send-email-nico@fluxnic.net>

On Wed, Sep 14, 2011 at 01:41:42AM -0400, Nicolas Pitre wrote:
> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> This patch provides the ability to boot using a device tree that is appended
> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> 
> Signed-off-by: John Bonesio <bones@secretlab.ca>
> [nico: adjusted to latest zImage changes plus additional cleanups]
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> Acked-by: Tony Lindgren <tony@atomide.com>
> ---
>  arch/arm/Kconfig                |    8 ++++
>  arch/arm/boot/compressed/head.S |   70 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 75 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 5ebc5d922e..83323c2b1f 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1781,6 +1781,14 @@ config ZBOOT_ROM_SH_MOBILE_SDHI
>  
>  endchoice
>  
> +config ARM_APPENDED_DTB
> +	bool "Use appended device tree blob to zImage"
> +	depends on OF && !ZBOOT_ROM
> +	help
> +	  With this option, the boot code will look for a device tree binary
> +	  (dtb) appended to zImage
> +	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> +
>  config CMDLINE
>  	string "Default kernel command string"
>  	default ""
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index e95a598960..3ce5738ddb 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -216,6 +216,59 @@ restart:	adr	r0, LC0
>  		mov	r10, r6
>  #endif
>  
> +		mov	r5, #0			@ init dtb size to 0
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/*
> + *   r0  = delta
> + *   r2  = BSS start
> + *   r3  = BSS end
> + *   r4  = final kernel address
> + *   r5  = appended dtb size (still unknown)
> + *   r6  = _edata
> + *   r7  = architecture ID
> + *   r8  = atags/device tree pointer
> + *   r9  = size of decompressed image
> + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
> + *   r11 = GOT start
> + *   r12 = GOT end
> + *   sp  = stack pointer
> + *
> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> + * dtb data will get relocated along with the kernel if necessary.
> + */
> +
> +		ldr	lr, [r6, #0]
> +#ifndef __ARMEB__
> +		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
> +#else
> +		ldr	r1, =0xd00dfeed
> +#endif

Do we worry that garbage in memory after the zImage might match this
magic number?

For example, if an ordinary userspace program allocates a huge number
of pages and fills them with bogus device tree headers, is there a chance
that the those headers could remain in memory across a reboot?

In principle this could lead to a security hole on platforms where the
boot images don't append a device tree, by allowing an attacker to
override the bootargs etc.

I don't know whether this is exploitable in practice, but it's worth
thinking about (apologies if it's already been discussed)


A possible workaround is to put a relative pointer or a flag at the
start of the zImage, which we can poke with a non-zero value when
the device tree is appended.

This makes appending the device tree non-trivial, but it's still pretty
simple to do; something like:

$ echo 'boo' | dd bs=4 count=1 seek=4 conv=notrunc of=zImage
$ cat dtb >>zImage

(Where I assume that the affected word in the zImage is initially not
'boo').

Cheers
---Dave

^ permalink raw reply

* [PATCH 4/6] ARM: zImage: gather some string functions into string.c
From: Nicolas Pitre @ 2011-09-14 13:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110914131314.GC2104@arm.com>

On Wed, 14 Sep 2011, Dave Martin wrote:

> Have you seen this?
> http://en.wikipedia.org/wiki/Duff%27s_device

Yeah... I did stumble across something similar in the gcc test suite.

> Once I finished vomiting, this actually struck be as rather neat.
> 
> If I've understood right, it seems a good fit for the manual unrolling
> you do here.

Maybe.  However this patch is about moving code, not "improving" it.  ;-)


Nicolas

^ permalink raw reply

* [PATCH v2] AM3517 : support for suspend/resume
From: Abhilash K V @ 2011-09-14 13:48 UTC (permalink / raw)
  To: linux-arm-kernel

This patch-set adds support for suspension to RAM and
resumption on the AM3517. This includes:
1. Patch to disable dynamic sleep (as it is not supported
   on AM35xx).
2. Imported the unique suspend/resume sequence for AM3517,
   contained in the new file arch/arm/mach-omap2/sleep3517.S.

Caveat: If "no_console_suspend" is enabled (via boot-args),the
device doesnot resume but simply hangs.
Kevin's fix below should fix this:
 http://marc.info/?l=linux-omap&m=131593828001388&w=2#1

Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
Reviewed-by: Vaibhav Hiremath <hvaibhav@ti.com>
Signed-off-by: Abhilash K V <abhilash.kv@ti.com>
---
This patch is dependent on the following patch-sets:
* [PATCH v3 0/2] AM3517: Booting up
  at http://marc.info/?l=linux-omap&m=131548349725176&w=2
* [PATCH v2 0/3] AM35x: Adding PM init
  at http://marc.info/?l=linux-kernel&m=131548606728209&w=2

The patches are tested on master of tmlind/linux-omap-2.6.git.
Kernel version is 3.1-rc3 and last commit on top of which these patches
were added is:
	b148d763841161894ed6629794064065a834aa2b: Linux-omap rebuilt: Updated to
	use omap_sdrc_init

with the folowing commit reverted:
	f3637a5f2e2eb391ff5757bc83fb5de8f9726464: irq: Always set IRQF_ONESHOT
	if no primary handler is specified

Changes in v2:
 * Synchronised with the cleaned-up suspend-resume code for OMAP3
 * Removed unused *_get_restore_pointer code
 * Added SECURE_SRAM feature to disallow saving and restoring
   secure ram context for AM35x
 * Compacted the number of patches by squashing three closely coupled
   ones and eliminating one that was no longer needed.

 arch/arm/mach-omap2/Makefile          |    3 +-
 arch/arm/mach-omap2/id.c              |    4 +-
 arch/arm/mach-omap2/pm.h              |    3 +
 arch/arm/mach-omap2/pm34xx.c          |   21 ++++-
 arch/arm/mach-omap2/sleep3517.S       |  156 +++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/cpu.h |    2 +
 6 files changed, 183 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/mach-omap2/sleep3517.S

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 590e797..37f62ae 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -61,7 +61,7 @@ endif
 ifeq ($(CONFIG_PM),y)
 obj-$(CONFIG_ARCH_OMAP2)		+= pm24xx.o
 obj-$(CONFIG_ARCH_OMAP2)		+= sleep24xx.o
-obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o \
+obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o sleep3517.o \
 					   cpuidle34xx.o
 obj-$(CONFIG_ARCH_OMAP4)		+= pm44xx.o
 obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
@@ -70,6 +70,7 @@ obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3)	+= smartreflex-class3.o
 
 AFLAGS_sleep24xx.o			:=-Wa,-march=armv6
 AFLAGS_sleep34xx.o			:=-Wa,-march=armv7-a$(plus_sec)
+AFLAGS_sleep3517.o			:=-Wa,-march=armv7-a$(plus_sec)
 
 ifeq ($(CONFIG_PM_VERBOSE),y)
 CFLAGS_pm_bus.o				+= -DDEBUG
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index da71098..3e40c02 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -202,7 +202,9 @@ static void __init omap3_check_features(void)
 	if (cpu_is_omap3630())
 		omap_features |= OMAP3_HAS_192MHZ_CLK;
 	if (!cpu_is_omap3505() && !cpu_is_omap3517())
-		omap_features |= (OMAP3_HAS_IO_WAKEUP | OMAP3_HAS_SR);
+		omap_features |= (OMAP3_HAS_IO_WAKEUP
+					| OMAP3_HAS_SR
+					| OMAP3_HAS_SECURE_SRAM);
 
 	omap_features |= OMAP3_HAS_SDRC;
 
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index ce028f6..952eb2b 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -82,10 +82,13 @@ extern unsigned int omap24xx_cpu_suspend_sz;
 
 /* 3xxx */
 extern void omap34xx_cpu_suspend(int save_state);
+extern void omap3517_cpu_suspend(int save_state);
 
 /* omap3_do_wfi function pointer and size, for copy to SRAM */
 extern void omap3_do_wfi(void);
+extern void omap3517_do_wfi(void);
 extern unsigned int omap3_do_wfi_sz;
+extern unsigned int omap3517_do_wfi_sz;
 /* ... and its pointer from SRAM after copy */
 extern void (*omap3_do_wfi_sram)(void);
 
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 7255d9b..44f7bac 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -80,6 +80,7 @@ static LIST_HEAD(pwrst_list);
 
 static int (*_omap_save_secure_sram)(u32 *addr);
 void (*omap3_do_wfi_sram)(void);
+void (*omap3517_do_wfi_sram)(void);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
@@ -323,7 +324,10 @@ static void omap34xx_save_context(u32 *save)
 
 static int omap34xx_do_sram_idle(unsigned long save_state)
 {
-	omap34xx_cpu_suspend(save_state);
+	if (cpu_is_omap3505() || cpu_is_omap3517())
+		omap3517_cpu_suspend(save_state);
+	else
+		omap34xx_cpu_suspend(save_state);
 	return 0;
 }
 
@@ -485,6 +489,8 @@ console_still_active:
 
 int omap3_can_sleep(void)
 {
+	if (cpu_is_omap3505() || cpu_is_omap3517())
+		return 0;
 	if (!omap_uart_can_sleep())
 		return 0;
 	return 1;
@@ -843,11 +849,18 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
  */
 void omap_push_sram_idle(void)
 {
-	omap3_do_wfi_sram = omap_sram_push(omap3_do_wfi, omap3_do_wfi_sz);
+	if (cpu_is_omap3505() || cpu_is_omap3517())
+		omap3517_do_wfi_sram = omap_sram_push(omap3517_do_wfi,
+						 omap3517_do_wfi_sz);
+	else
+		omap3_do_wfi_sram = omap_sram_push(omap3_do_wfi,
+						 omap3_do_wfi_sz);
 
 	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
-		_omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
-				save_secure_ram_context_sz);
+		if (omap3_has_secure_sram())
+			_omap_save_secure_sram = omap_sram_push(
+						save_secure_ram_context,
+						save_secure_ram_context_sz);
 }
 
 static void __init pm_errata_configure(void)
diff --git a/arch/arm/mach-omap2/sleep3517.S b/arch/arm/mach-omap2/sleep3517.S
new file mode 100644
index 0000000..a4147bd
--- /dev/null
+++ b/arch/arm/mach-omap2/sleep3517.S
@@ -0,0 +1,156 @@
+/*
+ * AM3505/3517 Sleep Code.
+ * Ranjith Lohithakshan <ranjithl@ti.com>
+ *
+ * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; 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 <asm/assembler.h>
+#include <plat/sram.h>
+#include <mach/io.h>
+
+#include "cm2xxx_3xxx.h"
+#include "prm2xxx_3xxx.h"
+#include "sdrc.h"
+#include "control.h"
+
+#define CM_IDLEST1_CORE_V	OMAP34XX_CM_REGADDR(CORE_MOD, CM_IDLEST1)
+#define CM_CLKST_CORE_V		OMAP34XX_CM_REGADDR(CORE_MOD,\
+					 OMAP3430_CM_CLKSTST)
+#define CM_ICLKEN1_CORE_V	OMAP34XX_CM_REGADDR(CORE_MOD, CM_ICLKEN1)
+
+#define EMIF_PM_CTR_V		OMAP2_L3_IO_ADDRESS(0x6D000038)
+#define OMAP3517_CONF1_REG_V	OMAP2_L4_IO_ADDRESS(0x48002584)
+
+/*
+ * Forces OMAP into idle state
+ *
+ * omap3517_suspend() - This bit of code just executes the WFI
+ * for normal idles.
+ *
+ */
+ENTRY(omap3517_cpu_suspend)
+	stmfd	sp!, {r4-r12, lr}		@ save registers on stack
+loop:
+	/*b	loop*/	@Enable to debug by stepping through code
+	ldr	r4, omap3517_do_wfi_sram_addr
+	ldr	r5, [r4]
+	/*
+	 * Since OFF mode is unsupported r0 is always 0, and so no need to
+	 * save context
+	 */
+	bx	r5                      @  jump to the WFI code in SRAM
+/*
+ * Local variables
+ */
+omap3517_do_wfi_sram_addr:
+	.word omap3517_do_wfi_sram
+
+/* ===================================
+ * == WFI instruction => Enter idle ==
+ * ===================================
+ */
+
+/*
+ * Do WFI instruction
+ * Includes the resume path for non-OFF modes
+ *
+ * This code gets copied to internal SRAM and is accessible
+ * from both SDRAM and SRAM
+ * Always executed from SRAM (omap3517_do_wfi_sram),as AM35x does not
+ * support OFF mode
+ */
+	.align  3
+ENTRY(omap3517_do_wfi)
+	/* Put EMIF in self-refresh */
+	ldr	r4, emif_pm_ctrl
+	ldr	r5, [r4]
+	orr	r5, r5, #0x200
+	str	r5, [r4]
+
+	/* Disable SDRC and Control Module */
+	ldr	r4, cm_iclken1_core
+	ldr	r5, [r4]
+	str	r5, iclk_core_enable
+	ldr	r4, cm_iclken1_core
+	ldr	r5, clk_core_disable
+	str	r5, [r4]
+wait_sdrc_ok:
+	ldr	r4, cm_idlest1_core
+	ldr	r5, [r4]
+	and	r5, r5, #0x2
+	cmp	r5, #0x2
+	bne	wait_sdrc_ok
+
+	/* Gate DDR Phy clock */
+	ldr	r4, omap3517_conf1
+	ldr	r5, emif_phy_gate
+	str	r5, [r4]
+
+	/* Data memory barrier and Data sync barrier */
+	mov	r1, #0
+	mcr	p15, 0, r1, c7, c10, 4
+	mcr	p15, 0, r1, c7, c10, 5
+
+	wfi
+
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+
+	/* Enable SDRC and Control Module */
+	ldr	r4, cm_iclken1_core
+	ldr	r5, iclk_core_enable
+	str	r5, [r4]
+
+	/* Enable DDR Phy Clock */
+	ldr	r4, omap3517_conf1
+	ldr	r5, emif_phy_enable
+	str	r5, [r4]
+
+	/* Take EMIF out of self-refresh */
+	ldr	r4, emif_pm_ctrl
+	ldr	r5, [r4]
+	bic	r5, r5, #0x200
+	str	r5, [r4]
+
+	ldmfd	sp!, {r4-r12, pc}		@ restore regs and return
+
+clk_core_disable:
+	.word	0x0
+iclk_core_enable:
+	.word	0x0
+emif_phy_gate:
+	.word	0x2620
+emif_phy_enable:
+	.word	0x8620
+cm_idlest1_core:
+	.word	CM_IDLEST1_CORE_V
+cm_clkst_core:
+	.word	CM_CLKST_CORE_V
+emif_pm_ctrl:
+	.word	EMIF_PM_CTR_V
+cm_iclken1_core:
+	.word	CM_ICLKEN1_CORE_V
+omap3517_conf1:
+	.word	OMAP3517_CONF1_REG_V
+ENTRY(omap3517_do_wfi_sz)
+	.word	. - omap3517_do_wfi
+
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 294e015..7daeb36 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -492,6 +492,7 @@ extern u32 omap_features;
 #define OMAP4_HAS_MPU_1_2GHZ		BIT(9)
 #define OMAP4_HAS_MPU_1_5GHZ		BIT(10)
 #define OMAP3_HAS_SR			BIT(11)
+#define OMAP3_HAS_SECURE_SRAM		BIT(12)
 
 
 #define OMAP3_HAS_FEATURE(feat,flag)			\
@@ -509,6 +510,7 @@ OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK)
 OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP)
 OMAP3_HAS_FEATURE(sdrc, SDRC)
 OMAP3_HAS_FEATURE(sr, SR)
+OMAP3_HAS_FEATURE(secure_sram, SECURE_SRAM)
 
 /*
  * Runtime detection of OMAP4 features
-- 
1.7.1

^ permalink raw reply related

* [PATCH 2/6] ARM: zImage: Allow the appending of a device tree binary
From: Nicolas Pitre @ 2011-09-14 14:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110914133244.GE2104@arm.com>

On Wed, 14 Sep 2011, Dave Martin wrote:

> On Wed, Sep 14, 2011 at 01:41:42AM -0400, Nicolas Pitre wrote:
> > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > 
> > This patch provides the ability to boot using a device tree that is appended
> > to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > 
> > Signed-off-by: John Bonesio <bones@secretlab.ca>
> > [nico: adjusted to latest zImage changes plus additional cleanups]
> > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > ---
> >  arch/arm/Kconfig                |    8 ++++
> >  arch/arm/boot/compressed/head.S |   70 +++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 75 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 5ebc5d922e..83323c2b1f 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1781,6 +1781,14 @@ config ZBOOT_ROM_SH_MOBILE_SDHI
> >  
> >  endchoice
> >  
> > +config ARM_APPENDED_DTB
> > +	bool "Use appended device tree blob to zImage"
> > +	depends on OF && !ZBOOT_ROM
> > +	help
> > +	  With this option, the boot code will look for a device tree binary
> > +	  (dtb) appended to zImage
> > +	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > +
> >  config CMDLINE
> >  	string "Default kernel command string"
> >  	default ""
> > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > index e95a598960..3ce5738ddb 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -216,6 +216,59 @@ restart:	adr	r0, LC0
> >  		mov	r10, r6
> >  #endif
> >  
> > +		mov	r5, #0			@ init dtb size to 0
> > +#ifdef CONFIG_ARM_APPENDED_DTB
> > +/*
> > + *   r0  = delta
> > + *   r2  = BSS start
> > + *   r3  = BSS end
> > + *   r4  = final kernel address
> > + *   r5  = appended dtb size (still unknown)
> > + *   r6  = _edata
> > + *   r7  = architecture ID
> > + *   r8  = atags/device tree pointer
> > + *   r9  = size of decompressed image
> > + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
> > + *   r11 = GOT start
> > + *   r12 = GOT end
> > + *   sp  = stack pointer
> > + *
> > + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> > + * dtb data will get relocated along with the kernel if necessary.
> > + */
> > +
> > +		ldr	lr, [r6, #0]
> > +#ifndef __ARMEB__
> > +		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
> > +#else
> > +		ldr	r1, =0xd00dfeed
> > +#endif
> 
> Do we worry that garbage in memory after the zImage might match this
> magic number?
> 
> For example, if an ordinary userspace program allocates a huge number
> of pages and fills them with bogus device tree headers, is there a chance
> that the those headers could remain in memory across a reboot?

In theory this _could_ be possible.  However I don't expect this feature 
to be enabled if you are not going to actually use it, especially in a 
production setup.  If you are not appending a DTB to your kernel then 
there is simply no point keeping this config option set (normally you 
should use this option only because you have no other choices).


Nicolas

^ permalink raw reply

* [PATCH 2/6] ARM: zImage: Allow the appending of a device tree binary
From: Dave Martin @ 2011-09-14 14:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1109140957000.20358@xanadu.home>

On Wed, Sep 14, 2011 at 10:04:28AM -0400, Nicolas Pitre wrote:
> On Wed, 14 Sep 2011, Dave Martin wrote:
> 
> > On Wed, Sep 14, 2011 at 01:41:42AM -0400, Nicolas Pitre wrote:
> > > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > 
> > > This patch provides the ability to boot using a device tree that is appended
> > > to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > > 
> > > Signed-off-by: John Bonesio <bones@secretlab.ca>
> > > [nico: adjusted to latest zImage changes plus additional cleanups]
> > > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> > > Acked-by: Tony Lindgren <tony@atomide.com>
> > > ---
> > >  arch/arm/Kconfig                |    8 ++++
> > >  arch/arm/boot/compressed/head.S |   70 +++++++++++++++++++++++++++++++++++++--
> > >  2 files changed, 75 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index 5ebc5d922e..83323c2b1f 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -1781,6 +1781,14 @@ config ZBOOT_ROM_SH_MOBILE_SDHI
> > >  
> > >  endchoice
> > >  
> > > +config ARM_APPENDED_DTB
> > > +	bool "Use appended device tree blob to zImage"
> > > +	depends on OF && !ZBOOT_ROM
> > > +	help
> > > +	  With this option, the boot code will look for a device tree binary
> > > +	  (dtb) appended to zImage
> > > +	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > > +
> > >  config CMDLINE
> > >  	string "Default kernel command string"
> > >  	default ""
> > > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > > index e95a598960..3ce5738ddb 100644
> > > --- a/arch/arm/boot/compressed/head.S
> > > +++ b/arch/arm/boot/compressed/head.S
> > > @@ -216,6 +216,59 @@ restart:	adr	r0, LC0
> > >  		mov	r10, r6
> > >  #endif
> > >  
> > > +		mov	r5, #0			@ init dtb size to 0
> > > +#ifdef CONFIG_ARM_APPENDED_DTB
> > > +/*
> > > + *   r0  = delta
> > > + *   r2  = BSS start
> > > + *   r3  = BSS end
> > > + *   r4  = final kernel address
> > > + *   r5  = appended dtb size (still unknown)
> > > + *   r6  = _edata
> > > + *   r7  = architecture ID
> > > + *   r8  = atags/device tree pointer
> > > + *   r9  = size of decompressed image
> > > + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
> > > + *   r11 = GOT start
> > > + *   r12 = GOT end
> > > + *   sp  = stack pointer
> > > + *
> > > + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> > > + * dtb data will get relocated along with the kernel if necessary.
> > > + */
> > > +
> > > +		ldr	lr, [r6, #0]
> > > +#ifndef __ARMEB__
> > > +		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
> > > +#else
> > > +		ldr	r1, =0xd00dfeed
> > > +#endif
> > 
> > Do we worry that garbage in memory after the zImage might match this
> > magic number?
> > 
> > For example, if an ordinary userspace program allocates a huge number
> > of pages and fills them with bogus device tree headers, is there a chance
> > that the those headers could remain in memory across a reboot?
> 
> In theory this _could_ be possible.  However I don't expect this feature 
> to be enabled if you are not going to actually use it, especially in a 
> production setup.  If you are not appending a DTB to your kernel then 
> there is simply no point keeping this config option set (normally you 
> should use this option only because you have no other choices).

That seems reasonable.

Should we document this recommendation, in the Kconfig help or
Documentation/?


On the other hand, if there's any chance this might need to be fixed later,
it could make sense to fix it now, particularly given that the fix is simple
(based on the assumption that whatever is documented, people can and will
sometimes do the wrong thing when deploying a platform...)

Anyway, I don't have a strong feeling on this myself.

Cheers
---Dave

^ permalink raw reply

* [PATCH 4/6] ARM: zImage: gather some string functions into string.c
From: Dave Martin @ 2011-09-14 14:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1109140942250.20358@xanadu.home>

On Wed, Sep 14, 2011 at 09:45:00AM -0400, Nicolas Pitre wrote:
> On Wed, 14 Sep 2011, Dave Martin wrote:
> 
> > Have you seen this?
> > http://en.wikipedia.org/wiki/Duff%27s_device
> 
> Yeah... I did stumble across something similar in the gcc test suite.
> 
> > Once I finished vomiting, this actually struck be as rather neat.
> > 
> > If I've understood right, it seems a good fit for the manual unrolling
> > you do here.
> 
> Maybe.  However this patch is about moving code, not "improving" it.  ;-)

Fair enough -- it was really just a side-comment rather than an issue
with the patch.

---Dave

^ permalink raw reply

* [PATCH 4/6] ARM: zImage: gather some string functions into string.c
From: Nicolas Pitre @ 2011-09-14 14:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110914142326.GG2104@arm.com>

On Wed, 14 Sep 2011, Dave Martin wrote:

> On Wed, Sep 14, 2011 at 09:45:00AM -0400, Nicolas Pitre wrote:
> > On Wed, 14 Sep 2011, Dave Martin wrote:
> > 
> > > Have you seen this?
> > > http://en.wikipedia.org/wiki/Duff%27s_device
> > 
> > Yeah... I did stumble across something similar in the gcc test suite.
> > 
> > > Once I finished vomiting, this actually struck be as rather neat.
> > > 
> > > If I've understood right, it seems a good fit for the manual unrolling
> > > you do here.
> > 
> > Maybe.  However this patch is about moving code, not "improving" it.  ;-)
> 
> Fair enough -- it was really just a side-comment rather than an issue
> with the patch.

Might be worth investigating separately though.


Nicolas

^ permalink raw reply

* [PATCH 2/6] ARM: zImage: Allow the appending of a device tree binary
From: Nicolas Pitre @ 2011-09-14 15:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110914142030.GF2104@arm.com>

On Wed, 14 Sep 2011, Dave Martin wrote:

> On Wed, Sep 14, 2011 at 10:04:28AM -0400, Nicolas Pitre wrote:
> > On Wed, 14 Sep 2011, Dave Martin wrote:
> > 
> > > On Wed, Sep 14, 2011 at 01:41:42AM -0400, Nicolas Pitre wrote:
> > > > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > > 
> > > > This patch provides the ability to boot using a device tree that is appended
> > > > to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > > > 
> > > > Signed-off-by: John Bonesio <bones@secretlab.ca>
> > > > [nico: adjusted to latest zImage changes plus additional cleanups]
> > > > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> > > > Acked-by: Tony Lindgren <tony@atomide.com>
> > > > ---
> > > >  arch/arm/Kconfig                |    8 ++++
> > > >  arch/arm/boot/compressed/head.S |   70 +++++++++++++++++++++++++++++++++++++--
> > > >  2 files changed, 75 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > index 5ebc5d922e..83323c2b1f 100644
> > > > --- a/arch/arm/Kconfig
> > > > +++ b/arch/arm/Kconfig
> > > > @@ -1781,6 +1781,14 @@ config ZBOOT_ROM_SH_MOBILE_SDHI
> > > >  
> > > >  endchoice
> > > >  
> > > > +config ARM_APPENDED_DTB
> > > > +	bool "Use appended device tree blob to zImage"
> > > > +	depends on OF && !ZBOOT_ROM
> > > > +	help
> > > > +	  With this option, the boot code will look for a device tree binary
> > > > +	  (dtb) appended to zImage
> > > > +	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > > > +
> > > >  config CMDLINE
> > > >  	string "Default kernel command string"
> > > >  	default ""
> > > > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > > > index e95a598960..3ce5738ddb 100644
> > > > --- a/arch/arm/boot/compressed/head.S
> > > > +++ b/arch/arm/boot/compressed/head.S
> > > > @@ -216,6 +216,59 @@ restart:	adr	r0, LC0
> > > >  		mov	r10, r6
> > > >  #endif
> > > >  
> > > > +		mov	r5, #0			@ init dtb size to 0
> > > > +#ifdef CONFIG_ARM_APPENDED_DTB
> > > > +/*
> > > > + *   r0  = delta
> > > > + *   r2  = BSS start
> > > > + *   r3  = BSS end
> > > > + *   r4  = final kernel address
> > > > + *   r5  = appended dtb size (still unknown)
> > > > + *   r6  = _edata
> > > > + *   r7  = architecture ID
> > > > + *   r8  = atags/device tree pointer
> > > > + *   r9  = size of decompressed image
> > > > + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
> > > > + *   r11 = GOT start
> > > > + *   r12 = GOT end
> > > > + *   sp  = stack pointer
> > > > + *
> > > > + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> > > > + * dtb data will get relocated along with the kernel if necessary.
> > > > + */
> > > > +
> > > > +		ldr	lr, [r6, #0]
> > > > +#ifndef __ARMEB__
> > > > +		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
> > > > +#else
> > > > +		ldr	r1, =0xd00dfeed
> > > > +#endif
> > > 
> > > Do we worry that garbage in memory after the zImage might match this
> > > magic number?
> > > 
> > > For example, if an ordinary userspace program allocates a huge number
> > > of pages and fills them with bogus device tree headers, is there a chance
> > > that the those headers could remain in memory across a reboot?
> > 
> > In theory this _could_ be possible.  However I don't expect this feature 
> > to be enabled if you are not going to actually use it, especially in a 
> > production setup.  If you are not appending a DTB to your kernel then 
> > there is simply no point keeping this config option set (normally you 
> > should use this option only because you have no other choices).
> 
> That seems reasonable.
> 
> Should we document this recommendation, in the Kconfig help or
> Documentation/?

I'll add some scary wording to the help text, and make it depend on 
EXPERIMENTAL as well.  I prefer not to impose some expectations on the 
zImage layout for this even if not in use, like being stuck with an 
offset that we'll always have to guard against corruption due to people 
blindly scripting the zImage poking you suggested even when it is not 
needed.

People will find ways to screw it up if they really want to anyway.  So 
I'd lean towards keeping this simple and not create any legacy around 
this hopefully temporary accommodation feature.


Nicolas

^ permalink raw reply

* [PATCH] ARM: rename temp variable in read*_relaxed()
From: Olof Johansson @ 2011-09-14 15:11 UTC (permalink / raw)
  To: linux-arm-kernel

This resolves the following sparse warning from readl() and other macros,
which ends up embedding readl_relaxed() using the same variable:

arch/arm/mach-tegra/dma.c:169:8: warning: symbol '__v' shadows an earlier one
arch/arm/mach-tegra/dma.c:169:8: originally declared here

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/arm/include/asm/io.h |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index d66605d..bbeb11e 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -189,11 +189,11 @@ extern void _memset_io(volatile void __iomem *, int, size_t);
  * IO port primitives for more information.
  */
 #ifdef __mem_pci
-#define readb_relaxed(c) ({ u8  __v = __raw_readb(__mem_pci(c)); __v; })
-#define readw_relaxed(c) ({ u16 __v = le16_to_cpu((__force __le16) \
-					__raw_readw(__mem_pci(c))); __v; })
-#define readl_relaxed(c) ({ u32 __v = le32_to_cpu((__force __le32) \
-					__raw_readl(__mem_pci(c))); __v; })
+#define readb_relaxed(c) ({ u8  __r = __raw_readb(__mem_pci(c)); __r; })
+#define readw_relaxed(c) ({ u16 __r = le16_to_cpu((__force __le16) \
+					__raw_readw(__mem_pci(c))); __r; })
+#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
+					__raw_readl(__mem_pci(c))); __r; })
 
 #define writeb_relaxed(v,c)	((void)__raw_writeb(v,__mem_pci(c)))
 #define writew_relaxed(v,c)	((void)__raw_writew((__force u16) \
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 13/25] OMAP4: PM: Add WakeupGen module as OMAP gic_arch_extn
From: Tony Lindgren @ 2011-09-14 15:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMQu2gyYETPZRJu=4MWX6bULgmMaJb8MRwfhvURtOuyyWhLkAQ@mail.gmail.com>

* Shilimkar, Santosh <santosh.shilimkar@ti.com> [110913 22:01]:
> Tony,
> On Wed, Sep 14, 2011 at 2:06 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Santosh Shilimkar <santosh.shilimkar@ti.com> [110904 06:23]:
> >> OMAP WakeupGen is the interrupt controller extension used along
> >> with ARM GIC to wake the CPU out from low power states on
> >> external interrupts.
> >>
> >> The WakeupGen unit is responsible for generating wakeup event
> >> from the incoming interrupts and enable bits. It is implemented
> >> in MPU always ON power domain. During normal operation,
> >> WakeupGen delivers external interrupts directly to the GIC.
> > ...
> >
> >> + ? ? /*
> >> + ? ? ?* Override GIC architecture specific functions to add
> >> + ? ? ?* OMAP WakeupGen interrupt controller along with GIC
> >> + ? ? ?*/
> >> + ? ? gic_arch_extn.irq_mask = wakeupgen_mask;
> >> + ? ? gic_arch_extn.irq_unmask = wakeupgen_unmask;
> >> + ? ? gic_arch_extn.irq_set_wake = wakeupgen_set_wake;
> >> + ? ? gic_arch_extn.flags = IRQCHIP_MASK_ON_SUSPEND;
> >
> > As I've commented before, there should not be any need to tweak
> > the wakeupgen registers for each interrupt during the runtime.
> >
> And I gave you all the reasons why it needs to be done this way.

Hmm, I don't think you ever answered the main question:

Why would you need to write the wakeupgen registers for every
interrupt during the runtime instead of arming them only when
entering idle?
 
> > AFAIK the wakeupgen registers only need to be armed every time
> > before entering idle.
> >
> No that doesn't work and it completely hacky approach.

And how is writing the registers over and over again unnecessarily
a non-hacky approach?

> This problem is for all SOC's using A9 SMP and GIC and every soc
> has an architecture specific interrupt controller extension. And that
> was the reason the GIC arch_extn was proposed.
> It's just another IRQCHIP and works seamlessly being part of the
> framework.  And it will also initialized with primary IRQCHIP( GIC).

Sure, but I don't know if there is really a need for for that?

Regards,

Tony

^ permalink raw reply

* [PATCH 0/6] zImage improvements with DTB append and ATAG compatibility
From: Thomas Abraham @ 2011-09-14 15:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315978906-15829-1-git-send-email-nico@fluxnic.net>

On 14 September 2011 11:11, Nicolas Pitre <nico@fluxnic.net> wrote:
> This is the latest incarnation of my patch series allowing a DTB
> to be appended to zImage, including the ATAG conversion wrapper and
> other minor cleanups.
>
> This can also be pulled from:
>
> ? ? ? ?git://git.linaro.org/people/nico/linux zImage_DTB_append
>
> This works for me on the test setup I have.
> ACKs/Tested-By's would be appreciated.


Works fine for Exynos4 device tree support.

Tested-by: Thomas Abraham <thomas.abraham@linaro.org>

>
>
>
>
> Nicolas
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply

* [PATCH] ARM: rename temp variable in read*_relaxed()
From: Nicolas Pitre @ 2011-09-14 15:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316013082-21641-1-git-send-email-olof@lixom.net>

On Wed, 14 Sep 2011, Olof Johansson wrote:

> This resolves the following sparse warning from readl() and other macros,
> which ends up embedding readl_relaxed() using the same variable:
> 
> arch/arm/mach-tegra/dma.c:169:8: warning: symbol '__v' shadows an earlier one
> arch/arm/mach-tegra/dma.c:169:8: originally declared here
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

> ---
>  arch/arm/include/asm/io.h |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> index d66605d..bbeb11e 100644
> --- a/arch/arm/include/asm/io.h
> +++ b/arch/arm/include/asm/io.h
> @@ -189,11 +189,11 @@ extern void _memset_io(volatile void __iomem *, int, size_t);
>   * IO port primitives for more information.
>   */
>  #ifdef __mem_pci
> -#define readb_relaxed(c) ({ u8  __v = __raw_readb(__mem_pci(c)); __v; })
> -#define readw_relaxed(c) ({ u16 __v = le16_to_cpu((__force __le16) \
> -					__raw_readw(__mem_pci(c))); __v; })
> -#define readl_relaxed(c) ({ u32 __v = le32_to_cpu((__force __le32) \
> -					__raw_readl(__mem_pci(c))); __v; })
> +#define readb_relaxed(c) ({ u8  __r = __raw_readb(__mem_pci(c)); __r; })
> +#define readw_relaxed(c) ({ u16 __r = le16_to_cpu((__force __le16) \
> +					__raw_readw(__mem_pci(c))); __r; })
> +#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
> +					__raw_readl(__mem_pci(c))); __r; })
>  
>  #define writeb_relaxed(v,c)	((void)__raw_writeb(v,__mem_pci(c)))
>  #define writew_relaxed(v,c)	((void)__raw_writew((__force u16) \
> -- 
> 1.7.4.1
> 

^ permalink raw reply

* [PATCH] arm: omap: Fix build error in ispccdc.c
From: Roedel, Joerg @ 2011-09-14 15:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315317735-5255-1-git-send-email-joerg.roedel@amd.com>

Ping?

On Tue, Sep 06, 2011 at 10:02:15AM -0400, Joerg Roedel wrote:
> The following build error occurs with 3.1-rc5:
> 
>   CC      drivers/media/video/omap3isp/ispccdc.o
> /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c: In function 'ccdc_lsc_config':
> /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c:427:2: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
> /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c:427:6: warning: assignment makes pointer from integer without a cast [enabled by default]
> 
> This patch adds the missing 'linux/slab.h' include to fix the problem.
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: linux-media at vger.kernel.org
> Cc: linux-omap at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/media/video/omap3isp/ispccdc.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
> index 9d3459d..80796eb 100644
> --- a/drivers/media/video/omap3isp/ispccdc.c
> +++ b/drivers/media/video/omap3isp/ispccdc.c
> @@ -31,6 +31,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/mm.h>
>  #include <linux/sched.h>
> +#include <linux/slab.h>
>  #include <media/v4l2-event.h>
>  
>  #include "isp.h"
> -- 
> 1.7.4.1

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

^ permalink raw reply

* [PATCH v2 2/2] iommu/omap: migrate to the generic fault report mechanism
From: Roedel, Joerg @ 2011-09-14 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315941989-17199-1-git-send-email-ohad@wizery.com>

On Tue, Sep 13, 2011 at 03:26:29PM -0400, Ohad Ben-Cohen wrote:
> Start using the generic fault report mechanism, as provided by
> the IOMMU core, and remove its now-redundant omap_iommu_set_isr API.
> 
> Currently we're only interested in letting upper layers know about the
> fault, so in case the faulting device is a remote processor, they could
> restart it.
> 
> Dynamic PTE/TLB loading is not supported.
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> ---
>  arch/arm/plat-omap/include/plat/iommu.h |    3 +--
>  drivers/iommu/omap-iommu.c              |   31 +++----------------------------
>  2 files changed, 4 insertions(+), 30 deletions(-)

Applied both to new branch iommu/fault-reporting and merged into my next
branch. Thanks Ohad.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

^ permalink raw reply

* [PATCH V3 3/6] MTD : add the database for the NANDs
From: Brian Norris @ 2011-09-14 15:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTi=59br1KueZupGevB4DgP7UqBKRgbGgRvsjgH-8@mail.gmail.com>

Hi,

I see nothing has happened with this thread recently. It doesn't
deserve to die though.

On Thu, Mar 31, 2011 at 10:17 AM, Huang Shijie <shijie8@gmail.com> wrote:
> 2011/3/31 Artem Bityutskiy <dedekind1@gmail.com>:
>>> > If you have new chips to support in the future, you should add them in
>>> > drivers/mtd/nand/nand_ids.c and not keep this file.
>>> >
>>> The data structure ?nand_flash_dev{} does not contain enough information.
>>> So I want to the nand_device_info{} to replace it in future.
>>
>> Just add this information, if it is of generic nature (like SLC/MLC
>> flag, required ECC strength, etc).

Are SLC/MLC and ECC strength necessary to track? I don't see a good
benefit to cost ratio of trying to detect ECC strength especially,
since the levels specified in data sheets vary without much pattern
and nobody provides a way for decoding this from ID; do you really
want to have to make separate chip entries for every single chip that
comes around?

>>> > I still do not understand why this would be needed, is it because the generic
>>> > code does not provide enough informations for your driver?
>>> >
>>> yes.
>>>
>>> IMHO, the generic code is somehow trapped in a wrong way. :(

I may agree, but can you be more specific on the trap?

>>> Paring the id bytes is not sufficient to get all the information you
>>> need which is faced by me.

What's an alternative to "paring the id bytes"?

>>> What's more, I think the paring code is ugly, see the nand_get_flash_type().
>>
>> You are welcome to fix this. There is _a lot_ of ugly code in MTD
>> because no one loves it. Give it some love :-)

While I agree that a lot of this is ugly, it is mostly a matter of
necessity. NAND manufacturers do not have a standard (ONFI doesn't
really count, since there's basically 1 manufacturer using it), and so
the most "generic" code is to have different code paths for different
manufacturers (often with exceptions to each rule, since even a single
manufacturer changes its standards arbitrarily). Then you find
manufacturers like Toshiba that recently don't have any (published)
pattern at all for detecting OOB size generically.

Thus, it seems to me like we will need some form of the approach
provided by Huang. As I've found, there are some things that just
can't be decoded from the ID these days, so our ID table will need to
be able to track:
* full ID string (not just the 2nd byte "Device ID")
* relevant bad block scanning options

FWIW, we already implement an exception table in our own driver that
includes the above 2 items, for chips that can't be handled with the
table/detection structure as-is. We don't particularly need SLC/MLC
but I don't object to recording it if we can determine it reliably,
and while ECC level could be useful, it is difficult to discover
generically, as I mentioned above.

>>> Why not create a new database which contains all the necessary
>>> information for a nand, and can be easy
>>> find by the id bytes as the keyword?
>>
>> You can create this database by extending/improving/cleaning up the
>> existing code base with a nice series of patches.
>>
> ok. I will try to fix the generic code before my driver is submitted.

What happened to this statement? I see that your driver was submitted,
but no efforts were made for chip detection cleanup. Were you
satisfied with the current detection?

While we're on the subject, I'll direct your attention to this NAND
data table that I've worked on. It should provide a decent summary of
most of the data sheets I have sorted through. It should also show you
that certain things are going to be very hard to detect (e.g., ECC
level). It should also show that while some chips are not supported by
standard code (e.g., weird bad block scanning features that haven't
been supported, non-standard OOB sizes with no given pattern in the
datasheets), the vast majority of the chips I've come across should be
detected properly:

http://linux-mtd.infradead.org/nand-data/nanddata.html

And of course, please contribute to
git://git.infradead.org/mtd-www.git if you have additions or edits for
the table. It's not perfect, but it can help for sorting through
different chips.

Brian

^ permalink raw reply

* [PATCH] arm: omap: Fix build error in ispccdc.c
From: Laurent Pinchart @ 2011-09-14 15:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110914153303.GW11701@amd.com>

Mauro,

On Wednesday 14 September 2011 17:33:03 Roedel, Joerg wrote:
> Ping?

I've acked the patch on September the 6th and asked in the e-mail if you could 
pick it and push it to v3.1. Is there anything else I need to do ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH 1/2 v3] power: bq20z75: devicetree init support
From: Grant Likely @ 2011-09-14 15:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315948860-21482-1-git-send-email-rklein@nvidia.com>

On Tue, Sep 13, 2011 at 02:20:59PM -0700, Rhyland Klein wrote:
> Adding support to generate platform data when kernel is configured
> through device tree.
> 
> Signed-off-by: Rhyland Klein <rklein@nvidia.com>
> ---
> 	v3: Changed to use "ti," properties
> 	    Changed to use gpio property with flag for polarity
> 	    Removed unnecessary call to of_match_device
> 
>  drivers/power/bq20z75.c |   75 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 75 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
> index 9c5e5be..bb4afb2 100644
> --- a/drivers/power/bq20z75.c
> +++ b/drivers/power/bq20z75.c
> @@ -613,6 +613,78 @@ static void bq20z75_delayed_work(struct work_struct *work)
>  	}
>  }
>  
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
> +static const struct of_device_id bq20z75_dt_ids[] = {
> +	{ .compatible = "ti,bq20z75" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(platform, bq20z75_dt_ids);
> +
> +static struct bq20z75_platform_data *bq20z75_of_populate_pdata(
> +	struct i2c_client *client)
> +{
> +	const struct of_device_id *dtid;
> +	struct device_node *of_node = client->dev.of_node;
> +	struct bq20z75_platform_data *pdata = client->dev.platform_data;
> +	enum of_gpio_flags gpio_flags;
> +	int rc;
> +	u32 prop;
> +
> +	/* verify this driver matches this device */
> +	if (!of_node)
> +		return NULL;
> +
> +	/* if platform data is set, honor it */
> +	if (pdata)
> +		return pdata;
> +
> +	/* first make sure at least one property is set, otherwise
> +	 * it won't change behavior from running without pdata.
> +	 */
> +	if (!of_get_property(of_node, "ti,i2c-retry-count", NULL) &&
> +		!of_get_property(of_node, "ti,poll-retry-count", NULL) &&
> +		!of_get_property(of_node, "ti,battery-detect-gpios", NULL))
> +		goto of_out;
> +
> +	pdata = devm_kzalloc(&client->dev, sizeof(struct bq20z75_platform_data),
> +				GFP_KERNEL);
> +	if (!pdata)
> +		goto of_out;
> +
> +	rc = of_property_read_u32(of_node, "ti,i2c-retry-count", &prop);
> +	if (!rc)
> +		pdata->i2c_retry_count = prop;
> +
> +	rc = of_property_read_u32(of_node, "ti,poll-retry-count", &prop);
> +	if (!rc)
> +		pdata->poll_retry_count = prop;
> +
> +	if (!of_get_property(of_node, "ti,battery-detect-gpios", NULL)) {
> +		pdata->battery_detect = -1;
> +		goto of_out;
> +	}
> +
> +	pdata->battery_detect = of_get_named_gpio_flags(of_node,
> +			"ti,battery-detect-gpios", 0, &gpio_flags);
> +
> +	if (gpio_flags & OF_GPIO_ACTIVE_LOW)
> +		pdata->battery_detect_present = 0;
> +	else
> +		pdata->battery_detect_present = 1;
> +
> +of_out:
> +	return pdata;
> +}
> +#else
> +#define bq20z75_dt_ids NULL
> +static struct bq20z75_platform_data *bq20z75_of_populate_pdata(
> +	struct i2c_client *client)
> +{
> +	return NULL;
> +}

This looks like the driver will break when !CONFIG_OF.  The caller of
bq20z75_of_populate_pdata will get NULL and therefore won't have any
pdata.

> +#endif
> +
>  static int __devinit bq20z75_probe(struct i2c_client *client,
>  	const struct i2c_device_id *id)
>  {
> @@ -642,6 +714,8 @@ static int __devinit bq20z75_probe(struct i2c_client *client,
>  	bq20z75_device->power_supply.external_power_changed =
>  		bq20z75_external_power_changed;
>  
> +	pdata = bq20z75_of_populate_pdata(client);
> +
>  	if (pdata) {
>  		bq20z75_device->gpio_detect =
>  			gpio_is_valid(pdata->battery_detect);
> @@ -775,6 +849,7 @@ static struct i2c_driver bq20z75_battery_driver = {
>  	.id_table	= bq20z75_id,
>  	.driver = {
>  		.name	= "bq20z75-battery",
> +		.of_match_table = bq20z75_dt_ids,
>  	},
>  };
>  
> -- 
> 1.7.6
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/2 v3] power: bq20z75: Add Device Tree Binding
From: Grant Likely @ 2011-09-14 15:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315948899-21536-1-git-send-email-rklein@nvidia.com>

On Tue, Sep 13, 2011 at 02:21:39PM -0700, Rhyland Klein wrote:
> Adding the binding for the TI bq20z75 fuel gadge and the
> bq20z75 driver.

Squash this patch into the first patch.  There is no reason to keep
them separate.

> 
> Signed-off-by: Rhyland Klein <rklein@nvidia.com>
> ---
> 	v2: Fixed typo in binding description
> 	v3: Changed unique properties to be use "ti,"
> 	    Changed to use single gpio entry for gpio info
> 
>  .../bindings/power_supply/ti_bq20z75.txt           |   23 ++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt
> 
> diff --git a/Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt b/Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt
> new file mode 100644
> index 0000000..7571294
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt
> @@ -0,0 +1,23 @@
> +TI bq20z75
> +~~~~~~~~~~
> +
> +Required properties :
> + - compatible : "ti,bq20z75"
> +
> +Optional properties :
> + - ti,i2c-retry-count : The number of times to retry i2c transactions on i2c
> +   IO failure.
> + - ti,poll-retry-count : The number of times to try looking for new status
> +   after an external change notification.
> + - ti,battery-detect-gpios : The gpio which signals battery detection and
> +   a flag specifying its polarity.
> +
> +Example:
> +
> +	bq20z75 at b {
> +		compatible = "ti,bq20z75";
> +		reg = < 0xb >;
> +		ti,i2c-retry-count = <2>;
> +		ti,poll-retry-count = <10>;
> +		ti,battery-detect-gpios = <&gpio-controller 122 1>;
> +	}
> -- 
> 1.7.6
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/6] ARM: zImage: Allow the appending of a device tree binary
From: Dave Martin @ 2011-09-14 16:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1109141056150.20358@xanadu.home>

On Wed, Sep 14, 2011 at 11:10:44AM -0400, Nicolas Pitre wrote:
> On Wed, 14 Sep 2011, Dave Martin wrote:
> 
> > On Wed, Sep 14, 2011 at 10:04:28AM -0400, Nicolas Pitre wrote:
> > > On Wed, 14 Sep 2011, Dave Martin wrote:
> > > 

[...]

> > > > Do we worry that garbage in memory after the zImage might match this
> > > > magic number?
> > > > 
> > > > For example, if an ordinary userspace program allocates a huge number
> > > > of pages and fills them with bogus device tree headers, is there a chance
> > > > that the those headers could remain in memory across a reboot?
> > > 
> > > In theory this _could_ be possible.  However I don't expect this feature 
> > > to be enabled if you are not going to actually use it, especially in a 
> > > production setup.  If you are not appending a DTB to your kernel then 
> > > there is simply no point keeping this config option set (normally you 
> > > should use this option only because you have no other choices).
> > 
> > That seems reasonable.
> > 
> > Should we document this recommendation, in the Kconfig help or
> > Documentation/?
> 
> I'll add some scary wording to the help text, and make it depend on 
> EXPERIMENTAL as well.  I prefer not to impose some expectations on the 
> zImage layout for this even if not in use, like being stuck with an 
> offset that we'll always have to guard against corruption due to people 
> blindly scripting the zImage poking you suggested even when it is not 
> needed.
> 
> People will find ways to screw it up if they really want to anyway.  So 
> I'd lean towards keeping this simple and not create any legacy around 
> this hopefully temporary accommodation feature.

Yeah, sure -- I think documentating it is enough for now.

And I agree we don't really want to reinvent the rdev nastiness for zImages...

Cheers
---Dave

^ permalink raw reply

* [PATCH 2/2] input: samsung-keypad: Add device tree support
From: Grant Likely @ 2011-09-14 16:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315916779-21793-3-git-send-email-thomas.abraham@linaro.org>

On Tue, Sep 13, 2011 at 05:56:19PM +0530, Thomas Abraham wrote:
> Add device tree based discovery support for Samsung's keypad controller.
> 
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Donghwa Lee <dh09.lee@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  .../devicetree/bindings/input/samsung-keypad.txt   |   88 ++++++++++
>  drivers/input/keyboard/samsung-keypad.c            |  177 ++++++++++++++++++--
>  2 files changed, 253 insertions(+), 12 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/samsung-keypad.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/samsung-keypad.txt b/Documentation/devicetree/bindings/input/samsung-keypad.txt
> new file mode 100644
> index 0000000..e1c7237
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/samsung-keypad.txt
> @@ -0,0 +1,88 @@
> +* Samsung's Keypad Controller device tree bindings
> +
> +Samsung's Keypad controller is used to interface a SoC with a matrix-type
> +keypad device. The keypad controller supports multiple row and column lines.
> +A key can be placed at each intersection of a unique row and a unique column.
> +The keypad controller can sense a key-press and key-release and report the
> +event using a interrupt to the cpu.
> +
> +Required SoC Specific Properties:
> +- compatible: should be one of the following
> +  - "samsung,s3c6410-keypad": For controllers compatible with s3c6410 keypad
> +    controller.
> +  - "samsung,s5pv210-keypad": For controllers compatible with s5pv210 keypad
> +    controller.
> +
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +- interrupts: The interrupt number to the cpu.
> +
> +Required Board Specific Properties:
> +- samsung,keypad-num-rows: Number of row lines connected to the keypad
> +  controller.
> +
> +- samsung,keypad-num-columns: Number of column lines connected to the
> +  keypad controller.
> +
> +- row-gpios: List of gpios used as row lines. The gpio specifier for
> +  this property depends on the gpio controller to which these row lines
> +  are connected.
> +
> +- col-gpios: List of gpios used as column lines. The gpio specifier for
> +  this property depends on the gpio controller to which these column
> +  lines are connected.

I know we've got overlapping terminology here.  When you say GPIOs
here, does it refer to the pin multiplexing feature of the samsung
parts, and thus how the keypad controller IO is routed to the pins?
Or does the driver manipulate GPIO lines manually?

If it is pin multiplexing, then using the GPIO binding seems rather
odd. It may be entirely appropriate, but it will need to play well
with the pinmux stuff that linusw is working on.

> +
> +- Keys represented as child nodes: Each key connected to the keypad
> +  controller is represented as a child node to the keypad controller
> +  device node and should include the following properties.
> +  - keypad,row: the row number to which the key is connected.
> +  - keypad,column: the column number to which the key is connected.
> +  - keypad,key-code: the key-code to be reported when the key is pressed
> +    and released.

What defines the meanings of the key codes?

> +
> +Optional Properties specific to linux:
> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
> +- linux,keypad-wakeup: use any event on keypad as wakeup event.

Is this really a linux-specific feature?

> +
> +
> +Example:
> +	keypad at 100A0000 {
> +		compatible = "samsung,s5pv210-keypad";
> +		reg = <0x100A0000 0x100>;
> +		interrupts = <173>;
> +		samsung,keypad-num-rows = <2>;
> +		samsung,keypad-num-columns = <8>;
> +		linux,input-no-autorepeat;
> +		linux,input-wakeup;
> +
> +		row-gpios = <&gpx2 0 3 3 0
> +			     &gpx2 1 3 3 0>;
> +
> +		col-gpios = <&gpx1 0 3 0 0
> +			     &gpx1 1 3 0 0
> +			     &gpx1 2 3 0 0
> +			     &gpx1 3 3 0 0
> +			     &gpx1 4 3 0 0
> +			     &gpx1 5 3 0 0
> +			     &gpx1 6 3 0 0
> +			     &gpx1 7 3 0 0>;
> +
> +		key_1 {
> +			keypad,row = <0>;
> +			keypad,column = <3>;
> +			keypad,key-code = <2>;
> +		};
> +
> +		key_2 {
> +			keypad,row = <0>;
> +			keypad,column = <4>;
> +			keypad,key-code = <3>;
> +		};
> +
> +		key_3 {
> +			keypad,row = <0>;
> +			keypad,column = <5>;
> +			keypad,key-code = <4>;
> +		};
> +	};
> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> index f689f49..cf01a56 100644
> --- a/drivers/input/keyboard/samsung-keypad.c
> +++ b/drivers/input/keyboard/samsung-keypad.c
> @@ -21,6 +21,8 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
>  #include <linux/sched.h>
>  #include <plat/keypad.h>
>  
> @@ -68,31 +70,26 @@ struct samsung_keypad {
>  	wait_queue_head_t wait;
>  	bool stopped;
>  	int irq;
> +	enum samsung_keypad_type type;
>  	unsigned int row_shift;
>  	unsigned int rows;
>  	unsigned int cols;
>  	unsigned int row_state[SAMSUNG_MAX_COLS];
> +#ifdef CONFIG_OF
> +	int row_gpios[SAMSUNG_MAX_ROWS];
> +	int col_gpios[SAMSUNG_MAX_COLS];
> +#endif
>  	unsigned short keycodes[];
>  };
>  
> -static int samsung_keypad_is_s5pv210(struct device *dev)
> -{
> -	struct platform_device *pdev = to_platform_device(dev);
> -	enum samsung_keypad_type type =
> -		platform_get_device_id(pdev)->driver_data;
> -
> -	return type == KEYPAD_TYPE_S5PV210;
> -}
> -
>  static void samsung_keypad_scan(struct samsung_keypad *keypad,
>  				unsigned int *row_state)
>  {
> -	struct device *dev = keypad->input_dev->dev.parent;
>  	unsigned int col;
>  	unsigned int val;
>  
>  	for (col = 0; col < keypad->cols; col++) {
> -		if (samsung_keypad_is_s5pv210(dev)) {
> +		if (keypad->type == KEYPAD_TYPE_S5PV210) {
>  			val = S5PV210_KEYIFCOLEN_MASK;
>  			val &= ~(1 << col) << 8;
>  		} else {
> @@ -235,6 +232,129 @@ static void samsung_keypad_close(struct input_dev *input_dev)
>  	samsung_keypad_stop(keypad);
>  }
>  
> +#ifdef CONFIG_OF
> +static
> +struct samsung_keypad_platdata *samsung_keypad_parse_dt(struct device *dev)

Nit: keep everything up to the function name on the same line.  It is
more grep-friendly to make line breaks in the parameters list.

> +{
> +	struct samsung_keypad_platdata *pdata;
> +	struct matrix_keymap_data *keymap_data;
> +	uint32_t *keymap;
> +	struct device_node *np = dev->of_node, *key_np;
> +	unsigned int key_count = 0;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata) {
> +		dev_err(dev, "could not allocate memory for platform data\n");
> +		return NULL;
> +	}
> +
> +	of_property_read_u32(np, "samsung,keypad-num-rows", &pdata->rows);
> +	of_property_read_u32(np, "samsung,keypad-num-columns", &pdata->cols);

pdata->rows and ->cols needs to be changed to a u32 for this to be
safe.

> +	if (!pdata->rows || !pdata->cols) {
> +		dev_err(dev, "number of keypad rows/columns not specified\n");
> +		return NULL;
> +	}
> +
> +	keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
> +	if (!keymap_data) {
> +		dev_err(dev, "could not allocate memory for keymap data\n");
> +		return NULL;
> +	}
> +	pdata->keymap_data = keymap_data;
> +
> +	for_each_child_of_node(np, key_np)
> +		key_count++;
> +
> +	keymap_data->keymap_size = key_count;
> +	keymap = devm_kzalloc(dev, sizeof(uint32_t) * key_count, GFP_KERNEL);
> +	if (!keymap) {
> +		dev_err(dev, "could not allocate memory for keymap\n");
> +		return NULL;
> +	}
> +	keymap_data->keymap = keymap;
> +
> +	for_each_child_of_node(np, key_np) {
> +		unsigned int row, col, key_code;

These need to be u32

> +		of_property_read_u32(key_np, "keypad,row", &row);
> +		of_property_read_u32(key_np, "keypad,column", &col);
> +		of_property_read_u32(key_np, "keypad,key-code", &key_code);
> +		*keymap++ = KEY(row, col, key_code);
> +	}
> +
> +	if (of_get_property(np, "linux,input-no-autorepeat", NULL))
> +		pdata->no_autorepeat = true;
> +	if (of_get_property(np, "linux,input-wakeup", NULL))
> +		pdata->wakeup = true;
> +
> +	return pdata;
> +}
> +
> +static void samsung_keypad_parse_dt_gpio(struct device *dev,
> +				struct samsung_keypad *keypad)
> +{
> +	struct device_node *np = dev->of_node;
> +	int gpio, ret, row, col;
> +
> +	for (row = 0; row < keypad->rows; row++) {
> +		gpio = of_get_named_gpio(np, "row-gpios", row);
> +		keypad->row_gpios[row] = gpio;
> +		if (!gpio_is_valid(gpio)) {
> +			dev_err(dev, "keypad row[%d]: invalid gpio %d\n",
> +					row, gpio);
> +			continue;
> +		}
> +
> +		ret = gpio_request(gpio, "keypad-row");
> +		if (ret)
> +			dev_err(dev, "keypad row[%d] gpio request failed\n",
> +					row);
> +	}
> +
> +	for (col = 0; col < keypad->cols; col++) {
> +		gpio = of_get_named_gpio(np, "col-gpios", col);
> +		keypad->col_gpios[col] = gpio;
> +		if (!gpio_is_valid(gpio)) {
> +			dev_err(dev, "keypad column[%d]: invalid gpio %d\n",
> +					col, gpio);
> +			continue;
> +		}
> +
> +		ret = gpio_request(col, "keypad-col");
> +		if (ret)
> +			dev_err(dev, "keypad column[%d] gpio request failed\n",
> +					col);
> +	}
> +}
> +
> +static void samsung_keypad_dt_gpio_free(struct samsung_keypad *keypad)
> +{
> +	int cnt;
> +
> +	for (cnt = 0; cnt < keypad->rows; cnt++)
> +		if (gpio_is_valid(keypad->row_gpios[cnt]))
> +			gpio_free(keypad->row_gpios[cnt]);
> +
> +	for (cnt = 0; cnt < keypad->cols; cnt++)
> +		if (gpio_is_valid(keypad->col_gpios[cnt]))
> +			gpio_free(keypad->col_gpios[cnt]);
> +}
> +#else
> +static
> +struct samsung_keypad_platdata *samsung_keypad_parse_dt(struct device *dev)
> +{
> +	return NULL;
> +}
> +
> +static void samsung_keypad_parse_dt_gpio(struct device *dev,
> +				struct samsung_keypad *keypad)
> +{
> +}
> +
> +static void samsung_keypad_dt_gpio_free(struct samsung_keypad *keypad)
> +{
> +}
> +#endif
> +
>  static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>  {
>  	const struct samsung_keypad_platdata *pdata;
> @@ -246,7 +366,10 @@ static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>  	unsigned int keymap_size;
>  	int error;
>  
> -	pdata = pdev->dev.platform_data;
> +	if (pdev->dev.of_node)
> +		pdata = samsung_keypad_parse_dt(&pdev->dev);
> +	else
> +		pdata = pdev->dev.platform_data;
>  	if (!pdata) {
>  		dev_err(&pdev->dev, "no platform data defined\n");
>  		return -EINVAL;
> @@ -303,6 +426,16 @@ static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>  	keypad->cols = pdata->cols;
>  	init_waitqueue_head(&keypad->wait);
>  
> +	if (pdev->dev.of_node) {
> +		samsung_keypad_parse_dt_gpio(&pdev->dev, keypad);
> +#ifdef CONFIG_OF
> +		keypad->type = of_device_is_compatible(pdev->dev.of_node,
> +					"samsung,s5pv210-keypad");
> +#endif

It is odd having the #ifdef around only part of the if() block.

> +	} else {
> +		keypad->type = platform_get_device_id(pdev)->driver_data;
> +	}
> +
>  	input_dev->name = pdev->name;
>  	input_dev->id.bustype = BUS_HOST;
>  	input_dev->dev.parent = &pdev->dev;
> @@ -343,12 +476,19 @@ static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>  
>  	device_init_wakeup(&pdev->dev, pdata->wakeup);
>  	platform_set_drvdata(pdev, keypad);
> +
> +	if (pdev->dev.of_node) {
> +		devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
> +		devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
> +		devm_kfree(&pdev->dev, (void *)pdata);
> +	}

Don't need to do this.  The driver core will take care of freeing the
devm for you when removed.

A few things that should be fixed up, but otherwise looks pretty good.

>  	return 0;
>  
>  err_free_irq:
>  	free_irq(keypad->irq, keypad);
>  err_put_clk:
>  	clk_put(keypad->clk);
> +	samsung_keypad_dt_gpio_free(keypad);
>  err_unmap_base:
>  	iounmap(keypad->base);
>  err_free_mem:
> @@ -374,6 +514,7 @@ static int __devexit samsung_keypad_remove(struct platform_device *pdev)
>  	free_irq(keypad->irq, keypad);
>  
>  	clk_put(keypad->clk);
> +	samsung_keypad_dt_gpio_free(keypad);
>  
>  	iounmap(keypad->base);
>  	kfree(keypad);
> @@ -447,6 +588,17 @@ static const struct dev_pm_ops samsung_keypad_pm_ops = {
>  };
>  #endif
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id samsung_keypad_dt_match[] = {
> +	{ .compatible = "samsung,s3c6410-keypad" },
> +	{ .compatible = "samsung,s5pv210-keypad" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, samsung_keypad_dt_match);
> +#else
> +#define samsung_keypad_dt_match NULL
> +#endif
> +
>  static struct platform_device_id samsung_keypad_driver_ids[] = {
>  	{
>  		.name		= "samsung-keypad",
> @@ -465,6 +617,7 @@ static struct platform_driver samsung_keypad_driver = {
>  	.driver		= {
>  		.name	= "samsung-keypad",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = samsung_keypad_dt_match,
>  #ifdef CONFIG_PM
>  		.pm	= &samsung_keypad_pm_ops,
>  #endif
> -- 
> 1.6.6.rc2
> 

^ permalink raw reply

* [PATCH v3 4/6] DMA: PL330: Add device tree support
From: Grant Likely @ 2011-09-14 16:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315852165-32402-5-git-send-email-thomas.abraham@linaro.org>

On Mon, Sep 12, 2011 at 11:59:23PM +0530, Thomas Abraham wrote:
> For PL330 dma controllers instantiated from device tree, the channel
> lookup is based on phandle of the dma controller and dma request id
> specified by the client node. During probe, the private data of each
> channel of the controller is set to point to the device node of the
> dma controller. The 'chan_id' of the each channel is used as the
> dma request id.
> 
> Client driver requesting dma channels specify the phandle of the
> dma controller and the request id. The pl330 filter function
> converts the phandle to the device node pointer and matches that
> with channel's private data. If a match is found, the request id
> from the client node and the 'chan_id' of the channel is matched.
> A channel is found if both the values match.
> 
> Cc: Jassi Brar <jassisinghbrar@gmail.com>
> Cc: Boojin Kim <boojin.kim@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> Reviewed-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  .../devicetree/bindings/dma/arm-pl330.txt          |   29 +++++++++++++++++
>  drivers/dma/pl330.c                                |   33 +++++++++++++++++--
>  2 files changed, 58 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/dma/arm-pl330.txt
> 
> diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt
> new file mode 100644
> index 0000000..05b007d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt
> @@ -0,0 +1,29 @@
> +* ARM PrimeCell PL330 DMA Controller
> +
> +The ARM PrimeCell PL330 DMA controller can move blocks of memory contents
> +between memory and peripherals or memory to memory.
> +
> +Required properties:
> +  - compatible: should include both "arm,pl330" and "arm,primecell".
> +  - reg: physical base address of the controller and length of memory mapped
> +    region.
> +  - interrupts: interrupt number to the cpu.
> +
> +Example: (from Samsung's Exynos4 processor dtsi file)
> +
> +	pdma0: pdma at 12680000 {
> +		compatible = "arm,pl330", "arm,primecell";
> +		reg = <0x12680000 0x1000>;
> +		interrupts = <99>;
> +	};
> +
> +Client drivers (device nodes requiring dma transfers from dev-to-mem or
> +mem-to-dev) should specify the DMA channel numbers using a two-value pair
> +as shown below.
> +
> +  [property name]  = <[phandle of the dma controller] [dma request id]>;
> +
> +      where 'dma request id' is the dma request number which is connected
> +      to the client controller.
> +
> +  Example:  tx-dma-channel = <&pdma0 12>;

Looks good to me.  You may want to specify that the property name
should be in the form: <name>-dma-channel just to enforce a bit of
convention on the users.

> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 992bf82..7a4ebf1 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -19,6 +19,7 @@
>  #include <linux/amba/pl330.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/scatterlist.h>
> +#include <linux/of.h>
>  
>  #define NR_DEFAULT_DESC	16
>  
> @@ -277,6 +278,20 @@ bool pl330_filter(struct dma_chan *chan, void *param)
>  	if (chan->device->dev->driver != &pl330_driver.drv)
>  		return false;
>  
> +#ifdef CONFIG_OF
> +	if (chan->device->dev->of_node) {
> +		const __be32 *prop_value;
> +		phandle phandle;
> +		struct device_node *node;
> +
> +		prop_value = ((struct property *)param)->value;
> +		phandle = be32_to_cpup(prop_value++);
> +		node = of_find_node_by_phandle(phandle);
> +		return ((chan->private == node) &&
> +				(chan->chan_id == be32_to_cpup(prop_value)));

Don't open code this.  There is already a function to decode a phandle
property.  of_parse_phandle() should do the trick.

Otherwise looks good to me.

g.

> +	}
> +#endif
> +
>  	peri_id = chan->private;
>  	return *peri_id == (unsigned)param;
>  }
> @@ -855,12 +870,17 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>  	INIT_LIST_HEAD(&pd->channels);
>  
>  	/* Initialize channel parameters */
> -	num_chan = max(pdat ? pdat->nr_valid_peri : 0, (u8)pi->pcfg.num_chan);
> +	num_chan = max(pdat ? pdat->nr_valid_peri : (u8)pi->pcfg.num_peri,
> +			(u8)pi->pcfg.num_chan);
>  	pdmac->peripherals = kzalloc(num_chan * sizeof(*pch), GFP_KERNEL);
>  
>  	for (i = 0; i < num_chan; i++) {
>  		pch = &pdmac->peripherals[i];
> -		pch->chan.private = pdat ? &pdat->peri_id[i] : NULL;
> +		if (!adev->dev.of_node)
> +			pch->chan.private = pdat ? &pdat->peri_id[i] : NULL;
> +		else
> +			pch->chan.private = adev->dev.of_node;
> +
>  		INIT_LIST_HEAD(&pch->work_list);
>  		spin_lock_init(&pch->lock);
>  		pch->pl330_chid = NULL;
> @@ -874,10 +894,15 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>  	}
>  
>  	pd->dev = &adev->dev;
> -	if (pdat)
> +	if (pdat) {
>  		pd->cap_mask = pdat->cap_mask;
> -	else
> +	} else {
>  		dma_cap_set(DMA_MEMCPY, pd->cap_mask);
> +		if (pi->pcfg.num_peri) {
> +			dma_cap_set(DMA_SLAVE, pd->cap_mask);
> +			dma_cap_set(DMA_CYCLIC, pd->cap_mask);
> +		}
> +	}
>  
>  	pd->device_alloc_chan_resources = pl330_alloc_chan_resources;
>  	pd->device_free_chan_resources = pl330_free_chan_resources;
> -- 
> 1.6.6.rc2
> 

^ permalink raw reply

* [PATCH v3 6/6] ARM: EXYNOS4: Limit usage of pl330 device instance to non-dt build
From: Grant Likely @ 2011-09-14 16:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1315852165-32402-7-git-send-email-thomas.abraham@linaro.org>

On Mon, Sep 12, 2011 at 11:59:25PM +0530, Thomas Abraham wrote:
> The pl330 device instances and associated platform data is required only
> for non-device-tree builds. With device tree enabled, the data about the
> platform is obtained from the device tree. For images that include both
> dt and non-dt platforms, an addditional check is added to ensure that
> static amba device registrations is applicable to only non-dt platforms.
> 
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
> diff --git a/arch/arm/mach-exynos4/dma.c b/arch/arm/mach-exynos4/dma.c
> index c3c0d17..3203a31 100644
> --- a/arch/arm/mach-exynos4/dma.c
> +++ b/arch/arm/mach-exynos4/dma.c
> @@ -24,6 +24,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/amba/bus.h>
>  #include <linux/amba/pl330.h>
> +#include <linux/of.h>
>  
>  #include <asm/irq.h>
>  #include <plat/devs.h>
> @@ -138,6 +139,11 @@ struct amba_device exynos4_device_pdma1 = {
>  
>  static int __init exynos4_dma_init(void)
>  {
> +#ifdef CONFIG_OF
> +	if (of_have_populated_dt())
> +		return 0;
> +#endif
> +

Drop the #ifdef.  of_have_populated_dt() has an empty stub for
!CONFIG_OF.  Otherwise looks good to me.  Well done not breaking
non-DT support when CONFIG_OF is enabled.  :-)

The other patches in this series look good to me too.

g.

^ permalink raw reply

* [ltt-dev] LTTng 2.0 on ARM
From: Jon Medhurst (Tixy) @ 2011-09-14 16:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110914100931.GB2104@arm.com>

On Wed, 2011-09-14 at 11:09 +0100, Dave Martin wrote:
> On Tue, Sep 13, 2011 at 11:14:47PM +0530, Rabin Vincent wrote:
[...]
> > The problem is that the addresses returned by kallsyms_lookup_name()
> > does not have the zero bit, which is what is expected for Thumb
> > functions because the BLX instruction which is used to call them uses
> > this bit to determines which mode to switch into.  Since it's cleared,
> > you switch to ARM mode and attempt to execute Thumb-2 code, with obvious
> > results.
> > 
> > A cursory look at the parties involved shows that nm doesn't show the
> > zero bit (even though it's set in the vmlinux symbol table), and
> > scripts/kallsyms builds the table by parsing nm's output.
> 
> It's not quite as simple as saying "the output of nm is wrong" though...
> 
> When getting the address of a function, there are actually two
> separate answers:
> 
>  a) the pointer which can be used to call the function
> 
>  b) the address of the start of the function body
> 
> On many arches these they are identical, but on some they are different.
> On ARM, they are identical for ARM code but different for Thumb code
> (because the Thumb bit must be set in case (a) but not in case (b))
> 
> It may be worth looking at what is done in the kernel for ia64 and ppc64.
> I believe that (a) and (b) are quite different for these because
> functions are called through descriptors.  Don't quote me on that though:
> I'm mostly ignorant about these arches.
> 
> For the Thumb-2 kernel case, we can probably hack around this: there
> are various places in the kernel where we just force-set the Thumb bit
> in addresses without really knowing what the target code is.  We get
> away with this because the kernel is (very nearly) 100% Thumb code
> for a Thumb-2 kernel.
> 
> However, if the kernel already has a correct approach for solving this
> problem, we should probably be using it.

This is the same issue I found recently with kprobes [1]. There is also
an inconsistency as function symbols in loadable module do have bit zero
set, but if the module is built-in then bit zero is clear. 

-- 
Tixy

[1] http://www.spinics.net/lists/arm-kernel/msg138283.html

^ permalink raw reply

* [PATCH 0/5] GIC OF bindings
From: Rob Herring @ 2011-09-14 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rob Herring <rob.herring@calxeda.com>

This series introduces of_irq_init to scan the device tree for interrupt
controller nodes and call their init functions in proper order. The GIC
init function is then called from this function. The platform code then
looks something like this:

const static struct of_device_id irq_match[] = {
	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init, },
	{}
};

static void __init highbank_init_irq(void)
{
	of_irq_init(irq_match);
}

The binding for GIC PPIs is now done with a 3rd interrupt cell to specify
a cpu mask for which cpu the PPI is connected to. This was discussed at LPC
and suggested by Grant.

I dropped the public intc_desc struct. The the interrupt controller's node
and the interrupt parent's node are passed in directly to the controller's
init function. The linux irq assignment is now done dynamically using
irq_alloc_descs.

The first 2 patches are minor fixes to irqdomains.

Rob

Rob Herring (5):
  irq: add declaration of irq_domain_simple_ops to irqdomain.h
  irq: fix existing domain check in irq_domain_add
  of/irq: introduce of_irq_init
  ARM: gic: allow irq_start to be 0
  ARM: gic: add OF based initialization

 Documentation/devicetree/bindings/arm/gic.txt |   53 ++++++++++++++
 arch/arm/common/gic.c                         |   57 +++++++++++++--
 arch/arm/include/asm/hardware/gic.h           |   10 +++
 drivers/of/irq.c                              |   96 +++++++++++++++++++++++++
 include/linux/irqdomain.h                     |    1 +
 include/linux/of_irq.h                        |    1 +
 kernel/irq/irqdomain.c                        |    2 +-
 7 files changed, 214 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/gic.txt

-- 
1.7.5.4

^ permalink raw reply

* [PATCH 1/5] irq: add declaration of irq_domain_simple_ops to irqdomain.h
From: Rob Herring @ 2011-09-14 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316017900-19918-1-git-send-email-robherring2@gmail.com>

From: Rob Herring <rob.herring@calxeda.com>

irq_domain_simple_ops is exported, but is not declared in irqdomain.h, so
add it.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irqdomain.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index e807ad6..3ad553e 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -80,6 +80,7 @@ extern void irq_domain_del(struct irq_domain *domain);
 #endif /* CONFIG_IRQ_DOMAIN */
 
 #if defined(CONFIG_IRQ_DOMAIN) && defined(CONFIG_OF_IRQ)
+extern struct irq_domain_ops irq_domain_simple_ops;
 extern void irq_domain_add_simple(struct device_node *controller, int irq_base);
 extern void irq_domain_generate_simple(const struct of_device_id *match,
 					u64 phys_base, unsigned int irq_start);
-- 
1.7.5.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox