LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 5/9] of: put default string compare and #a/s-cell values into common header
From: Benjamin Herrenschmidt @ 2010-02-14  6:11 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213160252.4767.94582.stgit@angua>

On Sat, 2010-02-13 at 09:02 -0700, Grant Likely wrote:
> Most architectures don't need to change these.  Put them into common
> code to eliminate some duplication
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> 
>  arch/microblaze/include/asm/prom.h |    7 -------
>  arch/powerpc/include/asm/prom.h    |    7 -------
>  include/linux/of.h                 |   13 +++++++++++++
>  3 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/prom.h b/arch/microblaze/include/asm/prom.h
> index 6c6b386..ddc5c57 100644
> --- a/arch/microblaze/include/asm/prom.h
> +++ b/arch/microblaze/include/asm/prom.h
> @@ -26,13 +26,6 @@
>  #include <asm/irq.h>
>  #include <asm/atomic.h>
>  
> -#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	1
> -#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
> -
> -#define of_compat_cmp(s1, s2, l)	strncasecmp((s1), (s2), (l))
> -#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
> -#define of_node_cmp(s1, s2)		strcasecmp((s1), (s2))
> -
>  extern struct device_node *of_chosen;
>  
>  #define HAVE_ARCH_DEVTREE_FIXUPS
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index f384db8..4a5070e 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -23,13 +23,6 @@
>  #include <asm/irq.h>
>  #include <asm/atomic.h>
>  
> -#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	1
> -#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
> -
> -#define of_compat_cmp(s1, s2, l)	strcasecmp((s1), (s2))
> -#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
> -#define of_node_cmp(s1, s2)		strcasecmp((s1), (s2))
> -
>  extern struct device_node *of_chosen;
>  
>  #define HAVE_ARCH_DEVTREE_FIXUPS
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 48b0ee6..5cd2840 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -116,6 +116,19 @@ static inline unsigned long of_read_ulong(const __be32 *cell, int size)
>  
>  #include <asm/prom.h>
>  
> +/* Default #address and #size cells.  Allow arch asm/prom.h to override */
> +#if !defined(OF_ROOT_NODE_ADDR_CELLS_DEFAULT)
> +#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 1
> +#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
> +#endif
> +
> +/* Default string compare functions, Allow arch asm/prom.h to override */
> +#if !defined(of_compat_cmp)
> +#define of_compat_cmp(s1, s2, l)	strncasecmp((s1), (s2), (l))
> +#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
> +#define of_node_cmp(s1, s2)		strcasecmp((s1), (s2))
> +#endif
> +
>  /* flag descriptions */
>  #define OF_DYNAMIC	1 /* node and properties were allocated via kmalloc */
>  #define OF_DETACHED	2 /* node has been detached from the device tree */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH 6/9] of: remove unused extern reference to devtree_lock
From: Benjamin Herrenschmidt @ 2010-02-14  6:12 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213160258.4767.22371.stgit@angua>

On Sat, 2010-02-13 at 09:02 -0700, Grant Likely wrote:
> Neither the powerpc nor the microblaze code use devtree_lock anymore.
> Remove the extern reference.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>


>  arch/microblaze/include/asm/prom.h |    2 --
>  arch/powerpc/kernel/prom.c         |    2 --
>  2 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/prom.h b/arch/microblaze/include/asm/prom.h
> index ddc5c57..8b1ebd3 100644
> --- a/arch/microblaze/include/asm/prom.h
> +++ b/arch/microblaze/include/asm/prom.h
> @@ -30,8 +30,6 @@ extern struct device_node *of_chosen;
>  
>  #define HAVE_ARCH_DEVTREE_FIXUPS
>  
> -extern rwlock_t devtree_lock;	/* temporary while merging */
> -
>  /* Other Prototypes */
>  extern int early_uartlite_console(void);
>  
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 5bbbdb2..4869c93 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -67,8 +67,6 @@ int __initdata iommu_force_on;
>  unsigned long tce_alloc_start, tce_alloc_end;
>  #endif
>  
> -extern rwlock_t devtree_lock;	/* temporary while merging */
> -
>  /* export that to outside world */
>  struct device_node *of_chosen;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH 8/9] of/sparc: Remove sparc-local declaration of allnodes and devtree_lock
From: Benjamin Herrenschmidt @ 2010-02-14  6:13 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213160309.4767.94443.stgit@angua>

On Sat, 2010-02-13 at 09:03 -0700, Grant Likely wrote:
> Both allnodes and devtree_lock are defined in common code.  The
> extern declaration should be in the common header too so that the
> compiler can type check.  allnodes is already in of.h, but
> devtree_lock should be declared there too.
> 
> This patch removes the SPARC declarations and uses decls in of.h instead.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
> 
>  arch/sparc/kernel/prom.h |    3 ---
>  include/linux/of.h       |    2 ++
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/sparc/kernel/prom.h b/arch/sparc/kernel/prom.h
> index 453397f..a8591ef 100644
> --- a/arch/sparc/kernel/prom.h
> +++ b/arch/sparc/kernel/prom.h
> @@ -4,9 +4,6 @@
>  #include <linux/spinlock.h>
>  #include <asm/prom.h>
>  
> -extern struct device_node *allnodes;	/* temporary while merging */
> -extern rwlock_t devtree_lock;	/* temporary while merging */
> -
>  extern void * prom_early_alloc(unsigned long size);
>  extern void irq_trans_init(struct device_node *dp);
>  
> diff --git a/include/linux/of.h b/include/linux/of.h
> index d34cc5d..f6d9cbc 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -19,6 +19,7 @@
>  #include <linux/bitops.h>
>  #include <linux/kref.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/spinlock.h>
>  
>  #include <asm/byteorder.h>
>  
> @@ -67,6 +68,7 @@ struct device_node {
>  /* Pointer for first entry in chain of all nodes. */
>  extern struct device_node *allnodes;
>  extern struct device_node *of_chosen;
> +extern rwlock_t devtree_lock;
>  
>  static inline int of_node_check_flag(struct device_node *n, unsigned long flag)
>  {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH 9/9] of: remove undefined request_OF_resource & release_OF_resource
From: Benjamin Herrenschmidt @ 2010-02-14  6:14 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213160314.4767.93924.stgit@angua>

On Sat, 2010-02-13 at 09:03 -0700, Grant Likely wrote:
> Neither request_OF_resource or release_OF_resource are defined
> anywhere.  Remove the declarations.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

>  arch/microblaze/include/asm/prom.h |    4 ----
>  arch/powerpc/include/asm/prom.h    |    5 -----
>  2 files changed, 0 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/prom.h b/arch/microblaze/include/asm/prom.h
> index aa1a437..03f45a9 100644
> --- a/arch/microblaze/include/asm/prom.h
> +++ b/arch/microblaze/include/asm/prom.h
> @@ -31,10 +31,6 @@
>  /* Other Prototypes */
>  extern int early_uartlite_console(void);
>  
> -extern struct resource *request_OF_resource(struct device_node *node,
> -				int index, const char *name_postfix);
> -extern int release_OF_resource(struct device_node *node, int index);
> -
>  /*
>   * OF address retreival & translation
>   */
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 7f9a50a..ddd408a 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -39,11 +39,6 @@ extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
>  extern void pci_create_OF_bus_map(void);
>  #endif
>  
> -extern struct resource *request_OF_resource(struct device_node* node,
> -				int index, const char* name_postfix);
> -extern int release_OF_resource(struct device_node* node, int index);
> -
> -
>  /*
>   * OF address retreival & translation
>   */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH 7/9] of: move definition of of_chosen into common code.
From: Benjamin Herrenschmidt @ 2010-02-14  6:12 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213160303.4767.36411.stgit@angua>

On Sat, 2010-02-13 at 09:03 -0700, Grant Likely wrote:
> Rather than defining of_chosen in each arch, it can be defined for all
> in driver/of/base.c
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
> 
>  arch/microblaze/include/asm/prom.h |    2 --
>  arch/microblaze/kernel/prom.c      |    3 ---
>  arch/powerpc/include/asm/prom.h    |    2 --
>  arch/powerpc/kernel/prom.c         |    3 ---
>  drivers/of/base.c                  |    1 +
>  include/linux/of.h                 |    1 +
>  6 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/prom.h b/arch/microblaze/include/asm/prom.h
> index 8b1ebd3..aa1a437 100644
> --- a/arch/microblaze/include/asm/prom.h
> +++ b/arch/microblaze/include/asm/prom.h
> @@ -26,8 +26,6 @@
>  #include <asm/irq.h>
>  #include <asm/atomic.h>
>  
> -extern struct device_node *of_chosen;
> -
>  #define HAVE_ARCH_DEVTREE_FIXUPS
>  
>  /* Other Prototypes */
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index a7dcaf0..a15ef6d 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -42,9 +42,6 @@
>  #include <asm/sections.h>
>  #include <asm/pci-bridge.h>
>  
> -/* export that to outside world */
> -struct device_node *of_chosen;
> -
>  void __init early_init_dt_scan_chosen_arch(unsigned long node)
>  {
>  	/* No Microblaze specific code here */
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 4a5070e..7f9a50a 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -23,8 +23,6 @@
>  #include <asm/irq.h>
>  #include <asm/atomic.h>
>  
> -extern struct device_node *of_chosen;
> -
>  #define HAVE_ARCH_DEVTREE_FIXUPS
>  
>  #ifdef CONFIG_PPC32
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4869c93..43238b2 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -67,9 +67,6 @@ int __initdata iommu_force_on;
>  unsigned long tce_alloc_start, tce_alloc_end;
>  #endif
>  
> -/* export that to outside world */
> -struct device_node *of_chosen;
> -
>  static int __init early_parse_mem(char *p)
>  {
>  	if (!p)
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 873479a..cb96888 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -23,6 +23,7 @@
>  #include <linux/proc_fs.h>
>  
>  struct device_node *allnodes;
> +struct device_node *of_chosen;
>  
>  /* use when traversing tree through the allnext, child, sibling,
>   * or parent members of struct device_node.
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 5cd2840..d34cc5d 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -66,6 +66,7 @@ struct device_node {
>  
>  /* Pointer for first entry in chain of all nodes. */
>  extern struct device_node *allnodes;
> +extern struct device_node *of_chosen;
>  
>  static inline int of_node_check_flag(struct device_node *n, unsigned long flag)
>  {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH 3/9] of: protect linux/of.h with CONFIG_OF
From: Benjamin Herrenschmidt @ 2010-02-14  6:10 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213160242.4767.57641.stgit@angua>

On Sat, 2010-02-13 at 09:02 -0700, Grant Likely wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> For platforms that have CONFIG_OF optional, we need to make the contents
> of linux/of.h conditional on CONFIG_OF.
> 
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

For now...

In the long run, maybe we want some of the iterators to be empty inlines
returning NULL ?

Cheers,
Ben.

> ---
> 
>  include/linux/of.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 5c7b6a6..48b0ee6 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -22,6 +22,8 @@
>  
>  #include <asm/byteorder.h>
>  
> +#ifdef CONFIG_OF
> +
>  typedef u32 phandle;
>  typedef u32 ihandle;
>  
> @@ -194,4 +196,5 @@ extern void of_attach_node(struct device_node *);
>  extern void of_detach_node(struct device_node *);
>  #endif
>  
> +#endif /* CONFIG_OF */
>  #endif /* _LINUX_OF_H */

^ permalink raw reply

* Re: [PATCHv4 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Peter Zijlstra @ 2010-02-14 10:12 UTC (permalink / raw)
  To: Joel Schopp; +Cc: Ingo Molnar, linuxppc-dev, linux-kernel, ego
In-Reply-To: <1265403478.6089.41.camel@jschopp-laptop>

On Fri, 2010-02-05 at 14:57 -0600, Joel Schopp wrote:
> On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
> there is performance benefit to idling the higher numbered threads in
> the core.  
> 
> This patch implements arch_scale_smt_power to dynamically update smt
> thread power in these idle cases in order to prefer threads 0,1 over
> threads 2,3 within a core.
> 
> Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
> ---

> Index: linux-2.6.git/arch/powerpc/kernel/smp.c
> ===================================================================
> --- linux-2.6.git.orig/arch/powerpc/kernel/smp.c
> +++ linux-2.6.git/arch/powerpc/kernel/smp.c
> @@ -620,3 +620,61 @@ void __cpu_die(unsigned int cpu)
>  		smp_ops->cpu_die(cpu);
>  }
>  #endif
> +
> +#ifdef CONFIG_SCHED_SMT
> +unsigned long arch_scale_smt_power(struct sched_domain *sd, int cpu)
> +{
> +	int sibling;
> +	int idle_count = 0;
> +	int thread;
> +
> +	/* Setup the default weight and smt_gain used by most cpus for SMT
> +	 * Power.  Doing this right away covers the default case and can be
> +	 * used by cpus that modify it dynamically.
> +	 */
> +	struct cpumask *sibling_map = sched_domain_span(sd);
> +	unsigned long weight = cpumask_weight(sibling_map);
> +	unsigned long smt_gain = sd->smt_gain;
> +
> +
> +	if (cpu_has_feature(CPU_FTR_ASYNC_SMT4) && weight == 4) {
> +		for_each_cpu(sibling, sibling_map) {
> +			if (idle_cpu(sibling))
> +				idle_count++;
> +		}
> +
> +		/* the following section attempts to tweak cpu power based
> +		 * on current idleness of the threads dynamically at runtime
> +		 */
> +		if (idle_count > 1) {
> +			thread = cpu_thread_in_core(cpu);
> +			if (thread < 2) {
> +				/* add 75 % to thread power */
> +				smt_gain += (smt_gain >> 1) + (smt_gain >> 2);
> +			} else {
> +				 /* subtract 75 % to thread power */
> +				smt_gain = smt_gain >> 2;
> +			}
> +		}
> +	}
> +
> +	/* default smt gain is 1178, weight is # of SMT threads */
> +	switch (weight) {
> +	case 1:
> +		/*divide by 1, do nothing*/
> +		break;
> +	case 2:
> +		smt_gain = smt_gain >> 1;
> +		break;
> +	case 4:
> +		smt_gain = smt_gain >> 2;
> +		break;
> +	default:
> +		smt_gain /= weight;
> +		break;
> +	}
> +
> +	return smt_gain;
> +
> +}
> +#endif

Suppose for a moment we have 2 threads (hot-unplugged thread 1 and 3, we
can construct an equivalent but more complex example for 4 threads), and
we have 4 tasks, 3 SCHED_OTHER of equal nice level and 1 SCHED_FIFO, the
SCHED_FIFO task will consume exactly 50% walltime of whatever cpu it
ends up on.

In that situation, provided that each cpu's cpu_power is of equal
measure, scale_rt_power() ensures that we run 2 SCHED_OTHER tasks on the
cpu that doesn't run the RT task, and 1 SCHED_OTHER task next to the RT
task, so that each task consumes 50%, which is all fair and proper.

However, if you do the above, thread 0 will have +75% = 1.75 and thread
2 will have -75% = 0.25, then if the RT task will land on thread 0,
we'll be having: 0.875 vs 0.25, or on thread 3, 1.75 vs 0.125. In either
case thread 0 will receive too many (if not all) SCHED_OTHER tasks.

That is, unless these threads 2 and 3 really are _that_ weak, at which
point one wonders why IBM bothered with the silicon ;-)

So tell me again, why is fiddling with the cpu_power a good placement
tool?

^ permalink raw reply

* Re: [PATCH 4/9] of/flattree: Don't assume HAVE_LMB
From: Michal Simek @ 2010-02-14 14:00 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213160247.4767.74578.stgit@angua>

Grant Likely wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> We don't always have lmb available, so make arches provide an
> early_init_dt_alloc_memory_arch() to handle the allocation of
> memory in the fdt code.
> 
> When we don't have lmb.h included, we need asm/page.h for __va.
> 
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
>  arch/microblaze/kernel/prom.c |    5 +++++
>  arch/powerpc/kernel/prom.c    |    5 +++++
>  drivers/of/fdt.c              |    9 ++++++---
>  include/linux/of_fdt.h        |    1 +
>  4 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index 050b799..a7dcaf0 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -55,6 +55,11 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>  	lmb_add(base, size);
>  }
>  
> +u64 __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
> +{
> +	return lmb_alloc(size, align);
> +}
> +
>  #ifdef CONFIG_EARLY_PRINTK
>  /* MS this is Microblaze specifig function */
>  static int __init early_init_dt_scan_serial(unsigned long node,
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 43c78d7..5bbbdb2 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -510,6 +510,11 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>  	memstart_addr = min((u64)memstart_addr, base);
>  }
>  
> +u64 __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
> +{
> +	return lmb_alloc(size, align);
> +}
> +
>  #ifdef CONFIG_BLK_DEV_INITRD
>  void __init early_init_dt_setup_initrd_arch(unsigned long start,
>  		unsigned long end)
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index b51f797..406757a 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -10,16 +10,18 @@
>   */
>  
>  #include <linux/kernel.h>
> -#include <linux/lmb.h>
>  #include <linux/initrd.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
> -
> +#include <linux/string.h>
> +#include <linux/errno.h>
>  
>  #ifdef CONFIG_PPC
>  #include <asm/machdep.h>
>  #endif /* CONFIG_PPC */

It is not part of your patch but I think that will be great completely 
remove this CONFIG_PPC part from generic OF file.
Would it be possible to include your asm/machdep.h through asm/page.h or 
any other file?

Michal

>  
> +#include <asm/page.h>
> +
>  int __initdata dt_root_addr_cells;
>  int __initdata dt_root_size_cells;
>  
> @@ -560,7 +562,8 @@ void __init unflatten_device_tree(void)
>  	pr_debug("  size is %lx, allocating...\n", size);
>  
>  	/* Allocate memory for the expanded device tree */
> -	mem = lmb_alloc(size + 4, __alignof__(struct device_node));
> +	mem = early_init_dt_alloc_memory_arch(size + 4,
> +			__alignof__(struct device_node));
>  	mem = (unsigned long) __va(mem);
>  
>  	((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index c9cb8a7..a1ca92c 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -78,6 +78,7 @@ extern void early_init_dt_check_for_initrd(unsigned long node);
>  extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>  				     int depth, void *data);
>  extern void early_init_dt_add_memory_arch(u64 base, u64 size);
> +extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
>  extern u64 dt_mem_next_cell(int s, __be32 **cellp);
>  
>  /*
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


-- 
Michal Simek, Ing. (M.Eng)
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663

^ permalink raw reply

* Re: [PATCH 3/9] of: protect linux/of.h with CONFIG_OF
From: Grant Likely @ 2010-02-14 14:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <1266127834.16346.60.camel@pasglop>

On Sat, Feb 13, 2010 at 11:10 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Sat, 2010-02-13 at 09:02 -0700, Grant Likely wrote:
>> From: Jeremy Kerr <jeremy.kerr@canonical.com>
>>
>> For platforms that have CONFIG_OF optional, we need to make the contents
>> of linux/of.h conditional on CONFIG_OF.
>>
>> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> For now...
>
> In the long run, maybe we want some of the iterators to be empty inlines
> returning NULL ?

Yes.  I'll add them as they are needed.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 0/9] Random OF cleanups and merges
From: Michal Simek @ 2010-02-14 14:06 UTC (permalink / raw)
  To: Grant Likely
  Cc: sfr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <20100213154838.4767.83881.stgit@angua>

Grant Likely wrote:
> Here's another batch of cleanup patches from my test-devicetree branch.
> A number of cleanups, corrections and minor merges of common code.
> Also a patch from Jeremy to make the flat tree work on architectures
> without LMB (arm).
> 
> Once I've collected acks on these, I'll move them over to my
> next-devicetree branch on git://git.secretlab.ca/git/linux-2.6

All changes look ok. Please add them to linux-next which I am testing. I 
don't expect any problem with it but anywya if any problems arise, I 
will let you know.

I don't want to reply every email as Ben that's why one my big ACK
for that 9 patches.

Acked-by: Michal Simek <monstr@monstr.eu>

> 
> Cheers,
> g.
> 
> ---
> 
> Grant Likely (6):
>       of: remove undefined request_OF_resource & release_OF_resource
>       of/sparc: Remove sparc-local declaration of allnodes and devtree_lock
>       of: move definition of of_chosen into common code.
>       of: remove unused extern reference to devtree_lock
>       of: put default string compare and #a/s-cell values into common header
>       of: Remove old and misplaced function declarations
> 
> Jeremy Kerr (3):
>       of/flattree: Don't assume HAVE_LMB
>       of: protect linux/of.h with CONFIG_OF
>       proc_devtree: fix THIS_MODULE without module.h
> 
> 
>  arch/microblaze/include/asm/prom.h |   15 ---------------
>  arch/microblaze/kernel/prom.c      |    8 +++++---
>  arch/powerpc/include/asm/prom.h    |   14 --------------
>  arch/powerpc/kernel/prom.c         |   10 +++++-----
>  arch/sparc/kernel/prom.h           |    3 ---
>  drivers/of/base.c                  |    1 +
>  drivers/of/fdt.c                   |    9 ++++++---
>  fs/proc/proc_devtree.c             |    1 +
>  include/linux/of.h                 |   27 +++++++++++++++++++++++++++
>  include/linux/of_fdt.h             |   11 +----------
>  10 files changed, 46 insertions(+), 53 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply

* Re: [PATCH 4/9] of/flattree: Don't assume HAVE_LMB
From: Grant Likely @ 2010-02-14 14:08 UTC (permalink / raw)
  To: michal.simek
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr, davem
In-Reply-To: <4B7801FA.9080203@petalogix.com>

On Sun, Feb 14, 2010 at 7:00 AM, Michal Simek
<michal.simek@petalogix.com> wrote:
> Grant Likely wrote:
>>
>> From: Jeremy Kerr <jeremy.kerr@canonical.com>
>>
>> We don't always have lmb available, so make arches provide an
>> early_init_dt_alloc_memory_arch() to handle the allocation of
>> memory in the fdt code.
>>
>> When we don't have lmb.h included, we need asm/page.h for __va.
>>
>> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>>
>> =A0arch/microblaze/kernel/prom.c | =A0 =A05 +++++
>> =A0arch/powerpc/kernel/prom.c =A0 =A0| =A0 =A05 +++++
>> =A0drivers/of/fdt.c =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A09 ++++++---
>> =A0include/linux/of_fdt.h =A0 =A0 =A0 =A0| =A0 =A01 +
>> =A04 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom=
.c
>> index 050b799..a7dcaf0 100644
>> --- a/arch/microblaze/kernel/prom.c
>> +++ b/arch/microblaze/kernel/prom.c
>> @@ -55,6 +55,11 @@ void __init early_init_dt_add_memory_arch(u64 base, u=
64
>> size)
>> =A0 =A0 =A0 =A0lmb_add(base, size);
>> =A0}
>> =A0+u64 __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
>> +{
>> + =A0 =A0 =A0 return lmb_alloc(size, align);
>> +}
>> +
>> =A0#ifdef CONFIG_EARLY_PRINTK
>> =A0/* MS this is Microblaze specifig function */
>> =A0static int __init early_init_dt_scan_serial(unsigned long node,
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 43c78d7..5bbbdb2 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -510,6 +510,11 @@ void __init early_init_dt_add_memory_arch(u64 base,
>> u64 size)
>> =A0 =A0 =A0 =A0memstart_addr =3D min((u64)memstart_addr, base);
>> =A0}
>> =A0+u64 __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
>> +{
>> + =A0 =A0 =A0 return lmb_alloc(size, align);
>> +}
>> +
>> =A0#ifdef CONFIG_BLK_DEV_INITRD
>> =A0void __init early_init_dt_setup_initrd_arch(unsigned long start,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0unsigned long end)
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index b51f797..406757a 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -10,16 +10,18 @@
>> =A0*/
>> =A0=A0#include <linux/kernel.h>
>> -#include <linux/lmb.h>
>> =A0#include <linux/initrd.h>
>> =A0#include <linux/of.h>
>> =A0#include <linux/of_fdt.h>
>> -
>> +#include <linux/string.h>
>> +#include <linux/errno.h>
>> =A0=A0#ifdef CONFIG_PPC
>> =A0#include <asm/machdep.h>
>> =A0#endif /* CONFIG_PPC */
>
> It is not part of your patch but I think that will be great completely
> remove this CONFIG_PPC part from generic OF file.
> Would it be possible to include your asm/machdep.h through asm/page.h or =
any
> other file?

Yup.  I'll get there unless someone else beats me to it.  :-)

>
> Michal
>
>> =A0+#include <asm/page.h>
>> +
>> =A0int __initdata dt_root_addr_cells;
>> =A0int __initdata dt_root_size_cells;
>> =A0@@ -560,7 +562,8 @@ void __init unflatten_device_tree(void)
>> =A0 =A0 =A0 =A0pr_debug(" =A0size is %lx, allocating...\n", size);
>> =A0 =A0 =A0 =A0 =A0/* Allocate memory for the expanded device tree */
>> - =A0 =A0 =A0 mem =3D lmb_alloc(size + 4, __alignof__(struct device_node=
));
>> + =A0 =A0 =A0 mem =3D early_init_dt_alloc_memory_arch(size + 4,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __alignof__(struct device_=
node));
>> =A0 =A0 =A0 =A0mem =3D (unsigned long) __va(mem);
>> =A0 =A0 =A0 =A0 =A0((__be32 *)mem)[size / 4] =3D cpu_to_be32(0xdeadbeef)=
;
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index c9cb8a7..a1ca92c 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -78,6 +78,7 @@ extern void early_init_dt_check_for_initrd(unsigned lo=
ng
>> node);
>> =A0extern int early_init_dt_scan_memory(unsigned long node, const char
>> *uname,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
int depth, void *data);
>> =A0extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>> +extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
>> =A0extern u64 dt_mem_next_cell(int s, __be32 **cellp);
>> =A0=A0/*
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" =
in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at =A0http://www.tux.org/lkml/
>
>
> --
> Michal Simek, Ing. (M.Eng)
> PetaLogix - Linux Solutions for a Reconfigurable World
> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 8/9] of/sparc: Remove sparc-local declaration of allnodes and devtree_lock
From: David Miller @ 2010-02-14 19:50 UTC (permalink / raw)
  To: grant.likely
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, linux-kernel,
	linuxppc-dev, sparclinux, jeremy.kerr
In-Reply-To: <20100213160309.4767.94443.stgit@angua>

From: Grant Likely <grant.likely@secretlab.ca>
Date: Sat, 13 Feb 2010 09:03:09 -0700

> Both allnodes and devtree_lock are defined in common code.  The
> extern declaration should be in the common header too so that the
> compiler can type check.  allnodes is already in of.h, but
> devtree_lock should be declared there too.
> 
> This patch removes the SPARC declarations and uses decls in of.h instead.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* [PATCH] powerpc: Don't clear larx reservation on system call exit
From: Anton Blanchard @ 2010-02-15  1:40 UTC (permalink / raw)
  To: benh, paulus; +Cc: linuxppc-dev


Right now we clear the larx reservation on every system call exit. No code
should exist that tries to make use of larx/stcx across a system call (because
it would fail 100% of the time).

We could continue to play it safe but system call latency affects so many
workloads. In the past we have already cut down the set of registers we
save and restore across a system call and this could be seen as an
extension of that. The PowerPC system call ABI does not (and could not)
preserve a larx reservation.

On POWER6 the poster child for system call improvements, getppid, improves 6%.

A more useful test is the private futex wake system call and that improves 5%.
This is a decent speedup on an important system call for threaded applications.

Signed-off-by: Anton Blanchard <anton@samba.org>
---
If my previous patches didn't worry you then this one is sure to.

Getting this wrong will make someone's life miserable, so it could do with
some double checking (eg we don't branch through there on other exceptions and
we dont invoke system calls from the kernel that rely on the reservation being
cleared).

Index: powerpc.git/arch/powerpc/kernel/entry_64.S
===================================================================
--- powerpc.git.orig/arch/powerpc/kernel/entry_64.S	2010-02-13 16:26:43.794322638 +1100
+++ powerpc.git/arch/powerpc/kernel/entry_64.S	2010-02-13 16:27:03.205575405 +1100
@@ -202,7 +202,6 @@ syscall_exit:
 	bge-	syscall_error
 syscall_error_cont:
 	ld	r7,_NIP(r1)
-	stdcx.	r0,0,r1			/* to clear the reservation */
 	andi.	r6,r8,MSR_PR
 	ld	r4,_LINK(r1)
 	/*

^ permalink raw reply

* Re: [PATCH] powerpc: Don't clear larx reservation on system call exit
From: Benjamin Herrenschmidt @ 2010-02-15  2:24 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: linuxppc-dev, paulus
In-Reply-To: <20100215014028.GB13355@kryten>

On Mon, 2010-02-15 at 12:40 +1100, Anton Blanchard wrote:
> Right now we clear the larx reservation on every system call exit. No code
> should exist that tries to make use of larx/stcx across a system call (because
> it would fail 100% of the time).
> 
> We could continue to play it safe but system call latency affects so many
> workloads. In the past we have already cut down the set of registers we
> save and restore across a system call and this could be seen as an
> extension of that. The PowerPC system call ABI does not (and could not)
> preserve a larx reservation.
> 
> On POWER6 the poster child for system call improvements, getppid, improves 6%.
> 
> A more useful test is the private futex wake system call and that improves 5%.
> This is a decent speedup on an important system call for threaded applications.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> If my previous patches didn't worry you then this one is sure to.
> 
> Getting this wrong will make someone's life miserable, so it could do with
> some double checking (eg we don't branch through there on other exceptions and
> we dont invoke system calls from the kernel that rely on the reservation being
> cleared).

Well, the main issue here is leaking kernel reservations into userspace,
and thus the question of whether it is a big deal or not. There's also
an issue I can see with signals.

The risk with kernel reservations leaking into userspace is a problem on
some processors that do not compare the reservation address locally
(only for snoops), thus userspace code doing lwarx/syscall/stwcx. might
end up with a suceeding stwcx. despite the fact that the original
reservation was long lost. 

At this stage it becomes an ABI problem, ie, whether we define the
behaviour of a lwarx/stwcx. accross a syscall as defined or not.

The other problem I see is that signal handlers would have to be made
very careful not to leave dangling reservations since the return from
the syscall is a syscall, unless we add code specifically to this (and
set_context too I'd say) to clear reservations.

IE. You could have something like:

lwarx, <interrupt>, signal handler, sigreturn, stwcx.

In the above case, the reservation would be cleared by the return from
the interrupt, but the signal handler might leave a dangling one, which
sigreturn might fail to clear (in practice, our current implementation
of sys_sigreturn() will probably clear any reservation as a side effect
of restore_sigmask() spinlock or set_thread_flag() but it sounds a bit
fragile to rely on unless it's well documented). 

Cheers,
Ben.

> Index: powerpc.git/arch/powerpc/kernel/entry_64.S
> ===================================================================
> --- powerpc.git.orig/arch/powerpc/kernel/entry_64.S	2010-02-13 16:26:43.794322638 +1100
> +++ powerpc.git/arch/powerpc/kernel/entry_64.S	2010-02-13 16:27:03.205575405 +1100
> @@ -202,7 +202,6 @@ syscall_exit:
>  	bge-	syscall_error
>  syscall_error_cont:
>  	ld	r7,_NIP(r1)
> -	stdcx.	r0,0,r1			/* to clear the reservation */
>  	andi.	r6,r8,MSR_PR
>  	ld	r4,_LINK(r1)
>  	/*

^ permalink raw reply

* Re: [PATCH] powerpc: Don't clear larx reservation on system call exit
From: Anton Blanchard @ 2010-02-15  4:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus
In-Reply-To: <1266200687.16346.110.camel@pasglop>

 
Hi Ben,

> Well, the main issue here is leaking kernel reservations into userspace,
> and thus the question of whether it is a big deal or not. There's also
> an issue I can see with signals.
> 
> The risk with kernel reservations leaking into userspace is a problem on
> some processors that do not compare the reservation address locally
> (only for snoops), thus userspace code doing lwarx/syscall/stwcx. might
> end up with a suceeding stwcx. despite the fact that the original
> reservation was long lost. 

Yeah that was my primary concern. Right now these things fail 100%, so
no one is relying on it. The worry is if people start writing their own
crazy low level system call + locking stubs that might work most of the
time (if we remove the stwcx in syscall exit).

> At this stage it becomes an ABI problem, ie, whether we define the
> behaviour of a lwarx/stwcx. accross a syscall as defined or not.
> 
> The other problem I see is that signal handlers would have to be made
> very careful not to leave dangling reservations since the return from
> the syscall is a syscall, unless we add code specifically to this (and
> set_context too I'd say) to clear reservations.
> 
> IE. You could have something like:
> 
> lwarx, <interrupt>, signal handler, sigreturn, stwcx.
> 
> In the above case, the reservation would be cleared by the return from
> the interrupt, but the signal handler might leave a dangling one, which
> sigreturn might fail to clear (in practice, our current implementation
> of sys_sigreturn() will probably clear any reservation as a side effect
> of restore_sigmask() spinlock or set_thread_flag() but it sounds a bit
> fragile to rely on unless it's well documented). 

Good point, I hadn't thought of signals and I agree we'd need to clear the
reservation in the sigreturn path.

Anton

^ permalink raw reply

* Re: [PATCH] powerpc: Don't clear larx reservation on system call exit
From: Benjamin Herrenschmidt @ 2010-02-15  4:15 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: linuxppc-dev, paulus
In-Reply-To: <20100215040657.GA24270@kryten>

On Mon, 2010-02-15 at 15:06 +1100, Anton Blanchard wrote:
> 
> Yeah that was my primary concern. Right now these things fail 100%, so
> no one is relying on it. The worry is if people start writing their own
> crazy low level system call + locking stubs that might work most of the
> time (if we remove the stwcx in syscall exit).

Worse than that. It will look like it works, but it won't really since
the dangling reservation matches a completely unrelated lwarx done by
the kernel and not the original userspace one, so the stwcx. might
succeed despite the fact that the original value -was- changed.

So it's really a matter of documentation I suppose but we have to be
careful as I've learned with time that there is nothing too silly for
userspace to do :-)

> Good point, I hadn't thought of signals and I agree we'd need to clear the
> reservation in the sigreturn path.

Right. As I said, I'm pretty sure it will happen as a side effect of
other things but I'd rather pay the small price of having an explicit
blurb to do it in sigreturn regardless.

Cheers,
Ben.

^ permalink raw reply

* [Patch 0/1] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XIII
From: K.Prasad @ 2010-02-15  5:56 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Michael Neuling, Benjamin Herrenschmidt, shaggy,
	Frederic Weisbecker, David Gibson, Alan Stern, paulus,
	Roland McGrath

Hi Ben,
	Please find the version XIII of the patch that ports the
hw-breakpoint interfaces (in kernel/hw_breakpoint.c) to PPC64. A new
patchset is necessitated due to a bug triggered by user-space breakpoint
requests.

This patch should not conflict with work done to enable debug register
use through ptrace interface for Book-E processors. Kindly include this
patch as a part of powerpc -next tree to enable wider testing and to
be eventually pushed upstream.

Changelog - ver XIII
--------------------
(Version XII: linuxppc-dev ref: 20100121084640.GA3252@in.ibm.com)

- Fixed a bug for user-space breakpoints (triggered following the
  failure of a breakpoint request) causing data exception (due to access
  of NULL/spurious address).
- Re-based on commit 724e6d3fe8003c3f60bf404bf22e4e331327c596 of
  linux-2.6
 
Thanks,
K.Prasad

Changelog - ver XII
--------------------
(Version XI: linuxppc-dev ref: 20100119091234.GA9971@in.ibm.com)

- Unset MSR_SE only if kernel was not previously in single-step mode.
- Pre-emption is now enabled before returning from the hw-breakpoint exception
  handler.
- Variables to track the source of single-step exception (breakpoint from kernel,
  user-space vs single-stepping due to other requests) are added.
- Extraneous hw-breakpoint exceptions (due to memory accesses lying outside
  monitored symbol length) is now done for both kernel and user-space
  (previously only user-space).
- single_step_dabr_instruction() now returns NOTIFY_DONE if kernel was in
  single-step mode even before the hw-breakpoint. This enables other users of
  single-step mode to be notified of the exception.
- User-space instructions are not emulated from kernel-space, they are instead
  single-stepped.
  
Changelog - ver XI
------------------
(Version X: linuxppc-dev ref: 20091211160144.GA23156@in.ibm.com)
- Conditionally unset MSR_SE in the single-step handler
- Added comments to explain the duration and need for pre-emption
disable following hw-breakpoint exception.

Changelog - ver X
------------------
- Re-write the PPC64 patches for the new implementation of hw-breakpoints that
  uses the perf-layer.
- Rebased to commit 7622fc234190a37d4e9fe3ed944a2b61a63fca03 of -tip.

Changelog - ver IX
-------------------
- Invocation of user-defined callback will be 'trigger-after-execute' (except
  for ptrace).
- Creation of a new global per-CPU breakpoint structure to help invocation of
  user-defined callback from single-step handler.
(Changes made now)
- Validation before registration will fail only if the address does not match
  the kernel symbol's (if specified) resolved address
  (through kallsyms_lookup_name()).
- 'symbolsize' value is expected to within the range contained by the symbol's
  starting address and the end of a double-word boundary (8 Bytes).
- PPC64's arch-dependant code is now aware of 'cpumask' in 'struct hw_breakpoint'
  and can accomodate requests for a subset of CPUs in the system.
- Introduced arch_disable_hw_breakpoint() required for
  <enable><disable>_hw_breakpoint() APIs.

Changelog - ver VIII
-------------------
- Reverting changes to allow one-shot breakpoints only for ptrace requests.
- Minor changes in sanity checking in arch_validate_hwbkpt_settings().
- put_cpu_no_resched() is no longer available. Converted to put_cpu().

Changelog - ver VII
-------------------
- Allow the one-shot behaviour for exception handlers to be defined by the user.
  A new 'is_one_shot' flag is added to 'struct arch_hw_breakpoint'.

Changelog - ver VI
------------------
The task of identifying 'genuine' breakpoint exceptions from those caused by
'out-of-range' accesses turned out to be more tricky than originally thought.
Some changes to this effect were made in version IV of this patchset, but they
were not sufficient for user-space. Basically the breakpoint address received
through ptrace is always aligned to 8-bytes since ptrace receives an encoded
'data' (consisting of address | translation_enable | bkpt_type), and the size of
the symbol is not known. However for kernel-space addresses, the symbol-size can
be determined using kallsyms_lookup_size_offset() and this is used to check if
DAR (in the exception context) is
'bkpt_address <= DAR <= (bkpt_address + symbol_size)', failing which we conclude
it as a stray exception.

The following changes are made to enable check:
- Addition of a symbolsize field in 'struct arch_hw_breakpoint' field.
- Store the size of the 'watched' kernel symbol into 'symbolsize' field in
  arch_store_info(0 routine.
- Verify if the above described condition is true when is_one_shot is FALSE in
  hw_breakpoint_handler().

Changelog - ver V
------------------
- Breakpoint requests from ptrace (for user-space) are designed to be one-shot
in PPC64. The patch contains changes to retain this behaviour by returning early
in hw_breakpoint_handler() [without re-initialising DABR] and unregistering the
user-space request in ptrace_triggered(). It is safe to make a
unregister_user_hw_breakpoint() call from the breakpoint exception context
[through ptrace_triggered()] without giving rise to circular locking-dependancy.
This is because there can be no kernel code running on the CPU (which received
the exception) with the same spinlock held.

- Minor change in 'type' member of 'struct arch_hw_breakpoint' from u8 to 'int'.

Changelog - ver IV
------------------
- While DABR register requires double-word (8 bytes) aligned addresses, i.e.
the breakpoint is active over a range of 8 bytes, PPC64 allows byte-level
addressability. This may lead to stray exceptions which have to be ignored in
hw_breakpoint_handler(), when DAR != (Breakpoint request address). However DABR
will be populated with the requested breakpoint address aligned to the previous
double-word address. The code is now modified to store user-requested address
in 'bp->info.address' but update the DABR with a double-word aligned address.

- Please note that the Data Breakpoint facility in Xmon is broken as of 2.6.29
and the same has not been integrated into this facility as described in Ver I.

Changelog - ver III
------------------
- Patches are based on commit 08f16e060bf54bdc34f800ed8b5362cdeda75d8b of -tip
tree.

- The declarations in arch/powerpc/include/asm/hw_breakpoint.h are done only if
CONFIG_PPC64 is defined. This eliminates the need to conditionally include this
header file.

- load_debug_registers() is done in start_secondary() i.e. during CPU
initialisation.

- arch_check_va_<> routines in hw_breakpoint.c are now replaced with a much
simpler is_kernel_addr() check in arch_validate_hwbkpt_settings()

- Return code of hw_breakpoint_handler() when triggered due to Lazy debug
register switching is now changed to NOTIFY_STOP.

- The ptrace code no longer sets the TIF_DEBUG task flag as it is proposed to
be done in register_user_hw_breakpoint() routine.

- hw_breakpoint_handler() is now modified to use hbp_kernel_pos value to
  determine if the trigger was a user/kernel space address. The DAR register
  value is checked with the address stored in 'struct hw_breakpoint' to avoid
  handling of exceptions that belong to kprobe/Xmon.


Changelog - ver II
------------------
- Split the monolithic patch into six logical patches

- Changed the signature of arch_check_va_in_<user><kernel>space functions. They
  are now marked static.

- HB_NUM is now called as HBP_NUM (to preserve a consistent short-name
  convention)

- Introduced hw_breakpoint_disable() and changes to kexec code to disable
  breakpoints before a reboot.
  
- Minor changes in ptrace code to use macro-defined constants instead of
  numbers.

- Introduced a new constant definition INSTRUCTION_LEN in reg.h

^ permalink raw reply

* [Patch 1/1] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
From: K.Prasad @ 2010-02-15  5:59 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Michael Neuling, Benjamin Herrenschmidt, shaggy,
	Frederic Weisbecker, David Gibson, Alan Stern, paulus,
	Roland McGrath
In-Reply-To: <20100215055605.GB3670@in.ibm.com>

Implement perf-events based hw-breakpoint interfaces for PPC64 processors.
These interfaces help arbitrate requests from various users and schedules
them as appropriate.

Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig                     |    1 
 arch/powerpc/include/asm/hw_breakpoint.h |   55 ++++
 arch/powerpc/include/asm/processor.h     |    6 
 arch/powerpc/include/asm/reg.h           |    1 
 arch/powerpc/kernel/Makefile             |    2 
 arch/powerpc/kernel/hw_breakpoint.c      |  363 +++++++++++++++++++++++++++++++
 arch/powerpc/kernel/process.c            |    5 
 arch/powerpc/kernel/ptrace.c             |   77 ++++++
 arch/powerpc/mm/fault.c                  |   14 -
 9 files changed, 515 insertions(+), 9 deletions(-)

Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h	2010-02-15 02:10:43.000000000 +0530
@@ -0,0 +1,55 @@
+#ifndef	_PPC64_HW_BREAKPOINT_H
+#define	_PPC64_HW_BREAKPOINT_H
+
+#ifdef	__KERNEL__
+#define	__ARCH_HW_BREAKPOINT_H
+#ifdef CONFIG_PPC64
+
+struct arch_hw_breakpoint {
+	u8		len; /* length of the target symbol */
+	int		type;
+	char		*name; /* Contains name of the symbol to set bkpt */
+	unsigned long	address;
+};
+
+#include <linux/kdebug.h>
+#include <asm/reg.h>
+#include <asm/system.h>
+
+/* Total number of available HW breakpoint registers */
+#define HBP_NUM 1
+
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp,
+						struct task_struct *tsk);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+						unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+void hw_breakpoint_pmu_unthrottle(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+			struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+	set_dabr(0);
+}
+
+#else
+static inline void hw_breakpoint_disable(void)
+{
+	/* Function is defined only on PPC64 for now */
+}
+#endif	/* CONFIG_PPC64 */
+#endif	/* __KERNEL__ */
+#endif	/* _PPC64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c	2010-02-15 02:10:43.000000000 +0530
@@ -0,0 +1,363 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2009 IBM Corporation
+ * Author: K.Prasad <prasad@linux.vnet.ibm.com>
+ *
+ */
+
+#include <linux/hw_breakpoint.h>
+#include <linux/notifier.h>
+#include <linux/kprobes.h>
+#include <linux/percpu.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/init.h>
+#include <linux/smp.h>
+
+#include <asm/hw_breakpoint.h>
+#include <asm/processor.h>
+#include <asm/sstep.h>
+
+/*
+ * Store the 'bp' that caused the hw-breakpoint exception just before we
+ * single-step. Used to distinguish a single-step exception (due to a previous
+ * hw-breakpoint exception) from a normal one
+ */
+static DEFINE_PER_CPU(struct perf_event *, last_hit_bp);
+
+/*
+ * Flag to denote if the kernel was already single-stepping. Used to
+ * conditionally unset the MSR_SE flag in the single-step exception
+ * following the breakpoint exception.
+ */
+static DEFINE_PER_CPU(bool, is_cpu_singlestep);
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for each cpus
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+	struct perf_event **slot = &__get_cpu_var(bp_per_reg);
+
+	if (!*slot)
+		*slot = bp;
+	else {
+		WARN_ONCE(1, "Can't find any breakpoint slot");
+		return -EBUSY;
+	}
+
+	set_dabr(info->address | info->type | DABR_TRANSLATION);
+	return 0;
+}
+
+/*
+ * Uninstall the breakpoint contained in the given counter.
+ *
+ * First we search the debug address register it uses and then we disable
+ * it.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+{
+	struct perf_event **slot = &__get_cpu_var(bp_per_reg);
+
+	if (*slot == bp)
+		*slot = NULL;
+	else {
+		WARN_ONCE(1, "Can't find the breakpoint slot");
+		return;
+	}
+	set_dabr(0);
+}
+
+/*
+ * Validate the arch-specific HW Breakpoint register settings
+ */
+int arch_validate_hwbkpt_settings(struct perf_event *bp,
+						struct task_struct *tsk)
+{
+	int is_kernel, ret = -EINVAL;
+	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+
+	if (!bp)
+		return ret;
+
+	switch (bp->attr.bp_type) {
+	case HW_BREAKPOINT_R:
+		info->type = DABR_DATA_READ;
+		break;
+	case HW_BREAKPOINT_W:
+		info->type = DABR_DATA_WRITE;
+		break;
+	case HW_BREAKPOINT_R | HW_BREAKPOINT_W:
+		info->type = (DABR_DATA_READ | DABR_DATA_WRITE);
+		break;
+	default:
+		return ret;
+	}
+	/* TODO: Check for a valid triggered function */
+	/* if (!bp->triggered)
+		return -EINVAL; */
+
+	is_kernel = is_kernel_addr(bp->attr.bp_addr);
+	if ((tsk && is_kernel) || (!tsk && !is_kernel))
+		return -EINVAL;
+
+	info->address = bp->attr.bp_addr;
+	info->len = bp->attr.bp_len;
+
+	/*
+	 * Since breakpoint length can be a maximum of HW_BREAKPOINT_LEN(8)
+	 * and breakpoint addresses are aligned to nearest double-word
+	 * HW_BREAKPOINT_ALIGN by rounding off to the lower address, the
+	 * 'symbolsize' should satisfy the check below.
+	 */
+	if (info->len >
+	    (HW_BREAKPOINT_LEN - (info->address & HW_BREAKPOINT_ALIGN)))
+		return -EINVAL;
+	return 0;
+}
+
+/*
+ * Handle debug exception notifications.
+ */
+int __kprobes hw_breakpoint_handler(struct die_args *args)
+{
+	int rc = NOTIFY_STOP;
+	struct perf_event *bp;
+	struct pt_regs *regs = args->regs;
+	unsigned long dar = regs->dar;
+	int cpu, is_kernel, stepped = 1;
+	struct arch_hw_breakpoint *info;
+
+	/* Disable breakpoints during exception handling */
+	set_dabr(0);
+	cpu = get_cpu();
+	/*
+	 * The counter may be concurrently released but that can only
+	 * occur from a call_rcu() path. We can then safely fetch
+	 * the breakpoint, use its callback, touch its counter
+	 * while we are in an rcu_read_lock() path.
+	 */
+	rcu_read_lock();
+
+	bp = per_cpu(bp_per_reg, cpu);
+	if (!bp)
+		goto out;
+	info = counter_arch_bp(bp);
+	is_kernel = is_kernel_addr(bp->attr.bp_addr);
+
+	/*
+	 * Verify if dar lies within the address range occupied by the symbol
+	 * being watched to filter extraneous exceptions.
+	 */
+	    if (!((bp->attr.bp_addr <= dar) &&
+	    (dar <= (bp->attr.bp_addr + bp->attr.bp_len))))
+		/*
+		 * This exception is triggered not because of a memory access on
+		 * the monitored variable but in the double-word address range
+		 * in which it is contained. We will consume this exception,
+		 * considering it as 'noise'.
+		 */
+		goto out;
+
+	/*
+	 * Return early after invoking user-callback function without restoring
+	 * DABR if the breakpoint is from ptrace which always operates in
+	 * one-shot mode
+	 */
+	if (bp->overflow_handler == ptrace_triggered) {
+		(bp->overflow_handler)(bp, 0, NULL, regs);
+		rc = NOTIFY_DONE;
+		goto out;
+	}
+
+	/*
+	 * Do not emulate user-space instructions from kernel-space,
+	 * instead single-step them.
+	 */
+	if (!is_kernel) {
+		current->thread.last_hit_ubp = bp;
+		goto out;
+	}
+
+	/*
+	 * Emulating single-step over causative instruction if already
+	 * in MSR_SE mode will harm other users of kernel single-stepping
+	 * (like an in-kernel debugger), single-step over them.
+	 */
+
+	if (regs->msr & MSR_SE) {
+		per_cpu(is_cpu_singlestep, cpu) = true;
+		per_cpu(last_hit_bp, cpu) = bp;
+		goto out;
+	}
+
+	stepped = emulate_step(regs, regs->nip);
+	 /* emulate_step() could not execute it, single-step them */
+	if (stepped == 0) {
+		regs->msr |= MSR_SE;
+		if (is_kernel)
+			per_cpu(last_hit_bp, cpu) = bp;
+		else
+			current->thread.last_hit_ubp = bp;
+		per_cpu(last_hit_bp, cpu) = bp;
+		goto out;
+	}
+	/*
+	 * As a policy, the callback is invoked in a 'trigger-after-execute'
+	 * fashion
+	 */
+	(bp->overflow_handler)(bp, 0, NULL, regs);
+	set_dabr(info->address | info->type | DABR_TRANSLATION);
+	per_cpu(is_cpu_singlestep, cpu) = false;
+out:
+	rcu_read_unlock();
+	put_cpu();
+	return rc;
+}
+
+/*
+ * Handle single-step exceptions following a DABR hit.
+ */
+int __kprobes single_step_dabr_instruction(struct die_args *args)
+{
+	struct pt_regs *regs = args->regs;
+	int cpu = get_cpu();
+	int ret = NOTIFY_DONE;
+	siginfo_t info;
+	struct perf_event *bp = NULL, *kernel_bp, *user_bp;
+	struct arch_hw_breakpoint *bp_info;
+
+	/*
+	 * Identify the cause of single-stepping and find the corresponding
+	 * breakpoint structure
+	 */
+	kernel_bp = per_cpu(last_hit_bp, cpu);
+	user_bp = current->thread.last_hit_ubp;
+	if (kernel_bp) {
+		bp = kernel_bp;
+		per_cpu(last_hit_bp, cpu) = NULL;
+	} else if (user_bp) {
+		bp = user_bp;
+		current->thread.last_hit_ubp = NULL;
+	}
+
+	/*
+	 * Check if we are single-stepping as a result of a
+	 * previous HW Breakpoint exception
+	 */
+	if (!bp)
+		goto out;
+
+	bp_info = counter_arch_bp(bp);
+
+	/*
+	 * We shall invoke the user-defined callback function in the single
+	 * stepping handler to confirm to 'trigger-after-execute' semantics
+	 */
+	(bp->overflow_handler)(bp, 0, NULL, regs);
+
+	if (!(per_cpu(is_cpu_singlestep, cpu) ||
+	      test_thread_flag(TIF_SINGLESTEP)))
+		regs->msr &= ~MSR_SE;
+
+	/* Deliver signal to user-space */
+	if (!is_kernel_addr(bp->attr.bp_addr)) {
+		info.si_signo = SIGTRAP;
+		info.si_errno = 0;
+		info.si_code = TRAP_HWBKPT;
+		info.si_addr = (void __user *)bp_info->address;
+		force_sig_info(SIGTRAP, &info, current);
+	}
+
+	set_dabr(bp_info->address | bp_info->type | DABR_TRANSLATION);
+	/*
+	 * The kernel was in single-step mode before hw-breakpoint
+	 * exception, allow them to process the single-step exception further.
+	 */
+	if (per_cpu(is_cpu_singlestep, cpu)) {
+		per_cpu(is_cpu_singlestep, cpu) = false;
+		ret = NOTIFY_DONE;
+	} else
+		ret = NOTIFY_STOP;
+out:
+	put_cpu();
+	return ret;
+}
+
+/*
+ * Handle debug exception notifications.
+ */
+int __kprobes hw_breakpoint_exceptions_notify(
+		struct notifier_block *unused, unsigned long val, void *data)
+{
+	int ret = NOTIFY_DONE;
+
+	switch (val) {
+	case DIE_DABR_MATCH:
+		ret = hw_breakpoint_handler(data);
+		break;
+	case DIE_SSTEP:
+		ret = single_step_dabr_instruction(data);
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * Release the user breakpoints used by ptrace
+ */
+void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
+{
+	struct thread_struct *t = &tsk->thread;
+
+	unregister_hw_breakpoint(t->ptrace_bps[0]);
+	t->ptrace_bps[0] = NULL;
+}
+
+void hw_breakpoint_pmu_read(struct perf_event *bp)
+{
+	/* TODO */
+}
+
+void hw_breakpoint_pmu_unthrottle(struct perf_event *bp)
+{
+	/* TODO */
+}
+
+
Index: linux-2.6.ppc64_test/arch/powerpc/Kconfig
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/Kconfig	2010-02-03 07:37:58.000000000 +0530
+++ linux-2.6.ppc64_test/arch/powerpc/Kconfig	2010-02-15 02:10:43.000000000 +0530
@@ -140,6 +140,7 @@
 	select HAVE_SYSCALL_WRAPPERS if PPC64
 	select GENERIC_ATOMIC64 if PPC32
 	select HAVE_PERF_EVENTS
+	select HAVE_HW_BREAKPOINT if PPC64
 
 config EARLY_PRINTK
 	bool
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/Makefile
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/Makefile	2010-02-03 07:37:59.000000000 +0530
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/Makefile	2010-02-15 02:10:43.000000000 +0530
@@ -33,7 +33,7 @@
 obj-y				+= vdso32/
 obj-$(CONFIG_PPC64)		+= setup_64.o sys_ppc32.o \
 				   signal_64.o ptrace32.o \
-				   paca.o nvram_64.o firmware.o
+				   paca.o nvram_64.o firmware.o hw_breakpoint.o
 obj-$(CONFIG_PPC_BOOK3S_64)	+= cpu_setup_ppc970.o cpu_setup_pa6t.o
 obj64-$(CONFIG_RELOCATABLE)	+= reloc_64.o
 obj-$(CONFIG_PPC_BOOK3E_64)	+= exceptions-64e.o
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/reg.h
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/reg.h	2010-02-03 07:37:59.000000000 +0530
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/reg.h	2010-02-15 02:10:43.000000000 +0530
@@ -180,6 +180,7 @@
 #define   CTRL_TE	0x00c00000	/* thread enable */
 #define   CTRL_RUNLATCH	0x1
 #define SPRN_DABR	0x3F5	/* Data Address Breakpoint Register */
+#define   HBP_NUM	1	/* Number of physical HW breakpoint registers */
 #define   DABR_TRANSLATION	(1UL << 2)
 #define   DABR_DATA_WRITE	(1UL << 1)
 #define   DABR_DATA_READ	(1UL << 0)
Index: linux-2.6.ppc64_test/arch/powerpc/mm/fault.c
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/mm/fault.c	2010-02-03 07:37:59.000000000 +0530
+++ linux-2.6.ppc64_test/arch/powerpc/mm/fault.c	2010-02-15 02:10:43.000000000 +0530
@@ -137,6 +137,12 @@
 		error_code &= 0x48200000;
 	else
 		is_write = error_code & DSISR_ISSTORE;
+
+	if (error_code & DSISR_DABRMATCH) {
+		/* DABR match */
+		do_dabr(regs, address, error_code);
+		return 0;
+	}
 #else
 	is_write = error_code & ESR_DST;
 #endif /* CONFIG_4xx || CONFIG_BOOKE */
@@ -151,14 +157,6 @@
 	if (!user_mode(regs) && (address >= TASK_SIZE))
 		return SIGSEGV;
 
-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
-  	if (error_code & DSISR_DABRMATCH) {
-		/* DABR match */
-		do_dabr(regs, address, error_code);
-		return 0;
-	}
-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
-
 	if (in_atomic() || mm == NULL) {
 		if (!user_mode(regs))
 			return SIGSEGV;
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/processor.h
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/processor.h	2010-02-03 07:37:59.000000000 +0530
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/processor.h	2010-02-15 02:10:43.000000000 +0530
@@ -177,6 +177,12 @@
 #ifdef CONFIG_PPC64
 	unsigned long	start_tb;	/* Start purr when proc switched in */
 	unsigned long	accum_tb;	/* Total accumilated purr for process */
+	struct perf_event *ptrace_bps[HBP_NUM];
+	/*
+	 * Point to the hw-breakpoint last. Helps safe pre-emption and
+	 * hw-breakpoint re-enablement.
+	 */
+	struct perf_event *last_hit_ubp;
 #endif
 	unsigned long	dabr;		/* Data address breakpoint register */
 #ifdef CONFIG_ALTIVEC
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/ptrace.c
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/ptrace.c	2010-02-03 07:37:59.000000000 +0530
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/ptrace.c	2010-02-15 02:11:24.000000000 +0530
@@ -32,6 +32,8 @@
 #ifdef CONFIG_PPC32
 #include <linux/module.h>
 #endif
+#include <linux/hw_breakpoint.h>
+#include <linux/perf_event.h>
 
 #include <asm/uaccess.h>
 #include <asm/page.h>
@@ -755,9 +757,32 @@
 	clear_tsk_thread_flag(task, TIF_SINGLESTEP);
 }
 
+void ptrace_triggered(struct perf_event *bp, int nmi,
+		      struct perf_sample_data *data, struct pt_regs *regs)
+{
+	struct perf_event_attr attr;
+
+	/*
+	 * Disable the breakpoint request here since ptrace has defined a
+	 * one-shot behaviour for breakpoint exceptions in PPC64.
+	 * The SIGTRAP signal is generated automatically for us in do_dabr().
+	 * We don't have to do anything about that here
+	 */
+	attr = bp->attr;
+	attr.disabled = true;
+	modify_user_hw_breakpoint(bp, &attr);
+}
+
 int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 			       unsigned long data)
 {
+#ifdef CONFIG_PPC64
+	int ret;
+	struct thread_struct *thread = &(task->thread);
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+#endif /* CONFIG_PPC64 */
+
 	/* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
 	 *  For embedded processors we support one DAC and no IAC's at the
 	 *  moment.
@@ -786,6 +811,60 @@
 	/* Ensure breakpoint translation bit is set */
 	if (data && !(data & DABR_TRANSLATION))
 		return -EIO;
+#ifdef CONFIG_PPC64
+	bp = thread->ptrace_bps[0];
+	if (data == 0) {
+		if (bp) {
+			unregister_hw_breakpoint(bp);
+			thread->ptrace_bps[0] = NULL;
+		}
+		return 0;
+	}
+	if (bp) {
+		attr = bp->attr;
+		attr.bp_addr = data & ~HW_BREAKPOINT_ALIGN;
+
+		switch (data & (DABR_DATA_WRITE | DABR_DATA_READ)) {
+		case DABR_DATA_READ:
+			attr.bp_type = HW_BREAKPOINT_R;
+			break;
+		case DABR_DATA_WRITE:
+			attr.bp_type = HW_BREAKPOINT_W;
+			break;
+		case (DABR_DATA_WRITE | DABR_DATA_READ):
+			attr.bp_type = HW_BREAKPOINT_R | HW_BREAKPOINT_W;
+			break;
+		}
+		ret =  modify_user_hw_breakpoint(bp, &attr);
+		if (ret)
+			return ret;
+		thread->ptrace_bps[0] = bp;
+		thread->dabr = data;
+		return 0;
+	}
+
+	/* Create a new breakpoint request if one doesn't exist already */
+	hw_breakpoint_init(&attr);
+	attr.bp_addr = data & ~HW_BREAKPOINT_ALIGN;
+	switch (data & (DABR_DATA_WRITE | DABR_DATA_READ)) {
+	case DABR_DATA_READ:
+		attr.bp_type = HW_BREAKPOINT_R;
+		break;
+	case DABR_DATA_WRITE:
+		attr.bp_type = HW_BREAKPOINT_W;
+		break;
+	case (DABR_DATA_WRITE | DABR_DATA_READ):
+		attr.bp_type = HW_BREAKPOINT_R | HW_BREAKPOINT_W;
+		break;
+	}
+	thread->ptrace_bps[0] = bp = register_user_hw_breakpoint(&attr,
+							ptrace_triggered, task);
+	if (IS_ERR(bp)) {
+		thread->ptrace_bps[0] = NULL;
+		return PTR_ERR(bp);
+	}
+
+#endif /* CONFIG_PPC64 */
 
 	/* Move contents to the DABR register */
 	task->thread.dabr = data;
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/process.c
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/process.c	2010-02-03 07:37:59.000000000 +0530
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/process.c	2010-02-15 02:10:43.000000000 +0530
@@ -48,6 +48,7 @@
 #include <asm/machdep.h>
 #include <asm/time.h>
 #include <asm/syscalls.h>
+#include <asm/hw_breakpoint.h>
 #ifdef CONFIG_PPC64
 #include <asm/firmware.h>
 #endif
@@ -376,8 +377,11 @@
 	if (new->thread.dabr)
 		set_dabr(new->thread.dabr);
 #else
+/* For PPC64, we use the hw-breakpoint interfaces that would schedule DABR */
+#ifndef CONFIG_PPC64
 	if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
 		set_dabr(new->thread.dabr);
+#endif /* CONFIG_PPC64 */
 #endif
 
 
@@ -396,6 +400,7 @@
 		old_thread->accum_tb += (current_tb - start_tb);
 		new_thread->start_tb = current_tb;
 	}
+	flush_ptrace_hw_breakpoint(current);
 #endif
 
 	local_irq_save(flags);

^ permalink raw reply

* Re: register long sp asm("r1") incorrect
From: Pavel Machek @ 2010-02-15  7:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus, kernel list
In-Reply-To: <1265866273.16346.34.camel@pasglop>

> On Tue, 2010-02-09 at 16:24 +0100, Pavel Machek wrote:
> > ...according to gcc docs, sp should be global, or placement in
> > register is not guaranteed (except at asm boundaries, but there are
> > none).
> 
> Sorry I'm not sure I grok what you mean.

Well, according to gcc doscs and my experience, local "register int
__asm()" variables only work by accident (or not at all).
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH v3 02/11] powerpc/mpc5121: Add machine restart support
From: Anatolij Gustschin @ 2010-02-15 16:38 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <20100209232406.GA21807@pengutronix.de>

Hi Wolfram,

On Wed, 10 Feb 2010 00:24:06 +0100
Wolfram Sang <w.sang@pengutronix.de> wrote:

> Two comments:
...
> > +void mpc512x_restart(char *cmd)
> > +{
> > +	struct mpc512x_reset_module *rm = reset_module_base;
> 
> Why not using reset_module_base directly?

I will fix it in v4 patch.

> > @@ -62,4 +95,5 @@ void __init mpc512x_init(void)
> >  {
> >  	mpc512x_declare_of_platform_devices();
> >  	mpc5121_clk_init();
> > +	mpc512x_restart_init();
> 
> If the return value is not checked here, you could as well make the function
> void. (Not much one could do in the error-case, too.)

Will fix it too, thanks!

Anatolij

^ permalink raw reply

* [PATCH v4 02/11] powerpc/mpc5121: Add machine restart support
From: Anatolij Gustschin @ 2010-02-15 16:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Piotr Ziecik, dzu, Anatolij Gustschin, Wolfgang Denk
In-Reply-To: <fa686aa41002091832y671c3cci4125b1923d535876@mail.gmail.com>

Add reset module registers representation and
machine restart callback for mpc5121 platform.

Signed-off-by: Piotr Ziecik <kosmo@semihalf.com>
Signed-off-by: Wolfgang Denk <wd@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: John Rigby <jcrigby@gmail.com>
---
Changes since v3:
 - move reset module struct definition to
   arch/powerpc/include/asm/mpc5121.h header file.
   Because of this change we also need to fix [PATCH v3 04/11]
   "mtd: Add MPC5121 NAND Flash Controller driver" to include
   the correct header. v4 for this patch will use correct header.
 - fix mpc512x_restart_init():
     Return value was not checked anywhere, so make the function void.
     Also use 'reset_module_base' directly.

Changes since v2:
 - call mpc512x_restart_init() explicitely from platform
   init code

Changes since v1:
 - use 'struct mpc512x_reset_module *' type for 'reset_module_base'
 - remove empty line
 - remove leftover colon and use pr_err() instead of printk.

 arch/powerpc/include/asm/mpc5121.h            |   24 ++++++++++++++++++++
 arch/powerpc/platforms/512x/mpc5121_ads.c     |    1 +
 arch/powerpc/platforms/512x/mpc5121_generic.c |    1 +
 arch/powerpc/platforms/512x/mpc512x.h         |    1 +
 arch/powerpc/platforms/512x/mpc512x_shared.c  |   30 +++++++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/include/asm/mpc5121.h

diff --git a/arch/powerpc/include/asm/mpc5121.h b/arch/powerpc/include/asm/mpc5121.h
new file mode 100644
index 0000000..e6a30bb
--- /dev/null
+++ b/arch/powerpc/include/asm/mpc5121.h
@@ -0,0 +1,24 @@
+/*
+ * MPC5121 Prototypes and definitions
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.
+ */
+
+#ifndef __ASM_POWERPC_MPC5121_H__
+#define __ASM_POWERPC_MPC5121_H__
+
+/* MPC512x Reset module registers */
+struct mpc512x_reset_module {
+	u32	rcwlr;	/* Reset Configuration Word Low Register */
+	u32	rcwhr;	/* Reset Configuration Word High Register */
+	u32	reserved1;
+	u32	reserved2;
+	u32	rsr;	/* Reset Status Register */
+	u32	rmr;	/* Reset Mode Register */
+	u32	rpr;	/* Reset Protection Register */
+	u32	rcr;	/* Reset Control Register */
+	u32	rcer;	/* Reset Control Enable Register */
+};
+
+#endif /* __ASM_POWERPC_MPC5121_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
index 0f8f2e9..ee6ae12 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads.c
+++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
@@ -68,4 +68,5 @@ define_machine(mpc5121_ads) {
 	.init_IRQ		= mpc5121_ads_init_IRQ,
 	.get_irq		= ipic_get_irq,
 	.calibrate_decr		= generic_calibrate_decr,
+	.restart		= mpc512x_restart,
 };
diff --git a/arch/powerpc/platforms/512x/mpc5121_generic.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
index 9b8c9b0..a6c0e3a 100644
--- a/arch/powerpc/platforms/512x/mpc5121_generic.c
+++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
@@ -55,4 +55,5 @@ define_machine(mpc5121_generic) {
 	.init_IRQ		= mpc512x_init_IRQ,
 	.get_irq		= ipic_get_irq,
 	.calibrate_decr		= generic_calibrate_decr,
+	.restart		= mpc512x_restart,
 };
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index ac3da1a..b2daca0 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -15,4 +15,5 @@ extern void __init mpc512x_init_IRQ(void);
 extern void __init mpc512x_init(void);
 extern int __init mpc5121_clk_init(void);
 void __init mpc512x_declare_of_platform_devices(void);
+extern void mpc512x_restart(char *cmd);
 #endif				/* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index b683165..a45824a 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -21,9 +21,38 @@
 #include <asm/ipic.h>
 #include <asm/prom.h>
 #include <asm/time.h>
+#include <asm/mpc5121.h>
 
 #include "mpc512x.h"
 
+static struct mpc512x_reset_module __iomem *reset_module_base;
+
+static void __init mpc512x_restart_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-reset");
+	if (!np)
+		return;
+
+	reset_module_base = of_iomap(np, 0);
+	of_node_put(np);
+}
+
+void mpc512x_restart(char *cmd)
+{
+	if (reset_module_base) {
+		/* Enable software reset "RSTE" */
+		out_be32(&reset_module_base->rpr, 0x52535445);
+		/* Set software hard reset */
+		out_be32(&reset_module_base->rcr, 0x2);
+	} else {
+		pr_err("Restart module not mapped.\n");
+	}
+	for (;;)
+		;
+}
+
 void __init mpc512x_init_IRQ(void)
 {
 	struct device_node *np;
@@ -62,4 +91,5 @@ void __init mpc512x_init(void)
 {
 	mpc512x_declare_of_platform_devices();
 	mpc5121_clk_init();
+	mpc512x_restart_init();
 }
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v4 04/11] mtd: Add MPC5121 NAND Flash Controller driver
From: Anatolij Gustschin @ 2010-02-15 17:35 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Piotr Ziecik, Detlev Zundel, linux-mtd, Anatolij Gustschin,
	David Woodhouse, Wolfgang Denk
In-Reply-To: <1265377377-29327-5-git-send-email-agust@denx.de>

Adds NAND Flash Controller driver for MPC5121 Revision 2.
All device features, except hardware ECC and power management,
are supported.

Signed-off-by: Piotr Ziecik <kosmo@semihalf.com>
Signed-off-by: Wolfgang Denk <wd@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Cc: <linux-mtd@lists.infradead.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: John Rigby <jcrigby@gmail.com>
---
Changes since v3:
 - include 'asm/mpc5121.h' header instead of 'asm/mpc5xxx.h'.
   This change is needed because arch patch adding reset
   module definition was reworked and doesn't add mpc5121
   specific definitions to common header for mpc52xx/mpc5121.

Changes since v2:
 - move the arch bits into separate patch
   (it is the next patch in this series now)
 - use __devinit/__devexit/__devexit_p and __devinitdata

Changes since v1:
 - add logfile with changes since previous version 

Changes since the patch version submitted in May 2009:

 - move mpc5121_nfc.h to the driver .c as there is only one user
 - remove DRV_VERSION macro
 - replace printk() by dev_*()
 - drop unnecessary .suspend and .resume initializations
 - remove duplicate .name/.owner settings
 - fix mpc5121_nfc_init() to "return of_register_platform_driver(&mpc5121_nfc_driver);"
 - move module_init() to just below the init function
 - remove MODULE_VERSION
 - use "mtd: Add MPC5121 NAND Flash Controller driver" as the subject,
   previously it was "mpc5121: Added NAND Flash Controller driver.

 drivers/mtd/nand/Kconfig       |    7 +
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/mpc5121_nfc.c |  916 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 924 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mtd/nand/mpc5121_nfc.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 677cd53..099f002 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -442,6 +442,13 @@ config MTD_NAND_FSL_UPM
 	  Enables support for NAND Flash chips wired onto Freescale PowerPC
 	  processor localbus with User-Programmable Machine support.
 
+config MTD_NAND_MPC5121_NFC
+	tristate "MPC5121 built-in NAND Flash Controller support"
+	depends on PPC_MPC512x
+	help
+	  This enables the driver for the NAND flash controller on the
+	  MPC5121 SoC.
+
 config MTD_NAND_MXC
 	tristate "MXC NAND support"
 	depends on ARCH_MX2 || ARCH_MX3
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1407bd1..d4ddf05 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -42,5 +42,6 @@ obj-$(CONFIG_MTD_NAND_TXX9NDFMC)	+= txx9ndfmc.o
 obj-$(CONFIG_MTD_NAND_W90P910)		+= w90p910_nand.o
 obj-$(CONFIG_MTD_NAND_NOMADIK)		+= nomadik_nand.o
 obj-$(CONFIG_MTD_NAND_BCM_UMI)		+= bcm_umi_nand.o nand_bcm_umi.o
+obj-$(CONFIG_MTD_NAND_MPC5121_NFC)	+= mpc5121_nfc.o
 
 nand-objs := nand_base.o nand_bbt.o
diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
new file mode 100644
index 0000000..d7333f4
--- /dev/null
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -0,0 +1,916 @@
+/*
+ * Copyright 2004-2008 Freescale Semiconductor, Inc.
+ * Copyright 2009 Semihalf.
+ *
+ * Approved as OSADL project by a majority of OSADL members and funded
+ * by OSADL membership fees in 2009;  for details see www.osadl.org.
+ *
+ * Based on original driver from Freescale Semiconductor
+ * written by John Rigby <jrigby@freescale.com> on basis
+ * of drivers/mtd/nand/mxc_nand.c. Reworked and extended
+ * Piotr Ziecik <kosmo@semihalf.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; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+
+#include <asm/mpc5121.h>
+
+/* Addresses for NFC MAIN RAM BUFFER areas */
+#define NFC_MAIN_AREA(n)	((n) *  0x200)
+
+/* Addresses for NFC SPARE BUFFER areas */
+#define NFC_SPARE_BUFFERS	8
+#define NFC_SPARE_LEN		0x40
+#define NFC_SPARE_AREA(n)	(0x1000 + ((n) * NFC_SPARE_LEN))
+
+/* MPC5121 NFC registers */
+#define NFC_BUF_ADDR		0x1E04
+#define NFC_FLASH_ADDR		0x1E06
+#define NFC_FLASH_CMD		0x1E08
+#define NFC_CONFIG		0x1E0A
+#define NFC_ECC_STATUS1		0x1E0C
+#define NFC_ECC_STATUS2		0x1E0E
+#define NFC_SPAS		0x1E10
+#define NFC_WRPROT		0x1E12
+#define NFC_NF_WRPRST		0x1E18
+#define NFC_CONFIG1		0x1E1A
+#define NFC_CONFIG2		0x1E1C
+#define NFC_UNLOCKSTART_BLK0	0x1E20
+#define NFC_UNLOCKEND_BLK0	0x1E22
+#define NFC_UNLOCKSTART_BLK1	0x1E24
+#define NFC_UNLOCKEND_BLK1	0x1E26
+#define NFC_UNLOCKSTART_BLK2	0x1E28
+#define NFC_UNLOCKEND_BLK2	0x1E2A
+#define NFC_UNLOCKSTART_BLK3	0x1E2C
+#define NFC_UNLOCKEND_BLK3	0x1E2E
+
+/* Bit Definitions: NFC_BUF_ADDR */
+#define NFC_RBA_MASK		(7 << 0)
+#define NFC_ACTIVE_CS_SHIFT	5
+#define NFC_ACTIVE_CS_MASK	(3 << NFC_ACTIVE_CS_SHIFT)
+
+/* Bit Definitions: NFC_CONFIG */
+#define NFC_BLS_UNLOCKED	(1 << 1)
+
+/* Bit Definitions: NFC_CONFIG1 */
+#define NFC_ECC_4BIT		(1 << 0)
+#define NFC_FULL_PAGE_DMA	(1 << 1)
+#define NFC_SPARE_ONLY		(1 << 2)
+#define NFC_ECC_ENABLE		(1 << 3)
+#define NFC_INT_MASK		(1 << 4)
+#define NFC_BIG_ENDIAN		(1 << 5)
+#define NFC_RESET		(1 << 6)
+#define NFC_CE			(1 << 7)
+#define NFC_ONE_CYCLE		(1 << 8)
+#define NFC_PPB_32		(0 << 9)
+#define NFC_PPB_64		(1 << 9)
+#define NFC_PPB_128		(2 << 9)
+#define NFC_PPB_256		(3 << 9)
+#define NFC_PPB_MASK		(3 << 9)
+#define NFC_FULL_PAGE_INT	(1 << 11)
+
+/* Bit Definitions: NFC_CONFIG2 */
+#define NFC_COMMAND		(1 << 0)
+#define NFC_ADDRESS		(1 << 1)
+#define NFC_INPUT		(1 << 2)
+#define NFC_OUTPUT		(1 << 3)
+#define NFC_ID			(1 << 4)
+#define NFC_STATUS		(1 << 5)
+#define NFC_CMD_FAIL		(1 << 15)
+#define NFC_INT			(1 << 15)
+
+/* Bit Definitions: NFC_WRPROT */
+#define NFC_WPC_LOCK_TIGHT	(1 << 0)
+#define NFC_WPC_LOCK		(1 << 1)
+#define NFC_WPC_UNLOCK		(1 << 2)
+
+#define	DRV_NAME		"mpc5121_nfc"
+
+/* Timeouts */
+#define NFC_RESET_TIMEOUT	1000		/* 1 ms */
+#define NFC_TIMEOUT		(HZ / 10)	/* 1/10 s */
+
+struct mpc5121_nfc_prv {
+	struct mtd_info		mtd;
+	struct nand_chip	chip;
+	int			irq;
+	void __iomem		*regs;
+	struct clk		*clk;
+	wait_queue_head_t	irq_waitq;
+	uint			column;
+	int			spareonly;
+	void __iomem		*csreg;
+	struct device		*dev;
+};
+
+static void mpc5121_nfc_done(struct mtd_info *mtd);
+
+#ifdef CONFIG_MTD_PARTITIONS
+static const char *mpc5121_nfc_pprobes[] = { "cmdlinepart", NULL };
+#endif
+
+/* Read NFC register */
+static inline u16 nfc_read(struct mtd_info *mtd, uint reg)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+
+	return in_be16(prv->regs + reg);
+}
+
+/* Write NFC register */
+static inline void nfc_write(struct mtd_info *mtd, uint reg, u16 val)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+
+	out_be16(prv->regs + reg, val);
+}
+
+/* Set bits in NFC register */
+static inline void nfc_set(struct mtd_info *mtd, uint reg, u16 bits)
+{
+	nfc_write(mtd, reg, nfc_read(mtd, reg) | bits);
+}
+
+/* Clear bits in NFC register */
+static inline void nfc_clear(struct mtd_info *mtd, uint reg, u16 bits)
+{
+	nfc_write(mtd, reg, nfc_read(mtd, reg) & ~bits);
+}
+
+/* Invoke address cycle */
+static inline void mpc5121_nfc_send_addr(struct mtd_info *mtd, u16 addr)
+{
+	nfc_write(mtd, NFC_FLASH_ADDR, addr);
+	nfc_write(mtd, NFC_CONFIG2, NFC_ADDRESS);
+	mpc5121_nfc_done(mtd);
+}
+
+/* Invoke command cycle */
+static inline void mpc5121_nfc_send_cmd(struct mtd_info *mtd, u16 cmd)
+{
+	nfc_write(mtd, NFC_FLASH_CMD, cmd);
+	nfc_write(mtd, NFC_CONFIG2, NFC_COMMAND);
+	mpc5121_nfc_done(mtd);
+}
+
+/* Send data from NFC buffers to NAND flash */
+static inline void mpc5121_nfc_send_prog_page(struct mtd_info *mtd)
+{
+	nfc_clear(mtd, NFC_BUF_ADDR, NFC_RBA_MASK);
+	nfc_write(mtd, NFC_CONFIG2, NFC_INPUT);
+	mpc5121_nfc_done(mtd);
+}
+
+/* Receive data from NAND flash */
+static inline void mpc5121_nfc_send_read_page(struct mtd_info *mtd)
+{
+	nfc_clear(mtd, NFC_BUF_ADDR, NFC_RBA_MASK);
+	nfc_write(mtd, NFC_CONFIG2, NFC_OUTPUT);
+	mpc5121_nfc_done(mtd);
+}
+
+/* Receive ID from NAND flash */
+static inline void mpc5121_nfc_send_read_id(struct mtd_info *mtd)
+{
+	nfc_clear(mtd, NFC_BUF_ADDR, NFC_RBA_MASK);
+	nfc_write(mtd, NFC_CONFIG2, NFC_ID);
+	mpc5121_nfc_done(mtd);
+}
+
+/* Receive status from NAND flash */
+static inline void mpc5121_nfc_send_read_status(struct mtd_info *mtd)
+{
+	nfc_clear(mtd, NFC_BUF_ADDR, NFC_RBA_MASK);
+	nfc_write(mtd, NFC_CONFIG2, NFC_STATUS);
+	mpc5121_nfc_done(mtd);
+}
+
+/* NFC interrupt handler */
+static irqreturn_t mpc5121_nfc_irq(int irq, void *data)
+{
+	struct mtd_info *mtd = data;
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+
+	nfc_set(mtd, NFC_CONFIG1, NFC_INT_MASK);
+	wake_up(&prv->irq_waitq);
+
+	return IRQ_HANDLED;
+}
+
+/* Wait for operation complete */
+static void mpc5121_nfc_done(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+	int rv;
+
+	if ((nfc_read(mtd, NFC_CONFIG2) & NFC_INT) == 0) {
+		nfc_clear(mtd, NFC_CONFIG1, NFC_INT_MASK);
+		rv = wait_event_timeout(prv->irq_waitq,
+			(nfc_read(mtd, NFC_CONFIG2) & NFC_INT), NFC_TIMEOUT);
+
+		if (!rv)
+			dev_warn(prv->dev,
+				"Timeout while waiting for interrupt.\n");
+	}
+
+	nfc_clear(mtd, NFC_CONFIG2, NFC_INT);
+}
+
+/* Do address cycle(s) */
+static void mpc5121_nfc_addr_cycle(struct mtd_info *mtd, int column, int page)
+{
+	struct nand_chip *chip = mtd->priv;
+	u32 pagemask = chip->pagemask;
+
+	if (column != -1) {
+		mpc5121_nfc_send_addr(mtd, column);
+		if (mtd->writesize > 512)
+			mpc5121_nfc_send_addr(mtd, column >> 8);
+	}
+
+	if (page != -1) {
+		do {
+			mpc5121_nfc_send_addr(mtd, page & 0xFF);
+			page >>= 8;
+			pagemask >>= 8;
+		} while (pagemask);
+	}
+}
+
+/* Control chip select signals */
+static void mpc5121_nfc_select_chip(struct mtd_info *mtd, int chip)
+{
+	if (chip < 0) {
+		nfc_clear(mtd, NFC_CONFIG1, NFC_CE);
+		return;
+	}
+
+	nfc_clear(mtd, NFC_BUF_ADDR, NFC_ACTIVE_CS_MASK);
+	nfc_set(mtd, NFC_BUF_ADDR, (chip << NFC_ACTIVE_CS_SHIFT) &
+							NFC_ACTIVE_CS_MASK);
+	nfc_set(mtd, NFC_CONFIG1, NFC_CE);
+}
+
+/* Init external chip select logic on ADS5121 board */
+static int ads5121_chipselect_init(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+	struct device_node *dn;
+
+	dn = of_find_compatible_node(NULL, NULL, "fsl,mpc5121ads-cpld");
+	if (dn) {
+		prv->csreg = of_iomap(dn, 0);
+		of_node_put(dn);
+		if (!prv->csreg)
+			return -ENOMEM;
+
+		/* CPLD Register 9 controls NAND /CE Lines */
+		prv->csreg += 9;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+/* Control chips select signal on ADS5121 board */
+static void ads5121_select_chip(struct mtd_info *mtd, int chip)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct mpc5121_nfc_prv *prv = nand->priv;
+	u8 v;
+
+	v = in_8(prv->csreg);
+	v |= 0x0F;
+
+	if (chip >= 0) {
+		mpc5121_nfc_select_chip(mtd, 0);
+		v &= ~(1 << chip);
+	} else
+		mpc5121_nfc_select_chip(mtd, -1);
+
+	out_8(prv->csreg, v);
+}
+
+/* Read NAND Ready/Busy signal */
+static int mpc5121_nfc_dev_ready(struct mtd_info *mtd)
+{
+	/*
+	 * NFC handles ready/busy signal internally. Therefore, this function
+	 * always returns status as ready.
+	 */
+	return 1;
+}
+
+/* Write command to NAND flash */
+static void mpc5121_nfc_command(struct mtd_info *mtd, unsigned command,
+							int column, int page)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+
+	prv->column = (column >= 0) ? column : 0;
+	prv->spareonly = 0;
+
+	switch (command) {
+	case NAND_CMD_PAGEPROG:
+		mpc5121_nfc_send_prog_page(mtd);
+		break;
+	/*
+	 * NFC does not support sub-page reads and writes,
+	 * so emulate them using full page transfers.
+	 */
+	case NAND_CMD_READ0:
+		column = 0;
+		break;
+
+	case NAND_CMD_READ1:
+		prv->column += 256;
+		command = NAND_CMD_READ0;
+		column = 0;
+		break;
+
+	case NAND_CMD_READOOB:
+		prv->spareonly = 1;
+		command = NAND_CMD_READ0;
+		column = 0;
+		break;
+
+	case NAND_CMD_SEQIN:
+		mpc5121_nfc_command(mtd, NAND_CMD_READ0, column, page);
+		column = 0;
+		break;
+
+	case NAND_CMD_ERASE1:
+	case NAND_CMD_ERASE2:
+	case NAND_CMD_READID:
+	case NAND_CMD_STATUS:
+		break;
+
+	default:
+		return;
+	}
+
+	mpc5121_nfc_send_cmd(mtd, command);
+	mpc5121_nfc_addr_cycle(mtd, column, page);
+
+	switch (command) {
+	case NAND_CMD_READ0:
+		if (mtd->writesize > 512)
+			mpc5121_nfc_send_cmd(mtd, NAND_CMD_READSTART);
+		mpc5121_nfc_send_read_page(mtd);
+		break;
+
+	case NAND_CMD_READID:
+		mpc5121_nfc_send_read_id(mtd);
+		break;
+
+	case NAND_CMD_STATUS:
+		mpc5121_nfc_send_read_status(mtd);
+		if (chip->options & NAND_BUSWIDTH_16)
+			prv->column = 1;
+		else
+			prv->column = 0;
+		break;
+	}
+}
+
+/* Copy data from/to NFC spare buffers. */
+static void mpc5121_nfc_copy_spare(struct mtd_info *mtd, uint offset,
+						u8 *buffer, uint size, int wr)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct mpc5121_nfc_prv *prv = nand->priv;
+	uint o, s, sbsize, blksize;
+
+	/*
+	 * NAND spare area is available through NFC spare buffers.
+	 * The NFC divides spare area into (page_size / 512) chunks.
+	 * Each chunk is placed into separate spare memory area, using
+	 * first (spare_size / num_of_chunks) bytes of the buffer.
+	 *
+	 * For NAND device in which the spare area is not divided fully
+	 * by the number of chunks, number of used bytes in each spare
+	 * buffer is rounded down to the nearest even number of bytes,
+	 * and all remaining bytes are added to the last used spare area.
+	 *
+	 * For more information read section 26.6.10 of MPC5121e
+	 * Microcontroller Reference Manual, Rev. 3.
+	 */
+
+	/* Calculate number of valid bytes in each spare buffer */
+	sbsize = (mtd->oobsize / (mtd->writesize / 512)) & ~1;
+
+	while (size) {
+		/* Calculate spare buffer number */
+		s = offset / sbsize;
+		if (s > NFC_SPARE_BUFFERS - 1)
+			s = NFC_SPARE_BUFFERS - 1;
+
+		/*
+		 * Calculate offset to requested data block in selected spare
+		 * buffer and its size.
+		 */
+		o = offset - (s * sbsize);
+		blksize = min(sbsize - o, size);
+
+		if (wr)
+			memcpy_toio(prv->regs + NFC_SPARE_AREA(s) + o,
+							buffer, blksize);
+		else
+			memcpy_fromio(buffer,
+				prv->regs + NFC_SPARE_AREA(s) + o, blksize);
+
+		buffer += blksize;
+		offset += blksize;
+		size -= blksize;
+	};
+}
+
+/* Copy data from/to NFC main and spare buffers */
+static void mpc5121_nfc_buf_copy(struct mtd_info *mtd, u_char *buf, int len,
+									int wr)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+	uint c = prv->column;
+	uint l;
+
+	/* Handle spare area access */
+	if (prv->spareonly || c >= mtd->writesize) {
+		/* Calculate offset from beginning of spare area */
+		if (c >= mtd->writesize)
+			c -= mtd->writesize;
+
+		prv->column += len;
+		mpc5121_nfc_copy_spare(mtd, c, buf, len, wr);
+		return;
+	}
+
+	/*
+	 * Handle main area access - limit copy length to prevent
+	 * crossing main/spare boundary.
+	 */
+	l = min((uint)len, mtd->writesize - c);
+	prv->column += l;
+
+	if (wr)
+		memcpy_toio(prv->regs + NFC_MAIN_AREA(0) + c, buf, l);
+	else
+		memcpy_fromio(buf, prv->regs + NFC_MAIN_AREA(0) + c, l);
+
+	/* Handle crossing main/spare boundary */
+	if (l != len) {
+		buf += l;
+		len -= l;
+		mpc5121_nfc_buf_copy(mtd, buf, len, wr);
+	}
+}
+
+/* Read data from NFC buffers */
+static void mpc5121_nfc_read_buf(struct mtd_info *mtd, u_char *buf, int len)
+{
+	mpc5121_nfc_buf_copy(mtd, buf, len, 0);
+}
+
+/* Write data to NFC buffers */
+static void mpc5121_nfc_write_buf(struct mtd_info *mtd,
+						const u_char *buf, int len)
+{
+	mpc5121_nfc_buf_copy(mtd, (u_char *)buf, len, 1);
+}
+
+/* Compare buffer with NAND flash */
+static int mpc5121_nfc_verify_buf(struct mtd_info *mtd,
+						const u_char *buf, int len)
+{
+	u_char tmp[256];
+	uint bsize;
+
+	while (len) {
+		bsize = min(len, 256);
+		mpc5121_nfc_read_buf(mtd, tmp, bsize);
+
+		if (memcmp(buf, tmp, bsize))
+			return 1;
+
+		buf += bsize;
+		len -= bsize;
+	}
+
+	return 0;
+}
+
+/* Read byte from NFC buffers */
+static u8 mpc5121_nfc_read_byte(struct mtd_info *mtd)
+{
+	u8 tmp;
+
+	mpc5121_nfc_read_buf(mtd, &tmp, sizeof(tmp));
+
+	return tmp;
+}
+
+/* Read word from NFC buffers */
+static u16 mpc5121_nfc_read_word(struct mtd_info *mtd)
+{
+	u16 tmp;
+
+	mpc5121_nfc_read_buf(mtd, (u_char *)&tmp, sizeof(tmp));
+
+	return tmp;
+}
+
+/*
+ * Read NFC configuration from Reset Config Word
+ *
+ * NFC is configured during reset in basis of information stored
+ * in Reset Config Word. There is no other way to set NAND block
+ * size, spare size and bus width.
+ */
+static int mpc5121_nfc_read_hw_config(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+	struct mpc512x_reset_module *rm;
+	struct device_node *rmnode;
+	uint rcw_pagesize = 0;
+	uint rcw_sparesize = 0;
+	uint rcw_width;
+	uint rcwh;
+	uint romloc, ps;
+
+	rmnode = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-reset");
+	if (!rmnode) {
+		dev_err(prv->dev, "Missing 'fsl,mpc5121-reset' "
+					"node in device tree!\n");
+		return -ENODEV;
+	}
+
+	rm = of_iomap(rmnode, 0);
+	if (!rm) {
+		dev_err(prv->dev, "Error mapping reset module node!\n");
+		return -EBUSY;
+	}
+
+	rcwh = in_be32(&rm->rcwhr);
+
+	/* Bit 6: NFC bus width */
+	rcw_width = ((rcwh >> 6) & 0x1) ? 2 : 1;
+
+	/* Bit 7: NFC Page/Spare size */
+	ps = (rcwh >> 7) & 0x1;
+
+	/* Bits [22:21]: ROM Location */
+	romloc = (rcwh >> 21) & 0x3;
+
+	/* Decode RCW bits */
+	switch ((ps << 2) | romloc) {
+	case 0x00:
+	case 0x01:
+		rcw_pagesize = 512;
+		rcw_sparesize = 16;
+		break;
+	case 0x02:
+	case 0x03:
+		rcw_pagesize = 4096;
+		rcw_sparesize = 128;
+		break;
+	case 0x04:
+	case 0x05:
+		rcw_pagesize = 2048;
+		rcw_sparesize = 64;
+		break;
+	case 0x06:
+	case 0x07:
+		rcw_pagesize = 4096;
+		rcw_sparesize = 218;
+		break;
+	}
+
+	mtd->writesize = rcw_pagesize;
+	mtd->oobsize = rcw_sparesize;
+	if (rcw_width == 2)
+		chip->options |= NAND_BUSWIDTH_16;
+
+	dev_notice(prv->dev, "Configured for "
+				"%u-bit NAND, page size %u "
+				"with %u spare.\n",
+				rcw_width * 8, rcw_pagesize,
+				rcw_sparesize);
+	iounmap(rm);
+	of_node_put(rmnode);
+	return 0;
+}
+
+/* Free driver resources */
+static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+
+	if (prv->clk) {
+		clk_disable(prv->clk);
+		clk_put(prv->clk);
+	}
+
+	if (prv->csreg)
+		iounmap(prv->csreg);
+}
+
+static int __devinit mpc5121_nfc_probe(struct of_device *op,
+					const struct of_device_id *match)
+{
+	struct device_node *rootnode, *dn = op->node;
+	struct device *dev = &op->dev;
+	struct mpc5121_nfc_prv *prv;
+	struct resource res;
+	struct mtd_info *mtd;
+#ifdef CONFIG_MTD_PARTITIONS
+	struct mtd_partition *parts;
+#endif
+	struct nand_chip *chip;
+	unsigned long regs_paddr, regs_size;
+	const uint *chips_no;
+	int resettime = 0;
+	int retval = 0;
+	int rev, len;
+
+	/*
+	 * Check SoC revision. This driver supports only NFC
+	 * in MPC5121 revision 2.
+	 */
+	rev = (mfspr(SPRN_SVR) >> 4) & 0xF;
+	if (rev != 2) {
+		dev_err(dev, "SoC revision %u is not supported!\n", rev);
+		return -ENXIO;
+	}
+
+	prv = devm_kzalloc(dev, sizeof(*prv), GFP_KERNEL);
+	if (!prv) {
+		dev_err(dev, "Memory exhausted!\n");
+		return -ENOMEM;
+	}
+
+	mtd = &prv->mtd;
+	chip = &prv->chip;
+
+	mtd->priv = chip;
+	chip->priv = prv;
+	prv->dev = dev;
+
+	/* Read NFC configuration from Reset Config Word */
+	retval = mpc5121_nfc_read_hw_config(mtd);
+	if (retval) {
+		dev_err(dev, "Unable to read NFC config!\n");
+		return retval;
+	}
+
+	prv->irq = irq_of_parse_and_map(dn, 0);
+	if (prv->irq == NO_IRQ) {
+		dev_err(dev, "Error mapping IRQ!\n");
+		return -EINVAL;
+	}
+
+	retval = of_address_to_resource(dn, 0, &res);
+	if (retval) {
+		dev_err(dev, "Error parsing memory region!\n");
+		return retval;
+	}
+
+	chips_no = of_get_property(dn, "chips", &len);
+	if (!chips_no || len != sizeof(*chips_no)) {
+		dev_err(dev, "Invalid/missing 'chips' property!\n");
+		return -EINVAL;
+	}
+
+	regs_paddr = res.start;
+	regs_size = res.end - res.start + 1;
+
+	if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
+		dev_err(dev, "Error requesting memory region!\n");
+		return -EBUSY;
+	}
+
+	prv->regs = devm_ioremap(dev, regs_paddr, regs_size);
+	if (!prv->regs) {
+		dev_err(dev, "Error mapping memory region!\n");
+		return -ENOMEM;
+	}
+
+	mtd->name = "MPC5121 NAND";
+	chip->dev_ready = mpc5121_nfc_dev_ready;
+	chip->cmdfunc = mpc5121_nfc_command;
+	chip->read_byte = mpc5121_nfc_read_byte;
+	chip->read_word = mpc5121_nfc_read_word;
+	chip->read_buf = mpc5121_nfc_read_buf;
+	chip->write_buf = mpc5121_nfc_write_buf;
+	chip->verify_buf = mpc5121_nfc_verify_buf;
+	chip->select_chip = mpc5121_nfc_select_chip;
+	chip->options = NAND_NO_AUTOINCR | NAND_USE_FLASH_BBT;
+	chip->ecc.mode = NAND_ECC_SOFT;
+
+	/* Support external chip-select logic on ADS5121 board */
+	rootnode = of_find_node_by_path("/");
+	if (of_device_is_compatible(rootnode, "fsl,mpc5121ads")) {
+		retval = ads5121_chipselect_init(mtd);
+		if (retval) {
+			dev_err(dev, "Chipselect init error!\n");
+			of_node_put(rootnode);
+			return retval;
+		}
+
+		chip->select_chip = ads5121_select_chip;
+	}
+	of_node_put(rootnode);
+
+	/* Enable NFC clock */
+	prv->clk = clk_get(dev, "nfc_clk");
+	if (!prv->clk) {
+		dev_err(dev, "Unable to acquire NFC clock!\n");
+		retval = -ENODEV;
+		goto error;
+	}
+
+	clk_enable(prv->clk);
+
+	/* Reset NAND Flash controller */
+	nfc_set(mtd, NFC_CONFIG1, NFC_RESET);
+	while (nfc_read(mtd, NFC_CONFIG1) & NFC_RESET) {
+		if (resettime++ >= NFC_RESET_TIMEOUT) {
+			dev_err(dev, "Timeout while resetting NFC!\n");
+			retval = -EINVAL;
+			goto error;
+		}
+
+		udelay(1);
+	}
+
+	/* Enable write to NFC memory */
+	nfc_write(mtd, NFC_CONFIG, NFC_BLS_UNLOCKED);
+
+	/* Enable write to all NAND pages */
+	nfc_write(mtd, NFC_UNLOCKSTART_BLK0, 0x0000);
+	nfc_write(mtd, NFC_UNLOCKEND_BLK0, 0xFFFF);
+	nfc_write(mtd, NFC_WRPROT, NFC_WPC_UNLOCK);
+
+	/*
+	 * Setup NFC:
+	 *	- Big Endian transfers,
+	 *	- Interrupt after full page read/write.
+	 */
+	nfc_write(mtd, NFC_CONFIG1, NFC_BIG_ENDIAN | NFC_INT_MASK |
+							NFC_FULL_PAGE_INT);
+
+	/* Set spare area size */
+	nfc_write(mtd, NFC_SPAS, mtd->oobsize >> 1);
+
+	init_waitqueue_head(&prv->irq_waitq);
+	retval = devm_request_irq(dev, prv->irq, &mpc5121_nfc_irq, 0, DRV_NAME,
+									mtd);
+	if (retval) {
+		dev_err(dev, "Error requesting IRQ!\n");
+		goto error;
+	}
+
+	/* Detect NAND chips */
+	if (nand_scan(mtd, *chips_no)) {
+		dev_err(dev, "NAND Flash not found !\n");
+		devm_free_irq(dev, prv->irq, mtd);
+		retval = -ENXIO;
+		goto error;
+	}
+
+	/* Set erase block size */
+	switch (mtd->erasesize / mtd->writesize) {
+	case 32:
+		nfc_set(mtd, NFC_CONFIG1, NFC_PPB_32);
+		break;
+
+	case 64:
+		nfc_set(mtd, NFC_CONFIG1, NFC_PPB_64);
+		break;
+
+	case 128:
+		nfc_set(mtd, NFC_CONFIG1, NFC_PPB_128);
+		break;
+
+	case 256:
+		nfc_set(mtd, NFC_CONFIG1, NFC_PPB_256);
+		break;
+
+	default:
+		dev_err(dev, "Unsupported NAND flash!\n");
+		devm_free_irq(dev, prv->irq, mtd);
+		retval = -ENXIO;
+		goto error;
+	}
+
+	dev_set_drvdata(dev, mtd);
+
+	/* Register device in MTD */
+#ifdef CONFIG_MTD_PARTITIONS
+	retval = parse_mtd_partitions(mtd, mpc5121_nfc_pprobes, &parts, 0);
+#ifdef CONFIG_MTD_OF_PARTS
+	if (retval == 0)
+		retval = of_mtd_parse_partitions(dev, dn, &parts);
+#endif
+	if (retval < 0) {
+		dev_err(dev, "Error parsing MTD partitions!\n");
+		devm_free_irq(dev, prv->irq, mtd);
+		retval = -EINVAL;
+		goto error;
+	}
+
+	if (retval > 0)
+		retval = add_mtd_partitions(mtd, parts, retval);
+	else
+#endif
+		retval = add_mtd_device(mtd);
+
+	if (retval) {
+		dev_err(dev, "Error adding MTD device!\n");
+		devm_free_irq(dev, prv->irq, mtd);
+		goto error;
+	}
+
+	return 0;
+error:
+	mpc5121_nfc_free(dev, mtd);
+	return retval;
+}
+
+static int __devexit mpc5121_nfc_remove(struct of_device *op)
+{
+	struct device *dev = &op->dev;
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+	struct nand_chip *chip = mtd->priv;
+	struct mpc5121_nfc_prv *prv = chip->priv;
+
+	nand_release(mtd);
+	devm_free_irq(dev, prv->irq, mtd);
+	mpc5121_nfc_free(dev, mtd);
+
+	return 0;
+}
+
+static struct of_device_id mpc5121_nfc_match[] __devinitdata = {
+	{ .compatible = "fsl,mpc5121-nfc", },
+	{},
+};
+
+static struct of_platform_driver mpc5121_nfc_driver = {
+	.match_table	= mpc5121_nfc_match,
+	.probe		= mpc5121_nfc_probe,
+	.remove		= __devexit_p(mpc5121_nfc_remove),
+	.driver		= {
+		.name	= DRV_NAME,
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init mpc5121_nfc_init(void)
+{
+	return of_register_platform_driver(&mpc5121_nfc_driver);
+}
+
+module_init(mpc5121_nfc_init);
+
+static void __exit mpc5121_nfc_cleanup(void)
+{
+	of_unregister_platform_driver(&mpc5121_nfc_driver);
+}
+
+module_exit(mpc5121_nfc_cleanup);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("MPC5121 NAND MTD driver");
+MODULE_LICENSE("GPL");
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH v7 0/4] i2c-mpc: add support for the Freescale MPC512x and other fixes
From: Grant Likely @ 2010-02-15 19:34 UTC (permalink / raw)
  To: Wolfgang Grandegger, Ben Dooks
  Cc: Devicetree-discuss, Linuxppc-dev, Linux-i2c, Wolfgang Grandegger
In-Reply-To: <1265813711-16794-1-git-send-email-wg@grandegger.com>

Ben D.,

These 4 patches are totally fine.  Do you want to pick them up, or
should I take them through the PowerPC tree?

Cheers,
g.

On Wed, Feb 10, 2010 at 7:55 AM, Wolfgang Grandegger <wg@grandegger.com> wr=
ote:
> From: Wolfgang Grandegger <wg@denx.de>
>
> This patch series adds support for the MPC512x from Freescale to the
> i2c-mpc driver. At that occasion, issues with =A0__devinit[data] have
> been fixed and the doc of the FSL I2C dts bindings updated. It has
> been tested on a MPC5121ADS, TQM5200 and TQM8560 board
>
> Changes since v1:
>
> - use macro MPC_I2C_CLOCK_PRESERVE/SAFE for the special clock settings.
> - document the special DTS node "fsl,mpc5121-i2c-ctrl".
> - update and correct the Kconfig help.
> - some other minor fixes as suggested by Wolfram.
>
> Changes since v2:
>
> - use __init[data] instead of __devinit[data] for this driver.
>
> Changes since v3:
>
> - switch back to __devinit[data] as pointed out by Ben.
>
> Changes since v4:
>
> - check MPC_I2C_CLOCK_SAFE instead of "!clock" as suggested by Wolfram.
> - update MODULE_DESCRIPTION().
>
> Changes since v5 (suggested by Grant Likely):
>
> - various correctings for labling initialization functions and data
> =A0(this is tricky because section mismatches are not always obvious).
> - add a separate patch for renaming the setclock into setup functions.
> - correct the doc of the I2C bindings, e.g. don't mention the legacy
> =A0clock setting and remove obsolte parts.
>
> Changes since v6:
>
> - use __devinitconst for const data as suggested by Stephen Rothwell.
>
> Wolfgang
>
> Wolfgang Grandegger (4):
> =A0i2c-mpc: use __devinit[data] for initialization functions and data
> =A0i2c-mpc: rename "setclock" initialization functions to "setup"
> =A0i2c-mpc: add support for the MPC512x processors from Freescale
> =A0powerpc: doc/dts-bindings: update doc of FSL I2C bindings
>
> =A0Documentation/powerpc/dts-bindings/fsl/i2c.txt | =A0 30 +++-
> =A0drivers/i2c/busses/Kconfig =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =
=A0 =A07 +-
> =A0drivers/i2c/busses/i2c-mpc.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0=
194 +++++++++++++++---------
> =A03 files changed, 146 insertions(+), 85 deletions(-)
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH] leds-gpio: Fix default state handling on OF platforms
From: Grant Likely @ 2010-02-15 19:39 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: linuxppc-dev, Andrew Morton, Trent Piepho, linux-kernel,
	Richard Purdie
In-Reply-To: <20100205205437.GA4733@oksana.dev.rtsoft.ru>

On Fri, Feb 5, 2010 at 1:54 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> The driver wrongly sets default state for LEDs that don't specify
> default-state property.
>
> Currently the driver handles default state this way:
>
> memset(&led, 0, sizeof(led));
> for_each_child_of_node(np, child) {
> =A0 =A0 =A0 =A0state =3D of_get_property(child, "default-state", NULL);
> =A0 =A0 =A0 =A0if (state) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!strcmp(state, "keep"))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0led.default_state =3D LEDS=
_GPIO_DEFSTATE_KEEP;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0...
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0ret =3D create_gpio_led(&led, ...);
> }
>
> Which means that all LEDs that do not specify default-state will
> inherit the last value of the default-state property, which is wrong.
>
> This patch fixes the issue by moving LED's template initialization
> into the loop body.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

Richard, I assume you'll pick this one up via your tree?

g.

> ---
> =A0drivers/leds/leds-gpio.c | =A0 =A03 +--
> =A01 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index e5225d2..0823e26 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -211,7 +211,6 @@ static int __devinit of_gpio_leds_probe(struct of_dev=
ice *ofdev,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0const struct of_device_id *match)
> =A0{
> =A0 =A0 =A0 =A0struct device_node *np =3D ofdev->node, *child;
> - =A0 =A0 =A0 struct gpio_led led;
> =A0 =A0 =A0 =A0struct gpio_led_of_platform_data *pdata;
> =A0 =A0 =A0 =A0int count =3D 0, ret;
>
> @@ -226,8 +225,8 @@ static int __devinit of_gpio_leds_probe(struct of_dev=
ice *ofdev,
> =A0 =A0 =A0 =A0if (!pdata)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENOMEM;
>
> - =A0 =A0 =A0 memset(&led, 0, sizeof(led));
> =A0 =A0 =A0 =A0for_each_child_of_node(np, child) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct gpio_led led =3D {};
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0enum of_gpio_flags flags;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char *state;
>
> --
> 1.6.5.7
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" i=
n
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at =A0http://www.tux.org/lkml/
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 3/3] of/gpio: Introduce of_put_gpio(), add ref counting for OF GPIO chips
From: Grant Likely @ 2010-02-15 19:49 UTC (permalink / raw)
  To: avorontsov
  Cc: Michal Simek, David Brownell, devicetree-discuss, linux-kernel,
	linuxppc-dev, microblaze-uclinux, David Miller
In-Reply-To: <20100209191427.GA18263@oksana.dev.rtsoft.ru>

On Tue, Feb 9, 2010 at 12:14 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Tue, Feb 09, 2010 at 10:28:15AM -0700, Grant Likely wrote:
> [...]
>> Rather than having a lock at the device tree data pointer level which
>> mixes usage with potentially many other drivers; wouldn't it make more
>> sense to use a mutex at the of_gc subsystem context?
>
> I don't think so.
>
> of_gc = np->data;
> lock(of_gc); (or lock(devtree))
> <do something with of_gc>
>
> doesn't provide us what we need, i.e. it doesn't guarantee that
> np->data (of_gc) is still alive.
>
> And here:
>
> lock(np->data); (or lock(devtree))
> of_gc = np->data;
> lock(of_gc);
> <do something with of_gc>
>
> The second lock becomes useless (unless you also refcount np->data
> usage and can drop the devtree/np->data lock, and grab some other
> kind of lock, e.g. mutex, but this is silly).

Okay, I'm convinced now.  The model is wrong.  struct of_gc does need
to be a member of struct gpio_chip and conditionally compiled against
CONFIG_OF_GPIO.  This locking requirement is just too plain ugly, and
dereferencing the np->data pointer in this way is dangerous (what if
something that is not struct of_gc is stored there).

Put of_gc into struct gpio_chip, and I'll completely support that approach.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply


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