is the 'enable_neon_fpu' function in cortexA9.S (imx6 Platform SDK) missing a return?

[EdSutter]

I'm by no means an ARM assembler expert, but I am working with the iMX6 Platform SDK and just noticed that the assembler function enable_neon_fpu (copied below, extracted from iMX6_Platform_SDK/sdk/core/src/cortexA9.s) appears to be missing a return...

  .global enable_neon_fpu

  .func enable_neon_fpu

enable_neon_fpu:

    /* set NSACR, both Secure and Non-secure access are allowed to NEON */

    MRC p15, 0, r0, c1, c1, 2

    ORR r0, r0, #(0x3<<10) @ enable fpu/neon

    MCR p15, 0, r0, c1, c1, 2

    /* Set the CPACR for access to CP10 and CP11*/

    LDR r0, =0xF00000

    MCR p15, 0, r0, c1, c0, 2

    /* Set the FPEXC EN bit to enable the FPU */

    MOV r3, #0x40000000

    @VMSR FPEXC, r3

    MCR p10, 7, r3, c8, c0, 0

  .endfunc

Can someone verify this?

[EdSutter · Accepted solution]

I confirmed with some tests that there is definitely a missing "BX lr" as the last line of the "enable_neon_fpu" function.

So, while this doesn't appear to cause any harm simply because the platform_init.c code that calls enable_neon_fpu() immediately follows that with a call to disable_strict_align_check().  Nevertheless, this should be fixed.

[LeonardoSandova]

FYI GEFanuc_engr

[LeonardoSandova]

This function is not expected to return a value, so it is correct. This is the declaration:

./sdk/core/cortex_a9.h:77:void enable_neon_fpu(void);

and it is called on

./apps/common/platform_init.c:43: enable_neon_fpu();

Leo

[EdSutter]

Right, I see that it is a void; however, I'm not talking about the return of a value, I mean just the "return".  For example, shouldn't there be something like a "BX lr" or a "pop {r0,pc}" or something similar?  Looking at all the other functions in that file, they have something like that as the last line of the function. It appears to me that this function just falls through and uses the return that is part of the next function in that file (disable_strict_align_check).  Below is a larger snippet from that file (with my pointer to the location missing the return)...

  .global cpu_get_current

  @ int cpu_get_current(void)@

  @ get current CPU ID

  .func cpu_get_current

cpu_get_current:

    mrc   p15, 0, r0, c0, c0, 5

    and   r0, r0, #3

    BX    lr

  .endfunc    @cpu_get_current()@

  .global enable_neon_fpu

  .func enable_neon_fpu

enable_neon_fpu:

    /* set NSACR, both Secure and Non-secure access are allowed to NEON */

    MRC p15, 0, r0, c1, c1, 2

    ORR r0, r0, #(0x3<<10) @ enable fpu/neon

    MCR p15, 0, r0, c1, c1, 2

    /* Set the CPACR for access to CP10 and CP11*/

    LDR r0, =0xF00000

    MCR p15, 0, r0, c1, c0, 2

    /* Set the FPEXC EN bit to enable the FPU */

    MOV r3, #0x40000000

    @VMSR FPEXC, r3

    MCR p10, 7, r3, c8, c0, 0

                                         <<<<<<< missing return here?

  .endfunc

  .global disable_strict_align_check

  .func disable_strict_align_check

disable_strict_align_check:

    /*Ray's note: disable strict alignment fault checking.

    without disabling this, data abort will happen when accessing

    the BPB structure of file system since it is packed.*/

    push    {r0, lr}

    mrc p15, 0, r0, c1, c0, 0

    bic r0, r0, #(0x1<<1) @clear A bit of SCTLR

    mcr p15, 0, r0, c1, c0, 0

    pop {r0, pc}

  .endfunc