[RFC PATCH 0/2] Emit experimental YAML output

[RFC PATCH 0/2] Emit experimental YAML output

[Grant Likely]

Here is some code I've been experimenting with as part of evaluating what the
YAML encoding should be for DT. It adds YAML output to DTC.

Pantelis, I know you already wrote a patch[1] that does exactly this. The only
reason I wrote a new patch is because I wanted to try a couple of different
things:
- I want to preserve datatypes and integer groupings from the DT source as much
  as possible
- I want to use an existing yaml emitter (libyaml) instead of raw fprintf() to
  avoid emitting something that is not quite compatible with YAML.

This series consists of two patches. The first patch adds new markers to
indicate the datatypes and groupings. The treesource output routines are
modified to use the datatype marker information when emitting .dts source. If
the new markers are not present (like when reading from a DTB source), then it
falls back to the existing scheme of guessing the datatype.

The first patch is pretty close to complete. I need to go through the error
conditions again, but otherwise it could be merged. Having these markers is
generally useful, and could be used as part of validation/schema checks at
compile time. For instance, keeping the groupings of interrupt specifiers would
make it easier to check that an interrupt specifier is the correct size. I
could also see adding an intermediary stage after reading a .dtb file to
interpret the properties and add the correct markers wherever possible.
phandles could be resolved back to labels in this way, which would make
decompiling .dtbs a lot more useful.

The second patch is not complete. It cannot be merged until the yaml encoding
is nailed down. It also uses libyaml, but I haven't wired in the linker flags
correctly. dtc doesn't currently use any external library apart from libc, so
I'm not sure how best to add it in. DTC should still compile successfully if
libyaml isn't available.

[1] https://github.com/pantoniou/dtc/commits/yaml

Here's an example of the output with the original source included as comments
for comparison. This example comes from tests/label01.dts.

# / {
#   model = "MyBoardName";
#   compatible = "MyBoardName", "MyBoardFamilyName";
#   #address-cells = <2>;
#   #size-cells = <2>;
- model: ["MyBoardName"]
  compatible: ["MyBoardName", "MyBoardFamilyName"]
  '#address-cells': [[0x2]]
  '#size-cells': [[0x2]]

  # cpus {
  #   linux,phandle = <0x1>;
  #   #address-cells = <1>;
  #   #size-cells = <0>;
  cpus:
    linux,phandle: [[0x1]]
    '#address-cells': [[0x1]]
    '#size-cells': [[0x0]]


    # PowerPC,970@0 {
    #   name = "PowerPC,970";
    #   device_type = "cpu";
    #   reg = <0x00000000>;
    #   clock-frequency = <1600000000>;
    #   timebase-frequency = <33333333>;
    #   linux,boot-cpu;
    #   i-cache-size = <65536>;
    #   d-cache-size = <32768>;
    # };
    PowerPC,970@0:
      device_type: ["cpu"]
      reg: [[0x0]]
      clock-frequency: [[0x5f5e1000]]
      timebase-frequency: [[0x1fca055]]
      linux,boot-cpu: true
      i-cache-size: [[0x10000]]
      d-cache-size: [[0x8000]]

    # PowerPC,970@1 {
    #   name = "PowerPC,970";
    #   device_type = "cpu";
    #   reg = <0x00000001>;
    #   clock-frequency = <1600000000>;
    #   timebase-frequency = <33333333>;
    #   i-cache-size = <65536>;
    #   d-cache-size = <32768>;
    # };
    PowerPC,970@1:
      device_type: ["cpu"]
      reg: [[0x1]]
      clock-frequency: [[0x5f5e1000]]
      timebase-frequency: [[0x1fca055]]
      i-cache-size: [[0x10000]]
      d-cache-size: [[0x8000]]

  # node: randomnode {
  #   prop: string = str: "foo", str_mid: "stuffstuff\t\t\t\n\n\n" str_end: ;
  #   blob = [byte: 0a 0b 0c 0d byte_mid: de ea ad be ef byte_end: ];
  #   ref = < cell: &{/memory@0} 0x0 cell_mid: 0xffffffff cell_end: >;
  #   mixed = "abc", pre: [1234] post: , gap: < aligned: 0xa 0xb 0xc>;
  #   tricky1 = [61 lt1: 62 63 00];
  #   subnode: child {
  #   };
  #   /* subnode_end: is auto-generated by node emit */
  # };
  randomnode:
    $labels: [node]
    string: ["foo", "stuffstuff\t\t\t\n\n\n"]
    blob: [!u8 [0xa, 0xb, 0xc, 0xd, 0xde, 0xea, 0xad, 0xbe, 0xef]]
    ref: [[!phandle "/memory@0", 0x0, 0xffffffff]]
    mixed: ["abc", !u8 [0x12, 0x34], [0xa, 0xb, 0xc]]
    tricky1: [!u8 [0x61, 0x62, 0x63, 0x0]]
    child:
      $labels: [subnode]

  # memory@0 {
  #   device_type = "memory";
  #   memreg: reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
  # };
  memory@0:
    device_type: ["memory"]
    reg: [[0x0, 0x0, 0x0, 0x20000000]]
    phandle: [[0x2]]

  # chosen {
  #   bootargs = "root=/dev/sda2";
  #   linux,platform = <0x600>;
  # };
  chosen:
    bootargs: ["root=/dev/sda2"]
    linux,platform: [[0x600]]
...

[RFC PATCH 1/2] Preserve datatype information when parsing dts

[Grant Likely]

The current code throws away all the data type and grouping information
when parsing the DTS source file, which makes it difficult to
reconstruct the data format when emitting a format that can express data
types (ie. dts and yaml). Use the marker list to mark the beginning and
end of each integer array block (<> and []), the datatype contained in
each (8, 16, 32 & 64 bit widths), and the start of each string.

At the same time, factor out the heuristic code used to guess a property
type at emit time. It is a pretty well defined standalone block that
could be used elsewhere, for instance, when emitting YAML source. Factor
it out into a separate function so that it can be reused, and also to
simplify the write_propval() function.

When emitting, group integer output back into the same groups as the
original source and use the REF_PATH and REF_PHANDLE markers to emit the
the node reference instead of a raw path or phandle.

Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
---
 data.c       |   4 +-
 dtc-parser.y |  21 +++--
 dtc.h        |   9 +++
 livetree.c   |  49 ++++++++++++
 treesource.c | 249 ++++++++++++++++++++++++++++++++---------------------------
 5 files changed, 212 insertions(+), 120 deletions(-)

diff --git a/data.c b/data.c
index aa37a16..0aef0b5 100644
--- a/data.c
+++ b/data.c
@@ -74,7 +74,8 @@ struct data data_copy_escape_string(const char *s, int len)
 	struct data d;
 	char *q;
 
-	d = data_grow_for(empty_data, len + 1);
+	d = data_add_marker(empty_data, MARKER_STRING, NULL);
+	d = data_grow_for(d, len + 1);
 
 	q = d.val;
 	while (i < len) {
@@ -94,6 +95,7 @@ struct data data_copy_file(FILE *f, size_t maxlen)
 {
 	struct data d = empty_data;
 
+	d = data_add_marker(d, MARKER_BLOB, NULL);
 	while (!feof(f) && (d.len < maxlen)) {
 		size_t chunksize, ret;
 
diff --git a/dtc-parser.y b/dtc-parser.y
index 44af170..41fbd3f 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -266,11 +266,13 @@ propdata:
 		}
 	| propdataprefix arrayprefix '>'
 		{
-			$$ = data_merge($1, $2.data);
+			$1 = data_merge($1, $2.data);
+			$$ = data_add_marker($1, MARKER_NONE, NULL);
 		}
 	| propdataprefix '[' bytestring ']'
 		{
-			$$ = data_merge($1, $3);
+			$1 = data_merge($1, $3);
+			$$ = data_add_marker($1, MARKER_NONE, NULL);
 		}
 	| propdataprefix DT_REF
 		{
@@ -327,22 +329,27 @@ arrayprefix:
 	DT_BITS DT_LITERAL '<'
 		{
 			unsigned long long bits;
+			enum markertype type = MARKER_UINT32;
 
 			bits = $2;
 
-			if ((bits !=  8) && (bits != 16) &&
-			    (bits != 32) && (bits != 64)) {
+			switch (bits) {
+			case 8: type = MARKER_UINT8; break;
+			case 16: type = MARKER_UINT16; break;
+			case 32: type = MARKER_UINT32; break;
+			case 64: type = MARKER_UINT64; break;
+			default:
 				ERROR(&@2, "Array elements must be"
 				      " 8, 16, 32 or 64-bits");
 				bits = 32;
 			}
 
-			$$.data = empty_data;
+			$$.data = data_add_marker(empty_data, type, NULL);
 			$$.bits = bits;
 		}
 	| '<'
 		{
-			$$.data = empty_data;
+			$$.data = data_add_marker(empty_data, MARKER_UINT32, NULL);
 			$$.bits = 32;
 		}
 	| arrayprefix integer_prim
@@ -486,7 +493,7 @@ integer_unary:
 bytestring:
 	  /* empty */
 		{
-			$$ = empty_data;
+			$$ = data_add_marker(empty_data, MARKER_UINT8, NULL);
 		}
 	| bytestring DT_BYTE
 		{
diff --git a/dtc.h b/dtc.h
index 3b18a42..32a7655 100644
--- a/dtc.h
+++ b/dtc.h
@@ -74,9 +74,17 @@ typedef uint32_t cell_t;
 
 /* Data blobs */
 enum markertype {
+	MARKER_NONE = 0,
 	REF_PHANDLE,
 	REF_PATH,
 	LABEL,
+	MARKER_UINT8,
+	MARKER_UINT16,
+	MARKER_UINT32,
+	MARKER_UINT64,
+	MARKER_BLOB,
+	MARKER_STRING,
+	NUM_MARKERS,
 };
 
 struct  marker {
@@ -198,6 +206,7 @@ struct property *build_property(char *name, struct data val);
 struct property *build_property_delete(char *name);
 struct property *chain_property(struct property *first, struct property *list);
 struct property *reverse_properties(struct property *first);
+enum markertype guess_propval_type(struct property *prop);
 
 struct node *build_node(struct property *proplist, struct node *children);
 struct node *build_node_delete(void);
diff --git a/livetree.c b/livetree.c
index 57b7db2..1bbdaf8 100644
--- a/livetree.c
+++ b/livetree.c
@@ -336,6 +336,55 @@ void append_to_property(struct node *node,
 	}
 }
 
+static bool isstring(char c)
+{
+	return (isprint((unsigned char)c)
+		|| (c == '\0')
+		|| strchr("\a\b\t\n\v\f\r", c));
+}
+
+enum markertype guess_propval_type(struct property *prop)
+{
+	int len = prop->val.len;
+	const char *p = prop->val.val;
+	struct marker *m = prop->val.markers;
+	int nnotstring = 0, nnul = 0;
+	int nnotstringlbl = 0, nnotcelllbl = 0;
+	int i;
+
+	/* Check if the property is type annotated */
+	while (m && (m->type == LABEL || m->type == REF_PHANDLE))
+		m = m->next;
+	if (m && m->offset == 0)
+		return MARKER_NONE;
+
+	m = prop->val.markers;
+
+	/* data type information missing, need to guess */
+	for (i = 0; i < len; i++) {
+		if (! isstring(p[i]))
+			nnotstring++;
+		if (p[i] == '\0')
+			nnul++;
+	}
+
+	for_each_marker_of_type(m, LABEL) {
+		if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
+			nnotstringlbl++;
+		if ((m->offset % sizeof(cell_t)) != 0)
+			nnotcelllbl++;
+	}
+
+	if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
+	    && (nnotstringlbl == 0)) {
+		return MARKER_STRING;
+	} else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
+		return MARKER_UINT32;
+	} else {
+		return MARKER_UINT8;
+	}
+}
+
 struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
 {
 	struct reserve_info *new = xmalloc(sizeof(*new));
diff --git a/treesource.c b/treesource.c
index 2461a3d..cc3b223 100644
--- a/treesource.c
+++ b/treesource.c
@@ -54,29 +54,15 @@ static void write_prefix(FILE *f, int level)
 		fputc('\t', f);
 }
 
-static bool isstring(char c)
+static void write_propval_string(FILE *f, const char *str, size_t len)
 {
-	return (isprint((unsigned char)c)
-		|| (c == '\0')
-		|| strchr("\a\b\t\n\v\f\r", c));
-}
-
-static void write_propval_string(FILE *f, struct data val)
-{
-	const char *str = val.val;
 	int i;
-	struct marker *m = val.markers;
 
-	assert(str[val.len-1] == '\0');
+	assert(str[len-1] == '\0');
 
-	while (m && (m->offset == 0)) {
-		if (m->type == LABEL)
-			fprintf(f, "%s: ", m->ref);
-		m = m->next;
-	}
 	fprintf(f, "\"");
 
-	for (i = 0; i < (val.len-1); i++) {
+	for (i = 0; i < (len-1); i++) {
 		char c = str[i];
 
 		switch (c) {
@@ -108,15 +94,7 @@ static void write_propval_string(FILE *f, struct data val)
 			fprintf(f, "\\\"");
 			break;
 		case '\0':
-			fprintf(f, "\", ");
-			while (m && (m->offset <= (i + 1))) {
-				if (m->type == LABEL) {
-					assert(m->offset == (i+1));
-					fprintf(f, "%s: ", m->ref);
-				}
-				m = m->next;
-			}
-			fprintf(f, "\"");
+			fprintf(f, "\", \"");
 			break;
 		default:
 			if (isprint((unsigned char)c))
@@ -126,114 +104,161 @@ static void write_propval_string(FILE *f, struct data val)
 		}
 	}
 	fprintf(f, "\"");
-
-	/* Wrap up any labels at the end of the value */
-	for_each_marker_of_type(m, LABEL) {
-		assert (m->offset == val.len);
-		fprintf(f, " %s:", m->ref);
-	}
 }
 
-static void write_propval_cells(FILE *f, struct data val)
+static void write_propval_ref(FILE *f, struct node *np, const char *hint)
 {
-	void *propend = val.val + val.len;
-	fdt32_t *cp = (fdt32_t *)val.val;
-	struct marker *m = val.markers;
-
-	fprintf(f, "<");
-	for (;;) {
-		while (m && (m->offset <= ((char *)cp - val.val))) {
-			if (m->type == LABEL) {
-				assert(m->offset == ((char *)cp - val.val));
-				fprintf(f, "%s: ", m->ref);
-			}
-			m = m->next;
-		}
+	struct label *l;
+	struct node *root = np;
 
-		fprintf(f, "0x%x", fdt32_to_cpu(*cp++));
-		if ((void *)cp >= propend)
-			break;
-		fprintf(f, " ");
-	}
+	while (root->parent)
+		root = root->parent;
 
-	/* Wrap up any labels at the end of the value */
-	for_each_marker_of_type(m, LABEL) {
-		assert (m->offset == val.len);
-		fprintf(f, " %s:", m->ref);
+	/* If possible, use the hint string as the reference */
+	if (hint && np == get_node_by_ref(root, hint)) {
+		fprintf(f, hint[0] == '/' ? " &{%s}" : " &%s", hint);
+		return;
 	}
-	fprintf(f, ">");
-}
-
-static void write_propval_bytes(FILE *f, struct data val)
-{
-	void *propend = val.val + val.len;
-	const char *bp = val.val;
-	struct marker *m = val.markers;
-
-	fprintf(f, "[");
-	for (;;) {
-		while (m && (m->offset == (bp-val.val))) {
-			if (m->type == LABEL)
-				fprintf(f, "%s: ", m->ref);
-			m = m->next;
-		}
 
-		fprintf(f, "%02hhx", (unsigned char)(*bp++));
-		if ((const void *)bp >= propend)
-			break;
-		fprintf(f, " ");
+	/* Check if the node has a usable label */
+	for_each_label(np->labels, l) {
+		fprintf(f, " &%s", l->label);
+		return;
 	}
 
-	/* Wrap up any labels at the end of the value */
-	for_each_marker_of_type(m, LABEL) {
-		assert (m->offset == val.len);
-		fprintf(f, " %s:", m->ref);
-	}
-	fprintf(f, "]");
+	fprintf(f, " &{%s}", np->fullpath);
 }
 
-static void write_propval(FILE *f, struct property *prop)
+const char * delim_start[NUM_MARKERS] = {
+	[MARKER_BLOB] = "[",
+	[MARKER_UINT8] = "[",
+	[MARKER_UINT16] = "/bits/ 16 <",
+	[MARKER_UINT32] = "<",
+	[MARKER_UINT64] = "/bits/ 64 <",
+};
+const char * delim_end[NUM_MARKERS] = {
+	[MARKER_BLOB] = " ]",
+	[MARKER_UINT8] = " ]",
+	[MARKER_UINT16] = " >",
+	[MARKER_UINT32] = " >",
+	[MARKER_UINT64] = " >",
+};
+static void write_propval(FILE *f, struct node *root, struct property *prop)
 {
-	int len = prop->val.len;
-	const char *p = prop->val.val;
+	size_t len = prop->val.len;
+	size_t chunk_len;
 	struct marker *m = prop->val.markers;
-	int nnotstring = 0, nnul = 0;
-	int nnotstringlbl = 0, nnotcelllbl = 0;
-	int i;
+	struct marker dummy_marker = {
+		.offset = 0, .type = MARKER_NONE, .next = m, .ref = NULL
+	};
+	enum markertype emit_type = MARKER_NONE;
+	struct node *np;
+	cell_t phandle;
 
 	if (len == 0) {
 		fprintf(f, ";\n");
 		return;
 	}
 
-	for (i = 0; i < len; i++) {
-		if (! isstring(p[i]))
-			nnotstring++;
-		if (p[i] == '\0')
-			nnul++;
-	}
+	fprintf(f, " = ");
 
-	for_each_marker_of_type(m, LABEL) {
-		if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
-			nnotstringlbl++;
-		if ((m->offset % sizeof(cell_t)) != 0)
-			nnotcelllbl++;
-	}
+	dummy_marker.type = guess_propval_type(prop);
+	if (dummy_marker.type != MARKER_NONE)
+		m = &dummy_marker;
 
-	fprintf(f, " = ");
-	if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
-	    && (nnotstringlbl == 0)) {
-		write_propval_string(f, prop->val);
-	} else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
-		write_propval_cells(f, prop->val);
-	} else {
-		write_propval_bytes(f, prop->val);
+	for_each_marker(m) {
+		chunk_len = (m->next ? m->next->offset : len) - m->offset;
+		const char *p = &prop->val.val[m->offset];
+		const char *end = p + chunk_len;
+
+		switch(m->type) {
+		case MARKER_NONE:
+			if (emit_type != MARKER_NONE)
+				fprintf(f, "%s", delim_end[emit_type]);
+			emit_type = m->type;
+			break;
+		case MARKER_STRING:
+		case MARKER_BLOB:
+		case MARKER_UINT8:
+		case MARKER_UINT16:
+		case MARKER_UINT32:
+		case MARKER_UINT64:
+			emit_type = m->type;
+			fprintf(f, m->offset ? ", %s" : "%s",
+			           delim_start[emit_type] ? : "");
+			break;
+		case LABEL:
+			fprintf(f, " %s:", m->ref);
+			break;
+		case REF_PHANDLE:
+			assert(emit_type == MARKER_UINT32);
+			assert(chunk_len >= sizeof(fdt32_t));
+			phandle = fdt32_to_cpu(*(const fdt32_t*)p);
+			np = get_node_by_phandle(root, phandle);
+			if (np) {
+				write_propval_ref(f, np, NULL);
+				chunk_len -= sizeof(fdt32_t);
+				p += sizeof(fdt32_t);
+			}
+			break;
+		case REF_PATH:
+			assert(emit_type == MARKER_NONE);
+			assert(strnlen(p, chunk_len) == chunk_len - 1);
+			np = get_node_by_ref(root, p);
+			if (np) {
+				write_propval_ref(f, np, m->ref);
+				p += chunk_len;
+				chunk_len = 0;
+			}
+			break;
+		default:
+			break;
+		}
+
+		if (chunk_len <= 0)
+			continue;
+
+		switch (emit_type) {
+		case MARKER_BLOB:
+		case MARKER_UINT8:
+			for (; p < end; p++)
+				fprintf(f, " %02"PRIx8, *(const uint8_t *)p);
+			break;
+		case MARKER_UINT16:
+			assert((chunk_len % sizeof(fdt16_t)) == 0);
+			for (; p < end; p += sizeof(fdt16_t))
+				fprintf(f, " 0x%02"PRIx16,
+				        fdt16_to_cpu(*(const fdt16_t*)p));
+			break;
+		case MARKER_UINT32:
+			assert((chunk_len % sizeof(fdt32_t)) == 0);
+			for (; p < end; p += sizeof(fdt32_t))
+				fprintf(f, " 0x%02"PRIx32,
+				        fdt32_to_cpu(*(const fdt32_t*)p));
+			break;
+		case MARKER_UINT64:
+			assert((chunk_len % sizeof(fdt64_t)) == 0);
+			for (; p < end; p += sizeof(fdt64_t))
+				fprintf(f, " 0x%02"PRIx64,
+				        fdt64_to_cpu(*(const fdt64_t*)p));
+			break;
+		case MARKER_STRING:
+			assert(p[chunk_len-1] == '\0');
+			write_propval_string(f, p, chunk_len);
+			break;
+		default:
+			/* Default to a block of raw bytes */
+			fprintf(f, "[ ");
+			for (; p < end; p++)
+				fprintf(f, " %02"PRIx8, *p);
+			fprintf(f, "]");
+		}
 	}
 
-	fprintf(f, ";\n");
+	fprintf(f, "%s;\n", delim_end[emit_type] ? : "");
 }
 
-static void write_tree_source_node(FILE *f, struct node *tree, int level)
+static void write_tree_source_node(FILE *f, struct node *root, struct node *tree, int level)
 {
 	struct property *prop;
 	struct node *child;
@@ -252,11 +277,11 @@ static void write_tree_source_node(FILE *f, struct node *tree, int level)
 		for_each_label(prop->labels, l)
 			fprintf(f, "%s: ", l->label);
 		fprintf(f, "%s", prop->name);
-		write_propval(f, prop);
+		write_propval(f, root, prop);
 	}
 	for_each_child(tree, child) {
 		fprintf(f, "\n");
-		write_tree_source_node(f, child, level+1);
+		write_tree_source_node(f, root, child, level+1);
 	}
 	write_prefix(f, level);
 	fprintf(f, "};\n");
@@ -279,6 +304,6 @@ void dt_to_source(FILE *f, struct dt_info *dti)
 			(unsigned long long)re->size);
 	}
 
-	write_tree_source_node(f, dti->dt, 0);
+	write_tree_source_node(f, dti->dt, dti->dt, 0);
 }
 
-- 
2.11.0

[RFC PATCH 2/2] Add support for YAML output

[Grant Likely]

DO NOT MERGE! This is prototype code and the YAML encoding has not been
finalized yet.

Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
---
 Documentation/manual.txt |   2 +
 Makefile                 |   1 +
 Makefile.dtc             |   1 +
 dtc.c                    |   5 +
 dtc.h                    |   4 +
 yamltree.c               | 355 +++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 368 insertions(+)
 create mode 100644 yamltree.c

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index 72403ac..5bedf05 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -78,6 +78,8 @@ The currently supported Output Formats are:
         then simply be added to your Makefile.  Additionally, the
         assembly file exports some symbols that can be used.
 
+     - "yaml": DT encoded in YAML
+
 
 3) Command Line
 
diff --git a/Makefile b/Makefile
index e1e138a..a1db301 100644
--- a/Makefile
+++ b/Makefile
@@ -35,6 +35,7 @@ INCLUDEDIR = $(PREFIX)/include
 HOSTOS := $(shell uname -s | tr '[:upper:]' '[:lower:]' | \
 	    sed -e 's/\(cygwin\|msys\).*/\1/')
 
+LDFLAGS = -L/usr/local/lib -lyaml
 ifeq ($(HOSTOS),darwin)
 SHAREDLIB_EXT     = dylib
 SHAREDLIB_CFLAGS  = -fPIC
diff --git a/Makefile.dtc b/Makefile.dtc
index bece49b..f97239d 100644
--- a/Makefile.dtc
+++ b/Makefile.dtc
@@ -8,6 +8,7 @@ DTC_SRCS = \
 	data.c \
 	dtc.c \
 	flattree.c \
+	yamltree.c \
 	fstree.c \
 	livetree.c \
 	srcpos.c \
diff --git a/dtc.c b/dtc.c
index c36994e..4926757 100644
--- a/dtc.c
+++ b/dtc.c
@@ -95,6 +95,7 @@ static const char * const usage_opts_help[] = {
 	"\n\tOutput formats are:\n"
 	 "\t\tdts - device tree source text\n"
 	 "\t\tdtb - device tree blob\n"
+	 "\t\tyaml - device tree encoded as YAML\n"
 	 "\t\tasm - assembler source",
 	"\n\tBlob version to produce, defaults to "stringify(DEFAULT_FDT_VERSION)" (for dtb and asm output)",
 	"\n\tOutput dependency file",
@@ -128,6 +129,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
 		return fallback;
 	if (!strcasecmp(s, ".dts"))
 		return "dts";
+	if (!strcasecmp(s, ".yaml"))
+		return "yaml";
 	if (!strcasecmp(s, ".dtb"))
 		return "dtb";
 	return fallback;
@@ -350,6 +353,8 @@ int main(int argc, char *argv[])
 
 	if (streq(outform, "dts")) {
 		dt_to_source(outf, dti);
+	} else if (streq(outform, "yaml")) {
+		dt_to_yaml(outf, dti);
 	} else if (streq(outform, "dtb")) {
 		dt_to_blob(outf, dti, outversion);
 	} else if (streq(outform, "asm")) {
diff --git a/dtc.h b/dtc.h
index 32a7655..8db4814 100644
--- a/dtc.h
+++ b/dtc.h
@@ -295,6 +295,10 @@ struct dt_info *dt_from_blob(const char *fname);
 void dt_to_source(FILE *f, struct dt_info *dti);
 struct dt_info *dt_from_source(const char *f);
 
+/* YAML source */
+
+void dt_to_yaml(FILE *f, struct dt_info *dti);
+
 /* FS trees */
 
 struct dt_info *dt_from_fs(const char *dirname);
diff --git a/yamltree.c b/yamltree.c
new file mode 100644
index 0000000..cfdc9f3
--- /dev/null
+++ b/yamltree.c
@@ -0,0 +1,355 @@
+/*
+ * (C) Copyright Arm Holdings.  2017
+ * (C) Copyright David Gibson <dwg-8fk3Idey6ehBDgjK7y7TUQ@public.gmane.org>, IBM Corporation.  2005.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
+ *                                                                   USA
+ */
+
+#include <stdlib.h>
+#include <yaml.h>
+#include "dtc.h"
+#include "srcpos.h"
+
+char *yaml_error_name[] = {
+	[YAML_NO_ERROR] = "no error",
+	[YAML_MEMORY_ERROR] = "memory error",
+	[YAML_READER_ERROR] = "reader error",
+	[YAML_SCANNER_ERROR] = "scanner error",
+	[YAML_PARSER_ERROR] = "parser error",
+	[YAML_COMPOSER_ERROR] = "composer error",
+	[YAML_WRITER_ERROR] = "writer error",
+	[YAML_EMITTER_ERROR] = "emitter error",
+};
+
+#define yaml_die(emitter) die("yaml '%s': %s in %s, line %i", yaml_error_name[(emitter)->error], (emitter)->problem, __func__, __LINE__)
+
+static void yaml_propval_ref(yaml_emitter_t *emitter, struct node *np, char *hint, bool phandle)
+{
+	yaml_event_t event;
+	struct label *l;
+	struct node *root = np;
+	char *ref = NULL;
+
+	while (root->parent)
+		root = root->parent;
+
+	/* If possible, use the hint string as the reference */
+	if (hint && np == get_node_by_ref(root, hint))
+		ref = hint;
+
+	/* Check if the node has a usable label */
+	if (!ref) {
+		for_each_label(np->labels, l) {
+			ref = l->label;
+			break;
+		}
+	}
+
+	if (!ref)
+		ref = np->fullpath;
+
+	yaml_scalar_event_initialize(&event, NULL,
+		(yaml_char_t *)(phandle ? "!phandle" : "!path"), (yaml_char_t*)ref,
+		strlen(ref), 0, 0, YAML_DOUBLE_QUOTED_SCALAR_STYLE);
+	if (!yaml_emitter_emit(emitter, &event))
+		yaml_die(emitter);
+}
+
+static void yaml_propval_int(yaml_emitter_t *emitter, const char *p, int len, int width)
+{
+	yaml_event_t event;
+	const char *end = p + len;
+	char buf[32];
+
+	assert(len % width == 0);
+	for (; p < end; p += width) {
+		switch(width) {
+		case 1:
+			sprintf(buf, "0x%"PRIx8, *(const uint8_t*)p);
+			break;
+		case 2:
+			sprintf(buf, "0x%"PRIx16, fdt16_to_cpu(*(const fdt16_t*)p));
+			break;
+		case 4:
+			sprintf(buf, "0x%"PRIx32, fdt32_to_cpu(*(const fdt32_t*)p));
+			break;
+		case 8:
+			sprintf(buf, "0x%"PRIx64, fdt64_to_cpu(*(const fdt64_t*)p));
+			break;
+		}
+
+		yaml_scalar_event_initialize(&event, NULL,
+			(yaml_char_t*)YAML_INT_TAG, (yaml_char_t *)buf,
+			strlen(buf), 1, 1, YAML_PLAIN_SCALAR_STYLE);
+		if (!yaml_emitter_emit(emitter, &event))
+			yaml_die(emitter);
+	}
+}
+
+static void yaml_propval_string(yaml_emitter_t *emitter, char *str, int len)
+{
+	yaml_event_t event;
+	int i;
+
+	assert(str[len-1] == '\0');
+
+	/* Make sure the entire string is in the lower 7-bit ascii range */
+	for (i = 0; i < len; i++) {
+		if (!isascii(str[i])) {
+			fprintf(stderr, "Warning: non-ASCII character(s) in property string\n");
+			yaml_propval_int(emitter, str, len, 1);
+			return;
+		}
+	}
+
+	yaml_scalar_event_initialize(&event, NULL,
+		(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)str,
+		len-1, 0, 1, YAML_DOUBLE_QUOTED_SCALAR_STYLE);
+	if (!yaml_emitter_emit(emitter, &event))
+		yaml_die(emitter);
+}
+
+static char *group_tags[NUM_MARKERS] = {
+	[MARKER_BLOB] = "!u8",
+	[MARKER_UINT8] = "!u8",
+	[MARKER_UINT16] = "!u16",
+	[MARKER_UINT32] = YAML_SEQ_TAG,
+	[MARKER_UINT16] = "!u64",
+};
+
+static void yaml_propval(yaml_emitter_t *emitter, struct node *root, struct property *prop)
+{
+	yaml_event_t event;
+	int len = prop->val.len;
+	struct marker *m = prop->val.markers;
+	struct marker dummy_marker = {
+		.offset = 0, .type = MARKER_NONE, .next = m, .ref = NULL
+	};
+	enum markertype emit_type = MARKER_NONE;
+	struct node *np;
+	cell_t phandle;
+	unsigned int nesting = 0;
+
+	/* Emit the property name */
+	yaml_scalar_event_initialize(&event, NULL,
+		(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)prop->name,
+		strlen(prop->name), 1, 1, YAML_PLAIN_SCALAR_STYLE);
+	if (!yaml_emitter_emit(emitter, &event))
+		yaml_die(emitter);
+
+	/* Boolean properties are easiest to deal with. Length is zero, so just emit 'true' */
+	if (len == 0) {
+		yaml_scalar_event_initialize(&event, NULL,
+			(yaml_char_t *)YAML_BOOL_TAG,
+			(yaml_char_t*)"true",
+			strlen("true"), 1, 0, YAML_PLAIN_SCALAR_STYLE);
+		if (!yaml_emitter_emit(emitter, &event))
+			yaml_die(emitter);
+		return;
+	}
+
+	dummy_marker.type = guess_propval_type(prop);
+	if (dummy_marker.type != MARKER_NONE)
+		m = &dummy_marker;
+
+	yaml_sequence_start_event_initialize(&event, NULL,
+		(yaml_char_t*)YAML_SEQ_TAG, 1, YAML_FLOW_SEQUENCE_STYLE);
+	if (!yaml_emitter_emit(emitter, &event))
+		yaml_die(emitter);
+	nesting++;
+
+	for_each_marker(m) {
+		size_t chunk_len = (m->next ? m->next->offset : len) - m->offset;
+		char *p = &prop->val.val[m->offset];
+
+		switch (m->type) {
+		case MARKER_NONE:
+			if (!nesting)
+				break;
+			yaml_sequence_end_event_initialize(&event);
+			if (!yaml_emitter_emit(emitter, &event))
+				yaml_die(emitter);
+			emit_type = MARKER_NONE;
+			nesting--;
+			break;
+		case MARKER_STRING:
+			emit_type = m->type;
+			break;
+		case MARKER_BLOB:
+		case MARKER_UINT8:
+		case MARKER_UINT16:
+		case MARKER_UINT32:
+		case MARKER_UINT64:
+			emit_type = m->type;
+			yaml_sequence_start_event_initialize(&event, NULL,
+				(yaml_char_t*)group_tags[emit_type], m->type == MARKER_UINT32, YAML_FLOW_SEQUENCE_STYLE);
+			if (!yaml_emitter_emit(emitter, &event))
+				yaml_die(emitter);
+			nesting++;
+			break;
+		case REF_PHANDLE:
+			assert(emit_type == MARKER_UINT32);
+			assert(chunk_len >= sizeof(fdt32_t));
+			phandle = fdt32_to_cpu(*(const fdt32_t*)p);
+			np = get_node_by_phandle(root, phandle);
+			if (np) {
+				yaml_propval_ref(emitter, np, NULL, true);
+				chunk_len -= sizeof(fdt32_t);
+				p += sizeof(fdt32_t);
+			}
+			break;
+		case REF_PATH:
+			assert(emit_type == MARKER_NONE);
+			assert(strnlen(p, chunk_len) == chunk_len - 1);
+			np = get_node_by_ref(root, p);
+			if (np) {
+				yaml_propval_ref(emitter, np, m->ref, false);
+				p += chunk_len;
+				chunk_len = 0;
+			}
+			break;
+		default:
+			break;
+		}
+
+		if (chunk_len == 0)
+			continue;
+
+		switch (emit_type) {
+			case MARKER_BLOB:
+			case MARKER_UINT8:
+				yaml_propval_int(emitter, p, chunk_len, 1);
+				break;
+			case MARKER_UINT16:
+				yaml_propval_int(emitter, p, chunk_len, 2);
+				break;
+			case MARKER_UINT32:
+				yaml_propval_int(emitter, p, chunk_len, 4);
+				break;
+			case MARKER_UINT64:
+				yaml_propval_int(emitter, p, chunk_len, 8);
+				break;
+			case MARKER_STRING:
+				yaml_propval_string(emitter, p, chunk_len);
+				break;
+			default:
+				break;
+		}
+	}
+
+	while (nesting) {
+		yaml_sequence_end_event_initialize(&event);
+		if (!yaml_emitter_emit(emitter, &event))
+			yaml_die(emitter);
+		nesting--;
+	}
+}
+
+
+static void yaml_tree(struct node *root, struct node *tree, yaml_emitter_t *emitter)
+{
+	struct property *prop;
+	struct node *child;
+	struct label *l;
+	yaml_event_t event;
+
+	if (tree->deleted)
+		return;
+
+	yaml_mapping_start_event_initialize(&event, NULL,
+		(yaml_char_t *)YAML_MAP_TAG, 1, YAML_ANY_MAPPING_STYLE);
+	if (!yaml_emitter_emit(emitter, &event))
+		yaml_die(emitter);
+
+	if (tree->labels) {
+		yaml_scalar_event_initialize(&event, NULL,
+			(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)"$labels",
+			strlen("$labels"), 1, 0, YAML_PLAIN_SCALAR_STYLE);
+		if (!yaml_emitter_emit(emitter, &event))
+			yaml_die(emitter);
+
+		yaml_sequence_start_event_initialize(&event, NULL,
+			(yaml_char_t*)YAML_SEQ_TAG, 1, YAML_FLOW_SEQUENCE_STYLE);
+		if (!yaml_emitter_emit(emitter, &event))
+			yaml_die(emitter);
+
+		for_each_label(tree->labels, l) {
+			yaml_scalar_event_initialize(&event, NULL,
+				(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)l->label,
+				strlen(l->label), 1, 0, YAML_PLAIN_SCALAR_STYLE);
+			if (!yaml_emitter_emit(emitter, &event))
+				yaml_die(emitter);
+		}
+
+		yaml_sequence_end_event_initialize(&event);
+		if (!yaml_emitter_emit(emitter, &event))
+			yaml_die(emitter);
+	}
+
+	for_each_property(tree, prop)
+		yaml_propval(emitter, root, prop);
+
+	/* Loop over all the children, emitting them into the map */
+	for_each_child(tree, child) {
+		yaml_scalar_event_initialize(&event, NULL,
+			(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)child->name,
+			strlen(child->name), 1, 0, YAML_PLAIN_SCALAR_STYLE);
+		if (!yaml_emitter_emit(emitter, &event))
+			yaml_die(emitter);
+		yaml_tree(root, child, emitter);
+	}
+
+	yaml_mapping_end_event_initialize(&event);
+	if (!yaml_emitter_emit(emitter, &event))
+		yaml_die(emitter);
+}
+
+void dt_to_yaml(FILE *f, struct dt_info *dti)
+{
+	yaml_emitter_t emitter;
+	yaml_event_t event;
+
+	yaml_emitter_initialize(&emitter);
+	yaml_emitter_set_output_file(&emitter, f);
+	yaml_stream_start_event_initialize(&event, YAML_UTF8_ENCODING);
+	if (!yaml_emitter_emit(&emitter, &event))
+		yaml_die(&emitter);
+
+	yaml_document_start_event_initialize(&event, NULL, NULL, NULL, 0);
+	if (!yaml_emitter_emit(&emitter, &event))
+		yaml_die(&emitter);
+
+	yaml_sequence_start_event_initialize(&event, NULL, (yaml_char_t *)YAML_SEQ_TAG, 1, YAML_ANY_SEQUENCE_STYLE);
+	if (!yaml_emitter_emit(&emitter, &event))
+		yaml_die(&emitter);
+
+	yaml_tree(dti->dt, dti->dt, &emitter);
+
+	yaml_sequence_end_event_initialize(&event);
+	if (!yaml_emitter_emit(&emitter, &event))
+		yaml_die(&emitter);
+
+	yaml_document_end_event_initialize(&event, 0);
+	if (!yaml_emitter_emit(&emitter, &event))
+		yaml_die(&emitter);
+
+	yaml_stream_end_event_initialize(&event);
+	if (!yaml_emitter_emit(&emitter, &event))
+		yaml_die(&emitter);
+
+	yaml_emitter_delete(&emitter);
+}
+
-- 
2.11.0

Re: [RFC PATCH 1/2] Preserve datatype information when parsing dts

[David Gibson]

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

On Tue, Nov 28, 2017 at 11:57:09AM +0000, Grant Likely wrote:
> The current code throws away all the data type and grouping information
> when parsing the DTS source file, which makes it difficult to
> reconstruct the data format when emitting a format that can express data
> types (ie. dts and yaml). Use the marker list to mark the beginning and
> end of each integer array block (<> and []), the datatype contained in
> each (8, 16, 32 & 64 bit widths), and the start of each string.
> 
> At the same time, factor out the heuristic code used to guess a property
> type at emit time. It is a pretty well defined standalone block that
> could be used elsewhere, for instance, when emitting YAML source. Factor
> it out into a separate function so that it can be reused, and also to
> simplify the write_propval() function.
> 
> When emitting, group integer output back into the same groups as the
> original source and use the REF_PATH and REF_PHANDLE markers to emit the
> the node reference instead of a raw path or phandle.
> 
> Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>

I'm a bit dubious how well forcing the marker mechanism to do all this
stuff it was never intended for can work in the long term.  Still,
it's an interesting experiment.

> ---
>  data.c       |   4 +-
>  dtc-parser.y |  21 +++--
>  dtc.h        |   9 +++
>  livetree.c   |  49 ++++++++++++
>  treesource.c | 249 ++++++++++++++++++++++++++++++++---------------------------
>  5 files changed, 212 insertions(+), 120 deletions(-)
> 
> diff --git a/data.c b/data.c
> index aa37a16..0aef0b5 100644
> --- a/data.c
> +++ b/data.c
> @@ -74,7 +74,8 @@ struct data data_copy_escape_string(const char *s, int len)
>  	struct data d;
>  	char *q;
>  
> -	d = data_grow_for(empty_data, len + 1);
> +	d = data_add_marker(empty_data, MARKER_STRING, NULL);
> +	d = data_grow_for(d, len + 1);
>  
>  	q = d.val;
>  	while (i < len) {
> @@ -94,6 +95,7 @@ struct data data_copy_file(FILE *f, size_t maxlen)
>  {
>  	struct data d = empty_data;
>  
> +	d = data_add_marker(d, MARKER_BLOB, NULL);
>  	while (!feof(f) && (d.len < maxlen)) {
>  		size_t chunksize, ret;
>  
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 44af170..41fbd3f 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -266,11 +266,13 @@ propdata:
>  		}
>  	| propdataprefix arrayprefix '>'
>  		{
> -			$$ = data_merge($1, $2.data);
> +			$1 = data_merge($1, $2.data);
> +			$$ = data_add_marker($1, MARKER_NONE, NULL);
>  		}
>  	| propdataprefix '[' bytestring ']'
>  		{
> -			$$ = data_merge($1, $3);
> +			$1 = data_merge($1, $3);
> +			$$ = data_add_marker($1, MARKER_NONE, NULL);
>  		}
>  	| propdataprefix DT_REF
>  		{
> @@ -327,22 +329,27 @@ arrayprefix:
>  	DT_BITS DT_LITERAL '<'
>  		{
>  			unsigned long long bits;
> +			enum markertype type = MARKER_UINT32;
>  
>  			bits = $2;
>  
> -			if ((bits !=  8) && (bits != 16) &&
> -			    (bits != 32) && (bits != 64)) {
> +			switch (bits) {
> +			case 8: type = MARKER_UINT8; break;
> +			case 16: type = MARKER_UINT16; break;
> +			case 32: type = MARKER_UINT32; break;
> +			case 64: type = MARKER_UINT64; break;
> +			default:
>  				ERROR(&@2, "Array elements must be"
>  				      " 8, 16, 32 or 64-bits");
>  				bits = 32;
>  			}
>  
> -			$$.data = empty_data;
> +			$$.data = data_add_marker(empty_data, type, NULL);
>  			$$.bits = bits;
>  		}
>  	| '<'
>  		{
> -			$$.data = empty_data;
> +			$$.data = data_add_marker(empty_data, MARKER_UINT32, NULL);
>  			$$.bits = 32;
>  		}
>  	| arrayprefix integer_prim
> @@ -486,7 +493,7 @@ integer_unary:
>  bytestring:
>  	  /* empty */
>  		{
> -			$$ = empty_data;
> +			$$ = data_add_marker(empty_data, MARKER_UINT8, NULL);
>  		}
>  	| bytestring DT_BYTE
>  		{
> diff --git a/dtc.h b/dtc.h
> index 3b18a42..32a7655 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -74,9 +74,17 @@ typedef uint32_t cell_t;
>  
>  /* Data blobs */
>  enum markertype {
> +	MARKER_NONE = 0,
>  	REF_PHANDLE,
>  	REF_PATH,
>  	LABEL,
> +	MARKER_UINT8,
> +	MARKER_UINT16,
> +	MARKER_UINT32,
> +	MARKER_UINT64,
> +	MARKER_BLOB,
> +	MARKER_STRING,
> +	NUM_MARKERS,
>  };
>  
>  struct  marker {
> @@ -198,6 +206,7 @@ struct property *build_property(char *name, struct data val);
>  struct property *build_property_delete(char *name);
>  struct property *chain_property(struct property *first, struct property *list);
>  struct property *reverse_properties(struct property *first);
> +enum markertype guess_propval_type(struct property *prop);
>  
>  struct node *build_node(struct property *proplist, struct node *children);
>  struct node *build_node_delete(void);
> diff --git a/livetree.c b/livetree.c
> index 57b7db2..1bbdaf8 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -336,6 +336,55 @@ void append_to_property(struct node *node,
>  	}
>  }
>  
> +static bool isstring(char c)
> +{
> +	return (isprint((unsigned char)c)
> +		|| (c == '\0')
> +		|| strchr("\a\b\t\n\v\f\r", c));
> +}
> +
> +enum markertype guess_propval_type(struct property *prop)
> +{
> +	int len = prop->val.len;
> +	const char *p = prop->val.val;
> +	struct marker *m = prop->val.markers;
> +	int nnotstring = 0, nnul = 0;
> +	int nnotstringlbl = 0, nnotcelllbl = 0;
> +	int i;
> +
> +	/* Check if the property is type annotated */
> +	while (m && (m->type == LABEL || m->type == REF_PHANDLE))
> +		m = m->next;
> +	if (m && m->offset == 0)
> +		return MARKER_NONE;
> +	m = prop->val.markers;
> +
> +	/* data type information missing, need to guess */
> +	for (i = 0; i < len; i++) {
> +		if (! isstring(p[i]))
> +			nnotstring++;
> +		if (p[i] == '\0')
> +			nnul++;
> +	}
> +
> +	for_each_marker_of_type(m, LABEL) {
> +		if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
> +			nnotstringlbl++;
> +		if ((m->offset % sizeof(cell_t)) != 0)
> +			nnotcelllbl++;
> +	}
> +
> +	if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
> +	    && (nnotstringlbl == 0)) {
> +		return MARKER_STRING;
> +	} else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
> +		return MARKER_UINT32;
> +	} else {
> +		return MARKER_UINT8;
> +	}
> +}
> +
>  struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
>  {
>  	struct reserve_info *new = xmalloc(sizeof(*new));
> diff --git a/treesource.c b/treesource.c
> index 2461a3d..cc3b223 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -54,29 +54,15 @@ static void write_prefix(FILE *f, int level)
>  		fputc('\t', f);
>  }
>  
> -static bool isstring(char c)
> +static void write_propval_string(FILE *f, const char *str, size_t len)
>  {
> -	return (isprint((unsigned char)c)
> -		|| (c == '\0')
> -		|| strchr("\a\b\t\n\v\f\r", c));
> -}
> -
> -static void write_propval_string(FILE *f, struct data val)
> -{
> -	const char *str = val.val;
>  	int i;
> -	struct marker *m = val.markers;
>  
> -	assert(str[val.len-1] == '\0');
> +	assert(str[len-1] == '\0');
>  
> -	while (m && (m->offset == 0)) {
> -		if (m->type == LABEL)
> -			fprintf(f, "%s: ", m->ref);
> -		m = m->next;
> -	}
>  	fprintf(f, "\"");
>  
> -	for (i = 0; i < (val.len-1); i++) {
> +	for (i = 0; i < (len-1); i++) {
>  		char c = str[i];
>  
>  		switch (c) {
> @@ -108,15 +94,7 @@ static void write_propval_string(FILE *f, struct data val)
>  			fprintf(f, "\\\"");
>  			break;
>  		case '\0':
> -			fprintf(f, "\", ");
> -			while (m && (m->offset <= (i + 1))) {
> -				if (m->type == LABEL) {
> -					assert(m->offset == (i+1));
> -					fprintf(f, "%s: ", m->ref);
> -				}
> -				m = m->next;
> -			}
> -			fprintf(f, "\"");
> +			fprintf(f, "\", \"");
>  			break;
>  		default:
>  			if (isprint((unsigned char)c))
> @@ -126,114 +104,161 @@ static void write_propval_string(FILE *f, struct data val)
>  		}
>  	}
>  	fprintf(f, "\"");
> -
> -	/* Wrap up any labels at the end of the value */
> -	for_each_marker_of_type(m, LABEL) {
> -		assert (m->offset == val.len);
> -		fprintf(f, " %s:", m->ref);
> -	}
>  }
>  
> -static void write_propval_cells(FILE *f, struct data val)
> +static void write_propval_ref(FILE *f, struct node *np, const char *hint)
>  {
> -	void *propend = val.val + val.len;
> -	fdt32_t *cp = (fdt32_t *)val.val;
> -	struct marker *m = val.markers;
> -
> -	fprintf(f, "<");
> -	for (;;) {
> -		while (m && (m->offset <= ((char *)cp - val.val))) {
> -			if (m->type == LABEL) {
> -				assert(m->offset == ((char *)cp - val.val));
> -				fprintf(f, "%s: ", m->ref);
> -			}
> -			m = m->next;
> -		}
> +	struct label *l;
> +	struct node *root = np;
>  
> -		fprintf(f, "0x%x", fdt32_to_cpu(*cp++));
> -		if ((void *)cp >= propend)
> -			break;
> -		fprintf(f, " ");
> -	}
> +	while (root->parent)
> +		root = root->parent;
>  
> -	/* Wrap up any labels at the end of the value */
> -	for_each_marker_of_type(m, LABEL) {
> -		assert (m->offset == val.len);
> -		fprintf(f, " %s:", m->ref);
> +	/* If possible, use the hint string as the reference */
> +	if (hint && np == get_node_by_ref(root, hint)) {
> +		fprintf(f, hint[0] == '/' ? " &{%s}" : " &%s", hint);
> +		return;
>  	}
> -	fprintf(f, ">");
> -}
> -
> -static void write_propval_bytes(FILE *f, struct data val)
> -{
> -	void *propend = val.val + val.len;
> -	const char *bp = val.val;
> -	struct marker *m = val.markers;
> -
> -	fprintf(f, "[");
> -	for (;;) {
> -		while (m && (m->offset == (bp-val.val))) {
> -			if (m->type == LABEL)
> -				fprintf(f, "%s: ", m->ref);
> -			m = m->next;
> -		}
>  
> -		fprintf(f, "%02hhx", (unsigned char)(*bp++));
> -		if ((const void *)bp >= propend)
> -			break;
> -		fprintf(f, " ");
> +	/* Check if the node has a usable label */
> +	for_each_label(np->labels, l) {
> +		fprintf(f, " &%s", l->label);
> +		return;
>  	}
>  
> -	/* Wrap up any labels at the end of the value */
> -	for_each_marker_of_type(m, LABEL) {
> -		assert (m->offset == val.len);
> -		fprintf(f, " %s:", m->ref);
> -	}
> -	fprintf(f, "]");
> +	fprintf(f, " &{%s}", np->fullpath);
>  }
>  
> -static void write_propval(FILE *f, struct property *prop)
> +const char * delim_start[NUM_MARKERS] = {
> +	[MARKER_BLOB] = "[",
> +	[MARKER_UINT8] = "[",
> +	[MARKER_UINT16] = "/bits/ 16 <",
> +	[MARKER_UINT32] = "<",
> +	[MARKER_UINT64] = "/bits/ 64 <",
> +};
> +const char * delim_end[NUM_MARKERS] = {
> +	[MARKER_BLOB] = " ]",
> +	[MARKER_UINT8] = " ]",
> +	[MARKER_UINT16] = " >",
> +	[MARKER_UINT32] = " >",
> +	[MARKER_UINT64] = " >",
> +};
> +static void write_propval(FILE *f, struct node *root, struct property *prop)
>  {
> -	int len = prop->val.len;
> -	const char *p = prop->val.val;
> +	size_t len = prop->val.len;
> +	size_t chunk_len;
>  	struct marker *m = prop->val.markers;
> -	int nnotstring = 0, nnul = 0;
> -	int nnotstringlbl = 0, nnotcelllbl = 0;
> -	int i;
> +	struct marker dummy_marker = {
> +		.offset = 0, .type = MARKER_NONE, .next = m, .ref = NULL
> +	};
> +	enum markertype emit_type = MARKER_NONE;
> +	struct node *np;
> +	cell_t phandle;
>  
>  	if (len == 0) {
>  		fprintf(f, ";\n");
>  		return;
>  	}
>  
> -	for (i = 0; i < len; i++) {
> -		if (! isstring(p[i]))
> -			nnotstring++;
> -		if (p[i] == '\0')
> -			nnul++;
> -	}
> +	fprintf(f, " = ");
>  
> -	for_each_marker_of_type(m, LABEL) {
> -		if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
> -			nnotstringlbl++;
> -		if ((m->offset % sizeof(cell_t)) != 0)
> -			nnotcelllbl++;
> -	}
> +	dummy_marker.type = guess_propval_type(prop);
> +	if (dummy_marker.type != MARKER_NONE)
> +		m = &dummy_marker;
>  
> -	fprintf(f, " = ");
> -	if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
> -	    && (nnotstringlbl == 0)) {
> -		write_propval_string(f, prop->val);
> -	} else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
> -		write_propval_cells(f, prop->val);
> -	} else {
> -		write_propval_bytes(f, prop->val);
> +	for_each_marker(m) {
> +		chunk_len = (m->next ? m->next->offset : len) - m->offset;
> +		const char *p = &prop->val.val[m->offset];
> +		const char *end = p + chunk_len;
> +
> +		switch(m->type) {
> +		case MARKER_NONE:
> +			if (emit_type != MARKER_NONE)
> +				fprintf(f, "%s", delim_end[emit_type]);
> +			emit_type = m->type;
> +			break;
> +		case MARKER_STRING:
> +		case MARKER_BLOB:
> +		case MARKER_UINT8:
> +		case MARKER_UINT16:
> +		case MARKER_UINT32:
> +		case MARKER_UINT64:
> +			emit_type = m->type;
> +			fprintf(f, m->offset ? ", %s" : "%s",
> +			           delim_start[emit_type] ? : "");
> +			break;
> +		case LABEL:
> +			fprintf(f, " %s:", m->ref);
> +			break;
> +		case REF_PHANDLE:
> +			assert(emit_type == MARKER_UINT32);
> +			assert(chunk_len >= sizeof(fdt32_t));
> +			phandle = fdt32_to_cpu(*(const fdt32_t*)p);
> +			np = get_node_by_phandle(root, phandle);
> +			if (np) {
> +				write_propval_ref(f, np, NULL);
> +				chunk_len -= sizeof(fdt32_t);
> +				p += sizeof(fdt32_t);
> +			}
> +			break;
> +		case REF_PATH:
> +			assert(emit_type == MARKER_NONE);
> +			assert(strnlen(p, chunk_len) == chunk_len - 1);
> +			np = get_node_by_ref(root, p);
> +			if (np) {
> +				write_propval_ref(f, np, m->ref);
> +				p += chunk_len;
> +				chunk_len = 0;
> +			}
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (chunk_len <= 0)
> +			continue;
> +
> +		switch (emit_type) {
> +		case MARKER_BLOB:
> +		case MARKER_UINT8:
> +			for (; p < end; p++)
> +				fprintf(f, " %02"PRIx8, *(const uint8_t *)p);
> +			break;
> +		case MARKER_UINT16:
> +			assert((chunk_len % sizeof(fdt16_t)) == 0);
> +			for (; p < end; p += sizeof(fdt16_t))
> +				fprintf(f, " 0x%02"PRIx16,
> +				        fdt16_to_cpu(*(const fdt16_t*)p));
> +			break;
> +		case MARKER_UINT32:
> +			assert((chunk_len % sizeof(fdt32_t)) == 0);
> +			for (; p < end; p += sizeof(fdt32_t))
> +				fprintf(f, " 0x%02"PRIx32,
> +				        fdt32_to_cpu(*(const fdt32_t*)p));
> +			break;
> +		case MARKER_UINT64:
> +			assert((chunk_len % sizeof(fdt64_t)) == 0);
> +			for (; p < end; p += sizeof(fdt64_t))
> +				fprintf(f, " 0x%02"PRIx64,
> +				        fdt64_to_cpu(*(const fdt64_t*)p));
> +			break;
> +		case MARKER_STRING:
> +			assert(p[chunk_len-1] == '\0');
> +			write_propval_string(f, p, chunk_len);
> +			break;
> +		default:
> +			/* Default to a block of raw bytes */
> +			fprintf(f, "[ ");
> +			for (; p < end; p++)
> +				fprintf(f, " %02"PRIx8, *p);
> +			fprintf(f, "]");
> +		}
>  	}
>  
> -	fprintf(f, ";\n");
> +	fprintf(f, "%s;\n", delim_end[emit_type] ? : "");
>  }
>  
> -static void write_tree_source_node(FILE *f, struct node *tree, int level)
> +static void write_tree_source_node(FILE *f, struct node *root, struct node *tree, int level)
>  {
>  	struct property *prop;
>  	struct node *child;
> @@ -252,11 +277,11 @@ static void write_tree_source_node(FILE *f, struct node *tree, int level)
>  		for_each_label(prop->labels, l)
>  			fprintf(f, "%s: ", l->label);
>  		fprintf(f, "%s", prop->name);
> -		write_propval(f, prop);
> +		write_propval(f, root, prop);
>  	}
>  	for_each_child(tree, child) {
>  		fprintf(f, "\n");
> -		write_tree_source_node(f, child, level+1);
> +		write_tree_source_node(f, root, child, level+1);
>  	}
>  	write_prefix(f, level);
>  	fprintf(f, "};\n");
> @@ -279,6 +304,6 @@ void dt_to_source(FILE *f, struct dt_info *dti)
>  			(unsigned long long)re->size);
>  	}
>  
> -	write_tree_source_node(f, dti->dt, 0);
> +	write_tree_source_node(f, dti->dt, dti->dt, 0);
>  }
>  

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH 1/2] Preserve datatype information when parsing dts

[Grant Likely]

On Tue, Dec 12, 2017 at 6:14 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Tue, Nov 28, 2017 at 11:57:09AM +0000, Grant Likely wrote:
>> The current code throws away all the data type and grouping information
>> when parsing the DTS source file, which makes it difficult to
>> reconstruct the data format when emitting a format that can express data
>> types (ie. dts and yaml). Use the marker list to mark the beginning and
>> end of each integer array block (<> and []), the datatype contained in
>> each (8, 16, 32 & 64 bit widths), and the start of each string.
>>
>> At the same time, factor out the heuristic code used to guess a property
>> type at emit time. It is a pretty well defined standalone block that
>> could be used elsewhere, for instance, when emitting YAML source. Factor
>> it out into a separate function so that it can be reused, and also to
>> simplify the write_propval() function.
>>
>> When emitting, group integer output back into the same groups as the
>> original source and use the REF_PATH and REF_PHANDLE markers to emit the
>> the node reference instead of a raw path or phandle.
>>
>> Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
>
> I'm a bit dubious how well forcing the marker mechanism to do all this
> stuff it was never intended for can work in the long term.  Still,
> it's an interesting experiment.

As long as the actual data is stored as flat buffer, the markers
mechanism works quite well for this. I tried doing something entirely
separate, and it turned out to be awful. Another alternative is to
break up the flat buffer into a chain of data blocks with attached
type information, but that is a very invasive change.

This approach has the advantage of being robust on accepting both
typed and anonymous data. If the markers are not there then the
existing behaviour can be maintained, but otherwise it can emit a
higher fidelity of source language.

Cheers,
g.

>
>> ---
>>  data.c       |   4 +-
>>  dtc-parser.y |  21 +++--
>>  dtc.h        |   9 +++
>>  livetree.c   |  49 ++++++++++++
>>  treesource.c | 249 ++++++++++++++++++++++++++++++++---------------------------
>>  5 files changed, 212 insertions(+), 120 deletions(-)
>>
>> diff --git a/data.c b/data.c
>> index aa37a16..0aef0b5 100644
>> --- a/data.c
>> +++ b/data.c
>> @@ -74,7 +74,8 @@ struct data data_copy_escape_string(const char *s, int len)
>>       struct data d;
>>       char *q;
>>
>> -     d = data_grow_for(empty_data, len + 1);
>> +     d = data_add_marker(empty_data, MARKER_STRING, NULL);
>> +     d = data_grow_for(d, len + 1);
>>
>>       q = d.val;
>>       while (i < len) {
>> @@ -94,6 +95,7 @@ struct data data_copy_file(FILE *f, size_t maxlen)
>>  {
>>       struct data d = empty_data;
>>
>> +     d = data_add_marker(d, MARKER_BLOB, NULL);
>>       while (!feof(f) && (d.len < maxlen)) {
>>               size_t chunksize, ret;
>>
>> diff --git a/dtc-parser.y b/dtc-parser.y
>> index 44af170..41fbd3f 100644
>> --- a/dtc-parser.y
>> +++ b/dtc-parser.y
>> @@ -266,11 +266,13 @@ propdata:
>>               }
>>       | propdataprefix arrayprefix '>'
>>               {
>> -                     $$ = data_merge($1, $2.data);
>> +                     $1 = data_merge($1, $2.data);
>> +                     $$ = data_add_marker($1, MARKER_NONE, NULL);
>>               }
>>       | propdataprefix '[' bytestring ']'
>>               {
>> -                     $$ = data_merge($1, $3);
>> +                     $1 = data_merge($1, $3);
>> +                     $$ = data_add_marker($1, MARKER_NONE, NULL);
>>               }
>>       | propdataprefix DT_REF
>>               {
>> @@ -327,22 +329,27 @@ arrayprefix:
>>       DT_BITS DT_LITERAL '<'
>>               {
>>                       unsigned long long bits;
>> +                     enum markertype type = MARKER_UINT32;
>>
>>                       bits = $2;
>>
>> -                     if ((bits !=  8) && (bits != 16) &&
>> -                         (bits != 32) && (bits != 64)) {
>> +                     switch (bits) {
>> +                     case 8: type = MARKER_UINT8; break;
>> +                     case 16: type = MARKER_UINT16; break;
>> +                     case 32: type = MARKER_UINT32; break;
>> +                     case 64: type = MARKER_UINT64; break;
>> +                     default:
>>                               ERROR(&@2, "Array elements must be"
>>                                     " 8, 16, 32 or 64-bits");
>>                               bits = 32;
>>                       }
>>
>> -                     $$.data = empty_data;
>> +                     $$.data = data_add_marker(empty_data, type, NULL);
>>                       $$.bits = bits;
>>               }
>>       | '<'
>>               {
>> -                     $$.data = empty_data;
>> +                     $$.data = data_add_marker(empty_data, MARKER_UINT32, NULL);
>>                       $$.bits = 32;
>>               }
>>       | arrayprefix integer_prim
>> @@ -486,7 +493,7 @@ integer_unary:
>>  bytestring:
>>         /* empty */
>>               {
>> -                     $$ = empty_data;
>> +                     $$ = data_add_marker(empty_data, MARKER_UINT8, NULL);
>>               }
>>       | bytestring DT_BYTE
>>               {
>> diff --git a/dtc.h b/dtc.h
>> index 3b18a42..32a7655 100644
>> --- a/dtc.h
>> +++ b/dtc.h
>> @@ -74,9 +74,17 @@ typedef uint32_t cell_t;
>>
>>  /* Data blobs */
>>  enum markertype {
>> +     MARKER_NONE = 0,
>>       REF_PHANDLE,
>>       REF_PATH,
>>       LABEL,
>> +     MARKER_UINT8,
>> +     MARKER_UINT16,
>> +     MARKER_UINT32,
>> +     MARKER_UINT64,
>> +     MARKER_BLOB,
>> +     MARKER_STRING,
>> +     NUM_MARKERS,
>>  };
>>
>>  struct  marker {
>> @@ -198,6 +206,7 @@ struct property *build_property(char *name, struct data val);
>>  struct property *build_property_delete(char *name);
>>  struct property *chain_property(struct property *first, struct property *list);
>>  struct property *reverse_properties(struct property *first);
>> +enum markertype guess_propval_type(struct property *prop);
>>
>>  struct node *build_node(struct property *proplist, struct node *children);
>>  struct node *build_node_delete(void);
>> diff --git a/livetree.c b/livetree.c
>> index 57b7db2..1bbdaf8 100644
>> --- a/livetree.c
>> +++ b/livetree.c
>> @@ -336,6 +336,55 @@ void append_to_property(struct node *node,
>>       }
>>  }
>>
>> +static bool isstring(char c)
>> +{
>> +     return (isprint((unsigned char)c)
>> +             || (c == '\0')
>> +             || strchr("\a\b\t\n\v\f\r", c));
>> +}
>> +
>> +enum markertype guess_propval_type(struct property *prop)
>> +{
>> +     int len = prop->val.len;
>> +     const char *p = prop->val.val;
>> +     struct marker *m = prop->val.markers;
>> +     int nnotstring = 0, nnul = 0;
>> +     int nnotstringlbl = 0, nnotcelllbl = 0;
>> +     int i;
>> +
>> +     /* Check if the property is type annotated */
>> +     while (m && (m->type == LABEL || m->type == REF_PHANDLE))
>> +             m = m->next;
>> +     if (m && m->offset == 0)
>> +             return MARKER_NONE;
>> +     m = prop->val.markers;
>> +
>> +     /* data type information missing, need to guess */
>> +     for (i = 0; i < len; i++) {
>> +             if (! isstring(p[i]))
>> +                     nnotstring++;
>> +             if (p[i] == '\0')
>> +                     nnul++;
>> +     }
>> +
>> +     for_each_marker_of_type(m, LABEL) {
>> +             if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
>> +                     nnotstringlbl++;
>> +             if ((m->offset % sizeof(cell_t)) != 0)
>> +                     nnotcelllbl++;
>> +     }
>> +
>> +     if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
>> +         && (nnotstringlbl == 0)) {
>> +             return MARKER_STRING;
>> +     } else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
>> +             return MARKER_UINT32;
>> +     } else {
>> +             return MARKER_UINT8;
>> +     }
>> +}
>> +
>>  struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
>>  {
>>       struct reserve_info *new = xmalloc(sizeof(*new));
>> diff --git a/treesource.c b/treesource.c
>> index 2461a3d..cc3b223 100644
>> --- a/treesource.c
>> +++ b/treesource.c
>> @@ -54,29 +54,15 @@ static void write_prefix(FILE *f, int level)
>>               fputc('\t', f);
>>  }
>>
>> -static bool isstring(char c)
>> +static void write_propval_string(FILE *f, const char *str, size_t len)
>>  {
>> -     return (isprint((unsigned char)c)
>> -             || (c == '\0')
>> -             || strchr("\a\b\t\n\v\f\r", c));
>> -}
>> -
>> -static void write_propval_string(FILE *f, struct data val)
>> -{
>> -     const char *str = val.val;
>>       int i;
>> -     struct marker *m = val.markers;
>>
>> -     assert(str[val.len-1] == '\0');
>> +     assert(str[len-1] == '\0');
>>
>> -     while (m && (m->offset == 0)) {
>> -             if (m->type == LABEL)
>> -                     fprintf(f, "%s: ", m->ref);
>> -             m = m->next;
>> -     }
>>       fprintf(f, "\"");
>>
>> -     for (i = 0; i < (val.len-1); i++) {
>> +     for (i = 0; i < (len-1); i++) {
>>               char c = str[i];
>>
>>               switch (c) {
>> @@ -108,15 +94,7 @@ static void write_propval_string(FILE *f, struct data val)
>>                       fprintf(f, "\\\"");
>>                       break;
>>               case '\0':
>> -                     fprintf(f, "\", ");
>> -                     while (m && (m->offset <= (i + 1))) {
>> -                             if (m->type == LABEL) {
>> -                                     assert(m->offset == (i+1));
>> -                                     fprintf(f, "%s: ", m->ref);
>> -                             }
>> -                             m = m->next;
>> -                     }
>> -                     fprintf(f, "\"");
>> +                     fprintf(f, "\", \"");
>>                       break;
>>               default:
>>                       if (isprint((unsigned char)c))
>> @@ -126,114 +104,161 @@ static void write_propval_string(FILE *f, struct data val)
>>               }
>>       }
>>       fprintf(f, "\"");
>> -
>> -     /* Wrap up any labels at the end of the value */
>> -     for_each_marker_of_type(m, LABEL) {
>> -             assert (m->offset == val.len);
>> -             fprintf(f, " %s:", m->ref);
>> -     }
>>  }
>>
>> -static void write_propval_cells(FILE *f, struct data val)
>> +static void write_propval_ref(FILE *f, struct node *np, const char *hint)
>>  {
>> -     void *propend = val.val + val.len;
>> -     fdt32_t *cp = (fdt32_t *)val.val;
>> -     struct marker *m = val.markers;
>> -
>> -     fprintf(f, "<");
>> -     for (;;) {
>> -             while (m && (m->offset <= ((char *)cp - val.val))) {
>> -                     if (m->type == LABEL) {
>> -                             assert(m->offset == ((char *)cp - val.val));
>> -                             fprintf(f, "%s: ", m->ref);
>> -                     }
>> -                     m = m->next;
>> -             }
>> +     struct label *l;
>> +     struct node *root = np;
>>
>> -             fprintf(f, "0x%x", fdt32_to_cpu(*cp++));
>> -             if ((void *)cp >= propend)
>> -                     break;
>> -             fprintf(f, " ");
>> -     }
>> +     while (root->parent)
>> +             root = root->parent;
>>
>> -     /* Wrap up any labels at the end of the value */
>> -     for_each_marker_of_type(m, LABEL) {
>> -             assert (m->offset == val.len);
>> -             fprintf(f, " %s:", m->ref);
>> +     /* If possible, use the hint string as the reference */
>> +     if (hint && np == get_node_by_ref(root, hint)) {
>> +             fprintf(f, hint[0] == '/' ? " &{%s}" : " &%s", hint);
>> +             return;
>>       }
>> -     fprintf(f, ">");
>> -}
>> -
>> -static void write_propval_bytes(FILE *f, struct data val)
>> -{
>> -     void *propend = val.val + val.len;
>> -     const char *bp = val.val;
>> -     struct marker *m = val.markers;
>> -
>> -     fprintf(f, "[");
>> -     for (;;) {
>> -             while (m && (m->offset == (bp-val.val))) {
>> -                     if (m->type == LABEL)
>> -                             fprintf(f, "%s: ", m->ref);
>> -                     m = m->next;
>> -             }
>>
>> -             fprintf(f, "%02hhx", (unsigned char)(*bp++));
>> -             if ((const void *)bp >= propend)
>> -                     break;
>> -             fprintf(f, " ");
>> +     /* Check if the node has a usable label */
>> +     for_each_label(np->labels, l) {
>> +             fprintf(f, " &%s", l->label);
>> +             return;
>>       }
>>
>> -     /* Wrap up any labels at the end of the value */
>> -     for_each_marker_of_type(m, LABEL) {
>> -             assert (m->offset == val.len);
>> -             fprintf(f, " %s:", m->ref);
>> -     }
>> -     fprintf(f, "]");
>> +     fprintf(f, " &{%s}", np->fullpath);
>>  }
>>
>> -static void write_propval(FILE *f, struct property *prop)
>> +const char * delim_start[NUM_MARKERS] = {
>> +     [MARKER_BLOB] = "[",
>> +     [MARKER_UINT8] = "[",
>> +     [MARKER_UINT16] = "/bits/ 16 <",
>> +     [MARKER_UINT32] = "<",
>> +     [MARKER_UINT64] = "/bits/ 64 <",
>> +};
>> +const char * delim_end[NUM_MARKERS] = {
>> +     [MARKER_BLOB] = " ]",
>> +     [MARKER_UINT8] = " ]",
>> +     [MARKER_UINT16] = " >",
>> +     [MARKER_UINT32] = " >",
>> +     [MARKER_UINT64] = " >",
>> +};
>> +static void write_propval(FILE *f, struct node *root, struct property *prop)
>>  {
>> -     int len = prop->val.len;
>> -     const char *p = prop->val.val;
>> +     size_t len = prop->val.len;
>> +     size_t chunk_len;
>>       struct marker *m = prop->val.markers;
>> -     int nnotstring = 0, nnul = 0;
>> -     int nnotstringlbl = 0, nnotcelllbl = 0;
>> -     int i;
>> +     struct marker dummy_marker = {
>> +             .offset = 0, .type = MARKER_NONE, .next = m, .ref = NULL
>> +     };
>> +     enum markertype emit_type = MARKER_NONE;
>> +     struct node *np;
>> +     cell_t phandle;
>>
>>       if (len == 0) {
>>               fprintf(f, ";\n");
>>               return;
>>       }
>>
>> -     for (i = 0; i < len; i++) {
>> -             if (! isstring(p[i]))
>> -                     nnotstring++;
>> -             if (p[i] == '\0')
>> -                     nnul++;
>> -     }
>> +     fprintf(f, " = ");
>>
>> -     for_each_marker_of_type(m, LABEL) {
>> -             if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
>> -                     nnotstringlbl++;
>> -             if ((m->offset % sizeof(cell_t)) != 0)
>> -                     nnotcelllbl++;
>> -     }
>> +     dummy_marker.type = guess_propval_type(prop);
>> +     if (dummy_marker.type != MARKER_NONE)
>> +             m = &dummy_marker;
>>
>> -     fprintf(f, " = ");
>> -     if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
>> -         && (nnotstringlbl == 0)) {
>> -             write_propval_string(f, prop->val);
>> -     } else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
>> -             write_propval_cells(f, prop->val);
>> -     } else {
>> -             write_propval_bytes(f, prop->val);
>> +     for_each_marker(m) {
>> +             chunk_len = (m->next ? m->next->offset : len) - m->offset;
>> +             const char *p = &prop->val.val[m->offset];
>> +             const char *end = p + chunk_len;
>> +
>> +             switch(m->type) {
>> +             case MARKER_NONE:
>> +                     if (emit_type != MARKER_NONE)
>> +                             fprintf(f, "%s", delim_end[emit_type]);
>> +                     emit_type = m->type;
>> +                     break;
>> +             case MARKER_STRING:
>> +             case MARKER_BLOB:
>> +             case MARKER_UINT8:
>> +             case MARKER_UINT16:
>> +             case MARKER_UINT32:
>> +             case MARKER_UINT64:
>> +                     emit_type = m->type;
>> +                     fprintf(f, m->offset ? ", %s" : "%s",
>> +                                delim_start[emit_type] ? : "");
>> +                     break;
>> +             case LABEL:
>> +                     fprintf(f, " %s:", m->ref);
>> +                     break;
>> +             case REF_PHANDLE:
>> +                     assert(emit_type == MARKER_UINT32);
>> +                     assert(chunk_len >= sizeof(fdt32_t));
>> +                     phandle = fdt32_to_cpu(*(const fdt32_t*)p);
>> +                     np = get_node_by_phandle(root, phandle);
>> +                     if (np) {
>> +                             write_propval_ref(f, np, NULL);
>> +                             chunk_len -= sizeof(fdt32_t);
>> +                             p += sizeof(fdt32_t);
>> +                     }
>> +                     break;
>> +             case REF_PATH:
>> +                     assert(emit_type == MARKER_NONE);
>> +                     assert(strnlen(p, chunk_len) == chunk_len - 1);
>> +                     np = get_node_by_ref(root, p);
>> +                     if (np) {
>> +                             write_propval_ref(f, np, m->ref);
>> +                             p += chunk_len;
>> +                             chunk_len = 0;
>> +                     }
>> +                     break;
>> +             default:
>> +                     break;
>> +             }
>> +
>> +             if (chunk_len <= 0)
>> +                     continue;
>> +
>> +             switch (emit_type) {
>> +             case MARKER_BLOB:
>> +             case MARKER_UINT8:
>> +                     for (; p < end; p++)
>> +                             fprintf(f, " %02"PRIx8, *(const uint8_t *)p);
>> +                     break;
>> +             case MARKER_UINT16:
>> +                     assert((chunk_len % sizeof(fdt16_t)) == 0);
>> +                     for (; p < end; p += sizeof(fdt16_t))
>> +                             fprintf(f, " 0x%02"PRIx16,
>> +                                     fdt16_to_cpu(*(const fdt16_t*)p));
>> +                     break;
>> +             case MARKER_UINT32:
>> +                     assert((chunk_len % sizeof(fdt32_t)) == 0);
>> +                     for (; p < end; p += sizeof(fdt32_t))
>> +                             fprintf(f, " 0x%02"PRIx32,
>> +                                     fdt32_to_cpu(*(const fdt32_t*)p));
>> +                     break;
>> +             case MARKER_UINT64:
>> +                     assert((chunk_len % sizeof(fdt64_t)) == 0);
>> +                     for (; p < end; p += sizeof(fdt64_t))
>> +                             fprintf(f, " 0x%02"PRIx64,
>> +                                     fdt64_to_cpu(*(const fdt64_t*)p));
>> +                     break;
>> +             case MARKER_STRING:
>> +                     assert(p[chunk_len-1] == '\0');
>> +                     write_propval_string(f, p, chunk_len);
>> +                     break;
>> +             default:
>> +                     /* Default to a block of raw bytes */
>> +                     fprintf(f, "[ ");
>> +                     for (; p < end; p++)
>> +                             fprintf(f, " %02"PRIx8, *p);
>> +                     fprintf(f, "]");
>> +             }
>>       }
>>
>> -     fprintf(f, ";\n");
>> +     fprintf(f, "%s;\n", delim_end[emit_type] ? : "");
>>  }
>>
>> -static void write_tree_source_node(FILE *f, struct node *tree, int level)
>> +static void write_tree_source_node(FILE *f, struct node *root, struct node *tree, int level)
>>  {
>>       struct property *prop;
>>       struct node *child;
>> @@ -252,11 +277,11 @@ static void write_tree_source_node(FILE *f, struct node *tree, int level)
>>               for_each_label(prop->labels, l)
>>                       fprintf(f, "%s: ", l->label);
>>               fprintf(f, "%s", prop->name);
>> -             write_propval(f, prop);
>> +             write_propval(f, root, prop);
>>       }
>>       for_each_child(tree, child) {
>>               fprintf(f, "\n");
>> -             write_tree_source_node(f, child, level+1);
>> +             write_tree_source_node(f, root, child, level+1);
>>       }
>>       write_prefix(f, level);
>>       fprintf(f, "};\n");
>> @@ -279,6 +304,6 @@ void dt_to_source(FILE *f, struct dt_info *dti)
>>                       (unsigned long long)re->size);
>>       }
>>
>> -     write_tree_source_node(f, dti->dt, 0);
>> +     write_tree_source_node(f, dti->dt, dti->dt, 0);
>>  }
>>
>
> --
> 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

Re: [RFC PATCH 1/2] Preserve datatype information when parsing dts

[David Gibson]

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

On Tue, Dec 12, 2017 at 12:48:10PM +0000, Grant Likely wrote:
> On Tue, Dec 12, 2017 at 6:14 AM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Tue, Nov 28, 2017 at 11:57:09AM +0000, Grant Likely wrote:
> >> The current code throws away all the data type and grouping information
> >> when parsing the DTS source file, which makes it difficult to
> >> reconstruct the data format when emitting a format that can express data
> >> types (ie. dts and yaml). Use the marker list to mark the beginning and
> >> end of each integer array block (<> and []), the datatype contained in
> >> each (8, 16, 32 & 64 bit widths), and the start of each string.
> >>
> >> At the same time, factor out the heuristic code used to guess a property
> >> type at emit time. It is a pretty well defined standalone block that
> >> could be used elsewhere, for instance, when emitting YAML source. Factor
> >> it out into a separate function so that it can be reused, and also to
> >> simplify the write_propval() function.
> >>
> >> When emitting, group integer output back into the same groups as the
> >> original source and use the REF_PATH and REF_PHANDLE markers to emit the
> >> the node reference instead of a raw path or phandle.
> >>
> >> Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
> >
> > I'm a bit dubious how well forcing the marker mechanism to do all this
> > stuff it was never intended for can work in the long term.  Still,
> > it's an interesting experiment.
> 
> As long as the actual data is stored as flat buffer, the markers
> mechanism works quite well for this. I tried doing something entirely
> separate, and it turned out to be awful. Another alternative is to
> break up the flat buffer into a chain of data blocks with attached
> type information, but that is a very invasive change.
> 
> This approach has the advantage of being robust on accepting both
> typed and anonymous data. If the markers are not there then the
> existing behaviour can be maintained, but otherwise it can emit a
> higher fidelity of source language.

Hm, true.  The approach is growing on me.  I guess what I'm still
dubious about is how much this type annotation can get us to approach
the YAML model.  For example, YAML can distinguish between [ [1, 2],
[3, 4] ] and [1, 2, 3, 4] which isn't really feasible in dtc.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH 1/2] Preserve datatype information when parsing dts

[Grant Likely]

On Mon, Dec 18, 2017 at 4:34 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Tue, Dec 12, 2017 at 12:48:10PM +0000, Grant Likely wrote:
>> On Tue, Dec 12, 2017 at 6:14 AM, David Gibson
>> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> > On Tue, Nov 28, 2017 at 11:57:09AM +0000, Grant Likely wrote:
>> >> The current code throws away all the data type and grouping information
>> >> when parsing the DTS source file, which makes it difficult to
>> >> reconstruct the data format when emitting a format that can express data
>> >> types (ie. dts and yaml). Use the marker list to mark the beginning and
>> >> end of each integer array block (<> and []), the datatype contained in
>> >> each (8, 16, 32 & 64 bit widths), and the start of each string.
>> >>
>> >> At the same time, factor out the heuristic code used to guess a property
>> >> type at emit time. It is a pretty well defined standalone block that
>> >> could be used elsewhere, for instance, when emitting YAML source. Factor
>> >> it out into a separate function so that it can be reused, and also to
>> >> simplify the write_propval() function.
>> >>
>> >> When emitting, group integer output back into the same groups as the
>> >> original source and use the REF_PATH and REF_PHANDLE markers to emit the
>> >> the node reference instead of a raw path or phandle.
>> >>
>> >> Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
>> >
>> > I'm a bit dubious how well forcing the marker mechanism to do all this
>> > stuff it was never intended for can work in the long term.  Still,
>> > it's an interesting experiment.
>>
>> As long as the actual data is stored as flat buffer, the markers
>> mechanism works quite well for this. I tried doing something entirely
>> separate, and it turned out to be awful. Another alternative is to
>> break up the flat buffer into a chain of data blocks with attached
>> type information, but that is a very invasive change.
>>
>> This approach has the advantage of being robust on accepting both
>> typed and anonymous data. If the markers are not there then the
>> existing behaviour can be maintained, but otherwise it can emit a
>> higher fidelity of source language.
>
> Hm, true.  The approach is growing on me.  I guess what I'm still
> dubious about is how much this type annotation can get us to approach
> the YAML model.  For example, YAML can distinguish between [ [1, 2],
> [3, 4] ] and [1, 2, 3, 4] which isn't really feasible in dtc.

To start with I'm constraining what is permissible in the YAML
encoding. So, even though YAML can encode multiple nested lists, I'm
not permitting that in this iteration. To take an example:

in dts: reg = <0x1000 0x100> <0x4000 0x300>;
In YAML I'm encoding as:   reg: [ [0x1000, 0x100], [0x4000, 0x300] ]

in dts: compatible = "acme,uart9000", "ns16550"
is in YAML: compatible: [ "acme,uart9000", "ns16550"]

in dts: #size-cells = <1>;
in YAML: "#size-cells": [ [ 1 ] ]

in dts: uint16-prop = /bits/ 16 <31>;
in YAML: uint16-prop: [ !uint16 [31] ]

I'm not allowing anything outside that pattern. So, the following are
all disallowed currently:
reg: [0x1000, 0x100, 0x4000, 0x300] /* integers need to be in a list -
maps to <...> in dts */
compatible: "ns16550" /* not encoded into list */
reg: [ [ [0x4, 0xffff0000], 0x80000], [ [0x4, 0xfffe0000], 0x40000] ]
/* Triple nesting not allowed*/

This could be relaxed later to allow arbitrary YAML structure and
encoding rules to go with them. The markers could even be nested to
keep track of the nesting in DTC, but I want to take the cautious
route and simply disallow that for now.

<digress>
You'll notice that I'm always encoding a property as a list. That's to
be able to preserve the grouping that is usually in .dts files, but
avoid ambiguity between when something is a single value or a tuple.
So, even though "#size-cells":1 would make the most sense, DTC has no
good way to know when a property is a single value, a list of single
values, or a list of tuples.

For example: given in dts interrupts = <10>; how should DTC encode it
into YAML without knowing the binding? Is it:

interrupts:10
interrupts: [ 10 ]
interrupts: [ [ 10 ] ]

In this case the best choice would be [ [ 10 ] ] because each
interrupt specifier is a tuple, and interrupts is a list of interrupt
specifiers. Being really strict about this makes it simpler to craft
schema tests because the schema doesn't need to account for multiple
encodings.

I am thinking about another way to skin this though. If the binding
schemas are available to DTC at YAML emit time, then I can force the
correct encoding. So, if DTC knows it is a #size-cells property, then
it know that is must be encoded as a bare integer.
</digress>

Cheers,
g.


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