[PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

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.

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

[PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
Cc: Michal Marek <mmarek-IBi9RG/b67k@public.gmane.org>
Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
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.

Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Rob Herring]

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
>

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4342 bytes --]

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!

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
>>> Cc: Michal Marek <mmarek-IBi9RG/b67k@public.gmane.org>
>>> Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> CC: linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> ---
>>> 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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> ---
>>>  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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

On 08/15/17 15:36, Rob Herring wrote:
> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5492 bytes --]

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.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5723 bytes --]

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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> > Cc: Michal Marek <mmarek-IBi9RG/b67k@public.gmane.org>
> > Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > CC: linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > ---
> > 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.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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?

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Pantelis Antoniou]

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
> 

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Pantelis Antoniou]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Pantelis Antoniou]

Hi Frank,

> On Aug 16, 2017, at 02:57 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> On 08/15/17 15:36, Rob Herring wrote:
>> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
>>> Cc: Michal Marek <mmarek-IBi9RG/b67k@public.gmane.org>
>>> Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> CC: linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> ---
>>> 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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> ---
>>> 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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 8813 bytes --]

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?

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Rob Herring]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

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.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Rob Herring]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6071 bytes --]

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.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6356 bytes --]

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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> > Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> > Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> >> > Cc: Michal Marek <mmarek-IBi9RG/b67k@public.gmane.org>
> >> > Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> > CC: linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> > Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >> > ---
> >> > 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.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Rob Herring]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Rob Herring]

On Wed, Aug 16, 2017 at 10:41 AM, Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >> > Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >> > Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
>> >> > Cc: Michal Marek <mmarek-IBi9RG/b67k@public.gmane.org>
>> >> > Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> >> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> >> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> >> > CC: linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> >> > Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> >> > ---
>> >> > 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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Rob Herring]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

On 08/16/17 09:16, Rob Herring wrote:
> On Wed, Aug 16, 2017 at 10:57 AM, Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
>>>>>> Cc: Michal Marek <mmarek-IBi9RG/b67k@public.gmane.org>
>>>>>> Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>>> CC: linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>>> Signed-off-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>> ---
>>>>>> 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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Frank Rowand]

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.