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
