Re: [PATCHv6 00/26] HSM rearch series for TI K3 devices

Lokesh Vutla
2021-06-11T13:43:38Z

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
>>
>