Patchwork [RFC] arch-armv4.inc: add --fix-v4bx to TARGET_LD_KERNEL_ARCH only for armv4 and strongarm1100

login
register
mail settings
Submitter Martin Jansa
Date Nov. 23, 2012, 9:09 a.m.
Message ID <1353661767-13873-1-git-send-email-Martin.Jansa@gmail.com>
Download mbox | patch
Permalink /patch/39531/
State Superseded, archived
Headers show

Comments

Martin Jansa - Nov. 23, 2012, 9:09 a.m.
* without this patch it does apply --fix-v4bx not only to armv4, but
  also all higher (because they also have armv4 in TUNE_FEATURES)
* it causes SIGILL on armv4t
  http://lists.linuxtogo.org/pipermail/openembedded-devel/2012-November/042298.html
* someone please test on armv4 device (I tested only bitbake -e output
  that it's correctly applied with DEFAULTTUNE == armv4
* maybe we can should fix this in binutils instead (both 2.22 and 2.23
  are affected)

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/conf/machine/include/arm/arch-armv4.inc     | 2 +-
 meta/conf/machine/include/tune-strongarm1100.inc | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
Phil Blundell - Nov. 23, 2012, 9:52 a.m.
On Fri, 2012-11-23 at 10:09 +0100, Martin Jansa wrote:
> -TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("TUNE_FEATURES", "armv4", "--fix-v4bx", "", d)}"
> +TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("DEFAULTTUNE", "armv4", "--fix-v4bx", "", d)}"

Testing DEFAULTTUNE doesn't really seem like the right thing.  Not that
I am any expert in the twisty maze of tune variables, but I would have
thought this should be:

TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("TUNE_FEATURES", "thumb", "", "--fix-v4bx", d)}"

Or maybe it could just be fixed in the kernel, since its own build
system already knows whether Thumb is available or not.

p.
Martin Jansa - Nov. 23, 2012, 10:22 a.m.
On Fri, Nov 23, 2012 at 09:52:28AM +0000, Phil Blundell wrote:
> On Fri, 2012-11-23 at 10:09 +0100, Martin Jansa wrote:
> > -TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("TUNE_FEATURES", "armv4", "--fix-v4bx", "", d)}"
> > +TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("DEFAULTTUNE", "armv4", "--fix-v4bx", "", d)}"
> 
> Testing DEFAULTTUNE doesn't really seem like the right thing.  Not that
> I am any expert in the twisty maze of tune variables, but I would have
> thought this should be:
> 
> TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("TUNE_FEATURES", "thumb", "", "--fix-v4bx", d)}"

Depends if we want to apply it on e.g. armv5te device without thumb in
TUNE_FEATURES.

> Or maybe it could just be fixed in the kernel, since its own build
> system already knows whether Thumb is available or not.

True, but then we need to fix every kernel recipe and because it took me
quite long time to find commit which caused it, I fear that it will
reappear in future when some layer adds kernel recipe without such fix.

Cheers,
Phil Blundell - Nov. 23, 2012, 10:48 a.m.
On Fri, 2012-11-23 at 11:22 +0100, Martin Jansa wrote:
> On Fri, Nov 23, 2012 at 09:52:28AM +0000, Phil Blundell wrote:
> > On Fri, 2012-11-23 at 10:09 +0100, Martin Jansa wrote:
> > > -TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("TUNE_FEATURES", "armv4", "--fix-v4bx", "", d)}"
> > > +TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("DEFAULTTUNE", "armv4", "--fix-v4bx", "", d)}"
> > 
> > Testing DEFAULTTUNE doesn't really seem like the right thing.  Not that
> > I am any expert in the twisty maze of tune variables, but I would have
> > thought this should be:
> > 
> > TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("TUNE_FEATURES", "thumb", "", "--fix-v4bx", d)}"
> 
> Depends if we want to apply it on e.g. armv5te device without thumb in
> TUNE_FEATURES.

Oh, yes, right.  Well, if you aren't using thumb then --fix-v4bx is
going to be mostly harmless, but I suppose the correct thing would be to
check for TUNE_FEATURES containing neither thumb nor armv5. 

p.

Patch

diff --git a/meta/conf/machine/include/arm/arch-armv4.inc b/meta/conf/machine/include/arm/arch-armv4.inc
index cb747ac..f028ca6 100644
--- a/meta/conf/machine/include/arm/arch-armv4.inc
+++ b/meta/conf/machine/include/arm/arch-armv4.inc
@@ -4,7 +4,7 @@  ARMPKGARCH ?= "armv4"
 
 TUNEVALID[armv4] = "Enable instructions for ARMv4"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "armv4", "-march=armv4${ARMPKGSFX_THUMB}", "", d)}"
-TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("TUNE_FEATURES", "armv4", "--fix-v4bx", "", d)}"
+TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("DEFAULTTUNE", "armv4", "--fix-v4bx", "", d)}"
 MACHINEOVERRIDES .= "${@bb.utils.contains("TUNE_FEATURES", "armv4", ":armv4", "" ,d)}"
 
 require conf/machine/include/arm/arch-arm.inc
diff --git a/meta/conf/machine/include/tune-strongarm1100.inc b/meta/conf/machine/include/tune-strongarm1100.inc
index 66bab8e..66885eb 100644
--- a/meta/conf/machine/include/tune-strongarm1100.inc
+++ b/meta/conf/machine/include/tune-strongarm1100.inc
@@ -8,5 +8,4 @@  TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "strongarm", "-mtune=stron
 AVAILTUNES += "strongarm"
 TUNE_FEATURES_tune-strongarm = "${TUNE_FEATURES_tune-armv4} strongarm"
 PACKAGE_EXTRA_ARCHS_tune-strongarm = "${PACKAGE_EXTRA_ARCHS_tune-armv4}"
-
-
+TARGET_LD_KERNEL_ARCH += "${@bb.utils.contains("DEFAULTTUNE", "strongarm", "--fix-v4bx", "", d)}"