Message ID | 20210611084527.7048-1-kristo@kernel.org |
---|---|
Headers | show |
Series | HSM rearch series for TI K3 devices | expand |
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) 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) 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) 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) 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) 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) 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. Thanks and regards, Lokesh
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. > > > Thanks and regards, > Lokesh >
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? Thanks and regards, Lokesh >> >> >> Thanks and regards, >> Lokesh >> >
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 :)