| Message ID | 1395385319-25386-1-git-send-email-chao.xie@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/21/2014 01:01 AM, Chao Xie wrote: > PJ4 is based on V7, but it has some changes. For example, some > coprocessor settings. The series, Tested-by: Stephen Warren <swarren@nvidia.com> Reviewed-by: Stephen Warren <swarren@nvidia.com>
Chao Xie <chao.xie@marvell.com> writes: > PJ4 is based on V7, but it has some changes. For example, some > coprocessor settings. > > Signed-off-by: Chao Xie <chao.xie@marvell.com> For this updated series: Reviewed-by: Kevin Hilman <khilman@linaro.org> Tested-by: Kevin Hilman <khilman@linaro.org> Can you submit this to Russell's patch tracking system ASAP please so we can fix the multiple boot failures we're seeing in -next. Thanks, Kevin [1] http://www.arm.linux.org.uk/developer/patches/
On Fri, Mar 21, 2014 at 03:01:58PM +0800, Chao Xie wrote: > PJ4 is based on V7, but it has some changes. For example, some > coprocessor settings. > > Signed-off-by: Chao Xie <chao.xie@marvell.com> Retested on bcm28155-ap for the updated series: Tested-by: Matt Porter <mporter@linaro.org> -Matt
On 03/21/2014 01:01 AM, Chao Xie wrote: > PJ4 is based on V7, but it has some changes. For example, some > coprocessor settings. I don't see this patch in linux-next, yet I think the runtime problem has gone away somehow. How did that happen?
Stephen Warren <swarren@wwwdotorg.org> writes: > On 03/21/2014 01:01 AM, Chao Xie wrote: >> PJ4 is based on V7, but it has some changes. For example, some >> coprocessor settings. > > I don't see this patch in linux-next, yet I think the runtime problem > has gone away somehow. How did that happen? The multi_v7 change that introduced the regression (addtion of MACH_DOVE) was reverted in arm-soc/for-next while waiting for the patch to be submitted to Russell's tracker. Looks like they've now been submitted to the tracker, so hopefully Russell will get to them on his next pass. They'll be needed for v3.15. Kevin
Dear Chao Xie, On Fri, 21 Mar 2014 15:01:58 +0800, Chao Xie wrote: > +/* > + * Marvell's PJ4 core is based on V7 version. It has some modification > + * for coprocessor setting. For this reason, we need a way to distinguish > + * it. > + */ > +#ifndef CONFIG_CPU_PJ4 > +#define cpu_is_pj4() 0 > +#else > +static inline int cpu_is_pj4(void) > +{ > + unsigned int id; > + > + id = read_cpuid_id(); > + if ((id & 0xfffffff0) == 0x562f5840) > + return 1; Unfortunately, this doesn't work correctly, because this cpuid check also matches Armada XP, which uses the PJ4B-MP core, but does not have the iWMMXt extension. Therefore, when you build mach-mvebu with Dove support enabled, it does not boot on Armada XP: it fails with an undefined instruction. Also, I believe the test is not sufficient because the Dove Armada 510 uses a PJ4 core, but is not matched by the above test. For reference, the cpuid of various platforms: * Armada XP, PJ4B-MP core, 0x562f5842 * Armada 370, PJ4B core, 0x561f5811 * Dove Armada 510, PJ4 core, 560f5815 Can you check on your platforms what was the exact cpuid, to see if we can find a way of correcting this cpu_is_pj4() function? Can you also check whether your CPU uses a PJ4, PJ4B or PJ4B-MP core? Thanks, Thomas
On Mon, Apr 14, 2014 at 03:12:36PM +0200, Thomas Petazzoni wrote: > Dear Chao Xie, > > On Fri, 21 Mar 2014 15:01:58 +0800, Chao Xie wrote: > > > +/* > > + * Marvell's PJ4 core is based on V7 version. It has some modification > > + * for coprocessor setting. For this reason, we need a way to distinguish > > + * it. > > + */ > > +#ifndef CONFIG_CPU_PJ4 > > +#define cpu_is_pj4() 0 > > +#else > > +static inline int cpu_is_pj4(void) > > +{ > > + unsigned int id; > > + > > + id = read_cpuid_id(); > > + if ((id & 0xfffffff0) == 0x562f5840) > > + return 1; > > Unfortunately, this doesn't work correctly, because this cpuid check > also matches Armada XP, which uses the PJ4B-MP core, but does not have > the iWMMXt extension. Therefore, when you build mach-mvebu with Dove > support enabled, it does not boot on Armada XP: it fails with an > undefined instruction. Grr, that's annoying. And we can't just revert this because of the arm-soc debacle - doing so will break the boot for a whole pile of other platforms. This is a nice illustration of why the arm-soc process - with arm-soc effectively *forcing* me to take patches - is rather broken. I hope arm-soc people start behaving more responsibly in the future.
On Monday 14 April 2014 14:43:53 Russell King - ARM Linux wrote: > On Mon, Apr 14, 2014 at 03:12:36PM +0200, Thomas Petazzoni wrote: > > Dear Chao Xie, > > > > On Fri, 21 Mar 2014 15:01:58 +0800, Chao Xie wrote: > > > > > +/* > > > + * Marvell's PJ4 core is based on V7 version. It has some modification > > > + * for coprocessor setting. For this reason, we need a way to distinguish > > > + * it. > > > + */ > > > +#ifndef CONFIG_CPU_PJ4 > > > +#define cpu_is_pj4() 0 > > > +#else > > > +static inline int cpu_is_pj4(void) > > > +{ > > > + unsigned int id; > > > + > > > + id = read_cpuid_id(); > > > + if ((id & 0xfffffff0) == 0x562f5840) > > > + return 1; > > > > Unfortunately, this doesn't work correctly, because this cpuid check > > also matches Armada XP, which uses the PJ4B-MP core, but does not have > > the iWMMXt extension. Therefore, when you build mach-mvebu with Dove > > support enabled, it does not boot on Armada XP: it fails with an > > undefined instruction. > > Grr, that's annoying. And we can't just revert this because of the > arm-soc debacle - doing so will break the boot for a whole pile of > other platforms. It is very unfortunate that the bug didn't get caught earlier in order to get fixed before the merge window. Having Marvell Dove enabled in multi_v7_defconfig in the branch I submitted to Linus was a stupid mistake of mine, no malicious intentions, and I already apologized for that before. > This is a nice illustration of why the arm-soc process - with arm-soc > effectively *forcing* me to take patches - is rather broken. I hope > arm-soc people start behaving more responsibly in the future. Do you have a better idea? We could for the moment make Dove mutually exclusive with the other platforms it breaks in Kconfig for the moment if you think that would be better. That would at least avoid the regression against 3.14 at the expense of losing the dove multiplatform support. We can then decide later after we have a proper fix whether we are confident enough that it works in all cases, or whether it's too late for 3.15 by then. Arnd
This is a patch set fixing regressions in v3.15-rc1 ultimately caused by adding DT-enabled Marvell Dove to MULTI_V7. There was a fix introduced late in the merge window to fix a related regression for non-PJ4 architectures, that turned out to introduce another regression on PJ4B-based Armada 370/XP. Therefore, this now takes care of iWMMXt and PJ4[B] related code to properly fix all regressions observed in v3.15-rc1. At the end, one patch adding support for iWMMXt on PJ4B as found on Marvell Berlin BG2 SoCs is added. Patch 1 reworks iwmmxt.S preprocessor directives to allow to build it only if a supported platform is enabled. Also, it rewrites them to explicitly check for all CPUs of the currently supported architectures. Patch 2 fixes pj4_cp0_init to only enable iWMMXt capabilities, if corresponding kernel support code is also enabled by CONFIG_IWMMXT. Patch 3 fixes pj4_cp0_init to properly perform runtime checks for absence/presence of iWMMXt coprocessor. This effectively fixes boot regressions observed on Armada 370/XP. Patch 4 fixes cpu_is_pj4's cpuid check to check for both PJ4 and PJ4B. This effectively fixes iWMMXt support on MULTI_V7 Dove. Patch 5 finally allows PJ4B to also enable iWMMXt support as there are some PJ4B based SoCs, e.g. Marvell Armada 1500, that have those coprocessors. This is _not_ a fix but an improvement and should be treated as such, i.e. taken for v3.16. As a side note, after looking into this: if XScale based SoCs also properly perform runtime checks for iWMMXt presence, I'd be interested on comments if iWMMXt can possibly also be reworked to be build as a module dynamically adding/removing iWMMXt support? I expect proper Tested-by's for Armada 370/XP, where some preliminary patches have been boot tested by Thomas Petazzoni. I boot tested it on Marvell Dove and Marvell Berlin BG2. Any Tested-by's from XScale and/or non-PJ4[B] architectures are also appreciated. Russell, please let me know if/when you are happy with the fixes and the improvement. I'll be adding them to your patch tracker then. Sebastian Sebastian Hesselbarth (5): ARM: iwmmxt: explicitly check for supported architectures ARM: pj4: enable iWMMXt only if CONFIG_IWMMXT is set ARM: pj4: properly detect existence of iWMMXt coprocessor ARM: pj4: fix cpu_is_pj4 check ARM: iwmmxt: allow to build iWMMXt on Marvell PJ4B arch/arm/Kconfig | 6 +++--- arch/arm/include/asm/cputype.h | 14 +++++++------- arch/arm/kernel/Makefile | 1 + arch/arm/kernel/iwmmxt.S | 8 ++++++-- arch/arm/kernel/pj4-cp0.c | 42 +++++++++++++++++++++++++++++++++++++++--- 5 files changed, 56 insertions(+), 15 deletions(-) --- Cc: Russell King <linux@arm.linux.org.uk> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Olof Johansson <olof@lixom.net> Cc: Kevin Hilman <khilman@linaro.org> Cc: Andrew Lunn <andrew@lunn.ch> Cc: Jason Cooper <jason@lakedaemon.net> Cc: Gregory Clement <gregory.clement@free-electrons.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Eric Miao <eric.y.miao@gmail.com> Cc: Haojian Zhuang <haojian.zhuang@gmail.com> Cc: Chao Xie <xiechao.mail@gmail.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org
Dear Sebastian Hesselbarth, On Tue, 15 Apr 2014 20:15:58 +0200, Sebastian Hesselbarth wrote: > Sebastian Hesselbarth (5): > ARM: iwmmxt: explicitly check for supported architectures > ARM: pj4: enable iWMMXt only if CONFIG_IWMMXT is set > ARM: pj4: properly detect existence of iWMMXt coprocessor > ARM: pj4: fix cpu_is_pj4 check > ARM: iwmmxt: allow to build iWMMXt on Marvell PJ4B Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>. It does fix the boot on PJ4B-MP based processors that don't have iWMMXt extensions such as the Armada XP. I've tested the following cases: * Armada XP (PJ4B-MP), with CONFIG_MACH_DOVE disabled and enabled * Armada 370 (PJ4B), with CONFIG_MACH_DOVE disabled and enabled and all of them boot, while before your patch series, 3.15-rc1 fails to boot on Armada XP with CONFIG_MACH_DOVE enabled. Thanks! Thomas
Sebastian, Russell, On Tue, 15 Apr 2014 20:15:58 +0200, Sebastian Hesselbarth wrote: > This is a patch set fixing regressions in v3.15-rc1 ultimately caused > by adding DT-enabled Marvell Dove to MULTI_V7. There was a fix > introduced late in the merge window to fix a related regression for > non-PJ4 architectures, that turned out to introduce another regression > on PJ4B-based Armada 370/XP. [...] > Russell, please let me know if/when you are happy with the fixes > and the improvement. I'll be adding them to your patch tracker then. Any plans towards merging those patches, or at least PATCH 1 to 4 ? They fix a regression on 3.15-r1 affecting mvebu platforms, and it would be good to have that fixed by 3.15 final. Thanks! Thomas
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes: > This is a patch set fixing regressions in v3.15-rc1 ultimately caused > by adding DT-enabled Marvell Dove to MULTI_V7. There was a fix > introduced late in the merge window to fix a related regression for > non-PJ4 architectures, that turned out to introduce another regression > on PJ4B-based Armada 370/XP. Tested-by: Kevin Hilman <khilman@linaro.org> FWIW, I tested this series on top of v3.15-rc1 using multi_v7_defconfig on all of multi-platform boards I have (20), and they all passed the boot test. Full boot report below. Kevin Subject: test/dove-regression boot: 20 pass, 0 fail (v3.15-rc1-5-gc1d3c1beba74) Tree/Branch: test/dove-regression Git describe: v3.15-rc1-5-gc1d3c1beba74 Commit: c1d3c1beba ARM: iwmmxt: allow to build iWMMXt on Marvell PJ4B Full Report =========== arm-multi_v7_defconfig ---------------------- qcom-apq8074-dragonboard 0 min 15.7 sec: PASS ste-snowball 1 min 20.5 sec: PASS tegra30-beaver 0 min 17.1 sec: PASS am335x-boneblack 0 min 37.7 sec: PASS omap3-beagle-xm 0 min 47.5 sec: PASS sun7i-a20-cubieboard2 0 min 12.4 sec: PASS armada-370-mirabox 0 min 22.4 sec: PASS omap4-panda 0 min 58.2 sec: PASS armada-xp-openblocks-ax3-4 0 min 25.2 sec: PASS sun4i-a10-cubieboard 0 min 19.1 sec: PASS bcm28155-ap 0 min 27.8 sec: PASS omap3-overo-tobi 0 min 22.3 sec: PASS (Warnings: 1) imx6dl-wandboard,wand-solo 0 min 18.2 sec: PASS am335x-bone 0 min 26.7 sec: PASS omap3-overo-storm-tobi 0 min 20.9 sec: PASS omap5-uevm 1 min 40.0 sec: PASS (Warnings: 1) omap3-n900 0 min 34.5 sec: PASS (Warnings: 1) imx6q-wandboard 0 min 17.2 sec: PASS omap4-panda-es 0 min 54.5 sec: PASS imx6dl-wandboard,wand-dual 0 min 18.1 sec: PASS
diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h index 42f0889..c651e3b 100644 --- a/arch/arm/include/asm/cputype.h +++ b/arch/arm/include/asm/cputype.h @@ -221,4 +221,23 @@ static inline int cpu_is_xsc3(void) #define cpu_is_xscale() 1 #endif +/* + * Marvell's PJ4 core is based on V7 version. It has some modification + * for coprocessor setting. For this reason, we need a way to distinguish + * it. + */ +#ifndef CONFIG_CPU_PJ4 +#define cpu_is_pj4() 0 +#else +static inline int cpu_is_pj4(void) +{ + unsigned int id; + + id = read_cpuid_id(); + if ((id & 0xfffffff0) == 0x562f5840) + return 1; + + return 0; +} +#endif #endif
PJ4 is based on V7, but it has some changes. For example, some coprocessor settings. Signed-off-by: Chao Xie <chao.xie@marvell.com> --- arch/arm/include/asm/cputype.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)