Re: [PATCH] Makefile: binman: Search board directory before srctree

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3943 bytes --]

On Fri, May 15, 2026 at 12:32:30PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 15 May 2026 at 12:24, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, May 15, 2026 at 12:13:50PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 15 May 2026 at 10:03, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, May 15, 2026 at 09:35:10AM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Fri, 15 May 2026 at 08:54, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Fri, May 15, 2026 at 07:43:27AM +0200, Wadim Egorov wrote:
> > > > > >
> > > > > > > A file like rm-cfg.yaml accidentally left in the source tree root
> > > > > > > shadows the board-specific copy. binman builds the wrong YAML, the
> > > > > > > resulting rm-cfg.bin may match a different SoC, and we end up with
> > > > > > > the following error:
> > > > > > >
> > > > > > >   k3_system_controller sysctrler: k3_sysctrler_start:
> > > > > > >       Boot Notification response failed. ret = -110
> > > > > > >
> > > > > > > Move the board directory ahead of the srctree root so that the
> > > > > > > most-specific match wins.
> > > > > > >
> > > > > > > Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> > > > > > > ---
> > > > > > >  Makefile | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > index 552328f9f2c..1ec29dfe0be 100644
> > > > > > > --- a/Makefile
> > > > > > > +++ b/Makefile
> > > > > > > @@ -1682,7 +1682,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
> > > > > > >               build -u -d $(binman_dtb) -O . -m \
> > > > > > >               --allow-missing --fake-ext-blobs \
> > > > > > >               $(if $(BINMAN_ALLOW_MISSING),--ignore-missing) \
> > > > > > > -             -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
> > > > > > > +             -I . -I $(srctree)/board/$(BOARDDIR) -I $(srctree) \
> > > > > > >               $(foreach f,$(of_list_dirs),-I $(f)) -a of-list=$(of_list) \
> > > > > > >               $(foreach f,$(BINMAN_INDIRS),-I $(f)) \
> > > > > > >               -a atf-bl1-path=${BL1} \
> > > > > >
> > > > > > Why do we need to look in $(srctree) itself?
> > > > >
> > > > > See for example:
> > > > >
> > > > > arch/arm/dts/k3-j721e-binman.dtsi:308:  private-key =
> > > > > "arch/arm/mach-k3/keys/custMpk.pem";
> > > >
> > > > That's not great, I'm sorry I missed it at the time. I wonder how easy
> > > > replacing that key is with a non-dummy key. Hopefully it's done in a way
> > > > that doesn't encourage checking private keys in to repositories..
> > >
> > > Typically we would use an environment var passed in from the Makefile,
> > > as we do with BL31, TEE, TI_DM, etc.
> > >
> > > >
> > > > > Also, people sometimes put files in the source tree, e.g. BL31 images,
> > > > > in the hope that they will be found.
> > > >
> > > > That's also a bad practice and what BINMAN_INDIRS is for.
> > >
> > > Yes, but that came quite a bit later with the bintool stuff (2021).
> > >
> > > Anyway, with a bit of work I believe we could drop the source dir.
> > > Would you like to look at it, or should I?
> >
> > Well, the first step would be getting some more feedback on how people
> > override some of these files for example. Even more environment files
> > seems like a bad idea and I'm not sure what BINMAN_INDIRS can't cover
> > today.
> 
> Assuming you mean 'environment variables', we do need the actual
> filename, not just the directory. We could have default filenames for
> each entry type, I suppose. Some do already.

Yes, I think moving that way is the right direction. I was going to use
-scp-path=$(SCP) as an example of something to move away from, but it
looks like that's already being done in a way BINMAN_INDIRS should
handle.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]