| Message ID | D23DC9B8BD1C245D+20250211140740.1812778-2-wangyuli@uniontech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: Explicitly check KBUILD_SYM32=n | expand |
On Tue, 11 Feb 2025, WangYuli wrote: > During make module_install, the need_compiler variable becomes 0, > so Makefile.compiler isn't included. > > This results in call cc-option-yn returning nothing. > > Add a check for KBUILD_SYM32=n to avoid the > "CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32" error > when KBUILD_SYM32 is unset (meaning it's not 'y' or 'n'). Jeez, just wrap it into `ifdef need-compiler' as I told you previously. This stuff isn't used with `make modules_install', none of the compiler flags matter as the compiler isn't ever called, which is obvious from Makefile.compiler not being included in the first place. I only did not do it with commit a79a404e6c22 ("MIPS: Fix CONFIG_CPU_DADDI_WORKAROUNDS `modules_install' regression"), because I was unaware about this LLVM's limitation and GCC version requirements at the time implied the presence of this feature at all times. See also commit 4fe4a6374c4d ("MIPS: Only fiddle with CHECKFLAGS if `need-compiler'") for a similar change from the same series[1]. Also need-compiler is nil rather than 0 in the relevant case. Please also double-check your change description before posting, at least when it comes to referring to codebase artifacts: s/module_install/modules_install/ s/need_compiler/need-compiler/ This is stuff people may be grepping for. References: [1] "MIPS: Fix build issues from the introduction of `need-compiler'", <https://lore.kernel.org/r/alpine.DEB.2.21.2307180025120.62448@angie.orcam.me.uk/> Maciej
Hi Maciej, Your reply is a little bit confusing, so let me rephrase to make sure I got this right: There are expression and spelling errors in my commit message and code comments, necessitating a patch v2 for corrections, is that right? As for whether to check need-compiler or KBUILD_SYM32 in the code, the effect is essentially the same, correct? Thanks, -- WangYuli
On Wed, 12 Feb 2025, WangYuli wrote: > There are expression and spelling errors in my commit message and code > comments, necessitating a patch v2 for corrections, is that right? These are semantic errors, yes. Also I find: Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed") invalid here, as your change does not address an issue introduced with said commit. It does fix mine, when it comes to LLVM support, so I think instead it has to be: Fixes: a79a404e6c22 ("MIPS: Fix CONFIG_CPU_DADDI_WORKAROUNDS `modules_install' regression") And if you want it backported, then you need to swap the order of the changes or discard 1/2 altogether, as that is syntactic noise only and clearly not a fix. Apologies not to point these issues out in the previous message. > As for whether to check need-compiler or KBUILD_SYM32 in the code, the effect > is essentially the same, correct? No, code has to express intent and the intent here is not to fiddle with the compilation flags when no compiler is going to be used. And this purpose is served by the `need-compiler' setting; anything else is code obfuscation and a workaround at best. And the very need to add a comment has made it very obvious already: the best code is self-explanatory and the use of `need-compiler' is a common idiom, obviating the need for a comment here. Have I made myself clear here? Maciej
diff --git a/arch/mips/Makefile b/arch/mips/Makefile index 962eb749ed23..2a0bf69c842b 100644 --- a/arch/mips/Makefile +++ b/arch/mips/Makefile @@ -304,8 +304,13 @@ ifdef CONFIG_64BIT ifeq ($(KBUILD_SYM32), y) cflags-y += -msym32 -DKBUILD_64BIT_SYM32 else - ifeq ($(CONFIG_CPU_DADDI_WORKAROUNDS), y) - $(error CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32) +# During make module_install, the need_compiler variable +# becomes 0, so Makefile.compiler isn't included. +# This results in call cc-option-yn returning nothing. + ifeq ($(KBUILD_SYM32), n) + ifeq ($(CONFIG_CPU_DADDI_WORKAROUNDS), y) + $(error CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32) + endif endif endif endif
During make module_install, the need_compiler variable becomes 0, so Makefile.compiler isn't included. This results in call cc-option-yn returning nothing. Add a check for KBUILD_SYM32=n to avoid the "CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32" error when KBUILD_SYM32 is unset (meaning it's not 'y' or 'n'). Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed") Fixes: 18ca63a2e23c ("MIPS: Probe toolchain support of -msym32") Reported-by: Maciej W. Rozycki <macro@orcam.me.uk> Closes: https://lore.kernel.org/all/alpine.DEB.2.21.2501030535080.49841@angie.orcam.me.uk/ Co-developed-by: Chen Linxuan <chenlinxuan@uniontech.com> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com> Signed-off-by: WangYuli <wangyuli@uniontech.com> --- arch/mips/Makefile | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)