[PATCH] powerpc: fix userspace build of ptrace.h
From ff056c080d2b0b93bac07ad71125fee701919f5e Mon Sep 17 00:00:00 2001 From: Sam Ravnborg s...@ravnborg.org Date: Sun, 9 May 2010 08:52:31 +0200 Subject: [PATCH] powerpc: fix userspace build of ptrace.h Build of ptrace.h failed for assembly because it pulls in stdint.h. Use exportable types (__u32, __u64) to avoid the dependency on stdint.h. Signed-off-by: Sam Ravnborg s...@ravnborg.org Cc: Andrey Volkov avol...@varma-el.com Cc: Dave Kleikamp sha...@linux.vnet.ibm.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org --- A better fix is to remove the use of stdint like the following patch does. Note - I have not even build tested this patch! Sam arch/powerpc/include/asm/ptrace.h | 32 ++-- 1 files changed, 14 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h index 9e2d84c..77bbc68 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h @@ -24,11 +24,7 @@ * 2 of the License, or (at your option) any later version. */ -#ifdef __KERNEL__ #include linux/types.h -#else -#include stdint.h -#endif #ifndef __ASSEMBLY__ @@ -300,13 +296,13 @@ do { \ #ifndef __ASSEMBLY__ struct ppc_debug_info { - uint32_t version; /* Only version 1 exists to date */ - uint32_t num_instruction_bps; - uint32_t num_data_bps; - uint32_t num_condition_regs; - uint32_t data_bp_alignment; - uint32_t sizeof_condition; /* size of the DVC register */ - uint64_t features; + __u32 version; /* Only version 1 exists to date */ + __u32 num_instruction_bps; + __u32 num_data_bps; + __u32 num_condition_regs; + __u32 data_bp_alignment; + __u32 sizeof_condition; /* size of the DVC register */ + __u64 features; }; #endif /* __ASSEMBLY__ */ @@ -322,13 +318,13 @@ struct ppc_debug_info { #ifndef __ASSEMBLY__ struct ppc_hw_breakpoint { - uint32_t version; /* currently, version must be 1 */ - uint32_t trigger_type; /* only some combinations allowed */ - uint32_t addr_mode; /* address match mode */ - uint32_t condition_mode;/* break/watchpoint condition flags */ - uint64_t addr; /* break/watchpoint address */ - uint64_t addr2; /* range end or mask */ - uint64_t condition_value; /* contents of the DVC register */ + __u32 version; /* currently, version must be 1 */ + __u32 trigger_type; /* only some combinations allowed */ + __u32 addr_mode;/* address match mode */ + __u32 condition_mode; /* break/watchpoint condition flags */ + __u64 addr; /* break/watchpoint address */ + __u64 addr2;/* range end or mask */ + __u64 condition_value; /* contents of the DVC register */ }; #endif /* __ASSEMBLY__ */ -- 1.6.0.6 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: fix userspace build of ptrace.h
On Sun, 2010-05-09 at 08:59 +0200, Sam Ravnborg wrote: From ff056c080d2b0b93bac07ad71125fee701919f5e Mon Sep 17 00:00:00 2001 From: Sam Ravnborg s...@ravnborg.org Date: Sun, 9 May 2010 08:52:31 +0200 Subject: [PATCH] powerpc: fix userspace build of ptrace.h Build of ptrace.h failed for assembly because it pulls in stdint.h. Use exportable types (__u32, __u64) to avoid the dependency on stdint.h. Signed-off-by: Sam Ravnborg s...@ravnborg.org Cc: Andrey Volkov avol...@varma-el.com Cc: Dave Kleikamp sha...@linux.vnet.ibm.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org --- A better fix is to remove the use of stdint like the following patch does. Note - I have not even build tested this patch! Ack, thanks, I'll test and apply. Cheers, Ben. Sam arch/powerpc/include/asm/ptrace.h | 32 ++-- 1 files changed, 14 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h index 9e2d84c..77bbc68 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h @@ -24,11 +24,7 @@ * 2 of the License, or (at your option) any later version. */ -#ifdef __KERNEL__ #include linux/types.h -#else -#include stdint.h -#endif #ifndef __ASSEMBLY__ @@ -300,13 +296,13 @@ do { \ #ifndef __ASSEMBLY__ struct ppc_debug_info { - uint32_t version; /* Only version 1 exists to date */ - uint32_t num_instruction_bps; - uint32_t num_data_bps; - uint32_t num_condition_regs; - uint32_t data_bp_alignment; - uint32_t sizeof_condition; /* size of the DVC register */ - uint64_t features; + __u32 version; /* Only version 1 exists to date */ + __u32 num_instruction_bps; + __u32 num_data_bps; + __u32 num_condition_regs; + __u32 data_bp_alignment; + __u32 sizeof_condition; /* size of the DVC register */ + __u64 features; }; #endif /* __ASSEMBLY__ */ @@ -322,13 +318,13 @@ struct ppc_debug_info { #ifndef __ASSEMBLY__ struct ppc_hw_breakpoint { - uint32_t version; /* currently, version must be 1 */ - uint32_t trigger_type; /* only some combinations allowed */ - uint32_t addr_mode; /* address match mode */ - uint32_t condition_mode;/* break/watchpoint condition flags */ - uint64_t addr; /* break/watchpoint address */ - uint64_t addr2; /* range end or mask */ - uint64_t condition_value; /* contents of the DVC register */ + __u32 version; /* currently, version must be 1 */ + __u32 trigger_type; /* only some combinations allowed */ + __u32 addr_mode;/* address match mode */ + __u32 condition_mode; /* break/watchpoint condition flags */ + __u64 addr; /* break/watchpoint address */ + __u64 addr2;/* range end or mask */ + __u64 condition_value; /* contents of the DVC register */ }; #endif /* __ASSEMBLY__ */ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Fix [e]glibc build process
Dave Kleikamp wrote: On Sat, 2010-05-08 at 23:56 +0400, Andrey Volkov wrote: This patch fix [e]glibc build process destruction (more precisely _assembler_ is die when try to compile getcontext.S since stdint.h coldn't be assembled) intruduced by patch: commit: 162d92dfb79a0b5fc03380b8819fa5f870ebf1e Date: Mon, 8 Feb 2010 11:51:05 + (11:51 +) from: Dave Kleikamp Signed-off-by: Andrey Volkov avol...@varma-el.com --- arch/powerpc/include/asm/ptrace.h |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h index 9e2d84c..025912b 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h @@ -27,8 +27,10 @@ #ifdef __KERNEL__ #include linux/types.h #else +#ifndef __ASSEMBLY__ #include stdint.h #endif +#endif #ifndef __ASSEMBLY__ Assembly code won't need to pull in linux/types.h either, so this would be simpler: Questionable assertion. linux/types.h contain (indirectly) not only types definitions, but some useful, for assembly, defines too. So patch, which Sam Ravnborg offered, is better for me (if it will work certainly ;)). -- Andrey Volkov Signed-off-by: Dave Kleikamp sha...@linux.vnet.ibm.com diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h index 9e2d84c..0ed710e 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h @@ -24,14 +24,14 @@ * 2 of the License, or (at your option) any later version. */ +#ifndef __ASSEMBLY__ + #ifdef __KERNEL__ #include linux/types.h #else #include stdint.h #endif -#ifndef __ASSEMBLY__ - struct pt_regs { unsigned long gpr[32]; unsigned long nip; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] serial: mpc52xx_uart: fix null pointer dereference
On Wed, May 5, 2010 at 12:18 AM, Anatolij Gustschin ag...@denx.de wrote: Commit 6acc6833510db8f72b5ef343296d97480555fda9 introduced NULL pointer dereference and kernel crash on ppc32 machines while booting. Fix this bug now. Reported-by: Leonardo Chiquitto leonardo.li...@gmail.com Signed-off-by: Anatolij Gustschin ag...@denx.de Thanks for being so quick to pick up on this Anatolij. I've been traveling and not able to respond to stuff. I see that Greg has already picked this up (thanks Greg!), but FWIW: Acked-by: Grant Likely grant.lik...@secretlab.ca Actually, now that I look, this fix needs to go into Linus' tree right away (not just linux-next) since the offending patch went in during the 2.6.34 merge window. Greg, I can either pick this up and push it out to Linus tomorrow, or let you do it. Whichever you prefer. Let me know so I don't cause conflicts in your tree. On another note, this patch isn't actually the right fix, but that's because this driver is such a horribly bad example of handling private data that doing the right thing requires some re-architecting. The driver should not need any of that extra code in the module_init hook. For now this is the best thing to do, but I'll dig into it tomorrow and see if I can churn out a patch to fix it properly for the 2.6.35 merge window. Cheers and thanks, g. --- drivers/serial/mpc52xx_uart.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c index a176ab4..02469c3 100644 --- a/drivers/serial/mpc52xx_uart.c +++ b/drivers/serial/mpc52xx_uart.c @@ -1467,7 +1467,7 @@ mpc52xx_uart_init(void) /* * Map the PSC FIFO Controller and init if on MPC512x. */ - if (psc_ops-fifoc_init) { + if (psc_ops psc_ops-fifoc_init) { ret = psc_ops-fifoc_init(); if (ret) return ret; -- 1.7.0.4 -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: serial driver for MPC5554?
2010/4/16 Németh Márton nm...@freemail.hu: Hi, is there somebody working on the MPC5554 [1] serial port driver? No I could only find linux/drivers/serial/mpc52xx_uart.c but I'm not sure whether this could work together with the eSCI on-chip hardware module which can be found in MPC5554 [2]. Unfortunately, not the same device. You'll have to start from scratch it seems. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Bug #15589] 2.6.34-rc1: Badness at fs/proc/generic.c:316
On Sun, 9 May 2010 at 23:17, Rafael J. Wysocki wrote: Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=15589 Subject : 2.6.34-rc1: Badness at fs/proc/generic.c:316 Submitter : Christian Kujau li...@nerdbynature.de Date : 2010-03-13 23:53 (58 days old) Message-ID: alpine.deb.2.01.1003131544340.5...@bogon.housecafe.de References: http://marc.info/?l=linux-kernelm=126852442903680w=2 The bug is still present in -rc6, but Michael Ellerman has a patch[0] which made the warning go away. @Michael: will you post your patch with a Sign-Off, so that it can be pushed into mainline? Thanks, Christian. [0] http://patchwork.ozlabs.org/patch/50557/ diff --git a/fs/proc/proc_devtree.c b/fs/proc/proc_devtree.c index ce94801..019581d 100644 --- a/fs/proc/proc_devtree.c +++ b/fs/proc/proc_devtree.c @@ -176,6 +176,24 @@ retry: return fixed_name; } +static const char *unslash_name(const char *name) +{ + char *p, *fixed_name; + + fixed_name = kstrdup(name, GFP_KERNEL); + if (!fixed_name) { + printk(KERN_ERR device-tree: Out of memory trying to unslash + name \%s\\n, name); + return name; + } + + p = fixed_name; + while ((p = strstr(p, /))) + *p++ = '_'; + + return fixed_name; +} + /* * Process a node, adding entries for its children and its properties. */ @@ -212,6 +230,9 @@ void proc_device_tree_add_node(struct device_node *np, if (duplicate_name(de, p)) p = fixup_name(np, de, p); + if (strstr(p, /)) + p = unslash_name(p); + ent = __proc_device_tree_add_prop(de, pp, p); if (ent == NULL) break; -- BOFH excuse #188: ..disk or the processor is on fire. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 1/2] powerpc: cleanup APIs for cpu/thread/core mappings
On Fri, May 07, 2010 at 05:18:42PM +0530, Vaidyanathan Srinivasan wrote: These APIs take logical cpu number as input Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling() Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling() These APIs convert core number (index) to logical cpu/thread numbers Add cpu_first_thread_of_core(int core) Changed cpu_thread_to_core() to cpu_core_of_thread(int cpu) Why make all these changes? The end result doesn't seem any cleaner or better than how it was before, and your patch description doesn't give any reason for us to think yes, we should make this change. I assume you think this is a good change to make, so you need to explain why it's a good idea and convince the rest of us. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Bug #15589] 2.6.34-rc1: Badness at fs/proc/generic.c:316
On Sun, 2010-05-09 at 15:27 -0700, Christian Kujau wrote: On Sun, 9 May 2010 at 23:17, Rafael J. Wysocki wrote: Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=15589 Subject : 2.6.34-rc1: Badness at fs/proc/generic.c:316 Submitter : Christian Kujau li...@nerdbynature.de Date: 2010-03-13 23:53 (58 days old) Message-ID : alpine.deb.2.01.1003131544340.5...@bogon.housecafe.de References : http://marc.info/?l=linux-kernelm=126852442903680w=2 The bug is still present in -rc6, but Michael Ellerman has a patch[0] which made the warning go away. @Michael: will you post your patch with a Sign-Off, so that it can be pushed into mainline? No, Benh and I decided it's better to just drop those properties all together. But it's too late in this cycle for a patch like that just to fix a warning - so we'll do that patch for 35. cheers signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: Add power management support to VIO bus
On Fri, 2010-05-07 at 13:58 -0500, Brian King wrote: Adds support for suspend/resume for VIO devices. This is needed for support for HMC initiated hibernation. Signed-off-by: Brian King brk...@linux.vnet.ibm.com --- arch/powerpc/kernel/vio.c | 24 1 file changed, 24 insertions(+) diff -puN arch/powerpc/kernel/vio.c~powerpc_vio_bus_pm arch/powerpc/kernel/vio.c --- linux-2.6/arch/powerpc/kernel/vio.c~powerpc_vio_bus_pm2010-05-07 13:49:16.0 -0500 +++ linux-2.6-bjking1/arch/powerpc/kernel/vio.c 2010-05-07 13:49:16.0 -0500 @@ -1358,6 +1358,29 @@ static int vio_hotplug(struct device *de return 0; } +static int vio_pm_suspend(struct device *dev) +{ + const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL; + + if (pm pm-suspend) + return pm-suspend(dev); + return 0; +} + +static int vio_pm_resume(struct device *dev) +{ + const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL; + + if (pm pm-resume) + return pm-resume(dev); + return 0; +} + +const struct dev_pm_ops vio_dev_pm_ops = { + .suspend = vio_pm_suspend, + .resume = vio_pm_resume, +}; That just looks like boiler plate, is there not a generic version somewhere? cheers signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: remove unused 'protect4gb' boot parameter
'protect4gb' boot parameter was introduced to avoid allocating dma space acrossing 4GB boundary in 2007 (the commit 569975591c5530fdc9c7a3c45122e5e46f075a74). In 2008, the IOMMU was fixed to use the boundary_mask parameter per device properly. So 'protect4gb' workaround was removed (the 383af9525bb27f927511874f6306247ec13f1c28). But somehow I messed the 'protect4gb' boot parameter that was used to enable the workaround. Signed-off-by: FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp --- arch/powerpc/kernel/iommu.c | 12 1 files changed, 0 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c index ec94f90..d583917 100644 --- a/arch/powerpc/kernel/iommu.c +++ b/arch/powerpc/kernel/iommu.c @@ -43,20 +43,9 @@ #define DBG(...) static int novmerge; -static int protect4gb = 1; static void __iommu_free(struct iommu_table *, dma_addr_t, unsigned int); -static int __init setup_protect4gb(char *str) -{ - if (strcmp(str, on) == 0) - protect4gb = 1; - else if (strcmp(str, off) == 0) - protect4gb = 0; - - return 1; -} - static int __init setup_iommu(char *str) { if (!strcmp(str, novmerge)) @@ -66,7 +55,6 @@ static int __init setup_iommu(char *str) return 1; } -__setup(protect4gb=, setup_protect4gb); __setup(iommu=, setup_iommu); static unsigned long iommu_range_alloc(struct device *dev, -- 1.6.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] kexec-tools, ppc64: Fix segfault on parsing of large device trees.
In message 4be78e06.6080...@ozlabs.org you wrote: ppc64's fs2dt used to use a fixed-size array into which the device tree was parsed. There was no bounds checking, so with a large device tree other heap data ended up getting stomped -- SIGSEGV time. This patch adds a function, 'dt_reserve', to check whether there's enough spa ce left prior to writing data to the array. If not, the array is realloced. Signed-off-by: Matt Evans m...@ozlabs.org FWIW... Ack-by: Michael Neuling mi...@neuling.org (also added linuxppc-...@ozlabs.org to CC list) --- kexec/arch/ppc64/fs2dt.c | 62 +++- - 1 files changed, 53 insertions(+), 9 deletions(-) diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c index f168cbc..762bf04 100644 --- a/kexec/arch/ppc64/fs2dt.c +++ b/kexec/arch/ppc64/fs2dt.c @@ -35,13 +35,14 @@ #define MAXPATH 1024 /* max path name length */ #define NAMESPACE 16384 /* max bytes for property names */ -#define TREEWORDS 65536 /* max 32 bit words for property values */ +#define INIT_TREE_WORDS 65536/* Initial num words for prop values */ #define MEMRESERVE 256 /* max number of reserved memory blocks */ #define MAX_MEMORY_RANGES 1024 static char pathname[MAXPATH], *pathstart; static char propnames[NAMESPACE] = { 0 }; -static unsigned dtstruct[TREEWORDS] __attribute__ ((aligned (8))), *dt; +static unsigned *dt_base, *dt; +static unsigned int dt_cur_size; static unsigned long long mem_rsrv[2*MEMRESERVE] = { 0, 0 }; static int crash_param = 0; @@ -50,6 +51,28 @@ extern mem_rgns_t usablemem_rgns; static struct bootblock bb[1]; extern int my_debug; +/* Before we add something to the dt, reserve N words using this. + * If there isn't enough room, it's realloced -- and you don't overflow and + * splat bits of your heap. + */ +void dt_reserve(unsigned **dt_ptr, unsigned words) +{ + if (((*dt_ptr - dt_base) + words) = dt_cur_size) { + int offset; + unsigned int new_size = dt_cur_size + INIT_TREE_WORDS; + unsigned *new_dt = realloc(dt_base, new_size*4); + + if (!new_dt) + die(unrecoverable error: Can't realloc %d bytes for + device tree\n, new_size*4); + offset = *dt_ptr - dt_base; + dt_base = new_dt; + dt_cur_size = new_size; + *dt_ptr = dt_base + offset; + memset(*dt_ptr, 0, (new_size - offset)*4); + } +} + void reserve(unsigned long long where, unsigned long long length) { size_t offset; @@ -181,6 +204,7 @@ static void add_dyn_reconf_usable_mem_property(int fd) /* * Add linux,drconf-usable-memory property. */ + dt_reserve(dt, 4+((rlen + 3)/4)); *dt++ = 3; *dt++ = rlen; *dt++ = propnum(linux,drconf-usable-memory); @@ -253,6 +277,7 @@ static void add_usable_mem_property(int fd, size_t len) /* * No add linux,usable-memory property. */ + dt_reserve(dt, 4+((rlen + 3)/4)); *dt++ = 3; *dt++ = rlen; *dt++ = propnum(linux,usable-memory); @@ -317,6 +342,7 @@ static void putprops(char *fn, struct dirent **nlist, int numlist) len = statbuf.st_size; + dt_reserve(dt, 4+((len + 3)/4)); *dt++ = 3; *dt++ = len; *dt++ = propnum(fn); @@ -388,13 +414,17 @@ static void putnode(void) struct dirent **namelist; int numlist, i; struct stat statbuf; + int plen; + plen = *pathstart ? strlen(pathstart) : 1; + /* Reserve space for string packed to words; e.g. string length 10 + * occupies 3 words, length 12 occupies 4 (for terminating \0s). + * So round up include the \0: + */ + dt_reserve(dt, 1+((plen + 4)/4)); *dt++ = 1; strcpy((void *)dt, *pathstart ? pathstart : /); - while(*dt) - dt++; - if (dt[-1] 0xff) - dt++; + dt += ((plen + 4)/4); numlist = scandir(pathname, namelist, 0, comparefunc); if (numlist 0) @@ -415,6 +445,8 @@ static void putnode(void) if (initrd_base !strcmp(basename,/chosen/)) { int len = 8; unsigned long long initrd_end; + + dt_reserve(dt, 12); /* both props, of 6 words ea. */ *dt++ = 3; *dt++ = len; *dt++ = propnum(linux,initrd-start); @@ -487,6 +519,7 @@ static void putnode(void) cmd_len = cmd_len + 1; /* add new bootargs */ + dt_reserve(dt, 4+((cmd_len+3)/4)); *dt++ = 3; *dt++ = cmd_len; *dt++ = propnum(bootargs); @@ -571,6 +604,7 @@ no_debug: putnode(); } + dt_reserve(dt, 1); *dt++ = 2; dn[-1] = '\0';
Re: [RFC PATCH v2 1/2] powerpc: cleanup APIs for cpu/thread/core mappings
* Paul Mackerras pau...@samba.org [2010-05-10 09:05:22]: On Fri, May 07, 2010 at 05:18:42PM +0530, Vaidyanathan Srinivasan wrote: These APIs take logical cpu number as input Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling() Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling() These APIs convert core number (index) to logical cpu/thread numbers Add cpu_first_thread_of_core(int core) Changed cpu_thread_to_core() to cpu_core_of_thread(int cpu) Why make all these changes? The end result doesn't seem any cleaner or better than how it was before, and your patch description doesn't give any reason for us to think yes, we should make this change. I assume you think this is a good change to make, so you need to explain why it's a good idea and convince the rest of us. Sure Paul.. let me explain. The crux of the issue is to make 'threads_per_core' accessible to the pseries_energy module. In the first RFC, I had directly exported the variable which is not a good design. Ben H suggested to make an API around it and then export the function: http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-April/081610.html Instead of making an API to read threads_per_core, as Ben suggested, I have made a wrapper at a higher level to make an API to convert from logical cpu number to core number. The current APIs cpu_first_thread_in_core() and cpu_last_thread_in_core() returns logical CPU number while cpu_thread_to_core() returns core number or index which is not a logical CPU number. Ben recommended to clearly name them to distinguish 'core number' versus first and last 'logical cpu number' in that core. Hence in the new scheme, I have: Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling() Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling() which work on logical cpu numbers. While cpu_first_thread_of_core() and cpu_core_of_thread() work on core index. Example usage: (4 threads per core system) cpu_leftmost_thread_sibling(5) = 4 cpu_rightmost_thread_sibling(5) = 7 cpu_core_of_thread(5) = 1 cpu_first_thread_of_core(1) = 4 cpu_core_of_thread() is used in cpu_to_drc_index() in the module and cpu_first_thread_of_core() is used in drc_index_to_cpu() in the module. These APIs may be useful in other modules in future, and the proposed design is a good method to export these APIs to modules. An alternative approach could be to move both the base functions cpu_to_drc_index() and drc_index_to_cpu() into the kernel like in arch/powerpc/kernel/smp.c Thanks for the review, I hope I have explained the requirements and design for this cleanup. --Vaidy ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev