* Re: [PATCH 3/4] Use embedded libfdt in the bootwrapper
From: Jerry Van Baren @ 2007-11-09 0:52 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <473392A3.4010203@freescale.com>
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?
>
> -Scott
Hi Scott,
My concern from the u-boot side is that u-boot has to know exactly
*where* to put the expanded blob because it has to pass it to linux and
keep it out of linux' way so it doesn't get "stepped on." Linux has an
advantage in that it "owns" all of memory and can allocate and
deallocate whatever and wherever and it won't step on itself (hopefully).
I'm assuming your boot wedge has the advantage of being able to use
linux's malloc() and thus don't have to worry about coordinating with
linux on memory allocation.
With the current u-boot fdt command, you can resize the blob, and this
can be done in a script with all the (somewhat limited) capabilities of
the u-boot shell (an adaptation of hush).
In the u-boot world, we fixate on memory maps and putting things in
specific places. Maybe I'm making a problem where one doesn't exist,
but an arbitrary malloc() in u-boot (as opposed to linux's malloc) seems
like a Bad Thing[tm] because it is uncontrolled and may end up in a very
bad place when linux starts (e.g. the linux start up script expands
linux on top of the blob).
Best regards,
gvb
^ permalink raw reply
* Re: DTS Bytestrings Representation in /dts-v1/ files
From: David Gibson @ 2007-11-09 0:21 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1IqCtu-0000ZE-Vi@jdl.com>
On Thu, Nov 08, 2007 at 01:18:50PM -0600, Jon Loeliger wrote:
>
> Folks,
>
> When the new DTS /dts-v1/ support is released Real Soon Now,
> it will support C-like literal constants. Hex values will be
> prefixed with 0x, binary with 0b, and bare numbers will be
> decimal unless they start with a leading 0.
>
> One outstanding question on which I'd like some feedback
> is the issue of bytestring value representation.
>
> Currently they look like this:
>
> stuff = [ 0b 31 22 de ea ad be ef ];
Or, equivalently, like this:
stuff = [0b3122deeaadbeef];
I think it's important to be aware of the more compact form when
considering whether to change the representation.
> One opinion is to have them continue to look like that
> and be in hex only.
>
> Another opinion is to allow the new, consistent C-style
> literals and expressions so that one could have:
>
> new_stuff = [ 0x31 49 '1' 23 17 ];
>
> Opinions?
>
> Thanks,
> jdl
--
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: [PATCH] Use SLB size from the device tree
From: Olof Johansson @ 2007-11-09 0:16 UTC (permalink / raw)
To: Michael Neuling; +Cc: linuxppc-dev, paulus, Will Schmidt
In-Reply-To: <510.1194565218@neuling.org>
On Fri, Nov 09, 2007 at 10:40:18AM +1100, Michael Neuling wrote:
> Currently we hardwire the number of SLBs but the PAPR says we export an
> ibm,slb-size property to specify the number of SLB entries. This patch
> uses this property instead of assuming 64 always. If no property is
> found, we assume 64 entries as before.
>
> This soft patches the SLB handler, so it won't change performance at
> all.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
Acked-by: Olof Johansson <olof@lixom.net>
I wonder if it isn't time soon to move all the mmu_.* globals to a
struct. That's a separate issue though.
-Olof
^ permalink raw reply
* Re: [PATCH] DTC: Polish up the DTS Version 1 implementation.
From: David Gibson @ 2007-11-09 0:13 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <47331979.7030803@freescale.com>
On Thu, Nov 08, 2007 at 08:13:13AM -0600, Jon Loeliger wrote:
> David Gibson wrote:
> >
> > Yes, I know, but I don't think it's even worth having the unused
> > internal parameterization.
>
> OK. We can eliminate it then; no problem.
>
> >> I ran my "old versus new DTC" comparison script and found it. :-)
> >
> > Heh, we should fold that into the testsuite, too.
>
> I'll start by simply adding the script to the test directory then.
Ok.
> >> Because it wasn't working, as explained in the comment I added.
> >> Specifically, (1<<bits), with bits==64 overflowed and yielded
> >> the value 0.
> >
> > Ah...
> >
> > Well, I assumed (1ULL << 64) would equal 0, which is why the
> > comparison has the (-1) - I was expecting for bits == 64 it would end
> > up being against -1, i.e. 0xffffffffffffffff. This appears to work on
> > the systems I've been using.
>
> But not on an x86 system.
>
> > But I just remembered that (x << n) has undefined behaviour in C when
> > n >= word size.
>
> Exactly. In fact, I think x86 takes the shift value, bit-wise
> ANDs it with 63 internally, and then shifts left by that value.
>
> So I guess (1 << 64) is just returning garbage - I
>
> In fact, it is yielding 1 on an x86 machine.
>
> > suspect I didn't catch it because I've been building with -O0 for
> > gdb-ability, which might change the behaviour of corner cases like
> > that.
>
> Or works on a PPC machine? :-)
Actually I was working from home on an x86 machine when I wrote that,
so I think it must have been the -O0.
> > So I guess we need
> > else if ((errno == ERANGE)
> > || ((bits < 64) && (val >= (1ULL << bits))))
>
> Sounds good. I'll commit --amend that into the patch!
>
>
> >> And in the blue corner, touting consistent hex forms, ...
> >
> > And in the red, compact bytestring representations.
>
> > No, seriously, the inconsistency bothers me too. But so does the fact
> > that using 0x in the bytestring would double the minimum size for
> > representing bytestrings, somewhat changing the flavour of [] as well
> > (because spaces are no longer optional). I'm looking for a killer
> > argument one way or the other, but I haven't found it yet.
>
> But why does it even have to be hex numbers at all?
> I guess my point is that they could just be expressions.
> You could use 0x31 or 49 or '1' or 061, whichever made
> more sense in some application. You don't necessarily take
> a representational size hit.
But you do take a hit w.r.t. *minimum* representation size - there's
no form amongst all the possibilities here more compact than pure hex.
Especially since spaces are optional in the old form. The fact that
[ab cd 00] and [abcd00] are equivalent was a deliberate choice in the
original form.
The point of [] is for random binary data which is neither strings
(even with the odd strange character) nor sensibly organized into
32-bit (or larger) integers. Wanting something other than hex here is
much rarer than in the < > case.
You're seeing < > and [ ] as basically the same thing - a list of
values - with the only difference being the size of those values.
That's not wrong, but it's not the only way to look at it - and it's
not the way I was thinking of [ ] when I invented it. Your proposal
makes perfect sense while you think of [] as a list of values - but
not so much when it's thought of as a direct binary representation.
So I'm thinking perhaps we need two different things here: a "list of
values" representation, which can accomodate expressions and can also
have multiple sizes (because expressions which are evaluated to a
16-bit or 64-bit value could also be useful under the right
circumstances), and the [ ] "bytestring
literal" representation. Perhaps something like:
(32-bit values)
<0xdeadbeef (1+1)>
or <.32 0xdeadbeef (1+1)>
(64-bit values)
<.64 (0xdeadbeef << 32) (-1)>
(8-bit values)
<.8 0x00 0x0a 0xe4 0x2c 0x23 (0x10 + n)>
i.e. < > is list of values form, with size of each value as a sort of
parameter (defaulting to 32-bit, of course). I'm not sure I like that
particular syntax, it's just the first thing I came up with to
demonstrate the idea.
--
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
* [PATCH] Use SLB size from the device tree
From: Michael Neuling @ 2007-11-08 23:40 UTC (permalink / raw)
To: paulus; +Cc: Olof Johansson, linuxppc-dev, Will Schmidt
Currently we hardwire the number of SLBs but the PAPR says we export an
ibm,slb-size property to specify the number of SLB entries. This patch
uses this property instead of assuming 64 always. If no property is
found, we assume 64 entries as before.
This soft patches the SLB handler, so it won't change performance at
all.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
Paulus: for your 2.6.25 tree.
Olof: this touches the pasemi code, but I've not tested with one.
Will: this will interact with your SLB xmon patch, but is easy to
resolve.
arch/powerpc/kernel/prom.c | 11 +++++++++++
arch/powerpc/mm/hash_utils_64.c | 1 +
arch/powerpc/mm/slb.c | 3 +++
arch/powerpc/mm/slb_low.S | 5 +++--
arch/powerpc/platforms/pasemi/setup.c | 3 ++-
arch/powerpc/xmon/xmon.c | 2 +-
include/asm-powerpc/mmu-hash64.h | 1 +
include/asm-powerpc/reg.h | 6 ------
8 files changed, 22 insertions(+), 10 deletions(-)
Index: linux-2.6-ozlabs/arch/powerpc/kernel/prom.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/prom.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/prom.c
@@ -583,6 +583,16 @@ static void __init check_cpu_pa_features
ibm_pa_features, ARRAY_SIZE(ibm_pa_features));
}
+static void __init check_cpu_slb_size(unsigned long node)
+{
+ u32 *slb_size_ptr;
+
+ slb_size_ptr = of_get_flat_dt_prop(node, "ibm,slb-size", NULL);
+ if (slb_size_ptr != NULL) {
+ mmu_slb_size = *slb_size_ptr;
+ }
+}
+
static struct feature_property {
const char *name;
u32 min_value;
@@ -701,6 +711,7 @@ static int __init early_init_dt_scan_cpu
check_cpu_feature_properties(node);
check_cpu_pa_features(node);
+ check_cpu_slb_size(node);
#ifdef CONFIG_PPC_PSERIES
if (nthreads > 1)
Index: linux-2.6-ozlabs/arch/powerpc/mm/hash_utils_64.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/mm/hash_utils_64.c
+++ linux-2.6-ozlabs/arch/powerpc/mm/hash_utils_64.c
@@ -95,6 +95,7 @@ int mmu_vmalloc_psize = MMU_PAGE_4K;
int mmu_io_psize = MMU_PAGE_4K;
int mmu_kernel_ssize = MMU_SEGSIZE_256M;
int mmu_highuser_ssize = MMU_SEGSIZE_256M;
+u16 mmu_slb_size = 64;
#ifdef CONFIG_HUGETLB_PAGE
int mmu_huge_psize = MMU_PAGE_16M;
unsigned int HPAGE_SHIFT;
Index: linux-2.6-ozlabs/arch/powerpc/mm/slb.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/mm/slb.c
+++ linux-2.6-ozlabs/arch/powerpc/mm/slb.c
@@ -255,6 +255,7 @@ void slb_initialize(void)
static int slb_encoding_inited;
extern unsigned int *slb_miss_kernel_load_linear;
extern unsigned int *slb_miss_kernel_load_io;
+ extern unsigned int *slb_compare_rr_to_size;
/* Prepare our SLB miss handler based on our page size */
linear_llp = mmu_psize_defs[mmu_linear_psize].sllp;
@@ -268,6 +269,8 @@ void slb_initialize(void)
SLB_VSID_KERNEL | linear_llp);
patch_slb_encoding(slb_miss_kernel_load_io,
SLB_VSID_KERNEL | io_llp);
+ patch_slb_encoding(slb_compare_rr_to_size,
+ mmu_slb_size);
DBG("SLB: linear LLP = %04x\n", linear_llp);
DBG("SLB: io LLP = %04x\n", io_llp);
Index: linux-2.6-ozlabs/arch/powerpc/mm/slb_low.S
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/mm/slb_low.S
+++ linux-2.6-ozlabs/arch/powerpc/mm/slb_low.S
@@ -227,8 +227,9 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_ISER
7: ld r10,PACASTABRR(r13)
addi r10,r10,1
- /* use a cpu feature mask if we ever change our slb size */
- cmpldi r10,SLB_NUM_ENTRIES
+ /* This gets soft patched on boot. */
+_GLOBAL(slb_compare_rr_to_size)
+ cmpldi r10,0
blt+ 4f
li r10,SLB_NUM_BOLTED
Index: linux-2.6-ozlabs/arch/powerpc/platforms/pasemi/setup.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/platforms/pasemi/setup.c
+++ linux-2.6-ozlabs/arch/powerpc/platforms/pasemi/setup.c
@@ -36,6 +36,7 @@
#include <asm/smp.h>
#include <asm/time.h>
#include <asm/of_platform.h>
+#include <asm/mmu.h>
#include <pcmcia/ss.h>
#include <pcmcia/cistpl.h>
@@ -295,7 +296,7 @@ static int pas_machine_check_handler(str
int i;
printk(KERN_ERR "slb contents:\n");
- for (i = 0; i < SLB_NUM_ENTRIES; i++) {
+ for (i = 0; i < mmu_slb_size; i++) {
asm volatile("slbmfee %0,%1" : "=r" (e) : "r" (i));
asm volatile("slbmfev %0,%1" : "=r" (v) : "r" (i));
printk(KERN_ERR "%02d %016lx %016lx\n", i, e, v);
Index: linux-2.6-ozlabs/arch/powerpc/xmon/xmon.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/xmon/xmon.c
+++ linux-2.6-ozlabs/arch/powerpc/xmon/xmon.c
@@ -2531,7 +2531,7 @@ static void dump_slb(void)
printf("SLB contents of cpu %x\n", smp_processor_id());
- for (i = 0; i < SLB_NUM_ENTRIES; i++) {
+ for (i = 0; i < mmu_slb_size; i++) {
asm volatile("slbmfee %0,%1" : "=r" (tmp) : "r" (i));
printf("%02d %016lx ", i, tmp);
Index: linux-2.6-ozlabs/include/asm-powerpc/mmu-hash64.h
===================================================================
--- linux-2.6-ozlabs.orig/include/asm-powerpc/mmu-hash64.h
+++ linux-2.6-ozlabs/include/asm-powerpc/mmu-hash64.h
@@ -180,6 +180,7 @@ extern int mmu_vmalloc_psize;
extern int mmu_io_psize;
extern int mmu_kernel_ssize;
extern int mmu_highuser_ssize;
+extern u16 mmu_slb_size;
/*
* If the processor supports 64k normal pages but not 64k cache
Index: linux-2.6-ozlabs/include/asm-powerpc/reg.h
===================================================================
--- linux-2.6-ozlabs.orig/include/asm-powerpc/reg.h
+++ linux-2.6-ozlabs/include/asm-powerpc/reg.h
@@ -695,12 +695,6 @@
#define PV_BE 0x0070
#define PV_PA6T 0x0090
-/*
- * Number of entries in the SLB. If this ever changes we should handle
- * it with a use a cpu feature fixup.
- */
-#define SLB_NUM_ENTRIES 64
-
/* Macros for setting and retrieving special purpose registers */
#ifndef __ASSEMBLY__
#define mfmsr() ({unsigned long rval; \
^ permalink raw reply
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox