LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: PMU_IOC_SLEEP failed
From: jjDaNiMoTh @ 2010-12-28 23:35 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1293569648.17779.20.camel@pasglop>

2010/12/28 Benjamin Herrenschmidt <benh@kernel.crashing.org>:
> On Tue, 2010-12-28 at 17:39 +0100, jjDaNiMoTh wrote:
>> 2010/12/28 Michel D=C3=A4nzer <michel@daenzer.net>:
>> [cut]
>> > What exactly does "can't switch over VT" mean?
>> After X is running, I can't switch over ctrl + alt + f{1,2,3,4,5,6}
>> consoles ( the screen flicker, then blank ). If I switch to f7, where
>> X live, all return to normal and I can see normal X window.
>> This doesn't happend with X active.
>>
>> > Sleep requires radeonfb, is that enabled?
>>
>> Of course, but I think there is something wrong:
>>
>> (dmesg..)
>> [...]
>> radeonfb_pci_register BEGIN
>> radeonfb 0000:00:10.0: BAR 0: can't reserve [mem 0xb8000000-0xbfffffff p=
ref]
>> radeonfb (0000:00:10.0): cannot request region 0.
>> radeonfb: probe of 0000:00:10.0 failed with error -16
>
> That looks bad indeed. Send me the complete dmesg, a snapshot of the
> device-tree (tar -c /proc/device-tree >foo.tar) and the output
> of /proc/iomem please.

[1] http://nat.vacau.com/linux/iomem.txt.gz
[2] http://nat.vacau.com/linux/dmesg_37-rc.txt.gz
[3] http://nat.vacau.com/linux/device_tree.tar

Thank you

^ permalink raw reply

* problem backporting talitos for MPC8272
From: Alexandru Ionut Grama @ 2010-12-29  1:10 UTC (permalink / raw)
  To: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]

Hello,

I'm trying to backporting cryptoApi from 2.6.35.7 to 2.6.21 for making
backward compatibility with the crypto-processor of powerpc MPC8272. This
uses the SEC1.0 from Freescale for crypto. This processor it's mounted on a
custom board with a non-standard closed bootloader, but it can pass
parameters to the kernel.

I've made already the backport for the cryptoApi and talitos and the
compiles and runs. Consulting /proc/crypto I can see normal results, but no
drivers involving talitos. This it's normal because talitos_init() runs, but
not talitos_probe(). ¿Why? ¿How can I force a driver to be probed? I can see
that talitos it's registered on "of drivers tree", but nobody calls the
function talitos_probe(). I've making print this activating the print of
changing kobject on Kernel Hacking.

My final question it's about of-tree. I have the mpc8272ads.dts file with a
tree that involves talitos, but I've changed the properties acording the new
version of it on 2.6.35. ¿This file (mpc8272ads.dts) when it's used?I think
I don't use this file for nothing (it doesn't appears on make V=1) because I
don't have a normal bootloader and I have to load the kernel like a
flat-binary gz, so there's no more step after building System.map.

In the ocf-linux talitos driver specifications says that the only difference
between SEC1.0 and SEC2.0 it's the clock velocity. It has the same number of
channels and so on... ¿so talitos can be compatible with SEC1.0?

Also, ¿where I can submit the work already done (backporting of cryptoApi) ?
Maybe it's useful for someone.

I appreciate your help because this work it's a part for my finial bachelor
work.Thank you.

King regads,
Alexandru.

-- 
*---------------------------------------------------------------
Alexandru Ionut Grama
**email: gramaalexandruionut@gmail.com**
*

[-- Attachment #2: Type: text/html, Size: 2125 bytes --]

^ permalink raw reply

* [PATCH] KVM: PPC: Fix SPRG get/set for Book3S and BookE
From: Peter Tyser @ 2010-12-29 19:51 UTC (permalink / raw)
  To: kvm-ppc; +Cc: Peter Tyser, linuxppc-dev, agraf, kvm

Previously SPRGs 4-7 were improperly read and written in
kvm_arch_vcpu_ioctl_get_regs() and kvm_arch_vcpu_ioctl_set_regs();

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
I noticed this while grepping for somthing unrelated and assume its
a typo.  Feel free to add to the patch description; I don't use KVM
so don't know what the high-level consequences of this change are.

 arch/powerpc/kvm/book3s.c |   14 ++++++++------
 arch/powerpc/kvm/booke.c  |   14 ++++++++------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index e316847..5eb86c9 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1141,9 +1141,10 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 	regs->sprg1 = vcpu->arch.shared->sprg1;
 	regs->sprg2 = vcpu->arch.shared->sprg2;
 	regs->sprg3 = vcpu->arch.shared->sprg3;
-	regs->sprg5 = vcpu->arch.sprg4;
-	regs->sprg6 = vcpu->arch.sprg5;
-	regs->sprg7 = vcpu->arch.sprg6;
+	regs->sprg4 = vcpu->arch.sprg4;
+	regs->sprg5 = vcpu->arch.sprg5;
+	regs->sprg6 = vcpu->arch.sprg6;
+	regs->sprg7 = vcpu->arch.sprg7;
 
 	for (i = 0; i < ARRAY_SIZE(regs->gpr); i++)
 		regs->gpr[i] = kvmppc_get_gpr(vcpu, i);
@@ -1167,9 +1168,10 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 	vcpu->arch.shared->sprg1 = regs->sprg1;
 	vcpu->arch.shared->sprg2 = regs->sprg2;
 	vcpu->arch.shared->sprg3 = regs->sprg3;
-	vcpu->arch.sprg5 = regs->sprg4;
-	vcpu->arch.sprg6 = regs->sprg5;
-	vcpu->arch.sprg7 = regs->sprg6;
+	vcpu->arch.sprg4 = regs->sprg4;
+	vcpu->arch.sprg5 = regs->sprg5;
+	vcpu->arch.sprg6 = regs->sprg6;
+	vcpu->arch.sprg7 = regs->sprg7;
 
 	for (i = 0; i < ARRAY_SIZE(regs->gpr); i++)
 		kvmppc_set_gpr(vcpu, i, regs->gpr[i]);
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 77575d0..ef76acb 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -546,9 +546,10 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 	regs->sprg1 = vcpu->arch.shared->sprg1;
 	regs->sprg2 = vcpu->arch.shared->sprg2;
 	regs->sprg3 = vcpu->arch.shared->sprg3;
-	regs->sprg5 = vcpu->arch.sprg4;
-	regs->sprg6 = vcpu->arch.sprg5;
-	regs->sprg7 = vcpu->arch.sprg6;
+	regs->sprg4 = vcpu->arch.sprg4;
+	regs->sprg5 = vcpu->arch.sprg5;
+	regs->sprg6 = vcpu->arch.sprg6;
+	regs->sprg7 = vcpu->arch.sprg7;
 
 	for (i = 0; i < ARRAY_SIZE(regs->gpr); i++)
 		regs->gpr[i] = kvmppc_get_gpr(vcpu, i);
@@ -572,9 +573,10 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 	vcpu->arch.shared->sprg1 = regs->sprg1;
 	vcpu->arch.shared->sprg2 = regs->sprg2;
 	vcpu->arch.shared->sprg3 = regs->sprg3;
-	vcpu->arch.sprg5 = regs->sprg4;
-	vcpu->arch.sprg6 = regs->sprg5;
-	vcpu->arch.sprg7 = regs->sprg6;
+	vcpu->arch.sprg4 = regs->sprg4;
+	vcpu->arch.sprg5 = regs->sprg5;
+	vcpu->arch.sprg6 = regs->sprg6;
+	vcpu->arch.sprg7 = regs->sprg7;
 
 	for (i = 0; i < ARRAY_SIZE(regs->gpr); i++)
 		kvmppc_set_gpr(vcpu, i, regs->gpr[i]);
-- 
1.7.0.4

^ permalink raw reply related

* PowerPC BUG: using smp_processor_id() in preemptible code
From: Hugh Dickins @ 2010-12-29 22:54 UTC (permalink / raw)
  To: Ben Herrenschmidt
  Cc: Jeremy Fitzhardinge, linuxppc-dev, Nick Piggin, linux-kernel

With recent 2.6.37-rc, with CONFIG_PREEMPT=y CONFIG_DEBUG_PREEMPT=y
on the PowerPC G5, I get spammed by BUG warnings each time I swapoff:

BUG: using smp_processor_id() in preemptible [00000000] code: swapoff/3974
caller is .hpte_need_flush+0x4c/0x2e8
Call Trace:
[c0000001b4a3f830] [c00000000000f3cc] .show_stack+0x6c/0x16c (unreliable)
[c0000001b4a3f8e0] [c00000000023eda0] .debug_smp_processor_id+0xe4/0x11c
[c0000001b4a3f970] [c00000000002f2f4] .hpte_need_flush+0x4c/0x2e8
[c0000001b4a3fa30] [c0000000000e7ef8] .vunmap_pud_range+0x148/0x200
[c0000001b4a3fb10] [c0000000000e8058] .vunmap_page_range+0xa8/0xd4
[c0000001b4a3fbb0] [c0000000000e80a4] .free_unmap_vmap_area+0x20/0x38
[c0000001b4a3fc40] [c0000000000e8138] .remove_vm_area+0x7c/0xb4
[c0000001b4a3fcd0] [c0000000000e8308] .__vunmap+0x50/0x104
[c0000001b4a3fd60] [c0000000000ef3fc] .SyS_swapoff+0x59c/0x6a8
[c0000001b4a3fe30] [c0000000000075a8] syscall_exit+0x0/0x40

I notice hpte_need_flush() itself acknowledges
 * Must be called from within some kind of spinlock/non-preempt region...

Though I didn't actually bisect, I believe this is since Jeremy's
64141da587241301ce8638cc945f8b67853156ec "vmalloc: eagerly clear ptes
on vunmap", which moves a call to vunmap_page_range() from one place
(which happened to be inside a spinlock) to another (where it's not).

I guess my warnings would be easily silenced by moving that call to
vunmap_page_range() down just inside the spinlock below it; but I'm
dubious that that's the right fix - it looked as if there are other
paths through vmalloc.c where vunmap_page_range() has been getting
called without preemption disabled, long before Jeremy's change,
just paths that I never happen to go down in my limited testing.

For the moment I'm using the obvious patch below to keep it quiet;
but I doubt that this is the right patch either.  I'm hoping that
ye who understand the importance of hpte_need_flush() will be best
able to judge what to do.  Or might there be other architectures
expecting to be unpreemptible there?

Thanks,
Hugh

--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -37,11 +37,13 @@ static void vunmap_pte_range(pmd_t *pmd,
 {
 	pte_t *pte;
 
+	preempt_disable(); /* Stop __vunmap() triggering smp_processor_id() in preemptible from hpte_need_flush() */
 	pte = pte_offset_kernel(pmd, addr);
 	do {
 		pte_t ptent = ptep_get_and_clear(&init_mm, addr, pte);
 		WARN_ON(!pte_none(ptent) && !pte_present(ptent));
 	} while (pte++, addr += PAGE_SIZE, addr != end);
+	preempt_enable();
 }
 
 static void vunmap_pmd_range(pud_t *pud, unsigned long addr, unsigned long end)

^ permalink raw reply

* Re: [PATCH 1/7] fdt: Add Kconfig for EARLY_FLATTREE
From: Grant Likely @ 2010-12-30  0:15 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <d43cf8d1-8821-4400-9c58-692846dd37ac@SG2EHSMHS005.ehs.local>

On Thu, Nov 18, 2010 at 03:54:56PM -0800, Stephen Neuendorffer wrote:
> The device tree code is now in two pieces: some which can be used generically
> on any platform which selects CONFIG_OF_FLATTREE, and some early which is used
> at boot time on only a few architectures.  This patch segregates the early
> code so that only those architectures which care about it need compile it.
> This also means that some of the requirements in the early code (such as
> a cmd_line variable) that most architectures (e.g. X86) don't provide
> can be ignored.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Applied, thanks.  I had to fix up the #ifdef which was testing for
CONFIG_EARLY_FLATTREE instead of CONFIG_OF_EARLY_FLATTREE, and I
reworked OF_EARLY_FLATTREE to select instead of depend on OF_FLATTREE.

Cheers,
g.

> 
> ----
> 
> Grant,
> We had discussed doing something like this a loooong time ago.  This (or at
> least some other way of dealing with cmd_line) is
> actually necessary to compile on X86.  Do you still want to do it this way?
> Looking at my old email, you suggested only powerpc and microblaze need this.
> Is that still the case?
> 
> Conflicts:
> 
> 	drivers/of/fdt.c
> ---
>  arch/microblaze/Kconfig |    1 +
>  arch/powerpc/Kconfig    |    1 +
>  drivers/of/Kconfig      |    5 +++++
>  drivers/of/fdt.c        |    4 ++++
>  4 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> index 692fdfc..384375c 100644
> --- a/arch/microblaze/Kconfig
> +++ b/arch/microblaze/Kconfig
> @@ -20,6 +20,7 @@ config MICROBLAZE
>  	select TRACING_SUPPORT
>  	select OF
>  	select OF_FLATTREE
> +	select OF_EARLY_FLATTREE
>  
>  config SWAP
>  	def_bool n
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 631e5a0..2cd7b32 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -119,6 +119,7 @@ config PPC
>  	default y
>  	select OF
>  	select OF_FLATTREE
> +	select OF_EARLY_FLATTREE
>  	select HAVE_FTRACE_MCOUNT_RECORD
>  	select HAVE_DYNAMIC_FTRACE
>  	select HAVE_FUNCTION_TRACER
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index aa675eb..0199157 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -19,6 +19,10 @@ config OF_FLATTREE
>  	bool
>  	select DTC
>  
> +config OF_EARLY_FLATTREE
> +	bool
> +	depends on OF_FLATTREE
> +
>  config OF_PROMTREE
>  	bool
>  
> @@ -62,3 +66,4 @@ config OF_MDIO
>  	  OpenFirmware MDIO bus (Ethernet PHY) accessors
>  
>  endmenu # OF
> +
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c1360e0..4d71b29 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -27,6 +27,8 @@ int __initdata dt_root_size_cells;
>  
>  struct boot_param_header *initial_boot_params;
>  
> +#ifdef CONFIG_EARLY_FLATTREE
> +
>  char *find_flat_dt_string(u32 offset)
>  {
>  	return ((char *)initial_boot_params) +
> @@ -604,3 +606,5 @@ void __init unflatten_device_tree(void)
>  
>  	pr_debug(" <- unflatten_device_tree()\n");
>  }
> +
> +#endif /* CONFIG_EARLY_FLATTREE */
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: [PATCH 2/7] arch/x86: Add support for device tree code.
From: Grant Likely @ 2010-12-30  0:17 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <98608b2e-d138-4dc0-808d-3662ab2c9938@VA3EHSMHS025.ehs.local>

On Thu, Nov 18, 2010 at 03:54:57PM -0800, Stephen Neuendorffer wrote:
> A few support device-tree related support functions that x86 didn't
> have before.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Skipping this patch.  I expect to pick up Dirk's version instead.

g.

> 
> ----
> 
> Looks like just some irq related junk left!
> ---
>  arch/x86/include/asm/irq.h |    2 ++
>  arch/x86/kernel/irq.c      |   11 +++++++++++
>  2 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/include/asm/irq.h b/arch/x86/include/asm/irq.h
> index 5458380..af4e630 100644
> --- a/arch/x86/include/asm/irq.h
> +++ b/arch/x86/include/asm/irq.h
> @@ -10,6 +10,8 @@
>  #include <asm/apicdef.h>
>  #include <asm/irq_vectors.h>
>  
> +#define irq_dispose_mapping(...)
> +
>  static inline int irq_canonicalize(int irq)
>  {
>  	return ((irq == 2) ? 9 : irq);
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index 91fd0c7..a3aaed4 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -364,3 +364,14 @@ void fixup_irqs(void)
>  	}
>  }
>  #endif
> +
> +#ifdef CONFIG_OF
> +#include <linux/of_irq.h>
> +unsigned int irq_create_of_mapping(struct device_node *controller,
> +				   const u32 *intspec, unsigned int intsize)
> +{
> +	return intspec[0] + 1;
> +}
> +EXPORT_SYMBOL_GPL(irq_create_of_mapping);
> +
> +#endif
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: [PATCH 3/7] arch/x86: select OF and OF_FLATTREE
From: Grant Likely @ 2010-12-30  0:17 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <111913e5-7481-46b8-b04d-eee3cdc8fc32@SG2EHSMHS002.ehs.local>

On Thu, Nov 18, 2010 at 03:54:58PM -0800, Stephen Neuendorffer wrote:
> Testing patch to verify that the device tree code can be compiled on X86.
> ---

Skipping this one also.

>  arch/x86/Kconfig |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cea0cd9..0f2ed5b 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -59,6 +59,8 @@ config X86
>  	select ANON_INODES
>  	select HAVE_ARCH_KMEMCHECK
>  	select HAVE_USER_RETURN_NOTIFIER
> +	select OF
> +	select OF_FLATTREE
>  
>  config INSTRUCTION_DECODER
>  	def_bool (KPROBES || PERF_EVENTS)
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: [PATCH 4/7] fdt.c: Add non-boottime device tree functions
From: Grant Likely @ 2010-12-30  0:34 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <d433b5eb-2d15-4a28-8083-97582bf4617f@VA3EHSMHS022.ehs.local>

On Thu, Nov 18, 2010 at 03:54:59PM -0800, Stephen Neuendorffer wrote:
> In preparation for providing run-time handling of device trees, factor
> out some of the basic functions so that they take an arbitrary blob,
> rather than relying on the single boot-time tree.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Thanks.  Merged, but one comment below.  Please fix with a followup patch.

g.

> 
> --
> 
> V2: functions have of_fdt_* names
>     removed find_flat_dt_string
>     blob argument is first
> ---
>  drivers/of/fdt.c       |  133 ++++++++++++++++++++++++++++-------------------
>  include/linux/of_fdt.h |   11 ++++
>  2 files changed, 90 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 4d71b29..190e26c 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -22,6 +22,82 @@
>  
>  #include <asm/page.h>
>  
> +char *of_fdt_get_string(struct boot_param_header *blob, u32 offset)
> +{
> +	return ((char *)blob) +
> +		be32_to_cpu(blob->off_dt_strings) + offset;
> +}
> +
> +/**
> + * of_fdt_get_property - Given a node in the given flat blob, return
> + * the property ptr
> + */
> +void *of_fdt_get_property(struct boot_param_header *blob,
> +		       unsigned long node, const char *name,
> +		       unsigned long *size)
> +{
> +	unsigned long p = node;
> +
> +	do {
> +		u32 tag = be32_to_cpup((__be32 *)p);
> +		u32 sz, noff;
> +		const char *nstr;
> +
> +		p += 4;
> +		if (tag == OF_DT_NOP)
> +			continue;
> +		if (tag != OF_DT_PROP)
> +			return NULL;
> +
> +		sz = be32_to_cpup((__be32 *)p);
> +		noff = be32_to_cpup((__be32 *)(p + 4));
> +		p += 8;
> +		if (be32_to_cpu(blob->version) < 0x10)
> +			p = ALIGN(p, sz >= 8 ? 8 : 4);
> +
> +		nstr = of_fdt_get_string(blob, noff);
> +		if (nstr == NULL) {
> +			pr_warning("Can't find property index name !\n");
> +			return NULL;
> +		}
> +		if (strcmp(name, nstr) == 0) {
> +			if (size)
> +				*size = sz;
> +			return (void *)p;
> +		}
> +		p += sz;
> +		p = ALIGN(p, 4);
> +	} while (1);
> +}
> +
> +/**
> + * of_fdt_is_compatible - Return true if given node from the given blob has
> + * compat in its compatible list
> + * @blob: A device tree blob
> + * @node: node to test
> + * @compat: compatible string to compare with compatible list.
> + */
> +int of_fdt_is_compatible(struct boot_param_header *blob,
> +		      unsigned long node, const char *compat)
> +{
> +	const char *cp;
> +	unsigned long cplen, l;
> +
> +	cp = of_fdt_get_property(blob, node, "compatible", &cplen);
> +	if (cp == NULL)
> +		return 0;
> +	while (cplen > 0) {
> +		if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> +			return 1;

Need to verify that strlen(cp) + 1 is not larger than cplen.
Otherwise cplen could wrap around and the loop won't terminate because
cplen is an unsigned long.

> +		l = strlen(cp) + 1;
> +		cp += l;
> +		cplen -= l;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Everything below here references initial_boot_params directly. */
>  int __initdata dt_root_addr_cells;
>  int __initdata dt_root_size_cells;
>  
> @@ -29,12 +105,6 @@ struct boot_param_header *initial_boot_params;
>  
>  #ifdef CONFIG_EARLY_FLATTREE
>  
> -char *find_flat_dt_string(u32 offset)
> -{
> -	return ((char *)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_strings) + offset;
> -}
> -
>  /**
>   * of_scan_flat_dt - scan flattened tree blob and call callback on each.
>   * @it: callback function
> @@ -123,38 +193,7 @@ unsigned long __init of_get_flat_dt_root(void)
>  void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
>  				 unsigned long *size)
>  {
> -	unsigned long p = node;
> -
> -	do {
> -		u32 tag = be32_to_cpup((__be32 *)p);
> -		u32 sz, noff;
> -		const char *nstr;
> -
> -		p += 4;
> -		if (tag == OF_DT_NOP)
> -			continue;
> -		if (tag != OF_DT_PROP)
> -			return NULL;
> -
> -		sz = be32_to_cpup((__be32 *)p);
> -		noff = be32_to_cpup((__be32 *)(p + 4));
> -		p += 8;
> -		if (be32_to_cpu(initial_boot_params->version) < 0x10)
> -			p = ALIGN(p, sz >= 8 ? 8 : 4);
> -
> -		nstr = find_flat_dt_string(noff);
> -		if (nstr == NULL) {
> -			pr_warning("Can't find property index name !\n");
> -			return NULL;
> -		}
> -		if (strcmp(name, nstr) == 0) {
> -			if (size)
> -				*size = sz;
> -			return (void *)p;
> -		}
> -		p += sz;
> -		p = ALIGN(p, 4);
> -	} while (1);
> +	return of_fdt_get_property(initial_boot_params, node, name, size);
>  }
>  
>  /**
> @@ -164,21 +203,7 @@ void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
>   */
>  int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
>  {
> -	const char *cp;
> -	unsigned long cplen, l;
> -
> -	cp = of_get_flat_dt_prop(node, "compatible", &cplen);
> -	if (cp == NULL)
> -		return 0;
> -	while (cplen > 0) {
> -		if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> -			return 1;
> -		l = strlen(cp) + 1;
> -		cp += l;
> -		cplen -= l;
> -	}
> -
> -	return 0;
> +	return of_fdt_is_compatible(initial_boot_params, node, compat);
>  }
>  
>  static void *__init unflatten_dt_alloc(unsigned long *mem, unsigned long size,
> @@ -303,7 +328,7 @@ unsigned long __init unflatten_dt_node(unsigned long mem,
>  		if (be32_to_cpu(initial_boot_params->version) < 0x10)
>  			*p = ALIGN(*p, sz >= 8 ? 8 : 4);
>  
> -		pname = find_flat_dt_string(noff);
> +		pname = of_fdt_get_string(initial_boot_params, noff);
>  		if (pname == NULL) {
>  			pr_info("Can't find property name in list !\n");
>  			break;
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 7bbf5b3..70c5b73 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -58,6 +58,17 @@ struct boot_param_header {
>  };
>  
>  #if defined(CONFIG_OF_FLATTREE)
> +
> +/* For scanning an arbitrary device-tree at any time */
> +extern char *of_fdt_get_string(struct boot_param_header *blob, u32 offset);
> +extern void *of_fdt_get_property(struct boot_param_header *blob,
> +				 unsigned long node,
> +				 const char *name,
> +				 unsigned long *size);
> +extern int of_fdt_is_compatible(struct boot_param_header *blob,
> +				unsigned long node,
> +				const char *compat);
> +
>  /* TBD: Temporary export of fdt globals - remove when code fully merged */
>  extern int __initdata dt_root_addr_cells;
>  extern int __initdata dt_root_size_cells;
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: [PATCH 5/7] fdt.c: Refactor unflatten_dt_node
From: Grant Likely @ 2010-12-30  0:35 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <e5face75-4978-4686-9d75-d92fa9ec7328@VA3EHSMHS013.ehs.local>

On Thu, Nov 18, 2010 at 03:55:00PM -0800, Stephen Neuendorffer wrote:
> unflatten_dt_node is a helper function that does most of the work to
> convert a device tree blob into tree of device nodes.  This code
> now uses a passed-in blob instead of using the single boot-time blob,
> allowing it to be called in more contexts.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Merged, thanks.

g.

> 
> ---
> 
> V2: don't reorder
>     blob argument first.
> ---
>  drivers/of/fdt.c |   27 ++++++++++++++++-----------
>  1 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 190e26c..a07fd1a 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -206,7 +206,7 @@ int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
>  	return of_fdt_is_compatible(initial_boot_params, node, compat);
>  }
>  
> -static void *__init unflatten_dt_alloc(unsigned long *mem, unsigned long size,
> +static void *unflatten_dt_alloc(unsigned long *mem, unsigned long size,
>  				       unsigned long align)
>  {
>  	void *res;
> @@ -220,16 +220,18 @@ static void *__init unflatten_dt_alloc(unsigned long *mem, unsigned long size,
>  
>  /**
>   * unflatten_dt_node - Alloc and populate a device_node from the flat tree
> + * @blob: The parent device tree blob
>   * @p: pointer to node in flat tree
>   * @dad: Parent struct device_node
>   * @allnextpp: pointer to ->allnext from last allocated device_node
>   * @fpsize: Size of the node path up at the current depth.
>   */
> -unsigned long __init unflatten_dt_node(unsigned long mem,
> -					unsigned long *p,
> -					struct device_node *dad,
> -					struct device_node ***allnextpp,
> -					unsigned long fpsize)
> +unsigned long unflatten_dt_node(struct boot_param_header *blob,
> +				unsigned long mem,
> +				unsigned long *p,
> +				struct device_node *dad,
> +				struct device_node ***allnextpp,
> +				unsigned long fpsize)
>  {
>  	struct device_node *np;
>  	struct property *pp, **prev_pp = NULL;
> @@ -325,10 +327,10 @@ unsigned long __init unflatten_dt_node(unsigned long mem,
>  		sz = be32_to_cpup((__be32 *)(*p));
>  		noff = be32_to_cpup((__be32 *)((*p) + 4));
>  		*p += 8;
> -		if (be32_to_cpu(initial_boot_params->version) < 0x10)
> +		if (be32_to_cpu(blob->version) < 0x10)
>  			*p = ALIGN(*p, sz >= 8 ? 8 : 4);
>  
> -		pname = of_fdt_get_string(initial_boot_params, noff);
> +		pname = of_fdt_get_string(blob, noff);
>  		if (pname == NULL) {
>  			pr_info("Can't find property name in list !\n");
>  			break;
> @@ -407,7 +409,8 @@ unsigned long __init unflatten_dt_node(unsigned long mem,
>  		if (tag == OF_DT_NOP)
>  			*p += 4;
>  		else
> -			mem = unflatten_dt_node(mem, p, np, allnextpp, fpsize);
> +			mem = unflatten_dt_node(blob, mem, p, np, allnextpp,
> +						fpsize);
>  		tag = be32_to_cpup((__be32 *)(*p));
>  	}
>  	if (tag != OF_DT_END_NODE) {
> @@ -599,7 +602,8 @@ void __init unflatten_device_tree(void)
>  	/* First pass, scan for size */
>  	start = ((unsigned long)initial_boot_params) +
>  		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	size = unflatten_dt_node(0, &start, NULL, NULL, 0);
> +	size = unflatten_dt_node(initial_boot_params, 0, &start,
> +				 NULL, NULL, 0);
>  	size = (size | 3) + 1;
>  
>  	pr_debug("  size is %lx, allocating...\n", size);
> @@ -616,7 +620,8 @@ void __init unflatten_device_tree(void)
>  	/* Second pass, do actual unflattening */
>  	start = ((unsigned long)initial_boot_params) +
>  		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	unflatten_dt_node(mem, &start, NULL, &allnextp, 0);
> +	unflatten_dt_node(initial_boot_params, mem, &start,
> +			  NULL, &allnextp, 0);
>  	if (be32_to_cpup((__be32 *)start) != OF_DT_END)
>  		pr_warning("Weird tag at end of tree: %08x\n", *((u32 *)start));
>  	if (be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef)
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: [PATCH 6/7] fdt.c: Reorder unflatten_dt_node
From: Grant Likely @ 2010-12-30  0:36 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <972516c0-f170-469f-937f-bfa6be8dd04a@SG2EHSMHS004.ehs.local>

On Thu, Nov 18, 2010 at 03:55:01PM -0800, Stephen Neuendorffer wrote:
> Move unflatten_dt_node to be grouped with non-__init functions.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Merged, thanks.

g.

> ---
>  drivers/of/fdt.c |  218 +++++++++++++++++++++++++++---------------------------
>  1 files changed, 109 insertions(+), 109 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index a07fd1a..236a8c9 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -97,115 +97,6 @@ int of_fdt_is_compatible(struct boot_param_header *blob,
>  	return 0;
>  }
>  
> -/* Everything below here references initial_boot_params directly. */
> -int __initdata dt_root_addr_cells;
> -int __initdata dt_root_size_cells;
> -
> -struct boot_param_header *initial_boot_params;
> -
> -#ifdef CONFIG_EARLY_FLATTREE
> -
> -/**
> - * of_scan_flat_dt - scan flattened tree blob and call callback on each.
> - * @it: callback function
> - * @data: context data pointer
> - *
> - * This function is used to scan the flattened device-tree, it is
> - * used to extract the memory information at boot before we can
> - * unflatten the tree
> - */
> -int __init of_scan_flat_dt(int (*it)(unsigned long node,
> -				     const char *uname, int depth,
> -				     void *data),
> -			   void *data)
> -{
> -	unsigned long p = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	int rc = 0;
> -	int depth = -1;
> -
> -	do {
> -		u32 tag = be32_to_cpup((__be32 *)p);
> -		char *pathp;
> -
> -		p += 4;
> -		if (tag == OF_DT_END_NODE) {
> -			depth--;
> -			continue;
> -		}
> -		if (tag == OF_DT_NOP)
> -			continue;
> -		if (tag == OF_DT_END)
> -			break;
> -		if (tag == OF_DT_PROP) {
> -			u32 sz = be32_to_cpup((__be32 *)p);
> -			p += 8;
> -			if (be32_to_cpu(initial_boot_params->version) < 0x10)
> -				p = ALIGN(p, sz >= 8 ? 8 : 4);
> -			p += sz;
> -			p = ALIGN(p, 4);
> -			continue;
> -		}
> -		if (tag != OF_DT_BEGIN_NODE) {
> -			pr_err("Invalid tag %x in flat device tree!\n", tag);
> -			return -EINVAL;
> -		}
> -		depth++;
> -		pathp = (char *)p;
> -		p = ALIGN(p + strlen(pathp) + 1, 4);
> -		if ((*pathp) == '/') {
> -			char *lp, *np;
> -			for (lp = NULL, np = pathp; *np; np++)
> -				if ((*np) == '/')
> -					lp = np+1;
> -			if (lp != NULL)
> -				pathp = lp;
> -		}
> -		rc = it(p, pathp, depth, data);
> -		if (rc != 0)
> -			break;
> -	} while (1);
> -
> -	return rc;
> -}
> -
> -/**
> - * of_get_flat_dt_root - find the root node in the flat blob
> - */
> -unsigned long __init of_get_flat_dt_root(void)
> -{
> -	unsigned long p = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -
> -	while (be32_to_cpup((__be32 *)p) == OF_DT_NOP)
> -		p += 4;
> -	BUG_ON(be32_to_cpup((__be32 *)p) != OF_DT_BEGIN_NODE);
> -	p += 4;
> -	return ALIGN(p + strlen((char *)p) + 1, 4);
> -}
> -
> -/**
> - * of_get_flat_dt_prop - Given a node in the flat blob, return the property ptr
> - *
> - * This function can be used within scan_flattened_dt callback to get
> - * access to properties
> - */
> -void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
> -				 unsigned long *size)
> -{
> -	return of_fdt_get_property(initial_boot_params, node, name, size);
> -}
> -
> -/**
> - * of_flat_dt_is_compatible - Return true if given node has compat in compatible list
> - * @node: node to test
> - * @compat: compatible string to compare with compatible list.
> - */
> -int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
> -{
> -	return of_fdt_is_compatible(initial_boot_params, node, compat);
> -}
> -
>  static void *unflatten_dt_alloc(unsigned long *mem, unsigned long size,
>  				       unsigned long align)
>  {
> @@ -421,6 +312,115 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob,
>  	return mem;
>  }
>  
> +/* Everything below here references initial_boot_params directly. */
> +int __initdata dt_root_addr_cells;
> +int __initdata dt_root_size_cells;
> +
> +struct boot_param_header *initial_boot_params;
> +
> +#ifdef CONFIG_EARLY_FLATTREE
> +
> +/**
> + * of_scan_flat_dt - scan flattened tree blob and call callback on each.
> + * @it: callback function
> + * @data: context data pointer
> + *
> + * This function is used to scan the flattened device-tree, it is
> + * used to extract the memory information at boot before we can
> + * unflatten the tree
> + */
> +int __init of_scan_flat_dt(int (*it)(unsigned long node,
> +				     const char *uname, int depth,
> +				     void *data),
> +			   void *data)
> +{
> +	unsigned long p = ((unsigned long)initial_boot_params) +
> +		be32_to_cpu(initial_boot_params->off_dt_struct);
> +	int rc = 0;
> +	int depth = -1;
> +
> +	do {
> +		u32 tag = be32_to_cpup((__be32 *)p);
> +		char *pathp;
> +
> +		p += 4;
> +		if (tag == OF_DT_END_NODE) {
> +			depth--;
> +			continue;
> +		}
> +		if (tag == OF_DT_NOP)
> +			continue;
> +		if (tag == OF_DT_END)
> +			break;
> +		if (tag == OF_DT_PROP) {
> +			u32 sz = be32_to_cpup((__be32 *)p);
> +			p += 8;
> +			if (be32_to_cpu(initial_boot_params->version) < 0x10)
> +				p = ALIGN(p, sz >= 8 ? 8 : 4);
> +			p += sz;
> +			p = ALIGN(p, 4);
> +			continue;
> +		}
> +		if (tag != OF_DT_BEGIN_NODE) {
> +			pr_err("Invalid tag %x in flat device tree!\n", tag);
> +			return -EINVAL;
> +		}
> +		depth++;
> +		pathp = (char *)p;
> +		p = ALIGN(p + strlen(pathp) + 1, 4);
> +		if ((*pathp) == '/') {
> +			char *lp, *np;
> +			for (lp = NULL, np = pathp; *np; np++)
> +				if ((*np) == '/')
> +					lp = np+1;
> +			if (lp != NULL)
> +				pathp = lp;
> +		}
> +		rc = it(p, pathp, depth, data);
> +		if (rc != 0)
> +			break;
> +	} while (1);
> +
> +	return rc;
> +}
> +
> +/**
> + * of_get_flat_dt_root - find the root node in the flat blob
> + */
> +unsigned long __init of_get_flat_dt_root(void)
> +{
> +	unsigned long p = ((unsigned long)initial_boot_params) +
> +		be32_to_cpu(initial_boot_params->off_dt_struct);
> +
> +	while (be32_to_cpup((__be32 *)p) == OF_DT_NOP)
> +		p += 4;
> +	BUG_ON(be32_to_cpup((__be32 *)p) != OF_DT_BEGIN_NODE);
> +	p += 4;
> +	return ALIGN(p + strlen((char *)p) + 1, 4);
> +}
> +
> +/**
> + * of_get_flat_dt_prop - Given a node in the flat blob, return the property ptr
> + *
> + * This function can be used within scan_flattened_dt callback to get
> + * access to properties
> + */
> +void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
> +				 unsigned long *size)
> +{
> +	return of_fdt_get_property(initial_boot_params, node, name, size);
> +}
> +
> +/**
> + * of_flat_dt_is_compatible - Return true if given node has compat in compatible list
> + * @node: node to test
> + * @compat: compatible string to compare with compatible list.
> + */
> +int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
> +{
> +	return of_fdt_is_compatible(initial_boot_params, node, compat);
> +}
> +
>  #ifdef CONFIG_BLK_DEV_INITRD
>  /**
>   * early_init_dt_check_for_initrd - Decode initrd location from flat tree
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: [PATCH 7/7] fdt.c: Refactor unflatten_device_tree and add fdt_unflatten_tree
From: Grant Likely @ 2010-12-30  0:43 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <8aead437-fb77-4e72-8b66-2b1626f6316d@SG2EHSMHS008.ehs.local>

On Thu, Nov 18, 2010 at 03:55:02PM -0800, Stephen Neuendorffer wrote:
> unflatten_device_tree has two dependencies on things that happen
> during boot time.  Firstly, it references the initial device tree
> directly. Secondly, it allocates memory using the early boot
> allocator.  This patch factors out these dependencies and uses
> the new __unflatten_device_tree function to implement a driver-visible
> fdt_unflatten_tree function, which can be used to unflatten a
> blob after boot time.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Merged, thanks.  One comment below, please fix with a followup patch.

g.

> 
> --
> 
> V2: remove extra __va() call
> 	 make dt_alloc functions return void *.  This doesn't fix the general
>     strangeness in this code that constantly casts back and forth between
>     unsigned long and __be32 *
> ---
>  drivers/of/fdt.c       |  149 +++++++++++++++++++++++++++++++----------------
>  include/linux/of_fdt.h |    2 +
>  2 files changed, 100 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 236a8c9..e29dbe2 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -11,10 +11,12 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/initrd.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
>  #include <linux/string.h>
>  #include <linux/errno.h>
> +#include <linux/slab.h>
>  
>  #ifdef CONFIG_PPC
>  #include <asm/machdep.h>
> @@ -312,6 +314,94 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob,
>  	return mem;
>  }
>  
> +/**
> + * __unflatten_device_tree - create tree of device_nodes from flat blob
> + *
> + * unflattens a device-tree, creating the
> + * tree of struct device_node. It also fills the "name" and "type"
> + * pointers of the nodes so the normal device-tree walking functions
> + * can be used.
> + * @blob: The blob to expand
> + * @mynodes: The device_node tree created by the call
> + * @dt_alloc: An allocator that provides a virtual address to memory
> + * for the resulting tree
> + */
> +void __unflatten_device_tree(struct boot_param_header *blob,
> +			     struct device_node **mynodes,
> +			     void * (*dt_alloc)(u64 size, u64 align))
> +{
> +	unsigned long start, mem, size;
> +	struct device_node **allnextp = mynodes;
> +
> +	pr_debug(" -> unflatten_device_tree()\n");
> +
> +	if (!blob) {
> +		pr_debug("No device tree pointer\n");
> +		return;
> +	}
> +
> +	pr_debug("Unflattening device tree:\n");
> +	pr_debug("magic: %08x\n", be32_to_cpu(blob->magic));
> +	pr_debug("size: %08x\n", be32_to_cpu(blob->totalsize));
> +	pr_debug("version: %08x\n", be32_to_cpu(blob->version));
> +
> +	if (be32_to_cpu(blob->magic) != OF_DT_HEADER) {
> +		pr_err("Invalid device tree blob header\n");
> +		return;
> +	}
> +
> +	/* First pass, scan for size */
> +	start = ((unsigned long)blob) +
> +		be32_to_cpu(blob->off_dt_struct);
> +	size = unflatten_dt_node(blob, 0, &start, NULL, NULL, 0);
> +	size = (size | 3) + 1;
> +
> +	pr_debug("  size is %lx, allocating...\n", size);
> +
> +	/* Allocate memory for the expanded device tree */
> +	mem = (unsigned long)
> +		dt_alloc(size + 4, __alignof__(struct device_node));
> +
> +	((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);
> +
> +	pr_debug("  unflattening %lx...\n", mem);
> +
> +	/* Second pass, do actual unflattening */
> +	start = ((unsigned long)blob) +
> +		be32_to_cpu(blob->off_dt_struct);
> +	unflatten_dt_node(blob, mem, &start, NULL, &allnextp, 0);
> +	if (be32_to_cpup((__be32 *)start) != OF_DT_END)
> +		pr_warning("Weird tag at end of tree: %08x\n", *((u32 *)start));
> +	if (be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef)
> +		pr_warning("End of tree marker overwritten: %08x\n",
> +			   be32_to_cpu(((__be32 *)mem)[size / 4]));
> +	*allnextp = NULL;
> +
> +	pr_debug(" <- unflatten_device_tree()\n");
> +}
> +
> +static void *kernel_tree_alloc(u64 size, u64 align)
> +{
> +	return kzalloc(size, GFP_KERNEL);
> +}
> +
> +/**
> + * of_fdt_unflatten_tree - create tree of device_nodes from flat blob
> + *
> + * unflattens the device-tree passed by the firmware, creating the
> + * tree of struct device_node. It also fills the "name" and "type"
> + * pointers of the nodes so the normal device-tree walking functions
> + * can be used.
> + */
> +void of_fdt_unflatten_tree(unsigned long *blob,
> +			struct device_node **mynodes)
> +{
> +	struct boot_param_header *device_tree =
> +		(struct boot_param_header *)blob;
> +	__unflatten_device_tree(device_tree, mynodes, &kernel_tree_alloc);
> +}
> +EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);

The blob parameter to of_fdt_unflatten_tree should either be void* or
boot_param_header*.  What's the reason for unsigned long *?  Also,
blob can be passed directly into the __unflatten_device_tree() call.
No reason for the device_tree = blob assignment.  In fact, you could
probably eliminate this function entirely and rename
__unflatten_device_tree to of_fdt_unflatten_tree.

> +
>  /* Everything below here references initial_boot_params directly. */
>  int __initdata dt_root_addr_cells;
>  int __initdata dt_root_size_cells;
> @@ -569,6 +659,12 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  	return 1;
>  }
>  
> +static void *__init early_device_tree_alloc(u64 size, u64 align)
> +{
> +	unsigned long mem = early_init_dt_alloc_memory_arch(size, align);
> +	return __va(mem);
> +}
> +
>  /**
>   * unflatten_device_tree - create tree of device_nodes from flat blob
>   *
> @@ -579,62 +675,13 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>   */
>  void __init unflatten_device_tree(void)
>  {
> -	unsigned long start, mem, size;
> -	struct device_node **allnextp = &allnodes;
> -
> -	pr_debug(" -> unflatten_device_tree()\n");
> -
> -	if (!initial_boot_params) {
> -		pr_debug("No device tree pointer\n");
> -		return;
> -	}
> -
> -	pr_debug("Unflattening device tree:\n");
> -	pr_debug("magic: %08x\n", be32_to_cpu(initial_boot_params->magic));
> -	pr_debug("size: %08x\n", be32_to_cpu(initial_boot_params->totalsize));
> -	pr_debug("version: %08x\n", be32_to_cpu(initial_boot_params->version));
> -
> -	if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
> -		pr_err("Invalid device tree blob header\n");
> -		return;
> -	}
> -
> -	/* First pass, scan for size */
> -	start = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	size = unflatten_dt_node(initial_boot_params, 0, &start,
> -				 NULL, NULL, 0);
> -	size = (size | 3) + 1;
> -
> -	pr_debug("  size is %lx, allocating...\n", size);
> -
> -	/* Allocate memory for the expanded device tree */
> -	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);
> -
> -	pr_debug("  unflattening %lx...\n", mem);
> -
> -	/* Second pass, do actual unflattening */
> -	start = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	unflatten_dt_node(initial_boot_params, mem, &start,
> -			  NULL, &allnextp, 0);
> -	if (be32_to_cpup((__be32 *)start) != OF_DT_END)
> -		pr_warning("Weird tag at end of tree: %08x\n", *((u32 *)start));
> -	if (be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef)
> -		pr_warning("End of tree marker overwritten: %08x\n",
> -			   be32_to_cpu(((__be32 *)mem)[size / 4]));
> -	*allnextp = NULL;
> +	__unflatten_device_tree(initial_boot_params, &allnodes,
> +				early_device_tree_alloc);
>  
>  	/* Get pointer to OF "/chosen" node for use everywhere */
>  	of_chosen = of_find_node_by_path("/chosen");
>  	if (of_chosen == NULL)
>  		of_chosen = of_find_node_by_path("/chosen@0");
> -
> -	pr_debug(" <- unflatten_device_tree()\n");
>  }
>  
>  #endif /* CONFIG_EARLY_FLATTREE */
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 70c5b73..9ce5dfd 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -68,6 +68,8 @@ extern void *of_fdt_get_property(struct boot_param_header *blob,
>  extern int of_fdt_is_compatible(struct boot_param_header *blob,
>  				unsigned long node,
>  				const char *compat);
> +extern void of_fdt_unflatten_tree(unsigned long *blob,
> +			       struct device_node **mynodes);
>  
>  /* TBD: Temporary export of fdt globals - remove when code fully merged */
>  extern int __initdata dt_root_addr_cells;
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Jeremy Fitzhardinge @ 2010-12-30  1:26 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Andrew Morton, linuxppc-dev, Nick Piggin, linux-kernel
In-Reply-To: <alpine.LSU.2.00.1012291413360.22939@sister.anvils>

On 12/30/2010 09:54 AM, Hugh Dickins wrote:
> With recent 2.6.37-rc, with CONFIG_PREEMPT=y CONFIG_DEBUG_PREEMPT=y
> on the PowerPC G5, I get spammed by BUG warnings each time I swapoff:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: swapoff/3974
> caller is .hpte_need_flush+0x4c/0x2e8
> Call Trace:
> [c0000001b4a3f830] [c00000000000f3cc] .show_stack+0x6c/0x16c (unreliable)
> [c0000001b4a3f8e0] [c00000000023eda0] .debug_smp_processor_id+0xe4/0x11c
> [c0000001b4a3f970] [c00000000002f2f4] .hpte_need_flush+0x4c/0x2e8
> [c0000001b4a3fa30] [c0000000000e7ef8] .vunmap_pud_range+0x148/0x200
> [c0000001b4a3fb10] [c0000000000e8058] .vunmap_page_range+0xa8/0xd4
> [c0000001b4a3fbb0] [c0000000000e80a4] .free_unmap_vmap_area+0x20/0x38
> [c0000001b4a3fc40] [c0000000000e8138] .remove_vm_area+0x7c/0xb4
> [c0000001b4a3fcd0] [c0000000000e8308] .__vunmap+0x50/0x104
> [c0000001b4a3fd60] [c0000000000ef3fc] .SyS_swapoff+0x59c/0x6a8
> [c0000001b4a3fe30] [c0000000000075a8] syscall_exit+0x0/0x40
>
> I notice hpte_need_flush() itself acknowledges
>  * Must be called from within some kind of spinlock/non-preempt region...
>
> Though I didn't actually bisect, I believe this is since Jeremy's
> 64141da587241301ce8638cc945f8b67853156ec "vmalloc: eagerly clear ptes
> on vunmap", which moves a call to vunmap_page_range() from one place
> (which happened to be inside a spinlock) to another (where it's not).

Yes.  I was  bit worried by the interaction between vb->lock and the pte
locks, and thought it safer to keep it outside.  Though I suppose kernel
mappings don't have pte locks, so that's a non-issue.

> I guess my warnings would be easily silenced by moving that call to
> vunmap_page_range() down just inside the spinlock below it; but I'm
> dubious that that's the right fix - it looked as if there are other
> paths through vmalloc.c where vunmap_page_range() has been getting
> called without preemption disabled, long before Jeremy's change,
> just paths that I never happen to go down in my limited testing.
>
> For the moment I'm using the obvious patch below to keep it quiet;
> but I doubt that this is the right patch either.  I'm hoping that
> ye who understand the importance of hpte_need_flush() will be best
> able to judge what to do.  Or might there be other architectures
> expecting to be unpreemptible there?

Since kernel mappings don't have a formal pte lock to rely on for
synchronization, each subsystem defines its own ad-hoc one.  In this
case that's probably vb->lock anyway, so the fix is to move
vunmap_page_range() back into its protection.

> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -37,11 +37,13 @@ static void vunmap_pte_range(pmd_t *pmd,
>  {
>  	pte_t *pte;
>  
> +	preempt_disable(); /* Stop __vunmap() triggering smp_processor_id() in preemptible from hpte_need_flush() */
>  	pte = pte_offset_kernel(pmd, addr);
>  	do {
>  		pte_t ptent = ptep_get_and_clear(&init_mm, addr, pte);
>  		WARN_ON(!pte_none(ptent) && !pte_present(ptent));
>  	} while (pte++, addr += PAGE_SIZE, addr != end);
> +	preempt_enable();
>  }
>  
>  static void vunmap_pmd_range(pud_t *pud, unsigned long addr, unsigned long end)

That looks OK, but it interferes with my plans to use
apply_to_page_range(_batch) to replace all this code.

    J

^ permalink raw reply

* Re: [PATCH 1/6] mpc512x_dma: scatter/gather fix
From: Grant Likely @ 2010-12-30  5:35 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-2-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:55AM +0200, Ilya Yanok wrote:
> While testing mpc512x-dma driver with dmatest module I've found that
> I can hang the mpc512x-dma issueing request from multiple threads to
> the single channel.
> (insmod dmatest.ko max_channels=1 threads_per_chan=16)
> After investingating this case I've managed to find that this happens
> if and only if we have more than one quequed requests.
> In this case the driver tries to make use of hardware scatter/gather
> functionality. I've found two problems with scatter/gather:
>  1. When TCD is copied form RAM to the TCD register space with memcpy_io()
> e_sg bit eventually gets cleared. This results in only first TCD being
> executed. I've added setting of e_sg bit excplicitly in the TCD registers.
> BTW, what is the correct way to do this? (How can I use setbits with bitfield
> structure?) After that hardware loads consecutive TCDs and we hit the
> second issue.
>  2. Existing code clears int_maj bit in the last TCD so we never get
> an interrupt on transfefr completion.
> 
> With these fixes my tests with many threads of single channel succeed but
> tests that use many channels simultaneously still don't work reliable.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 4e9cbf3..1bc04aa 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -252,11 +252,13 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>  		prev = mdesc;
>  	}
>  
> -	prev->tcd->start = 0;
>  	prev->tcd->int_maj = 1;
>  
>  	/* Send first descriptor in chain into hardware */
>  	memcpy_toio(&mdma->tcd[cid], first->tcd, sizeof(struct mpc_dma_tcd));
> +
> +	if (first != prev)
> +		mdma->tcd[cid].e_sg = 1;
>  	out_8(&mdma->regs->dmassrt, cid);
>  }
>  
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 3/6] mpc512x_dma: add MPC8308 support
From: Grant Likely @ 2010-12-30  5:35 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-4-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:57AM +0200, Ilya Yanok wrote:
> MPC8308 has pretty much the same DMA controller as MPC5121 and
> this patch adds support for MPC8308 to the mpc512x_dma driver.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/Kconfig       |    2 +-
>  drivers/dma/mpc512x_dma.c |   95 +++++++++++++++++++++++++++++++++-----------
>  2 files changed, 72 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 9520cf0..5c5e95b 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -100,7 +100,7 @@ config FSL_DMA
>  
>  config MPC512X_DMA
>  	tristate "Freescale MPC512x built-in DMA engine support"
> -	depends on PPC_MPC512x
> +	depends on PPC_MPC512x || PPC_MPC831x
>  	select DMA_ENGINE
>  	---help---
>  	  Enable support for the Freescale MPC512x built-in DMA engine.
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 0717527..97b92ec 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -1,6 +1,7 @@
>  /*
>   * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
>   * Copyright (C) Semihalf 2009
> + * Copyright (C) Ilya Yanok, Emcraft Systems 2010
>   *
>   * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description
>   * (defines, structures and comments) was taken from MPC5121 DMA driver
> @@ -70,6 +71,8 @@
>  #define MPC_DMA_DMAES_SBE	(1 << 1)
>  #define MPC_DMA_DMAES_DBE	(1 << 0)
>  
> +#define MPC_DMA_DMAGPOR_SNOOP_ENABLE	(1 << 6)
> +
>  #define MPC_DMA_TSIZE_1		0x00
>  #define MPC_DMA_TSIZE_2		0x01
>  #define MPC_DMA_TSIZE_4		0x02
> @@ -104,7 +107,10 @@ struct __attribute__ ((__packed__)) mpc_dma_regs {
>  	/* 0x30 */
>  	u32 dmahrsh;		/* DMA hw request status high(ch63~32) */
>  	u32 dmahrsl;		/* DMA hardware request status low(ch31~0) */
> -	u32 dmaihsa;		/* DMA interrupt high select AXE(ch63~32) */
> +	union {
> +		u32 dmaihsa;	/* DMA interrupt high select AXE(ch63~32) */
> +		u32 dmagpor;	/* (General purpose register on MPC8308) */
> +	};
>  	u32 dmailsa;		/* DMA interrupt low select AXE(ch31~0) */
>  	/* 0x40 ~ 0xff */
>  	u32 reserve0[48];	/* Reserved */
> @@ -195,7 +201,9 @@ struct mpc_dma {
>  	struct mpc_dma_regs __iomem	*regs;
>  	struct mpc_dma_tcd __iomem	*tcd;
>  	int				irq;
> +	int				irq2;
>  	uint				error_status;
> +	int				is_mpc8308;
>  
>  	/* Lock for error_status field in this structure */
>  	spinlock_t			error_status_lock;
> @@ -307,8 +315,10 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	spin_unlock(&mdma->error_status_lock);
>  
>  	/* Handle interrupt on each channel */
> -	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
> +	if (mdma->dma.chancnt > 32) {
> +		mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
>  					in_be32(&mdma->regs->dmaerrh), 32);
> +	}
>  	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
>  					in_be32(&mdma->regs->dmaerrl), 0);
>  
> @@ -562,6 +572,7 @@ static struct dma_async_tx_descriptor *
>  mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  					size_t len, unsigned long flags)
>  {
> +	struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
>  	struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
>  	struct mpc_dma_desc *mdesc = NULL;
>  	struct mpc_dma_tcd *tcd;
> @@ -590,7 +601,8 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  		tcd->dsize = MPC_DMA_TSIZE_32;
>  		tcd->soff = 32;
>  		tcd->doff = 32;
> -	} else if (IS_ALIGNED(src | dst | len, 16)) {
> +	} else if (!mdma->is_mpc8308 && IS_ALIGNED(src | dst | len, 16)) {
> +		/* MPC8308 doesn't support 16 byte transfers */
>  		tcd->ssize = MPC_DMA_TSIZE_16;
>  		tcd->dsize = MPC_DMA_TSIZE_16;
>  		tcd->soff = 16;
> @@ -650,6 +662,15 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  		return -EINVAL;
>  	}
>  
> +	if (of_device_is_compatible(dn, "fsl,mpc8308-dma")) {
> +		mdma->is_mpc8308 = 1;
> +		mdma->irq2 = irq_of_parse_and_map(dn, 1);
> +		if (mdma->irq2 == 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");
> @@ -680,11 +701,23 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  		return -EINVAL;
>  	}
>  
> +	if (mdma->is_mpc8308) {
> +		retval = devm_request_irq(dev, mdma->irq2, &mpc_dma_irq, 0,
> +				DRV_NAME, mdma);
> +		if (retval) {
> +			dev_err(dev, "Error requesting IRQ2!\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	spin_lock_init(&mdma->error_status_lock);
>  
>  	dma = &mdma->dma;
>  	dma->dev = dev;
> -	dma->chancnt = MPC_DMA_CHANNELS;
> +	if (!mdma->is_mpc8308)
> +		dma->chancnt = MPC_DMA_CHANNELS;
> +	else
> +		dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */
>  	dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources;
>  	dma->device_free_chan_resources = mpc_dma_free_chan_resources;
>  	dma->device_issue_pending = mpc_dma_issue_pending;
> @@ -720,26 +753,40 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  	 * - Round-robin group arbitration,
>  	 * - Round-robin channel arbitration.
>  	 */
> -	out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
> -				MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
> -
> -	/* Disable hardware DMA requests */
> -	out_be32(&mdma->regs->dmaerqh, 0);
> -	out_be32(&mdma->regs->dmaerql, 0);
> -
> -	/* Disable error interrupts */
> -	out_be32(&mdma->regs->dmaeeih, 0);
> -	out_be32(&mdma->regs->dmaeeil, 0);
> -
> -	/* Clear interrupts status */
> -	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> -
> -	/* Route interrupts to IPIC */
> -	out_be32(&mdma->regs->dmaihsa, 0);
> -	out_be32(&mdma->regs->dmailsa, 0);
> +	if (!mdma->is_mpc8308) {
> +		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
> +					MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
> +
> +		/* Disable hardware DMA requests */
> +		out_be32(&mdma->regs->dmaerqh, 0);
> +		out_be32(&mdma->regs->dmaerql, 0);
> +
> +		/* Disable error interrupts */
> +		out_be32(&mdma->regs->dmaeeih, 0);
> +		out_be32(&mdma->regs->dmaeeil, 0);
> +
> +		/* Clear interrupts status */
> +		out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> +
> +		/* Route interrupts to IPIC */
> +		out_be32(&mdma->regs->dmaihsa, 0);
> +		out_be32(&mdma->regs->dmailsa, 0);
> +	} else {
> +		/* MPC8308 has 16 channels and lacks some registers */
> +		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
> +
> +		/* enable snooping */
> +		out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
> +		/* Disable error interrupts */
> +		out_be32(&mdma->regs->dmaeeil, 0);
> +
> +		/* Clear interrupts status */
> +		out_be32(&mdma->regs->dmaintl, 0xFFFF);
> +		out_be32(&mdma->regs->dmaerrl, 0xFFFF);
> +	}
>  
>  	/* Register DMA engine */
>  	dev_set_drvdata(dev, mdma);
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 6/6] mpc8308_p1m: add DMA controller device-tree node
From: Grant Likely @ 2010-12-30  5:35 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu
In-Reply-To: <1288137180-3220-7-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:53:00AM +0200, Ilya Yanok wrote:
> MPC8308 has DMA controller compatible with mpc512x_dma driver. This
> patch adds device-tree node to support DMA controller on MPC8308 P1M
> board.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>

Merged for -next, thanks.

g.

> ---
>  arch/powerpc/boot/dts/mpc8308_p1m.dts |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts b/arch/powerpc/boot/dts/mpc8308_p1m.dts
> index 05a76cc..697b3f6 100644
> --- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
> +++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
> @@ -297,6 +297,14 @@
>  			interrupt-parent = < &ipic >;
>  		};
>  
> +		dma@2c000 {
> +			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
> +			reg = <0x2c000 0x1800>;
> +			interrupts = <3 0x8
> +					94 0x8>;
> +			interrupt-parent = < &ipic >;
> +		};
> +
>  	};
>  
>  	pci0: pcie@e0009000 {
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 5/6] MPC8308RDB: add DMA controller device-tree node
From: Grant Likely @ 2010-12-30  5:36 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu
In-Reply-To: <1288137180-3220-6-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:59AM +0200, Ilya Yanok wrote:
> MPC8308 has DMA controller compatible with mpc512x_dma driver. This
> patch adds device-tree node to support DMA controller on MPC8308RDB
> board.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>

Merged for -next, thanks.

g.

> ---
>  arch/powerpc/boot/dts/mpc8308rdb.dts |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8308rdb.dts b/arch/powerpc/boot/dts/mpc8308rdb.dts
> index 1e2b888..a0bd188 100644
> --- a/arch/powerpc/boot/dts/mpc8308rdb.dts
> +++ b/arch/powerpc/boot/dts/mpc8308rdb.dts
> @@ -265,6 +265,14 @@
>  			interrupt-parent = < &ipic >;
>  		};
>  
> +		dma@2c000 {
> +			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
> +			reg = <0x2c000 0x1800>;
> +			interrupts = <3 0x8
> +					94 0x8>;
> +			interrupt-parent = < &ipic >;
> +		};
> +
>  	};
>  
>  	pci0: pcie@e0009000 {
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 2/6] mpc512x_dma: fix the hanged transfer issue
From: Grant Likely @ 2010-12-30  5:36 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-3-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:56AM +0200, Ilya Yanok wrote:
> Current code clears interrupt active status _after_ submiting new
> transfers. This leaves a possibility of clearing the interrupt for this
> new transfer (if it is triggered fast enough) and thus lose this
> interrupt. We want to clear interrupt active status _before_ new
> transfers is submited and for current channel only.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 1bc04aa..0717527 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -276,6 +276,9 @@ static void mpc_dma_irq_process(struct mpc_dma *mdma, u32 is, u32 es, int off)
>  
>  		spin_lock(&mchan->lock);
>  
> +		out_8(&mdma->regs->dmacint, ch + off);
> +		out_8(&mdma->regs->dmacerr, ch + off);
> +
>  		/* Check error status */
>  		if (es & (1 << ch))
>  			list_for_each_entry(mdesc, &mchan->active, node)
> @@ -309,12 +312,6 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
>  					in_be32(&mdma->regs->dmaerrl), 0);
>  
> -	/* Ack interrupt on all channels */
> -	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> -
>  	/* Schedule tasklet */
>  	tasklet_schedule(&mdma->tasklet);
>  
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 4/6] mpc512x_dma: try to free descriptors in case of allocation failure
From: Grant Likely @ 2010-12-30  5:36 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-5-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:58AM +0200, Ilya Yanok wrote:
> Currently completed descriptors are processed in the tasklet. This can
> lead to dead lock in case of CONFIG_NET_DMA enabled (new requests are
> submitted from softirq context and dma_memcpy_to_iovec() busy loops until
> the requests is submitted). To prevent this we should process completed
> descriptors from the allocation failure path in prepare_memcpy too.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |   79 +++++++++++++++++++++++++-------------------
>  1 files changed, 45 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 97b92ec..59c2701 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -328,19 +328,55 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -/* DMA Tasklet */
> -static void mpc_dma_tasklet(unsigned long data)
> +/* proccess completed descriptors */
> +static void mpc_dma_process_completed(struct mpc_dma *mdma)
>  {
> -	struct mpc_dma *mdma = (void *)data;
>  	dma_cookie_t last_cookie = 0;
>  	struct mpc_dma_chan *mchan;
>  	struct mpc_dma_desc *mdesc;
>  	struct dma_async_tx_descriptor *desc;
>  	unsigned long flags;
>  	LIST_HEAD(list);
> -	uint es;
>  	int i;
>  
> +	for (i = 0; i < mdma->dma.chancnt; i++) {
> +		mchan = &mdma->channels[i];
> +
> +		/* Get all completed descriptors */
> +		spin_lock_irqsave(&mchan->lock, flags);
> +		if (!list_empty(&mchan->completed))
> +			list_splice_tail_init(&mchan->completed, &list);
> +		spin_unlock_irqrestore(&mchan->lock, flags);
> +
> +		if (list_empty(&list))
> +			continue;
> +
> +		/* Execute callbacks and run dependencies */
> +		list_for_each_entry(mdesc, &list, node) {
> +			desc = &mdesc->desc;
> +
> +			if (desc->callback)
> +				desc->callback(desc->callback_param);
> +
> +			last_cookie = desc->cookie;
> +			dma_run_dependencies(desc);
> +		}
> +
> +		/* Free descriptors */
> +		spin_lock_irqsave(&mchan->lock, flags);
> +		list_splice_tail_init(&list, &mchan->free);
> +		mchan->completed_cookie = last_cookie;
> +		spin_unlock_irqrestore(&mchan->lock, flags);
> +	}
> +}
> +
> +/* DMA Tasklet */
> +static void mpc_dma_tasklet(unsigned long data)
> +{
> +	struct mpc_dma *mdma = (void *)data;
> +	unsigned long flags;
> +	uint es;
> +
>  	spin_lock_irqsave(&mdma->error_status_lock, flags);
>  	es = mdma->error_status;
>  	mdma->error_status = 0;
> @@ -379,35 +415,7 @@ static void mpc_dma_tasklet(unsigned long data)
>  			dev_err(mdma->dma.dev, "- Destination Bus Error\n");
>  	}
>  
> -	for (i = 0; i < mdma->dma.chancnt; i++) {
> -		mchan = &mdma->channels[i];
> -
> -		/* Get all completed descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		if (!list_empty(&mchan->completed))
> -			list_splice_tail_init(&mchan->completed, &list);
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> -
> -		if (list_empty(&list))
> -			continue;
> -
> -		/* Execute callbacks and run dependencies */
> -		list_for_each_entry(mdesc, &list, node) {
> -			desc = &mdesc->desc;
> -
> -			if (desc->callback)
> -				desc->callback(desc->callback_param);
> -
> -			last_cookie = desc->cookie;
> -			dma_run_dependencies(desc);
> -		}
> -
> -		/* Free descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		list_splice_tail_init(&list, &mchan->free);
> -		mchan->completed_cookie = last_cookie;
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> -	}
> +	mpc_dma_process_completed(mdma);
>  }
>  
>  /* Submit descriptor to hardware */
> @@ -587,8 +595,11 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  	}
>  	spin_unlock_irqrestore(&mchan->lock, iflags);
>  
> -	if (!mdesc)
> +	if (!mdesc) {
> +		/* try to free completed descriptors */
> +		mpc_dma_process_completed(mdma);
>  		return NULL;
> +	}
>  
>  	mdesc->error = 0;
>  	tcd = mdesc->tcd;
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH] eSPI: change the read behavior of the SPIRF
From: Grant Likely @ 2010-12-30  6:04 UTC (permalink / raw)
  To: Mingkai Hu; +Cc: linuxppc-dev, kumar.gala, spi-devel-general
In-Reply-To: <1291195758-12322-1-git-send-email-Mingkai.hu@freescale.com>

On Wed, Dec 01, 2010 at 05:29:18PM +0800, Mingkai Hu wrote:
> The user must read N bytes of SPIRF (1 <= N <= 4) that do not exceed the
> amount of data in the receive FIFO, so read the SPIRF byte by byte when
> the data in receive FIFO is less than 4 bytes.
> 
> On Simics, when read N bytes that exceed the amout of data in receive
> FIFO, we can't read the data out, that is we can't clear the rx FIFO,
> then the CPU will loop on the espi rx interrupt.
> 
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>

Applied for -next, thanks.

g.

> ---
>  drivers/spi/spi_fsl_espi.c |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c
> index e3b4f64..ae78926 100644
> --- a/drivers/spi/spi_fsl_espi.c
> +++ b/drivers/spi/spi_fsl_espi.c
> @@ -507,16 +507,29 @@ void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
>  
>  	/* We need handle RX first */
>  	if (events & SPIE_NE) {
> -		u32 rx_data;
> +		u32 rx_data, tmp;
> +		u8 rx_data_8;
>  
>  		/* Spin until RX is done */
>  		while (SPIE_RXCNT(events) < min(4, mspi->len)) {
>  			cpu_relax();
>  			events = mpc8xxx_spi_read_reg(&reg_base->event);
>  		}
> -		mspi->len -= 4;
>  
> -		rx_data = mpc8xxx_spi_read_reg(&reg_base->receive);
> +		if (mspi->len >= 4) {
> +			rx_data = mpc8xxx_spi_read_reg(&reg_base->receive);
> +		} else {
> +			tmp = mspi->len;
> +			rx_data = 0;
> +			while (tmp--) {
> +				rx_data_8 = in_8((u8 *)&reg_base->receive);
> +				rx_data |= (rx_data_8 << (tmp * 8));
> +			}
> +
> +			rx_data <<= (4 - mspi->len) * 8;
> +		}
> +
> +		mspi->len -= 4;
>  
>  		if (mspi->rx)
>  			mspi->get_rx(rx_data, mspi);
> -- 
> 1.7.0.4
> 
> 

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Grant Likely @ 2010-12-30  6:50 UTC (permalink / raw)
  To: Lan Chunhe; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <1291978340-15933-1-git-send-email-b25806@freescale.com>

On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
> Currently, of_platform_bus_probe() completely skips nodes which do not
> explicitly match the 'matches' table passed in.  Or, if the root node
> matches, then it registers all the children unconditionally.  However,
> there are situations, such as registering devices from the root node,
> when it is desirable to register child nodes, but only if they actually
> represent devices.  For example, the root node may contain both a local
> bus and a PCI device, but it also contains the chosen, aliases and cpus
> nodes which don't represent real devices.
> 
> This patch changes of_platform_bus_probe() to register all nodes at the
> top level if they either match the matches table (the current behaviour),
> or if they have a 'compatible' value (indicating it represents a device).
> 
> Signed-off-by: Lan Chunhe <b25806@freescale.com>

I believe this is my patch I wrote and pushed out to my tree back in
October.  Was this cherry-picked out of the test-devicetree branch?

> ---
>  drivers/of/platform.c |   28 +++++++++++++++++++++++-----
>  1 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 5b4a07f..02755ab 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node *root,
>  	struct device_node *child;
>  	struct platform_device *dev;
>  	int rc = 0;
> +	const void *compat;
> +	const struct of_device_id *match;
>  
>  	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
>  		return -EINVAL;
> @@ -741,16 +743,32 @@ int of_platform_bus_probe(struct device_node *root,
>  		rc = of_platform_bus_create(root, matches, &dev->dev);
>  		goto bail;
>  	}
> +
> +	/*
> +	 * Register each child node if either:
> +	 *  a) it has a 'compatible' value indicating they are a device, or
> +	 *  b) it is specified by the 'matches' table (by name or device_type)
> +	 * If a node is specified in the matches table, then all its children
> +	 * also get registered.
> +	 */
>  	for_each_child_of_node(root, child) {
> -		if (!of_match_node(matches, child))
> +		compat = of_get_property(child, "compatible", NULL);
> +		match = of_match_node(matches, child);
> +		if (!compat && !match)
>  			continue;
>  
> -		pr_debug("  match: %s\n", child->full_name);
> +		pr_debug("  register device: %s\n", child->full_name);
>  		dev = of_platform_device_create(child, NULL, parent);
> -		if (dev == NULL)
> +		if (!dev) {
>  			rc = -ENOMEM;
> -		else
> -			rc = of_platform_bus_create(child, matches, &dev->dev);
> +			of_node_put(child);
> +			break;
> +		}
> +		if (!match)
> +			continue;
> +
> +		pr_debug("  register children of: %s\n", child->full_name);
> +		rc = of_platform_bus_create(child, matches, &dev->dev);
>  		if (rc) {
>  			of_node_put(child);
>  			break;
> -- 
> 1.5.4.5
> 
> 

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Lan Chunhe @ 2010-12-30  7:23 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <20101230065045.GB843@angua.secretlab.ca>

On Thu, 30 Dec 2010 14:50:45 +0800, Grant Likely  
<grant.likely@secretlab.ca> wrote:

> On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
>> Currently, of_platform_bus_probe() completely skips nodes which do not
>> explicitly match the 'matches' table passed in.  Or, if the root node
>> matches, then it registers all the children unconditionally.  However,
>> there are situations, such as registering devices from the root node,
>> when it is desirable to register child nodes, but only if they actually
>> represent devices.  For example, the root node may contain both a local
>> bus and a PCI device, but it also contains the chosen, aliases and cpus
>> nodes which don't represent real devices.
>>
>> This patch changes of_platform_bus_probe() to register all nodes at the
>> top level if they either match the matches table (the current  
>> behaviour),
>> or if they have a 'compatible' value (indicating it represents a  
>> device).
>>
>> Signed-off-by: Lan Chunhe <b25806@freescale.com>
>
> I believe this is my patch I wrote and pushed out to my tree back in
> October.  Was this cherry-picked out of the test-devicetree branch?

    Yes, I will add the signed off with you.

      Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> Signed-off-by: Lan Chunhe <b25806@freescale.com>

    At present I think that it is cherry-picked, and I have tested it which
    is OK.
    Do you have better method?

    Thanks.
    -Lan

>> ---
>>  drivers/of/platform.c |   28 +++++++++++++++++++++++-----
>>  1 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index 5b4a07f..02755ab 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node *root,
>>  	struct device_node *child;
>>  	struct platform_device *dev;
>>  	int rc = 0;
>> +	const void *compat;
>> +	const struct of_device_id *match;
>>
>>  	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
>>  		return -EINVAL;
>> @@ -741,16 +743,32 @@ int of_platform_bus_probe(struct device_node  
>> *root,
>>  		rc = of_platform_bus_create(root, matches, &dev->dev);
>>  		goto bail;
>>  	}
>> +
>> +	/*
>> +	 * Register each child node if either:
>> +	 *  a) it has a 'compatible' value indicating they are a device, or
>> +	 *  b) it is specified by the 'matches' table (by name or device_type)
>> +	 * If a node is specified in the matches table, then all its children
>> +	 * also get registered.
>> +	 */
>>  	for_each_child_of_node(root, child) {
>> -		if (!of_match_node(matches, child))
>> +		compat = of_get_property(child, "compatible", NULL);
>> +		match = of_match_node(matches, child);
>> +		if (!compat && !match)
>>  			continue;
>>
>> -		pr_debug("  match: %s\n", child->full_name);
>> +		pr_debug("  register device: %s\n", child->full_name);
>>  		dev = of_platform_device_create(child, NULL, parent);
>> -		if (dev == NULL)
>> +		if (!dev) {
>>  			rc = -ENOMEM;
>> -		else
>> -			rc = of_platform_bus_create(child, matches, &dev->dev);
>> +			of_node_put(child);
>> +			break;
>> +		}
>> +		if (!match)
>> +			continue;
>> +
>> +		pr_debug("  register children of: %s\n", child->full_name);
>> +		rc = of_platform_bus_create(child, matches, &dev->dev);
>>  		if (rc) {
>>  			of_node_put(child);
>>  			break;
>> --
>> 1.5.4.5
>>
>>
>

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Grant Likely @ 2010-12-30  7:31 UTC (permalink / raw)
  To: Lan Chunhe; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <op.voimsxy9z91x4h@localhost.localdomain>

On Thu, Dec 30, 2010 at 03:23:11PM +0800, Lan Chunhe wrote:
> On Thu, 30 Dec 2010 14:50:45 +0800, Grant Likely
> <grant.likely@secretlab.ca> wrote:
> 
> >On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
> >>Currently, of_platform_bus_probe() completely skips nodes which do not
> >>explicitly match the 'matches' table passed in.  Or, if the root node
> >>matches, then it registers all the children unconditionally.  However,
> >>there are situations, such as registering devices from the root node,
> >>when it is desirable to register child nodes, but only if they actually
> >>represent devices.  For example, the root node may contain both a local
> >>bus and a PCI device, but it also contains the chosen, aliases and cpus
> >>nodes which don't represent real devices.
> >>
> >>This patch changes of_platform_bus_probe() to register all nodes at the
> >>top level if they either match the matches table (the current
> >>behaviour),
> >>or if they have a 'compatible' value (indicating it represents a
> >>device).
> >>
> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
> >
> >I believe this is my patch I wrote and pushed out to my tree back in
> >October.  Was this cherry-picked out of the test-devicetree branch?
> 
>    Yes, I will add the signed off with you.
> 
>      Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
> 
>    At present I think that it is cherry-picked, and I have tested it which
>    is OK.
>    Do you have better method?

You need to be very careful about stuff like this.  Posting a patch
without attribution of the original author and has the original
signed-off-by lines stripped off is an absolute no-no.  In literary
circles this would be called plagiarism.

I don't think you did it intentionally, but proper attribution is a
must, particularly because there are copyright issues involved.

As for this particular patch, I'm not particularly happy with it and I
was planning to rewrite it.  It is an inelegant solution and I think
the registration of devices from the device tree can be made simpler
instead of more complex like in this patch.

g.

> 
>    Thanks.
>    -Lan
> 
> >>---
> >> drivers/of/platform.c |   28 +++++++++++++++++++++++-----
> >> 1 files changed, 23 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> >>index 5b4a07f..02755ab 100644
> >>--- a/drivers/of/platform.c
> >>+++ b/drivers/of/platform.c
> >>@@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node *root,
> >> 	struct device_node *child;
> >> 	struct platform_device *dev;
> >> 	int rc = 0;
> >>+	const void *compat;
> >>+	const struct of_device_id *match;
> >>
> >> 	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
> >> 		return -EINVAL;
> >>@@ -741,16 +743,32 @@ int of_platform_bus_probe(struct
> >>device_node *root,
> >> 		rc = of_platform_bus_create(root, matches, &dev->dev);
> >> 		goto bail;
> >> 	}
> >>+
> >>+	/*
> >>+	 * Register each child node if either:
> >>+	 *  a) it has a 'compatible' value indicating they are a device, or
> >>+	 *  b) it is specified by the 'matches' table (by name or device_type)
> >>+	 * If a node is specified in the matches table, then all its children
> >>+	 * also get registered.
> >>+	 */
> >> 	for_each_child_of_node(root, child) {
> >>-		if (!of_match_node(matches, child))
> >>+		compat = of_get_property(child, "compatible", NULL);
> >>+		match = of_match_node(matches, child);
> >>+		if (!compat && !match)
> >> 			continue;
> >>
> >>-		pr_debug("  match: %s\n", child->full_name);
> >>+		pr_debug("  register device: %s\n", child->full_name);
> >> 		dev = of_platform_device_create(child, NULL, parent);
> >>-		if (dev == NULL)
> >>+		if (!dev) {
> >> 			rc = -ENOMEM;
> >>-		else
> >>-			rc = of_platform_bus_create(child, matches, &dev->dev);
> >>+			of_node_put(child);
> >>+			break;
> >>+		}
> >>+		if (!match)
> >>+			continue;
> >>+
> >>+		pr_debug("  register children of: %s\n", child->full_name);
> >>+		rc = of_platform_bus_create(child, matches, &dev->dev);
> >> 		if (rc) {
> >> 			of_node_put(child);
> >> 			break;
> >>--
> >>1.5.4.5
> >>
> >>
> >
> 
> 

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Lan Chunhe @ 2010-12-30  8:04 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <20101230073157.GC843@angua.secretlab.ca>

On Thu, 30 Dec 2010 15:31:57 +0800, Grant Likely  
<grant.likely@secretlab.ca> wrote:

> On Thu, Dec 30, 2010 at 03:23:11PM +0800, Lan Chunhe wrote:
>> On Thu, 30 Dec 2010 14:50:45 +0800, Grant Likely
>> <grant.likely@secretlab.ca> wrote:
>>
>> >On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
>> >>Currently, of_platform_bus_probe() completely skips nodes which do not
>> >>explicitly match the 'matches' table passed in.  Or, if the root node
>> >>matches, then it registers all the children unconditionally.  However,
>> >>there are situations, such as registering devices from the root node,
>> >>when it is desirable to register child nodes, but only if they  
>> actually
>> >>represent devices.  For example, the root node may contain both a  
>> local
>> >>bus and a PCI device, but it also contains the chosen, aliases and  
>> cpus
>> >>nodes which don't represent real devices.
>> >>
>> >>This patch changes of_platform_bus_probe() to register all nodes at  
>> the
>> >>top level if they either match the matches table (the current
>> >>behaviour),
>> >>or if they have a 'compatible' value (indicating it represents a
>> >>device).
>> >>
>> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
>> >
>> >I believe this is my patch I wrote and pushed out to my tree back in
>> >October.  Was this cherry-picked out of the test-devicetree branch?
>>
>>    Yes, I will add the signed off with you.
>>
>>      Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
>>
>>    At present I think that it is cherry-picked, and I have tested it  
>> which
>>    is OK.
>>    Do you have better method?
>
> You need to be very careful about stuff like this.  Posting a patch
> without attribution of the original author and has the original
> signed-off-by lines stripped off is an absolute no-no.  In literary
> circles this would be called plagiarism.
>
> I don't think you did it intentionally, but proper attribution is a
> must, particularly because there are copyright issues involved.

    Sorry, thank you that you remind me.

> As for this particular patch, I'm not particularly happy with it and I
> was planning to rewrite it.  It is an inelegant solution and I think
> the registration of devices from the device tree can be made simpler
> instead of more complex like in this patch.

    Now this patch registers devices from the device tree.
    Then which function of which file do you will modify?

    Thanks.
    -Lan
> g.
>
>>
>>    Thanks.
>>    -Lan
>>
>> >>---
>> >> drivers/of/platform.c |   28 +++++++++++++++++++++++-----
>> >> 1 files changed, 23 insertions(+), 5 deletions(-)
>> >>
>> >>diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> >>index 5b4a07f..02755ab 100644
>> >>--- a/drivers/of/platform.c
>> >>+++ b/drivers/of/platform.c
>> >>@@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node  
>> *root,
>> >> 	struct device_node *child;
>> >> 	struct platform_device *dev;
>> >> 	int rc = 0;
>> >>+	const void *compat;
>> >>+	const struct of_device_id *match;
>> >>
>> >> 	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
>> >> 		return -EINVAL;
>> >>@@ -741,16 +743,32 @@ int of_platform_bus_probe(struct
>> >>device_node *root,
>> >> 		rc = of_platform_bus_create(root, matches, &dev->dev);
>> >> 		goto bail;
>> >> 	}
>> >>+
>> >>+	/*
>> >>+	 * Register each child node if either:
>> >>+	 *  a) it has a 'compatible' value indicating they are a device, or
>> >>+	 *  b) it is specified by the 'matches' table (by name or  
>> device_type)
>> >>+	 * If a node is specified in the matches table, then all its  
>> children
>> >>+	 * also get registered.
>> >>+	 */
>> >> 	for_each_child_of_node(root, child) {
>> >>-		if (!of_match_node(matches, child))
>> >>+		compat = of_get_property(child, "compatible", NULL);
>> >>+		match = of_match_node(matches, child);
>> >>+		if (!compat && !match)
>> >> 			continue;
>> >>
>> >>-		pr_debug("  match: %s\n", child->full_name);
>> >>+		pr_debug("  register device: %s\n", child->full_name);
>> >> 		dev = of_platform_device_create(child, NULL, parent);
>> >>-		if (dev == NULL)
>> >>+		if (!dev) {
>> >> 			rc = -ENOMEM;
>> >>-		else
>> >>-			rc = of_platform_bus_create(child, matches, &dev->dev);
>> >>+			of_node_put(child);
>> >>+			break;
>> >>+		}
>> >>+		if (!match)
>> >>+			continue;
>> >>+
>> >>+		pr_debug("  register children of: %s\n", child->full_name);
>> >>+		rc = of_platform_bus_create(child, matches, &dev->dev);
>> >> 		if (rc) {
>> >> 			of_node_put(child);
>> >> 			break;
>> >>--
>> >>1.5.4.5
>> >>
>> >>
>> >
>>
>>
>

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Benjamin Herrenschmidt @ 2010-12-30 10:45 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Jeremy Fitzhardinge, linuxppc-dev, Nick Piggin, linux-kernel
In-Reply-To: <alpine.LSU.2.00.1012291413360.22939@sister.anvils>

On Wed, 2010-12-29 at 14:54 -0800, Hugh Dickins wrote:
> With recent 2.6.37-rc, with CONFIG_PREEMPT=y CONFIG_DEBUG_PREEMPT=y
> on the PowerPC G5, I get spammed by BUG warnings each time I swapoff:
> 
> BUG: using smp_processor_id() in preemptible [00000000] code: swapoff/3974
> caller is .hpte_need_flush+0x4c/0x2e8
> Call Trace:
> [c0000001b4a3f830] [c00000000000f3cc] .show_stack+0x6c/0x16c (unreliable)
> [c0000001b4a3f8e0] [c00000000023eda0] .debug_smp_processor_id+0xe4/0x11c
> [c0000001b4a3f970] [c00000000002f2f4] .hpte_need_flush+0x4c/0x2e8
> [c0000001b4a3fa30] [c0000000000e7ef8] .vunmap_pud_range+0x148/0x200
> [c0000001b4a3fb10] [c0000000000e8058] .vunmap_page_range+0xa8/0xd4
> [c0000001b4a3fbb0] [c0000000000e80a4] .free_unmap_vmap_area+0x20/0x38
> [c0000001b4a3fc40] [c0000000000e8138] .remove_vm_area+0x7c/0xb4
> [c0000001b4a3fcd0] [c0000000000e8308] .__vunmap+0x50/0x104
> [c0000001b4a3fd60] [c0000000000ef3fc] .SyS_swapoff+0x59c/0x6a8
> [c0000001b4a3fe30] [c0000000000075a8] syscall_exit+0x0/0x40
> 
> I notice hpte_need_flush() itself acknowledges
>  * Must be called from within some kind of spinlock/non-preempt region...

Yes, we assume that the PTE lock is always held when modifying page
tables...

> Though I didn't actually bisect, I believe this is since Jeremy's
> 64141da587241301ce8638cc945f8b67853156ec "vmalloc: eagerly clear ptes
> on vunmap", which moves a call to vunmap_page_range() from one place
> (which happened to be inside a spinlock) to another (where it's not).
> 
> I guess my warnings would be easily silenced by moving that call to
> vunmap_page_range() down just inside the spinlock below it; but I'm
> dubious that that's the right fix - it looked as if there are other
> paths through vmalloc.c where vunmap_page_range() has been getting
> called without preemption disabled, long before Jeremy's change,
> just paths that I never happen to go down in my limited testing.
> 
> For the moment I'm using the obvious patch below to keep it quiet;
> but I doubt that this is the right patch either.  I'm hoping that
> ye who understand the importance of hpte_need_flush() will be best
> able to judge what to do.  Or might there be other architectures
> expecting to be unpreemptible there?

Well, it looks like our kernel mappings tend to take some nasty
shortcuts with the PTE locking, which I suppose are legit but do break
some of my assumptions there. I need to have a closer look. Thanks for
the report !

Cheers,
Ben.

> Thanks,
> Hugh
> 
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -37,11 +37,13 @@ static void vunmap_pte_range(pmd_t *pmd,
>  {
>  	pte_t *pte;
>  
> +	preempt_disable(); /* Stop __vunmap() triggering smp_processor_id() in preemptible from hpte_need_flush() */
>  	pte = pte_offset_kernel(pmd, addr);
>  	do {
>  		pte_t ptent = ptep_get_and_clear(&init_mm, addr, pte);
>  		WARN_ON(!pte_none(ptent) && !pte_present(ptent));
>  	} while (pte++, addr += PAGE_SIZE, addr != end);
> +	preempt_enable();
>  }
>  
>  static void vunmap_pmd_range(pud_t *pud, unsigned long addr, unsigned long end)

^ permalink raw reply

* Re: [PATCH] spufs: use simple_write_to_buffer
From: Arnd Bergmann @ 2010-12-30 13:04 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: cbe-oss-dev, linux-kernel, Paul Mackerras, Jeremy Kerr,
	linuxppc-dev
In-Reply-To: <1293257039-2580-1-git-send-email-akinobu.mita@gmail.com>

On Saturday 25 December 2010, Akinobu Mita wrote:
> Simplify several write fileoperations for spufs by using
> simple_write_to_buffer().
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Jeremy Kerr <jk@ozlabs.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: cbe-oss-dev@lists.ozlabs.org

Looks good to me.

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ 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