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

[Heinrich Schuchardt]

On 2020-04-09 16:25, Sicris Rey Embay wrote:
> On Thu, Apr 9, 2020 at 8:49 PM Heinrich Schuchardt <xypron.glpk at 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 at 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
>