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

[Rob Herring]

+David Gibson

On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> 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.

Given the feedback that dtbo is already a standard, I'd suggest we
just stick with dts->dtbo.

> >+# 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).

Agreed.

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

I think we should stick with 'dtb-y' here.


> 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?

The way it does already. Either:

DTC_FLAGS += -@

in a directory of dts files. Or on a per file basis like:

DTC_FLAGS_foo_base += -@

> 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.

I should have added David here from the start. Honestly, I expected
some discussion there.

Rob