Re: [PATCH V2 1/4] cpufreq: add arm soc generic cpufreq driver
On Fri, Dec 16, 2011 at 12:26:03PM +0100, Heiko Stübner wrote: > Am Freitag, 16. Dezember 2011, 11:30:59 schrieb Richard Zhao: > > It support single core and multi-core ARM SoCs. But it assume > > all cores share the same frequency and voltage. > > > > Signed-off-by: Richard Zhao > > --- > > drivers/cpufreq/Kconfig.arm |8 ++ > > drivers/cpufreq/Makefile |1 + > > drivers/cpufreq/arm-cpufreq.c | 269 > > + 3 files changed, 278 > > insertions(+), 0 deletions(-) > > create mode 100644 drivers/cpufreq/arm-cpufreq.c > > looks quite cool, but should also provide a description of the devicetree > bindings (i.e. like the rest in Documentation/devicetree/bindings/...) for > future reference. Yes, Thanks for your remind. > > Heiko > ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 1/4] cpufreq: add arm soc generic cpufreq driver
On Fri, Dec 16, 2011 at 08:32:35AM -0600, Rob Herring wrote: > On 12/16/2011 04:30 AM, Richard Zhao wrote: > > It support single core and multi-core ARM SoCs. But it assume > > all cores share the same frequency and voltage. > > > > Signed-off-by: Richard Zhao > > --- > > drivers/cpufreq/Kconfig.arm |8 ++ > > drivers/cpufreq/Makefile |1 + > > drivers/cpufreq/arm-cpufreq.c | 269 > > + > > 3 files changed, 278 insertions(+), 0 deletions(-) > > create mode 100644 drivers/cpufreq/arm-cpufreq.c > > > > What makes this specific to ARM and not a generic DT + clk api + > regulator api driver? smp loops_per_jiffy update needs arm header . > > Also, you need documentation of the DT bindings. Yes, Thanks. Richard > > Rob > > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > > index 72a0044..a0f7d35 100644 > > --- a/drivers/cpufreq/Kconfig.arm > > +++ b/drivers/cpufreq/Kconfig.arm > > @@ -2,6 +2,14 @@ > > # ARM CPU Frequency scaling drivers > > # > > > > +config ARM_GENERIC_CPUFREQ > > + bool "ARM generic" > > + help > > + This adds ARM generic CPUFreq driver. It assumes all > > + cores of the SoC share the same clock and voltage. > > + > > + If in doubt, say N. > > + > > config ARM_S3C64XX_CPUFREQ > > bool "Samsung S3C64XX" > > depends on CPU_S3C6410 > > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile > > index ce75fcb..6ccf02d 100644 > > --- a/drivers/cpufreq/Makefile > > +++ b/drivers/cpufreq/Makefile > > @@ -39,6 +39,7 @@ obj-$(CONFIG_X86_CPUFREQ_NFORCE2) += cpufreq-nforce2.o > > > > > > ## > > # ARM SoC drivers > > +obj-$(CONFIG_ARM_GENERIC_CPUFREQ) += arm-cpufreq.o > > obj-$(CONFIG_UX500_SOC_DB8500) += db8500-cpufreq.o > > obj-$(CONFIG_ARM_S3C64XX_CPUFREQ) += s3c64xx-cpufreq.o > > obj-$(CONFIG_ARM_S5PV210_CPUFREQ) += s5pv210-cpufreq.o > > diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c > > new file mode 100644 > > index 000..e4d20da > > --- /dev/null > > +++ b/drivers/cpufreq/arm-cpufreq.c > > @@ -0,0 +1,269 @@ > > +/* > > + * Copyright (C) 2011 Freescale Semiconductor, Inc. > > + */ > > + > > +/* > > + * The code contained herein is licensed under the GNU General Public > > + * License. You may obtain a copy of the GNU General Public License > > + * Version 2 or later at the following locations: > > + * > > + * http://www.opensource.org/licenses/gpl-license.html > > + * http://www.gnu.org/copyleft/gpl.html > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +static u32 *cpu_freqs; /* HZ */ > > +static u32 *cpu_volts; /* uV */ > > +static u32 trans_latency; /* ns */ > > +static int cpu_op_nr; > > + > > +static DEFINE_PER_CPU(unsigned long, l_p_j_ref); > > +static unsigned long l_p_j_ref_freq; > > + > > +static struct clk *cpu_clk; > > +static struct regulator *cpu_reg; > > +static struct cpufreq_frequency_table *arm_freq_table; > > + > > +static int set_cpu_freq(unsigned long freq, int index, int higher) > > +{ > > + int ret = 0; > > + > > + if (higher && cpu_reg) > > + regulator_set_voltage(cpu_reg, > > + cpu_volts[index], cpu_volts[index]); > > + > > + ret = clk_set_rate(cpu_clk, freq); > > + if (ret != 0) { > > + printk(KERN_DEBUG "cannot set CPU clock rate\n"); > > + return ret; > > + } > > + > > + if (!higher && cpu_reg) > > + regulator_set_voltage(cpu_reg, > > + cpu_volts[index], cpu_volts[index]); > > + > > + return ret; > > +} > > + > > +static int arm_verify_speed(struct cpufreq_policy *policy) > > +{ > > + return cpufreq_frequency_table_verify(policy, arm_freq_table); > > +} > > + > > +static unsigned int arm_get_speed(unsigned int cpu) > > +{ > > + return clk_get_rate(cpu_clk) / 1000; > > +} > > + > > +static int arm_set_target(struct cpufreq_policy *policy, > > + unsigned int target_freq, unsigned int relation) > > +{ > > + struct cpufreq_freqs freqs; > > + unsigned long freq_Hz; > > + int cpu; > > + int ret = 0; > > + unsigned int index; > > + > > + cpufreq_frequency_table_target(policy, arm_freq_table, > > + target_freq, relation, &index); > > + freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]); > > + freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index]; > > + freqs.old = clk_get_rate(cpu_clk) / 1000; > > + freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]); > > + freqs.new = freq_Hz / 1000; > > + freqs.flags = 0; > > + > > + if (freqs.old == freqs.new) > > + return 0; > > + > > + for_each_possible_cpu(cpu) { > > + freqs.cpu = cpu; > > + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE); > > + } > > + > > + ret = set_cpu_freq(fre
Re: [PATCH V2 1/4] cpufreq: add arm soc generic cpufreq driver
On Fri, Dec 16, 2011 at 10:52:29AM +, Jamie Iles wrote: > Hi Richard, > > A couple of questions inline, but otherwise looks nice! Thanks for your review. > > Jamie > > On Fri, Dec 16, 2011 at 06:30:59PM +0800, Richard Zhao wrote: > > It support single core and multi-core ARM SoCs. But it assume > > all cores share the same frequency and voltage. > > > > Signed-off-by: Richard Zhao > > --- > [...] > > diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c > > new file mode 100644 > > index 000..e4d20da > > --- /dev/null > > +++ b/drivers/cpufreq/arm-cpufreq.c > > @@ -0,0 +1,269 @@ > > +/* > > + * Copyright (C) 2011 Freescale Semiconductor, Inc. > > + */ > > + > > +/* > > + * The code contained herein is licensed under the GNU General Public > > + * License. You may obtain a copy of the GNU General Public License > > + * Version 2 or later at the following locations: > > + * > > + * http://www.opensource.org/licenses/gpl-license.html > > + * http://www.gnu.org/copyleft/gpl.html > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +static u32 *cpu_freqs; /* HZ */ > > +static u32 *cpu_volts; /* uV */ > > +static u32 trans_latency; /* ns */ > > +static int cpu_op_nr; > > + > > +static DEFINE_PER_CPU(unsigned long, l_p_j_ref); > > +static unsigned long l_p_j_ref_freq; > > + > > +static struct clk *cpu_clk; > > This assumes that all CPU's share the same clk and run at the same rate. > Is that a fair/safe assumption? I honestly don't know the answer to > this so it's just a question!!! As I know, most share the same clk/volt. From the code: IMX6: yes Tegra: Yes, but strange it sets CPUFREQ_SHARED_TYPE_ALL. MSM is an exception. I can support the case, but I have to make sure it's handy to use. > > > +static struct regulator *cpu_reg; > > +static struct cpufreq_frequency_table *arm_freq_table; > > + > > +static int set_cpu_freq(unsigned long freq, int index, int higher) > > +{ > > + int ret = 0; > > + > > + if (higher && cpu_reg) > > + regulator_set_voltage(cpu_reg, > > + cpu_volts[index], cpu_volts[index]); > > + > > + ret = clk_set_rate(cpu_clk, freq); > > + if (ret != 0) { > > + printk(KERN_DEBUG "cannot set CPU clock rate\n"); > > Perhaps use pr_debug() and friends throughout this driver? ok. Thanks. > > > + return ret; > > + } > > + > > + if (!higher && cpu_reg) > > + regulator_set_voltage(cpu_reg, > > + cpu_volts[index], cpu_volts[index]); > > + > > + return ret; > > +} > > + > > +static int arm_verify_speed(struct cpufreq_policy *policy) > > +{ > > + return cpufreq_frequency_table_verify(policy, arm_freq_table); > > +} > > + > > +static unsigned int arm_get_speed(unsigned int cpu) > > +{ > > + return clk_get_rate(cpu_clk) / 1000; > > +} > > + > > +static int arm_set_target(struct cpufreq_policy *policy, > > + unsigned int target_freq, unsigned int relation) > > +{ > > + struct cpufreq_freqs freqs; > > + unsigned long freq_Hz; > > + int cpu; > > + int ret = 0; > > + unsigned int index; > > + > > + cpufreq_frequency_table_target(policy, arm_freq_table, > > + target_freq, relation, &index); > > + freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]); > > + freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index]; > > + freqs.old = clk_get_rate(cpu_clk) / 1000; > > + freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]); I forgot to delete this line. > > + freqs.new = freq_Hz / 1000; > > Why round the rate then overwrite it? Should this be freqs.new /= 1000? > > > + freqs.flags = 0; > > + > > + if (freqs.old == freqs.new) > > + return 0; > > + > > + for_each_possible_cpu(cpu) { > > + freqs.cpu = cpu; > > + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE); > > + } > > + > > + ret = set_cpu_freq(freq_Hz, index, (freqs.new > freqs.old)); > > + > > +#ifdef CONFIG_SMP > > + /* loops_per_jiffy is not updated by the cpufreq core for SMP systems. > > +* So update it for all CPUs. > > +*/ > > + for_each_possible_cpu(cpu) > > + per_cpu(cpu_data, cpu).loops_per_jiffy = > > + cpufreq_scale(per_cpu(l_p_j_ref, cpu), l_p_j_ref_freq, > > + freqs.new); > > +#endif > > + > > + for_each_possible_cpu(cpu) { > > + freqs.cpu = cpu; > > + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE); > > + } > > + > > + return ret; > > +} > > + > > +static int arm_cpufreq_init(struct cpufreq_policy *policy) > > +{ > > + int ret; > > + > > + if (policy->cpu >= num_possible_cpus()) > > + return -EINVAL; > > + > > + policy->cur = clk_get_rate(cpu_clk) / 1000; > > + policy->shared_type = CPUFREQ_SHARED_TYPE_ANY; > > + cpumask_setall(policy->cpus); > > + /* Manual states, that PLL stabili
Re: [PATCH V2 1/4] cpufreq: add arm soc generic cpufreq driver
On Fri, Dec 16, 2011 at 11:59:02AM -0800, Bryan Huntsman wrote: > On 12/16/2011 02:52 AM, Jamie Iles wrote: > > > >> +static DEFINE_PER_CPU(unsigned long, l_p_j_ref); > >> +static unsigned long l_p_j_ref_freq; > >> + > >> +static struct clk *cpu_clk; > > > > This assumes that all CPU's share the same clk and run at the same rate. > > Is that a fair/safe assumption? I honestly don't know the answer to > > this so it's just a question!!! > > On MSM, cpus independently scale both frequency and voltage. Our clock > driver isn't upstream yet. David Brown has a preliminary version here: If - cpu_clk are per_cpu, and get from cpu0 clk, cpu1 clk, etc. The same to regulators. - set CPUFREQ_SHARED_TYPE_NONE - don't set affect cpus. Does that help you? Thanks Richard > > https://www.codeaurora.org/gitweb/quic/kernel/?p=davidb/linux-msm.git;a=shortlog;h=refs/heads/msm-clock-rfc > > Once we get our driver upstream, MSM will be an exception and not select > ARM_GENERIC_CPUFREQ. We'll probably have a separate msm-cpufreq.c > driver under drivers/cpufreq/. > > - Bryan > > -- > Sent by an employee of the Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Patch] Regulator: Replace kzalloc with devm_kzalloc and if-else with a switch-case for da9052-regulator
On Thu, Dec 15, 2011 at 06:59:53PM +0530, Ashish Jangam wrote: > Reported-by: Mark Brown > Signed-off-by: David Dajun Chen > Signed-off-by: Ashish Jangam Applied, but this really should have been sent as two separate patches as it's two unrelated changes which don't overlap with each other. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 1/4] cpufreq: add arm soc generic cpufreq driver
On Saturday 17 December 2011 16:00:03 Richard Zhao wrote: > On Fri, Dec 16, 2011 at 08:32:35AM -0600, Rob Herring wrote: > > On 12/16/2011 04:30 AM, Richard Zhao wrote: > > > It support single core and multi-core ARM SoCs. But it assume > > > all cores share the same frequency and voltage. > > > > > > Signed-off-by: Richard Zhao > > > --- > > > drivers/cpufreq/Kconfig.arm |8 ++ > > > drivers/cpufreq/Makefile |1 + > > > drivers/cpufreq/arm-cpufreq.c | 269 > > > + > > > 3 files changed, 278 insertions(+), 0 deletions(-) > > > create mode 100644 drivers/cpufreq/arm-cpufreq.c > > > > > > > What makes this specific to ARM and not a generic DT + clk api + > > regulator api driver? > > smp loops_per_jiffy update needs arm header . I would suggest to instead change the definition of adjust_jiffies in the core so it can be overridden by the architecture, like this diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 987a165..174584d 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -189,6 +189,7 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_put); * systems as each CPU might be scaled differently. So, use the arch * per-CPU loops_per_jiffy value wherever possible. */ +#ifndef adjust_jiffies #ifndef CONFIG_SMP static unsigned long l_p_j_ref; static unsigned int l_p_j_ref_freq; @@ -218,7 +219,8 @@ static inline void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci) { return; } -#endif +#endif /* CONFIG_SMP */ +#endif /* adjust_jiffies */ /** Then ARM (and any others that want the driver) can provide their own implementation and set #define adjust_jiffies(val, ci) adjust_jiffies((val), (ci)) to let the core use that instead of the generic UP version. While we're there, we should probably try to fix drivers that use loops_per_jiffy, because that is not what they think it is on SMP. Arnd ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: "struct user" conflicts on arm
On 17 December 2011 09:17, peter green wrote: > While we are talking about modifying sys/ucontext.h David Given > raised another issue with that header (for those reading on the linaro list > his > post can be found at > http://lists.debian.org/debian-arm/2011/12/msg00048.html) > David Given>This might be a good time to mention that on ARM, sys/ucontext.h > defines > David Given>both symbols and preprocessor macros called R0, R1, R2, etc in > the > David Given>global namespace; this is causing one of my packages to fail to > build > David Given>due to symbol collision. > David Given> > David Given>I'm fixing the package, but naming symbols stuff like that is > still > David Given>pretty antisocial... > Does anyone know what the impact of renaming these to use a REG_ prefix like > i386, amd64 and sparc do* would be? at worst the packages that had to be workaround on arm* for this, can have the workaround removed. Since you're in the process of fixing things for glibc/arm, would you mind also looking at WCHAR_MIN/MAX undefined for arm? http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598937 Thanks Konstantinos ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v4 3/6] clk: introduce the common clock framework
On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote: > On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner wrote: > > On Tue, 13 Dec 2011, Mike Turquette wrote: > >> +void __clk_unprepare(struct clk *clk) > >> +{ > >> + if (!clk) > >> + return; > >> + > >> + if (WARN_ON(clk->prepare_count == 0)) > >> + return; > >> + > >> + if (--clk->prepare_count > 0) > >> + return; > >> + > >> + WARN_ON(clk->enable_count > 0); > > > > So this leaves the clock enable count set. I'm a bit wary about > > that. Shouldn't it either return (including bumping the prepare_count > > again) or call clk_disable() ? No it should not. > I've hit this in my port of OMAP. It comes from this simple situation: > > driver 1 (adapted for clk_prepare/clk_unprepare): > clk_prepare(clk); > clk_enable(clk); > > ... > > driver2 (not adapted for clk_prepare/clk_unprepare): > clk_enable(clk); So this is basically buggy. Look, it's quite simple. Convert _all_ your drivers to clk_prepare/clk_unprepare _before_ you start switching your platform to use these new functions. You can do that _today_ without exception. We must refuse to merge _any_ user which does this the old way - and we should have been doing this since my commit was merged into mainline to allow drivers to be converted. And stop trying to think of ways around this inside clk_prepare/ clk_unprepare/clk_enable/clk_disable. You can't do it. Just fix _all_ the drivers. Now. Before you start implementing clk_prepare/clk_unprepare. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: "struct user" conflicts on arm
Konstantinos Margaritis wrote: Does anyone know what the impact of renaming these to use a REG_ prefix like i386, amd64 and sparc do* would be? at worst the packages that had to be workaround on arm* for this, can have the workaround removed. I have prepared a patch (attatched) that eliminates the dependency on sys/procfs.h and renames R? to REG_R?. I have tested it by modifying the header locally. I am now attempting to rebuild glibc with the patch. After rebuilding glibc I will install it and attempt to rebuild GDB. Please comment on the patch and advise on the best route for submission (that is should I go through debian, through eglibc or direct to glibc?) Since you're in the process of fixing things for glibc/arm Note that I am not a glibc developer nor am I a dd (and even if I was a dd I don't think I would NMU glibc). I'm just a user with an interest in the future of debian on arm. would you mind also looking at WCHAR_MIN/MAX undefined for arm? http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598937 They most certainly are defined. The trouble is that WCHAR_MAX is defined in a strange way. #define __WCHAR_MAX ( (wchar_t) - 1 ) This definition is fine for normal c or c++ code but it cannot be properly evaluated in the context of a preprocessor conditional. The bug report has a patch (actually a replacement for an existing patch) which looks fine to me. Index: eglibc-2.13/ports/sysdeps/unix/sysv/linux/arm/sys/ucontext.h === --- eglibc-2.13.orig/ports/sysdeps/unix/sysv/linux/arm/sys/ucontext.h 2006-08-17 01:23:58.0 + +++ eglibc-2.13/ports/sysdeps/unix/sysv/linux/arm/sys/ucontext.h 2011-12-17 12:52:43.0 + @@ -23,7 +23,6 @@ #include #include -#include /* We need the signal context definitions even if they are not used included in . */ @@ -35,47 +34,64 @@ #define NGREG 18 /* Container for all general registers. */ -typedef elf_gregset_t gregset_t; +typedef greg_t gregset_t[NGREG]; /* Number of each register is the `gregset_t' array. */ enum { - R0 = 0, -#define R0 R0 - R1 = 1, -#define R1 R1 - R2 = 2, -#define R2 R2 - R3 = 3, -#define R3 R3 - R4 = 4, -#define R4 R4 - R5 = 5, -#define R5 R5 - R6 = 6, -#define R6 R6 - R7 = 7, -#define R7 R7 - R8 = 8, -#define R8 R8 - R9 = 9, -#define R9 R9 - R10 = 10, -#define R10 R10 - R11 = 11, -#define R11 R11 - R12 = 12, -#define R12 R12 - R13 = 13, -#define R13 R13 - R14 = 14, -#define R14 R14 - R15 = 15 -#define R15 R15 + REG_R0 = 0, +#define REG_R0 REG_R0 + REG_R1 = 1, +#define REG_R1 REG_R1 + REG_R2 = 2, +#define REG_R2 REG_R2 + REG_R3 = 3, +#define REG_R3 REG_R3 + REG_R4 = 4, +#define REG_R4 REG_R4 + REG_R5 = 5, +#define REG_R5 REG_R5 + REG_R6 = 6, +#define REG_R6 REG_R6 + REG_R7 = 7, +#define REG_R7 REG_R7 + REG_R8 = 8, +#define REG_R8 REG_R8 + REG_R9 = 9, +#define REG_R9 REG_R9 + REG_R10 = 10, +#define REG_R10 REG_R10 + REG_R11 = 11, +#define REG_R11 REG_R11 + REG_R12 = 12, +#define REG_R12 REG_R12 + REG_R13 = 13, +#define REG_R13 REG_R13 + REG_R14 = 14, +#define REG_R14 REG_R14 + REG_R15 = 15 +#define REG_R15 REG_R15 }; +struct _libc_fpstate +{ + struct + { +unsigned int sign1:1; +unsigned int unused:15; +unsigned int sign2:1; +unsigned int exponent:14; +unsigned int j:1; +unsigned int mantissa1:31; +unsigned int mantissa0:32; + } fpregs[8]; + unsigned int fpsr:32; + unsigned int fpcr:32; + unsigned char ftype[8]; + unsigned int init_flag; +}; /* Structure to describe FPU registers. */ -typedef elf_fpregset_t fpregset_t; +typedef struct _libc_fpstate fpregset_t; /* Context to describe whole processor state. This only describes the core registers; coprocessor registers get saved elsewhere ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: "struct user" conflicts on arm
Ulrich Weigand>Now, glibc also provides a file that defines Ulrich Weigand>layouts of register sets for use with signal handlers as well Ulrich Weigand>as the makecontext/setcontext/getcontext family of routines. Ulrich Weigand> Ulrich Weigand>Usually, those layouts tend to be in fact identical to the Ulrich Weigand>layouts described in / . Apparently, Ulrich Weigand>whoever implemented the ARM version of was Ulrich Weigand>trying to avoid some seemingly unnecessary code duplication Ulrich Weigand>by making *include* and using Ulrich Weigand>the information from there directly. This is not done on other Ulrich Weigand>platforms, for precisely the reason that the Ulrich Weigand>and headers do pollute the name space ... Ulrich Weigand> Ulrich Weigand>So I think the right thing to do in the short term would be to Ulrich Weigand>stop including , and instead add the Ulrich Weigand>register set information there directly, even if that means some Ulrich Weigand>duplication of code. (Again, since this is never-changing ABI, Ulrich Weigand>duplication isn't actually all that bad. Also, all the other Ulrich Weigand>platforms do it that way too, so why should ARM be different ...) Makes sense to me While we are talking about modifying sys/ucontext.h David Given raised another issue with that header (for those reading on the linaro list his post can be found at http://lists.debian.org/debian-arm/2011/12/msg00048.html) David Given>This might be a good time to mention that on ARM, sys/ucontext.h defines David Given>both symbols and preprocessor macros called R0, R1, R2, etc in the David Given>global namespace; this is causing one of my packages to fail to build David Given>due to symbol collision. David Given> David Given>I'm fixing the package, but naming symbols stuff like that is still David Given>pretty antisocial... Does anyone know what the impact of renaming these to use a REG_ prefix like i386, amd64 and sparc do* would be? * ia64,mips, mipsel, powerpc, 390 and s390x don't seem to define such symbols at all. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: "struct user" conflicts on arm
On Sat, Dec 17, 2011 at 7:57 AM, peter green wrote: >> mind also looking at WCHAR_MIN/MAX undefined for arm? >> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598937 > > They most certainly are defined. The trouble is that WCHAR_MAX is defined > in a strange way. > > #define __WCHAR_MAX ( (wchar_t) - 1 ) > > This definition is fine for normal c or c++ code but > it cannot be properly evaluated in the context of a preprocessor > conditional. > > The bug report has a patch (actually a replacement for > an existing patch) which looks fine to me. ISO C99 says that WCHAR_MAX must be a constant expression and the above definition is such an expression. Technically the program needs fixing (see below though for the "standards matter but so do users"), there is nothing wrong with a type cast and a constant value e.g. signed -1 converted to unsigned int (ARM GNU/Linux value for wchar_t). However, the real issue here is that it differs from x86, the most common architecture, and differences from x86 cause porting problems. The patch itself is insufficient because it doesn't take into account wordsize. When we switch to the 64-bit ARM ABI it should just work. Therefore you need to check for __WORDSIZE and *only* define a value if we are *not* 64-bits. You don't want to define anything for the 64-bit case until the 64-bit ARM ABI is out and finalized. Your patch to fix ucontext namespace pollution looks good, please post that to libc-ports for review and make sure to state what testing you've done with the patch. At a minimum you should run the glibc testsuite and build gdb with those newly installed headers. Cheers, Carlos. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev