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
