Re: [PATCH v4] Preserve datatype markers when emitting dts format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Thu, Jun 28, 2018 at 03:37:01PM -0600, Rob Herring wrote:
> From: Grant Likely <grant.likely@xxxxxxx>
> 
> If datatype markers are present in the property value, use them to
> output the data in the correct format instead of trying to guess the
> datatype. This also will preserve data grouping, such as in an
> interrupts list.
> 
> This is a step forward for preserving and using datatype information
> when processing DTS/DTB files. Schema validation tools can use the
> datatype information to make sure a DT is correctly formed and
> intepreted.
> 
> Signed-off-by: Grant Likely <grant.likely@xxxxxxx>
> [robh: rework marker handling and fix label output]
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>

Applied, thanks.

> ---
> v4:
> - s/MARKER_NONE/TYPE_NONE/
> - rename markers.dts to type-preservation.dts
> 
> v3:
> - Fixed the label output handling. The code previously was dropping and 
>   misplacing labels in the output as it was dependent on the order of 
>   the markers.
> - Re-factor code handling for no marker case. (Simon, not sure if this 
>   is what you had in mind.
> - Added a test case.
> - Support outputting '\0' as-is if the input has '\0'
> - Drop unneeded (width - 1) in calculating the data end
> - Fixed some style nits
> 
>  dtc.h                       |   1 +
>  tests/run_tests.sh          |   7 ++
>  tests/type-preservation.dts |  28 +++++
>  treesource.c                | 222 +++++++++++++++++++++---------------
>  4 files changed, 167 insertions(+), 91 deletions(-)
>  create mode 100644 tests/type-preservation.dts
> 
> diff --git a/dtc.h b/dtc.h
> index e648cdf0210b..303c2a6a73b7 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -74,6 +74,7 @@ typedef uint32_t cell_t;
>  
>  /* Data blobs */
>  enum markertype {
> +	TYPE_NONE,
>  	REF_PHANDLE,
>  	REF_PATH,
>  	LABEL,
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 28e528f369e2..d56bef6acfd1 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -551,6 +551,13 @@ dtc_tests () {
>  	run_test dtbs_equal_ordered $tree odts_$tree.test.dtb
>      done
>  
> +    # Check -Odts preserving type information
> +    for tree in type-preservation.dts; do
> +        run_dtc_test -I dts -O dts -o $tree.test.dts $tree
> +        run_dtc_test -I dts -O dts $tree.test.dts
> +        run_wrap_test cmp $tree $tree.test.dts
> +    done
> +
>      # Check version conversions
>      for tree in test_tree1.dtb ; do
>  	 for aver in 1 2 3 16 17; do
> diff --git a/tests/type-preservation.dts b/tests/type-preservation.dts
> new file mode 100644
> index 000000000000..62c0c235087a
> --- /dev/null
> +++ b/tests/type-preservation.dts
> @@ -0,0 +1,28 @@
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = < 0x01 >;
> +	#size-cells = < 0x00 >;
> +
> +	sub1: subnode@1 {
> +		prop_label: compatible = value_label: "subnode1";
> +		reg = < 0x01 >;
> +		int-array = < 0x00 0x01 >, int_value_label: < 0x02 0x03 >;
> +		int8 = [ 56 ];
> +		int8-array = [ 00 12 34 56 label: ];
> +		int16 = /bits/ 16 < 0x3210 >;
> +		int16-array = /bits/ 16 < 0x1234 0x5678 0x90ab 0xcdef >;
> +		int16-matrix = /bits/ 16 < 0x1234 0x5678 >, < 0x90ab 0xcdef >;
> +		int64 = /bits/ 64 < 0x200000000 >;
> +		int64-array = /bits/ 64 < 0x100000000 0x00 int64_array_label_end: >;
> +		a-string-with-nulls = "foo\0bar", "baz";
> +
> +		subsub1: subsubnode {
> +			compatible = "subsubnode1", "subsubnode";
> +
> +			subsubsub1: subsubsubnode {
> +				compatible = "subsubsubnode1", < 0x1234 >, valuea: valueb: "subsubsubnode";
> +			};
> +		};
> +	};
> +};
> diff --git a/treesource.c b/treesource.c
> index 2461a3d068a0..f99544d72344 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -61,24 +61,14 @@ static bool isstring(char c)
>  		|| strchr("\a\b\t\n\v\f\r", c));
>  }
>  
> -static void write_propval_string(FILE *f, struct data val)
> +static void write_propval_string(FILE *f, const char *s, size_t len)
>  {
> -	const char *str = val.val;
> -	int i;
> -	struct marker *m = val.markers;
> -
> -	assert(str[val.len-1] == '\0');
> +	const char *end = s + len - 1;
> +	assert(*end == '\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++) {
> -		char c = str[i];
> -
> +	while (s < end) {
> +		char c = *s++;
>  		switch (c) {
>  		case '\a':
>  			fprintf(f, "\\a");
> @@ -108,91 +98,73 @@ 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, "\\0");
>  			break;
>  		default:
>  			if (isprint((unsigned char)c))
>  				fprintf(f, "%c", c);
>  			else
> -				fprintf(f, "\\x%02hhx", c);
> +				fprintf(f, "\\x%02"PRIx8, c);
>  		}
>  	}
>  	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_int(FILE *f, const char *p, size_t len, size_t width)
>  {
> -	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;
> -		}
> +	const char *end = p + len;
> +	assert(len % width == 0);
>  
> -		fprintf(f, "0x%x", fdt32_to_cpu(*cp++));
> -		if ((void *)cp >= propend)
> +	for (; p < end; p += width) {
> +		switch (width) {
> +		case 1:
> +			fprintf(f, " %02"PRIx8, *(const uint8_t*)p);
>  			break;
> -		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);
> +		case 2:
> +			fprintf(f, " 0x%02"PRIx16, fdt16_to_cpu(*(const fdt16_t*)p));
> +			break;
> +		case 4:
> +			fprintf(f, " 0x%02"PRIx32, fdt32_to_cpu(*(const fdt32_t*)p));
> +			break;
> +		case 8:
> +			fprintf(f, " 0x%02"PRIx64, fdt64_to_cpu(*(const fdt64_t*)p));
> +			break;
> +		}
>  	}
> -	fprintf(f, ">");
>  }
>  
> -static void write_propval_bytes(FILE *f, struct data val)
> +static struct marker *next_type_marker(struct marker *m)
>  {
> -	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;
> -		}
> +	while (m && (m->type == LABEL || m->type == REF_PHANDLE || m->type == REF_PATH))
> +		m = m->next;
> +	return m;
> +}
>  
> -		fprintf(f, "%02hhx", (unsigned char)(*bp++));
> -		if ((const void *)bp >= propend)
> -			break;
> -		fprintf(f, " ");
> -	}
> +static size_t type_marker_length(struct marker *m)
> +{
> +	struct marker *next = next_type_marker(m->next);
>  
> -	/* 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, "]");
> +	if (next)
> +		return next->offset - m->offset;
> +	return 0;
>  }
>  
> -static void write_propval(FILE *f, struct property *prop)
> +static const char *delim_start[] = {
> +	[TYPE_UINT8] = "[",
> +	[TYPE_UINT16] = "/bits/ 16 <",
> +	[TYPE_UINT32] = "<",
> +	[TYPE_UINT64] = "/bits/ 64 <",
> +	[TYPE_STRING] = "",
> +};
> +static const char *delim_end[] = {
> +	[TYPE_UINT8] = " ]",
> +	[TYPE_UINT16] = " >",
> +	[TYPE_UINT32] = " >",
> +	[TYPE_UINT64] = " >",
> +	[TYPE_STRING] = "",
> +};
> +
> +static enum markertype guess_value_type(struct property *prop)
>  {
>  	int len = prop->val.len;
>  	const char *p = prop->val.val;
> @@ -201,11 +173,6 @@ static void write_propval(FILE *f, struct property *prop)
>  	int nnotstringlbl = 0, nnotcelllbl = 0;
>  	int i;
>  
> -	if (len == 0) {
> -		fprintf(f, ";\n");
> -		return;
> -	}
> -
>  	for (i = 0; i < len; i++) {
>  		if (! isstring(p[i]))
>  			nnotstring++;
> @@ -220,17 +187,91 @@ static void write_propval(FILE *f, struct property *prop)
>  			nnotcelllbl++;
>  	}
>  
> -	fprintf(f, " = ");
>  	if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
>  	    && (nnotstringlbl == 0)) {
> -		write_propval_string(f, prop->val);
> +		return TYPE_STRING;
>  	} else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
> -		write_propval_cells(f, prop->val);
> -	} else {
> -		write_propval_bytes(f, prop->val);
> +		return TYPE_UINT32;
>  	}
>  
> -	fprintf(f, ";\n");
> +	return TYPE_UINT8;
> +}
> +
> +static void write_propval(FILE *f, struct property *prop)
> +{
> +	size_t len = prop->val.len;
> +	struct marker *m = prop->val.markers;
> +	struct marker dummy_marker;
> +	enum markertype emit_type = TYPE_NONE;
> +
> +	if (len == 0) {
> +		fprintf(f, ";\n");
> +		return;
> +	}
> +
> +	fprintf(f, " = ");
> +
> +	if (!next_type_marker(m)) {
> +		/* data type information missing, need to guess */
> +		dummy_marker.type = guess_value_type(prop);
> +		dummy_marker.next = prop->val.markers;
> +		dummy_marker.offset = 0;
> +		dummy_marker.ref = NULL;
> +		m = &dummy_marker;
> +	}
> +
> +	struct marker *m_label = prop->val.markers;
> +	for_each_marker(m) {
> +		size_t chunk_len;
> +		const char *p = &prop->val.val[m->offset];
> +
> +		if (m->type < TYPE_UINT8)
> +			continue;
> +
> +		chunk_len = type_marker_length(m);
> +		if (!chunk_len)
> +			chunk_len = len - m->offset;
> +
> +		if (emit_type != TYPE_NONE)
> +			fprintf(f, "%s, ", delim_end[emit_type]);
> +		emit_type = m->type;
> +
> +		for_each_marker_of_type(m_label, LABEL) {
> +			if (m_label->offset > m->offset)
> +				break;
> +			fprintf(f, "%s: ", m_label->ref);
> +		}
> +
> +		fprintf(f, "%s", delim_start[emit_type]);
> +
> +		if (chunk_len <= 0)
> +			continue;
> +
> +		switch(emit_type) {
> +		case TYPE_UINT16:
> +			write_propval_int(f, p, chunk_len, 2);
> +			break;
> +		case TYPE_UINT32:
> +			write_propval_int(f, p, chunk_len, 4);
> +			break;
> +		case TYPE_UINT64:
> +			write_propval_int(f, p, chunk_len, 8);
> +			break;
> +		case TYPE_STRING:
> +			write_propval_string(f, p, chunk_len);
> +			break;
> +		default:
> +			write_propval_int(f, p, chunk_len, 1);
> +		}
> +	}
> +
> +	/* Wrap up any labels at the end of the value */
> +	for_each_marker_of_type(m_label, LABEL) {
> +		assert (m_label->offset == len);
> +		fprintf(f, " %s:", m_label->ref);
> +	}
> +
> +	fprintf(f, "%s;\n", delim_end[emit_type] ? : "");
>  }
>  
>  static void write_tree_source_node(FILE *f, struct node *tree, int level)
> @@ -281,4 +322,3 @@ void dt_to_source(FILE *f, struct dt_info *dti)
>  
>  	write_tree_source_node(f, 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: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux