diff mbox series

Series-to: u-boot Cover-letter: Fix Typo error in Makefile

Message ID 20200408153915.21032-1-sicris.embay@gmail.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series Series-to: u-boot Cover-letter: Fix Typo error in Makefile | expand

Commit Message

Sicris Rey Embay April 8, 2020, 3:39 p.m. UTC
This patch fixes the typo error in Makefile where
-I$(srctree)/arch/$(ARCH)/thumb1/include is not picked up
in the compiler flag when compiling for thumb2.
END

Signed-off-by: Sicris <sicris.embay@gmail.com>
---

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Heinrich Schuchardt April 9, 2020, 12:49 p.m. UTC | #1
On 2020-04-08 17:39, Sicris wrote:
> This patch fixes the typo error in Makefile where
> -I$(srctree)/arch/$(ARCH)/thumb1/include is not picked up
> in the compiler flag when compiling for thumb2.
> END
>
> Signed-off-by: Sicris <sicris.embay@gmail.com>
> ---
>
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 8de5ff6d94..503b30392d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -704,7 +704,7 @@ UBOOTINCLUDE    := \
>  		-Iinclude \
>  		$(if $(KBUILD_SRC), -I$(srctree)/include) \
>  		$(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
> -			$(if $(CONFIG_HAS_THUMB2),, \
> +			$(if $(CONFIG_HAS_THUMB2), \
>  				-I$(srctree)/arch/$(ARCH)/thumb1/include),) \

This patch relates to
https://stackoverflow.com/questions/61097841/error-selected-processor-does-not-support-requested-special-purpose-register

The original problem was using the wrong compiler as discussed on
Stackoverflow.

Compiling stm32f769-disco_defconfig works fine using the Debian Bullseye
package gcc-arm-linux-gnueabi without this patch.

The current logic is:
If it is thumb and not thumb2, include the thumb1 directory.
If it is thumb and thumb2, do not include the thumb1 directory.

You are inverting the logic with your patch:

If it is thumb and thumb2, include the thumb1 directory.
If it is thumb and not thumb2, do not include the thumb1 directory.

This does not make much sense to me.

For understanding the if-statement I found the following Makefile useful:

----
all:
        $(if ,,echo 1)
        $(if ,echo 2)
        $(if 1,,echo 3)
        $(if 1,echo 4)
---

The output contains 1 and 4.

Best regards

Heinrich


>  		-I$(srctree)/arch/$(ARCH)/include \
>  		-include $(srctree)/include/linux/kconfig.h
>
Sicris Rey Embay April 9, 2020, 2:25 p.m. UTC | #2
On Thu, Apr 9, 2020 at 8:49 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 2020-04-08 17:39, Sicris wrote:
> > This patch fixes the typo error in Makefile where
> > -I$(srctree)/arch/$(ARCH)/thumb1/include is not picked up
> > in the compiler flag when compiling for thumb2.
> > END
> >
> > Signed-off-by: Sicris <sicris.embay@gmail.com>
> > ---
> >
> >  Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 8de5ff6d94..503b30392d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -704,7 +704,7 @@ UBOOTINCLUDE    := \
> >               -Iinclude \
> >               $(if $(KBUILD_SRC), -I$(srctree)/include) \
> >               $(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
> > -                     $(if $(CONFIG_HAS_THUMB2),, \
> > +                     $(if $(CONFIG_HAS_THUMB2), \
> >                               -I$(srctree)/arch/$(ARCH)/thumb1/include),) \
>
> This patch relates to
> https://stackoverflow.com/questions/61097841/error-selected-processor-does-not-support-requested-special-purpose-register
>
> The original problem was using the wrong compiler as discussed on
> Stackoverflow.

Sorry, I was not very clear on the problem that I've encountered.

I tried the M-profile as suggested in the stackoverflow comment from
Frant.  However, I still get the same error
with CONFIG_CMD_UBI=y.

{standard input}:757: Error: selected processor does not support
requested special purpose register – msr cpsr_c,r3

This is due to the assembly inline function local_irq_save() /
local_irq_restore.  On closer inspection,
the wrong header for local_irq_save / local_irq_restore was picked up
by the compiler.

Wrong Header: /arch/arm/include/asm/proc-armv/system.h
(Closest i can find) Correct Header:
/arch/arm/thumb1/include/asm/proc-armv/system.h

>
> Compiling stm32f769-disco_defconfig works fine using the Debian Bullseye
> package gcc-arm-linux-gnueabi without this patch.

Can you help me check if this compiles using Debian Bullseye with
CONFIG_CMD_UBI=y?
>
> The current logic is:
> If it is thumb and not thumb2, include the thumb1 directory.
> If it is thumb and thumb2, do not include the thumb1 directory.
>
> You are inverting the logic with your patch:
>
> If it is thumb and thumb2, include the thumb1 directory.
> If it is thumb and not thumb2, do not include the thumb1 directory.
>
> This does not make much sense to me.
>

Does this suggestion make sense?

    $(if $(CONFIG_HAS_THUMB2), \
        -I$(srctree)/arch/$(ARCH)/thumb2/include),
        -I$(srctree)/arch/$(ARCH)/thumb1/include),) \

with thumb2/include contains the same thumb1/include contents.

> For understanding the if-statement I found the following Makefile useful:
>
> ----
> all:
>         $(if ,,echo 1)
>         $(if ,echo 2)
>         $(if 1,,echo 3)
>         $(if 1,echo 4)
> ---
>
> The output contains 1 and 4.
>
> Best regards
>
> Heinrich
>
>
> >               -I$(srctree)/arch/$(ARCH)/include \
> >               -include $(srctree)/include/linux/kconfig.h
> >
>

cheers,
Sicris Rey
Tom Rini April 9, 2020, 2:54 p.m. UTC | #3
On Thu, Apr 09, 2020 at 02:49:41PM +0200, Heinrich Schuchardt wrote:
> On 2020-04-08 17:39, Sicris wrote:
> > This patch fixes the typo error in Makefile where
> > -I$(srctree)/arch/$(ARCH)/thumb1/include is not picked up
> > in the compiler flag when compiling for thumb2.
> > END
> >
> > Signed-off-by: Sicris <sicris.embay@gmail.com>
> > ---
> >
> >  Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 8de5ff6d94..503b30392d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -704,7 +704,7 @@ UBOOTINCLUDE    := \
> >  		-Iinclude \
> >  		$(if $(KBUILD_SRC), -I$(srctree)/include) \
> >  		$(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
> > -			$(if $(CONFIG_HAS_THUMB2),, \
> > +			$(if $(CONFIG_HAS_THUMB2), \
> >  				-I$(srctree)/arch/$(ARCH)/thumb1/include),) \
> 
> This patch relates to
> https://stackoverflow.com/questions/61097841/error-selected-processor-does-not-support-requested-special-purpose-register
> 
> The original problem was using the wrong compiler as discussed on
> Stackoverflow.
> 
> Compiling stm32f769-disco_defconfig works fine using the Debian Bullseye
> package gcc-arm-linux-gnueabi without this patch.
> 
> The current logic is:
> If it is thumb and not thumb2, include the thumb1 directory.
> If it is thumb and thumb2, do not include the thumb1 directory.
> 
> You are inverting the logic with your patch:
> 
> If it is thumb and thumb2, include the thumb1 directory.
> If it is thumb and not thumb2, do not include the thumb1 directory.
> 
> This does not make much sense to me.
> 
> For understanding the if-statement I found the following Makefile useful:
> 
> ----
> all:
>         $(if ,,echo 1)
>         $(if ,echo 2)
>         $(if 1,,echo 3)
>         $(if 1,echo 4)
> ---
> 
> The output contains 1 and 4.

Perhaps things need testing with a bit wider range of compilers, to make
sure we're doing the right thing in all cases, and then perhaps changing
the logic to make sure we pass the right flags for what we're told to
build for.
Heinrich Schuchardt April 9, 2020, 5:23 p.m. UTC | #4
On 2020-04-09 16:25, Sicris Rey Embay wrote:
> On Thu, Apr 9, 2020 at 8:49 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 2020-04-08 17:39, Sicris wrote:
>>> This patch fixes the typo error in Makefile where
>>> -I$(srctree)/arch/$(ARCH)/thumb1/include is not picked up
>>> in the compiler flag when compiling for thumb2.
>>> END
>>>
>>> Signed-off-by: Sicris <sicris.embay@gmail.com>
>>> ---
>>>
>>>  Makefile | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 8de5ff6d94..503b30392d 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -704,7 +704,7 @@ UBOOTINCLUDE    := \
>>>               -Iinclude \
>>>               $(if $(KBUILD_SRC), -I$(srctree)/include) \
>>>               $(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
>>> -                     $(if $(CONFIG_HAS_THUMB2),, \
>>> +                     $(if $(CONFIG_HAS_THUMB2), \
>>>                               -I$(srctree)/arch/$(ARCH)/thumb1/include),) \
>>
>> This patch relates to
>> https://stackoverflow.com/questions/61097841/error-selected-processor-does-not-support-requested-special-purpose-register
>>
>> The original problem was using the wrong compiler as discussed on
>> Stackoverflow.
>
> Sorry, I was not very clear on the problem that I've encountered.
>
> I tried the M-profile as suggested in the stackoverflow comment from
> Frant.  However, I still get the same error
> with CONFIG_CMD_UBI=y.
>
> {standard input}:757: Error: selected processor does not support
> requested special purpose register – msr cpsr_c,r3
>
> This is due to the assembly inline function local_irq_save() /
> local_irq_restore.  On closer inspection,
> the wrong header for local_irq_save / local_irq_restore was picked up
> by the compiler.
>
> Wrong Header: /arch/arm/include/asm/proc-armv/system.h
> (Closest i can find) Correct Header:
> /arch/arm/thumb1/include/asm/proc-armv/system.h
>
>>
>> Compiling stm32f769-disco_defconfig works fine using the Debian Bullseye
>> package gcc-arm-linux-gnueabi without this patch.
>
> Can you help me check if this compiles using Debian Bullseye with
> CONFIG_CMD_UBI=y?

With this selected compiling fails.

{standard input}:965: Error: selected processor does not support
requested special purpose register -- `mrs r1,cpsr'

The CPSR register is not present on ARMv6-M and ARMv7-M processors.

So on these architectures arch/arm/include/asm/proc-armv/system.h has to
be replaced by arch/arm/thumb1/include/asm/proc-armv/system.h.

>>
>> The current logic is:
>> If it is thumb and not thumb2, include the thumb1 directory.
>> If it is thumb and thumb2, do not include the thumb1 directory.
>>
>> You are inverting the logic with your patch:
>>
>> If it is thumb and thumb2, include the thumb1 directory.
>> If it is thumb and not thumb2, do not include the thumb1 directory.
>>
>> This does not make much sense to me.
>>
>
> Does this suggestion make sense?
>
>     $(if $(CONFIG_HAS_THUMB2), \
>         -I$(srctree)/arch/$(ARCH)/thumb2/include),
>         -I$(srctree)/arch/$(ARCH)/thumb1/include),) \

We do not want to use a special include on ARMv7-A even if building in
thumbs mode. So we should explicitly check the CPU architecture here.

If CONFIG_HAS_THUMB2 is undefined or CONFIG_CPU_V7M is defined,
we should use -I$(srctree)/arch/$(ARCH)/thumb1/include.

--- a/Makefile
+++ b/Makefile
@@ -701,13 +701,15 @@ KBUILD_CFLAGS += $(KCFLAGS)
 # Use UBOOTINCLUDE when you must reference the include/ directory.
 # Needed to be compatible with the O= option
-UBOOTINCLUDE    := \
-		-Iinclude \
-		$(if $(KBUILD_SRC), -I$(srctree)/include) \
-		$(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
-			$(if $(CONFIG_HAS_THUMB2),, \
-				-I$(srctree)/arch/$(ARCH)/thumb1/include),) \
-		-I$(srctree)/arch/$(ARCH)/include \
-		-include $(srctree)/include/linux/kconfig.h
+UBOOTINCLUDE    := \
+	-Iinclude \
+	$(if $(KBUILD_SRC), -I$(srctree)/include) \
+	$(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
+		$(if $(CONFIG_HAS_THUMB2), \
+			$(if $(CONFIG_CPU_V7M), \
+				-I$(srctree)/arch/arm/thumb1/include), \
+			-I$(srctree)/arch/arm/thumb1/include)) \
+	-I$(srctree)/arch/$(ARCH)/include \
+	-include $(srctree)/include/linux/kconfig.h

Best regards

Heinrich

>
> with thumb2/include contains the same thumb1/include contents.
>
>> For understanding the if-statement I found the following Makefile useful:
>>
>> ----
>> all:
>>         $(if ,,echo 1)
>>         $(if ,echo 2)
>>         $(if 1,,echo 3)
>>         $(if 1,echo 4)
>> ---
>>
>> The output contains 1 and 4.
>>
>> Best regards
>>
>> Heinrich
>>
>>
>>>               -I$(srctree)/arch/$(ARCH)/include \
>>>               -include $(srctree)/include/linux/kconfig.h
>>>
>>
>
> cheers,
> Sicris Rey
>
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 8de5ff6d94..503b30392d 100644
--- a/Makefile
+++ b/Makefile
@@ -704,7 +704,7 @@  UBOOTINCLUDE    := \
 		-Iinclude \
 		$(if $(KBUILD_SRC), -I$(srctree)/include) \
 		$(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
-			$(if $(CONFIG_HAS_THUMB2),, \
+			$(if $(CONFIG_HAS_THUMB2), \
 				-I$(srctree)/arch/$(ARCH)/thumb1/include),) \
 		-I$(srctree)/arch/$(ARCH)/include \
 		-include $(srctree)/include/linux/kconfig.h