| Message ID | 1502831736-28282-1-git-send-email-trini@konsulko.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: > With support for stacked overlays being part of libfdt it is now > possible and likely that overlays which require __symbols__ will be > applied to the dtb files generated by the kernel. This is done by > passing -@ to dtc. This does increase the filesize (and resident memory > usage) based on the number of __symbol__ entries added to match the > contents of the dts. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Michal Marek <mmarek@suse.com> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > CC: linux-kbuild@vger.kernel.org > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > In order for a dtb file to be useful with all types of overlays, it > needs to be generated with the -@ flag passed to dtc so that __symbols__ > are generated. This however is not free, and increases the resulting > dtb file by up to approximately 50% today. In the current worst case > this is moving from 88KiB to 133KiB. In talking with Frank about this, Plus some amount for the unflattened tree in memory, too. > he outlined 3 possible ways (with the 4th option of something else > entirely). > > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > 2. In the kernel, if the kernel does not have overlay support, discard > the __symbols__ information that we've been passed. > 3. Have the bootloader pass in, or not, __symbols__ information. > > This patch is an attempt to implement something between the 3rd option > and a different, 4th option. Frank was thinking that we might introduce > a new symbol to control generation of __symbol__ information for option > 1. I think this gets the usage backwards and will lead to confusion > among users and developers. > > My proposal is that we do not want __symbols__ existence to be dependent > on some part of the kernel configuration for a number of reasons. > First, this is out of step with the rest of how dtbs are created today > and more importantly, thought about. Today, all dtb content is > independent of CONFIG options. If you build a dtb from a given kernel > tree, everyone will agree on the result. This is part of the "contract" > on passing old kernels and new dtb files even. Agree completely. I don't even like that building dtbs depends on the ARCH. However, option 2 may still be useful. There's no point exposing what can't be used. Furthermore, exposing __symbols__ in /proc/device-tree at all may be a bad idea. We should consider if it should always be hidden. That would also allow storing the __symbols__ data however we want internally (i.e. with less memory usage). The complication is always kexec which I haven't thought about too much here. Also, perhaps we need finer grain control of __symbols__ generation. We really don't want userspace to be able to modify anything in the DT at any point in time. That's a big can of worms and we don't want to start there. The problem is labels are widely used just for convenience and weren't part of the ABI. With overlays that changes, so we either need to restrict labels usage or define another way. It could be as simple as defining some prefix for label names for labels to export. > Second, I think this is out of step with how a lot of overlay usage will > occur. My thinking is that with maximally useful overlays being > available in mainline, lots of use-cases that we have today that result > in a number of DTS files being included can become just overlays. This > is true in terms of not only evaluation kits but also when these systems > are turned into custom hardware. This is even more true for SoM based > systems where a physical widget would be a SoM + carrier overlay + > custom parts overlay. These cases are going to be resolved with > overlays being applied outside of the kernel. > > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > drivers/of/unittest-data/Makefile | 5 ----- > scripts/Makefile.lib | 3 +++ > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile > index 6e00a9c69e58..70731cfe8900 100644 > --- a/drivers/of/unittest-data/Makefile > +++ b/drivers/of/unittest-data/Makefile > @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S > .PRECIOUS: \ > $(obj)/%.dtb.S \ > $(obj)/%.dtb > - > -# enable creation of __symbols__ node > -DTC_FLAGS_overlay := -@ > -DTC_FLAGS_overlay_bad_phandle := -@ > -DTC_FLAGS_overlay_base := -@ > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 58c05e5d9870..a1f4a6b29d75 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ > -Wproperty_name_chars_strict > endif > > +# enable creation of __symbols__ node > +DTC_FLAGS += -@ > + > DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) > > # Generate an assembly file to wrap the output of the device tree compiler > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 15, 2017 at 05:36:11PM -0500, Rob Herring wrote: > On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: > > With support for stacked overlays being part of libfdt it is now > > possible and likely that overlays which require __symbols__ will be > > applied to the dtb files generated by the kernel. This is done by > > passing -@ to dtc. This does increase the filesize (and resident memory > > usage) based on the number of __symbol__ entries added to match the > > contents of the dts. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Frank Rowand <frowand.list@gmail.com> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > Cc: Michal Marek <mmarek@suse.com> > > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > CC: linux-kbuild@vger.kernel.org > > Signed-off-by: Tom Rini <trini@konsulko.com> > > --- > > In order for a dtb file to be useful with all types of overlays, it > > needs to be generated with the -@ flag passed to dtc so that __symbols__ > > are generated. This however is not free, and increases the resulting > > dtb file by up to approximately 50% today. In the current worst case > > this is moving from 88KiB to 133KiB. In talking with Frank about this, > > Plus some amount for the unflattened tree in memory, too. > > > he outlined 3 possible ways (with the 4th option of something else > > entirely). > > > > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > > 2. In the kernel, if the kernel does not have overlay support, discard > > the __symbols__ information that we've been passed. > > 3. Have the bootloader pass in, or not, __symbols__ information. > > > > This patch is an attempt to implement something between the 3rd option > > and a different, 4th option. Frank was thinking that we might introduce > > a new symbol to control generation of __symbol__ information for option > > 1. I think this gets the usage backwards and will lead to confusion > > among users and developers. > > > > My proposal is that we do not want __symbols__ existence to be dependent > > on some part of the kernel configuration for a number of reasons. > > First, this is out of step with the rest of how dtbs are created today > > and more importantly, thought about. Today, all dtb content is > > independent of CONFIG options. If you build a dtb from a given kernel > > tree, everyone will agree on the result. This is part of the "contract" > > on passing old kernels and new dtb files even. > > Agree completely. I don't even like that building dtbs depends on the ARCH. > > However, option 2 may still be useful. There's no point exposing what > can't be used. Furthermore, exposing __symbols__ in /proc/device-tree > at all may be a bad idea. We should consider if it should always be > hidden. That would also allow storing the __symbols__ data however we > want internally (i.e. with less memory usage). The complication is > always kexec which I haven't thought about too much here. A further patch to the kernel at run-time, OK. If you give me some crumbs I'll see if I can figure out the next steps. > Also, perhaps we need finer grain control of __symbols__ generation. Here I have to disagree. > We really don't want userspace to be able to modify anything in the DT > at any point in time. That's a big can of worms and we don't want to > start there. The problem is labels are widely used just for > convenience and weren't part of the ABI. With overlays that changes, > so we either need to restrict labels usage or define another way. It > could be as simple as defining some prefix for label names for labels > to export. I think there needs to be a difference noted between "here is what policy the kernel is going to enforce about run time changes" and "here is what the user is going to assemble a system to look like". Again, stemming from the part where the Linux kernel is where dts files reside and are generated from normally. If we have it in __symbols__, someone can make use of it in hardware design (again, think of the SoM + carrier + custom) bit, I've seen so many real life products now that would be simplified in this manner). Thanks!
On 08/15/17 14:15, Tom Rini wrote: > With support for stacked overlays being part of libfdt it is now > possible and likely that overlays which require __symbols__ will be > applied to the dtb files generated by the kernel. This is done by > passing -@ to dtc. This does increase the filesize (and resident memory > usage) based on the number of __symbol__ entries added to match the > contents of the dts. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Michal Marek <mmarek@suse.com> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > CC: linux-kbuild@vger.kernel.org > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > In order for a dtb file to be useful with all types of overlays, it > needs to be generated with the -@ flag passed to dtc so that __symbols__ > are generated. This however is not free, and increases the resulting > dtb file by up to approximately 50% today. In the current worst case > this is moving from 88KiB to 133KiB. In talking with Frank about this, > he outlined 3 possible ways (with the 4th option of something else > entirely). > > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > 2. In the kernel, if the kernel does not have overlay support, discard > the __symbols__ information that we've been passed. > 3. Have the bootloader pass in, or not, __symbols__ information. I also was hoping that other people might have ideas for additional approaches. > This patch is an attempt to implement something between the 3rd option > and a different, 4th option. Frank was thinking that we might introduce > a new symbol to control generation of __symbol__ information for option > 1. I think this gets the usage backwards and will lead to confusion > among users and developers. > > My proposal is that we do not want __symbols__ existence to be dependent > on some part of the kernel configuration for a number of reasons. > First, this is out of step with the rest of how dtbs are created today > and more importantly, thought about. Today, all dtb content is > independent of CONFIG options. If you build a dtb from a given kernel > tree, everyone will agree on the result. This is part of the "contract" > on passing old kernels and new dtb files even. I hope that dtb contents are independent of CONFIG options, but I don't feel confident is stating that there is not such dependency. (Of course, whether to build a dtb can be dependent on a CONFIG option in the Makefile, but that is not the same concept.) The only existing rule that I am aware of that helps avoid a dts dependency on kernel CONFIG options is that included files can not be from general kernel header files; they must be in include/dt-bindings/. Should we add text to Documentation/devicetree/bindings/submitting-patches.txt that explicitly states that dts files are not allowed to contain any dependency on kernel CONFIG options? > Second, I think this is out of step with how a lot of overlay usage will > occur. My thinking is that with maximally useful overlays being > available in mainline, lots of use-cases that we have today that result > in a number of DTS files being included can become just overlays. This I disagree with this. My _opinion_ is that overlays should be the exception, not the common case. Overlays require extra complexity in the various subsystems that interact with device trees. For an overlay to work, these subsystems must be able to react to changes made to the device tree by an overlay. The current mechanism is via notifiers, which only exist for a few subsystems. This extra complexity has the potential to be rather fragile, and receive less extensive testing than the normal boot path. My preference is that the number of subsystems supporting overlays is minimized, to minimize the fragility. I suspect that drivers that support overlays will also be more fragile. And it is not just subsystems and drivers. I suspect that overlays will also expose more of the boot time ordering issues that we keep running into. > is true in terms of not only evaluation kits but also when these systems > are turned into custom hardware. This is even more true for SoM based > systems where a physical widget would be a SoM + carrier overlay + > custom parts overlay. These cases are going to be resolved with > overlays being applied outside of the kernel. > > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > drivers/of/unittest-data/Makefile | 5 ----- > scripts/Makefile.lib | 3 +++ > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile > index 6e00a9c69e58..70731cfe8900 100644 > --- a/drivers/of/unittest-data/Makefile > +++ b/drivers/of/unittest-data/Makefile > @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S > .PRECIOUS: \ > $(obj)/%.dtb.S \ > $(obj)/%.dtb > - > -# enable creation of __symbols__ node > -DTC_FLAGS_overlay := -@ > -DTC_FLAGS_overlay_bad_phandle := -@ > -DTC_FLAGS_overlay_base := -@ > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib No to that change. I want to be able to control whether or not __SYMBOLS__ is generated for each of those files. > index 58c05e5d9870..a1f4a6b29d75 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ > -Wproperty_name_chars_strict > endif > > +# enable creation of __symbols__ node > +DTC_FLAGS += -@ > + > DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) > > # Generate an assembly file to wrap the output of the device tree compiler > And no to that change. A dtb should not have __SYMBOLS__ enabled unless it is needed (or plausibly needed). The default case for the majority of dtbs is that there is not need to support overlays and thus the overhead of the __SYMBOLS__ node is all overhead with no benefit. This method turns on __SYMBOLS__ for all dtbs. -Frank -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/15/17 15:36, Rob Herring wrote: > On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >> With support for stacked overlays being part of libfdt it is now >> possible and likely that overlays which require __symbols__ will be >> applied to the dtb files generated by the kernel. This is done by >> passing -@ to dtc. This does increase the filesize (and resident memory >> usage) based on the number of __symbol__ entries added to match the >> contents of the dts. >> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Frank Rowand <frowand.list@gmail.com> >> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >> Cc: Michal Marek <mmarek@suse.com> >> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >> Cc: devicetree@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> CC: linux-kbuild@vger.kernel.org >> Signed-off-by: Tom Rini <trini@konsulko.com> >> --- >> In order for a dtb file to be useful with all types of overlays, it >> needs to be generated with the -@ flag passed to dtc so that __symbols__ >> are generated. This however is not free, and increases the resulting >> dtb file by up to approximately 50% today. In the current worst case >> this is moving from 88KiB to 133KiB. In talking with Frank about this, > > Plus some amount for the unflattened tree in memory, too. > >> he outlined 3 possible ways (with the 4th option of something else >> entirely). >> >> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >> 2. In the kernel, if the kernel does not have overlay support, discard >> the __symbols__ information that we've been passed. >> 3. Have the bootloader pass in, or not, __symbols__ information. >> >> This patch is an attempt to implement something between the 3rd option >> and a different, 4th option. Frank was thinking that we might introduce >> a new symbol to control generation of __symbol__ information for option >> 1. I think this gets the usage backwards and will lead to confusion >> among users and developers. >> >> My proposal is that we do not want __symbols__ existence to be dependent >> on some part of the kernel configuration for a number of reasons. >> First, this is out of step with the rest of how dtbs are created today >> and more importantly, thought about. Today, all dtb content is >> independent of CONFIG options. If you build a dtb from a given kernel >> tree, everyone will agree on the result. This is part of the "contract" >> on passing old kernels and new dtb files even. > > Agree completely. I don't even like that building dtbs depends on the ARCH. > > However, option 2 may still be useful. There's no point exposing what > can't be used. Furthermore, exposing __symbols__ in /proc/device-tree > at all may be a bad idea. We should consider if it should always be > hidden. That would also allow storing the __symbols__ data however we > want internally (i.e. with less memory usage). Yes. I would prefer to treat the __symbols__ node as an internal representation of information used by the device tree subsystem. It is not hardware description. > The complication is > always kexec which I haven't thought about too much here. That should not be an issue, because the device tree is exposed to kexec via /sys/firmware/fdt instead of /sys/firmware/devicetree/base (which is what /proc/device-tree links to), according to Documentation/ABI/testing/sysfs-firmware-ofw. So the __symbols__ node will be exposed to kexec. > Also, perhaps we need finer grain control of __symbols__ generation. > We really don't want userspace to be able to modify anything in the DT > at any point in time. That's a big can of worms and we don't want to > start there. The problem is labels are widely used just for > convenience and weren't part of the ABI. With overlays that changes, > so we either need to restrict labels usage or define another way. It > could be as simple as defining some prefix for label names for labels > to export. Agreed. We could also restrict labels in connector nodes to be visible. >> Second, I think this is out of step with how a lot of overlay usage will >> occur. My thinking is that with maximally useful overlays being >> available in mainline, lots of use-cases that we have today that result >> in a number of DTS files being included can become just overlays. This >> is true in terms of not only evaluation kits but also when these systems >> are turned into custom hardware. This is even more true for SoM based >> systems where a physical widget would be a SoM + carrier overlay + >> custom parts overlay. These cases are going to be resolved with >> overlays being applied outside of the kernel. >> >> Signed-off-by: Tom Rini <trini@konsulko.com> >> --- >> drivers/of/unittest-data/Makefile | 5 ----- >> scripts/Makefile.lib | 3 +++ >> 2 files changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile >> index 6e00a9c69e58..70731cfe8900 100644 >> --- a/drivers/of/unittest-data/Makefile >> +++ b/drivers/of/unittest-data/Makefile >> @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S >> .PRECIOUS: \ >> $(obj)/%.dtb.S \ >> $(obj)/%.dtb >> - >> -# enable creation of __symbols__ node >> -DTC_FLAGS_overlay := -@ >> -DTC_FLAGS_overlay_bad_phandle := -@ >> -DTC_FLAGS_overlay_base := -@ >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >> index 58c05e5d9870..a1f4a6b29d75 100644 >> --- a/scripts/Makefile.lib >> +++ b/scripts/Makefile.lib >> @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ >> -Wproperty_name_chars_strict >> endif >> >> +# enable creation of __symbols__ node >> +DTC_FLAGS += -@ >> + >> DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) >> >> # Generate an assembly file to wrap the output of the device tree compiler >> -- >> 1.9.1 >> > . > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/15/17 16:57, Frank Rowand wrote: > On 08/15/17 15:36, Rob Herring wrote: >> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >>> With support for stacked overlays being part of libfdt it is now >>> possible and likely that overlays which require __symbols__ will be >>> applied to the dtb files generated by the kernel. This is done by >>> passing -@ to dtc. This does increase the filesize (and resident memory >>> usage) based on the number of __symbol__ entries added to match the >>> contents of the dts. >>> >>> Cc: Rob Herring <robh+dt@kernel.org> >>> Cc: Frank Rowand <frowand.list@gmail.com> >>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >>> Cc: Michal Marek <mmarek@suse.com> >>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >>> Cc: devicetree@vger.kernel.org >>> Cc: linux-kernel@vger.kernel.org >>> CC: linux-kbuild@vger.kernel.org >>> Signed-off-by: Tom Rini <trini@konsulko.com> >>> --- >>> In order for a dtb file to be useful with all types of overlays, it >>> needs to be generated with the -@ flag passed to dtc so that __symbols__ >>> are generated. This however is not free, and increases the resulting >>> dtb file by up to approximately 50% today. In the current worst case >>> this is moving from 88KiB to 133KiB. In talking with Frank about this, >> >> Plus some amount for the unflattened tree in memory, too. >> >>> he outlined 3 possible ways (with the 4th option of something else >>> entirely). >>> >>> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >>> 2. In the kernel, if the kernel does not have overlay support, discard >>> the __symbols__ information that we've been passed. >>> 3. Have the bootloader pass in, or not, __symbols__ information. >>> >>> This patch is an attempt to implement something between the 3rd option >>> and a different, 4th option. Frank was thinking that we might introduce >>> a new symbol to control generation of __symbol__ information for option >>> 1. I think this gets the usage backwards and will lead to confusion >>> among users and developers. >>> >>> My proposal is that we do not want __symbols__ existence to be dependent >>> on some part of the kernel configuration for a number of reasons. >>> First, this is out of step with the rest of how dtbs are created today >>> and more importantly, thought about. Today, all dtb content is >>> independent of CONFIG options. If you build a dtb from a given kernel >>> tree, everyone will agree on the result. This is part of the "contract" >>> on passing old kernels and new dtb files even. >> >> Agree completely. I don't even like that building dtbs depends on the ARCH. >> >> However, option 2 may still be useful. There's no point exposing what >> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree >> at all may be a bad idea. We should consider if it should always be >> hidden. That would also allow storing the __symbols__ data however we >> want internally (i.e. with less memory usage). > > Yes. I would prefer to treat the __symbols__ node as an internal > representation of information used by the device tree subsystem. > It is not hardware description. > > >> The complication is >> always kexec which I haven't thought about too much here. > > That should not be an issue, because the device tree is exposed to kexec > via /sys/firmware/fdt instead of /sys/firmware/devicetree/base (which > is what /proc/device-tree links to), according to > Documentation/ABI/testing/sysfs-firmware-ofw. So the __symbols__ > node will be exposed to kexec. > > >> Also, perhaps we need finer grain control of __symbols__ generation. >> We really don't want userspace to be able to modify anything in the DT >> at any point in time. That's a big can of worms and we don't want to >> start there. The problem is labels are widely used just for >> convenience and weren't part of the ABI. With overlays that changes, >> so we either need to restrict labels usage or define another way. It >> could be as simple as defining some prefix for label names for labels >> to export. > > Agreed. We could also restrict labels in connector nodes to be visible. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ I meant to say restrict visibility of labels, so that only labels in connector nodes would be visible. > > >>> Second, I think this is out of step with how a lot of overlay usage will >>> occur. My thinking is that with maximally useful overlays being >>> available in mainline, lots of use-cases that we have today that result >>> in a number of DTS files being included can become just overlays. This >>> is true in terms of not only evaluation kits but also when these systems >>> are turned into custom hardware. This is even more true for SoM based >>> systems where a physical widget would be a SoM + carrier overlay + >>> custom parts overlay. These cases are going to be resolved with >>> overlays being applied outside of the kernel. >>> >>> Signed-off-by: Tom Rini <trini@konsulko.com> >>> --- >>> drivers/of/unittest-data/Makefile | 5 ----- >>> scripts/Makefile.lib | 3 +++ >>> 2 files changed, 3 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile >>> index 6e00a9c69e58..70731cfe8900 100644 >>> --- a/drivers/of/unittest-data/Makefile >>> +++ b/drivers/of/unittest-data/Makefile >>> @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S >>> .PRECIOUS: \ >>> $(obj)/%.dtb.S \ >>> $(obj)/%.dtb >>> - >>> -# enable creation of __symbols__ node >>> -DTC_FLAGS_overlay := -@ >>> -DTC_FLAGS_overlay_bad_phandle := -@ >>> -DTC_FLAGS_overlay_base := -@ >>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >>> index 58c05e5d9870..a1f4a6b29d75 100644 >>> --- a/scripts/Makefile.lib >>> +++ b/scripts/Makefile.lib >>> @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ >>> -Wproperty_name_chars_strict >>> endif >>> >>> +# enable creation of __symbols__ node >>> +DTC_FLAGS += -@ >>> + >>> DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) >>> >>> # Generate an assembly file to wrap the output of the device tree compiler >>> -- >>> 1.9.1 >>> >> . >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/15/17 15:36, Rob Herring wrote: > On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >> With support for stacked overlays being part of libfdt it is now < snip > >> My proposal is that we do not want __symbols__ existence to be dependent >> on some part of the kernel configuration for a number of reasons. >> First, this is out of step with the rest of how dtbs are created today >> and more importantly, thought about. Today, all dtb content is >> independent of CONFIG options. If you build a dtb from a given kernel >> tree, everyone will agree on the result. This is part of the "contract" >> on passing old kernels and new dtb files even. > > Agree completely. I don't even like that building dtbs depends on the ARCH. < snip > I also agree that use of CONFIG options is a solution that I do not like. It has always seemed that there is a bit of an impedence mismatch in that we build a dtb in an environment that is configured for a specific board and architecture. Does anyone have any thoughts on another way to control whether or not a given dtb or a given build of a dtb would contain the symbols needed for overlays or not include the symbols? -Frank -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/15/17 14:15, Tom Rini wrote: > With support for stacked overlays being part of libfdt it is now > possible and likely that overlays which require __symbols__ will be > applied to the dtb files generated by the kernel. This is done by > passing -@ to dtc. This does increase the filesize (and resident memory > usage) based on the number of __symbol__ entries added to match the > contents of the dts. < snip > And for some background, for those who were not on the irc channel, here is the conversation that Tom and I had: <Tartarus> frowand: So, dtc knows -@, but the kernel doesn't globally use -@ yet due to increased memory usage, in the dtb? <frowand> yes <frowand> It is available via the pattern that you can see in drivers/of/unittest-data/Makefile <Tartarus> Is there an opt-in way to get the extra symbols? <frowand> # enable creation of __symbols__ node <frowand> DTC_FLAGS_overlay := -@ <Tartarus> ah, hm <frowand> The problem is that it is always on, once the flags are added <frowand> there are a few ways I have thought of making it optional <frowand> 1) DTC_FLAGS_overlay := CONFIG_OVERLAY_ENABLED_IN_BASE <Tartarus> OK. There's at least a few vendors that are eagerly awaiting overlays Just Working, is why I'm asking <Tartarus> How much extra memory usage are we talking about, on a 'big' platform for example? <frowand> where CONFIG_OVERLAY_ENABLED_IN_BASE is a string of either empty or "-@". I don't know if the kconfig allows that, but I'm guessing there is some way to do it <frowand> Not sure of size penalty. <Tartarus> CONFIG_USE_xxx as a bool, CONFIG_xxx as the string <Tartarus> I would kind of assume wanting overlays to just work would be the common case, at least in terms of in-kernel users <frowand> 2) When instantiating the device tree from the FDT, do not keep the __SYMBOLS__ node if overlays are not enabled in the kernel <Tartarus> ie if it's a CONFIG opt-in, multi_v7_defconfig, etc, would want it on <frowand> the issue with 2, is that the boot image still has the size penalty <frowand> 3) the bootloader could choose whether to pass the __SYMBOLS__ node to the kernel or not <Tartarus> I'm not sure about 3 honestly <frowand> Yep, 3 is my least favorite. <Tartarus> If one has a case where there's a desire for no overlay support, for whatever reason, that's a feature of the kernel <frowand> There is probably also option 4, 5, etc. But I haven't thought it through too deeply yet. <Tartarus> I can see some sort of CONFIG option, default y, being a reasonable option here <frowand> I'm not sure about the common case. There are some boards where overlays would almost always be used, but there may also be boards where there are rarely used. <frowand> But the common case is a bikeshed issue to me. <Tartarus> heh <Tartarus> Well, here's why I was thinking that <frowand> As long as the users and distros have a way to control it. <Tartarus> Lots of proprietary boards I've dealt with could just be an EVM + overlay <Tartarus> wrt dtb <frowand> what is EVM? <Tartarus> Today it's take the evm dts, start hacking <Tartarus> evaluation platform <Tartarus> The second, similar, example would be custom platforms based on SoMs <frowand> Are you saying to use an overlay to add onto the base EVM dts kinda sorta the way that some driver writers use kernel modules to add drivers? <Tartarus> A reasonable analogy, yes <frowand> ok, just wanted to verify I understood the model <frowand> I see that as a reasonable development technique <frowand> I have some time critical errands to run, so I'll be gone for a few hours. It would be good to share what we just said with the mail list, and see if anyone else has some brilliant ideas. Start with what the objectives/issues are, some possible solutions... -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote: > On 08/15/17 14:15, Tom Rini wrote: > > With support for stacked overlays being part of libfdt it is now > > possible and likely that overlays which require __symbols__ will be > > applied to the dtb files generated by the kernel. This is done by > > passing -@ to dtc. This does increase the filesize (and resident memory > > usage) based on the number of __symbol__ entries added to match the > > contents of the dts. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Frank Rowand <frowand.list@gmail.com> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > Cc: Michal Marek <mmarek@suse.com> > > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > CC: linux-kbuild@vger.kernel.org > > Signed-off-by: Tom Rini <trini@konsulko.com> > > --- > > In order for a dtb file to be useful with all types of overlays, it > > needs to be generated with the -@ flag passed to dtc so that __symbols__ > > are generated. This however is not free, and increases the resulting > > dtb file by up to approximately 50% today. In the current worst case > > this is moving from 88KiB to 133KiB. In talking with Frank about this, > > he outlined 3 possible ways (with the 4th option of something else > > entirely). > > > > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > > 2. In the kernel, if the kernel does not have overlay support, discard > > the __symbols__ information that we've been passed. > > 3. Have the bootloader pass in, or not, __symbols__ information. > > I also was hoping that other people might have ideas for additional > approaches. Yes, please. > > This patch is an attempt to implement something between the 3rd option > > and a different, 4th option. Frank was thinking that we might introduce > > a new symbol to control generation of __symbol__ information for option > > 1. I think this gets the usage backwards and will lead to confusion > > among users and developers. > > > > My proposal is that we do not want __symbols__ existence to be dependent > > on some part of the kernel configuration for a number of reasons. > > First, this is out of step with the rest of how dtbs are created today > > and more importantly, thought about. Today, all dtb content is > > independent of CONFIG options. If you build a dtb from a given kernel > > tree, everyone will agree on the result. This is part of the "contract" > > on passing old kernels and new dtb files even. > > I hope that dtb contents are independent of CONFIG options, but I don't > feel confident is stating that there is not such dependency. (Of course, > whether to build a dtb can be dependent on a CONFIG option in the Makefile, > but that is not the same concept.) > > The only existing rule that I am aware of that helps avoid a dts dependency > on kernel CONFIG options is that included files can not be from general kernel > header files; they must be in include/dt-bindings/. I'm fairly certain for in-kernel stuff at least, the assumption is correct. > Should we add text to Documentation/devicetree/bindings/submitting-patches.txt > that explicitly states that dts files are not allowed to contain any > dependency on kernel CONFIG options? Certainly can't hurt. > > Second, I think this is out of step with how a lot of overlay usage will > > occur. My thinking is that with maximally useful overlays being > > available in mainline, lots of use-cases that we have today that result > > in a number of DTS files being included can become just overlays. This > > I disagree with this. My _opinion_ is that overlays should be the exception, > not the common case. Overlays require extra complexity in the various > subsystems that interact with device trees. For an overlay to work, these > subsystems must be able to react to changes made to the device tree by > an overlay. The current mechanism is via notifiers, which only exist > for a few subsystems. Ah. Now, I can't blame you for thinking with your kernel hat on, but, you're thinking with your kernel hat on :) (And taking mine off for a minute is why I changed my mind between when we talked on IRC, and what I posted). Kernel run-time apply an overlay has various use cases that I don't want to discount, but don't want to try and go in detail on either. At heart, one of the issues here is that the Linux kernel is the authoritative source of dts and dtb files. Assembling a dtb and N overlays at some point prior to booting Linux, in order to give it a complete and valid system is going to be a common case. Even setting aside all of the hobbyist board families out there, SoMs are a big thing. Eval modules are a big thing. This turns not just enabling these as a vendor but using them as a developer into a much less error prone system. To try and re-state my rational, if the Linux kernel needs some safe-guards or other mechanisms to restrict what can be done on top of OF_OVERLAY (which is not widely enabled in mainline), OK, that's certainly a discussion to have and think about implications of. But, the Linux kernel is the producer of most dtbs that will be consumed by a variety of platforms. I feel it needs to generate the dtb that can be used for all consumers, since it is the source of these resources.
On 08/15/17 17:42, Tom Rini wrote: > On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote: >> On 08/15/17 14:15, Tom Rini wrote: >>> With support for stacked overlays being part of libfdt it is now >>> possible and likely that overlays which require __symbols__ will be >>> applied to the dtb files generated by the kernel. This is done by >>> passing -@ to dtc. This does increase the filesize (and resident memory >>> usage) based on the number of __symbol__ entries added to match the >>> contents of the dts. >>> >>> Cc: Rob Herring <robh+dt@kernel.org> >>> Cc: Frank Rowand <frowand.list@gmail.com> >>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >>> Cc: Michal Marek <mmarek@suse.com> >>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >>> Cc: devicetree@vger.kernel.org >>> Cc: linux-kernel@vger.kernel.org >>> CC: linux-kbuild@vger.kernel.org >>> Signed-off-by: Tom Rini <trini@konsulko.com> >>> --- >>> In order for a dtb file to be useful with all types of overlays, it >>> needs to be generated with the -@ flag passed to dtc so that __symbols__ >>> are generated. This however is not free, and increases the resulting >>> dtb file by up to approximately 50% today. In the current worst case >>> this is moving from 88KiB to 133KiB. In talking with Frank about this, >>> he outlined 3 possible ways (with the 4th option of something else >>> entirely). >>> >>> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >>> 2. In the kernel, if the kernel does not have overlay support, discard >>> the __symbols__ information that we've been passed. >>> 3. Have the bootloader pass in, or not, __symbols__ information. >> >> I also was hoping that other people might have ideas for additional >> approaches. > > Yes, please. > >>> This patch is an attempt to implement something between the 3rd option >>> and a different, 4th option. Frank was thinking that we might introduce >>> a new symbol to control generation of __symbol__ information for option >>> 1. I think this gets the usage backwards and will lead to confusion >>> among users and developers. >>> >>> My proposal is that we do not want __symbols__ existence to be dependent >>> on some part of the kernel configuration for a number of reasons. >>> First, this is out of step with the rest of how dtbs are created today >>> and more importantly, thought about. Today, all dtb content is >>> independent of CONFIG options. If you build a dtb from a given kernel >>> tree, everyone will agree on the result. This is part of the "contract" >>> on passing old kernels and new dtb files even. >> >> I hope that dtb contents are independent of CONFIG options, but I don't >> feel confident is stating that there is not such dependency. (Of course, >> whether to build a dtb can be dependent on a CONFIG option in the Makefile, >> but that is not the same concept.) >> >> The only existing rule that I am aware of that helps avoid a dts dependency >> on kernel CONFIG options is that included files can not be from general kernel >> header files; they must be in include/dt-bindings/. > > I'm fairly certain for in-kernel stuff at least, the assumption is > correct. > >> Should we add text to Documentation/devicetree/bindings/submitting-patches.txt >> that explicitly states that dts files are not allowed to contain any >> dependency on kernel CONFIG options? > > Certainly can't hurt. > >>> Second, I think this is out of step with how a lot of overlay usage will >>> occur. My thinking is that with maximally useful overlays being >>> available in mainline, lots of use-cases that we have today that result >>> in a number of DTS files being included can become just overlays. This >> >> I disagree with this. My _opinion_ is that overlays should be the exception, >> not the common case. Overlays require extra complexity in the various >> subsystems that interact with device trees. For an overlay to work, these >> subsystems must be able to react to changes made to the device tree by >> an overlay. The current mechanism is via notifiers, which only exist >> for a few subsystems. > > Ah. Now, I can't blame you for thinking with your kernel hat on, but, > you're thinking with your kernel hat on :) (And taking mine off for a > minute is why I changed my mind between when we talked on IRC, and what > I posted). Kernel run-time apply an overlay has various use cases that > I don't want to discount, but don't want to try and go in detail on > either. > > At heart, one of the issues here is that the Linux kernel is the > authoritative source of dts and dtb files. Assembling a dtb and N > overlays at some point prior to booting Linux, in order to give it a > complete and valid system is going to be a common case. Even setting Yes. When discussing overlay issues and technologies we should be very clear about whether the context is post-boot run time loading of an overlay or using overlays applied on top of a base dtb to create a dtb to be fed to the kernel for booting. These are very different domains. For the case of constructing a boot time dtb from a base dtb and one or more overlays my primary concerns are related to the complexity (and again fragility) of the approach. It is already quite difficult to read source dts files and dts include files and determine what the final dtb will contain. This has been the motivation for several of my debugging tools. Adding overlays to the mix will increase the complexity of understanding the final product (dtb), where the individual items originated, and what component to modify to result in a change to the final product. It will add the further challenge that applying overlays in different orders may result in different final dtbs (depending on how strict the rules for applying an overlay are). I agree that this technology has some valuable upsides and use cases, but I also fear the negative impacts. The trick lies in finding a way to meet the technology needs in a manner that successfully balances the costs and the benefits. I mentioned my concerns about the robustness of overlays applied to a running Linux system in another reply. It seems to me that applying overlays pre-boot is more robust as far as the driver and subsystem initialization is concerned, because the normal boot path will be used to process the resulting dtb instead of using an overlay loading path in the kernel. > aside all of the hobbyist board families out there, SoMs are a big > thing. Eval modules are a big thing. This turns not just enabling > these as a vendor but using them as a developer into a much less error > prone system. If I understand the eval modules concept, then using overlays to develop and test device tree changes is somewhat analogous to using kernel modules to develop and test drivers. Yes, this may make some device tree source development more efficient. And in other cases may actually make it less efficient. I don't see the impact as being very large one direction or the other. > To try and re-state my rational, if the Linux kernel needs some > safe-guards or other mechanisms to restrict what can be done on top of > OF_OVERLAY (which is not widely enabled in mainline), OK, that's > certainly a discussion to have and think about implications of. But, > the Linux kernel is the producer of most dtbs that will be consumed by a > variety of platforms. I feel it needs to generate the dtb that can be > used for all consumers, since it is the source of these resources. This has gone far off topic from the original issue you brought up, which is how, in the context of the Linux kernel build environment, to enable building a base dtb that contains the symbols that are needed to load an overlay. Can we return to the base topic? And if anyone wants to discuss the other issues start a new thread? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tom, Sorry for taking a little bit of time to reply to this (vacation time). > On Aug 16, 2017, at 00:15 , Tom Rini <trini@konsulko.com> wrote: > > With support for stacked overlays being part of libfdt it is now > possible and likely that overlays which require __symbols__ will be > applied to the dtb files generated by the kernel. This is done by > passing -@ to dtc. This does increase the filesize (and resident memory > usage) based on the number of __symbol__ entries added to match the > contents of the dts. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Michal Marek <mmarek@suse.com> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > CC: linux-kbuild@vger.kernel.org > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > In order for a dtb file to be useful with all types of overlays, it > needs to be generated with the -@ flag passed to dtc so that __symbols__ > are generated. This however is not free, and increases the resulting > dtb file by up to approximately 50% today. In the current worst case > this is moving from 88KiB to 133KiB. In talking with Frank about this, > he outlined 3 possible ways (with the 4th option of something else > entirely). > > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > 2. In the kernel, if the kernel does not have overlay support, discard > the __symbols__ information that we've been passed. > 3. Have the bootloader pass in, or not, __symbols__ information. > > This patch is an attempt to implement something between the 3rd option > and a different, 4th option. Frank was thinking that we might introduce > a new symbol to control generation of __symbol__ information for option > 1. I think this gets the usage backwards and will lead to confusion > among users and developers. > > My proposal is that we do not want __symbols__ existence to be dependent > on some part of the kernel configuration for a number of reasons. > First, this is out of step with the rest of how dtbs are created today > and more importantly, thought about. Today, all dtb content is > independent of CONFIG options. If you build a dtb from a given kernel > tree, everyone will agree on the result. This is part of the "contract" > on passing old kernels and new dtb files even. > > Second, I think this is out of step with how a lot of overlay usage will > occur. My thinking is that with maximally useful overlays being > available in mainline, lots of use-cases that we have today that result > in a number of DTS files being included can become just overlays. This > is true in terms of not only evaluation kits but also when these systems > are turned into custom hardware. This is even more true for SoM based > systems where a physical widget would be a SoM + carrier overlay + > custom parts overlay. These cases are going to be resolved with > overlays being applied outside of the kernel. > FWIW here are some thoughts of mine on this subject. First, the whole business with the __symbols__ (& the fixup nodes) is meant to be used as a method to pass along symbol information, inband with the DTB in such a way as it would require absolutely no changes to booloaders and the kernel unflattening methods with the downside of the increased memory consumption. That said, there’s no reason to keep the __symbols__ node as part of the in kernel device tree structure after loading. In fact operations would be much easier if that would be the case. That would go hand in hand with the a previously posted patch that turns phandle lookups into hash/idr lookups. I would think that whether overlays would be supported could be a board level option, but I would hate for overlay support to be dependent on a kernel config option. Yes, this is contradictory, I know :(. The problem is that if you don’t have symbols generated at compile time of the kernel DTB you’re SOL loading an overlay later. I was thinking of a patch that would allow ‘patching in’ the symbols of an kernel at runtime, when that would be required (using a kernel module containing that symbol information). Finally Tom is absolutely correct; the way that system design for EVM+SoM is done leads naturally to thinking in ‘overlay’ terms. So instead of the all the different DTBs for different variants of boards we could have the .DTSIs compiled as overlays and then the final DTB reconstructed at boot time (whether by the bootloader or the kernel). Regards — Pantelis > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > drivers/of/unittest-data/Makefile | 5 ----- > scripts/Makefile.lib | 3 +++ > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile > index 6e00a9c69e58..70731cfe8900 100644 > --- a/drivers/of/unittest-data/Makefile > +++ b/drivers/of/unittest-data/Makefile > @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S > .PRECIOUS: \ > $(obj)/%.dtb.S \ > $(obj)/%.dtb > - > -# enable creation of __symbols__ node > -DTC_FLAGS_overlay := -@ > -DTC_FLAGS_overlay_bad_phandle := -@ > -DTC_FLAGS_overlay_base := -@ > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 58c05e5d9870..a1f4a6b29d75 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ > -Wproperty_name_chars_strict > endif > > +# enable creation of __symbols__ node > +DTC_FLAGS += -@ > + > DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) > > # Generate an assembly file to wrap the output of the device tree compiler > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Frank, > On Aug 16, 2017, at 02:57 , Frank Rowand <frowand.list@gmail.com> wrote: > > On 08/15/17 15:36, Rob Herring wrote: >> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >>> With support for stacked overlays being part of libfdt it is now >>> possible and likely that overlays which require __symbols__ will be >>> applied to the dtb files generated by the kernel. This is done by >>> passing -@ to dtc. This does increase the filesize (and resident memory >>> usage) based on the number of __symbol__ entries added to match the >>> contents of the dts. >>> >>> Cc: Rob Herring <robh+dt@kernel.org> >>> Cc: Frank Rowand <frowand.list@gmail.com> >>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >>> Cc: Michal Marek <mmarek@suse.com> >>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >>> Cc: devicetree@vger.kernel.org >>> Cc: linux-kernel@vger.kernel.org >>> CC: linux-kbuild@vger.kernel.org >>> Signed-off-by: Tom Rini <trini@konsulko.com> >>> --- >>> In order for a dtb file to be useful with all types of overlays, it >>> needs to be generated with the -@ flag passed to dtc so that __symbols__ >>> are generated. This however is not free, and increases the resulting >>> dtb file by up to approximately 50% today. In the current worst case >>> this is moving from 88KiB to 133KiB. In talking with Frank about this, >> >> Plus some amount for the unflattened tree in memory, too. >> >>> he outlined 3 possible ways (with the 4th option of something else >>> entirely). >>> >>> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >>> 2. In the kernel, if the kernel does not have overlay support, discard >>> the __symbols__ information that we've been passed. >>> 3. Have the bootloader pass in, or not, __symbols__ information. >>> >>> This patch is an attempt to implement something between the 3rd option >>> and a different, 4th option. Frank was thinking that we might introduce >>> a new symbol to control generation of __symbol__ information for option >>> 1. I think this gets the usage backwards and will lead to confusion >>> among users and developers. >>> >>> My proposal is that we do not want __symbols__ existence to be dependent >>> on some part of the kernel configuration for a number of reasons. >>> First, this is out of step with the rest of how dtbs are created today >>> and more importantly, thought about. Today, all dtb content is >>> independent of CONFIG options. If you build a dtb from a given kernel >>> tree, everyone will agree on the result. This is part of the "contract" >>> on passing old kernels and new dtb files even. >> >> Agree completely. I don't even like that building dtbs depends on the ARCH. >> >> However, option 2 may still be useful. There's no point exposing what >> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree >> at all may be a bad idea. We should consider if it should always be >> hidden. That would also allow storing the __symbols__ data however we >> want internally (i.e. with less memory usage). > > Yes. I would prefer to treat the __symbols__ node as an internal > representation of information used by the device tree subsystem. > It is not hardware description. > > This is correct. This is internal workaround against a serialization format omission. >> The complication is >> always kexec which I haven't thought about too much here. > > That should not be an issue, because the device tree is exposed to kexec > via /sys/firmware/fdt instead of /sys/firmware/devicetree/base (which > is what /proc/device-tree links to), according to > Documentation/ABI/testing/sysfs-firmware-ofw. So the __symbols__ > node will be exposed to kexec. > Which will have to be recreated if we throw away __symbols__ when converting to our internal representation of labels. > >> Also, perhaps we need finer grain control of __symbols__ generation. >> We really don't want userspace to be able to modify anything in the DT >> at any point in time. That's a big can of worms and we don't want to >> start there. The problem is labels are widely used just for >> convenience and weren't part of the ABI. With overlays that changes, >> so we either need to restrict labels usage or define another way. It >> could be as simple as defining some prefix for label names for labels >> to export. > > Agreed. We could also restrict labels in connector nodes to be visible. > I would disagree. This is only being considered because runtime device tree consistency checks currently is minimal (i.e. non existent). When we have a proper DT syntax and semantic checker (soon I suppose) this restriction will be useless and make things more complex. Regards — Pantelis > >>> Second, I think this is out of step with how a lot of overlay usage will >>> occur. My thinking is that with maximally useful overlays being >>> available in mainline, lots of use-cases that we have today that result >>> in a number of DTS files being included can become just overlays. This >>> is true in terms of not only evaluation kits but also when these systems >>> are turned into custom hardware. This is even more true for SoM based >>> systems where a physical widget would be a SoM + carrier overlay + >>> custom parts overlay. These cases are going to be resolved with >>> overlays being applied outside of the kernel. >>> >>> Signed-off-by: Tom Rini <trini@konsulko.com> >>> --- >>> drivers/of/unittest-data/Makefile | 5 ----- >>> scripts/Makefile.lib | 3 +++ >>> 2 files changed, 3 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile >>> index 6e00a9c69e58..70731cfe8900 100644 >>> --- a/drivers/of/unittest-data/Makefile >>> +++ b/drivers/of/unittest-data/Makefile >>> @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S >>> .PRECIOUS: \ >>> $(obj)/%.dtb.S \ >>> $(obj)/%.dtb >>> - >>> -# enable creation of __symbols__ node >>> -DTC_FLAGS_overlay := -@ >>> -DTC_FLAGS_overlay_bad_phandle := -@ >>> -DTC_FLAGS_overlay_base := -@ >>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >>> index 58c05e5d9870..a1f4a6b29d75 100644 >>> --- a/scripts/Makefile.lib >>> +++ b/scripts/Makefile.lib >>> @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ >>> -Wproperty_name_chars_strict >>> endif >>> >>> +# enable creation of __symbols__ node >>> +DTC_FLAGS += -@ >>> + >>> DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) >>> >>> # Generate an assembly file to wrap the output of the device tree compiler >>> -- >>> 1.9.1 >>> >> . -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 15, 2017 at 08:22:01PM -0700, Frank Rowand wrote: > On 08/15/17 17:42, Tom Rini wrote: > > On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote: > >> On 08/15/17 14:15, Tom Rini wrote: > >>> With support for stacked overlays being part of libfdt it is now > >>> possible and likely that overlays which require __symbols__ will be > >>> applied to the dtb files generated by the kernel. This is done by > >>> passing -@ to dtc. This does increase the filesize (and resident memory > >>> usage) based on the number of __symbol__ entries added to match the > >>> contents of the dts. > >>> > >>> Cc: Rob Herring <robh+dt@kernel.org> > >>> Cc: Frank Rowand <frowand.list@gmail.com> > >>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > >>> Cc: Michal Marek <mmarek@suse.com> > >>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > >>> Cc: devicetree@vger.kernel.org > >>> Cc: linux-kernel@vger.kernel.org > >>> CC: linux-kbuild@vger.kernel.org > >>> Signed-off-by: Tom Rini <trini@konsulko.com> > >>> --- > >>> In order for a dtb file to be useful with all types of overlays, it > >>> needs to be generated with the -@ flag passed to dtc so that __symbols__ > >>> are generated. This however is not free, and increases the resulting > >>> dtb file by up to approximately 50% today. In the current worst case > >>> this is moving from 88KiB to 133KiB. In talking with Frank about this, > >>> he outlined 3 possible ways (with the 4th option of something else > >>> entirely). > >>> > >>> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > >>> 2. In the kernel, if the kernel does not have overlay support, discard > >>> the __symbols__ information that we've been passed. > >>> 3. Have the bootloader pass in, or not, __symbols__ information. > >> > >> I also was hoping that other people might have ideas for additional > >> approaches. > > > > Yes, please. > > > >>> This patch is an attempt to implement something between the 3rd option > >>> and a different, 4th option. Frank was thinking that we might introduce > >>> a new symbol to control generation of __symbol__ information for option > >>> 1. I think this gets the usage backwards and will lead to confusion > >>> among users and developers. > >>> > >>> My proposal is that we do not want __symbols__ existence to be dependent > >>> on some part of the kernel configuration for a number of reasons. > >>> First, this is out of step with the rest of how dtbs are created today > >>> and more importantly, thought about. Today, all dtb content is > >>> independent of CONFIG options. If you build a dtb from a given kernel > >>> tree, everyone will agree on the result. This is part of the "contract" > >>> on passing old kernels and new dtb files even. > >> > >> I hope that dtb contents are independent of CONFIG options, but I don't > >> feel confident is stating that there is not such dependency. (Of course, > >> whether to build a dtb can be dependent on a CONFIG option in the Makefile, > >> but that is not the same concept.) > >> > >> The only existing rule that I am aware of that helps avoid a dts dependency > >> on kernel CONFIG options is that included files can not be from general kernel > >> header files; they must be in include/dt-bindings/. > > > > I'm fairly certain for in-kernel stuff at least, the assumption is > > correct. > > > >> Should we add text to Documentation/devicetree/bindings/submitting-patches.txt > >> that explicitly states that dts files are not allowed to contain any > >> dependency on kernel CONFIG options? > > > > Certainly can't hurt. > > > >>> Second, I think this is out of step with how a lot of overlay usage will > >>> occur. My thinking is that with maximally useful overlays being > >>> available in mainline, lots of use-cases that we have today that result > >>> in a number of DTS files being included can become just overlays. This > >> > >> I disagree with this. My _opinion_ is that overlays should be the exception, > >> not the common case. Overlays require extra complexity in the various > >> subsystems that interact with device trees. For an overlay to work, these > >> subsystems must be able to react to changes made to the device tree by > >> an overlay. The current mechanism is via notifiers, which only exist > >> for a few subsystems. > > > > Ah. Now, I can't blame you for thinking with your kernel hat on, but, > > you're thinking with your kernel hat on :) (And taking mine off for a > > minute is why I changed my mind between when we talked on IRC, and what > > I posted). Kernel run-time apply an overlay has various use cases that > > I don't want to discount, but don't want to try and go in detail on > > either. > > > > At heart, one of the issues here is that the Linux kernel is the > > authoritative source of dts and dtb files. Assembling a dtb and N > > overlays at some point prior to booting Linux, in order to give it a > > complete and valid system is going to be a common case. Even setting > > Yes. When discussing overlay issues and technologies we should be very > clear about whether the context is post-boot run time loading of an > overlay or using overlays applied on top of a base dtb to create a > dtb to be fed to the kernel for booting. These are very different > domains. > > For the case of constructing a boot time dtb from a base dtb and one > or more overlays my primary concerns are related to the complexity > (and again fragility) of the approach. It is already quite difficult > to read source dts files and dts include files and determine what the > final dtb will contain. This has been the motivation for several of > my debugging tools. Adding overlays to the mix will increase the > complexity of understanding the final product (dtb), where the > individual items originated, and what component to modify to result > in a change to the final product. It will add the further challenge > that applying overlays in different orders may result in different > final dtbs (depending on how strict the rules for applying an overlay > are). I agree that this technology has some valuable upsides and > use cases, but I also fear the negative impacts. The trick lies > in finding a way to meet the technology needs in a manner that > successfully balances the costs and the benefits. > > I mentioned my concerns about the robustness of overlays applied to > a running Linux system in another reply. It seems to me that > applying overlays pre-boot is more robust as far as the driver and > subsystem initialization is concerned, because the normal boot > path will be used to process the resulting dtb instead of using > an overlay loading path in the kernel. > > > > aside all of the hobbyist board families out there, SoMs are a big > > thing. Eval modules are a big thing. This turns not just enabling > > these as a vendor but using them as a developer into a much less error > > prone system. > > If I understand the eval modules concept, then using overlays to > develop and test device tree changes is somewhat analogous to using > kernel modules to develop and test drivers. Yes, this may make > some device tree source development more efficient. And in other > cases may actually make it less efficient. I don't see the impact > as being very large one direction or the other. > > > > To try and re-state my rational, if the Linux kernel needs some > > safe-guards or other mechanisms to restrict what can be done on top of > > OF_OVERLAY (which is not widely enabled in mainline), OK, that's > > certainly a discussion to have and think about implications of. But, > > the Linux kernel is the producer of most dtbs that will be consumed by a > > variety of platforms. I feel it needs to generate the dtb that can be > > used for all consumers, since it is the source of these resources. > > This has gone far off topic from the original issue you brought up, > which is how, in the context of the Linux kernel build environment, to > enable building a base dtb that contains the symbols that are needed to > load an overlay. Can we return to the base topic? And if anyone wants > to discuss the other issues start a new thread? Well, how about this? There certainly should be some discussion about how in the context of the Linux kernel run-time, the various impacts of overlays need to be handled. But for out of kernel pre-boot cases, I believe __symbols__ always is the only answer that won't bring everyone more pain down the line. What needs to happen before we can get to that point, since the Linux kernel is the gate-keeper to how dtbs are built?
On Tue, Aug 15, 2017 at 7:42 PM, Tom Rini <trini@konsulko.com> wrote: > On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote: >> On 08/15/17 14:15, Tom Rini wrote: >> > With support for stacked overlays being part of libfdt it is now >> > possible and likely that overlays which require __symbols__ will be >> > applied to the dtb files generated by the kernel. This is done by >> > passing -@ to dtc. This does increase the filesize (and resident memory >> > usage) based on the number of __symbol__ entries added to match the >> > contents of the dts. >> > >> > Cc: Rob Herring <robh+dt@kernel.org> >> > Cc: Frank Rowand <frowand.list@gmail.com> >> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >> > Cc: Michal Marek <mmarek@suse.com> >> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >> > Cc: devicetree@vger.kernel.org >> > Cc: linux-kernel@vger.kernel.org >> > CC: linux-kbuild@vger.kernel.org >> > Signed-off-by: Tom Rini <trini@konsulko.com> >> > --- >> > In order for a dtb file to be useful with all types of overlays, it >> > needs to be generated with the -@ flag passed to dtc so that __symbols__ >> > are generated. This however is not free, and increases the resulting >> > dtb file by up to approximately 50% today. In the current worst case >> > this is moving from 88KiB to 133KiB. In talking with Frank about this, >> > he outlined 3 possible ways (with the 4th option of something else >> > entirely). >> > >> > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >> > 2. In the kernel, if the kernel does not have overlay support, discard >> > the __symbols__ information that we've been passed. >> > 3. Have the bootloader pass in, or not, __symbols__ information. >> >> I also was hoping that other people might have ideas for additional >> approaches. > > Yes, please. A couple of other options come to mind: "make DTC_FLAGS='-@' dtbs" should already work. So there's already a way to build what you want and the kernel is not setting the policy. Do like we do for the unittests and make it a per board decision: DTC_FLAGS_my-som-board.dtb := -@ Then boards that actually need it like SoMs can turn it on. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 16, 2017 at 10:22:03AM -0500, Rob Herring wrote: > On Tue, Aug 15, 2017 at 7:42 PM, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote: > >> On 08/15/17 14:15, Tom Rini wrote: > >> > With support for stacked overlays being part of libfdt it is now > >> > possible and likely that overlays which require __symbols__ will be > >> > applied to the dtb files generated by the kernel. This is done by > >> > passing -@ to dtc. This does increase the filesize (and resident memory > >> > usage) based on the number of __symbol__ entries added to match the > >> > contents of the dts. > >> > > >> > Cc: Rob Herring <robh+dt@kernel.org> > >> > Cc: Frank Rowand <frowand.list@gmail.com> > >> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > >> > Cc: Michal Marek <mmarek@suse.com> > >> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > >> > Cc: devicetree@vger.kernel.org > >> > Cc: linux-kernel@vger.kernel.org > >> > CC: linux-kbuild@vger.kernel.org > >> > Signed-off-by: Tom Rini <trini@konsulko.com> > >> > --- > >> > In order for a dtb file to be useful with all types of overlays, it > >> > needs to be generated with the -@ flag passed to dtc so that __symbols__ > >> > are generated. This however is not free, and increases the resulting > >> > dtb file by up to approximately 50% today. In the current worst case > >> > this is moving from 88KiB to 133KiB. In talking with Frank about this, > >> > he outlined 3 possible ways (with the 4th option of something else > >> > entirely). > >> > > >> > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > >> > 2. In the kernel, if the kernel does not have overlay support, discard > >> > the __symbols__ information that we've been passed. > >> > 3. Have the bootloader pass in, or not, __symbols__ information. > >> > >> I also was hoping that other people might have ideas for additional > >> approaches. > > > > Yes, please. > > A couple of other options come to mind: > > "make DTC_FLAGS='-@' dtbs" should already work. So there's already a > way to build what you want and the kernel is not setting the policy. Not ideal since that drops out the -Wno... flags we pass in. I don't see off-hand why it's not appending to DTC_FLAGS, but that's a fixable problem. > Do like we do for the unittests and make it a per board decision: > > DTC_FLAGS_my-som-board.dtb := -@ > > Then boards that actually need it like SoMs can turn it on. A concern about that of mine is that we'll start to see a 'flood' of patches growing that list at the end of arch/arm/boot/dts/Makefile.
On Tue, Aug 15, 2017 at 5:49 PM, Tom Rini <trini@konsulko.com> wrote: > On Tue, Aug 15, 2017 at 05:36:11PM -0500, Rob Herring wrote: >> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >> > With support for stacked overlays being part of libfdt it is now >> > possible and likely that overlays which require __symbols__ will be >> > applied to the dtb files generated by the kernel. This is done by >> > passing -@ to dtc. This does increase the filesize (and resident memory >> > usage) based on the number of __symbol__ entries added to match the >> > contents of the dts. >> > >> > Cc: Rob Herring <robh+dt@kernel.org> >> > Cc: Frank Rowand <frowand.list@gmail.com> >> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >> > Cc: Michal Marek <mmarek@suse.com> >> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >> > Cc: devicetree@vger.kernel.org >> > Cc: linux-kernel@vger.kernel.org >> > CC: linux-kbuild@vger.kernel.org >> > Signed-off-by: Tom Rini <trini@konsulko.com> >> > --- >> > In order for a dtb file to be useful with all types of overlays, it >> > needs to be generated with the -@ flag passed to dtc so that __symbols__ >> > are generated. This however is not free, and increases the resulting >> > dtb file by up to approximately 50% today. In the current worst case >> > this is moving from 88KiB to 133KiB. In talking with Frank about this, >> >> Plus some amount for the unflattened tree in memory, too. >> >> > he outlined 3 possible ways (with the 4th option of something else >> > entirely). >> > >> > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >> > 2. In the kernel, if the kernel does not have overlay support, discard >> > the __symbols__ information that we've been passed. >> > 3. Have the bootloader pass in, or not, __symbols__ information. >> > >> > This patch is an attempt to implement something between the 3rd option >> > and a different, 4th option. Frank was thinking that we might introduce >> > a new symbol to control generation of __symbol__ information for option >> > 1. I think this gets the usage backwards and will lead to confusion >> > among users and developers. >> > >> > My proposal is that we do not want __symbols__ existence to be dependent >> > on some part of the kernel configuration for a number of reasons. >> > First, this is out of step with the rest of how dtbs are created today >> > and more importantly, thought about. Today, all dtb content is >> > independent of CONFIG options. If you build a dtb from a given kernel >> > tree, everyone will agree on the result. This is part of the "contract" >> > on passing old kernels and new dtb files even. >> >> Agree completely. I don't even like that building dtbs depends on the ARCH. >> >> However, option 2 may still be useful. There's no point exposing what >> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree >> at all may be a bad idea. We should consider if it should always be >> hidden. That would also allow storing the __symbols__ data however we >> want internally (i.e. with less memory usage). The complication is >> always kexec which I haven't thought about too much here. > > A further patch to the kernel at run-time, OK. If you give me some > crumbs I'll see if I can figure out the next steps. > >> Also, perhaps we need finer grain control of __symbols__ generation. > > Here I have to disagree. > >> We really don't want userspace to be able to modify anything in the DT >> at any point in time. That's a big can of worms and we don't want to >> start there. The problem is labels are widely used just for >> convenience and weren't part of the ABI. With overlays that changes, >> so we either need to restrict labels usage or define another way. It >> could be as simple as defining some prefix for label names for labels >> to export. > > I think there needs to be a difference noted between "here is what > policy the kernel is going to enforce about run time changes" and "here > is what the user is going to assemble a system to look like". Again, > stemming from the part where the Linux kernel is where dts files reside > and are generated from normally. If we have it in __symbols__, someone > can make use of it in hardware design (again, think of the SoM + carrier > + custom) bit, I've seen so many real life products now that would be > simplified in this manner). I agree the usecase is an important one and one we should target, but I think there are other issues to solve first before we get to the trivial change needing to enable __symbols__. Do we have any dts files actually structured for the SoM + carrier use case? I guess it's done with includes ATM if we do. The run-time restrictions aren't just kernel policy. The SoM itself is going to have restrictions defined by its pinout. I think those need to be described in DT via a connector binding. I worry about leaving things wide open and having overlays just be a DT configuration tool with every platform structuring things however they want. From what I've looked at on RPi, I'm very concerned about having things like CMA overlays to set the CMA size. (On the flip side as a user, it was very nice to just apply the RPi 1-wire gpio overlay and things just worked.) Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 16, 2017 at 10:43:16AM -0500, Rob Herring wrote: > On Tue, Aug 15, 2017 at 5:49 PM, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Aug 15, 2017 at 05:36:11PM -0500, Rob Herring wrote: > >> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: > >> > With support for stacked overlays being part of libfdt it is now > >> > possible and likely that overlays which require __symbols__ will be > >> > applied to the dtb files generated by the kernel. This is done by > >> > passing -@ to dtc. This does increase the filesize (and resident memory > >> > usage) based on the number of __symbol__ entries added to match the > >> > contents of the dts. > >> > > >> > Cc: Rob Herring <robh+dt@kernel.org> > >> > Cc: Frank Rowand <frowand.list@gmail.com> > >> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > >> > Cc: Michal Marek <mmarek@suse.com> > >> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > >> > Cc: devicetree@vger.kernel.org > >> > Cc: linux-kernel@vger.kernel.org > >> > CC: linux-kbuild@vger.kernel.org > >> > Signed-off-by: Tom Rini <trini@konsulko.com> > >> > --- > >> > In order for a dtb file to be useful with all types of overlays, it > >> > needs to be generated with the -@ flag passed to dtc so that __symbols__ > >> > are generated. This however is not free, and increases the resulting > >> > dtb file by up to approximately 50% today. In the current worst case > >> > this is moving from 88KiB to 133KiB. In talking with Frank about this, > >> > >> Plus some amount for the unflattened tree in memory, too. > >> > >> > he outlined 3 possible ways (with the 4th option of something else > >> > entirely). > >> > > >> > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. > >> > 2. In the kernel, if the kernel does not have overlay support, discard > >> > the __symbols__ information that we've been passed. > >> > 3. Have the bootloader pass in, or not, __symbols__ information. > >> > > >> > This patch is an attempt to implement something between the 3rd option > >> > and a different, 4th option. Frank was thinking that we might introduce > >> > a new symbol to control generation of __symbol__ information for option > >> > 1. I think this gets the usage backwards and will lead to confusion > >> > among users and developers. > >> > > >> > My proposal is that we do not want __symbols__ existence to be dependent > >> > on some part of the kernel configuration for a number of reasons. > >> > First, this is out of step with the rest of how dtbs are created today > >> > and more importantly, thought about. Today, all dtb content is > >> > independent of CONFIG options. If you build a dtb from a given kernel > >> > tree, everyone will agree on the result. This is part of the "contract" > >> > on passing old kernels and new dtb files even. > >> > >> Agree completely. I don't even like that building dtbs depends on the ARCH. > >> > >> However, option 2 may still be useful. There's no point exposing what > >> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree > >> at all may be a bad idea. We should consider if it should always be > >> hidden. That would also allow storing the __symbols__ data however we > >> want internally (i.e. with less memory usage). The complication is > >> always kexec which I haven't thought about too much here. > > > > A further patch to the kernel at run-time, OK. If you give me some > > crumbs I'll see if I can figure out the next steps. > > > >> Also, perhaps we need finer grain control of __symbols__ generation. > > > > Here I have to disagree. > > > >> We really don't want userspace to be able to modify anything in the DT > >> at any point in time. That's a big can of worms and we don't want to > >> start there. The problem is labels are widely used just for > >> convenience and weren't part of the ABI. With overlays that changes, > >> so we either need to restrict labels usage or define another way. It > >> could be as simple as defining some prefix for label names for labels > >> to export. > > > > I think there needs to be a difference noted between "here is what > > policy the kernel is going to enforce about run time changes" and "here > > is what the user is going to assemble a system to look like". Again, > > stemming from the part where the Linux kernel is where dts files reside > > and are generated from normally. If we have it in __symbols__, someone > > can make use of it in hardware design (again, think of the SoM + carrier > > + custom) bit, I've seen so many real life products now that would be > > simplified in this manner). > > I agree the usecase is an important one and one we should target, but > I think there are other issues to solve first before we get to the > trivial change needing to enable __symbols__. Do we have any dts files > actually structured for the SoM + carrier use case? I guess it's done > with includes ATM if we do. The run-time restrictions aren't just > kernel policy. The SoM itself is going to have restrictions defined by > its pinout. I think those need to be described in DT via a connector > binding. I worry about leaving things wide open and having overlays > just be a DT configuration tool with every platform structuring things > however they want. From what I've looked at on RPi, I'm very concerned > about having things like CMA overlays to set the CMA size. (On the > flip side as a user, it was very nice to just apply the RPi 1-wire > gpio overlay and things just worked.) I believe the various SoM and EVM and hobbyist cases are all either out of tree, or glued together (see imx6sx-udoo-neo-* in-tree, RPi or Hummingboard or TI DRA7 EVM + LCDs) as various groups decided it wouldn't be accepted to push in N "complete" DTS files for each valid combination). Moving forward with an in-kernel policy on how it should be done, structure-wise would help with consistency and defining what's really acceptable.
On Wed, Aug 16, 2017 at 10:41 AM, Tom Rini <trini@konsulko.com> wrote: > On Wed, Aug 16, 2017 at 10:22:03AM -0500, Rob Herring wrote: >> On Tue, Aug 15, 2017 at 7:42 PM, Tom Rini <trini@konsulko.com> wrote: >> > On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote: >> >> On 08/15/17 14:15, Tom Rini wrote: >> >> > With support for stacked overlays being part of libfdt it is now >> >> > possible and likely that overlays which require __symbols__ will be >> >> > applied to the dtb files generated by the kernel. This is done by >> >> > passing -@ to dtc. This does increase the filesize (and resident memory >> >> > usage) based on the number of __symbol__ entries added to match the >> >> > contents of the dts. >> >> > >> >> > Cc: Rob Herring <robh+dt@kernel.org> >> >> > Cc: Frank Rowand <frowand.list@gmail.com> >> >> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >> >> > Cc: Michal Marek <mmarek@suse.com> >> >> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >> >> > Cc: devicetree@vger.kernel.org >> >> > Cc: linux-kernel@vger.kernel.org >> >> > CC: linux-kbuild@vger.kernel.org >> >> > Signed-off-by: Tom Rini <trini@konsulko.com> >> >> > --- >> >> > In order for a dtb file to be useful with all types of overlays, it >> >> > needs to be generated with the -@ flag passed to dtc so that __symbols__ >> >> > are generated. This however is not free, and increases the resulting >> >> > dtb file by up to approximately 50% today. In the current worst case >> >> > this is moving from 88KiB to 133KiB. In talking with Frank about this, >> >> > he outlined 3 possible ways (with the 4th option of something else >> >> > entirely). >> >> > >> >> > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >> >> > 2. In the kernel, if the kernel does not have overlay support, discard >> >> > the __symbols__ information that we've been passed. >> >> > 3. Have the bootloader pass in, or not, __symbols__ information. >> >> >> >> I also was hoping that other people might have ideas for additional >> >> approaches. >> > >> > Yes, please. >> >> A couple of other options come to mind: >> >> "make DTC_FLAGS='-@' dtbs" should already work. So there's already a >> way to build what you want and the kernel is not setting the policy. > > Not ideal since that drops out the -Wno... flags we pass in. I don't > see off-hand why it's not appending to DTC_FLAGS, but that's a fixable > problem. I think that's a "feature" of make. Either it needs a different name or maybe it needs "DTC_FLAGS ?= ..." to pick up later "+=" which some of the arches do. It's been on my todo for a long time to kill off the per arch DTC_FLAGS BTW. > >> Do like we do for the unittests and make it a per board decision: >> >> DTC_FLAGS_my-som-board.dtb := -@ >> >> Then boards that actually need it like SoMs can turn it on. > > A concern about that of mine is that we'll start to see a 'flood' of > patches growing that list at the end of arch/arm/boot/dts/Makefile. Maybe that will motivate arm-soc to restructure things by vendor. If there's really a flood then maybe on should be the default and we turn it off per board. I'm somewhat skeptical that we'll see a flood though. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 16, 2017 at 10:57 AM, Tom Rini <trini@konsulko.com> wrote: > On Wed, Aug 16, 2017 at 10:43:16AM -0500, Rob Herring wrote: >> On Tue, Aug 15, 2017 at 5:49 PM, Tom Rini <trini@konsulko.com> wrote: >> > On Tue, Aug 15, 2017 at 05:36:11PM -0500, Rob Herring wrote: >> >> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >> >> > With support for stacked overlays being part of libfdt it is now >> >> > possible and likely that overlays which require __symbols__ will be >> >> > applied to the dtb files generated by the kernel. This is done by >> >> > passing -@ to dtc. This does increase the filesize (and resident memory >> >> > usage) based on the number of __symbol__ entries added to match the >> >> > contents of the dts. >> >> > >> >> > Cc: Rob Herring <robh+dt@kernel.org> >> >> > Cc: Frank Rowand <frowand.list@gmail.com> >> >> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >> >> > Cc: Michal Marek <mmarek@suse.com> >> >> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >> >> > Cc: devicetree@vger.kernel.org >> >> > Cc: linux-kernel@vger.kernel.org >> >> > CC: linux-kbuild@vger.kernel.org >> >> > Signed-off-by: Tom Rini <trini@konsulko.com> >> >> > --- >> >> > In order for a dtb file to be useful with all types of overlays, it >> >> > needs to be generated with the -@ flag passed to dtc so that __symbols__ >> >> > are generated. This however is not free, and increases the resulting >> >> > dtb file by up to approximately 50% today. In the current worst case >> >> > this is moving from 88KiB to 133KiB. In talking with Frank about this, >> >> >> >> Plus some amount for the unflattened tree in memory, too. >> >> >> >> > he outlined 3 possible ways (with the 4th option of something else >> >> > entirely). >> >> > >> >> > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >> >> > 2. In the kernel, if the kernel does not have overlay support, discard >> >> > the __symbols__ information that we've been passed. >> >> > 3. Have the bootloader pass in, or not, __symbols__ information. >> >> > >> >> > This patch is an attempt to implement something between the 3rd option >> >> > and a different, 4th option. Frank was thinking that we might introduce >> >> > a new symbol to control generation of __symbol__ information for option >> >> > 1. I think this gets the usage backwards and will lead to confusion >> >> > among users and developers. >> >> > >> >> > My proposal is that we do not want __symbols__ existence to be dependent >> >> > on some part of the kernel configuration for a number of reasons. >> >> > First, this is out of step with the rest of how dtbs are created today >> >> > and more importantly, thought about. Today, all dtb content is >> >> > independent of CONFIG options. If you build a dtb from a given kernel >> >> > tree, everyone will agree on the result. This is part of the "contract" >> >> > on passing old kernels and new dtb files even. >> >> >> >> Agree completely. I don't even like that building dtbs depends on the ARCH. >> >> >> >> However, option 2 may still be useful. There's no point exposing what >> >> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree >> >> at all may be a bad idea. We should consider if it should always be >> >> hidden. That would also allow storing the __symbols__ data however we >> >> want internally (i.e. with less memory usage). The complication is >> >> always kexec which I haven't thought about too much here. >> > >> > A further patch to the kernel at run-time, OK. If you give me some >> > crumbs I'll see if I can figure out the next steps. >> > >> >> Also, perhaps we need finer grain control of __symbols__ generation. >> > >> > Here I have to disagree. >> > >> >> We really don't want userspace to be able to modify anything in the DT >> >> at any point in time. That's a big can of worms and we don't want to >> >> start there. The problem is labels are widely used just for >> >> convenience and weren't part of the ABI. With overlays that changes, >> >> so we either need to restrict labels usage or define another way. It >> >> could be as simple as defining some prefix for label names for labels >> >> to export. >> > >> > I think there needs to be a difference noted between "here is what >> > policy the kernel is going to enforce about run time changes" and "here >> > is what the user is going to assemble a system to look like". Again, >> > stemming from the part where the Linux kernel is where dts files reside >> > and are generated from normally. If we have it in __symbols__, someone >> > can make use of it in hardware design (again, think of the SoM + carrier >> > + custom) bit, I've seen so many real life products now that would be >> > simplified in this manner). >> >> I agree the usecase is an important one and one we should target, but >> I think there are other issues to solve first before we get to the >> trivial change needing to enable __symbols__. Do we have any dts files >> actually structured for the SoM + carrier use case? I guess it's done >> with includes ATM if we do. The run-time restrictions aren't just >> kernel policy. The SoM itself is going to have restrictions defined by >> its pinout. I think those need to be described in DT via a connector >> binding. I worry about leaving things wide open and having overlays >> just be a DT configuration tool with every platform structuring things >> however they want. From what I've looked at on RPi, I'm very concerned >> about having things like CMA overlays to set the CMA size. (On the >> flip side as a user, it was very nice to just apply the RPi 1-wire >> gpio overlay and things just worked.) > > I believe the various SoM and EVM and hobbyist cases are all either out > of tree, or glued together (see imx6sx-udoo-neo-* in-tree, RPi or > Hummingboard or TI DRA7 EVM + LCDs) as various groups decided it > wouldn't be accepted to push in N "complete" DTS files for each valid > combination). Moving forward with an in-kernel policy on how it should > be done, structure-wise would help with consistency and defining what's > really acceptable. IMO, that starts with a connector binding. Stephen Boyd has been working towards that[1]. I guess "gpio nexus" is so obscure that all the masses clamoring for overlays and modular board support haven't noticed. Rob [1] https://www.spinics.net/lists/arm-kernel/msg600125.html -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/16/17 02:42, Pantelis Antoniou wrote: > Hi Frank, > >> On Aug 16, 2017, at 02:57 , Frank Rowand <frowand.list@gmail.com> wrote: >> >> On 08/15/17 15:36, Rob Herring wrote: >>> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >>>> With support for stacked overlays being part of libfdt it is now >>>> possible and likely that overlays which require __symbols__ will be >>>> applied to the dtb files generated by the kernel. This is done by >>>> passing -@ to dtc. This does increase the filesize (and resident memory >>>> usage) based on the number of __symbol__ entries added to match the >>>> contents of the dts. >>>> >>>> Cc: Rob Herring <robh+dt@kernel.org> >>>> Cc: Frank Rowand <frowand.list@gmail.com> >>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >>>> Cc: Michal Marek <mmarek@suse.com> >>>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >>>> Cc: devicetree@vger.kernel.org >>>> Cc: linux-kernel@vger.kernel.org >>>> CC: linux-kbuild@vger.kernel.org >>>> Signed-off-by: Tom Rini <trini@konsulko.com> >>>> --- >>>> In order for a dtb file to be useful with all types of overlays, it >>>> needs to be generated with the -@ flag passed to dtc so that __symbols__ >>>> are generated. This however is not free, and increases the resulting >>>> dtb file by up to approximately 50% today. In the current worst case >>>> this is moving from 88KiB to 133KiB. In talking with Frank about this, >>> >>> Plus some amount for the unflattened tree in memory, too. >>> >>>> he outlined 3 possible ways (with the 4th option of something else >>>> entirely). >>>> >>>> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >>>> 2. In the kernel, if the kernel does not have overlay support, discard >>>> the __symbols__ information that we've been passed. >>>> 3. Have the bootloader pass in, or not, __symbols__ information. >>>> >>>> This patch is an attempt to implement something between the 3rd option >>>> and a different, 4th option. Frank was thinking that we might introduce >>>> a new symbol to control generation of __symbol__ information for option >>>> 1. I think this gets the usage backwards and will lead to confusion >>>> among users and developers. >>>> >>>> My proposal is that we do not want __symbols__ existence to be dependent >>>> on some part of the kernel configuration for a number of reasons. >>>> First, this is out of step with the rest of how dtbs are created today >>>> and more importantly, thought about. Today, all dtb content is >>>> independent of CONFIG options. If you build a dtb from a given kernel >>>> tree, everyone will agree on the result. This is part of the "contract" >>>> on passing old kernels and new dtb files even. >>> >>> Agree completely. I don't even like that building dtbs depends on the ARCH. >>> >>> However, option 2 may still be useful. There's no point exposing what >>> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree >>> at all may be a bad idea. We should consider if it should always be >>> hidden. That would also allow storing the __symbols__ data however we >>> want internally (i.e. with less memory usage). >> >> Yes. I would prefer to treat the __symbols__ node as an internal >> representation of information used by the device tree subsystem. >> It is not hardware description. >> >> > > This is correct. This is internal workaround against a serialization format > omission. > >>> The complication is >>> always kexec which I haven't thought about too much here. >> >> That should not be an issue, because the device tree is exposed to kexec >> via /sys/firmware/fdt instead of /sys/firmware/devicetree/base (which >> is what /proc/device-tree links to), according to >> Documentation/ABI/testing/sysfs-firmware-ofw. So the __symbols__ >> node will be exposed to kexec. >> > > Which will have to be recreated if we throw away __symbols__ when converting > to our internal representation of labels. Nope. /sys/firmware/fdt is the fdt that is passed to the kernel. We are not proposing any changes to that fdt by the kernel. >> >>> Also, perhaps we need finer grain control of __symbols__ generation. >>> We really don't want userspace to be able to modify anything in the DT >>> at any point in time. That's a big can of worms and we don't want to >>> start there. The problem is labels are widely used just for >>> convenience and weren't part of the ABI. With overlays that changes, >>> so we either need to restrict labels usage or define another way. It >>> could be as simple as defining some prefix for label names for labels >>> to export. >> >> Agreed. We could also restrict labels in connector nodes to be visible. >> > > I would disagree. This is only being considered because runtime device tree > consistency checks currently is minimal (i.e. non existent). When we have > a proper DT syntax and semantic checker (soon I suppose) this restriction > will be useless and make things more complex. > > Regards > > — Pantelis > >> >>>> Second, I think this is out of step with how a lot of overlay usage will >>>> occur. My thinking is that with maximally useful overlays being >>>> available in mainline, lots of use-cases that we have today that result >>>> in a number of DTS files being included can become just overlays. This >>>> is true in terms of not only evaluation kits but also when these systems >>>> are turned into custom hardware. This is even more true for SoM based >>>> systems where a physical widget would be a SoM + carrier overlay + >>>> custom parts overlay. These cases are going to be resolved with >>>> overlays being applied outside of the kernel. >>>> >>>> Signed-off-by: Tom Rini <trini@konsulko.com> >>>> --- >>>> drivers/of/unittest-data/Makefile | 5 ----- >>>> scripts/Makefile.lib | 3 +++ >>>> 2 files changed, 3 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile >>>> index 6e00a9c69e58..70731cfe8900 100644 >>>> --- a/drivers/of/unittest-data/Makefile >>>> +++ b/drivers/of/unittest-data/Makefile >>>> @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S >>>> .PRECIOUS: \ >>>> $(obj)/%.dtb.S \ >>>> $(obj)/%.dtb >>>> - >>>> -# enable creation of __symbols__ node >>>> -DTC_FLAGS_overlay := -@ >>>> -DTC_FLAGS_overlay_bad_phandle := -@ >>>> -DTC_FLAGS_overlay_base := -@ >>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >>>> index 58c05e5d9870..a1f4a6b29d75 100644 >>>> --- a/scripts/Makefile.lib >>>> +++ b/scripts/Makefile.lib >>>> @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ >>>> -Wproperty_name_chars_strict >>>> endif >>>> >>>> +# enable creation of __symbols__ node >>>> +DTC_FLAGS += -@ >>>> + >>>> DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) >>>> >>>> # Generate an assembly file to wrap the output of the device tree compiler >>>> -- >>>> 1.9.1 >>>> >>> . > > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/16/17 09:16, Rob Herring wrote: > On Wed, Aug 16, 2017 at 10:57 AM, Tom Rini <trini@konsulko.com> wrote: >> On Wed, Aug 16, 2017 at 10:43:16AM -0500, Rob Herring wrote: >>> On Tue, Aug 15, 2017 at 5:49 PM, Tom Rini <trini@konsulko.com> wrote: >>>> On Tue, Aug 15, 2017 at 05:36:11PM -0500, Rob Herring wrote: >>>>> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@konsulko.com> wrote: >>>>>> With support for stacked overlays being part of libfdt it is now >>>>>> possible and likely that overlays which require __symbols__ will be >>>>>> applied to the dtb files generated by the kernel. This is done by >>>>>> passing -@ to dtc. This does increase the filesize (and resident memory >>>>>> usage) based on the number of __symbol__ entries added to match the >>>>>> contents of the dts. >>>>>> >>>>>> Cc: Rob Herring <robh+dt@kernel.org> >>>>>> Cc: Frank Rowand <frowand.list@gmail.com> >>>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >>>>>> Cc: Michal Marek <mmarek@suse.com> >>>>>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >>>>>> Cc: devicetree@vger.kernel.org >>>>>> Cc: linux-kernel@vger.kernel.org >>>>>> CC: linux-kbuild@vger.kernel.org >>>>>> Signed-off-by: Tom Rini <trini@konsulko.com> >>>>>> --- >>>>>> In order for a dtb file to be useful with all types of overlays, it >>>>>> needs to be generated with the -@ flag passed to dtc so that __symbols__ >>>>>> are generated. This however is not free, and increases the resulting >>>>>> dtb file by up to approximately 50% today. In the current worst case >>>>>> this is moving from 88KiB to 133KiB. In talking with Frank about this, >>>>> >>>>> Plus some amount for the unflattened tree in memory, too. >>>>> >>>>>> he outlined 3 possible ways (with the 4th option of something else >>>>>> entirely). >>>>>> >>>>>> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >>>>>> 2. In the kernel, if the kernel does not have overlay support, discard >>>>>> the __symbols__ information that we've been passed. >>>>>> 3. Have the bootloader pass in, or not, __symbols__ information. >>>>>> >>>>>> This patch is an attempt to implement something between the 3rd option >>>>>> and a different, 4th option. Frank was thinking that we might introduce >>>>>> a new symbol to control generation of __symbol__ information for option >>>>>> 1. I think this gets the usage backwards and will lead to confusion >>>>>> among users and developers. >>>>>> >>>>>> My proposal is that we do not want __symbols__ existence to be dependent >>>>>> on some part of the kernel configuration for a number of reasons. >>>>>> First, this is out of step with the rest of how dtbs are created today >>>>>> and more importantly, thought about. Today, all dtb content is >>>>>> independent of CONFIG options. If you build a dtb from a given kernel >>>>>> tree, everyone will agree on the result. This is part of the "contract" >>>>>> on passing old kernels and new dtb files even. >>>>> >>>>> Agree completely. I don't even like that building dtbs depends on the ARCH. >>>>> >>>>> However, option 2 may still be useful. There's no point exposing what >>>>> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree >>>>> at all may be a bad idea. We should consider if it should always be >>>>> hidden. That would also allow storing the __symbols__ data however we >>>>> want internally (i.e. with less memory usage). The complication is >>>>> always kexec which I haven't thought about too much here. >>>> >>>> A further patch to the kernel at run-time, OK. If you give me some >>>> crumbs I'll see if I can figure out the next steps. >>>> >>>>> Also, perhaps we need finer grain control of __symbols__ generation. >>>> >>>> Here I have to disagree. >>>> >>>>> We really don't want userspace to be able to modify anything in the DT >>>>> at any point in time. That's a big can of worms and we don't want to >>>>> start there. The problem is labels are widely used just for >>>>> convenience and weren't part of the ABI. With overlays that changes, >>>>> so we either need to restrict labels usage or define another way. It >>>>> could be as simple as defining some prefix for label names for labels >>>>> to export. >>>> >>>> I think there needs to be a difference noted between "here is what >>>> policy the kernel is going to enforce about run time changes" and "here >>>> is what the user is going to assemble a system to look like". Again, >>>> stemming from the part where the Linux kernel is where dts files reside >>>> and are generated from normally. If we have it in __symbols__, someone >>>> can make use of it in hardware design (again, think of the SoM + carrier >>>> + custom) bit, I've seen so many real life products now that would be >>>> simplified in this manner). >>> >>> I agree the usecase is an important one and one we should target, but >>> I think there are other issues to solve first before we get to the >>> trivial change needing to enable __symbols__. Do we have any dts files >>> actually structured for the SoM + carrier use case? I guess it's done >>> with includes ATM if we do. The run-time restrictions aren't just >>> kernel policy. The SoM itself is going to have restrictions defined by >>> its pinout. I think those need to be described in DT via a connector >>> binding. I worry about leaving things wide open and having overlays >>> just be a DT configuration tool with every platform structuring things >>> however they want. From what I've looked at on RPi, I'm very concerned >>> about having things like CMA overlays to set the CMA size. (On the >>> flip side as a user, it was very nice to just apply the RPi 1-wire >>> gpio overlay and things just worked.) >> >> I believe the various SoM and EVM and hobbyist cases are all either out >> of tree, or glued together (see imx6sx-udoo-neo-* in-tree, RPi or >> Hummingboard or TI DRA7 EVM + LCDs) as various groups decided it >> wouldn't be accepted to push in N "complete" DTS files for each valid >> combination). Moving forward with an in-kernel policy on how it should >> be done, structure-wise would help with consistency and defining what's >> really acceptable. > > IMO, that starts with a connector binding. Stephen Boyd has been > working towards that[1]. I guess "gpio nexus" is so obscure that all > the masses clamoring for overlays and modular board support haven't > noticed. > > Rob > > [1] https://www.spinics.net/lists/arm-kernel/msg600125.html Agreed. The connector binding is needed for overlays to move forward. -Frank -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/16/17 08:09, Tom Rini wrote: > On Tue, Aug 15, 2017 at 08:22:01PM -0700, Frank Rowand wrote: >> On 08/15/17 17:42, Tom Rini wrote: >>> On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote: >>>> On 08/15/17 14:15, Tom Rini wrote: >>>>> With support for stacked overlays being part of libfdt it is now >>>>> possible and likely that overlays which require __symbols__ will be >>>>> applied to the dtb files generated by the kernel. This is done by >>>>> passing -@ to dtc. This does increase the filesize (and resident memory >>>>> usage) based on the number of __symbol__ entries added to match the >>>>> contents of the dts. >>>>> >>>>> Cc: Rob Herring <robh+dt@kernel.org> >>>>> Cc: Frank Rowand <frowand.list@gmail.com> >>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >>>>> Cc: Michal Marek <mmarek@suse.com> >>>>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >>>>> Cc: devicetree@vger.kernel.org >>>>> Cc: linux-kernel@vger.kernel.org >>>>> CC: linux-kbuild@vger.kernel.org >>>>> Signed-off-by: Tom Rini <trini@konsulko.com> >>>>> --- >>>>> In order for a dtb file to be useful with all types of overlays, it >>>>> needs to be generated with the -@ flag passed to dtc so that __symbols__ >>>>> are generated. This however is not free, and increases the resulting >>>>> dtb file by up to approximately 50% today. In the current worst case >>>>> this is moving from 88KiB to 133KiB. In talking with Frank about this, >>>>> he outlined 3 possible ways (with the 4th option of something else >>>>> entirely). >>>>> >>>>> 1. Make passing -@ to dtc be dependent upon some CONFIG symbol. >>>>> 2. In the kernel, if the kernel does not have overlay support, discard >>>>> the __symbols__ information that we've been passed. >>>>> 3. Have the bootloader pass in, or not, __symbols__ information. >>>> >>>> I also was hoping that other people might have ideas for additional >>>> approaches. >>> >>> Yes, please. >>> >>>>> This patch is an attempt to implement something between the 3rd option >>>>> and a different, 4th option. Frank was thinking that we might introduce >>>>> a new symbol to control generation of __symbol__ information for option >>>>> 1. I think this gets the usage backwards and will lead to confusion >>>>> among users and developers. >>>>> >>>>> My proposal is that we do not want __symbols__ existence to be dependent >>>>> on some part of the kernel configuration for a number of reasons. >>>>> First, this is out of step with the rest of how dtbs are created today >>>>> and more importantly, thought about. Today, all dtb content is >>>>> independent of CONFIG options. If you build a dtb from a given kernel >>>>> tree, everyone will agree on the result. This is part of the "contract" >>>>> on passing old kernels and new dtb files even. >>>> >>>> I hope that dtb contents are independent of CONFIG options, but I don't >>>> feel confident is stating that there is not such dependency. (Of course, >>>> whether to build a dtb can be dependent on a CONFIG option in the Makefile, >>>> but that is not the same concept.) >>>> >>>> The only existing rule that I am aware of that helps avoid a dts dependency >>>> on kernel CONFIG options is that included files can not be from general kernel >>>> header files; they must be in include/dt-bindings/. >>> >>> I'm fairly certain for in-kernel stuff at least, the assumption is >>> correct. >>> >>>> Should we add text to Documentation/devicetree/bindings/submitting-patches.txt >>>> that explicitly states that dts files are not allowed to contain any >>>> dependency on kernel CONFIG options? >>> >>> Certainly can't hurt. >>> >>>>> Second, I think this is out of step with how a lot of overlay usage will >>>>> occur. My thinking is that with maximally useful overlays being >>>>> available in mainline, lots of use-cases that we have today that result >>>>> in a number of DTS files being included can become just overlays. This >>>> >>>> I disagree with this. My _opinion_ is that overlays should be the exception, >>>> not the common case. Overlays require extra complexity in the various >>>> subsystems that interact with device trees. For an overlay to work, these >>>> subsystems must be able to react to changes made to the device tree by >>>> an overlay. The current mechanism is via notifiers, which only exist >>>> for a few subsystems. >>> >>> Ah. Now, I can't blame you for thinking with your kernel hat on, but, >>> you're thinking with your kernel hat on :) (And taking mine off for a >>> minute is why I changed my mind between when we talked on IRC, and what >>> I posted). Kernel run-time apply an overlay has various use cases that >>> I don't want to discount, but don't want to try and go in detail on >>> either. >>> >>> At heart, one of the issues here is that the Linux kernel is the >>> authoritative source of dts and dtb files. Assembling a dtb and N >>> overlays at some point prior to booting Linux, in order to give it a >>> complete and valid system is going to be a common case. Even setting >> >> Yes. When discussing overlay issues and technologies we should be very >> clear about whether the context is post-boot run time loading of an >> overlay or using overlays applied on top of a base dtb to create a >> dtb to be fed to the kernel for booting. These are very different >> domains. >> >> For the case of constructing a boot time dtb from a base dtb and one >> or more overlays my primary concerns are related to the complexity >> (and again fragility) of the approach. It is already quite difficult >> to read source dts files and dts include files and determine what the >> final dtb will contain. This has been the motivation for several of >> my debugging tools. Adding overlays to the mix will increase the >> complexity of understanding the final product (dtb), where the >> individual items originated, and what component to modify to result >> in a change to the final product. It will add the further challenge >> that applying overlays in different orders may result in different >> final dtbs (depending on how strict the rules for applying an overlay >> are). I agree that this technology has some valuable upsides and >> use cases, but I also fear the negative impacts. The trick lies >> in finding a way to meet the technology needs in a manner that >> successfully balances the costs and the benefits. >> >> I mentioned my concerns about the robustness of overlays applied to >> a running Linux system in another reply. It seems to me that >> applying overlays pre-boot is more robust as far as the driver and >> subsystem initialization is concerned, because the normal boot >> path will be used to process the resulting dtb instead of using >> an overlay loading path in the kernel. >> >> >>> aside all of the hobbyist board families out there, SoMs are a big >>> thing. Eval modules are a big thing. This turns not just enabling >>> these as a vendor but using them as a developer into a much less error >>> prone system. >> >> If I understand the eval modules concept, then using overlays to >> develop and test device tree changes is somewhat analogous to using >> kernel modules to develop and test drivers. Yes, this may make >> some device tree source development more efficient. And in other >> cases may actually make it less efficient. I don't see the impact >> as being very large one direction or the other. >> >> >>> To try and re-state my rational, if the Linux kernel needs some >>> safe-guards or other mechanisms to restrict what can be done on top of >>> OF_OVERLAY (which is not widely enabled in mainline), OK, that's >>> certainly a discussion to have and think about implications of. But, >>> the Linux kernel is the producer of most dtbs that will be consumed by a >>> variety of platforms. I feel it needs to generate the dtb that can be >>> used for all consumers, since it is the source of these resources. >> >> This has gone far off topic from the original issue you brought up, >> which is how, in the context of the Linux kernel build environment, to >> enable building a base dtb that contains the symbols that are needed to >> load an overlay. Can we return to the base topic? And if anyone wants >> to discuss the other issues start a new thread? > > Well, how about this? There certainly should be some discussion about > how in the context of the Linux kernel run-time, the various impacts of > overlays need to be handled. But for out of kernel pre-boot cases, I > believe __symbols__ always is the only answer that won't bring everyone > more pain down the line. What needs to happen before we can get to that > point, since the Linux kernel is the gate-keeper to how dtbs are built? And I believe the opposite. Symbols should only be included in a dtb if there is an expectation they will be useful for that specific dtb. I see overlays as being useful in a minority of cases, not the majority. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile index 6e00a9c69e58..70731cfe8900 100644 --- a/drivers/of/unittest-data/Makefile +++ b/drivers/of/unittest-data/Makefile @@ -11,8 +11,3 @@ targets += overlay_base.dtb overlay_base.dtb.S .PRECIOUS: \ $(obj)/%.dtb.S \ $(obj)/%.dtb - -# enable creation of __symbols__ node -DTC_FLAGS_overlay := -@ -DTC_FLAGS_overlay_bad_phandle := -@ -DTC_FLAGS_overlay_base := -@ diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 58c05e5d9870..a1f4a6b29d75 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -293,6 +293,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ -Wproperty_name_chars_strict endif +# enable creation of __symbols__ node +DTC_FLAGS += -@ + DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) # Generate an assembly file to wrap the output of the device tree compiler