Re: [PATCH 2/2] ARM: don't assume 32-bit when no boards are selected

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08.06.23 10:59, Lior Weintraub wrote:
> Hi Ahmad,
> 
> I think that replacing Kconfig with .config on the error message will not help either.
> Maybe Kconfig is a good name because it suggest that the issue is there but again, not so helpful because it doesn't point to the Kconfig file and line that brock the configuration.

There is no single line that broke the configuration. The user is supposed
to take an existing defconfig and manipulate that. If they write their
own defconfig that doesn't enable any CPUs, then they get an error message
that they enabled no CPUs instead of random compiler errors.

> As I said, I have no prior experience with Kbuild so it could be out of scope for the barebox project.

Changing the error message to something more useful is definitely in-scope.
Suggestions are welcome.

> The patch I gave below just add this wrong line ("depends on HAVE_ARCH_STM32MP157") into arch/arm/mach-stm32mp/Kconfig
> Then you can run:
> "make stm32mp_defconfig" and see the strange warnings (which again doesn't help to pin point what is wrong with the Kconfig file).
> Once you build with "make" you will see the new error ("#error No boards/CPUs selected in Kconfig").

There's no HAVE_ARCH_STM32MP157 symbol defined with barebox.
(Symbols are defined e.g. with
  config HAVE_ARCH_STM32MP157
  bool
).

I don't think we can protect ourselves against arbitrary
modification of the Kconfig files. Most users just take
a defconfig and customize it with menuconfig.

In the past, developers had a bit harder time, but we have
multi_v8_defconfig and multi_v7_defconfig, which can be used
for new architectures as well (assuming the Kconfig of the
new architecture opts in into CONFIG_ARCH_MULTIARCH).

> After reverting the wrong Kconfig line, I tried to run "make mrproper" but this didn't help.
> I was still getting the same warning when I run "make stm32mp_defconfig" and the same error when I run "make".

I can't reproduce this. Even if you change arch/arm/mach-stm32mp/Kconfig as
described, you will still have other STM32MP1 boards that enable CPUs.

Cheers,
Ahmad

> 
> Hope this helps,
> Cheers,
> Lior.
> 
>> -----Original Message-----
>> From: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
>> Sent: Thursday, June 8, 2023 11:47 AM
>> To: Lior Weintraub <liorw@xxxxxxxxxx>; barebox@xxxxxxxxxxxxxxxxxxx
>> Cc: Ulrich Ölmann <u.oelmann@xxxxxxxxxxxxxx>; Sascha Hauer
>> <sha@xxxxxxxxxxxxxx>
>> Subject: Re: [PATCH 2/2] ARM: don't assume 32-bit when no boards are
>> selected
>>
>> CAUTION: External Sender
>>
>> Hello Lior,
>>
>> On 08.06.23 10:20, Lior Weintraub wrote:
>>> Hi Ahmad,
>>>
>>> Thanks for this fix!
>>> Few comments:
>>> 1. I was getting errors when I tried to apply your patch on either "master" or
>> "next" branch but then realized your fix was already merged to "next" :-).
>>
>> Ye, Sascha was fast :-)
>>
>>> 2. Indeed this fix doesn't show the original error we saw but now it shows:
>> "error: #error No boards/CPUs selected in Kconfig". It still doesn't help to find
>> the culprit.
>>
>> How would you rephrase it to be more useful? Replace `Kconfig' with `.config'?
>>
>>> 3. During the investigation, I also found another issue which is related to
>> temp\generated build files.
>>>
>>> The following patch introduces a mistake in Kconfig file.
>>
>> What defconfig do you source after applying the patch and
>> what commands do you run?
>>
>>>
>>> diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-
>> stm32mp/Kconfig
>>> index bc0a48d64c..97b61810d7 100644
>>> --- a/arch/arm/mach-stm32mp/Kconfig
>>> +++ b/arch/arm/mach-stm32mp/Kconfig
>>> @@ -30,6 +30,7 @@ config MACH_LXA_MC1
>>>       bool "Linux Automation MC-1 board"
>>>
>>>  config MACH_SEEED_ODYSSEY
>>> +     depends on HAVE_ARCH_STM32MP157
>>>       select ARCH_STM32MP157
>>>       bool "Seeed Studio Odyssey"
>>>
>>> Now run make stm32mp_defconfig and see:
>>>
>>> WARNING: unmet direct dependencies detected for USB_DWC2
>>>   Depends on [n]: USB_HOST [=y] && USB [=y] && HAS_DMA [=n]
>>>   Selected by [y]:
>>>   - USB_DWC2_HOST [=y] && USB_HOST [=y]
>>>   - USB_DWC2_GADGET [=y] && USB_HOST [=y] && USB_GADGET [=y]
>>>
>>> WARNING: unmet direct dependencies detected for USB_DWC2
>>>   Depends on [n]: USB_HOST [=y] && USB [=y] && HAS_DMA [=n]
>>>   Selected by [y]:
>>>   - USB_DWC2_HOST [=y] && USB_HOST [=y]
>>>   - USB_DWC2_GADGET [=y] && USB_HOST [=y] && USB_GADGET [=y]
>>>
>>> (Not very helpful warnings but then again I have no experience with Kbuild)
>>>
>>> Then make -j will produce the error "#error No boards/CPUs selected in
>> Kconfig".
>>> Now even if you revert the change on Kconfig there is no way to fix the build
>> (tried make clean) without getting a new clone into a fresh folder.
>>
>> make clean intentionally doesn't wipe the configuration, only the build
>> artifacts.
>>
>>> I assume there is something left from previous build that is not cleaned
>> correctly.
>>
>> If you want to source a defconfig, use
>>
>>   make my_platforms_defconfig
>>
>> If you want to delete everything, use
>>
>>   make mrproper
>>
>> Same applies to other Kbuild projects.
>>
>>>
>>> Cheers,
>>> Lior.
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
>>>> Sent: Wednesday, June 7, 2023 6:32 PM
>>>> To: barebox@xxxxxxxxxxxxxxxxxxx
>>>> Cc: Lior Weintraub <liorw@xxxxxxxxxx>; Ulrich Ölmann
>>>> <u.oelmann@xxxxxxxxxxxxxx>; Ahmad Fatoum
>> <a.fatoum@xxxxxxxxxxxxxx>
>>>> Subject: [PATCH 2/2] ARM: don't assume 32-bit when no boards are
>> selected
>>>>
>>>> CAUTION: External Sender
>>>>
>>>> barebox build errors are very confusing if no board is selected.
>>>> This should have been fixed with commit 14b296d2a7e6 ("arm: error
>>>> out if __LINUX_ARM_ARCH__ is undefined"), but unfortunately that's
>>>> only true for ARM32. On ARM64, the error message in question
>>>> is not printed, because build aborts even earlier, because Kbuild
>>>> assumes it should build for 32-bit ARM and thus passes the ARM64
>>>> compiler options that it can't understand:
>>>>
>>>>   aarch64-oe-linux-gcc: error: unrecognized argument in option '-
>> mabi=apcs-
>>>> gnu'
>>>>   aarch64-oe-linux-gcc: note: valid arguments to '-mabi=' are: ilp32 lp64
>>>>   aarch64-oe-linux-gcc: error: unrecognized command-line option '-msoft-
>>>> float'
>>>>   aarch64-oe-linux-gcc: error: unrecognized command-line option '-mno-
>>>> unaligned-access'
>>>>
>>>> Let's fix that for ARM64 builds by not assuming !CONFIG_CPU_64 to be 32-
>>>> bit,
>>>> but instead explicitly check that CONFIG_CPU_32 is set before doing
>>>> 32-bit specific changes.
>>>>
>>>> This ensures we now fail during compilation on both ARM32 and ARM64 if
>>>> no boards were selected. We can't fail earlier via $(error ...) as this
>>>> would impact use of targets like menuconfig.
>>>>
>>>> Reported-by: Lior Weintraub <liorw@xxxxxxxxxx>
>>>> Reported-by: Ulrich Ölmann <u.oelmann@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
>>>> ---
>>>>  arch/arm/Makefile | 35 ++++++++++++++---------------------
>>>>  1 file changed, 14 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>>>> index 926af7387f7f..337b7e9095fa 100644
>>>> --- a/arch/arm/Makefile
>>>> +++ b/arch/arm/Makefile
>>>> @@ -6,7 +6,8 @@ KBUILD_CPPFLAGS += -D__ARM__ -fno-strict-aliasing
>>>>  # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
>>>>  ifeq ($(CONFIG_CPU_64),y)
>>>>  KBUILD_CPPFLAGS        +=$(call cc-option,-maarch64,)
>>>> -else
>>>> +endif
>>>> +ifeq ($(CONFIG_CPU_32),y)
>>>>  KBUILD_CPPFLAGS        +=$(call cc-option,-marm,)
>>>>  KBUILD_CPPFLAGS        += -msoft-float
>>>>  endif
>>>> @@ -27,14 +28,12 @@ endif
>>>>  # at least some of the code would be executed with MMU off, lets be
>>>>  # conservative and instruct the compiler not to generate any unaligned
>>>>  # accesses
>>>> -ifneq ($(CONFIG_CPU_64),y)
>>>> +ifeq ($(CONFIG_CPU_32),y)
>>>>  KBUILD_CFLAGS += -mno-unaligned-access
>>>> -else
>>>> -KBUILD_CFLAGS += -mstrict-align
>>>>  endif
>>>> -
>>>> -# Prevent use of floating point and Advanced SIMD registers.
>>>>  ifeq ($(CONFIG_CPU_64),y)
>>>> +KBUILD_CFLAGS += -mstrict-align
>>>> +# Prevent use of floating point and Advanced SIMD registers.
>>>>  KBUILD_CFLAGS += -mgeneral-regs-only
>>>>  endif
>>>>
>>>> @@ -54,19 +53,11 @@ tune-$(CONFIG_CPU_ARM920T)  :=-
>>>> mtune=arm9tdmi
>>>>  tune-$(CONFIG_CPU_ARM926T)     :=-mtune=arm9tdmi
>>>>  tune-$(CONFIG_CPU_XSCALE)      :=$(call cc-option,-mtune=xscale,-
>>>> mtune=strongarm110) -Wa,-mcpu=xscale
>>>>
>>>> -ifeq ($(CONFIG_CPU_64), y)
>>>> -CFLAGS_ABI     :=-mabi=lp64
>>>> -else
>>>> -ifeq ($(CONFIG_AEABI),y)
>>>> -CFLAGS_ABI     :=-mabi=aapcs-linux
>>>> -else
>>>> -CFLAGS_ABI     :=$(call cc-option,-mapcs-32,-mabi=apcs-gnu) $(call cc-
>>>> option,-mno-thumb-interwork,)
>>>> -endif
>>>> -endif
>>>> +CFLAGS_ABI-$(CONFIG_CPU_64)    :=-mabi=lp64
>>>> +CFLAGS_ABI-$(CONFIG_CPU_32)    :=$(call cc-option,-mapcs-32,-
>> mabi=apcs-
>>>> gnu) $(call cc-option,-mno-thumb-interwork,)
>>>> +CFLAGS_ABI-$(CONFIG_AEABI)     :=-mabi=aapcs-linux
>>>>
>>>> -ifeq ($(CONFIG_ARM_UNWIND),y)
>>>> -CFLAGS_ABI     +=-funwind-tables
>>>> -endif
>>>> +CFLAGS_ABI-$(CONFIG_ARM_UNWIND)        +=-funwind-tables
>>>>
>>>>  ifeq ($(CONFIG_THUMB2_BAREBOX),y)
>>>>  AFLAGS_AUTOIT  :=$(call as-option,-Wa$(comma)-mimplicit-it=always,-
>>>> Wa$(comma)-mauto-it)
>>>> @@ -75,13 +66,15 @@ CFLAGS_THUMB2       :=-mthumb
>> $(AFLAGS_AUTOIT)
>>>> $(AFLAGS_NOWARN)
>>>>  AFLAGS_THUMB2  :=$(CFLAGS_THUMB2) -Wa$(comma)-mthumb
>>>>  endif
>>>>
>>>> +KBUILD_CPPFLAGS += $(CFLAGS_ABI-y) $(arch-y) $(tune-y)
>>>> +
>>>>  ifeq ($(CONFIG_CPU_64), y)
>>>> -KBUILD_CPPFLAGS += $(CFLAGS_ABI) $(arch-y) $(tune-y)
>>>>  KBUILD_AFLAGS   += -include asm/unified.h
>>>>  export S64_32 = 64
>>>>  export S64 = 64
>>>> -else
>>>> -KBUILD_CPPFLAGS += $(CFLAGS_ABI) $(arch-y) $(tune-y)
>>>> $(CFLAGS_THUMB2)
>>>> +endif
>>>> +ifeq ($(CONFIG_CPU_32), y)
>>>> +KBUILD_CPPFLAGS += $(CFLAGS_THUMB2)
>>>>  KBUILD_AFLAGS   += -include asm/unified.h -msoft-float
>> $(AFLAGS_THUMB2)
>>>>  export S64_32 = 32
>>>>  export S32 = 32
>>>> --
>>>> 2.39.2
>>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |





[Index of Archives]     [Linux Embedded]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux