On Fri, Jun 11, 2021 at 07:13:19PM +0530, Lokesh Vutla wrote:
Tom,
On 11/06/21 4:48 pm, Tero Kristo wrote:
On 11/06/2021 14:08, Lokesh Vutla wrote:
Hi Tero,
On 11/06/21 2:15 pm, Tero Kristo wrote:
Hello,
One more post, this time with the #ifdef hackery converted to use the IS_ENABLED / CONFIG_IS_ENABLED macros, and also removed the "common.h" include from k3-clk.h header. This version also contains fixes to any build issues reported by Lokesh, and these are squashed in to relevant patches.
Can you see if the below warnings can be fixed?
hsm/0018-arm-mach-k3-add-support-for-detecting-firmware-image.patch
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #26: FILE: arch/arm/mach-k3/common.c:31: +#if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
This is static data, can't be fixed. Unless we want to compile it in always?
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #35: FILE: arch/arm/mach-k3/common.c:40: +#if CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS)
Same, static data.
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #55: FILE: arch/arm/mach-k3/common.c:131: +#if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
This is actually just an old macro I changed from #ifdef to IS_ENABLED. Fixing the whole file from the existing #ifdef:s should be outside the scope of this series.
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #124: FILE: arch/arm/mach-k3/common.c:264: +#if CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS)
This code addresses the static data defined before, changing this will break compilation; unless we compile the data always in.
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #128: FILE: arch/arm/mach-k3/common.c:268: +#if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
Same as above.
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #150: FILE: arch/arm/mach-k3/common.c:290: +#if IS_ENABLED(CONFIG_TI_SECURE_DEVICE)
This can't be changed, the code it addresses is only linked in with the config, causing a linker time failure if this is fixed.
Imho, I am not too convinced about the checkpatch tool complaining about these issues. :)
-Tero
total: 0 errors, 6 warnings, 0 checks, 144 lines checked
NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace.
hsm/0018-arm-mach-k3-add-support-for-detecting-firmware-image.patch has style problems, please review.
hsm/0019-arm-mach-k3-do-board-config-for-PM-only-if-supported.patch
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #24: FILE: arch/arm/mach-k3/sysfw-loader.c:162: +#if !CONFIG_IS_ENABLED(K3_DM_FW)
total: 0 errors, 1 warnings, 0 checks, 13 lines checked
NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace.
I assume you are okay with this?
Well, if all the cases where we should have been using IS_ENABLED/CONFIG_IS_ENABLED but weren't have been fixed, and we aren't adding <common.h> to more files, then yes, everything else that can be done as a later clean-up can just be done as a later clean-up, and indeed if it's not possible to use an if (...) it's not possible to use an if :)