[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNAS4V0ohZM+V5xJypejb6Powx2bj_6_kvAGU9L5EnRF=Bw@mail.gmail.com>
Date: Tue, 12 Jan 2021 02:02:52 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Rob Herring <robh+dt@...nel.org>
Cc: David Gibson <david@...son.dropbear.id.au>,
Viresh Kumar <viresh.kumar@...aro.org>,
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)
On Tue, Jan 12, 2021 at 1:13 AM Rob Herring <robh+dt@...nel.org> wrote:
>
> +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.
OK.
dtbo seems a stanrdard already...
> > >+# 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 += -@
Ah yes. I like this.
We do not need the dtbo-y syntax.
We can simply use dtb-y for
base boards and overlay fragments:
dtb-$(CONFIG_ARCH_FOO) += \
foo-base.dtb \
foo-overlay1.dtbo \
foo-overlay2.dtbo
DTB_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
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists