[PATCH 20/20] Convert CONFIG_SYS_BOOTM_LEN to Kconfig

[Soeren Moch]

On 26.06.22 00:44, Tom Rini wrote:
> On Sun, Jun 26, 2022 at 12:01:23AM +0200, Soeren Moch wrote:
>>>>> diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
>>>>> index 892d7c60d283..f0ecfd049d65 100644
>>>>> --- a/configs/tbs2910_defconfig
>>>>> +++ b/configs/tbs2910_defconfig
>>>>> @@ -35,6 +35,7 @@ CONFIG_CMD_BOOTZ=y
>>>>>     # CONFIG_BOOTM_PLAN9 is not set
>>>>>     # CONFIG_BOOTM_RTEMS is not set
>>>>>     # CONFIG_BOOTM_VXWORKS is not set
>>>>> +CONFIG_SYS_BOOTM_LEN=0x1000000
>>>> For tbs2910 there is another value set here - not the old default, not
>>>> the new one. Why?
>>>> This looks like a carefully chosen value, but very likely it is not.
>>>>
>>>> Probably also this value is fine for this board, but for me it makes no
>>>> sense to set a board specific value here.
>>> I don't follow you, sorry.  Since tbs2910 isn't X86 or PPC or ARM64,
>>> it's using the current value instead.  Before this patch
>>> include/configs/tbs2910.h includes include/configs/mx6_common.h which
>>> sets it to 0x1000000.
>> OK, I missed that part in the long patch, my bad.
>>
>> As I wrote, all possible values are probably good for this board.
>> I just would prefer to use some default value (as before), since there
>> is no special requirement for this board in particular. To keep the
>> old value in this conversion of course makes sense.
>>> Which means I probably could put a || ARCH_MX6 in
>>> the Kconfig entry and drop out another 84 files (3 MX6 platforms set
>>> 0x4000000 instead today).
>>>
>> That would make it more obvious that the old default is used for
>> these boards. But if you prefer the board specific defconfig
>> setting, that of course is also a valid solution.
> To be clear, here (and unless I otherwise note, all of the other
> conversions I've been doing) the intention is the same values on every
> platform, before and after.  Many times the patches end up small because
> I can do a few default A if B lines and cover the vast majority if
> platforms.  Sadly sometimes we get cases like this where in hindsight, a
> bigger default, or a more consistent default for every architecture
> should have been used, but wasn't.  We can clean it up after but it
> needs to be separate so that someone can bisect a problem back to when
> the value was changed, not when it was migrated, if there's a problem.
I totally agree. For me it looked like the default was changed for
tbs2910, but it was not. My mistake.
In your v2 it is obvious that we use the same defaults, great.
>
> So this isn't a super platform dependent value, and should probably be
> something like 96MiB for 32bit ARM (I forget what the practical maximum
> image size is, I know I talked with rmk about it about a decade ago I
> think),
I _think_ the maximum text size (or image size?) is 16MiB for arm,
but I'm not totally sure about this. But this would match with
the mx6/mx7 defaults.
>   and some other similar sane maximum for other platforms.  But, a
> follow up to do that.
>
Yes, changes should be done in a separate patch. But I do not see
any need from my side. And I'm happy that there is no board
specific defconfig entry for tbs2910 in your v2.

Thanks,
Soeren