Hi Ilias, On Tue, 11 Jun 2024 at 08:27, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > On Tue, 11 Jun 2024 at 13:54, Simon Glass <s...@chromium.org> wrote: > > > > Hi Raymond, > > > > On Mon, 10 Jun 2024 at 13:13, Raymond Mao <raymond....@linaro.org> wrote: > > > > > > Hi Simon, > > > > > > On Tue, 4 Jun 2024 at 23:27, Simon Glass <s...@chromium.org> wrote: > > >> > > >> On some boards, the bloblist is created in SPL once SDRAM is ready. It > > >> cannot be accessed until that point, so is not available early in SPL. > > >> > > >> Add a condition to avoid a hang in this case. > > >> > > >> This fixes a hang in chromebook_coral > > >> > > >> Fixes: 70fe2385943 ("fdt: Allow the devicetree to come from a bloblist") > > >> > > >> Signed-off-by: Simon Glass <s...@chromium.org> > > >> --- > > >> > > >> 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_coral > > > > > > I 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. 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. 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 :-) Regards, Simon > > > Thanks > /Ilias > > > > > > > >> > > >> + * The necessary test is whether the previous stage passed a > > >> bloblist, > > >> + * not whether this one creates one. > > >> + */ > > >> + if (CONFIG_IS_ENABLED(OF_BLOBLIST) && > > >> + (spl_prev_phase() != PHASE_TPL || > > >> + !IS_ENABLED(CONFIG_TPL_BLOBLIST))) { > > >> ret = bloblist_maybe_init(); > > >> if (!ret) { > > >> gd->fdt_blob = > > >> bloblist_find(BLOBLISTT_CONTROL_FDT, 0); > > >> -- > > >> 2.34.1 > > >> > > > > > > Regards, > > > Raymond > > > > Regards, > > Simon