LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* OT: Strange delays / what usually happens every 10 min?
From: Florian Boelstler @ 2007-11-08 23:39 UTC (permalink / raw)
  To: linuxppc-embedded

Hi,

I am currently working on a MPC8540-based custom board, which runs Linux
2.6.15 (arch/ppc).

I set up a periodically running kernel thread, which is delayed for a
single jiffy using schedule_timeout() in an infinite loop. It is used to
measure delays between invocations of that thread. For measuring the
distance in time the PPC's time base lower half register is used
(obtained using get_cycles() defined in asm/timex.h).

The thread calculates the delay to the previous run and only outputs the
result if a new maximum value has been determined (in respect to all
previous cycles). Further the thread outputs a warning if a very "high"
delay was determined. I.e. a delay greater than 5ms.

While running that test driver a delay of about 10ms _exactly_ occurs
every 10 minutes.

The kernel is configured using CONFIG_HZ=1000 and CONFIG_PREEMPT.
The CCB is at 333MHz, whereas the TBR update rate is 333 MHz / 8, i.e.
41,625 MHz.

Apart of some kernel threads almost all user processes have been killed
during the test. Only SSH and a bash were running.
The kernel comes with compiled in CIFS support, some kernel debugging
features like soft-lockup detection and preemption debugging. I.e. ps
lists the kernel threads ksoftirqd, watchdog, events, khelper, kthread,
kblockd, pdflush, aio, cifsoplockd and cifsdnotifyd.

An appropriate userspace test tool based on nanosleep() determined the
same results like the kernel thread.

I also run tcpdump once to check whether some "random" network activity
might be the reason. While there havn't been any packets matching that
10 minutes interval, I got to know that a nearby Windows machine issues
 ARP requests every 10 minutes (though not matching the 10ms delay).
Which leads to the question whether Linux does something similar in the
network sub system?

Thanks for any idea, I'm lost.

Cheers,

  Florian

^ permalink raw reply

* [PATCH] [POWERPC] Optimize counting distinct entries in the relocation sections
From: Emil Medve @ 2007-11-08 23:36 UTC (permalink / raw)
  To: paulus, rusty, ntl, sfr, behlendorf1, galak, linuxppc-dev,
	linuxppc-embedded
  Cc: Emil Medve

When a module has relocation sections with tens of thousands worth of entries,
counting the distinct/unique entries only (i.e. no duplicates) at load time can
take tens of seconds and up to minutes. The sore point is the count_relocs()
function which is called as part of the architecture specific module loading
processing path:

	-> load_module()			generic
	   -> module_frob_arch_sections()	arch specific
	      -> get_plt_size()		32-bit
	      -> get_stubs_size()	64-bit
		 -> count_relocs()

(Not sure why the relocation tables could contain lots of duplicates and why
they are not trimmed at compile time by the linker. In some test cases, out of
35K relocation entries only 1.5K were distinct/unique)

The previous counting algorithm was having O(n^2) complexity. Basically two
solutions were proposed on the e-mail list: a hash based approach and a sort
based approach

The hash based approach is the fastest (O(n)) but the has it needs additional
memory and for certain corner cases it could take lots of memory due to the
degeneration of the hash. One such proposal was submitted here:

http://ozlabs.org/pipermail/linuxppc-dev/2007-June/037641.html

In this proposal, the symbol + addendum are hashed to generate a key and a
pointer to the relocation entry will be stored in it. The hash is implemented as
a linked list of memory pages with PAGE_SIZE / sizeof(Elfxx_Rela *) entries. In
case of collisions in all the existing pages, a new page is added to the list to
accommodate the new distinct relocation entry

For 32-bit PowerPCs with 4K pages, a page can accommodate 1K worth of pointers
to relocation entries. In the 35K entries scenario, as much/little of six (6)
pages could be allocated using 24K of extra memory during the module load

The sort based approach is slower (O(n * log n + n)) but if the sorting is done
"in place" it doesn't need additional memory. A proposal was submitted here:

http://ozlabs.org/pipermail/linuxppc-dev/2007-November/045854.html
(http://patchwork.ozlabs.org/linuxppc/patch?filter=default&id=14573)

In this proposal an array of pointers to the relocation entries is built and
then is sorted using the kernel sort() utility function. This is basically a heap
sort algorithm with a stable O(n * log n) complexity. With this counting the
distinct/unique entries is just linear (O(n)) complexity. The problem is the
extra memory needed in this proposal, which in the 35K relocation entries test
case it can be as much as 140K (for 32-bit PowerPCs; double for 64-bit). This is
much more then the memory needed by the hash based approach described
above/earlier but it doesn't hide potential degenerative corner cases

The current patch is a happy compromise between the two proposals above:
O(n + n * log n) performance with no additional memory requirements due to
sorting in place the relocation table itself

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---

The style errors below are false positives because checkpatch understands the
Elfxx_Rela as operands instead of types

powerpc> scripts/checkpatch.pl 0001-POWERPC-Optimize-counting-distinct-entries-in-the.patch 
ERROR: need consistent spacing around '*' (ctx:WxV)
#84: FILE: arch/powerpc/kernel/module_32.c:56:
+static uint32_t count_relocs(const Elf32_Rela *rela, uint32_t num)
                                               ^

ERROR: need consistent spacing around '*' (ctx:WxV)
#118: FILE: arch/powerpc/kernel/module_32.c:77:
+       const Elf32_Rela *x, *y;
                         ^

ERROR: need consistent spacing around '*' (ctx:WxV)
#190: FILE: arch/powerpc/kernel/module_64.c:83:
+static uint64_t count_relocs(const Elf64_Rela *rela, uint64_t num)
                                               ^

ERROR: need consistent spacing around '*' (ctx:WxV)
#235: FILE: arch/powerpc/kernel/module_64.c:124:
+       const Elf64_Rela *x, *y;
                         ^

total: 4 errors, 0 warnings, 0 checks, 225 lines checked
Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

 arch/powerpc/kernel/module_32.c |   78 +++++++++++++++++++++++++++++-------
 arch/powerpc/kernel/module_64.c |   85 ++++++++++++++++++++++++++++++--------
 2 files changed, 130 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/kernel/module_32.c b/arch/powerpc/kernel/module_32.c
index 07a89a3..866637a 100644
--- a/arch/powerpc/kernel/module_32.c
+++ b/arch/powerpc/kernel/module_32.c
@@ -24,6 +24,7 @@
 #include <linux/kernel.h>
 #include <linux/cache.h>
 #include <linux/bug.h>
+#include <linux/sort.h>
 
 #include "setup.h"
 
@@ -52,24 +53,61 @@ void module_free(struct module *mod, void *module_region)
 
 /* Count how many different relocations (different symbol, different
    addend) */
-static unsigned int count_relocs(const Elf32_Rela *rela, unsigned int num)
+static uint32_t count_relocs(const Elf32_Rela *rela, uint32_t num)
 {
-	unsigned int i, j, ret = 0;
-
-	/* Sure, this is order(n^2), but it's usually short, and not
-           time critical */
-	for (i = 0; i < num; i++) {
-		for (j = 0; j < i; j++) {
-			/* If this addend appeared before, it's
-                           already been counted */
-			if (ELF32_R_SYM(rela[i].r_info)
-			    == ELF32_R_SYM(rela[j].r_info)
-			    && rela[i].r_addend == rela[j].r_addend)
-				break;
+	int i;
+	uint32_t r_info, r_addend, _count_relocs;
+
+	_count_relocs = 0;
+	r_info = 0;
+	r_addend = 0;
+	for (i = 0; i < num; i++)
+		if (r_info != ELF32_R_SYM(rela[i].r_info) ||
+		   r_addend != rela[i].r_addend) {
+			_count_relocs++;
+			r_info = ELF32_R_SYM(rela[i].r_info);
+			r_addend = rela[i].r_addend;
 		}
-		if (j == i) ret++;
+
+	return _count_relocs;
+}
+
+static int relacmp(const void *_x, const void *_y)
+{
+	const Elf32_Rela *x, *y;
+
+	y = (Elf32_Rela *)_x;
+	x = (Elf32_Rela *)_y;
+
+	/* Compare the entire r_info (as opposed to ELF32_R_SYM(r_info) only) to
+	 * make the comparison cheaper/faster. It won't affect the sorting or
+	 * the counting algorithms' performance
+	 */
+	if (x->r_info < y->r_info)
+		return -1;
+	else if (x->r_info > y->r_info)
+		return 1;
+	else if (x->r_addend < y->r_addend)
+		return -1;
+	else if (x->r_addend > y->r_addend)
+		return 1;
+	else
+		return 0;
+}
+
+static void relaswap(void *_x, void *_y, int size)
+{
+	uint32_t *x, *y, tmp;
+	int i;
+
+	y = (uint32_t *)_x;
+	x = (uint32_t *)_y;
+
+	for (i = 0; i < sizeof(Elf32_Rela) / sizeof(uint32_t); i++) {
+		tmp = x[i];
+		x[i] = y[i];
+		y[i] = tmp;
 	}
-	return ret;
 }
 
 /* Get the potential trampolines size required of the init and
@@ -100,6 +138,16 @@ static unsigned long get_plt_size(const Elf32_Ehdr *hdr,
 			DEBUGP("Ptr: %p.  Number: %u\n",
 			       (void *)hdr + sechdrs[i].sh_offset,
 			       sechdrs[i].sh_size / sizeof(Elf32_Rela));
+
+			/* Sort the relocation information based on a symbol and
+			 * addend key. This is a stable O(n*log n) complexity
+			 * alogrithm but it will reduce the complexity of
+			 * count_relocs() to linear complexity O(n)
+			 */
+			sort((void *)hdr + sechdrs[i].sh_offset,
+			     sechdrs[i].sh_size / sizeof(Elf32_Rela),
+			     sizeof(Elf32_Rela), relacmp, relaswap);
+
 			ret += count_relocs((void *)hdr
 					     + sechdrs[i].sh_offset,
 					     sechdrs[i].sh_size
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 75c7c4f..2ffe43f 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -24,6 +24,7 @@
 #include <asm/module.h>
 #include <asm/uaccess.h>
 #include <asm/firmware.h>
+#include <linux/sort.h>
 
 #include "setup.h"
 
@@ -79,27 +80,27 @@ static struct ppc64_stub_entry ppc64_stub =
 
 /* Count how many different 24-bit relocations (different symbol,
    different addend) */
-static unsigned int count_relocs(const Elf64_Rela *rela, unsigned int num)
+static uint64_t count_relocs(const Elf64_Rela *rela, uint64_t num)
 {
-	unsigned int i, j, ret = 0;
+	int i;
+	uint64_t r_info, r_addend, _count_relocs;
 
 	/* FIXME: Only count external ones --RR */
-	/* Sure, this is order(n^2), but it's usually short, and not
-           time critical */
-	for (i = 0; i < num; i++) {
+	_count_relocs = 0;
+	r_info = 0;
+	r_addend = 0;
+	for (i = 0; i < num; i++)
 		/* Only count 24-bit relocs, others don't need stubs */
-		if (ELF64_R_TYPE(rela[i].r_info) != R_PPC_REL24)
-			continue;
-		for (j = 0; j < i; j++) {
-			/* If this addend appeared before, it's
-                           already been counted */
-			if (rela[i].r_info == rela[j].r_info
-			    && rela[i].r_addend == rela[j].r_addend)
-				break;
+		if (ELF64_R_TYPE(rela[i].r_info) == R_PPC_REL24) {
+			if (r_info != ELF64_R_SYM(rela[i].r_info) ||
+			   r_addend != rela[i].r_addend) {
+				_count_relocs++;
+				r_info = ELF64_R_SYM(rela[i].r_info);
+				r_addend = rela[i].r_addend;
+			}
 		}
-		if (j == i) ret++;
-	}
-	return ret;
+
+	return _count_relocs;
 }
 
 void *module_alloc(unsigned long size)
@@ -118,6 +119,44 @@ void module_free(struct module *mod, void *module_region)
            table entries. */
 }
 
+static int relacmp(const void *_x, const void *_y)
+{
+	const Elf64_Rela *x, *y;
+
+	y = (Elf64_Rela *)_x;
+	x = (Elf64_Rela *)_y;
+
+	/* Compare the entire r_info (as opposed to ELF64_R_SYM(r_info) only) to
+	 * make the comparison cheaper/faster. It won't affect the sorting or
+	 * the counting algorithms' performance
+	 */
+	if (x->r_info < y->r_info)
+		return -1;
+	else if (x->r_info > y->r_info)
+		return 1;
+	else if (x->r_addend < y->r_addend)
+		return -1;
+	else if (x->r_addend > y->r_addend)
+		return 1;
+	else
+		return 0;
+}
+
+static void relaswap(void *_x, void *_y, int size)
+{
+	uint64_t *x, *y, tmp;
+	int i;
+
+	y = (uint64_t *)_x;
+	x = (uint64_t *)_y;
+
+	for (i = 0; i < sizeof(Elf64_Rela) / sizeof(uint64_t); i++) {
+		tmp = x[i];
+		x[i] = y[i];
+		y[i] = tmp;
+	}
+}
+
 /* Get size of potential trampolines required. */
 static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
 				    const Elf64_Shdr *sechdrs)
@@ -133,6 +172,16 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
 			DEBUGP("Ptr: %p.  Number: %lu\n",
 			       (void *)sechdrs[i].sh_addr,
 			       sechdrs[i].sh_size / sizeof(Elf64_Rela));
+
+			/* Sort the relocation information based on a symbol and
+			 * addend key. This is a stable O(n*log n) complexity
+			 * alogrithm but it will reduce the complexity of
+			 * count_relocs() to linear complexity O(n)
+			 */
+			sort((void *)sechdrs[i].sh_addr,
+			     sechdrs[i].sh_size / sizeof(Elf64_Rela),
+			     sizeof(Elf64_Rela), relacmp, relaswap);
+
 			relocs += count_relocs((void *)sechdrs[i].sh_addr,
 					       sechdrs[i].sh_size
 					       / sizeof(Elf64_Rela));
@@ -343,7 +392,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 			/* Simply set it */
 			*(u32 *)location = value;
 			break;
-			
+
 		case R_PPC64_ADDR64:
 			/* Simply set it */
 			*(unsigned long *)location = value;
@@ -399,7 +448,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 			}
 
 			/* Only replace bits 2 through 26 */
-			*(uint32_t *)location 
+			*(uint32_t *)location
 				= (*(uint32_t *)location & ~0x03fffffc)
 				| (value & 0x03fffffc);
 			break;
-- 
1.5.3.GIT

^ permalink raw reply related

* Re: [PATCH 3/4] Use embedded libfdt in the bootwrapper
From: David Gibson @ 2007-11-08 23:29 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <473392A3.4010203@freescale.com>

On Thu, Nov 08, 2007 at 04:50:11PM -0600, Scott Wood wrote:
> David Gibson wrote:
> >> How hard would it be to get libfdt to dynamically allocate any extra space
> >> it needs?  This is a regression from the current flat device tree code...
> > 
> > Uh.. it already does.  Or rather, the shims in libfdt-wrapper.c do so,
> > when libfdt functions which can expand the tree report that they've
> > run out of room.
> 
> Ah, good -- I was looking in libfdt itself, not the wrapper.  Now if 
> only we could get something similar into u-boot... maybe libfdt proper 
> could accept an optional realloc() function pointer in fdt_init(), and 
> eliminate the need for the caller to provide such a wrapper?

I've considered something like it (more likely an optional
realloc()ing wrapper layer that comes with libfdt).

For the bootwrapper itself, however, I'm still hoping to get rid of
malloc() entirely...

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [RFC] powermac: proper sleep management
From: Benjamin Herrenschmidt @ 2007-11-08 23:19 UTC (permalink / raw)
  To: Scott Wood
  Cc: linuxppc-dev list, Johannes Berg, David Woodhouse, linux-pm,
	Paul Mackerras
In-Reply-To: <47336035.4020609@freescale.com>


On Thu, 2007-11-08 at 13:15 -0600, Scott Wood wrote:
> Johannes Berg wrote:
> > +/*
> > + * overrides the weak arch_suspend_disable_irqs in kernel/power/main.c
> > + *
> > + * XXX: Once Scott Wood's patch is merged, this needs to use the ppc_md
> > + *	hooks that patch adds!
> > + */
> > +void arch_suspend_disable_irqs(void)
> > +{
> > +#ifdef CONFIG_PMAC_BACKLIGHT
> > +	/* Tell backlight code not to muck around with the chip anymore */
> > +	pmu_backlight_set_sleep(1);
> > +#endif
>  > +
> > +	/* Call platform functions marked "on sleep" */
> > +	pmac_pfunc_i2c_suspend();
> > +	pmac_pfunc_base_suspend();
> 
> Shouldn't these be done from suspend methods of the relevant drivers?
> I don't understand why this needs to go in the disable IRQ hook.

The pmac_pfunc thing is low level platform stuff, no driver involved
there, this is the right place to do it.

As for the backlight bits, that could indeed be moved around I suppose,
I'd keep it there for now and look at cleaning the PMU driver
suspend/resume path in a second step.

Ben.

^ permalink raw reply

* Re: Kernel locks up after calling kernel_execve()
From: Benjamin Herrenschmidt @ 2007-11-08 23:20 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev
In-Reply-To: <20071108214723.135260@gmx.net>


On Thu, 2007-11-08 at 22:47 +0100, Gerhard Pircher wrote:
> Hi,
> 
> I tested my patches for the AmigaOne platform with the lastest 2.6.24-rc2
> kernel snapshot. The kernel runs through all initcalls, but locks up
> completely after calling INIT (/sbin/init) by kernel_execve(). Thus I
> couldn't capture any kernel oops or panic output. Also the magic sysrq
> key doesn't work. Enabling debug code for soft lockups and spinlock
> debugging didn't reveal any information.
> I'm not sure, but I think it is the same problem I had with all kernels
> >= 2.6.17. All of these kernels lock up shortly before or right at calling
> the init program (resp. as soon as the kernel forks some kernel theads).
> Any suggestions on how to track down this problem?

You don't have a HW debugger or anything like that ?

Ben.

^ permalink raw reply

* Re: [PATCH 3/4] Use embedded libfdt in the bootwrapper
From: Scott Wood @ 2007-11-08 22:50 UTC (permalink / raw)
  To: Scott Wood, linuxppc-dev, Paul Mackerras
In-Reply-To: <20071108224051.GB18592@localhost.localdomain>

David Gibson wrote:
>> How hard would it be to get libfdt to dynamically allocate any extra space
>> it needs?  This is a regression from the current flat device tree code...
> 
> Uh.. it already does.  Or rather, the shims in libfdt-wrapper.c do so,
> when libfdt functions which can expand the tree report that they've
> run out of room.

Ah, good -- I was looking in libfdt itself, not the wrapper.  Now if 
only we could get something similar into u-boot... maybe libfdt proper 
could accept an optional realloc() function pointer in fdt_init(), and 
eliminate the need for the caller to provide such a wrapper?

-Scott

^ permalink raw reply

* Re: [PATCH 3/4] Use embedded libfdt in the bootwrapper
From: David Gibson @ 2007-11-08 22:40 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20071108160839.GA4356@loki.buserror.net>

On Thu, Nov 08, 2007 at 10:08:39AM -0600, Scott Wood wrote:
> On Thu, Nov 08, 2007 at 02:36:03PM +1100, David Gibson wrote:
> > This patch incorporates libfdt (from the source embedded in an earlier
> > patch) into the wrapper.a library used by the bootwrapper.  This
> > includes adding a libfdt_env.h file, which the libfdt sources need in
> > order to integrate into the bootwrapper environment, and a
> > libfdt-wrapper.c which provides glue to connect the bootwrappers
> > abstract device tree callbacks to the libfdt functions.
> > 
> > In addition, this patch changes the various wrapper and platform files
> > to use libfdt functions instead of the older flatdevtree.c library.
> 
> Won't we need to change the dtc invocation in the wrapper to reserve some
> space now?
> 
> Speaking of which, it seems dtc still only supports setting the total size,
> as opposed to specifying the amount of additional space.  It's still a bit
> crappy having to guess how much space to add, but it's better than needing
> to know how big the dtb will be without additional space.
> 
> How hard would it be to get libfdt to dynamically allocate any extra space
> it needs?  This is a regression from the current flat device tree code...

Uh.. it already does.  Or rather, the shims in libfdt-wrapper.c do so,
when libfdt functions which can expand the tree report that they've
run out of room.

Ah... except that I haven't properly placed an fdt_open_into() with
possible expansion somewhere to make sure we can handle v16 or dtbs
with the blocks in unusual order.

I'll need to fix that before we merge.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Scott Wood @ 2007-11-08 22:30 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-embedded
In-Reply-To: <9e4733910711081417h7a571398md59a8987f15423a6@mail.gmail.com>

Jon Smirl wrote:
> On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
>> Several hundred?  I don't think there are several hundred unique
>> property names or compatible entries in arch/powerpc/boot/dts *total*,
> 
> There are 601 'compatible' attributes in the directory.

I said "unique".  And there do appear to be a couple hundred of those, 
though if I omit those containing vendor prefixes, it drops to under a 
hundred.  A big chunk of *those* are 5200-related. :-)

-Scott

^ permalink raw reply

* Re: [PATCH] powerpc: mpc8xxx MDS board RTC fixes
From: David Gibson @ 2007-11-08 22:30 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <47336CE7.5070807@freescale.com>

On Thu, Nov 08, 2007 at 02:09:11PM -0600, Scott Wood wrote:
> Kumar Gala wrote:
> > should the mpc8568mds.dts be updated?
> 
> It already has been.
> 
> > Also, what code is parsing the device tree for the rtc node?
> 
> There's code in fsl_soc.c (which should be made more generic).

Indeed.  I guess we're going to want a library function which any
device tree aware i2c-bridge driver can use to instantiate the right
i2c-layer data structures for all the bridge's subnodes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Jon Smirl @ 2007-11-08 22:17 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-embedded
In-Reply-To: <473387AF.6030105@freescale.com>

On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
> Jon Smirl wrote:
> > On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
> >>> As far as I know the only dts using vendor prefixes in the compatible
> >>> attributes is 5200lite one for the gpt entries. Everything else will
> >>> need to be changed.
> >> Look a little harder.  Many of the Freescale boards/devices do it right
> >> -- and yes, we need to fix the ones that don't.
> >
> > I see that there are quite a few with the ibm prefix. But by
> > eyeballing grep output it looks like several hundred attributes need
> > to be fixed. Code referencing these will need to be fixed too.
>
> Several hundred?  I don't think there are several hundred unique
> property names or compatible entries in arch/powerpc/boot/dts *total*,

There are 601 'compatible' attributes in the directory.

> much less that need fixing.  The main ones in fsl-land that I can think
> of are gianfar, i2c, talitos, QE, USB, tsi108, watchdog, SPI, and IPIC.
>
> CPM1 and CPM2 devices, PCI, 85xx memory/cache controllers, and 85xx
> global utilities have the prefix.
>
> For the most part, we've been adding prefixes whenever we have any other
> reason to touch the binding.
>
> > If lite5200 hadn't mixed half prefixed and half not prefixed I never
> > would have started looking at this.
> >
> > Let's make up our minds in the lite5200 dts so that I can sync up my
> > development hardware.
>
> Patches are welcome. :-)
>
> -Scott
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Scott Wood @ 2007-11-08 22:03 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-embedded
In-Reply-To: <9e4733910711081353h43ec8978u2db4618090688010@mail.gmail.com>

Jon Smirl wrote:
> On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
>>> As far as I know the only dts using vendor prefixes in the compatible
>>> attributes is 5200lite one for the gpt entries. Everything else will
>>> need to be changed.
>> Look a little harder.  Many of the Freescale boards/devices do it right
>> -- and yes, we need to fix the ones that don't.
> 
> I see that there are quite a few with the ibm prefix. But by
> eyeballing grep output it looks like several hundred attributes need
> to be fixed. Code referencing these will need to be fixed too.

Several hundred?  I don't think there are several hundred unique 
property names or compatible entries in arch/powerpc/boot/dts *total*, 
much less that need fixing.  The main ones in fsl-land that I can think 
of are gianfar, i2c, talitos, QE, USB, tsi108, watchdog, SPI, and IPIC.

CPM1 and CPM2 devices, PCI, 85xx memory/cache controllers, and 85xx 
global utilities have the prefix.

For the most part, we've been adding prefixes whenever we have any other 
reason to touch the binding.

> If lite5200 hadn't mixed half prefixed and half not prefixed I never
> would have started looking at this.
> 
> Let's make up our minds in the lite5200 dts so that I can sync up my
> development hardware.

Patches are welcome. :-)

-Scott

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Jon Smirl @ 2007-11-08 21:53 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-embedded
In-Reply-To: <473377F8.2050802@freescale.com>

On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
> > As far as I know the only dts using vendor prefixes in the compatible
> > attributes is 5200lite one for the gpt entries. Everything else will
> > need to be changed.
>
> Look a little harder.  Many of the Freescale boards/devices do it right
> -- and yes, we need to fix the ones that don't.

I see that there are quite a few with the ibm prefix. But by
eyeballing grep output it looks like several hundred attributes need
to be fixed. Code referencing these will need to be fixed too.

If lite5200 hadn't mixed half prefixed and half not prefixed I never
would have started looking at this.

Let's make up our minds in the lite5200 dts so that I can sync up my
development hardware.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Kernel locks up after calling kernel_execve()
From: Gerhard Pircher @ 2007-11-08 21:47 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

I tested my patches for the AmigaOne platform with the lastest 2.6.24-rc2
kernel snapshot. The kernel runs through all initcalls, but locks up
completely after calling INIT (/sbin/init) by kernel_execve(). Thus I
couldn't capture any kernel oops or panic output. Also the magic sysrq
key doesn't work. Enabling debug code for soft lockups and spinlock
debugging didn't reveal any information.
I'm not sure, but I think it is the same problem I had with all kernels
>= 2.6.17. All of these kernels lock up shortly before or right at calling
the init program (resp. as soon as the kernel forks some kernel theads).
Any suggestions on how to track down this problem?

regards,

Gerhard

-- 
Ist Ihr Browser Vista-kompatibel? Jetzt die neuesten 
Browser-Versionen downloaden: http://www.gmx.net/de/go/browser

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Scott Wood @ 2007-11-08 20:56 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-embedded
In-Reply-To: <9e4733910711081251w392b4a84m8e81d902305bf395@mail.gmail.com>

Jon Smirl wrote:
> On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
>> Jon Smirl wrote:
>>> No one has answered this yet. It makes no sense at all to mix use of
>>> the vendor prefix on some compatible entries and not on others. The
>>> syntax of compatible entries needs to be consistent.
>> Right, the vendor prefix should always be used.  Not all of the existing
>> trees are perfect, however, so sometimes the code needs to stay
>> compatible, especially when the device tree is difficult to change.
> 
> A bunch of patches are going to have to go into the kernel. Code
> expecting tree attributes without vendor prefixes is all over the
> kernel.

It's just nonstandard properties that need it, not all of them.

> All of the dts file need to be patched up, etc.

I don't think *all* the dts files have problems...

> As far as I know the only dts using vendor prefixes in the compatible
> attributes is 5200lite one for the gpt entries. Everything else will
> need to be changed.

Look a little harder.  Many of the Freescale boards/devices do it right 
-- and yes, we need to fix the ones that don't.

-Scott

^ permalink raw reply

* Re: [PATCH 2/6] ibm_newemac: Add ET1011c PHY support
From: Benjamin Herrenschmidt @ 2007-11-08 20:55 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <20071108060805.4c59b579@zod.rchland.ibm.com>


On Thu, 2007-11-08 at 06:08 -0600, Josh Boyer wrote:
> > Agreed, I though I had removed them, looks like I didn't. Will fix
> that
> > before submitting.
> 
> DENX is pretty good about having Signed-off-by lines in their tree...
> maybe you should add the original authors as well if it's there.

I picked it up from an already patched tree. Stefan, can you provide me
the proper SOB ?

Thanks,
Ben.

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Jon Smirl @ 2007-11-08 20:51 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-embedded
In-Reply-To: <473375E6.5080503@freescale.com>

On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
> Jon Smirl wrote:
> > No one has answered this yet. It makes no sense at all to mix use of
> > the vendor prefix on some compatible entries and not on others. The
> > syntax of compatible entries needs to be consistent.
>
> Right, the vendor prefix should always be used.  Not all of the existing
> trees are perfect, however, so sometimes the code needs to stay
> compatible, especially when the device tree is difficult to change.

A bunch of patches are going to have to go into the kernel. Code
expecting tree attributes without vendor prefixes is all over the
kernel. All of the dts file need to be patched up, etc.

As far as I know the only dts using vendor prefixes in the compatible
attributes is 5200lite one for the gpt entries. Everything else will
need to be changed.


>
> -Scott
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Grant Likely @ 2007-11-08 20:50 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-embedded
In-Reply-To: <9e4733910711081240l458a5e7h4898fb12e511f29a@mail.gmail.com>

On 11/8/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> No one has answered this yet. It makes no sense at all to mix use of
> the vendor prefix on some compatible entries and not on others. The
> syntax of compatible entries needs to be consistent.

The 5200 binding is poorly done on this point.  I've got a patch that
adds the 'fsl,' prefix so that future 5200 device trees are consistent
with the recommended practices (while not breaking older boards of
course)

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Scott Wood @ 2007-11-08 20:47 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-embedded
In-Reply-To: <9e4733910711081240l458a5e7h4898fb12e511f29a@mail.gmail.com>

Jon Smirl wrote:
> No one has answered this yet. It makes no sense at all to mix use of
> the vendor prefix on some compatible entries and not on others. The
> syntax of compatible entries needs to be consistent.

Right, the vendor prefix should always be used.  Not all of the existing 
trees are perfect, however, so sometimes the code needs to stay 
compatible, especially when the device tree is difficult to change.

-Scott

^ permalink raw reply

* Re: [PATCH] hugetlb: follow_hugetlb_page for write access
From: Ken Chen @ 2007-11-08 20:43 UTC (permalink / raw)
  To: Adam Litke; +Cc: linuxppc-dev, Andrew Morton, linux-kernel, Hoang-Nam Nguyen
In-Reply-To: <20071107195142.13505.49398.stgit@kernel>

On Nov 7, 2007 11:51 AM, Adam Litke <agl@us.ibm.com> wrote:
> When calling get_user_pages(), a write flag is passed in by the caller to
> indicate if write access is required on the faulted-in pages.  Currently,
> follow_hugetlb_page() ignores this flag and always faults pages for
> read-only access.  This can cause data corruption because a device driver
> that calls get_user_pages() with write set will not expect COW faults to
> occur on the returned pages.
>
> This patch passes the write flag down to follow_hugetlb_page() and makes
> sure hugetlb_fault() is called with the right write_access parameter.
>
> Signed-off-by: Adam Litke <agl@us.ibm.com>

Adam, this looks good.

Reviewed-by: Ken Chen <kenchen@google.com>

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Jon Smirl @ 2007-11-08 20:40 UTC (permalink / raw)
  To: Matt Sealey; +Cc: linuxppc-embedded
In-Reply-To: <9e4733910711071418u5914f188nffc7ab50e62cc88e@mail.gmail.com>

No one has answered this yet. It makes no sense at all to mix use of
the vendor prefix on some compatible entries and not on others. The
syntax of compatible entries needs to be consistent.

On 11/7/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 11/7/07, Matt Sealey <matt@genesi-usa.com> wrote:
> > Jon Smirl wrote:
> > > Sometimes the fsl prefix is being used and sometimes it isn't. Look at
> > > the two compatible strings. Which way is it going to be? Is
> > > fsl,has-wdt right?
> >
> > fsl,has-wdt is right, at least since someone changed it.
>
> What's the story with compatible?
> I would think the gdt entries are wrong.
>
>                gpt@600 {       // General Purpose Timer
>                        compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
>
> code in drivers/watchdog/mpc5200_wdt.c would need to be changed to.
>
> drivers/watchdog/mpc5200_wdt.c: { .compatible = "fsl,mpc5200-gpt", },
>
> --
> Jon Smirl
> jonsmirl@gmail.com
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: use of fsl, in lite5200b.dts in git current
From: Jon Smirl @ 2007-11-08 20:39 UTC (permalink / raw)
  To: Matt Sealey; +Cc: linuxppc-embedded
In-Reply-To: <47336805.4040807@genesi-usa.com>

On 11/8/07, Matt Sealey <matt@genesi-usa.com> wrote:
> Jon Smirl wrote:
> > On 11/8/07, Scott Wood <scottwood@freescale.com> wrote:
> >
> >> I think you may be placing too much faith in the vendors.
> >> Is a 7400 a Freescale powerpc chip, or a quad 2-input NAND gate? :-)
> >
> > There has to be more to the part number for the Freescale powerpc chip
> > than just 7400. 7400 is a shorthand name, it is not an orderable part number.
>
> The orderable part numbers add 3 or 4 characters to the front and about
> 8 after. There is a difference between MPC7400 and PPC7400, and the
> low voltage versions, and the different clock speeds. Orderable part
> number for a recent G4 might be PPC7448B1333NL - this is a ridiculous
> amount of specificity in a device tree, and it also does not match the
> datasheet (MPC7448 is the name of the chip).
>
> What people usually do is use what's in the datasheet.

Data sheet part number is fine, you don't need all that specificity.
The point is that the data sheet part number is sufficient, there is
no need for a vendor prefix. And these vendor prefixes can and do end
up being wrong when the chips families get sold. The datasheet part
numbers are maintained when a product line is sold.


>
> >> If you want to argue that the "MPC" part differentiates them, that's
> >> just a less readable and more obsolete vendor prefix.
> >
> > The MPC is what is printed on the chip. fsl is not printed there. MPC
> > is part of the orderable part number.
>
> Not all of them *ahem* :)
>
> Like I said, trust the datasheet, not the number on the chip.
>
> >> Vendor prefixes on properties are useful in that it might not mean
> >> exactly the same thing as a similar property that gets standardized
> >> later on.
> >>
> >>> That's life in the Linux world, no backwards binary compatibility.
> >> There's a huge difference between compatibility of kernel interfaces and
> >> compatibility of interfaces between the kernel and something else --
> >> whether it be userspace or firmware.
>
> Indeed, so.. at some point we should all sit down and hammer out the
> major issues in describing something like the MPC5121E because right
> now Genesi has a vested interest in that. Thanks Grant for your
> discussion on it, I agree of course :)
>
> One thing we don't want to go through again is the complaints we got
> because we named the chip node "/builtin" rather than "/soc". My fixup
> script is still handling that mess because you guys refused to
> accept it (and some drivers were coded to map from the MBAR contained
> in device_type soc's reg property rather than find a real device).
>
> If we could all agree on how it should be mapped out, with an example
> tree which shows *every damn thing available* so platform developers
> can pick and choose and OF developers can use it as a reference, it
> would make a much happier process.
>
> And then we can fix up the Efika to fit some definition of the new
> MPC5200 tree too.
>
> By the way while I was poking around the tree today I noticed that
> there is a PCI errata fixup handled by a Kconfig in there. Why? Surely
> this is something you check the PVR/SVR for and switch on that for
> a runtime solution, and not trick users with the possibility of
> forgetting to enable some obscure "PCI errata fix" configuration
> item? (CONFIG_PPC_MPC5200_BUGFIX)
>
> --
> Matt Sealey <matt@genesi-usa.com>
> Genesi, Manager, Developer Relations
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH] powerpc: mpc8xxx MDS board RTC fixes
From: Kim Phillips @ 2007-11-08 20:33 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <F67D40A1-D8C9-4F96-A526-E848FDFDEA0B@kernel.crashing.org>

On Thu, 8 Nov 2007 13:57:59 -0600
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Nov 8, 2007, at 1:37 PM, Kim Phillips wrote:
> 
> > Now the rtc class ds1374 driver has been added,
> > remove the old rtc driver hookup code, add rtc node
> > to device trees, and turn on the new driver in the defconfigs.
> >
> > Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
> > ---
> >  arch/powerpc/boot/dts/mpc832x_mds.dts      |    7 ++++
> >  arch/powerpc/boot/dts/mpc834x_mds.dts      |    9 +++++
> >  arch/powerpc/boot/dts/mpc836x_mds.dts      |    9 +++++
> >  arch/powerpc/configs/mpc832x_mds_defconfig |   48 +++++++++++++++++ 
> > ++++++++++-
> >  arch/powerpc/configs/mpc834x_mds_defconfig |   48 +++++++++++++++++ 
> > ++++++++++-
> >  arch/powerpc/configs/mpc836x_mds_defconfig |   48 +++++++++++++++++ 
> > ++++++++++-
> >  arch/powerpc/configs/mpc8568mds_defconfig  |   48 +++++++++++++++++ 
> > ++++++++++-
> >  arch/powerpc/platforms/83xx/mpc832x_mds.c  |   24 --------------
> >  arch/powerpc/platforms/83xx/mpc834x_mds.c  |   24 --------------
> >  arch/powerpc/platforms/83xx/mpc836x_mds.c  |   24 --------------
> >  10 files changed, 213 insertions(+), 76 deletions(-)
> 
> should the mpc8568mds.dts be updated?

it already has the rtc node.

> Also, what code is parsing the device tree for the rtc node?
> 
the i2c code in fsl_soc.c.

Kim

^ permalink raw reply

* Re: [PATCH] using mii-bitbang on different processor ports - update the booting-without-of.txt-file
From: Scott Wood @ 2007-11-08 20:20 UTC (permalink / raw)
  To: Sergej Stepanov; +Cc: linuxppc-dev, jgarzik, netdev
In-Reply-To: <1194442844.3571.14.camel@p60635-ste.ids.de>

Sergej Stepanov wrote:
> If both mdio and mdc controlling pins are on the same processor port,
> one resource should be used.
> Otherwise, two resources are used: the 1-st - mdio, the 2-nd - mdc.

How about:
The first reg resource is the I/O port register block on which MDIO 
resides.  The second reg resource is the I/O port register block on 
which MDC resides.  If there is only one reg resource, it is used for 
both MDIO and MDC.

We also need to change the reference to port C in fsl,mdio-pin and 
fsl,mdc-pin.

-Scott

^ permalink raw reply

* Re: [RFC] powermac: proper sleep management
From: Scott Wood @ 2007-11-08 19:15 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linuxppc-dev list, linux-pm, David Woodhouse, Paul Mackerras
In-Reply-To: <1194523729.6294.18.camel@johannes.berg>

Johannes Berg wrote:
> +/*
> + * overrides the weak arch_suspend_disable_irqs in kernel/power/main.c
> + *
> + * XXX: Once Scott Wood's patch is merged, this needs to use the ppc_md
> + *	hooks that patch adds!
> + */
> +void arch_suspend_disable_irqs(void)
> +{
> +#ifdef CONFIG_PMAC_BACKLIGHT
> +	/* Tell backlight code not to muck around with the chip anymore */
> +	pmu_backlight_set_sleep(1);
> +#endif
 > +
> +	/* Call platform functions marked "on sleep" */
> +	pmac_pfunc_i2c_suspend();
> +	pmac_pfunc_base_suspend();

Shouldn't these be done from suspend methods of the relevant drivers?
I don't understand why this needs to go in the disable IRQ hook.

-Scott

^ permalink raw reply

* RE: [PATCH] powerpc: mpc8xxx MDS board RTC fixes
From: Wang Haiying @ 2007-11-08 20:10 UTC (permalink / raw)
  To: Kumar Gala, Phillips Kim; +Cc: linuxppc-dev
In-Reply-To: <F67D40A1-D8C9-4F96-A526-E848FDFDEA0B@kernel.crashing.org>

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

Yes, 8568mds dts should be updated. And I tested this ds1374 driver on 8568mds board a couple of months ago, it worked fine. Code to parse device tree should be in fsl_soc.c.
 
Haiying

________________________________

From: linuxppc-dev-bounces+haiying.wang=freescale.com@ozlabs.org on behalf of Kumar Gala
Sent: Thu 11/8/2007 12:57 PM
To: Phillips Kim
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] powerpc: mpc8xxx MDS board RTC fixes




On Nov 8, 2007, at 1:37 PM, Kim Phillips wrote:

> Now the rtc class ds1374 driver has been added,
> remove the old rtc driver hookup code, add rtc node
> to device trees, and turn on the new driver in the defconfigs.
>
> Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
> ---
>  arch/powerpc/boot/dts/mpc832x_mds.dts      |    7 ++++
>  arch/powerpc/boot/dts/mpc834x_mds.dts      |    9 +++++
>  arch/powerpc/boot/dts/mpc836x_mds.dts      |    9 +++++
>  arch/powerpc/configs/mpc832x_mds_defconfig |   48 +++++++++++++++++
> ++++++++++-
>  arch/powerpc/configs/mpc834x_mds_defconfig |   48 +++++++++++++++++
> ++++++++++-
>  arch/powerpc/configs/mpc836x_mds_defconfig |   48 +++++++++++++++++
> ++++++++++-
>  arch/powerpc/configs/mpc8568mds_defconfig  |   48 +++++++++++++++++
> ++++++++++-
>  arch/powerpc/platforms/83xx/mpc832x_mds.c  |   24 --------------
>  arch/powerpc/platforms/83xx/mpc834x_mds.c  |   24 --------------
>  arch/powerpc/platforms/83xx/mpc836x_mds.c  |   24 --------------
>  10 files changed, 213 insertions(+), 76 deletions(-)

should the mpc8568mds.dts be updated?

Also, what code is parsing the device tree for the rtc node?

- k


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev



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

^ 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