Re: [RFC PATCH v2 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines
Hi Ben, Thanks a lot for the review. On 11/28/2011 04:18 AM, Benjamin Herrenschmidt wrote: > On Thu, 2011-11-17 at 16:58 +0530, Deepthi Dharwar wrote: >> This patch provides cpu_idle_wait() routine for the powerpc >> platform which is required by the cpuidle subsystem. This >> routine is requied to change the idle handler on SMP systems. >> The equivalent routine for x86 is in arch/x86/kernel/process.c >> but the powerpc implementation is different. >> >> Signed-off-by: Deepthi Dharwar >> Signed-off-by: Trinabh Gupta >> Signed-off-by: Arun R Bharadwaj >> --- > > No, that patch also adds this idle boot override thing (can you pick a > shorter name for boot_option_idle_override btw ?) which seems unrelated > and without any explanation as to what it's supposed to be about. Yes, we can pick a better and shorter name for this variable. This variable is used to determine if cpuidle framework needs to be enabled and pseries_driver to be loaded or not. We disable cpuidle framework only when powersave_off option is set or not enabled by the user. > > Additionally, I'm a bit worried (but maybe we already discussed that a > while back, I don't know) but cpu_idle_wait() has "wait" in the name, > which makes me think it might need to actually -wait- for all cpus to > have come out of the function. cpu_idle_wait is used to ensure that all the CPUs discard old idle handler and update to new one. Required while changing idle handler on SMP systems. > Now your implementation doesn't provide that guarantee. It might be > fine, I don't know, but if it is, you'd better document it well in the > comments surrounding the code, because as it is, all you do is shoot an > interrupt which will cause the target CPU to eventually come out of idle > some time in the future. I was hoping that sending an explicit reschedule to the cpus would do the trick but sure we can add some documentation around the code. > > Cheers, > Ben. > >> arch/powerpc/Kconfig |4 >> arch/powerpc/include/asm/processor.h |2 ++ >> arch/powerpc/include/asm/system.h|1 + >> arch/powerpc/kernel/idle.c | 26 ++ >> 4 files changed, 33 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index b177caa..87f8604 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -87,6 +87,10 @@ config ARCH_HAS_ILOG2_U64 >> bool >> default y if 64BIT >> >> +config ARCH_HAS_CPU_IDLE_WAIT >> +bool >> +default y >> + >> config GENERIC_HWEIGHT >> bool >> default y >> diff --git a/arch/powerpc/include/asm/processor.h >> b/arch/powerpc/include/asm/processor.h >> index eb11a44..811b7e7 100644 >> --- a/arch/powerpc/include/asm/processor.h >> +++ b/arch/powerpc/include/asm/processor.h >> @@ -382,6 +382,8 @@ static inline unsigned long get_clean_sp(struct pt_regs >> *regs, int is_32) >> } >> #endif >> >> +enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF}; >> + >> #endif /* __KERNEL__ */ >> #endif /* __ASSEMBLY__ */ >> #endif /* _ASM_POWERPC_PROCESSOR_H */ >> diff --git a/arch/powerpc/include/asm/system.h >> b/arch/powerpc/include/asm/system.h >> index e30a13d..ff66680 100644 >> --- a/arch/powerpc/include/asm/system.h >> +++ b/arch/powerpc/include/asm/system.h >> @@ -221,6 +221,7 @@ extern unsigned long klimit; >> extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask); >> >> extern int powersave_nap; /* set if nap mode can be used in idle loop */ >> +void cpu_idle_wait(void); >> >> /* >> * Atomic exchange >> diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c >> index 39a2baa..b478c72 100644 >> --- a/arch/powerpc/kernel/idle.c >> +++ b/arch/powerpc/kernel/idle.c >> @@ -39,9 +39,13 @@ >> #define cpu_should_die()0 >> #endif >> >> +unsigned long boot_option_idle_override = IDLE_NO_OVERRIDE; >> +EXPORT_SYMBOL(boot_option_idle_override); >> + >> static int __init powersave_off(char *arg) >> { >> ppc_md.power_save = NULL; >> +boot_option_idle_override = IDLE_POWERSAVE_OFF; >> return 0; >> } >> __setup("powersave=off", powersave_off); >> @@ -102,6 +106,28 @@ void cpu_idle(void) >> } >> } >> >> + >> +/* >> + * cpu_idle_wait - Used to ensure that all the CPUs come out of the old >> + * idle loop and start using the new idle loop. >> + * Required while changing idle handler on SMP systems. >> + * Caller must have changed idle handler to the new value before the call. >> + */ >> +void cpu_idle_wait(void) >> +{ >> +int cpu; >> +smp_mb(); >> + >> +/* kick all the CPUs so that they exit out of old idle routine */ >> +get_online_cpus(); >> +for_each_online_cpu(cpu) { >> +if (cpu != smp_processor_id()) >> +smp_send_reschedule(cpu); >> +} >> +put_online_cpus(); >> +} >> +EXPORT_SYMBOL_GPL(cpu_idle_wait); >> + >> int powersave_nap; >> >> #ifdef CONFIG_SYSCTL >> >>
Re: [RFC PATCH v2 2/4] cpuidle: (POWER) cpuidle driver for pSeries
On 11/28/2011 04:33 AM, Benjamin Herrenschmidt wrote: > On Thu, 2011-11-17 at 16:58 +0530, Deepthi Dharwar wrote: >> This patch implements a backhand cpuidle driver for pSeries >> based on pseries_dedicated_idle_loop and pseries_shared_idle_loop >> routines. The driver is built only if CONFIG_CPU_IDLE is set. This >> cpuidle driver uses global registration of idle states and >> not per-cpu. > > .../... > >> +#define MAX_IDLE_STATE_COUNT2 >> + >> +static int max_cstate = MAX_IDLE_STATE_COUNT - 1; > > Why "cstate" ? This isn't an x86 :-) Sure, I shall change it right away :) > >> +static struct cpuidle_device __percpu *pseries_idle_cpuidle_devices; > > Shorter name please. pseries_cpuidle_devs is fine. I ll do so. > >> +static struct cpuidle_state *cpuidle_state_table; >> + >> +void update_smt_snooze_delay(int snooze) >> +{ >> +struct cpuidle_driver *drv = cpuidle_get_driver(); >> +if (drv) >> +drv->states[0].target_residency = snooze; >> +} >> + >> +static inline void idle_loop_prolog(unsigned long *in_purr, ktime_t >> *kt_before) >> +{ >> + >> +*kt_before = ktime_get_real(); >> +*in_purr = mfspr(SPRN_PURR); >> +/* >> + * Indicate to the HV that we are idle. Now would be >> + * a good time to find other work to dispatch. >> + */ >> +get_lppaca()->idle = 1; >> +get_lppaca()->donate_dedicated_cpu = 1; >> +} > > I notice that you call this on shared processors as well. The old ocde > used to not set donate_dedicated_cpu in that case. I assume that's not a > big deal and that the HV will just ignore it in the shared processor > case but please add a comment after you've verified it. > Yes, the old code does not set donate_dedicated_cpu. But yes I will try testing it in a shared proc config but also remove this initialization for shared idle loop. >> +static inline s64 idle_loop_epilog(unsigned long in_purr, ktime_t >> kt_before) >> +{ >> +get_lppaca()->wait_state_cycles += mfspr(SPRN_PURR) - in_purr; >> +get_lppaca()->donate_dedicated_cpu = 0; >> +get_lppaca()->idle = 0; >> + >> +return ktime_to_us(ktime_sub(ktime_get_real(), kt_before)); >> +} >> + >> + >> +static int snooze_loop(struct cpuidle_device *dev, >> +struct cpuidle_driver *drv, >> +int index) >> +{ >> +unsigned long in_purr; >> +ktime_t kt_before; >> + >> +idle_loop_prolog(&in_purr, &kt_before); >> + >> +local_irq_enable(); >> +set_thread_flag(TIF_POLLING_NRFLAG); >> +while (!need_resched()) { >> +ppc64_runlatch_off(); >> +HMT_low(); >> +HMT_very_low(); >> +} >> +HMT_medium(); >> +clear_thread_flag(TIF_POLLING_NRFLAG); >> +smp_mb(); >> +local_irq_disable(); >> + >> +dev->last_residency = >> +(int)idle_loop_epilog(in_purr, kt_before); >> +return index; >> +} > > So your snooze loop has no timeout, is that handled by the cpuidle > driver using some kind of timer ? That sounds a lot less efficient than > passing a max delay to the snooze loop to handle getting into a deeper > state after a bit of snoozing rather than interrupting etc... My bad, snooze_loop is essential for a time out. Nope cpuidle driver doesn't have any timer mechanism. I ll fix it. Need to add loop for snooze time. >> +static int dedicated_cede_loop(struct cpuidle_device *dev, >> +struct cpuidle_driver *drv, >> +int index) >> +{ >> +unsigned long in_purr; >> +ktime_t kt_before; >> + >> +idle_loop_prolog(&in_purr, &kt_before); >> + >> +ppc64_runlatch_off(); >> +HMT_medium(); >> +cede_processor(); >> + >> +dev->last_residency = >> +(int)idle_loop_epilog(in_purr, kt_before); >> +return index; >> +} >> + >> +static int shared_cede_loop(struct cpuidle_device *dev, >> +struct cpuidle_driver *drv, >> +int index) >> +{ >> +unsigned long in_purr; >> +ktime_t kt_before; >> + >> +idle_loop_prolog(&in_purr, &kt_before); >> + >> +/* >> + * Yield the processor to the hypervisor. We return if >> + * an external interrupt occurs (which are driven prior >> + * to returning here) or if a prod occurs from another >> + * processor. When returning here, external interrupts >> + * are enabled. >> + */ >> +cede_processor(); >> + >> +dev->last_residency = >> +(int)idle_loop_epilog(in_purr, kt_before); >> +return index; >> +} >> + >> +/* >> + * States for dedicated partition case. >> + */ >> +static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = { >> +{ /* Snooze */ >> +.name = "snooze", >> +.desc = "snooze", >> +.flags = CPUIDLE_FLAG_TIME_VALID, >> +.exit_latency = 0, >> +.target_residency = 0, >> +.enter = &snooze_loop }, >> +{ /* CEDE */ >> +.name = "CEDE", >>
Re: [RFC PATCH v2 3/4] cpuidle: (POWER) Enable cpuidle and directly call cpuidle_idle_call() for pSeries
On 11/28/2011 04:35 AM, Benjamin Herrenschmidt wrote: > On Thu, 2011-11-17 at 16:58 +0530, Deepthi Dharwar wrote: >> This patch enables cpuidle for pSeries and cpuidle_idle_call() is >> directly called from the idle loop. As a result pseries_idle cpuidle >> driver registered with cpuidle subsystem comes into action. This patch >> also removes the routines pseries_shared_idle_sleep and >> pseries_dedicated_idle_sleep as they are now implemented as part of >> pseries_idle cpuidle driver. >> >> Signed-off-by: Deepthi Dharwar >> Signed-off-by: Trinabh Gupta >> Signed-off-by: Arun R Bharadwaj >> --- >> arch/powerpc/platforms/Kconfig |6 ++ >> arch/powerpc/platforms/pseries/setup.c | 86 >> +--- >> include/linux/cpuidle.h|2 - >> 3 files changed, 8 insertions(+), 86 deletions(-) >> >> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig >> index e458872..0d2a028 100644 >> --- a/arch/powerpc/platforms/Kconfig >> +++ b/arch/powerpc/platforms/Kconfig >> @@ -211,6 +211,12 @@ config PPC_PASEMI_CPUFREQ >> >> endmenu >> >> +menu "CPUIdle driver" >> + >> +source "drivers/cpuidle/Kconfig" >> + >> +endmenu >> + >> config PPC601_SYNC_FIX >> bool "Workarounds for PPC601 bugs" >> depends on 6xx && (PPC_PREP || PPC_PMAC) >> diff --git a/arch/powerpc/platforms/pseries/setup.c >> b/arch/powerpc/platforms/pseries/setup.c >> index 9c6716a..f624e74 100644 >> --- a/arch/powerpc/platforms/pseries/setup.c >> +++ b/arch/powerpc/platforms/pseries/setup.c >> @@ -39,6 +39,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> @@ -74,9 +75,6 @@ EXPORT_SYMBOL(CMO_PageSize); >> >> int fwnmi_active; /* TRUE if an FWNMI handler is present */ >> >> -static void pseries_shared_idle_sleep(void); >> -static void pseries_dedicated_idle_sleep(void); >> - >> static struct device_node *pSeries_mpic_node; >> >> static void pSeries_show_cpuinfo(struct seq_file *m) >> @@ -374,18 +372,9 @@ static void __init pSeries_setup_arch(void) >> >> pSeries_nvram_init(); >> >> -/* Choose an idle loop */ >> if (firmware_has_feature(FW_FEATURE_SPLPAR)) { >> vpa_init(boot_cpuid); >> -if (get_lppaca()->shared_proc) { >> -printk(KERN_DEBUG "Using shared processor idle loop\n"); >> -ppc_md.power_save = pseries_shared_idle_sleep; >> -} else { >> -printk(KERN_DEBUG "Using dedicated idle loop\n"); >> -ppc_md.power_save = pseries_dedicated_idle_sleep; >> -} >> -} else { >> -printk(KERN_DEBUG "Using default idle loop\n"); >> +ppc_md.power_save = (void *)cpuidle_idle_call; >> } > > I very very much dislike that cast. You should not have to cast a > function pointer ... EVER. Yes, I ll fix this. This actually bought out a design flaw with the current pseries idle as mentioned by you in the next patch of the series. > >> if (firmware_has_feature(FW_FEATURE_LPAR)) >> @@ -586,77 +575,6 @@ static int __init pSeries_probe(void) >> return 1; >> } >> > > Cheers, > Ben. > > Regards, Deepthi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 4/4] cpuidle: (POWER) Handle power_save=off
On 11/28/2011 04:37 AM, Benjamin Herrenschmidt wrote: > On Thu, 2011-11-17 at 16:59 +0530, Deepthi Dharwar wrote: >> This patch makes pseries_idle_driver not to be registered when >> power_save=off kernel boot option is specified. The >> boot_option_idle_override variable used here is similar to >> its usage on x86. > > Quick Q. With your changes, the CPU will never get into idle at all > until cpuidle initializes and the driver loads. > > That means not only much later in the boot process, but potentially > never if the distro has the driver as a module and fails to load it, or > similar. > > Can't that be an issue ? Shouldn't we keep at least one of the basic > idle functions as a fallback ? > On an LPAR if cpuidle is disabled, ppc_md.power_save is still set to cpuidle_idle_call by default here. This would result in calling of cpuidle_idle_call repeatedly, only for the call to return -ENODEV. The default idle is never executed. This would be a major design flaw. No fallback idle routine. We propose to fix this by checking the return value of ppc_md.power_save() call from void to int. Right now return value is void, but if we change this to int, this would solve two problems. One being removing the cast to a function pointer in the prev patch and this design flaw stated above. So by checking the return value of ppc_md.power_save(), we can invoke the default idle on failure. But my only concern is about the effects of changing the ppc_md.power_save() to return int on other powerpc architectures. Would it be a good idea to change the return type to int which would help us flag an error and fallback to default idle? > Cheers, > Ben. > > Regards, Deepthi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH net-next v3 4/4] powerpc: tqm8548/tqm8xx: add and update CAN device nodes
This patch enables or updates support for the CC770 and AN82527 CAN controller on the TQM8548 and TQM8xx boards. CC: devicetree-disc...@lists.ozlabs.org CC: linuxppc-...@ozlabs.org CC: Kumar Gala Signed-off-by: Wolfgang Grandegger --- arch/powerpc/boot/dts/tqm8548-bigflash.dts | 19 ++- arch/powerpc/boot/dts/tqm8548.dts | 19 ++- arch/powerpc/boot/dts/tqm8xx.dts | 25 + 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/boot/dts/tqm8548-bigflash.dts b/arch/powerpc/boot/dts/tqm8548-bigflash.dts index 9452c3c..d918752 100644 --- a/arch/powerpc/boot/dts/tqm8548-bigflash.dts +++ b/arch/powerpc/boot/dts/tqm8548-bigflash.dts @@ -352,7 +352,7 @@ ranges = < 0 0x0 0xfc00 0x0400 // NOR FLASH bank 1 1 0x0 0xf800 0x0800 // NOR FLASH bank 0 - 2 0x0 0xa300 0x8000 // CAN (2 x i82527) + 2 0x0 0xa300 0x8000 // CAN (2 x CC770) 3 0x0 0xa301 0x8000 // NAND FLASH >; @@ -393,18 +393,27 @@ }; /* Note: CAN support needs be enabled in U-Boot */ - can0@2,0 { - compatible = "intel,82527"; // Bosch CC770 + can@2,0 { + compatible = "bosch,cc770"; // Bosch CC770 reg = <2 0x0 0x100>; interrupts = <4 1>; interrupt-parent = <&mpic>; + bosch,external-clock-frequency = <1600>; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; + bosch,clock-out-frequency = <1600>; }; - can1@2,100 { - compatible = "intel,82527"; // Bosch CC770 + can@2,100 { + compatible = "bosch,cc770"; // Bosch CC770 reg = <2 0x100 0x100>; interrupts = <4 1>; interrupt-parent = <&mpic>; + bosch,external-clock-frequency = <1600>; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; }; /* Note: NAND support needs to be enabled in U-Boot */ diff --git a/arch/powerpc/boot/dts/tqm8548.dts b/arch/powerpc/boot/dts/tqm8548.dts index 619776f..988d887 100644 --- a/arch/powerpc/boot/dts/tqm8548.dts +++ b/arch/powerpc/boot/dts/tqm8548.dts @@ -352,7 +352,7 @@ ranges = < 0 0x0 0xfc00 0x0400 // NOR FLASH bank 1 1 0x0 0xf800 0x0800 // NOR FLASH bank 0 - 2 0x0 0xe300 0x8000 // CAN (2 x i82527) + 2 0x0 0xe300 0x8000 // CAN (2 x CC770) 3 0x0 0xe301 0x8000 // NAND FLASH >; @@ -393,18 +393,27 @@ }; /* Note: CAN support needs be enabled in U-Boot */ - can0@2,0 { - compatible = "intel,82527"; // Bosch CC770 + can@2,0 { + compatible = "bosch,cc770"; // Bosch CC770 reg = <2 0x0 0x100>; interrupts = <4 1>; interrupt-parent = <&mpic>; + bosch,external-clock-frequency = <1600>; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; + bosch,clock-out-frequency = <1600>; }; - can1@2,100 { - compatible = "intel,82527"; // Bosch CC770 + can@2,100 { + compatible = "bosch,cc770"; // Bosch CC770 reg = <2 0x100 0x100>; interrupts = <4 1>; interrupt-parent = <&mpic>; + bosch,external-clock-frequency = <1600>; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; }; /* Note: NAND support needs to be enabled in U-Boot */ diff --git a/arch/powerpc/boot/dts/tqm8xx.dts b/arch/powerpc/boot/dts/tqm8xx.dts index f6da7ec..c3dba25 100644 --- a/arch/powerpc/boot/dts/tqm8xx.dts +++ b/arch/powerpc/boot/dts/tqm8xx.dts @@ -57,6 +57,7 @@ ranges = < 0x0 0x0 0x4000 0x80 + 0x3 0x0 0xc000 0x200 >; flash@0,0 { @@ -67,
[PATCH net-next v3 3/4] can: cc770: add platform bus driver for the CC770 and AN82527
This driver works with both, static platform data and device tree bindings. It has been tested on a TQM855L board with two AN82527 CAN controllers on the local bus. CC: devicetree-disc...@lists.ozlabs.org CC: linuxppc-...@ozlabs.org CC: Kumar Gala Signed-off-by: Wolfgang Grandegger --- .../devicetree/bindings/net/can/cc770.txt | 56 drivers/net/can/cc770/Kconfig |7 + drivers/net/can/cc770/Makefile |1 + drivers/net/can/cc770/cc770_platform.c | 280 4 files changed, 344 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/can/cc770.txt create mode 100644 drivers/net/can/cc770/cc770_platform.c diff --git a/Documentation/devicetree/bindings/net/can/cc770.txt b/Documentation/devicetree/bindings/net/can/cc770.txt new file mode 100644 index 000..01e282d --- /dev/null +++ b/Documentation/devicetree/bindings/net/can/cc770.txt @@ -0,0 +1,56 @@ +Memory mapped Bosch CC770 and Intel AN82527 CAN controller + +Note: The CC770 is a CAN controller from Bosch, which is 100% +compatible with the old AN82527 from Intel, but with "bugs" being fixed. + +Required properties: + +- compatible : should be "bosch,cc770" for the CC770 and "intc,82527" + for the AN82527. + +- reg : should specify the chip select, address offset and size required + to map the registers of the controller. The size is usually 0x80. + +- interrupts : property with a value describing the interrupt source + (number and sensitivity) required for the controller. + +Optional properties: + +- bosch,external-clock-frequency : frequency of the external oscillator + clock in Hz. Note that the internal clock frequency used by the + controller is half of that value. If not specified, a default + value of 1600 (16 MHz) is used. + +- bosch,clock-out-frequency : slock frequency in Hz on the CLKOUT pin. + If not specified or if the specified value is 0, the CLKOUT pin + will be disabled. + +- bosch,slew-rate : slew rate of the CLKOUT signal. If not specified, + a resonable value will be calculated. + +- bosch,disconnect-rx0-input : see data sheet. + +- bosch,disconnect-rx1-input : see data sheet. + +- bosch,disconnect-tx1-output : see data sheet. + +- bosch,polarity-dominant : see data sheet. + +- bosch,divide-memory-clock : see data sheet. + +- bosch,iso-low-speed-mux : see data sheet. + +For further information, please have a look to the CC770 or AN82527. + +Examples: + +can@3,100 { + compatible = "bosch,cc770"; + reg = <3 0x100 0x80>; + interrupts = <2 0>; + interrupt-parent = <&mpic>; + bosch,external-clock-frequency = <1600>; +}; + + + diff --git a/drivers/net/can/cc770/Kconfig b/drivers/net/can/cc770/Kconfig index 28e4d48..22c07a8 100644 --- a/drivers/net/can/cc770/Kconfig +++ b/drivers/net/can/cc770/Kconfig @@ -11,4 +11,11 @@ config CAN_CC770_ISA connected to the ISA bus using I/O port, memory mapped or indirect access. +config CAN_CC770_PLATFORM + tristate "Generic Platform Bus based CC770 driver" + ---help--- + This driver adds support for the CC770 and AN82527 chips + connected to the "platform bus" (Linux abstraction for directly + to the processor attached devices). + endif diff --git a/drivers/net/can/cc770/Makefile b/drivers/net/can/cc770/Makefile index 872ecff..9fb8321 100644 --- a/drivers/net/can/cc770/Makefile +++ b/drivers/net/can/cc770/Makefile @@ -4,5 +4,6 @@ obj-$(CONFIG_CAN_CC770) += cc770.o obj-$(CONFIG_CAN_CC770_ISA) += cc770_isa.o +obj-$(CONFIG_CAN_CC770_PLATFORM) += cc770_platform.o ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c new file mode 100644 index 000..65e177e --- /dev/null +++ b/drivers/net/can/cc770/cc770_platform.c @@ -0,0 +1,280 @@ +/* + * Driver for CC770 and AN82527 CAN controllers on the platform bus + * + * Copyright (C) 2009, 2011 Wolfgang Grandegger + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the version 2 of the GNU General Public License + * as published by the Free Software Foundation + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + */ + +/* + * If platform data are used you should have similar definitions + * in your board-specific code: + * + * static struct cc770_platform_data myboard_cc770_pdata = { + * .osc_freq = 1600, + *
Re: [PATCH 2/6] powerpc/time: Use clockevents_calc_mult_shift
Hi Kumar, > > static void register_decrementer_clockevent(int cpu) > > { > > struct clock_event_device *dec = &per_cpu(decrementers, > > cpu).event; @@ -955,7 +928,8 @@ static void __init > > init_decrementer_cloc { > > int cpu = smp_processor_id(); > > > > - setup_clockevent_multiplier(ppc_tb_freq); > > + clockevents_calc_mult_shift(&decrementer_clockevent, > > ppc_tb_freq, 4); + > > Where's this magic 4 come from? No better reason than that's what most other users do. We weren't placing any limits on the shift/multiply in the old loop I don't think we need to, so we could probably just use 1 instead. Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: sam460ex, sm501 incorrect device id with kernel >=linux-2.6.39
On Sun, Nov 27, 2011 at 11:37 AM, acrux wrote: > > Acube Sam460ex has SM502 as onboard video. > I got this with any kernel newer than 2.6.38.x thus the framebuffer is lost > too: > > root@sam4x0:~# diff dmesg-2.6.38.8 dmesg-2.6.39.4 > 1,2c1,3 > < Using PowerPC 44x Platform machine description > < Linux version 2.6.38.8-Sam460ex (root@sam4x0) (gcc version 4.5.3 (CRUX PPC) > ) #1 Fri Nov 11 22:07:53 CET 2011 > --- >> Using Canyonlands machine description >> Initializing cgroup subsys cpu You seem to have switched from using the generic PowerPC 44x platform, to using a Canyonlands kernel and/or DTB. Not sure why that is, but it's probably not helping your issues at all. >> Linux version 2.6.39.4-Sam460ex (root@sam4x0) (gcc version 4.5.3 (CRUX PPC) >> ) #1 Fri Nov 11 19:06:02 CET 2011 > 17c18 > [cut] > 161,179c165,167 > < sm501 0001:00:06.0: SM501 At f548: Version 050100c0, 64 Mb, IRQ 19 > --- >> sm501 0001:00:06.0: incorrect device id c105 Something is reading the device ID in the wrong endian (and probably everything else). josh ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH net-next v3 3/4] can: cc770: add platform bus driver for the CC770 and AN82527
On 11/28/2011 12:30 PM, Wolfgang Grandegger wrote: > This driver works with both, static platform data and device tree > bindings. It has been tested on a TQM855L board with two AN82527 > CAN controllers on the local bus. > > CC: devicetree-disc...@lists.ozlabs.org > CC: linuxppc-...@ozlabs.org > CC: Kumar Gala > Signed-off-by: Wolfgang Grandegger See comment in the _remove function. Otherwise good. Add my: Acked-by: Marc Kleine-Budde > --- > .../devicetree/bindings/net/can/cc770.txt | 56 > drivers/net/can/cc770/Kconfig |7 + > drivers/net/can/cc770/Makefile |1 + > drivers/net/can/cc770/cc770_platform.c | 280 > > 4 files changed, 344 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/can/cc770.txt > create mode 100644 drivers/net/can/cc770/cc770_platform.c > > diff --git a/Documentation/devicetree/bindings/net/can/cc770.txt > b/Documentation/devicetree/bindings/net/can/cc770.txt > new file mode 100644 > index 000..01e282d > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/can/cc770.txt > @@ -0,0 +1,56 @@ > +Memory mapped Bosch CC770 and Intel AN82527 CAN controller > + > +Note: The CC770 is a CAN controller from Bosch, which is 100% > +compatible with the old AN82527 from Intel, but with "bugs" being fixed. > + > +Required properties: > + > +- compatible : should be "bosch,cc770" for the CC770 and "intc,82527" > + for the AN82527. > + > +- reg : should specify the chip select, address offset and size required > + to map the registers of the controller. The size is usually 0x80. > + > +- interrupts : property with a value describing the interrupt source > + (number and sensitivity) required for the controller. > + > +Optional properties: > + > +- bosch,external-clock-frequency : frequency of the external oscillator > + clock in Hz. Note that the internal clock frequency used by the > + controller is half of that value. If not specified, a default > + value of 1600 (16 MHz) is used. > + > +- bosch,clock-out-frequency : slock frequency in Hz on the CLKOUT pin. > + If not specified or if the specified value is 0, the CLKOUT pin > + will be disabled. > + > +- bosch,slew-rate : slew rate of the CLKOUT signal. If not specified, > + a resonable value will be calculated. > + > +- bosch,disconnect-rx0-input : see data sheet. > + > +- bosch,disconnect-rx1-input : see data sheet. > + > +- bosch,disconnect-tx1-output : see data sheet. > + > +- bosch,polarity-dominant : see data sheet. > + > +- bosch,divide-memory-clock : see data sheet. > + > +- bosch,iso-low-speed-mux : see data sheet. > + > +For further information, please have a look to the CC770 or AN82527. > + > +Examples: > + > +can@3,100 { > + compatible = "bosch,cc770"; > + reg = <3 0x100 0x80>; > + interrupts = <2 0>; > + interrupt-parent = <&mpic>; > + bosch,external-clock-frequency = <1600>; > +}; > + > + > + > diff --git a/drivers/net/can/cc770/Kconfig b/drivers/net/can/cc770/Kconfig > index 28e4d48..22c07a8 100644 > --- a/drivers/net/can/cc770/Kconfig > +++ b/drivers/net/can/cc770/Kconfig > @@ -11,4 +11,11 @@ config CAN_CC770_ISA > connected to the ISA bus using I/O port, memory mapped or > indirect access. > > +config CAN_CC770_PLATFORM > + tristate "Generic Platform Bus based CC770 driver" > + ---help--- > + This driver adds support for the CC770 and AN82527 chips > + connected to the "platform bus" (Linux abstraction for directly > + to the processor attached devices). > + > endif > diff --git a/drivers/net/can/cc770/Makefile b/drivers/net/can/cc770/Makefile > index 872ecff..9fb8321 100644 > --- a/drivers/net/can/cc770/Makefile > +++ b/drivers/net/can/cc770/Makefile > @@ -4,5 +4,6 @@ > > obj-$(CONFIG_CAN_CC770) += cc770.o > obj-$(CONFIG_CAN_CC770_ISA) += cc770_isa.o > +obj-$(CONFIG_CAN_CC770_PLATFORM) += cc770_platform.o > > ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG > diff --git a/drivers/net/can/cc770/cc770_platform.c > b/drivers/net/can/cc770/cc770_platform.c > new file mode 100644 > index 000..65e177e > --- /dev/null > +++ b/drivers/net/can/cc770/cc770_platform.c > @@ -0,0 +1,280 @@ > +/* > + * Driver for CC770 and AN82527 CAN controllers on the platform bus > + * > + * Copyright (C) 2009, 2011 Wolfgang Grandegger > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the version 2 of the GNU General Public License > + * as published by the Free Software Foundation > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * alo
Re: [PATCH 03/13] powerpc: Fix booke hugetlb preload code for PPC_MM_SLICES and 64-bit
On Nov 24, 2011, at 6:43 PM, Benjamin Herrenschmidt wrote: > On Mon, 2011-10-10 at 15:50 -0500, Becky Bruce wrote: > > .../... > >> #ifdef CONFIG_PPC_MM_SLICES >> -psize = mmu_get_tsize(get_slice_psize(mm, ea)); >> -tsize = mmu_get_psize(psize); >> +psize = get_slice_psize(mm, ea); >> +tsize = mmu_get_tsize(psize); >> shift = mmu_psize_defs[psize].shift; >> #else >> -vma = find_vma(mm, ea); >> -psize = vma_mmu_pagesize(vma); /* returns actual size in bytes */ >> -asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize)); >> -shift = 31 - lz; >> -tsize = 21 - lz; >> +psize = vma_mmu_pagesize(find_vma(mm, ea)); >> +shift = __ilog2(psize); >> +tsize = shift - 10; >> #endif > > Now, I know it was already there and you are just moving it around in > this patch but come on ... find_vma() here ? Really ? And with no result > checking nor boundary checking (remember it can return a vma that > doesn't enclose the address etc). Now I know in this specific case > it -should- be safe but still... > > Now, the caller is just doing: > > book3e_hugetlb_preload(vma->vm_mm, address, *ptep); > > So why not just change the prototype and pass the vma down instead ? Can we do this on top of the patchset instead of changing the existing patchset? - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On Nov 24, 2011, at 11:35 PM, Anton Blanchard wrote: > > On a 64bit book3s machine I have an oops from a system reset that > claims the book3e CE bit was set: > > MSR: 80021032 CR: 24004082 XER: 0010 > > On a book3s machine system reset sets IBM bit 46 and 47 depending on > the power saving mode. Separate the definitions by type and for > completeness add the rest of the bits in. > > Signed-off-by: Anton Blanchard > --- > > Index: linux-build/arch/powerpc/kernel/process.c > === > --- linux-build.orig/arch/powerpc/kernel/process.c2011-11-25 > 13:22:24.294919094 +1100 > +++ linux-build/arch/powerpc/kernel/process.c 2011-11-25 13:36:23.213834524 > +1100 > @@ -584,16 +584,32 @@ static struct regbit { > unsigned long bit; > const char *name; > } msr_bits[] = { > +#if defined(CONFIG_PPC64) && !defined(CONFIG_BOOKE) > + {MSR_SF,"SF"}, > + {MSR_HV,"HV"}, > +#endif > + {MSR_VEC, "VEC"}, > + {MSR_VSX, "VSX"}, > +#ifdef CONFIG_BOOKE > + {MSR_CE,"CE"}, > +#endif > {MSR_EE,"EE"}, > {MSR_PR,"PR"}, > {MSR_FP,"FP"}, > - {MSR_VEC, "VEC"}, > - {MSR_VSX, "VSX"}, > {MSR_ME,"ME"}, > - {MSR_CE,"CE"}, > +#ifdef CONFIG_BOOKE > {MSR_DE,"DE"}, > +#else > + {MSR_SE,"SE"}, > + {MSR_BE,"BE"}, > +#endif > {MSR_IR,"IR"}, > {MSR_DR,"DR"}, > + {MSR_PMM, "PMM"}, > +#ifndef CONFIG_BOOKE > + {MSR_RI,"RI"}, We have 'RI' on some BOOKE so lets allow for it to be decoded > + {MSR_LE,"LE"}, > +#endif > {0, NULL} > }; Since you're fixing this can you add the following for CONFIG_BOOKE: MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala wrote: > > On Nov 24, 2011, at 11:35 PM, Anton Blanchard wrote: > >> >> On a 64bit book3s machine I have an oops from a system reset that >> claims the book3e CE bit was set: >> >> MSR: 80021032 CR: 24004082 XER: 0010 >> >> On a book3s machine system reset sets IBM bit 46 and 47 depending on >> the power saving mode. Separate the definitions by type and for >> completeness add the rest of the bits in. >> >> Signed-off-by: Anton Blanchard >> --- >> >> Index: linux-build/arch/powerpc/kernel/process.c >> === >> --- linux-build.orig/arch/powerpc/kernel/process.c 2011-11-25 >> 13:22:24.294919094 +1100 >> +++ linux-build/arch/powerpc/kernel/process.c 2011-11-25 13:36:23.213834524 >> +1100 >> @@ -584,16 +584,32 @@ static struct regbit { >> unsigned long bit; >> const char *name; >> } msr_bits[] = { >> +#if defined(CONFIG_PPC64) && !defined(CONFIG_BOOKE) >> + {MSR_SF, "SF"}, >> + {MSR_HV, "HV"}, >> +#endif >> + {MSR_VEC, "VEC"}, >> + {MSR_VSX, "VSX"}, >> +#ifdef CONFIG_BOOKE >> + {MSR_CE, "CE"}, >> +#endif >> {MSR_EE, "EE"}, >> {MSR_PR, "PR"}, >> {MSR_FP, "FP"}, >> - {MSR_VEC, "VEC"}, >> - {MSR_VSX, "VSX"}, >> {MSR_ME, "ME"}, >> - {MSR_CE, "CE"}, >> +#ifdef CONFIG_BOOKE >> {MSR_DE, "DE"}, >> +#else >> + {MSR_SE, "SE"}, >> + {MSR_BE, "BE"}, >> +#endif >> {MSR_IR, "IR"}, >> {MSR_DR, "DR"}, >> + {MSR_PMM, "PMM"}, >> +#ifndef CONFIG_BOOKE >> + {MSR_RI, "RI"}, > > We have 'RI' on some BOOKE so lets allow for it to be decoded > >> + {MSR_LE, "LE"}, >> +#endif >> {0, NULL} >> }; > > Since you're fixing this can you add the following for CONFIG_BOOKE: > > MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM Those don't exist on 4xx, so CONFIG_BOOKE doesn't seem appropriate. josh ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
On Thu, Nov 24, 2011 at 08:12:25AM +, Shi Xuelin-B29237 wrote: > Hi Ira, > > Thanks for your review. > > After second thought, I think your scenario may not occur. > Because the cookie 20 we query must be returned by fsl_dma_tx_submit(...) in > practice. > We never query a cookie not returned by fsl_dma_tx_submit(...). > I agree about this part. > When we call fsl_tx_status(20), the chan->common.cookie is definitely wrote > as 20 and cpu2 could not read as 19. > This is what I don't agree about. However, I'm not an expert on CPU cache vs. memory accesses in an multi-processor system. The section titled "CACHE COHERENCY" in Documentation/memory-barriers.txt leads me to believe that the scenario I described is possible. What happens if CPU1's write of chan->common.cookie only goes into CPU1's cache. It never makes it to main memory before CPU2 fetches the old value of 19. I don't think you should see any performance impact from the smp_mb() operation. Thanks, Ira > -Original Message- > From: Ira W. Snyder [mailto:i...@ovro.caltech.edu] > Sent: 2011年11月23日 2:59 > To: Shi Xuelin-B29237 > Cc: dan.j.willi...@intel.com; Li Yang-R58472; z...@zh-kernel.org; > vinod.k...@intel.com; linuxppc-dev@lists.ozlabs.org; > linux-ker...@vger.kernel.org > Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing > spinlock use. > > On Tue, Nov 22, 2011 at 12:55:05PM +0800, b29...@freescale.com wrote: > > From: Forrest Shi > > > > dma status check function fsl_tx_status is heavily called in > > a tight loop and the desc lock in fsl_tx_status contended by > > the dma status update function. this caused the dma performance > > degrades much. > > > > this patch releases the lock in the fsl_tx_status function. > > I believe it has no neglect impact on the following call of > > dma_async_is_complete(...). > > > > we can see below three conditions will be identified as success > > a) x < complete < use > > b) x < complete+N < use+N > > c) x < complete < use+N > > here complete is the completed_cookie, use is the last_used > > cookie, x is the querying cookie, N is MAX cookie > > > > when chan->completed_cookie is being read, the last_used may > > be incresed. Anyway it has no neglect impact on the dma status > > decision. > > > > Signed-off-by: Forrest Shi > > --- > > drivers/dma/fsldma.c |5 - > > 1 files changed, 0 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index > > 8a78154..1dca56f 100644 > > --- a/drivers/dma/fsldma.c > > +++ b/drivers/dma/fsldma.c > > @@ -986,15 +986,10 @@ static enum dma_status fsl_tx_status(struct dma_chan > > *dchan, > > struct fsldma_chan *chan = to_fsl_chan(dchan); > > dma_cookie_t last_complete; > > dma_cookie_t last_used; > > - unsigned long flags; > > - > > - spin_lock_irqsave(&chan->desc_lock, flags); > > > > This will cause a bug. See below for a detailed explanation. You need this > instead: > > /* >* On an SMP system, we must ensure that this CPU has seen the >* memory accesses performed by another CPU under the >* chan->desc_lock spinlock. >*/ > smp_mb(); > > last_complete = chan->completed_cookie; > > last_used = dchan->cookie; > > > > - spin_unlock_irqrestore(&chan->desc_lock, flags); > > - > > dma_set_tx_state(txstate, last_complete, last_used, 0); > > return dma_async_is_complete(cookie, last_complete, last_used); } > > Facts: > - dchan->cookie is the same member as chan->common.cookie (same memory > location) > - chan->common.cookie is the "last allocated cookie for a pending transaction" > - chan->completed_cookie is the "last completed transaction" > > I have replaced "dchan->cookie" with "chan->common.cookie" in the below > explanation, to keep everything referenced from the same structure. > > Variable usage before your change. Everything is used locked. > - RW chan->common.cookie (fsl_dma_tx_submit) > - R chan->common.cookie (fsl_tx_status) > - R chan->completed_cookie (fsl_tx_status) > - W chan->completed_cookie (dma_do_tasklet) > > Variable usage after your change: > - RW chan->common.cookie LOCKED > - R chan->common.cookie NO LOCK > - R chan->completed_cookie NO LOCK > - W chan->completed_cookie LOCKED > > What if we assume that you have a 2 CPU system (such as a P2020). After your > changes, one possible sequence is: > > === CPU1 - allocate + submit descriptor: fsl_dma_tx_submit() === > spin_lock_irqsave > descriptor->cookie = 20 (x in your example) > chan->common.cookie = 20 (used in your example) > spin_unlock_irqrestore > > === CPU2 - immediately calls fsl_tx_status() === > chan->common.cookie == 19 > chan->completed_cookie == 19 > descriptor->cookie == 20 > > Since we don't have l
Re: [PATCH 03/13] powerpc: Fix booke hugetlb preload code for PPC_MM_SLICES and 64-bit
On Nov 24, 2011, at 6:43 PM, Benjamin Herrenschmidt wrote: > On Mon, 2011-10-10 at 15:50 -0500, Becky Bruce wrote: > > .../... > >> #ifdef CONFIG_PPC_MM_SLICES >> -psize = mmu_get_tsize(get_slice_psize(mm, ea)); >> -tsize = mmu_get_psize(psize); >> +psize = get_slice_psize(mm, ea); >> +tsize = mmu_get_tsize(psize); >> shift = mmu_psize_defs[psize].shift; >> #else >> -vma = find_vma(mm, ea); >> -psize = vma_mmu_pagesize(vma); /* returns actual size in bytes */ >> -asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize)); >> -shift = 31 - lz; >> -tsize = 21 - lz; >> +psize = vma_mmu_pagesize(find_vma(mm, ea)); >> +shift = __ilog2(psize); >> +tsize = shift - 10; >> #endif > > Now, I know it was already there and you are just moving it around in > this patch but come on ... find_vma() here ? Really ? And with no result > checking nor boundary checking (remember it can return a vma that > doesn't enclose the address etc). Now I know in this specific case > it -should- be safe but still... > > Now, the caller is just doing: > > book3e_hugetlb_preload(vma->vm_mm, address, *ptep); > > So why not just change the prototype and pass the vma down instead ? There's no reason - I just left the prototype the way it was in the original code, but it makes sense to change it given the changes to the function. Respin coming. -B ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
On 11/24/2011 01:37 AM, Li Yang-R58472 wrote: >> +static void io_to_buffer(struct mtd_info *mtd, int subpage, int oob) >> +{ >> +struct nand_chip *chip = mtd->priv; >> +struct fsl_elbc_mtd *priv = chip->priv; >> +struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand; >> +void *src, *dst; >> +int len = (oob ? 64 : 2048); >> + >> +if (oob) >> +dst = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64; >> +else >> +dst = elbc_fcm_ctrl->buffer + subpage * 2048; >> + >> +src = elbc_fcm_ctrl->addr + (oob ? 2048 : 0); >> +memcpy_fromio(dst, src, len); > > Might be safer to use _memcpy_fromio() How so? memcpy_fromio() is the public interface that will end up calling _memcpy_fromio() on powerpc. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/85xx: do not force PHYS_64BIT on the P1022DS
Kumar Gala wrote: > If you want me to apply this please also provided a 32-bit .dts for > p1022ds. This should be pretty trivial based on my recent .dts > cleanups. I think I found another bug in the 36-bit DTS. Looking at U-Boot, I see this: #ifdef CONFIG_PHYS_64BIT #define CONFIG_SYS_PCIE2_MEM_BUS0xe000 #define CONFIG_SYS_PCIE2_MEM_PHYS 0xc2000ull #else #define CONFIG_SYS_PCIE2_MEM_BUS0xa000 #define CONFIG_SYS_PCIE2_MEM_PHYS 0xa000 #endif But the 36-bit DTS has this: pci0: pcie@ffe09000 { reg = <0x0 0xffe09000 0 0x1000>; ranges = <0x200 0x0 0xa000 0xc 0x2000 0x0 0x2000 0x100 0x0 0x 0xf 0xffc1 0x0 0x1>; I don't think these match. I think the first 'ranges' line should have 0xe000 instead of 0xa000. I see the same problem with the other two PCI busses. It looks like the physical address is correct, but the BUS address is wrong (it's using the 32-bit bus address instead of the 36-bit bus address). -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/6] powerpc/time: Handle wrapping of decrementer
On 11/24/2011 12:07 AM, Anton Blanchard wrote: > Index: linux-build/arch/powerpc/kernel/irq.c > === > --- linux-build.orig/arch/powerpc/kernel/irq.c2011-11-17 > 10:04:16.551137554 +1100 > +++ linux-build/arch/powerpc/kernel/irq.c 2011-11-17 14:23:10.834514143 > +1100 > @@ -164,16 +164,13 @@ notrace void arch_local_irq_restore(unsi >*/ > local_paca->hard_enabled = en; > > -#ifndef CONFIG_BOOKE > - /* On server, re-trigger the decrementer if it went negative since > - * some processors only trigger on edge transitions of the sign bit. > - * > - * BookE has a level sensitive decrementer (latches in TSR) so we > - * don't need that > + /* > + * Trigger the decrementer if we have a pending event. Some processors > + * only trigger on edge transitions of the sign bit. We might also > + * have disabled interrupts long enough that the decrementer wrapped > + * to positive. >*/ > - if ((int)mfspr(SPRN_DEC) < 0) > - mtspr(SPRN_DEC, 1); > -#endif /* CONFIG_BOOKE */ > + decrementer_check_overflow(); Where did the #ifndef CONFIG_BOOKE go? BookE doesn't need this; the interrupt will continue asserting until software clears TSR[DIS]. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On 11/28/2011 10:23 AM, Josh Boyer wrote: > On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala > wrote: >> >> Since you're fixing this can you add the following for CONFIG_BOOKE: >> >> MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM > > Those don't exist on 4xx, so CONFIG_BOOKE doesn't seem appropriate. They're defined by book3e of ISA 2.06. Not all bits are going to exist on all CPUs -- does 4xx use these bits to mean something different? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On Mon, Nov 28, 2011 at 2:30 PM, Scott Wood wrote: > On 11/28/2011 10:23 AM, Josh Boyer wrote: >> On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala >> wrote: >>> >>> Since you're fixing this can you add the following for CONFIG_BOOKE: >>> >>> MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM >> >> Those don't exist on 4xx, so CONFIG_BOOKE doesn't seem appropriate. > > They're defined by book3e of ISA 2.06. 4xx existed before that, sadly (as did CONFIG_BOOKE). > Not all bits are going to exist on all CPUs -- does 4xx use these bits > to mean something different? No, marked as reserved. However, given the patch shows up in human readable output, I don't think we want reserved bits being decoded and showing up inadvertently. Could introduce BOOK3E_32 to cover cases like this. josh ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: sam460ex, sm501 incorrect device id with kernel >=linux-2.6.39
On Mon, 28 Nov 2011 07:12:38 -0500 Josh Boyer wrote: > On Sun, Nov 27, 2011 at 11:37 AM, acrux wrote: > > > > Acube Sam460ex has SM502 as onboard video. > > I got this with any kernel newer than 2.6.38.x thus the framebuffer > > is lost too: > > > > root@sam4x0:~# diff dmesg-2.6.38.8 dmesg-2.6.39.4 > > 1,2c1,3 > > < Using PowerPC 44x Platform machine description > > < Linux version 2.6.38.8-Sam460ex (root@sam4x0) (gcc version 4.5.3 > > (CRUX PPC) ) #1 Fri Nov 11 22:07:53 CET 2011 > > --- > >> Using Canyonlands machine description > >> Initializing cgroup subsys cpu > > You seem to have switched from using the generic PowerPC 44x platform, > to using a Canyonlands kernel and/or DTB. Not sure why that is, but > it's probably not helping your issues at all. > i think it was only a cosmetic change as it was always used Canyonlands platform and his own proper dtb. > >> Linux version 2.6.39.4-Sam460ex (root@sam4x0) (gcc version 4.5.3 > >> (CRUX PPC) ) #1 Fri Nov 11 19:06:02 CET 2011 > > 17c18 > > [cut] > > 161,179c165,167 > > < sm501 0001:00:06.0: SM501 At f548: Version 050100c0, 64 Mb, > > IRQ 19 > > > --- > >> sm501 0001:00:06.0: incorrect device id c105 > > Something is reading the device ID in the wrong endian (and probably > everything else). > it seems to be an endianess issue but i didn't find when it was introduced. Really strange this kind of issue was never noticed bumping from 2.6.38.x to 2.6.39.x . --nico -- GNU/Linux on Power Architecture CRUX PPC - http://cruxppc.org/ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/6] powerpc/time: Handle wrapping of decrementer
On Mon, 2011-11-28 at 11:44 -0600, Scott Wood wrote: > > -#ifndef CONFIG_BOOKE > > - /* On server, re-trigger the decrementer if it went negative since > > - * some processors only trigger on edge transitions of the sign bit. > > - * > > - * BookE has a level sensitive decrementer (latches in TSR) so we > > - * don't need that > > + /* > > + * Trigger the decrementer if we have a pending event. Some processors > > + * only trigger on edge transitions of the sign bit. We might also > > + * have disabled interrupts long enough that the decrementer wrapped > > + * to positive. > >*/ > > - if ((int)mfspr(SPRN_DEC) < 0) > > - mtspr(SPRN_DEC, 1); > > -#endif /* CONFIG_BOOKE */ > > + decrementer_check_overflow(); > > Where did the #ifndef CONFIG_BOOKE go? BookE doesn't need this; the > interrupt will continue asserting until software clears TSR[DIS]. Ooops, I didnt notice Anton was removing it. Please send me a followup patch to make decrementer_check_overflow() an empty inline on BookE. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC][PATCH] update FSL 16550 nodes to have...
Signed-off-by: Kumar Gala --- * Need to fixup the commit message arch/powerpc/boot/dts/asp834x-redboot.dts|4 ++-- arch/powerpc/boot/dts/fsl/pq3-duart-0.dtsi |4 ++-- arch/powerpc/boot/dts/fsl/qoriq-duart-0.dtsi |4 ++-- arch/powerpc/boot/dts/fsl/qoriq-duart-1.dtsi |4 ++-- arch/powerpc/boot/dts/gef_ppc9a.dts |4 ++-- arch/powerpc/boot/dts/gef_sbc310.dts |4 ++-- arch/powerpc/boot/dts/gef_sbc610.dts |4 ++-- arch/powerpc/boot/dts/kmeter1.dts|2 +- arch/powerpc/boot/dts/kuroboxHD.dts |4 ++-- arch/powerpc/boot/dts/kuroboxHG.dts |4 ++-- arch/powerpc/boot/dts/mpc8308_p1m.dts|4 ++-- arch/powerpc/boot/dts/mpc8308rdb.dts |4 ++-- arch/powerpc/boot/dts/mpc8313erdb.dts|4 ++-- arch/powerpc/boot/dts/mpc8315erdb.dts|4 ++-- arch/powerpc/boot/dts/mpc832x_mds.dts|4 ++-- arch/powerpc/boot/dts/mpc832x_rdb.dts|4 ++-- arch/powerpc/boot/dts/mpc8349emitx.dts |4 ++-- arch/powerpc/boot/dts/mpc8349emitxgp.dts |4 ++-- arch/powerpc/boot/dts/mpc834x_mds.dts|4 ++-- arch/powerpc/boot/dts/mpc836x_mds.dts|4 ++-- arch/powerpc/boot/dts/mpc836x_rdk.dts|4 ++-- arch/powerpc/boot/dts/mpc8377_mds.dts|4 ++-- arch/powerpc/boot/dts/mpc8377_rdb.dts|4 ++-- arch/powerpc/boot/dts/mpc8377_wlan.dts |4 ++-- arch/powerpc/boot/dts/mpc8378_mds.dts|4 ++-- arch/powerpc/boot/dts/mpc8378_rdb.dts|4 ++-- arch/powerpc/boot/dts/mpc8379_mds.dts|4 ++-- arch/powerpc/boot/dts/mpc8379_rdb.dts|4 ++-- arch/powerpc/boot/dts/mpc8540ads.dts |4 ++-- arch/powerpc/boot/dts/mpc8541cds.dts |4 ++-- arch/powerpc/boot/dts/mpc8555cds.dts |4 ++-- arch/powerpc/boot/dts/mpc8610_hpcd.dts |4 ++-- arch/powerpc/boot/dts/mpc8641_hpcn.dts |4 ++-- arch/powerpc/boot/dts/mpc8641_hpcn_36b.dts |4 ++-- arch/powerpc/boot/dts/sbc8349.dts|4 ++-- arch/powerpc/boot/dts/sbc8548.dts|4 ++-- arch/powerpc/boot/dts/sbc8641d.dts |4 ++-- arch/powerpc/boot/dts/socrates.dts |4 ++-- arch/powerpc/boot/dts/storcenter.dts |4 ++-- arch/powerpc/boot/dts/stxssa8555.dts |4 ++-- arch/powerpc/boot/dts/tqm8540.dts|4 ++-- arch/powerpc/boot/dts/tqm8541.dts|4 ++-- arch/powerpc/boot/dts/tqm8548-bigflash.dts |4 ++-- arch/powerpc/boot/dts/tqm8548.dts|4 ++-- arch/powerpc/boot/dts/tqm8555.dts|4 ++-- arch/powerpc/boot/dts/xcalibur1501.dts |4 ++-- arch/powerpc/boot/dts/xpedite5200.dts|4 ++-- arch/powerpc/boot/dts/xpedite5200_xmon.dts |4 ++-- arch/powerpc/boot/dts/xpedite5301.dts|4 ++-- arch/powerpc/boot/dts/xpedite5330.dts|4 ++-- arch/powerpc/boot/dts/xpedite5370.dts|4 ++-- 51 files changed, 101 insertions(+), 101 deletions(-) diff --git a/arch/powerpc/boot/dts/asp834x-redboot.dts b/arch/powerpc/boot/dts/asp834x-redboot.dts index 261d10c..227290d 100644 --- a/arch/powerpc/boot/dts/asp834x-redboot.dts +++ b/arch/powerpc/boot/dts/asp834x-redboot.dts @@ -256,7 +256,7 @@ serial0: serial@4500 { cell-index = <0>; device_type = "serial"; - compatible = "ns16550"; + compatible = "fsl,ns16550", "ns16550"; reg = <0x4500 0x100>; clock-frequency = <4>; interrupts = <9 0x8>; @@ -266,7 +266,7 @@ serial1: serial@4600 { cell-index = <1>; device_type = "serial"; - compatible = "ns16550"; + compatible = "fsl,ns16550", "ns16550"; reg = <0x4600 0x100>; clock-frequency = <4>; interrupts = <10 0x8>; diff --git a/arch/powerpc/boot/dts/fsl/pq3-duart-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-duart-0.dtsi index 00fa1fd..5e268fd 100644 --- a/arch/powerpc/boot/dts/fsl/pq3-duart-0.dtsi +++ b/arch/powerpc/boot/dts/fsl/pq3-duart-0.dtsi @@ -35,7 +35,7 @@ serial0: serial@4500 { cell-index = <0>; device_type = "serial"; - compatible = "ns16550"; + compatible = "fsl,ns16550", "ns16550"; reg = <0x4500 0x100>; clock-frequency = <0>; interrupts = <42 2 0 0>; @@ -44,7 +44,7 @@ serial0: serial@4500 { serial1: serial@4600 { cell-index = <1>; device_type = "serial"; - compatible = "ns16550"; + compatible = "fsl,ns16550", "ns16550"; reg = <0x4600 0x100>; clock-frequency = <0>; interrupts = <42 2 0 0>; diff --git a/arch/powerpc/boot/dts/fsl/qoriq-duart-0.dtsi b/arch/powerpc/boot/d
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On 11/28/2011 01:46 PM, Josh Boyer wrote: > On Mon, Nov 28, 2011 at 2:30 PM, Scott Wood wrote: >> On 11/28/2011 10:23 AM, Josh Boyer wrote: >>> On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala >>> wrote: Since you're fixing this can you add the following for CONFIG_BOOKE: MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM PMM is not just BookE, and is already present in the patch. RI is present on e500mc (despite being reserved in book3e), so might not want to stick that inside #ifndef CONFIG_BOOKE. >> Not all bits are going to exist on all CPUs -- does 4xx use these bits >> to mean something different? > > No, marked as reserved. However, given the patch shows up in human > readable output, I don't think we want reserved bits being decoded and > showing up inadvertently. Do the bits ever actually get set on 4xx (documented or otherwise), or is this a theoretical concern? If 4xx must be excluded, use something like: #if defined(CONFIG_BOOKE) && !defined(CONFIG_4xx) Do we also need to patch out things like MSR_VEC at runtime, in case it randomly shows up on a pre-Altivec CPU? > Could introduce BOOK3E_32 to cover cases like this. Why _32? These bits apply to 64-bit as well. MSR_CM is only for 64-bit. UCLE and PMM are present on pre-2.06 e500 cores as well. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On Mon, Nov 28, 2011 at 3:04 PM, Scott Wood wrote: > On 11/28/2011 01:46 PM, Josh Boyer wrote: >> On Mon, Nov 28, 2011 at 2:30 PM, Scott Wood wrote: >>> On 11/28/2011 10:23 AM, Josh Boyer wrote: On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala wrote: > > Since you're fixing this can you add the following for CONFIG_BOOKE: > > MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM > > PMM is not just BookE, and is already present in the patch. > > RI is present on e500mc (despite being reserved in book3e), so might not > want to stick that inside #ifndef CONFIG_BOOKE. > >>> Not all bits are going to exist on all CPUs -- does 4xx use these bits >>> to mean something different? >> >> No, marked as reserved. However, given the patch shows up in human >> readable output, I don't think we want reserved bits being decoded and >> showing up inadvertently. > > Do the bits ever actually get set on 4xx (documented or otherwise), or > is this a theoretical concern? > > If 4xx must be excluded, use something like: > #if defined(CONFIG_BOOKE) && !defined(CONFIG_4xx) I was going for something a bit simpler. Basically, CONFIG_BOOKE should be treated as a legacy Kconfig variable that has nothing to do with ISA 2.06 because it existed before that and is used by things that aren't compliant with 2.06 (both FSL and 4xx). If we use it for things that show up on these non-compliant CPUs, we'll have to continually add the !defined(WHATEVER) and that just gets ugly over time. > Do we also need to patch out things like MSR_VEC at runtime, in case it > randomly shows up on a pre-Altivec CPU? > >> Could introduce BOOK3E_32 to cover cases like this. > > Why _32? These bits apply to 64-bit as well. MSR_CM is only for 64-bit. Because CONFIG_BOOK3E depeonds on PPC_BOOK3E_64. So either that dependency needs to go so it's selectable elsewhere, or a similarly intended PPC_BOOK3E_32 needs to get created. Or something. > UCLE and PMM are present on pre-2.06 e500 cores as well. Sigh. Maybe there is no way to get un-ugly. josh ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: sam460ex, sm501 incorrect device id with kernel >=linux-2.6.39
On Mon, 28 Nov 2011 20:56:55 +0100 acrux wrote: ... > it seems to be an endianess issue but i didn't find when it was > introduced. Really strange this kind of issue was never noticed > bumping from 2.6.38.x to 2.6.39.x . Look at commit bf5f0019046d596d613caf74722ba4994e153899 (video, sm501: add I/O functions for use on powerpc). This is the issue, I think. Especially changes in include/linux/sm501.h by this commit. Since CONFIG_PPC32 is defined for canyonlands, ioread32be() is used to access the registers at PCI space which is wrong. The patch was tested on tqm5200 with sm501 connected on localbus, so using ioread32be() worked there. Your sm502 is on PCI bus I suppose. This issue needs to be fixed. HTH, Anatolij ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On 11/28/2011 02:12 PM, Josh Boyer wrote: > On Mon, Nov 28, 2011 at 3:04 PM, Scott Wood wrote: >> On 11/28/2011 01:46 PM, Josh Boyer wrote: >>> Could introduce BOOK3E_32 to cover cases like this. >> >> Why _32? These bits apply to 64-bit as well. MSR_CM is only for 64-bit. > > Because CONFIG_BOOK3E depeonds on PPC_BOOK3E_64. So either that > dependency needs to go so it's selectable elsewhere, or a similarly > intended PPC_BOOK3E_32 needs to get created. Or something. I think that dependency should go, in any case. We already have PPC_BOOK3E_64 for places that need to depend on that, and we don't want to end up having this all over the place: #if defined(CONFIG_PPC_BOOK3E_32) || defined(CONFIG_PPC_BOOK3E_64) >> UCLE and PMM are present on pre-2.06 e500 cores as well. > > Sigh. Maybe there is no way to get un-ugly. If we drop the 64-bit dependency, we could do this for UCLE if it really needs to be omitted from a 4xx kernel: #if defined(CONFIG_PPC_BOOK3E) || defined(CONFIG_E500) PMM is not just a BookE thing, so if 4xx really needs to exclude it, #ifndef CONFIG_4xx is the way to go. I wouldn't bother unless 4xx is known to set these bits, though. For GS and CM CONFIG_PPC_BOOK3E is OK, once 32-bit e500mc/e5500 kernels start selecting it. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Decode correct MSR bits in oops output
On Mon, 2011-11-28 at 10:04 -0600, Kumar Gala wrote: > > +#ifndef CONFIG_BOOKE > > + {MSR_RI,"RI"}, > > We have 'RI' on some BOOKE so lets allow for it to be decoded > > > + {MSR_LE,"LE"}, > > +#endif > > {0, NULL} > > }; > > Since you're fixing this can you add the following for CONFIG_BOOKE: > > MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM Please send a followup patch. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines
On Mon, 2011-11-28 at 16:32 +0530, Deepthi Dharwar wrote: > > Additionally, I'm a bit worried (but maybe we already discussed that a > > while back, I don't know) but cpu_idle_wait() has "wait" in the name, > > which makes me think it might need to actually -wait- for all cpus to > > have come out of the function. > > cpu_idle_wait is used to ensure that all the CPUs discard old idle > handler and update to new one. Required while changing idle > handler on SMP systems. > > > Now your implementation doesn't provide that guarantee. It might be > > fine, I don't know, but if it is, you'd better document it well in the > > comments surrounding the code, because as it is, all you do is shoot an > > interrupt which will cause the target CPU to eventually come out of idle > > some time in the future. > > > I was hoping that sending an explicit reschedule to the cpus would > do the trick but sure we can add some documentation around the code. Well, the question is what guarantee do you expect. Sending a reschedule IPI will take the other CPUs out of the actual sleep mode, but it will be some time from there back to getting out of the handler function (first back out of hypervisor etc...). The code as you implemented it doesn't wait for that to happen. It might be fine ... or not. I don't know what semantics you are after precisely. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 4/4] cpuidle: (POWER) Handle power_save=off
On Mon, 2011-11-28 at 16:33 +0530, Deepthi Dharwar wrote: > On an LPAR if cpuidle is disabled, ppc_md.power_save is still set to > cpuidle_idle_call by default here. This would result in calling of > cpuidle_idle_call repeatedly, only for the call to return -ENODEV. The > default idle is never executed. > This would be a major design flaw. No fallback idle routine. > > We propose to fix this by checking the return value of > ppc_md.power_save() call from void to int. > Right now return value is void, but if we change this to int, this > would solve two problems. One being removing the cast to a function > pointer in the prev patch and this design flaw stated above. > > So by checking the return value of ppc_md.power_save(), we can invoke > the default idle on failure. But my only concern is about the effects of > changing the ppc_md.power_save() to return int on other powerpc > architectures. Would it be a good idea to change the return type to int > which would help us flag an error and fallback to default idle? I would have preferred an approach where the cpuidle module sets ppc_md.power_save when loaded and restores it when unloaded ... but that would have to go into the cpuidle core as a powerpc specific tweak and might not be generally well received. So go for it, add the return value, but you'll have to update all the idle functions (grep for power_save in arch/powerpc to find them). Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: MPIC cleanup series
On Sun, Nov 27, 2011 at 18:51, Benjamin Herrenschmidt wrote: > Overall I really look your series. It doesn't quite apply cleanly > anymore so I'll as you for a new shoot after you address the comments > below, at which point, if you're fast enough, I'll stick it in -next :-) Awesome! Thanks! As I mentioned before, I have precious little of the hardware to test this all on, so I hope I don't break anything. At minimum I need to do a final build-and-run test on my e500 boards before I send it out. :-D > Just a couple of comments on some of the patches: > > - 5/10: search for open-pic device-tree node if NULL > > The idea is fine, however most callers ignore the device-type and only > compare on compatible, while you replace that with a match entry that > seems to require matching on both. This is likely to break stuff. The > "type" part of te march entry should be NULL I believe. If you re-read that, the match table used if no of_node is passed in has *two* separate entries, one of them with a "type" and the other with a "compatible", as opposed to a single entry which matches both "type" and "compatible". There are a lot of callers which do: dnp = of_find_node_by_type(NULL, "open-pic"); So I doubt I can remove the "type" entry all together, unfortunately. > - 9/10: cache the node > > of_node_get() is your friend. Yes, I actually messed this one up in the prior patch too, thanks for noticing. It should all be fixed now. > - 10/10: Makes me a bit nervous. It 'looks' right but I wouldn't bet on > Apple device-trees being sane vs. chaining. I would like a test that > doesn't do the cascade if the mpic is a primary to at least limit the > risk of messup. Oh, you mean to wrap that block like this? if (mpic->flags & MPIC_SECONDARY) { virq = irq_of_parse_and_map(mpic->node, 0); ... } Sure, makes sense to me. I've made that change. Thanks for the review! Cheers, Kyle Moffett -- Curious about my work on the Debian powerpcspe port? I'm keeping a blog here: http://pureperl.blogspot.com/ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: MPIC cleanup series
On Mon, 2011-11-28 at 15:48 -0500, Kyle Moffett wrote: > On Sun, Nov 27, 2011 at 18:51, Benjamin Herrenschmidt > wrote: > > Overall I really look your series. It doesn't quite apply cleanly > > anymore so I'll as you for a new shoot after you address the comments > > below, at which point, if you're fast enough, I'll stick it in -next :-) > > Awesome! Thanks! > > As I mentioned before, I have precious little of the hardware to test > this all on, so I hope I don't break anything. At minimum I need to > do a final build-and-run test on my e500 boards before I send it out. > :-D That's ok, I was planning on letting it simmer in -test for a week or so, giving myself time to test on a range of powermacs etc... > > Just a couple of comments on some of the patches: > > > > - 5/10: search for open-pic device-tree node if NULL > > > > The idea is fine, however most callers ignore the device-type and only > > compare on compatible, while you replace that with a match entry that > > seems to require matching on both. This is likely to break stuff. The > > "type" part of te march entry should be NULL I believe. > > If you re-read that, the match table used if no of_node is passed in > has *two* separate entries, one of them with a "type" and the other > with a "compatible", as opposed to a single entry which matches both > "type" and "compatible". Oh, my bad. Ok. > There are a lot of callers which do: > dnp = of_find_node_by_type(NULL, "open-pic"); > > So I doubt I can remove the "type" entry all together, unfortunately. > > > > - 9/10: cache the node > > > > of_node_get() is your friend. > > Yes, I actually messed this one up in the prior patch too, thanks for > noticing. It should all be fixed now. > > > > - 10/10: Makes me a bit nervous. It 'looks' right but I wouldn't bet on > > Apple device-trees being sane vs. chaining. I would like a test that > > doesn't do the cascade if the mpic is a primary to at least limit the > > risk of messup. > > Oh, you mean to wrap that block like this? > > if (mpic->flags & MPIC_SECONDARY) { > virq = irq_of_parse_and_map(mpic->node, 0); > ... > } Yes. > Sure, makes sense to me. I've made that change. > > Thanks for the review! Thanks. Re-post the whole series and I'll merge it. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] make gianfar eTSEC vlan hw acceleration work again.
Hi. There seems to be a breakage in the VLAN TX HW acceleration in gianfar (kernel 3.1). It seems like the previous patch that was submitted forgotten to initialize the TX registers. --- drivers/net/gianfar.c-orig 2011-11-28 11:04:09.318992481 +0100 +++ drivers/net/gianfar.c 2011-11-28 11:05:43.530990635 +0100 @@ -394,6 +394,9 @@ /* Init rctrl based on our settings */ gfar_write(®s->rctrl, rctrl); + if (ndev->features & NETIF_F_HW_VLAN_TX) + tctrl |= TCTRL_VLINS; + if (ndev->features & NETIF_F_IP_CSUM) tctrl |= TCTRL_INIT_CSUM; After this patch, it seems vlan rx/tx for eTSEC works again. Best regards Staale Aakermann CONFIDENTIALITY This e-mail and any attachment contain KONGSBERG information which may be proprietary, confidential or subject to export regulations, and is only meant for the intended recipient(s). Any disclosure, copying, distribution or use is prohibited, if not otherwise explicitly agreed with KONGSBERG. If received in error, please delete it immediately from your system and notify the sender properly. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
On 11/23/2011 06:14 AM, LiuShuo wrote: > 于 2011年11月23日 07:55, Scott Wood 写道: >> On 11/15/2011 03:29 AM, b35...@freescale.com wrote: >>> From: Liu Shuo >>> >>> -if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 || >>> +if (elbc_fcm_ctrl->column>= mtd->writesize) { >>> +/* write oob */ >>> +if (priv->page_size> 1) { >>> +/* when pagesize of chip is greater than 2048, >>> + * we have to write full page to write spare >>> + * region, so we fill '0xff' to main region >>> + * and some bytes of spare region which we >>> + * don't want to rewrite. >>> + * (write '1' won't change the original value) >>> + */ >>> +memset(elbc_fcm_ctrl->buffer, 0xff, >>> +elbc_fcm_ctrl->column); >> I don't like relying on this -- can we use RNDIN instead to do a >> discontiguous write? >> > I have no better way to implement it now. > Some chips have 'NOP' limitation, so I don't use the FIR_OP_UA to do a > oob write. I don't think each RNDIN counts separately against NOP (someone correct me if I'm wrong). You're writing discontiguous regions of the page in one operation. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
On 11/23/2011 06:41 PM, b35...@freescale.com wrote: > From: Liu Shuo > > Freescale FCM controller has a 2K size limitation of buffer RAM. In order > to support the Nand flash chip whose page size is larger than 2K bytes, > we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save > them to a large buffer. > > Signed-off-by: Liu Shuo > Signed-off-by: Li Yang > --- > drivers/mtd/nand/fsl_elbc_nand.c | 211 > +++--- > 1 files changed, 194 insertions(+), 17 deletions(-) > > diff --git a/drivers/mtd/nand/fsl_elbc_nand.c > b/drivers/mtd/nand/fsl_elbc_nand.c > index d634c5f..c96e714 100644 > --- a/drivers/mtd/nand/fsl_elbc_nand.c > +++ b/drivers/mtd/nand/fsl_elbc_nand.c > @@ -55,7 +55,9 @@ struct fsl_elbc_mtd { > struct device *dev; > int bank; /* Chip select bank number */ > u8 __iomem *vbase; /* Chip select base virtual address */ > - int page_size; /* NAND page size (0=512, 1=2048)*/ > + int page_size; /* NAND page size, the mutiple of 2048. > + * (0=512, 1=2048, 2=4096, 4=8192) > + */ Again, please remove this. It was sort-of reasonable when it was a boolean that selected between slightly different programming models. It doesn't make sense as "mtd->writesize == 512 ? 0 : mtd->writesize / 512". What is the plan for migrating bad block markers on first use? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
On 11/28/2011 03:48 PM, Scott Wood wrote: > On 11/23/2011 06:41 PM, b35...@freescale.com wrote: >> From: Liu Shuo >> >> Freescale FCM controller has a 2K size limitation of buffer RAM. In order >> to support the Nand flash chip whose page size is larger than 2K bytes, >> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save >> them to a large buffer. >> >> Signed-off-by: Liu Shuo >> Signed-off-by: Li Yang >> --- >> drivers/mtd/nand/fsl_elbc_nand.c | 211 >> +++--- >> 1 files changed, 194 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c >> b/drivers/mtd/nand/fsl_elbc_nand.c >> index d634c5f..c96e714 100644 >> --- a/drivers/mtd/nand/fsl_elbc_nand.c >> +++ b/drivers/mtd/nand/fsl_elbc_nand.c >> @@ -55,7 +55,9 @@ struct fsl_elbc_mtd { >> struct device *dev; >> int bank; /* Chip select bank number */ >> u8 __iomem *vbase; /* Chip select base virtual address */ >> -int page_size; /* NAND page size (0=512, 1=2048)*/ >> +int page_size; /* NAND page size, the mutiple of 2048. >> + * (0=512, 1=2048, 2=4096, 4=8192) >> + */ > > Again, please remove this. It was sort-of reasonable when it was a > boolean that selected between slightly different programming models. It > doesn't make sense as "mtd->writesize == 512 ? 0 : mtd->writesize / 512". Sorry, I meant "mtd->writesize == 512 ? 0 : mtd->writesize / 2048". -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 03/13] powerpc: Fix booke hugetlb preload code for PPC_MM_SLICES and 64-bit
> return; > > #ifdef CONFIG_PPC_MM_SLICES > - psize = mmu_get_tsize(get_slice_psize(mm, ea)); > - tsize = mmu_get_psize(psize); > + psize = get_slice_psize(mm, ea); > + tsize = mmu_get_tsize(psize); > shift = mmu_psize_defs[psize].shift; > #else > - vma = find_vma(mm, ea); > - psize = vma_mmu_pagesize(vma); /* returns actual size in bytes */ > - asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize)); > - shift = 31 - lz; > - tsize = 21 - lz; > + psize = vma_mmu_pagesize(find_vma(mm, ea)); > + shift = __ilog2(psize); > + tsize = shift - 10; > #endif > BTW. Can you remind me what is the business with slices vs. no slices on Book3E ? I'd like to avoid having to build separate kernels for A2 vs. FSL ... Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 2/8] [booke] Rename mapping based RELOCATABLE to DYNAMIC_MEMSTART for BookE
On 11/23/2011 10:47 AM, Josh Boyer wrote: > On Mon, Nov 14, 2011 at 12:41 AM, Suzuki K. Poulose wrote: >> The current implementation of CONFIG_RELOCATABLE in BookE is based >> on mapping the page aligned kernel load address to KERNELBASE. This >> approach however is not enough for platforms, where the TLB page size >> is large (e.g, 256M on 44x). So we are renaming the RELOCATABLE used >> currently in BookE to DYNAMIC_MEMSTART to reflect the actual method. Should reword the config help to make it clear what the alignment restriction is, or where to find the information for a particular platform. Someone reading "page aligned" without any context that we're talking about special large pages is going to think 4K -- and on e500, many large page sizes are supported, so the required alignment is found in Linux init code rather than a CPU manual. >> >> The CONFIG_RELOCATABLE for PPC32(BookE) based on processing of the >> dynamic relocations will be introduced in the later in the patch series. >> >> This change would allow the use of the old method of RELOCATABLE for >> platforms which can afford to enforce the page alignment (platforms with >> smaller TLB size). > > I'm OK with the general direction, but this touches a lot of non-4xx > code. I'd prefer it if Ben took this directly on whatever final > solution is done. > >> I haven tested this change only on 440x. I don't have an FSL BookE to verify >> the changes there. >> >> Scott, >> Could you please test this patch on FSL and let me know the results ? > > Scott, did you ever get around to testing this? In my opinion, this > shouldn't go in without a Tested-by: from someone that tried it on an > FSL platform. Booted OK for me on e500v2 with RAM starting at 256M. Tested-by: Scott Wood > We add DYNAMIC_MEMSTART for 32-bit, and we have RELOCATABLE for > 64-bit. Then throughout almost the rest of the patch, all we're doing > is duplicating what RELOCATABLE already did (e.g. if ! either thing). > It works, but it is kind of ugly. > > Instead, could we define a helper config variable that can be used in > place of that construct? Something like: > > config NONSTATIC_KERNEL (or whatever) > bool > default n > > ... > > config DYNAMIC_MEMSTART > > select NONSTATIC_KERNEL > > ... > > config RELOCATABLE > > select NONSTATIC_KERNEL I agree. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH-RFC 01/10] lib: move GENERIC_IOMAP to lib/Kconfig
On Thu, Nov 24, 2011 at 10:15:42PM +0200, Michael S. Tsirkin wrote: > define GENERIC_IOMAP in a central location > instead of all architectures. This will be helpful > for the follow-up patch which makes it select > other configs. Code is also a bit shorter this way. For the Hexagon config, Acked-by: Richard Kuo -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/6] powerpc/time: Use clocksource_register_hz
On Thu, 2011-11-24 at 17:07 +1100, Anton Blanchard wrote: > plain text document attachment (clock3) > Use clocksource_register_hz which calculates the shift/mult > factors for us. Also remove the shift = 22 assumption in > vsyscall_update - thanks to Paul Mackerras and John Stultz for > catching that. > > Signed-off-by: Anton Blanchard > --- > > Index: linux-build/arch/powerpc/kernel/time.c > === > --- linux-build.orig/arch/powerpc/kernel/time.c 2011-11-17 > 10:11:51.175038860 +1100 > +++ linux-build/arch/powerpc/kernel/time.c2011-11-17 10:11:55.547114957 > +1100 > @@ -86,8 +86,6 @@ static struct clocksource clocksource_rt > .rating = 400, > .flags= CLOCK_SOURCE_IS_CONTINUOUS, > .mask = CLOCKSOURCE_MASK(64), > - .shift= 22, > - .mult = 0, /* To be filled in */ > .read = rtc_read, > }; > > @@ -97,8 +95,6 @@ static struct clocksource clocksource_ti > .rating = 400, > .flags= CLOCK_SOURCE_IS_CONTINUOUS, > .mask = CLOCKSOURCE_MASK(64), > - .shift= 22, > - .mult = 0, /* To be filled in */ > .read = timebase_read, > }; > > @@ -822,9 +818,8 @@ void update_vsyscall(struct timespec *wa > ++vdso_data->tb_update_count; > smp_mb(); > > - /* XXX this assumes clock->shift == 22 */ > - /* 4611686018 ~= 2^(20+64-22) / 1e9 */ > - new_tb_to_xs = (u64) mult * 4611686018ULL; > + /* 19342813113834067 ~= 2^(20+64) / 1e9 */ > + new_tb_to_xs = (u64) mult * (19342813113834067ULL >> clock->shift); I never verified the math on this, but assuming Paul had it right, this patch looks good! Acked-by: John Stultz Thanks again! -john ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
Amit, You said that the work would be serialized "due to port additions being on work items on the same workqueue". I'm not seeing that. I've double checked this by using a mutex_trylock in hvc_console::hvc_alloc(), and here's the relevant output from dmesg: root@myubuntu:~# dmesg | grep MBH [3307216.210274] MBH: got hvc_ports_mutex [3307216.210690] MBH: trylock of hvc_ports_mutex failed [3307216.211143] MBH: got hvc_ports_mutex This is in a system with two virtio console ports, each of which is a console. I think if the VIRTIO_CONSOLE_CONSOLE_PORT message handling were actually being serialized, the trylock should never fail. What's the source of the serialization for the workqueue items? At first reading it looks like the control_work_handler gets called for each virtio interrupt? Miche On Wed, Nov 23, 2011 at 2:38 AM, Amit Shah wrote: > On (Thu) 17 Nov 2011 [10:57:37], Miche Baker-Harvey wrote: >> Rusty, Michael, Stephen, et al, >> >> Thanks for your comments on these patches. >> >> For what I'm trying to do, all three patches are necessary, but maybe >> I'm going about it the wrong way. Your input would be appreciated. >> I'm in no way claiming that these patches are "right", just that it's >> working for me, and that what's in the current pool is not. >> >> What I'm trying to do is: >> On X86, >> under KVM, >> start a virtio console device, >> with multiple ports on the device, >> at least one of which is also a console (as well as ttyS0). >> >> (Eventually, we want to be able to add virtio console ports on the >> fly, and to have multiple virtio console ports be consoles.) > > Are you using kvm-tool to do this? QEMU can already hot-plug ports > and virtio-console (virtio-serial) devices. > >> When all three of the patches are in place, this works great. (By >> great, I mean that getty's start up on all of ttyS0, hvc0 and hvc1, >> and console output goes to ttyS0 and to hvc0. >> "who" shows three users: ttyS0, hvc0, and hvc1. >> "cat /proc/consoles" shows both ttyS0 and hvc0. >> I can use all three getty's, and console output really does appear on >> both the consoles. >> >> Based on Rusty's comments, I tried removing each of the patches >> individually. Here's what happens today. I've seen other failure modes >> depending on what precisely I'm passing the guest. >> There's three patches: >> 1/3 "fix locking of vtermno" >> 2/3 "enforce one-time initialization with hvc_init >> "3/3 "use separate struct console * for each console" >> >> If I remove the "fix locking of vtermno", I only get one virtio >> console terminal. "who" shows the ttyS0 and the hvc0, and I can log >> into the gettys on both. I don't get the second virtio console getty. >> Interestingly, hvc0 shows up in /proc/consoles twice, and in fact the >> console output is dumped twice to hvc0 (as you'd expect from looking >> at printk.c, each line appears twice, followed by the next line.) > > I don't really understand why. "fix locking of vtermno" adds locks in > init_port_console(), which is called from add_port(), which should be > serialised due to port additions being on work items on the same > workqueue. I don't see a race here. > >> If I remove the "enforce one-time initialization with hvc_init" patch, >> which makes sure only a single thread is doing the hvc_init, and gates >> anyone from continuing until it has completed, I get different >> failures, including hangs, and dereferences of NULL pointers. >> >> If I remove the "use separate struct console * for each console"patch, >> what I'm seeing now is that while all of ttyS0, hvc0, and hvc1 are >> present with gettys running on them, of the three, only ttyS0 is a >> console. > > I don't see any difference in my testing with and without these > patches. > > This is how I tested with qemu: > > ./x86_64-softmmu/qemu-system-x86_64 -m 512 -smp 2 -chardev > socket,path=/tmp/foo,server,nowait,id=foo -chardev > socket,path=/tmp/bar,server,nowait,id=bar -device virtio-serial > -device virtconsole,chardev=foo,nr=4 -device > virtconsole,chardev=bar,nr=3 -net none -kernel > /home/amit/src/linux/arch/x86/boot/bzImage -append 'root=/dev/sda1 > console=tty0 console=ttyS0' -initrd /tmp/initramfs.img > /guests/f14-nolvm.qcow2 -enable-kvm -snapshot > > With this setup, with and without patches, I can spawn two consoles > via: > > /sbin/agetty /dev/hvc0 9600 vt100 > /sbin/agetty /dev/hvc1 9600 vt100 > > (Strange thing is, the second one gives a 'password incorrect' error > on login attempts, while the first one logs in fine. I do remember > testing multiple consoles just fine a year and a half back, so no idea > why this isn't behaving as expected -- but it mostly looks like a > userspace issue rather than kernel one.) > > As mentioned earlier, I've not looked at the hvc code, but given my > testing results, I'd like to first understand what you're seeing and > what your environment is. > > Amit > ___ Linuxppc-dev mailing li
Re: [PATCH 2/6] powerpc/85xx: consolidate of_platform_bus_probe calls
On Thu, Nov 17, 2011 at 11:56 AM, Dmitry Eremin-Solenikov wrote: > diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c > b/arch/powerpc/platforms/85xx/p1022_ds.c > index 00d93a4..cacb4d4 100644 > --- a/arch/powerpc/platforms/85xx/p1022_ds.c > +++ b/arch/powerpc/platforms/85xx/p1022_ds.c > @@ -330,10 +330,6 @@ static void __init p1022_ds_setup_arch(void) > } > > static struct of_device_id __initdata p1022_ds_ids[] = { > - { .type = "soc", }, > - { .compatible = "soc", }, > - { .compatible = "simple-bus", }, > - { .compatible = "gianfar", }, > /* So that the DMA channel nodes can be probed individually: */ > { .compatible = "fsl,eloplus-dma", }, > {}, > @@ -343,6 +339,7 @@ static int __init p1022_ds_publish_devices(void) > { > return of_platform_bus_probe(NULL, p1022_ds_ids, NULL); > } > +machine_device_initcall(p1022_ds, mpc85xx_common_publish_devices); > machine_device_initcall(p1022_ds, p1022_ds_publish_devices); I don't think this is working. I need to investigate some more to be sure, but it looks like this is not picking up "fsl,eloplus-dma". None of the DMA channels are being probed in the audio driver (sound/soc/fsl_dma.c). -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/book3e: Change hugetlb preload to take vma argument
From: Becky Bruce This avoids an extra find_vma() and is less error-prone. Signed-off-by: Becky Bruce --- arch/powerpc/include/asm/hugetlb.h |3 ++- arch/powerpc/mm/hugetlbpage-book3e.c |8 ++-- arch/powerpc/mm/mem.c|2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h index 555044c..863f49d 100644 --- a/arch/powerpc/include/asm/hugetlb.h +++ b/arch/powerpc/include/asm/hugetlb.h @@ -52,7 +52,8 @@ static inline int is_hugepage_only_range(struct mm_struct *mm, } #endif -void book3e_hugetlb_preload(struct mm_struct *mm, unsigned long ea, pte_t pte); +void book3e_hugetlb_preload(struct vm_area_struct *vma, unsigned long ea, + pte_t pte); void flush_hugetlb_page(struct vm_area_struct *vma, unsigned long vmaddr); void hugetlb_free_pgd_range(struct mmu_gather *tlb, unsigned long addr, diff --git a/arch/powerpc/mm/hugetlbpage-book3e.c b/arch/powerpc/mm/hugetlbpage-book3e.c index 4d6d849..3bc7006 100644 --- a/arch/powerpc/mm/hugetlbpage-book3e.c +++ b/arch/powerpc/mm/hugetlbpage-book3e.c @@ -37,12 +37,14 @@ static inline int book3e_tlb_exists(unsigned long ea, unsigned long pid) return found; } -void book3e_hugetlb_preload(struct mm_struct *mm, unsigned long ea, pte_t pte) +void book3e_hugetlb_preload(struct vm_area_struct *vma, unsigned long ea, + pte_t pte) { unsigned long mas1, mas2; u64 mas7_3; unsigned long psize, tsize, shift; unsigned long flags; + struct mm_struct *mm; #ifdef CONFIG_PPC_FSL_BOOK3E int index, ncams; @@ -51,12 +53,14 @@ void book3e_hugetlb_preload(struct mm_struct *mm, unsigned long ea, pte_t pte) if (unlikely(is_kernel_addr(ea))) return; + mm = vma->vm_mm; + #ifdef CONFIG_PPC_MM_SLICES psize = get_slice_psize(mm, ea); tsize = mmu_get_tsize(psize); shift = mmu_psize_defs[psize].shift; #else - psize = vma_mmu_pagesize(find_vma(mm, ea)); + psize = vma_mmu_pagesize(vma); shift = __ilog2(psize); tsize = shift - 10; #endif diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index 4dbc388..846065c 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -552,6 +552,6 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, #if (defined(CONFIG_PPC_BOOK3E_64) || defined(CONFIG_PPC_FSL_BOOK3E)) \ && defined(CONFIG_HUGETLB_PAGE) if (is_vm_hugetlb_page(vma)) - book3e_hugetlb_preload(vma->vm_mm, address, *ptep); + book3e_hugetlb_preload(vma, address, *ptep); #endif } -- 1.5.6.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: sam460ex, sm501 incorrect device id with kernel >=linux-2.6.39
>Da: ag...@denx.de >Data: 28/11/2011 21.22 >A: "acrux" >Cc: "Josh Boyer", >Ogg: Re: sam460ex, sm501 incorrect device id with kernel >=linux-2.6.39 > >On Mon, 28 Nov 2011 20:56:55 +0100 >acrux wrote: >... >> it seems to be an endianess issue but i didn't find when it was >> introduced. Really strange this kind of issue was never noticed >> bumping from 2.6.38.x to 2.6.39.x . > >Look at commit bf5f0019046d596d613caf74722ba4994e153899 >(video, sm501: add I/O functions for use on powerpc). >This is the issue, I think. Especially changes in include/linux/sm501.h >by this commit. Since CONFIG_PPC32 is defined for canyonlands, >ioread32be() is used to access the registers at PCI space which >is wrong. The patch was tested on tqm5200 with sm501 connected >on localbus, so using ioread32be() worked there. Your sm502 is on >PCI bus I suppose. This issue needs to be fixed. > >HTH, >Anatolij > hallo Anatolij, you are absolutely right altought i don't have a skill to fix it. Indeed, this SM502 is located on PCI bus. Here a schema: http://oi39.tinypic.com/34r9mw2.jpg cheers, --nico ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
> Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing > spinlock use. > > On Thu, Nov 24, 2011 at 08:12:25AM +, Shi Xuelin-B29237 wrote: > > Hi Ira, > > > > Thanks for your review. > > > > After second thought, I think your scenario may not occur. > > Because the cookie 20 we query must be returned by fsl_dma_tx_submit(...) in > practice. > > We never query a cookie not returned by fsl_dma_tx_submit(...). > > > > I agree about this part. > > > When we call fsl_tx_status(20), the chan->common.cookie is definitely wrote > > as > 20 and cpu2 could not read as 19. > > > > This is what I don't agree about. However, I'm not an expert on CPU cache vs. > memory accesses in an multi-processor system. The section titled "CACHE > COHERENCY" in Documentation/memory-barriers.txt leads me to believe that the > scenario I described is possible. For Freescale PowerPC, the chip automatically takes care of cache coherency. Even if this is a concern, spinlock can't address it. > > What happens if CPU1's write of chan->common.cookie only goes into CPU1's > cache. It never makes it to main memory before CPU2 fetches the old value of > 19. > > I don't think you should see any performance impact from the smp_mb() > operation. Smp_mb() do have impact on performance if it's in the hot path. While it might be safer having it, I doubt it is really necessary. If the CPU1 doesn't have the updated last_used, it's shouldn't have known there is a cookie 20 existed either. - Leo > > Thanks, > Ira > > > -Original Message- > > From: Ira W. Snyder [mailto:i...@ovro.caltech.edu] > > Sent: 2011年11月23日 2:59 > > To: Shi Xuelin-B29237 > > Cc: dan.j.willi...@intel.com; Li Yang-R58472; z...@zh-kernel.org; > > vinod.k...@intel.com; linuxppc-dev@lists.ozlabs.org; > > linux-ker...@vger.kernel.org > > Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing > spinlock use. > > > > On Tue, Nov 22, 2011 at 12:55:05PM +0800, b29...@freescale.com wrote: > > > From: Forrest Shi > > > > > > dma status check function fsl_tx_status is heavily called in > > > a tight loop and the desc lock in fsl_tx_status contended by > > > the dma status update function. this caused the dma performance > > > degrades much. > > > > > > this patch releases the lock in the fsl_tx_status function. > > > I believe it has no neglect impact on the following call of > > > dma_async_is_complete(...). > > > > > > we can see below three conditions will be identified as success > > > a) x < complete < use > > > b) x < complete+N < use+N > > > c) x < complete < use+N > > > here complete is the completed_cookie, use is the last_used > > > cookie, x is the querying cookie, N is MAX cookie > > > > > > when chan->completed_cookie is being read, the last_used may > > > be incresed. Anyway it has no neglect impact on the dma status > > > decision. > > > > > > Signed-off-by: Forrest Shi > > > --- > > > drivers/dma/fsldma.c |5 - > > > 1 files changed, 0 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index > > > 8a78154..1dca56f 100644 > > > --- a/drivers/dma/fsldma.c > > > +++ b/drivers/dma/fsldma.c > > > @@ -986,15 +986,10 @@ static enum dma_status fsl_tx_status(struct > dma_chan *dchan, > > > struct fsldma_chan *chan = to_fsl_chan(dchan); > > > dma_cookie_t last_complete; > > > dma_cookie_t last_used; > > > - unsigned long flags; > > > - > > > - spin_lock_irqsave(&chan->desc_lock, flags); > > > > > > > This will cause a bug. See below for a detailed explanation. You need this > > instead: > > > > /* > > * On an SMP system, we must ensure that this CPU has seen the > > * memory accesses performed by another CPU under the > > * chan->desc_lock spinlock. > > */ > > smp_mb(); > > > last_complete = chan->completed_cookie; > > > last_used = dchan->cookie; > > > > > > - spin_unlock_irqrestore(&chan->desc_lock, flags); > > > - > > > dma_set_tx_state(txstate, last_complete, last_used, 0); > > > return dma_async_is_complete(cookie, last_complete, last_used); } > > > > Facts: > > - dchan->cookie is the same member as chan->common.cookie (same memory > > location) > > - chan->common.cookie is the "last allocated cookie for a pending > > transaction" > > - chan->completed_cookie is the "last completed transaction" > > > > I have replaced "dchan->cookie" with "chan->common.cookie" in the below > explanation, to keep everything referenced from the same structure. > > > > Variable usage before your change. Everything is used locked. > > - RW chan->common.cookie(fsl_dma_tx_submit) > > - R chan->common.cookie(fsl_tx_status) > > - R chan->completed_cookie (fsl_tx_status) > > - W chan->completed_cookie (dma_do_tasklet) > > > > Variable usage after your change: > > - RW chan->common.cookieLOCKED > > - R chan
RE: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
Hi Ira, see my comments below. Thanks, Forrest -Original Message- From: Ira W. Snyder [mailto:i...@ovro.caltech.edu] Sent: 2011年11月29日 0:38 To: Shi Xuelin-B29237 Cc: vinod.k...@intel.com; dan.j.willi...@intel.com; linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; linux-ker...@vger.kernel.org Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use. On Thu, Nov 24, 2011 at 08:12:25AM +, Shi Xuelin-B29237 wrote: > Hi Ira, > > Thanks for your review. > > After second thought, I think your scenario may not occur. > Because the cookie 20 we query must be returned by fsl_dma_tx_submit(...) in > practice. > We never query a cookie not returned by fsl_dma_tx_submit(...). > I agree about this part. > When we call fsl_tx_status(20), the chan->common.cookie is definitely wrote > as 20 and cpu2 could not read as 19. > This is what I don't agree about. However, I'm not an expert on CPU cache vs. memory accesses in an multi-processor system. The section titled "CACHE COHERENCY" in Documentation/memory-barriers.txt leads me to believe that the scenario I described is possible. [Shi Xuelin-B29237] If query is used without rules, your scenario may happen. But in DMA usage here, the query is used something like sequentially. Only after chan->common.cookie is updated in fsl_dma_tx_submit(...) and returned, then it could be queried. So you scenario will not happen. What happens if CPU1's write of chan->common.cookie only goes into CPU1's cache. It never makes it to main memory before CPU2 fetches the old value of 19. [Shi Xuelin-B29237] As you see, chan->common.cookie is updated in fsl_dma_tx_submit(...) and enclosed by spinlock. The spinlock implementation in PPC will guarantee the cache coherence among cores, something like you called implicit smp_mb. I don't think you should see any performance impact from the smp_mb() operation. [Shi Xuelin-B29237] Only add smp_mb() doesn't work. It only sync on this step, but next step is the same as just getting into this function without smp_mb operation. Thanks, Ira > -Original Message- > From: Ira W. Snyder [mailto:i...@ovro.caltech.edu] > Sent: 2011年11月23日 2:59 > To: Shi Xuelin-B29237 > Cc: dan.j.willi...@intel.com; Li Yang-R58472; z...@zh-kernel.org; > vinod.k...@intel.com; linuxppc-dev@lists.ozlabs.org; > linux-ker...@vger.kernel.org > Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing > spinlock use. > > On Tue, Nov 22, 2011 at 12:55:05PM +0800, b29...@freescale.com wrote: > > From: Forrest Shi > > > > dma status check function fsl_tx_status is heavily called in > > a tight loop and the desc lock in fsl_tx_status contended by > > the dma status update function. this caused the dma performance > > degrades much. > > > > this patch releases the lock in the fsl_tx_status function. > > I believe it has no neglect impact on the following call of > > dma_async_is_complete(...). > > > > we can see below three conditions will be identified as success > > a) x < complete < use > > b) x < complete+N < use+N > > c) x < complete < use+N > > here complete is the completed_cookie, use is the last_used > > cookie, x is the querying cookie, N is MAX cookie > > > > when chan->completed_cookie is being read, the last_used may > > be incresed. Anyway it has no neglect impact on the dma status > > decision. > > > > Signed-off-by: Forrest Shi > > --- > > drivers/dma/fsldma.c |5 - > > 1 files changed, 0 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index > > 8a78154..1dca56f 100644 > > --- a/drivers/dma/fsldma.c > > +++ b/drivers/dma/fsldma.c > > @@ -986,15 +986,10 @@ static enum dma_status fsl_tx_status(struct dma_chan > > *dchan, > > struct fsldma_chan *chan = to_fsl_chan(dchan); > > dma_cookie_t last_complete; > > dma_cookie_t last_used; > > - unsigned long flags; > > - > > - spin_lock_irqsave(&chan->desc_lock, flags); > > > > This will cause a bug. See below for a detailed explanation. You need this > instead: > > /* >* On an SMP system, we must ensure that this CPU has seen the >* memory accesses performed by another CPU under the >* chan->desc_lock spinlock. >*/ > smp_mb(); > > last_complete = chan->completed_cookie; > > last_used = dchan->cookie; > > > > - spin_unlock_irqrestore(&chan->desc_lock, flags); > > - > > dma_set_tx_state(txstate, last_complete, last_used, 0); > > return dma_async_is_complete(cookie, last_complete, last_used); } > > Facts: > - dchan->cookie is the same member as chan->common.cookie (same memory > location) > - chan->common.cookie is the "last allocated cookie for a pending transaction" > - chan->completed_cookie is the "last completed transaction" > > I have replaced "dchan->cookie" with "chan->common.cookie" in the below > exp
Re: [PATCH 02/13] powerpc: hugetlb: fix huge_ptep_set_access_flags return value
On Mon, 2011-10-10 at 15:50 -0500, Becky Bruce wrote: > diff --git a/arch/powerpc/include/asm/hugetlb.h > b/arch/powerpc/include/asm/hugetlb.h > index 8600493..70f9885 100644 > --- a/arch/powerpc/include/asm/hugetlb.h > +++ b/arch/powerpc/include/asm/hugetlb.h > @@ -124,7 +124,18 @@ static inline int huge_ptep_set_access_flags(struct > vm_area_struct *vma, >unsigned long addr, pte_t *ptep, >pte_t pte, int dirty) > { > +#if defined(CONFIG_PPC_MMU_NOHASH) && \ > + !(defined(CONFIG_PPC_FSL_BOOK3E) && defined(CONFIG_PPC32)) The above conditional makes my brain hurt. Can you change that to instead #ifdef HUGETLB_NEED_PRELOAD ... or something like that, which you then #define in the right mmu-.h header ? Cheers, Ben. > + /* > + * The "return 1" forces a call of update_mmu_cache, which will write a > + * TLB entry. Without this, platforms that don't do a write of the TLB > + * entry in the TLB miss handler asm will fault ad infinitum. > + */ > + ptep_set_access_flags(vma, addr, ptep, pte, dirty); > + return 1; > +#else > return ptep_set_access_flags(vma, addr, ptep, pte, dirty); > +#endif > } > > static inline pte_t huge_ptep_get(pte_t *ptep) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
boot_cpuid and init_thread_info.cpu are redundant, just use the var that stays around longer and add a define to make boot_cpuid point at the correct value boot_cpudid_phys is not needed and can completly go away from the SMP case, we leave it there for the non-SMP case since the paca struct is not around to store this info This patch also has the effect of having the logical cpu number of the boot cpu be updated correctly independently of the ordering of the cpu nodes in the device tree. Signed-off-by: Matthew McClintock --- v2: Fix compile issue for peries Remove '-1' initial value arch/powerpc/include/asm/smp.h |2 +- arch/powerpc/kernel/setup_32.c |5 +++-- arch/powerpc/kernel/setup_64.c |1 - arch/powerpc/sysdev/xics/xics-common.c |1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h index adba970..f26c554 100644 --- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h @@ -29,7 +29,7 @@ #endif #include -extern int boot_cpuid; +#define boot_cpuid (init_thread_info.cpu) extern int spinning_secondaries; extern void cpu_die(void); diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c index ac76108..8d4df4c 100644 --- a/arch/powerpc/kernel/setup_32.c +++ b/arch/powerpc/kernel/setup_32.c @@ -46,10 +46,11 @@ extern void bootx_init(unsigned long r4, unsigned long phys); -int boot_cpuid = -1; -EXPORT_SYMBOL_GPL(boot_cpuid); +/* we need a place to store phys cpu for non-SMP case */ +#ifndef CONFIG_SMP int boot_cpuid_phys; EXPORT_SYMBOL_GPL(boot_cpuid_phys); +#endif int smp_hw_index[NR_CPUS]; diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index fb9bb46..6d0f00f 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -73,7 +73,6 @@ #define DBG(fmt...) #endif -int boot_cpuid = 0; int __initdata spinning_secondaries; u64 ppc64_pft_size; diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c index d72eda6..8998b7a 100644 --- a/arch/powerpc/sysdev/xics/xics-common.c +++ b/arch/powerpc/sysdev/xics/xics-common.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include -- 1.7.6.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 04/13] powerpc: Update hugetlb huge_pte_alloc and tablewalk code for FSL BOOKE
On Mon, 2011-10-10 at 15:50 -0500, Becky Bruce wrote: > From: Becky Bruce > > This updates the hugetlb page table code to handle 64-bit FSL_BOOKE. > The previous 32-bit work counted on the inner levels of the page table > collapsing. Seriously, my brain hurts !!! So I've tried to understand that code and so far, what I came up with is this, please reply and let me know if I'm full of crack or what ... - David's code has entire levels "branching off" into hugepd's which are hugetlb specific page dirs. That requires address space constrainst. - The hack you do with HUGEPD_PGD_SHIFT defined to PGDIR_SHIFT and HUGEPD_PUD_SHIFT to PUD_SHIFT consists of collasping that back into the normal levels. - I really don't understand what you are doing in __hugepte_alloc(). It seems to me that you are trying to make things still point to some kind of separate hugepd dir with the hugeptes in it and have the page tables point to that but I don't quite get it. - Couldn't we just instead ditch the whole hugepd concept alltogether and simply have the huge ptes in the page table at the right level, using possibly multiple consecutive of them for a single page when needed ? Example: 4K base page size. PMD maps 2M. a 16M page could be representing by having 8 consecutive hugeptes pointing to the same huge page in the PMD directory. I believe we always "know" when accessing a PTE whether it's going to be huge or not and if it's huge, the page size. IE. All the functions we care about either get the vma (which gets you everything you need) or the size (huge_pte_alloc). This should be much simpler than what we have today. That way, we have a completely generic accross-the-board way to store huge pte's in our page tables, which is totally disconnected from the slices. The later remains a purely server-only thing which only affects the SLB code and get_unmapped_area(). That means that we'll have to find another option for PowerEN giant indirects but that's a non issue at the moment. I think we can keep the complexity local to the PowerEN code by doing shadows there if we need to. What do you think ? Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/40x: Add APM8018X SOC support
On Mon, Nov 28, 2011 at 4:49 AM, Benjamin Herrenschmidt wrote: > On Fri, 2011-11-25 at 17:49 +0530, Tanmay Inamdar wrote: > >> >> > >> > +#if defined(CONFIG_APM8018X) >> > +/* CPR */ >> > +#define DCRN_CPR0_CONFIG_ADDR 0xa >> > +#define DCRN_CPR1_CONFIG_DATA 0xb >> > +/* AHB */ >> > +#define DCRN_SDR1_CONFIG_ADDR 0xc >> > +#define DCRN_SDR1_CONFIG_DATA 0xd >> > +#else >> > /* CPRs (440GX and 440SP/440SPe) */ >> > #define DCRN_CPR0_CONFIG_ADDR 0xc >> > #define DCRN_CPR0_CONFIG_DATA 0xd >> > +#endif /* CONFIG_APM8018X */ >> >> >> same comment as above. >> >> Some existing drivers use these macros. If I change the names, I will >> have to update the >> driver code. > > Right, the best approach is to create a small wrapper that provides cpr > and sdr accesses using helpers. That way you can: > > - Properly lock since it's all indirect > > - Obtain the right DCRs at boot time, stick them in variables > and use them at runtime, avoiding the hard coded constants completely > > - Make the code generally look much better > > Ie. Provide something like read_sdr1() and write_sdr1() accessors and > change the drivers to use them. Ok. There are 'mfdcri' and 'mtdcri' macros in "arch/powerpc/include/asm/dcr-native.h" file. They internally use '__mfdcri' and '__mtdcri' functions which can be used for the same purpose as mentioned above. Instead of putting this change in current patch, I will make separate patch for this purpose. > >> > diff --git a/arch/powerpc/kernel/cputable.c >> b/arch/powerpc/kernel/cputable.c >> > index edae5bb..e5c51a6 100644 >> > --- a/arch/powerpc/kernel/cputable.c >> > +++ b/arch/powerpc/kernel/cputable.c >> > @@ -1505,6 +1505,58 @@ static struct cpu_spec __initdata >> cpu_specs[] = { >> > .machine_check = machine_check_4xx, >> > .platform = "ppc405", >> > }, >> > + { /* APM80186-SK */ >> > + .pvr_mask = 0x, >> > + .pvr_value = 0x7ff11432, >> >> >> If you mask out the lower bits, you only need a single entry >> instead of >> four identical ones. >> >> You are right. But each PVR represent different version of SOC. If I >> create single entry, then I will have to give generic cpu_name which I >> don't want. > > Note that you should really tell you designers to move away from using > the PVR to identify the SoC's. This is BAD and isn't sustainable long > run. Stick to representing the core itself in the PVR and provide a > different mechanism to identify the SoC (different SPR would do, or even > a DCR). > >> > --- a/arch/powerpc/kernel/udbg_16550.c >> > +++ b/arch/powerpc/kernel/udbg_16550.c >> > @@ -18,6 +18,19 @@ extern void real_writeb(u8 data, volatile >> u8 __iomem *addr); >> > extern u8 real_205_readb(volatile u8 __iomem *addr); >> > extern void real_205_writeb(u8 data, volatile u8 __iomem >> *addr); >> > >> > +#ifdef CONFIG_UART_16550_WORD_ADDRESSABLE >> > +struct NS16550 { >> > + /* this struct must be packed */ >> > + unsigned char rbr; /* 0 */ u8 s0[3]; >> > + unsigned char ier; /* 1 */ u8 s1[3]; >> > + unsigned char fcr; /* 2 */ u8 s2[3]; >> > + unsigned char lcr; /* 3 */ u8 s3[3]; >> > + unsigned char mcr; /* 4 */ u8 s4[3]; >> > + unsigned char lsr; /* 5 */ u8 s5[3]; >> > + unsigned char msr; /* 6 */ u8 s6[3]; >> > + unsigned char scr; /* 7 */ u8 s7[3]; >> > +}; >> > +#else >> > struct NS16550 { >> > /* this struct must be packed */ >> > unsigned char rbr; /* 0 */ >> > @@ -29,6 +42,7 @@ struct NS16550 { >> > unsigned char msr; /* 6 */ >> > unsigned char scr; /* 7 */ >> > }; >> > +#endif /* CONFIG_UART_16550_WORD_ADDRESSABLE */ >> > >> >> >> Same things as with the register definitions. Please make this >> run-time selectable to allow build-time configurations that >> support both layouts. >> >> Yes. I will try to find better solution. > > Cheers, > Ben. > > > Regards, Tanmay CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply
Re: [RFC PATCH v2 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines
On 11/29/2011 02:05 AM, Benjamin Herrenschmidt wrote: > On Mon, 2011-11-28 at 16:32 +0530, Deepthi Dharwar wrote: > >>> Additionally, I'm a bit worried (but maybe we already discussed that a >>> while back, I don't know) but cpu_idle_wait() has "wait" in the name, >>> which makes me think it might need to actually -wait- for all cpus to >>> have come out of the function. >> >> cpu_idle_wait is used to ensure that all the CPUs discard old idle >> handler and update to new one. Required while changing idle >> handler on SMP systems. >> >>> Now your implementation doesn't provide that guarantee. It might be >>> fine, I don't know, but if it is, you'd better document it well in the >>> comments surrounding the code, because as it is, all you do is shoot an >>> interrupt which will cause the target CPU to eventually come out of idle >>> some time in the future. >> >> >> I was hoping that sending an explicit reschedule to the cpus would >> do the trick but sure we can add some documentation around the code. > > Well, the question is what guarantee do you expect. Sending a reschedule > IPI will take the other CPUs out of the actual sleep mode, but it will > be some time from there back to getting out of the handler function > (first back out of hypervisor etc...). > > The code as you implemented it doesn't wait for that to happen. It might > be fine ... or not. I don't know what semantics you are after precisely. > > Cheers, > Ben. > > Yes, this could be problematic as there is small window for the race condition to occur . Otherwise we need to manually schedule it by running a kernel thread but this would definitely have a overhead and would be an overkill. Regards, Deepthi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 4/4] cpuidle: (POWER) Handle power_save=off
On 11/29/2011 02:09 AM, Benjamin Herrenschmidt wrote: > On Mon, 2011-11-28 at 16:33 +0530, Deepthi Dharwar wrote: > >> On an LPAR if cpuidle is disabled, ppc_md.power_save is still set to >> cpuidle_idle_call by default here. This would result in calling of >> cpuidle_idle_call repeatedly, only for the call to return -ENODEV. The >> default idle is never executed. >> This would be a major design flaw. No fallback idle routine. >> >> We propose to fix this by checking the return value of >> ppc_md.power_save() call from void to int. >> Right now return value is void, but if we change this to int, this >> would solve two problems. One being removing the cast to a function >> pointer in the prev patch and this design flaw stated above. >> >> So by checking the return value of ppc_md.power_save(), we can invoke >> the default idle on failure. But my only concern is about the effects of >> changing the ppc_md.power_save() to return int on other powerpc >> architectures. Would it be a good idea to change the return type to int >> which would help us flag an error and fallback to default idle? > > I would have preferred an approach where the cpuidle module sets > ppc_md.power_save when loaded and restores it when unloaded ... but that > would have to go into the cpuidle core as a powerpc specific tweak and > might not be generally well received. > > So go for it, add the return value, but you'll have to update all the > idle functions (grep for power_save in arch/powerpc to find them). > Thanks Ben. Yes, I will update all the idle functions under powerpc. I will re-work these patches with the discussed changes. Regards, Deepthi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] powerpc/40x: Add APM8018X SOC support
The AppliedMicro APM8018X embedded processor targets embedded applications that require low power and a small footprint. It features a PowerPC 405 processor core built in a 65nm low-power CMOS process with a five-stage pipeline executing up to one instruction per cycle. The family has 128-kbytes of on-chip memory, a 128-bit local bus and on-chip DDR2 SDRAM controller with 16-bit interface. Features: *CPU speed (frequency): up to 600 MHz *Five-stage pipeline, executes up to one instruction per cycle *16 KB-I/16 KB-D L1 caches, two-way set-associative *128 KB on-chip memory *128-bit processor local bus (PLB) *Separate 128-bit read and 128-bit write data bus *Up to 6.4 GBps of peak on-chip bandwidth at 200 MHz *On-chip DDR2 SDRAM controller with 16-bit interface *Support for one rank of DDR2 SDRAM up to 512 MB *One Gen 1 single-lane PCI Express interface *One Gen 1 single lane miniPCIe interface *Configurable as root or endpoint *One SATA controller operating at up to 3.0 Gbps with integrated SerDes *Two Ethernet 10/100/1000 Mbps, full-duplex MACs (RGMII/TMII/MII) *TCP/IP acceleration hardware, QoS, and jumbo frame support *IEEE 1588 V1 and V2 support *On-chip IPsec acceleration with header/trailer processing *Supports DES, 3DES, and AES encryption *Operates at 1.5/12/480 Mbps bus speeds version 2: 1. compressed defconfig. 2. cleaned dts. 3. cputable with single cpu entry instead of 4 similar entries. 4. removed uart specific changes. Signed-off-by: Tanmay Inamdar --- arch/powerpc/boot/dts/klondike.dts | 227 +++ arch/powerpc/configs/40x/klondike_defconfig | 55 +++ arch/powerpc/kernel/cputable.c | 13 ++ arch/powerpc/platforms/40x/Kconfig | 11 ++ arch/powerpc/platforms/40x/ppc40x_simple.c |1 + 5 files changed, 307 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/boot/dts/klondike.dts create mode 100644 arch/powerpc/configs/40x/klondike_defconfig diff --git a/arch/powerpc/boot/dts/klondike.dts b/arch/powerpc/boot/dts/klondike.dts new file mode 100644 index 000..8c94290 --- /dev/null +++ b/arch/powerpc/boot/dts/klondike.dts @@ -0,0 +1,227 @@ +/* + * Device Tree for Klondike (APM8018X) board. + * + * Copyright (c) 2010, Applied Micro Circuits Corporation + * Author: Tanmay Inamdar + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + * + */ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + model = "apm,klondike"; + compatible = "apm,klondike"; + dcr-parent = <&{/cpus/cpu@0}>; + + aliases { + ethernet0 = &EMAC0; + ethernet1 = &EMAC1; + }; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + device_type = "cpu"; + model = "PowerPC,apm8018x"; + reg = <0x>; + clock-frequency = <3>; /* Filled in by U-Boot */ + timebase-frequency = <3>; /* Filled in by U-Boot */ + i-cache-line-size = <32>; + d-cache-line-size = <32>; + i-cache-size = <16384>; /* 16 kB */ + d-cache-size = <16384>; /* 16 kB */ + dcr-controller; + dcr-access-method = "native"; + }; + }; + + memory { + device_type = "memory"; + reg = <0x 0x2000>; /* Filled in by U-Boot */ + }; + + UIC0: interrupt-controller { + compatible = "ibm,uic"; + interrupt-controller; + cell-index = <0>; + dcr-reg = <0x0c0 0x010>; + #address-cells = <0>; + #size-cells = <0>; + #interrupt-cells = <2>; + }; + + UIC1: interrupt-controller1 { + compatible = "ibm,uic"; + interrupt-controller; + cell-index = <1>; + dcr-reg = <0x0d0 0x010>; + #address-cells = <0>; + #size-cells = <0>; + #interrupt-cells = <2>; + interrupts = <0x1e 0x4 0x1f 0x4>; /* cascade */ + interrupt-p
Re: [RFC PATCH v2 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines
On Tue, 2011-11-29 at 12:12 +0530, Deepthi Dharwar wrote: > > Yes, this could be problematic as there is small window for the > race condition to occur . Otherwise we need to manually schedule > it by running a kernel thread but this would definitely have a > overhead and would be an overkill. Depends what this "window" is. IE. What are you trying to protect yourself against ? What's the risk ? If it's just module unload, then stop_machine is probably your friend :-) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines
On 11/29/2011 12:31 PM, Benjamin Herrenschmidt wrote: > On Tue, 2011-11-29 at 12:12 +0530, Deepthi Dharwar wrote: >> >> Yes, this could be problematic as there is small window for the >> race condition to occur . Otherwise we need to manually schedule >> it by running a kernel thread but this would definitely have a >> overhead and would be an overkill. > > Depends what this "window" is. IE. What are you trying to protect > yourself against ? What's the risk ? > > If it's just module unload, then stop_machine is probably your > friend :-) > > Cheers, > Ben. > > Yup, it is the module unload that I am worried about. Otherwise manually doing it using kernel thread would be an overkill -:( Regards, Deepthi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev