[...]
> > > >> ---
> > > >>
> > > >> 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.
Yes please a comment explaining why that piece of code is there is far
more intuitive.
> 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
> 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.
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.
Regards
/Ilias
>
> 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
