| Message ID | 20100211210844.E77B4F891E@sepang.rtg.net |
|---|---|
| State | Accepted |
| Delegated to: | Stefan Bader |
| Headers | show |
On 10 Feb 11, Tim Gardner wrote: > The following changes since commit 4b1196706272826e74e35e9ebf2cc94ba8d4062c: > Leann Ogasawara (1): > UBUNTU: Ubuntu-2.6.31-108.21 > > are available in the git repository at: > > git://kernel.ubuntu.com/rtg/ubuntu-karmic fsl-imx51-neon > > Bryan Wu (1): > UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > arch/arm/vfp/vfpmodule.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > From d77e3635c6d402cebdda49f654402c67460acfab Mon Sep 17 00:00:00 2001 > From: Bryan Wu <bryan.wu@canonical.com> > Date: Tue, 26 Jan 2010 19:00:22 +0000 > Subject: [PATCH] UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > BugLink: http://bugs.launchpad.net/bugs/507416 > > NEON function is broken on old Freescale iMX51 silicons, such as rev1.x and > rev2.x. Since more and more iMX51 based products will move to rev3.x or later > silicon, we need to enable NEON on these newer silicons and disable NEON on > those old ones. > > This patch will detect the silicon revision dynamically and only export NEON > flag to userspace on Freescale iMX51 rev3.x or later silicon. It was tested on > Babbage 3.0 board and Babbage 2.x board. > > Signed-off-by: Bryan Wu <bryan.wu@canonical.com> > Signed-off-by: Andy Whitcroft <apw@canonical.com> > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > --- > arch/arm/vfp/vfpmodule.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > index 2d7423a..e0d6b01 100644 > --- a/arch/arm/vfp/vfpmodule.c > +++ b/arch/arm/vfp/vfpmodule.c > @@ -432,6 +432,10 @@ out: > > #include <linux/smp.h> > > +#if defined(CONFIG_ARCH_MX51) && defined(CONFIG_NEON) > +#include <mach/hardware.h> > +#endif > + > /* > * VFP support code initialisation. > */ > @@ -498,7 +502,8 @@ static int __init vfp_init(void) > * load/store instructions, integer and single > * precision floating point operations. > */ > - if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > + if (((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > + && cpu_is_mx51_rev(CHIP_REV_3_0) > 0) > elf_hwcap |= HWCAP_NEON; > #endif > } Change looks correct. Acked-by: Amit Kucheria <amit.kucheria@canonical.com>
On Thu, 2010-02-11 at 14:08 -0700, Tim Gardner wrote: > The following changes since commit 4b1196706272826e74e35e9ebf2cc94ba8d4062c: > Leann Ogasawara (1): > UBUNTU: Ubuntu-2.6.31-108.21 > > are available in the git repository at: > > git://kernel.ubuntu.com/rtg/ubuntu-karmic fsl-imx51-neon > > Bryan Wu (1): > UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > arch/arm/vfp/vfpmodule.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > From d77e3635c6d402cebdda49f654402c67460acfab Mon Sep 17 00:00:00 2001 > From: Bryan Wu <bryan.wu@canonical.com> > Date: Tue, 26 Jan 2010 19:00:22 +0000 > Subject: [PATCH] UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > BugLink: http://bugs.launchpad.net/bugs/507416 > > NEON function is broken on old Freescale iMX51 silicons, such as rev1.x and > rev2.x. Since more and more iMX51 based products will move to rev3.x or later > silicon, we need to enable NEON on these newer silicons and disable NEON on > those old ones. > > This patch will detect the silicon revision dynamically and only export NEON > flag to userspace on Freescale iMX51 rev3.x or later silicon. It was tested on > Babbage 3.0 board and Babbage 2.x board. > > Signed-off-by: Bryan Wu <bryan.wu@canonical.com> > Signed-off-by: Andy Whitcroft <apw@canonical.com> > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > --- > arch/arm/vfp/vfpmodule.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > index 2d7423a..e0d6b01 100644 > --- a/arch/arm/vfp/vfpmodule.c > +++ b/arch/arm/vfp/vfpmodule.c > @@ -432,6 +432,10 @@ out: > > #include <linux/smp.h> > > +#if defined(CONFIG_ARCH_MX51) && defined(CONFIG_NEON) > +#include <mach/hardware.h> > +#endif > + > /* > * VFP support code initialisation. > */ > @@ -498,7 +502,8 @@ static int __init vfp_init(void) > * load/store instructions, integer and single > * precision floating point operations. > */ > - if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > + if (((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > + && cpu_is_mx51_rev(CHIP_REV_3_0) > 0) > elf_hwcap |= HWCAP_NEON; > #endif > } > -- > 1.6.2.4 > > The change matches the description, and revision check looks sane to me. Acked-by: Colin King <colin.king@canonical.com>
On Thu, Feb 11, 2010 at 02:08:44PM -0700, Tim Gardner wrote: > The following changes since commit 4b1196706272826e74e35e9ebf2cc94ba8d4062c: > Leann Ogasawara (1): > UBUNTU: Ubuntu-2.6.31-108.21 > > are available in the git repository at: > > git://kernel.ubuntu.com/rtg/ubuntu-karmic fsl-imx51-neon > > Bryan Wu (1): > UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > arch/arm/vfp/vfpmodule.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > From d77e3635c6d402cebdda49f654402c67460acfab Mon Sep 17 00:00:00 2001 > From: Bryan Wu <bryan.wu@canonical.com> > Date: Tue, 26 Jan 2010 19:00:22 +0000 > Subject: [PATCH] UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > BugLink: http://bugs.launchpad.net/bugs/507416 > > NEON function is broken on old Freescale iMX51 silicons, such as rev1.x and > rev2.x. Since more and more iMX51 based products will move to rev3.x or later > silicon, we need to enable NEON on these newer silicons and disable NEON on > those old ones. > > This patch will detect the silicon revision dynamically and only export NEON > flag to userspace on Freescale iMX51 rev3.x or later silicon. It was tested on > Babbage 3.0 board and Babbage 2.x board. > > Signed-off-by: Bryan Wu <bryan.wu@canonical.com> > Signed-off-by: Andy Whitcroft <apw@canonical.com> > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > --- > arch/arm/vfp/vfpmodule.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > index 2d7423a..e0d6b01 100644 > --- a/arch/arm/vfp/vfpmodule.c > +++ b/arch/arm/vfp/vfpmodule.c > @@ -432,6 +432,10 @@ out: > > #include <linux/smp.h> > > +#if defined(CONFIG_ARCH_MX51) && defined(CONFIG_NEON) > +#include <mach/hardware.h> > +#endif > + > /* > * VFP support code initialisation. > */ > @@ -498,7 +502,8 @@ static int __init vfp_init(void) > * load/store instructions, integer and single > * precision floating point operations. > */ > - if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > + if (((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > + && cpu_is_mx51_rev(CHIP_REV_3_0) > 0) > elf_hwcap |= HWCAP_NEON; > #endif Although this looks perfectly good for the fsl-imx51 branch, this appears wrong for non-imx51 use. I think we should be programming with a view to this being merged into mainline at some point and not be adding anything which makes that harder. Something more like this (with the appropriate incantation in the first stanza): && (!IMX51 || cpu_is_mx51_rev(CHIP_REV_3_0) > 0) -apw
Andy Whitcroft wrote: > On Thu, Feb 11, 2010 at 02:08:44PM -0700, Tim Gardner wrote: >> The following changes since commit 4b1196706272826e74e35e9ebf2cc94ba8d4062c: >> Leann Ogasawara (1): >> UBUNTU: Ubuntu-2.6.31-108.21 >> >> are available in the git repository at: >> >> git://kernel.ubuntu.com/rtg/ubuntu-karmic fsl-imx51-neon >> >> Bryan Wu (1): >> UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon >> >> arch/arm/vfp/vfpmodule.c | 7 ++++++- >> 1 files changed, 6 insertions(+), 1 deletions(-) >> >> From d77e3635c6d402cebdda49f654402c67460acfab Mon Sep 17 00:00:00 2001 >> From: Bryan Wu <bryan.wu@canonical.com> >> Date: Tue, 26 Jan 2010 19:00:22 +0000 >> Subject: [PATCH] UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon >> >> BugLink: http://bugs.launchpad.net/bugs/507416 >> >> NEON function is broken on old Freescale iMX51 silicons, such as rev1.x and >> rev2.x. Since more and more iMX51 based products will move to rev3.x or later >> silicon, we need to enable NEON on these newer silicons and disable NEON on >> those old ones. >> >> This patch will detect the silicon revision dynamically and only export NEON >> flag to userspace on Freescale iMX51 rev3.x or later silicon. It was tested on >> Babbage 3.0 board and Babbage 2.x board. >> >> Signed-off-by: Bryan Wu <bryan.wu@canonical.com> >> Signed-off-by: Andy Whitcroft <apw@canonical.com> >> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> >> --- >> arch/arm/vfp/vfpmodule.c | 7 ++++++- >> 1 files changed, 6 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c >> index 2d7423a..e0d6b01 100644 >> --- a/arch/arm/vfp/vfpmodule.c >> +++ b/arch/arm/vfp/vfpmodule.c >> @@ -432,6 +432,10 @@ out: >> >> #include <linux/smp.h> >> >> +#if defined(CONFIG_ARCH_MX51) && defined(CONFIG_NEON) >> +#include <mach/hardware.h> >> +#endif >> + >> /* >> * VFP support code initialisation. >> */ >> @@ -498,7 +502,8 @@ static int __init vfp_init(void) >> * load/store instructions, integer and single >> * precision floating point operations. >> */ >> - if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100) >> + if (((fmrx(MVFR1) & 0x000fff00) == 0x00011100) >> + && cpu_is_mx51_rev(CHIP_REV_3_0) > 0) >> elf_hwcap |= HWCAP_NEON; >> #endif > > Although this looks perfectly good for the fsl-imx51 branch, this > appears wrong for non-imx51 use. I think we should be programming with > a view to this being merged into mainline at some point and not be > adding anything which makes that harder. Something more like this (with > the appropriate incantation in the first stanza): > > && (!IMX51 || cpu_is_mx51_rev(CHIP_REV_3_0) > 0) > > -apw > You're right, though I'm not too worried about upstreaming Karmic. I think this patch is even a bit worse then you've described. It won't compile without CONFIG_ARCH_MX51. rtg
On 10 Feb 13, Andy Whitcroft wrote: > On Thu, Feb 11, 2010 at 02:08:44PM -0700, Tim Gardner wrote: > > The following changes since commit 4b1196706272826e74e35e9ebf2cc94ba8d4062c: > > Leann Ogasawara (1): > > UBUNTU: Ubuntu-2.6.31-108.21 > > > > are available in the git repository at: > > > > git://kernel.ubuntu.com/rtg/ubuntu-karmic fsl-imx51-neon > > > > Bryan Wu (1): > > UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > > > arch/arm/vfp/vfpmodule.c | 7 ++++++- > > 1 files changed, 6 insertions(+), 1 deletions(-) > > > > From d77e3635c6d402cebdda49f654402c67460acfab Mon Sep 17 00:00:00 2001 > > From: Bryan Wu <bryan.wu@canonical.com> > > Date: Tue, 26 Jan 2010 19:00:22 +0000 > > Subject: [PATCH] UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon > > > > BugLink: http://bugs.launchpad.net/bugs/507416 > > > > NEON function is broken on old Freescale iMX51 silicons, such as rev1.x and > > rev2.x. Since more and more iMX51 based products will move to rev3.x or later > > silicon, we need to enable NEON on these newer silicons and disable NEON on > > those old ones. > > > > This patch will detect the silicon revision dynamically and only export NEON > > flag to userspace on Freescale iMX51 rev3.x or later silicon. It was tested on > > Babbage 3.0 board and Babbage 2.x board. > > > > Signed-off-by: Bryan Wu <bryan.wu@canonical.com> > > Signed-off-by: Andy Whitcroft <apw@canonical.com> > > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > > --- > > arch/arm/vfp/vfpmodule.c | 7 ++++++- > > 1 files changed, 6 insertions(+), 1 deletions(-) > > > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > > index 2d7423a..e0d6b01 100644 > > --- a/arch/arm/vfp/vfpmodule.c > > +++ b/arch/arm/vfp/vfpmodule.c > > @@ -432,6 +432,10 @@ out: > > > > #include <linux/smp.h> > > > > +#if defined(CONFIG_ARCH_MX51) && defined(CONFIG_NEON) > > +#include <mach/hardware.h> > > +#endif > > + > > /* > > * VFP support code initialisation. > > */ > > @@ -498,7 +502,8 @@ static int __init vfp_init(void) > > * load/store instructions, integer and single > > * precision floating point operations. > > */ > > - if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > > + if (((fmrx(MVFR1) & 0x000fff00) == 0x00011100) > > + && cpu_is_mx51_rev(CHIP_REV_3_0) > 0) > > elf_hwcap |= HWCAP_NEON; > > #endif > > Although this looks perfectly good for the fsl-imx51 branch, this > appears wrong for non-imx51 use. I think we should be programming with > a view to this being merged into mainline at some point and not be > adding anything which makes that harder. Something more like this (with > the appropriate incantation in the first stanza): > > && (!IMX51 || cpu_is_mx51_rev(CHIP_REV_3_0) > 0) All this code is getting thrown away for upstreaming. Even the functions prototypes to check for cpu revisions are different. But you're right about the the general coding guidelines. /Amit
Andy Whitcroft 写道: > On Thu, Feb 11, 2010 at 02:08:44PM -0700, Tim Gardner wrote: >> The following changes since commit 4b1196706272826e74e35e9ebf2cc94ba8d4062c: >> Leann Ogasawara (1): >> UBUNTU: Ubuntu-2.6.31-108.21 >> >> are available in the git repository at: >> >> git://kernel.ubuntu.com/rtg/ubuntu-karmic fsl-imx51-neon >> >> Bryan Wu (1): >> UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon >> >> arch/arm/vfp/vfpmodule.c | 7 ++++++- >> 1 files changed, 6 insertions(+), 1 deletions(-) >> >> From d77e3635c6d402cebdda49f654402c67460acfab Mon Sep 17 00:00:00 2001 >> From: Bryan Wu <bryan.wu@canonical.com> >> Date: Tue, 26 Jan 2010 19:00:22 +0000 >> Subject: [PATCH] UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon >> >> BugLink: http://bugs.launchpad.net/bugs/507416 >> >> NEON function is broken on old Freescale iMX51 silicons, such as rev1.x and >> rev2.x. Since more and more iMX51 based products will move to rev3.x or later >> silicon, we need to enable NEON on these newer silicons and disable NEON on >> those old ones. >> >> This patch will detect the silicon revision dynamically and only export NEON >> flag to userspace on Freescale iMX51 rev3.x or later silicon. It was tested on >> Babbage 3.0 board and Babbage 2.x board. >> >> Signed-off-by: Bryan Wu <bryan.wu@canonical.com> >> Signed-off-by: Andy Whitcroft <apw@canonical.com> >> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> >> --- >> arch/arm/vfp/vfpmodule.c | 7 ++++++- >> 1 files changed, 6 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c >> index 2d7423a..e0d6b01 100644 >> --- a/arch/arm/vfp/vfpmodule.c >> +++ b/arch/arm/vfp/vfpmodule.c >> @@ -432,6 +432,10 @@ out: >> >> #include <linux/smp.h> >> >> +#if defined(CONFIG_ARCH_MX51) && defined(CONFIG_NEON) >> +#include <mach/hardware.h> >> +#endif >> + >> /* >> * VFP support code initialisation. >> */ >> @@ -498,7 +502,8 @@ static int __init vfp_init(void) >> * load/store instructions, integer and single >> * precision floating point operations. >> */ >> - if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100) >> + if (((fmrx(MVFR1) & 0x000fff00) == 0x00011100) >> + && cpu_is_mx51_rev(CHIP_REV_3_0) > 0) >> elf_hwcap |= HWCAP_NEON; >> #endif > > Although this looks perfectly good for the fsl-imx51 branch, this > appears wrong for non-imx51 use. I think we should be programming with > a view to this being merged into mainline at some point and not be > adding anything which makes that harder. Something more like this (with > the appropriate incantation in the first stanza): > > && (!IMX51 || cpu_is_mx51_rev(CHIP_REV_3_0) > 0) > > -apw > Actually, at the beginning I just wanna to provide a workaround for this dirty silicon issue. I was not very sure about our upstream approach at that time. It is pretty sure the compiling will fail on other !ARCH_MX51. If we just keep it locally, it is OK just for fsl-imx51 branch. But if we are going to push it to upstream since currently Amit's imx51 patches will be merged into mainline soon, I do like to rework it to make it in right way. Cheers,
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c index 2d7423a..e0d6b01 100644 --- a/arch/arm/vfp/vfpmodule.c +++ b/arch/arm/vfp/vfpmodule.c @@ -432,6 +432,10 @@ out: #include <linux/smp.h> +#if defined(CONFIG_ARCH_MX51) && defined(CONFIG_NEON) +#include <mach/hardware.h> +#endif + /* * VFP support code initialisation. */ @@ -498,7 +502,8 @@ static int __init vfp_init(void) * load/store instructions, integer and single * precision floating point operations. */ - if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100) + if (((fmrx(MVFR1) & 0x000fff00) == 0x00011100) + && cpu_is_mx51_rev(CHIP_REV_3_0) > 0) elf_hwcap |= HWCAP_NEON; #endif }
The following changes since commit 4b1196706272826e74e35e9ebf2cc94ba8d4062c: Leann Ogasawara (1): UBUNTU: Ubuntu-2.6.31-108.21 are available in the git repository at: git://kernel.ubuntu.com/rtg/ubuntu-karmic fsl-imx51-neon Bryan Wu (1): UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon arch/arm/vfp/vfpmodule.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) From d77e3635c6d402cebdda49f654402c67460acfab Mon Sep 17 00:00:00 2001 From: Bryan Wu <bryan.wu@canonical.com> Date: Tue, 26 Jan 2010 19:00:22 +0000 Subject: [PATCH] UBUNTU: SAUCE: IMX51: only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon BugLink: http://bugs.launchpad.net/bugs/507416 NEON function is broken on old Freescale iMX51 silicons, such as rev1.x and rev2.x. Since more and more iMX51 based products will move to rev3.x or later silicon, we need to enable NEON on these newer silicons and disable NEON on those old ones. This patch will detect the silicon revision dynamically and only export NEON flag to userspace on Freescale iMX51 rev3.x or later silicon. It was tested on Babbage 3.0 board and Babbage 2.x board. Signed-off-by: Bryan Wu <bryan.wu@canonical.com> Signed-off-by: Andy Whitcroft <apw@canonical.com> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> --- arch/arm/vfp/vfpmodule.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)