[RFC:PATCH dtc-1.3.0] dtc: Add --strip-disabled option to dtc.
David Gibson
dwg at au1.ibm.com
Fri Aug 17 16:04:15 EST 2012
On Wed, Aug 15, 2012 at 01:38:45PM +0100, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla at st.com>
>
> This patch allows dtc to strip out nodes in its output based on status
> property. Now the dtc has additional long option --strip-disabled to
> strip all the nodes which have status property set to disabled.
>
> SOCs have lot of device tree infrastructure files which mark the
> device nodes as disabled and the board level device tree enables them if
> required. However while creating device tree blob, the compiler can
> exclude nodes marked as disabled, doing this way will reduce the size
> of device tree blob.
>
> However care has to be taken if your boardloader is is updating status
> property.
>
> In our case this has reduced the blob size from 29K to 15K.
>
> Also nodes with status="disabled" is are never probed by dt platform bus
> code.
>
> Again, this is an optional parameter to dtc, Can be used by people who
> want to strip all the device nodes which are marked as disabled.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla at st.com>
> ---
> Hi Jon,
>
> I have noticed that the dtb blob also contains device nodes with property status = "disabled",
> But these device nodes are not used by device tree platform bus probe code or any of the kernel code.
>
> If there is no active code which actually modifies status property at runtime, it makes sense to remove
> those nodes from the dtb to reduce the overall size of dtb.
>
> The size change will be significant once the SOC adds all the possible devices in to the device trees.
> As there could be 100s of Ips on SOCs but the board actually uses may be 20-25 IP's.
>
> My patch adds option --strip-disabled option to dtc to strip such nodes, resulting in only nodes which
> are supposed to be instantiated.
>
> This patch is generated on top of git://git.jdl.com/software/dtc.git.
>
> previous discussion on this patch is at:http://comments.gmane.org/gmane.linux.kbuild.devel/8527
>
> Comments?
>
> Thanks,
> srini
>
> dtc.c | 14 ++++++++++++--
> dtc.h | 4 ++++
> flattree.c | 3 +++
> livetree.c | 17 +++++++++++++++++
> treesource.c | 3 +++
> 5 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/dtc.c b/dtc.c
> index a375683..b303cab 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -30,6 +30,7 @@ int quiet; /* Level of quietness */
> int reservenum; /* Number of memory reservation slots */
> int minsize; /* Minimum blob size */
> int padsize; /* Additional padding to blob */
> +int strip_disabled; /* strip disabled nodes in output */
> int phandle_format = PHANDLE_BOTH; /* Use linux,phandle or phandle properties */
>
> static void fill_fullpaths(struct node *tree, const char *prefix)
> @@ -96,6 +97,8 @@ static void __attribute__ ((noreturn)) usage(void)
> fprintf(stderr, "\t-W [no-]<checkname>\n");
> fprintf(stderr, "\t-E [no-]<checkname>\n");
> fprintf(stderr, "\t\t\tenable or disable warnings and errors\n");
> + fprintf(stderr, "\t--strip-disabled\n");
> + fprintf(stderr, "\t\tStrip nodes with status property as disabled\n");
> exit(3);
> }
>
> @@ -112,15 +115,22 @@ int main(int argc, char *argv[])
> FILE *outf = NULL;
> int outversion = DEFAULT_FDT_VERSION;
> long long cmdline_boot_cpuid = -1;
> + struct option loptions[] = {
> + {"strip-disabled", no_argument, &strip_disabled, 1},
> + };
>
> quiet = 0;
> reservenum = 0;
> minsize = 0;
> padsize = 0;
> + strip_disabled = 0;
>
> - while ((opt = getopt(argc, argv, "hI:O:o:V:d:R:S:p:fqb:i:vH:sW:E:"))
> - != EOF) {
> + while ((opt = getopt_long(argc, argv,
> + "hI:O:o:V:d:R:S:p:fqb:i:vH:sW:E:",
> + loptions, NULL)) != EOF) {
> switch (opt) {
> + case 0: /* Long option set */
> + break;
> case 'I':
> inform = optarg;
> break;
> diff --git a/dtc.h b/dtc.h
> index 7ee2d54..67cdadc 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -31,6 +31,7 @@
> #include <ctype.h>
> #include <errno.h>
> #include <unistd.h>
> +#include <getopt.h>
>
> #include <libfdt_env.h>
> #include <fdt.h>
> @@ -53,6 +54,7 @@ extern int quiet; /* Level of quietness */
> extern int reservenum; /* Number of memory reservation slots */
> extern int minsize; /* Minimum blob size */
> extern int padsize; /* Additional padding to blob */
> +extern int strip_disabled; /* strip disabled nodes in output */
> extern int phandle_format; /* Use linux,phandle or phandle properties */
>
> #define PHANDLE_LEGACY 0x1
> @@ -224,6 +226,8 @@ struct boot_info *build_boot_info(struct reserve_info *reservelist,
> struct node *tree, uint32_t boot_cpuid_phys);
> void sort_tree(struct boot_info *bi);
>
> +int is_device_node_avaiable(struct node *node);
> +
> /* Checks */
>
> void parse_checks_option(bool warn, bool error, const char *optarg);
> diff --git a/flattree.c b/flattree.c
> index 28d0b23..7d4e13b 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -304,6 +304,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
> }
>
> for_each_child(tree, child) {
> + if (strip_disabled && !is_device_node_avaiable(child))
> + continue;
> +
Hrm, it's not a lot of code, but I don't like that the strip logic is
duplicated between the flat tree and treesource backends. What I'd
prefer to see here is just if (node_is_stripped(...)) with the
node_is_stripped() function taking into account the option values and
whatever other information.
> flatten_tree(child, emit, etarget, strbuf, vi);
> }
>
> diff --git a/livetree.c b/livetree.c
> index c9209d5..7155926 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -607,3 +607,20 @@ void sort_tree(struct boot_info *bi)
> sort_reserve_entries(bi);
> sort_node(bi->dt);
> }
> +
> +int is_device_node_avaiable(struct node *node)
> +{
> + struct property *status;
> +
> + status = get_property(node, "status");
> + if (status == NULL)
> + return 1;
> +
> + if (status->val.len > 0) {
> + if (!strcmp(status->val.val, "okay") ||
> + !strcmp(status->val.val, "ok"))
> + return 1;
> + }
The name still isn't quite right - it doesn't just strip disabled
nodes but anything that isn't "okay", OF defines "failed" at least as
another possibility for the status property.
> + return 0;
> +}
> diff --git a/treesource.c b/treesource.c
> index 33eeba5..2f1ac1a 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -255,6 +255,9 @@ static void write_tree_source_node(FILE *f, struct node *tree, int level)
> write_propval(f, prop);
> }
> for_each_child(tree, child) {
> + if (strip_disabled && !is_device_node_avaiable(child))
> + continue;
> +
> fprintf(f, "\n");
> write_tree_source_node(f, child, level+1);
> }
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
More information about the devicetree-discuss
mailing list