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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Jan 2021 10:13:24 -0600
From:   Rob Herring <robh+dt@...nel.org>
To:     Masahiro Yamada <masahiroy@...nel.org>,
        David Gibson <david@...son.dropbear.id.au>,
        Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Arnd Bergmann <arnd@...db.de>, Olof Johansson <olof@...om.net>,
        Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
        Frank Rowand <frowand.list@...il.com>,
        Michal Marek <michal.lkml@...kovi.net>,
        DTML <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Bill Mills <bill.mills@...aro.org>, tero.kristo@...il.com
Subject: Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)

+David Gibson

On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@...aro.org> 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