RE: [4/4] powerpc/mpc8548: Add workaround for erratum NMG_SRIO135
OK. I will do. -Chenhui From: Wood Scott-B07421 Sent: Thursday, October 17, 2013 7:20 To: Zhao Chenhui-B35336 Cc: linuxppc-dev@lists.ozlabs.org Subject: Re: [4/4] powerpc/mpc8548: Add workaround for erratum NMG_SRIO135 On Tue, Mar 06, 2012 at 05:10:56PM +0800, chenhui zhao wrote: From: chenhui zhao chenhui.z...@freescale.com Issue: Applications using lwarx/stwcx instructions in the core to compete for a software lock or semaphore with a device on RapidIO using read atomic set, clr, inc, or dec in a similar manner may falsely result in both masters seeing the lock as available. This could result in data corruption as both masters try to modify the same piece of data protected by the lock. Workaround: Set bits 13 and 29 of CCSR offset 0x01010 (EEBPCR register of the ECM) during initialization and leave them set indefinitely. This may slightly degrade overall system performance. Refer to SRIO39 in MPC8548 errata document. Signed-off-by: Gong Chen g.c...@freescale.com Signed-off-by: Zhao Chenhui chenhui.z...@freescale.com Signed-off-by: Li Yang le...@freescale.com --- arch/powerpc/sysdev/fsl_rio.c | 44 + 1 files changed, 44 insertions(+), 0 deletions(-) [snip] @@ -358,6 +391,17 @@ int fsl_rio_setup(struct platform_device *dev) dev-dev.of_node-full_name); return -EFAULT; } + + /* Fix erratum NMG_SRIO135 */ + if (fsl_svr_is(SVR_8548) || fsl_svr_is(SVR_8548_E)) { + rc = fixup_erratum_srio135(dev-dev); + if (rc) { + dev_err(dev-dev, + Failed to fix the erratum NMG_SRIO135.); + return rc; + } + } This needs to be respun based on the current tree. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 02/17] powerpc/85xx: do not sync time base at boot time
No other reason. Just avoid doing it again at boot time in kernel. -Chenhui From: Kumar Gala [ga...@kernel.crashing.org] Sent: Wednesday, April 03, 2013 23:10 To: Zhao Chenhui-B35336 Cc: linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 02/17] powerpc/85xx: do not sync time base at boot time On Apr 3, 2013, at 8:09 AM, Zhao Chenhui wrote: From: Chen-Hui Zhao chenhui.z...@freescale.com The bootloader have done time base sync for all cores, so skip the synchronization process at boot time of kernel. Signed-off-by: Zhao Chenhui chenhui.z...@freescale.com Signed-off-by: Li Yang le...@freescale.com Signed-off-by: Andy Fleming aflem...@freescale.com --- arch/powerpc/platforms/85xx/smp.c |8 1 files changed, 8 insertions(+), 0 deletions(-) What harm is there in doing the sync? I'm sure there is another reason you want to skip the TB sync that should be conveyed in the commit message. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v7 2/5] powerpc/85xx: add HOTPLUG_CPU support
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Friday, July 13, 2012 8:15 PM To: Zhao Chenhui-B35336 Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; linux-ker...@vger.kernel.org; Li Yang-R58472 Subject: Re: [PATCH v7 2/5] powerpc/85xx: add HOTPLUG_CPU support On Jul 3, 2012, at 5:21 AM, Zhao Chenhui wrote: From: Li Yang le...@freescale.com Add support to disable and re-enable individual cores at runtime on MPC85xx/QorIQ SMP machines. Currently support e500v1/e500v2 core. MPC85xx machines use ePAPR spin-table in boot page for CPU kick-off. This patch uses the boot page from bootloader to boot core at runtime. It supports 32-bit and 36-bit physical address. Add generic_set_cpu_up() to set cpu_state as CPU_UP_PREPARE in kick_cpu(). Shouldn't the generic_setup_cpu_up() be a separate patch, and refactor smp_generic_kick_cpu() to use it. Also, we should pull the conversion of the spintable from #defines to struct into a separate patch before this one. Ok. I will split this patch. -Chenhui ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v6 1/5] powerpc/85xx: implement hardware timebase sync
-Original Message- From: Linuxppc-dev [mailto:linuxppc-dev-bounces+chenhui.zhao=freescale@lists.ozlabs.org] On Behalf Of Kumar Gala Sent: Friday, June 29, 2012 2:30 AM To: Zhao Chenhui-B35336 Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org list; linux-ker...@vger.kernel.org list Subject: Re: [PATCH v6 1/5] powerpc/85xx: implement hardware timebase sync On Jun 28, 2012, at 5:50 AM, Benjamin Herrenschmidt wrote: On Thu, 2012-06-28 at 11:38 +0800, Zhao Chenhui wrote: The bootloader have done a timebase sync. If we do not need KEXEC or HOTPLUG_CPU feature, it is unnecessary to do it again at boot time of kernel. I only compile the timebase sync routines when users enable KEXEC or HOTPLUG_CPU. Still, how much are you really saving ? Is it worth the added mess and loss of test coverage ? We have too many conditional stuff like that already. Cheers, Ben. I'd also be interested to know how long it actually takes to do time base sync this way. Since you are freezing the timers for some period how long does it really take between the freeze/unfreeze in mpc85xx_give_timebase() + mpc85xx_timebase_freeze(1); ... + mpc85xx_timebase_freeze(0); You can use ATBL/U as a way to see # of cycles taken. - k I measured it using ATBL on MPC8572DS with 1.5GHz core frequency and 600MHz CCB frequency. The average of 10 times is 1019 clock. It seems that most of the time spent by isync and msync. -Chenhui ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
power management patch set for mpc85xx
Hi Ben and Paul, I am sorry to trouble you. It seems that Kumar is busy recently. Could you have a review on the following patches? These patches implement the power management support on MPC85xx platform. http://patchwork.ozlabs.org/patch/158484/ http://patchwork.ozlabs.org/patch/158485/ http://patchwork.ozlabs.org/patch/158487/ http://patchwork.ozlabs.org/patch/158486/ http://patchwork.ozlabs.org/patch/158488/ Thanks. Best Regards, Chenhui ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [linuxppc-release] [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync
Hi Kumar, There is no comment for these patches so far. Do you think these patches can be merged? We really want these patches to be merged in this merge window. Thanks. Best Regards, Chenhui -Original Message- From: Zhao Chenhui-B35336 Sent: Friday, May 25, 2012 3:09 PM To: Wood Scott-B07421; ga...@kernel.crashing.org Cc: Li Yang-R58472 Subject: RE: [linuxppc-release] [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync Hi Scott and Kumar, Do you have comments for these patches? http://patchwork.ozlabs.org/patch/158484/ http://patchwork.ozlabs.org/patch/158485/ http://patchwork.ozlabs.org/patch/158487/ http://patchwork.ozlabs.org/patch/158486/ http://patchwork.ozlabs.org/patch/158488/ Thanks. Best Regards, Chenhui -Original Message- From: linuxppc-release-boun...@linux.freescale.net [mailto:linuxppc-release- boun...@linux.freescale.net] On Behalf Of Zhao Chenhui-B35336 Sent: Friday, May 11, 2012 7:54 PM To: linuxppc-dev@lists.ozlabs.org Cc: Wood Scott-B07421; Li Yang-R58472; linux-ker...@vger.kernel.org; ga...@kernel.crashing.org Subject: [linuxppc-release] [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync Do hardware timebase sync. Firstly, stop all timebases, and transfer the timebase value of the boot core to the other core. Finally, start all timebases. Only apply to dual-core chips, such as MPC8572, P2020, etc. Signed-off-by: Zhao Chenhui chenhui.z...@freescale.com Signed-off-by: Li Yang le...@freescale.com --- arch/powerpc/include/asm/fsl_guts.h |2 + arch/powerpc/platforms/85xx/smp.c | 93 +-- 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/include/asm/fsl_guts.h b/arch/powerpc/include/asm/fsl_guts.h index aa4c488..dd5ba2c 100644 --- a/arch/powerpc/include/asm/fsl_guts.h +++ b/arch/powerpc/include/asm/fsl_guts.h @@ -48,6 +48,8 @@ struct ccsr_guts { __be32 dmuxcr;/* 0x.0068 - DMA Mux Control Register */ u8 res06c[0x70 - 0x6c]; __be32 devdisr;/* 0x.0070 - Device Disable Control */ +#define CCSR_GUTS_DEVDISR_TB1 0x1000 +#define CCSR_GUTS_DEVDISR_TB0 0x4000 __be32 devdisr2; /* 0x.0074 - Device Disable Control 2 */ u8 res078[0x7c - 0x78]; __be32 pmjcr; /* 0x.007c - 4 Power Management Jog Control Register */ diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index ff42490..6862dda 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -24,6 +24,7 @@ #include asm/mpic.h #include asm/cacheflush.h #include asm/dbell.h +#include asm/fsl_guts.h #include sysdev/fsl_soc.h #include sysdev/mpic.h @@ -115,13 +116,70 @@ smp_85xx_kick_cpu(int nr) struct smp_ops_t smp_85xx_ops = { .kick_cpu = smp_85xx_kick_cpu, -#ifdef CONFIG_KEXEC - .give_timebase = smp_generic_give_timebase, - .take_timebase = smp_generic_take_timebase, -#endif }; #ifdef CONFIG_KEXEC +static struct ccsr_guts __iomem *guts; +static u64 timebase; +static int tb_req; +static int tb_valid; + +static void mpc85xx_timebase_freeze(int freeze) +{ + unsigned int mask; + + if (!guts) + return; + + mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1; + if (freeze) + setbits32(guts-devdisr, mask); + else + clrbits32(guts-devdisr, mask); + + in_be32(guts-devdisr); +} + +static void mpc85xx_give_timebase(void) +{ + unsigned long flags; + + local_irq_save(flags); + + while (!tb_req) + barrier(); + tb_req = 0; + + mpc85xx_timebase_freeze(1); + timebase = get_tb(); + mb(); + tb_valid = 1; + + while (tb_valid) + barrier(); + + mpc85xx_timebase_freeze(0); + + local_irq_restore(flags); +} + +static void mpc85xx_take_timebase(void) +{ + unsigned long flags; + + local_irq_save(flags); + + tb_req = 1; + while (!tb_valid) + barrier(); + + set_tb(timebase 32, timebase 0x); + mb(); + tb_valid = 0; + + local_irq_restore(flags); +} + atomic_t kexec_down_cpus = ATOMIC_INIT(0); void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary) @@ -228,6 +286,20 @@ smp_85xx_setup_cpu(int cpu_nr) doorbell_setup_this_cpu(); } +#ifdef CONFIG_KEXEC +static const struct of_device_id guts_ids[] = { + { .compatible = fsl,mpc8572-guts, }, + { .compatible = fsl,mpc8560-guts, }, + { .compatible = fsl,mpc8536-guts, }, + { .compatible = fsl,p1020-guts, }, + { .compatible = fsl,p1021-guts, }, + { .compatible = fsl,p1022-guts, }, + { .compatible = fsl,p1023-guts, }, + { .compatible = fsl,p2020-guts, }, + {}, +}; +#endif + void
RE: [PATCH 2/4] fsl_pci: Add a workaround for PCI 5 errata in MPC8548
On Mar 6, 2012, at 3:10 AM, Zhao Chenhui wrote: + if ((fsl_svr_is(SVR_8548) || fsl_svr_is(SVR_8548_E)) Should this also have 8547, 8547E, 8545, 8545E, 8543, 8543E? Yes. I will include these chips. -Chenhui + !early_find_capability(hose, 0, 0, PCI_CAP_ID_PCIX)) { + early_read_config_word(hose, 0, 0, + PCI_BUS_FUNCTION, temp); + temp |= PCI_BUS_FUNCTION_MDS; + early_write_config_word(hose, 0, 0, + PCI_BUS_FUNCTION, temp); + } } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/9] pci_ids: Add device ID for IBM PCI-X bridge
On Mar 6, 2012, at 3:05 AM, Zhao Chenhui wrote: Signed-off-by: Zhao Chenhui chenhui.z...@freescale.com --- include/linux/pci_ids.h |1 + 1 files changed, 1 insertions(+), 0 deletions(-) Just merge this with the 2nd patch that actually uses the ID. - k Ok. I put it in the file mpc85xx_cds.c. -Chenhui ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/9] powerpc/mpc85xxcds: Fix PCI I/O space resource of PCI bridge
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c b/arch/powerpc/platforms/85xx/mpc85xx_cds.c index 40f03da..c009c5b 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c @@ -3,7 +3,7 @@ * * Maintained by Kumar Gala (see MAINTAINERS for contact information) * - * Copyright 2005 Freescale Semiconductor Inc. + * Copyright 2005, 2011-2012 Freescale Semiconductor Inc. * * 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 @@ -158,6 +158,31 @@ DECLARE_PCI_FIXUP_EARLY(0x1957, 0x3fff, skip_fake_bridge); DECLARE_PCI_FIXUP_EARLY(0x3fff, 0x1957, skip_fake_bridge); DECLARE_PCI_FIXUP_EARLY(0xff3f, 0x5719, skip_fake_bridge); +/* + * Fix Tsi310 PCI-X bridge resource. + * Force the bridge to open a window from 0x-0x1fff in PCI I/O space. + * This allows legacy I/O(i8259, etc) on the VIA southbridge to be accessed. + */ This comment and the code don't make sense. Why is the bridge described as Tsi310 in comments but the vendor ID is IBM ? This chip is from IBM originally, and bought by IDT. The vendor ID is IBM, but the part number is Tsi310(IDT). -Chenhui +void mpc85xx_cds_fixup_bus(struct pci_bus *bus) +{ + struct pci_dev *dev = bus-self; + struct resource *res = bus-resource[0]; + + if (dev != NULL + dev-vendor == PCI_VENDOR_ID_IBM + dev-device == PCI_DEVICE_ID_IBM_PCIX_BRIDGE) { + if (res) { + res-start = 0; + res-end = 0x1fff; + res-flags = IORESOURCE_IO; + pr_info(mpc85xx_cds: PCI bridge resource fixup applied\n); + pr_info(mpc85xx_cds: %pR\n, res); + } + } + + fsl_pcibios_fixup_bus(bus); +} ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v3] powerpc/85xx: add support to JOG feature using cpufreq interface
On 12/27/2011 05:25 AM, Zhao Chenhui wrote: * The driver doesn't support MPC8536 Rev 1.0 due to a JOG erratum. Subsequent revisions of MPC8536 have corrected the erratum. Where do you check for this? Nowhere. I just notify this patch don't support MPC8536 Rev 1.0. +#define POWMGTCSR_LOSSLESS_MASK0x0040 +#define POWMGTCSR_JOG_MASK 0x0020 Are these really masks, or just values to use? They are masks. +#define POWMGTCSR_CORE0_IRQ_MSK0x8000 +#define POWMGTCSR_CORE0_CI_MSK 0x4000 +#define POWMGTCSR_CORE0_DOZING 0x0008 +#define POWMGTCSR_CORE0_NAPPING0x0004 + +#define POWMGTCSR_CORE_INT_MSK 0x0800 +#define POWMGTCSR_CORE_CINT_MSK0x0400 +#define POWMGTCSR_CORE_UDE_MSK 0x0200 +#define POWMGTCSR_CORE_MCP_MSK 0x0100 +#define P1022_POWMGTCSR_MSK(POWMGTCSR_CORE_INT_MSK | \ +POWMGTCSR_CORE_CINT_MSK | \ +POWMGTCSR_CORE_UDE_MSK | \ +POWMGTCSR_CORE_MCP_MSK) + +static void keep_waking_up(void *dummy) +{ + unsigned long flags; + + local_irq_save(flags); + mb(); + + in_jog_process = 1; + mb(); + + while (in_jog_process != 0) + mb(); + + local_irq_restore(flags); +} Please document this. Compare in_jog_process == 1, not != 0 -- it's unlikely, but what if the other cpu sees that in_jog_process has been set to 1, exits and sets in_jog_process to 0, then re-enters set_pll and sets in_jog_process to -1 again before this function does another load of in_jog_process? Thanks. I'll fix it. Do you really need all these mb()s? I think this would suffice: local_irq_save(flags); in_jog_process = 1; while (in_jog_process == 1) barrier(); local_irq_restore(); It's not really a performance issue, just simplicity. +static int p1022_set_pll(unsigned int cpu, unsigned int pll) +{ + int index, hw_cpu = get_hard_smp_processor_id(cpu); + int shift; + u32 corefreq, val, mask = 0; + unsigned int cur_pll = get_pll(hw_cpu); + unsigned long flags; + int ret = 0; + + if (pll == cur_pll) + return 0; + + shift = hw_cpu * CORE_RATIO_BITS + CORE0_RATIO_SHIFT; + val = (pll CORE_RATIO_MASK) shift; + + corefreq = sysfreq * pll / 2; + /* +* Set the COREx_SPD bit if the requested core frequency +* is larger than the threshold frequency. +*/ + if (corefreq FREQ_533MHz) + val |= PMJCR_CORE0_SPD_MASK hw_cpu; P1022 manual says the threshold is 500 MHz (but doesn't say how to set the bit if the frequency is exactly 500 MHz). Where did 53334 come from? Please refer to Chapter 25 25.4.1.11 Power Management Jog Control Register (PMJCR). + + mask = (CORE_RATIO_MASK shift) | (PMJCR_CORE0_SPD_MASK hw_cpu); + clrsetbits_be32(guts + PMJCR, mask, val); + + /* readback to sync write */ + val = in_be32(guts + PMJCR); You don't use val after this -- just ignore the return value from in_be32(). OK. + /* +* A Jog request can not be asserted when any core is in a low +* power state on P1022. Before executing a jog request, any +* core which is in a low power state must be waked by a +* interrupt, and keep waking up until the sequence is +* finished. +*/ + for_each_present_cpu(index) { + if (!cpu_online(index)) + return -EFAULT; + } EFAULT is not the appropriate error code -- it is for when userspace passes a bad virtual address. Better, don't fail here -- bring the other core out of the low power state in order to do the jog. cpufreq shouldn't stop working just because we took a core offline. What prevents a core from going offline just after you check here? + in_jog_process = -1; + mb(); + smp_call_function(keep_waking_up, NULL, 0); What does keep waking up mean? Something like spin_while_jogging might be clearer. + local_irq_save(flags); + mb(); + /* Wait for the other core to wake. */ + while (in_jog_process != 1) + mb(); Timeout? And more unnecessary mb()s. Might be nice to support more than two cores, even if this code isn't currently expected to be used on such hardware (it's just a generic hold other cpus loop; might as well make it reusable). You could do this by using an atomic count for other cores to check in and out of the spin loop. This is just for P1022, a dual-core chip. A separate patch will support multi-core chips, such as P4080, etc. + out_be32(guts + POWMGTCSR, POWMGTCSR_JOG_MASK | P1022_POWMGTCSR_MSK); + + if (!spin_event_timeout(((in_be32(guts + POWMGTCSR) + POWMGTCSR_JOG_MASK) == 0), 1, 10)) { + pr_err(%s: Fail to switch the core frequency.\n, __func__); + ret =
RE: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
-Original Message- From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] Sent: Friday, November 11, 2011 12:23 PM To: Zhao Chenhui-B35336 Cc: linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support On Fri, 2011-11-04 at 20:31 +0800, Zhao Chenhui wrote: #ifdef CONFIG_SMP /* When we get here, r24 needs to hold the CPU # */ .globl __secondary_start diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c index 7bf2187..12a54f0 100644 --- a/arch/powerpc/kernel/smp.c +++ b/arch/powerpc/kernel/smp.c @@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu) for (i = 0; i 100; i++) { smp_rmb(); - if (per_cpu(cpu_state, cpu) == CPU_DEAD) + if (per_cpu(cpu_state, cpu) == CPU_DEAD) { + /* +* After another core sets cpu_state to CPU_DEAD, +* it needs some time to die. +*/ + msleep(10); return; + } msleep(100); } printk(KERN_ERR CPU%d didn't die...\n, cpu); I don't really see why you need to wait. This loop is about waiting for the CPU to mark itself DEAD, not necessarily to be physically powered off. If I don't add this delay, the kernel hangs sometimes when hotpluging a core. I will move this function to 85xx/smp.c in the next revision. +struct epapr_entry { + u32 addr_h; + u32 addr_l; + u32 r3_h; + u32 r3_l; + u32 reserved; + u32 pir; + u32 r6_h; + u32 r6_l; +}; This should probably be in a generic location. +static int is_corenet; This global is a bit gross... ... Agree. I will remove it. + +static void __cpuinit smp_85xx_reset_core(int nr) +{ + __iomem u32 *vaddr, *pir_vaddr; + u32 val, cpu_mask; + + /* If CoreNet platform, use BRR as release register. */ + if (is_corenet) { + cpu_mask = 1 nr; + vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4); + } else { + cpu_mask = 1 (24 + nr); + vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4); + } Instead, can't you instead have two functions using a common helper and pick/hook the right one ? Also in what context is that reset_core() called ? Doing ioremap isn't something you can do at any random time... Cheers, Ben. Thanks. I will fix them. -chenhui ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev