| Message ID | cover.1609844956.git.viresh.kumar@linaro.org |
|---|---|
| Headers | show |
| Series | kbuild: Add support to build overlays (%.dtbo) | expand |
On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Hello, > > Here is an attempt to make some changes in the kernel to allow building > of device tree overlays. > > While at it, I would also like to discuss about how we should mention > the base DT blobs in the Makefiles for the overlays, so they can be > build tested to make sure the overlays apply properly. > > A simple way is to mention that with -base extension, like this: > > $(overlay-file)-base := platform-base.dtb > > Any other preference ? I think we'll want something similar to how '-objs' works for modules: foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb (One difference here is we will want all the intermediate targets unlike .o files.) You wouldn't necessarily have all the above combinations, but you have to allow for them. I'm not sure how we'd handle applying any common overlays where the base and overlay are in different directories. Another thing here is adding all the above is not really going to scale on arm32 where we have a single dts directory. We need to move things to per vendor/soc family directories. I have the script to do this. We just need to agree on the vendor names and get Arnd/Olof to run it. I also want that so we can enable schema checks by default once a vendor is warning free (the whole tree is going to take forever). > Also fdtoverlay is an external entity right now, and is not part of the > kernel. Do we need to make it part of the kernel ? Or keep using the > external entity ? Part of the kernel. We just need to add it to the dtc sync script and makefile I think. Rob
On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote: > > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > Hello, > > > > Here is an attempt to make some changes in the kernel to allow building > > of device tree overlays. > > > > While at it, I would also like to discuss about how we should mention > > the base DT blobs in the Makefiles for the overlays, so they can be > > build tested to make sure the overlays apply properly. > > > > A simple way is to mention that with -base extension, like this: > > > > $(overlay-file)-base := platform-base.dtb > > > > Any other preference ? 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. We use chained extension in some places, for example, .dt.yaml for schema yaml files. dtb-$(CONFIG_ARCH_FOO) += \ foo-board.dtb \ foo-overlay1.ovl.dtb \ foo-overlay2.ovl.dtb Overlay DT source file names must end with '.ovl.dts' > > I think we'll want something similar to how '-objs' works for modules: > > foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo > foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo > foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo > dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb > > (One difference here is we will want all the intermediate targets > unlike .o files.) > > You wouldn't necessarily have all the above combinations, but you have > to allow for them. I'm not sure how we'd handle applying any common > overlays where the base and overlay are in different directories. I guess the motivation for supporting -dtbs is to add per-board -@ option only when it contains *.dtbo pattern. But, as you notice, if the overlay files are located under drivers/, it is difficult to add -@ per board. Another scenario is, some people may want to compile downstream overlay files (i.e. similar concept as external modules), then we have no idea which base board should be given with the -@ flag. I'd rather be tempted to add it globally ifdef CONFIG_OF_OVERLAY DTC_FLAGS += -@ endif > > Another thing here is adding all the above is not really going to > scale on arm32 where we have a single dts directory. We need to move > things to per vendor/soc family directories. I have the script to do > this. We just need to agree on the vendor names and get Arnd/Olof to > run it. I also want that so we can enable schema checks by default > once a vendor is warning free (the whole tree is going to take > forever). If this is a big churn, perhaps we could make it extreme to decouple DT and Linux-arch. arch/*/boot/dts/*.dts -> dts/<vendor>/*.dts Documentation/devicetree/bindings -> dts/Bindings/ include/dt-bindings/ -> dts/include/dt-bindings/ Then, other project can take dts/ to reuse for them. > > Also fdtoverlay is an external entity right now, and is not part of the > > kernel. Do we need to make it part of the kernel ? Or keep using the > > external entity ? > > Part of the kernel. We just need to add it to the dtc sync script and > makefile I think. > > Rob -- Best Regards Masahiro Yamada
On 07-01-21, 14:28, Masahiro Yamada wrote: > On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > Hello, > > > > > > Here is an attempt to make some changes in the kernel to allow building > > > of device tree overlays. > > > > > > While at it, I would also like to discuss about how we should mention > > > the base DT blobs in the Makefiles for the overlays, so they can be > > > build tested to make sure the overlays apply properly. > > > > > > A simple way is to mention that with -base extension, like this: > > > > > > $(overlay-file)-base := platform-base.dtb > > > > > > Any other preference ? > > Viresh's patch is not enough. > > We will need to change .gitignore > and scripts/Makefile.dtbinst as well. Thanks. > In my understanding, the build rule is completely the same > between .dtb and .dtbo Right. > 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. > > We use chained extension in some places, for example, > .dt.yaml for schema yaml files. > > > > dtb-$(CONFIG_ARCH_FOO) += \ > foo-board.dtb \ > foo-overlay1.ovl.dtb \ > foo-overlay2.ovl.dtb > > > Overlay DT source file names must end with '.ovl.dts' I am fine with any approach that you guys feel is better, .dts or .ovl.dts. I wanted to start a discussion where we can resolve this and be done with it. Thanks. -- viresh
On Wed, Jan 6, 2021 at 10:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > Hello, > > > > > > Here is an attempt to make some changes in the kernel to allow building > > > of device tree overlays. > > > > > > While at it, I would also like to discuss about how we should mention > > > the base DT blobs in the Makefiles for the overlays, so they can be > > > build tested to make sure the overlays apply properly. > > > > > > A simple way is to mention that with -base extension, like this: > > > > > > $(overlay-file)-base := platform-base.dtb > > > > > > Any other preference ? > > > > 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. > > We use chained extension in some places, for example, > .dt.yaml for schema yaml files. > > > > dtb-$(CONFIG_ARCH_FOO) += \ > foo-board.dtb \ > foo-overlay1.ovl.dtb \ > foo-overlay2.ovl.dtb > > > Overlay DT source file names must end with '.ovl.dts' I like that suggestion as then it's also clear looking at the source files which ones are overlays. Or we'd need .dtso to be consistent. > > I think we'll want something similar to how '-objs' works for modules: > > > > foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo > > foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo > > foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo > > dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb > > > > (One difference here is we will want all the intermediate targets > > unlike .o files.) > > > > You wouldn't necessarily have all the above combinations, but you have > > to allow for them. I'm not sure how we'd handle applying any common > > overlays where the base and overlay are in different directories. > > > I guess the motivation for supporting -dtbs is to > add per-board -@ option only when it contains *.dtbo pattern. I hadn't thought that far, but yeah, that would be good. Really, I just want it to be controlled per SoC family at least. > But, as you notice, if the overlay files are located > under drivers/, it is difficult to add -@ per board. Generally, they shouldn't be. The exceptions are what we already have there which are old dt fixups and unittests. We want the stripped DT repo (devicetree-rebasing) to have all this and drivers/ is stripped out. (Which reminds me, the DT repo will need some work to support all this. It's a different build sys.) > Another scenario is, some people may want to compile > downstream overlay files (i.e. similar concept as external modules), > then we have no idea which base board should be given with the -@ flag. > > > I'd rather be tempted to add it globally > > > ifdef CONFIG_OF_OVERLAY > DTC_FLAGS += -@ > endif We've already rejected doing that. Turning on '-@' can grow the dtb size by a significant amount which could be problematic for some boards. > > Another thing here is adding all the above is not really going to > > scale on arm32 where we have a single dts directory. We need to move > > things to per vendor/soc family directories. I have the script to do > > this. We just need to agree on the vendor names and get Arnd/Olof to > > run it. I also want that so we can enable schema checks by default > > once a vendor is warning free (the whole tree is going to take > > forever). > > > If this is a big churn, perhaps we could make it extreme > to decouple DT and Linux-arch. I would be fine with that, but I don't think we'll get agreement there. With that amount of change, we'll be discussing git submodule again. Rereading the thread on vendor directories[1], we may just move boards one vendor at a time. We could just make that a prerequisite for vendor supporting overlays. > arch/*/boot/dts/*.dts > -> dts/<vendor>/*.dts > > Documentation/devicetree/bindings > -> dts/Bindings/ > > include/dt-bindings/ > -> dts/include/dt-bindings/ > > > > Then, other project can take dts/ > to reuse for them. This is already possible with devicetree-rebasing.git. Though it is still by arch. Rob [1] https://lore.kernel.org/linux-devicetree/20181204183649.GA5716@bogus/
Hello, On 1/7/21 2:02 PM, Rob Herring wrote: > On Wed, Jan 6, 2021 at 10:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote: >> >> On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote: >>> >>> On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: >>>> >>>> Hello, >>>> >>>> Here is an attempt to make some changes in the kernel to allow building >>>> of device tree overlays. >>>> >>>> While at it, I would also like to discuss about how we should mention >>>> the base DT blobs in the Makefiles for the overlays, so they can be >>>> build tested to make sure the overlays apply properly. >>>> >>>> A simple way is to mention that with -base extension, like this: >>>> >>>> $(overlay-file)-base := platform-base.dtb >>>> >>>> Any other preference ? >> >> >> >> 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. *.dtbo is already a well established defato standard. I see little value in changing it and doing so will likely just bifurcate common usage. >> >> We use chained extension in some places, for example, >> .dt.yaml for schema yaml files. >> >> >> >> dtb-$(CONFIG_ARCH_FOO) += \ >> foo-board.dtb \ >> foo-overlay1.ovl.dtb \ >> foo-overlay2.ovl.dtb >> >> >> Overlay DT source file names must end with '.ovl.dts' > > I like that suggestion as then it's also clear looking at the source > files which ones are overlays. Or we'd need .dtso to be consistent. > I don't think there is much of a problem renaming the source side. Don't know if that helps if the output is still dtbo. "Be consistent on dtso" sounds good to me. Can it be enforced via build time checks? > >>> I think we'll want something similar to how '-objs' works for modules: >>> >>> foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo >>> foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo >>> foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo >>> dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb >>> >>> (One difference here is we will want all the intermediate targets >>> unlike .o files.) >>> >>> You wouldn't necessarily have all the above combinations, but you have >>> to allow for them. I'm not sure how we'd handle applying any common >>> overlays where the base and overlay are in different directories. >> >> >> I guess the motivation for supporting -dtbs is to >> add per-board -@ option only when it contains *.dtbo pattern. > > I hadn't thought that far, but yeah, that would be good. Really, I > just want it to be controlled per SoC family at least. > >> But, as you notice, if the overlay files are located >> under drivers/, it is difficult to add -@ per board. > > Generally, they shouldn't be. The exceptions are what we already have > there which are old dt fixups and unittests. > > We want the stripped DT repo (devicetree-rebasing) to have all this > and drivers/ is stripped out. (Which reminds me, the DT repo will need > some work to support all this. It's a different build sys.) > >> Another scenario is, some people may want to compile >> downstream overlay files (i.e. similar concept as external modules), >> then we have no idea which base board should be given with the -@ flag. >> >> >> I'd rather be tempted to add it globally >> >> >> ifdef CONFIG_OF_OVERLAY >> DTC_FLAGS += -@ >> endif > > We've already rejected doing that. Turning on '-@' can grow the dtb > size by a significant amount which could be problematic for some > boards > >>> Another thing here is adding all the above is not really going to >>> scale on arm32 where we have a single dts directory. We need to move >>> things to per vendor/soc family directories. I have the script to do >>> this. We just need to agree on the vendor names and get Arnd/Olof to >>> run it. I also want that so we can enable schema checks by default >>> once a vendor is warning free (the whole tree is going to take >>> forever). >> >> >> If this is a big churn, perhaps we could make it extreme >> to decouple DT and Linux-arch. > > I would be fine with that, but I don't think we'll get agreement > there. With that amount of change, we'll be discussing git submodule > again. > > Rereading the thread on vendor directories[1], we may just move boards > one vendor at a time. We could just make that a prerequisite for > vendor supporting overlays. > >> arch/*/boot/dts/*.dts >> -> dts/<vendor>/*.dts >> >> Documentation/devicetree/bindings >> -> dts/Bindings/ >> >> include/dt-bindings/ >> -> dts/include/dt-bindings/ >> >> >> >> Then, other project can take dts/ >> to reuse for them. > > This is already possible with devicetree-rebasing.git. Though it is > still by arch. > > Rob > > [1] https://lore.kernel.org/linux-devicetree/20181204183649.GA5716@bogus/ > Thanks for digging up this script! Bill
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
On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@linaro.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. >+# 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
+David Gibson On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@linaro.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
On Tue, Jan 12, 2021 at 1:13 AM Rob Herring <robh+dt@kernel.org> wrote: > > +David Gibson > > On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@linaro.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
On Fri, Jan 8, 2021 at 4:02 AM Rob Herring <robh+dt@kernel.org> wrote: > > On Wed, Jan 6, 2021 at 10:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > > > Hello, > > > > > > > > Here is an attempt to make some changes in the kernel to allow building > > > > of device tree overlays. > > > > > > > > While at it, I would also like to discuss about how we should mention > > > > the base DT blobs in the Makefiles for the overlays, so they can be > > > > build tested to make sure the overlays apply properly. > > > > > > > > A simple way is to mention that with -base extension, like this: > > > > > > > > $(overlay-file)-base := platform-base.dtb > > > > > > > > Any other preference ? > > > > > > > > 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. > > > > We use chained extension in some places, for example, > > .dt.yaml for schema yaml files. > > > > > > > > dtb-$(CONFIG_ARCH_FOO) += \ > > foo-board.dtb \ > > foo-overlay1.ovl.dtb \ > > foo-overlay2.ovl.dtb > > > > > > Overlay DT source file names must end with '.ovl.dts' > > I like that suggestion as then it's also clear looking at the source > files which ones are overlays. Or we'd need .dtso to be consistent. > > > > > I think we'll want something similar to how '-objs' works for modules: > > > > > > foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo > > > foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo > > > foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo > > > dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb > > > > > > (One difference here is we will want all the intermediate targets > > > unlike .o files.) > > > > > > You wouldn't necessarily have all the above combinations, but you have > > > to allow for them. I'm not sure how we'd handle applying any common > > > overlays where the base and overlay are in different directories. > > > > > > I guess the motivation for supporting -dtbs is to > > add per-board -@ option only when it contains *.dtbo pattern. > > I hadn't thought that far, but yeah, that would be good. Really, I > just want it to be controlled per SoC family at least. > > > But, as you notice, if the overlay files are located > > under drivers/, it is difficult to add -@ per board. > > Generally, they shouldn't be. The exceptions are what we already have > there which are old dt fixups and unittests. > > We want the stripped DT repo (devicetree-rebasing) to have all this > and drivers/ is stripped out. (Which reminds me, the DT repo will need > some work to support all this. It's a different build sys.) > > > Another scenario is, some people may want to compile > > downstream overlay files (i.e. similar concept as external modules), > > then we have no idea which base board should be given with the -@ flag. > > > > > > I'd rather be tempted to add it globally > > > > > > ifdef CONFIG_OF_OVERLAY > > DTC_FLAGS += -@ > > endif > > We've already rejected doing that. Turning on '-@' can grow the dtb > size by a significant amount which could be problematic for some > boards. > > > > > Another thing here is adding all the above is not really going to > > > scale on arm32 where we have a single dts directory. We need to move > > > things to per vendor/soc family directories. I have the script to do > > > this. We just need to agree on the vendor names and get Arnd/Olof to > > > run it. I also want that so we can enable schema checks by default > > > once a vendor is warning free (the whole tree is going to take > > > forever). > > > > > > If this is a big churn, perhaps we could make it extreme > > to decouple DT and Linux-arch. > > I would be fine with that, but I don't think we'll get agreement > there. With that amount of change, we'll be discussing git submodule > again. > > Rereading the thread on vendor directories[1], we may just move boards > one vendor at a time. We could just make that a prerequisite for > vendor supporting overlays. > > > arch/*/boot/dts/*.dts > > -> dts/<vendor>/*.dts > > > > Documentation/devicetree/bindings > > -> dts/Bindings/ > > > > include/dt-bindings/ > > -> dts/include/dt-bindings/ > > > > > > > > Then, other project can take dts/ > > to reuse for them. > > This is already possible with devicetree-rebasing.git. Though it is > still by arch. Yes, I know this project. There are still cross-references between arm and arm64. Associating DT with Linux-arch is not good because it is possible to boot the 32-bit kernel (arch/arm/) on the 64-bit boards (arch/arm64/boot/dts/). > Rob > > [1] https://lore.kernel.org/linux-devicetree/20181204183649.GA5716@bogus/ -- Best Regards Masahiro Yamada
On 1/5/21 9:21 AM, Rob Herring wrote: > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: >> >> Hello, >> >> Here is an attempt to make some changes in the kernel to allow building >> of device tree overlays. >> >> While at it, I would also like to discuss about how we should mention >> the base DT blobs in the Makefiles for the overlays, so they can be >> build tested to make sure the overlays apply properly. >> >> A simple way is to mention that with -base extension, like this: >> >> $(overlay-file)-base := platform-base.dtb >> >> Any other preference ? > > I think we'll want something similar to how '-objs' works for modules: > > foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo > foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo > foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo > dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb (Thinking ahead....) I'm not sure how to fit connector nodes and the corresponding plugin overlays into this model. A single plugin .dtbo will need to be relocated onto one or more connector nodes. fdtoverlay and the run time overlay apply code do not know how to do this yet. -Frank > > (One difference here is we will want all the intermediate targets > unlike .o files.) > > You wouldn't necessarily have all the above combinations, but you have > to allow for them. I'm not sure how we'd handle applying any common > overlays where the base and overlay are in different directories. > > Another thing here is adding all the above is not really going to > scale on arm32 where we have a single dts directory. We need to move > things to per vendor/soc family directories. I have the script to do > this. We just need to agree on the vendor names and get Arnd/Olof to > run it. I also want that so we can enable schema checks by default > once a vendor is warning free (the whole tree is going to take > forever). > >> Also fdtoverlay is an external entity right now, and is not part of the >> kernel. Do we need to make it part of the kernel ? Or keep using the >> external entity ? > > Part of the kernel. We just need to add it to the dtc sync script and > makefile I think. > > Rob >
On 12-01-21, 02:02, Masahiro Yamada wrote: > On Tue, Jan 12, 2021 at 1:13 AM Rob Herring <robh+dt@kernel.org> wrote: > > On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > We do not need the dtbo-y syntax. +1 And we are left with much simpler diff with what we agreed on. Does this look okay now ? --- .gitignore | 3 +-- Makefile | 4 ++-- scripts/Makefile.dtbinst | 3 +++ scripts/Makefile.lib | 4 +++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index d01cda8e1177..0458c36f3cb2 100644 --- a/.gitignore +++ b/.gitignore @@ -17,8 +17,7 @@ *.bz2 *.c.[012]*.* *.dt.yaml -*.dtb -*.dtb.S +*.dtb* *.dwo *.elf *.gcno diff --git a/Makefile b/Makefile index 9e73f82e0d86..b84f5e0b46ab 100644 --- a/Makefile +++ b/Makefile @@ -1334,7 +1334,7 @@ endif ifneq ($(dtstree),) -%.dtb: include/config/kernel.release scripts_dtc +%.dtb %.dtbo: include/config/kernel.release scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ PHONY += dtbs dtbs_install dtbs_check @@ -1816,7 +1816,7 @@ clean: $(clean-dirs) @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \ \( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \ -o -name '*.ko.*' \ - -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ + -o -name '*.dtb' -o -name '*.dtbo' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ -o -name '*.dwo' -o -name '*.lst' \ -o -name '*.su' -o -name '*.mod' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ diff --git a/scripts/Makefile.dtbinst b/scripts/Makefile.dtbinst index 50d580d77ae9..ba01f5ba2517 100644 --- a/scripts/Makefile.dtbinst +++ b/scripts/Makefile.dtbinst @@ -29,6 +29,9 @@ quiet_cmd_dtb_install = INSTALL $@ $(dst)/%.dtb: $(obj)/%.dtb $(call cmd,dtb_install) +$(dst)/%.dtbo: $(obj)/%.dtbo + $(call cmd,dtb_install) + PHONY += $(subdirs) $(subdirs): $(Q)$(MAKE) $(dtbinst)=$@ dst=$(patsubst $(obj)/%,$(dst)/%,$@) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 213677a5ed33..30bc0a8e0087 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -86,7 +86,9 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(dtb-) ifneq ($(CHECK_DTBS),) extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y)) +extra-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y)) extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-)) +extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-)) endif # Add subdir path @@ -324,7 +326,7 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; -d $(depfile).dtc.tmp $(dtc-tmp) ; \ cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE +$(obj)/%.dtb $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE $(call if_changed_dep,dtc) DT_CHECKER ?= dt-validate