Hi Ilias,
On Wed, 12 Jun 2024 at 00:02, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
[...]
lib/fdtdec.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index b2c59ab3818..b141244e3b9 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1669,8 +1669,16 @@ int fdtdec_setup(void) { int ret = -ENOENT;
/* If allowing a bloblist, check that first */if (CONFIG_IS_ENABLED(BLOBLIST)) {
/** If allowing a bloblist, check that first. This would be better* handled with an OF_BLOBLIST Kconfig, but that caused far too much* argument, so add a hack here, used e.g. by chromebook_coralI am a bit confused by this comment - It means you will not use OF_BLOBLIST, but actually you are using it below. Is it a typo?
Basically it would be cleaner to have a separate, phase-specific Kconfig control as to whether the DT can come from the bloblist (I can't remember the Kconfig name I suggested, nor the patch as it was last year sometime). But for now I am adding this hack to get a few boards working again.
I am a bit confused. First of all the comment is innapropriate. We went through a lengthy discussion on BLOBLIST/OF_BLOSLIST etc and, even Tom chimed in and we made up our minds. Why are you adding this comment now? Why do code comments have to illustrate your personal opinion -- which was rejected?
I'm sorry for the tone of the comment. I am not trying to offend anyone here and I'm happy to change the language.
Yes please a comment explaining why that piece of code is there is far more intuitive.
OK, once we have agreed the below I can do that.
As I probably mentioned at the time, my accepted patch breaks my workflow and several boards. I hope you can understand how frustrating that sort of thing can be.
Yes, I do and I am fine with a short-term patch that fixes issues, as long as its not too intrusive. There is a very thin line between quick and dirty fixes to spaghetti unreadable code. But we should have comments and/or commit messages indicating that this needs a proper fix
I spent a lot of time explaining this last time.
Also, now that I have my lab back up and running and I would like these boards to work again on mainline!
Grepping for OF_BLOBLIST, I can't find any matches, so is the above if a typo?
Remember, it was a patch you rejected :-)
I don't maintain any of that. I only gave some feedback along the lines of "bloblist was designed to be auto-discoverable, I don't see how adding an explicit Kconfig helps". IIRC we eventually followed what Tom suggested.
I'm not trying to point the finger here. So far the boards are broken in mainline...I'm just trying to fix that,
In any case, the amount of bike-shedding in the topic is too much. Do you mind explaining the problem in your workflow again? Perhaps we can find a solution that is integrated in bloblist_maybe_init() instead of injecting ifs on when a bloblist should or should not be searched for all over the place.
TPL (or SPL) sets up a bloblist with bits of info in it, but no DT (which is in memory-mapped SPI flash) U-Boot proper starts up, it wants to get the bloblist but not hang if the bloblist doesn't have a DT
Regards, Simon