Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)

[Masahiro Yamada]

On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> On 07-01-21, 14:28, Masahiro Yamada wrote:
> > Viresh's patch is not enough.
> >
> > We will need to change .gitignore
> > and scripts/Makefile.dtbinst as well.
> >
> > In my understanding, the build rule is completely the same
> > between .dtb and .dtbo
> > As Rob mentioned, I am not sure if we really need/want
> > a separate extension.
> >
> > A counter approach is to use an extension like '.ovl.dtb'
> > It clarifies it is an overlay fragment without changing
> > anything in our build system or the upstream DTC project.
>
> By the time you gave feedback, I have already sent the dtbo change for
> DTC to the device-tree-compiler list (based on Rob's suggestion).
>
> And it got merged today by David:
>
> https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc
>
> Can we please finalize what we need to do with naming here and be done
> with it, so I can rework my patches and get going ?
>
> Thanks.
>
> --
> viresh



It is unfortunate to see such a patch merged
before getting agreement about how it should work
as a whole.




>+# enable creation of __symbols__ node
>+ifneq ($(dtbo-y),)
>+DTC_FLAGS += -@
>+endif

I am not convinced with this code.

A single user of the dtbo-y syntax gives -@ to all
device trees in the same directory.

This is not a solution since Rob already stated -@ should be
given per board (or per platform, at least).

I still do not understand why adding the new syntax dtbo-y
is helpful.




Have we already decided to use separate ".dtb" and ".dtbo" for blobs?

Will we use ".dts" for all source files?
Or, will we use ".dtso" for overlay source files?

How should the build system determine the targets
that should have -@ option?



For consistency, will we need a patch like follows?


diff --git a/dtc.c b/dtc.c
index bdb3f59..474401e 100644
--- a/dtc.c
+++ b/dtc.c
@@ -120,6 +120,8 @@ static const char *guess_type_by_name(const char
*fname, const char *fallback)
                return fallback;
        if (!strcasecmp(s, ".dts"))
                return "dts";
+       if (!strcasecmp(s, ".dtso"))
+               return "dts";
        if (!strcasecmp(s, ".yaml"))
                return "yaml";
        if (!strcasecmp(s, ".dtb"))
@@ -349,6 +351,8 @@ int main(int argc, char *argv[])

        if (streq(outform, "dts")) {
                dt_to_source(outf, dti);
+       else if (streq(outform, "dtso")) {
+               dt_to_source(outf, dti);
 #ifndef NO_YAML
        } else if (streq(outform, "yaml")) {
                if (!streq(inform, "dts"))



Overall solution looks unclear to me.


Again, it is unfortunate that we did not take enough time
(in spite of the RFC prefix) before proceeding.


-- 
Best Regards
Masahiro Yamada