All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
@ 2019-11-05 15:26 Walter Lozano
  2019-11-05 16:56 ` Ezequiel Garcia
  0 siblings, 1 reply; 9+ messages in thread
From: Walter Lozano @ 2019-11-05 15:26 UTC (permalink / raw)
  To: u-boot

The support of libfdt should only be needed when OF_CONTROL
is enabled and OF_PLATDATA is not, as in other cases there is no
DT file to query.

This patch fixes this dependency allowing to save some space.

Signed-off-by: Walter Lozano <walter.lozano@collabora.com>
---
 drivers/core/ofnode.c | 132 +++++++++++++++++++++++++++++++++++++--
 include/dm/read.h     | 141 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 267 insertions(+), 6 deletions(-)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 8f0eab2ca6..524d763379 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -23,11 +23,14 @@ int ofnode_read_u32(ofnode node, const char *propname, u32 *outp)
 	if (ofnode_is_np(node)) {
 		return of_read_u32(ofnode_to_np(node), propname, outp);
 	} else {
-		const fdt32_t *cell;
+		const fdt32_t *cell = NULL;
 		int len;
 
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node),
 				   propname, &len);
+#endif
+
 		if (!cell || len < sizeof(int)) {
 			debug("(not found)\n");
 			return -EINVAL;
@@ -57,7 +60,7 @@ int ofnode_read_s32_default(ofnode node, const char *propname, s32 def)
 
 int ofnode_read_u64(ofnode node, const char *propname, u64 *outp)
 {
-	const unaligned_fdt64_t *cell;
+	const unaligned_fdt64_t *cell = NULL;
 	int len;
 
 	assert(ofnode_valid(node));
@@ -66,8 +69,10 @@ int ofnode_read_u64(ofnode node, const char *propname, u64 *outp)
 	if (ofnode_is_np(node))
 		return of_read_u64(ofnode_to_np(node), propname, outp);
 
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname,
 			   &len);
+#endif
 	if (!cell || len < sizeof(*cell)) {
 		debug("(not found)\n");
 		return -EINVAL;
@@ -118,8 +123,10 @@ const char *ofnode_read_string(ofnode node, const char *propname)
 			len = prop->length;
 		}
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		str = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node),
 				  propname, &len);
+#endif
 	}
 	if (!str) {
 		debug("<not found>\n");
@@ -150,8 +157,11 @@ ofnode ofnode_find_subnode(ofnode node, const char *subnode_name)
 		}
 		subnode = np_to_ofnode(np);
 	} else {
-		int ooffset = fdt_subnode_offset(gd->fdt_blob,
+		int ooffset = -FDT_ERR_NOTFOUND;
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
+		ooffset = fdt_subnode_offset(gd->fdt_blob,
 				ofnode_to_offset(node), subnode_name);
+#endif
 		subnode = offset_to_ofnode(ooffset);
 	}
 	debug("%s\n", ofnode_valid(subnode) ?
@@ -170,9 +180,13 @@ int ofnode_read_u32_array(ofnode node, const char *propname,
 		return of_read_u32_array(ofnode_to_np(node), propname,
 					 out_values, sz);
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdtdec_get_int_array(gd->fdt_blob,
 					    ofnode_to_offset(node), propname,
 					    out_values, sz);
+#else
+		return -FDT_ERR_NOTFOUND;
+#endif
 	}
 }
 
@@ -182,8 +196,12 @@ ofnode ofnode_first_subnode(ofnode node)
 	if (ofnode_is_np(node))
 		return np_to_ofnode(node.np->child);
 
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return offset_to_ofnode(
 		fdt_first_subnode(gd->fdt_blob, ofnode_to_offset(node)));
+#else
+	return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 }
 
 ofnode ofnode_next_subnode(ofnode node)
@@ -192,8 +210,12 @@ ofnode ofnode_next_subnode(ofnode node)
 	if (ofnode_is_np(node))
 		return np_to_ofnode(node.np->sibling);
 
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return offset_to_ofnode(
 		fdt_next_subnode(gd->fdt_blob, ofnode_to_offset(node)));
+#else
+	return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 }
 
 ofnode ofnode_get_parent(ofnode node)
@@ -204,9 +226,12 @@ ofnode ofnode_get_parent(ofnode node)
 	if (ofnode_is_np(node))
 		parent = np_to_ofnode(of_get_parent(ofnode_to_np(node)));
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		parent.of_offset = fdt_parent_offset(gd->fdt_blob,
 						     ofnode_to_offset(node));
-
+#else
+		parent.of_offset = -FDT_ERR_NOTFOUND;
+#endif
 	return parent;
 }
 
@@ -220,7 +245,11 @@ const char *ofnode_get_name(ofnode node)
 	if (ofnode_is_np(node))
 		return strrchr(node.np->full_name, '/') + 1;
 
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return fdt_get_name(gd->fdt_blob, ofnode_to_offset(node), NULL);
+#endif
+
+	return NULL;
 }
 
 ofnode ofnode_get_by_phandle(uint phandle)
@@ -230,16 +259,18 @@ ofnode ofnode_get_by_phandle(uint phandle)
 	if (of_live_active())
 		node = np_to_ofnode(of_find_node_by_phandle(phandle));
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		node.of_offset = fdt_node_offset_by_phandle(gd->fdt_blob,
 							    phandle);
+#else
+		node.of_offset = -FDT_ERR_NOTFOUND;
+#endif
 
 	return node;
 }
 
 int ofnode_read_size(ofnode node, const char *propname)
 {
-	int len;
-
 	if (ofnode_is_np(node)) {
 		struct property *prop = of_find_property(
 				ofnode_to_np(node), propname, NULL);
@@ -247,9 +278,12 @@ int ofnode_read_size(ofnode node, const char *propname)
 		if (prop)
 			return prop->length;
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
+		int len;
 		if (fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname,
 				&len))
 			return len;
+#endif
 	}
 
 	return -EINVAL;
@@ -282,9 +316,13 @@ fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
 	} else {
 		na = ofnode_read_simple_addr_cells(ofnode_get_parent(node));
 		ns = ofnode_read_simple_size_cells(ofnode_get_parent(node));
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdtdec_get_addr_size_fixed(gd->fdt_blob,
 						  ofnode_to_offset(node), "reg",
 						  index, na, ns, size, true);
+#else
+		return FDT_ADDR_T_NONE;
+#endif
 	}
 
 	return FDT_ADDR_T_NONE;
@@ -309,6 +347,7 @@ int ofnode_stringlist_search(ofnode node, const char *property,
 		return of_property_match_string(ofnode_to_np(node),
 						property, string);
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		int ret;
 
 		ret = fdt_stringlist_search(gd->fdt_blob,
@@ -320,6 +359,9 @@ int ofnode_stringlist_search(ofnode node, const char *property,
 			return -EINVAL;
 
 		return ret;
+#else
+		return -ENODATA;
+#endif
 	}
 }
 
@@ -330,6 +372,7 @@ int ofnode_read_string_index(ofnode node, const char *property, int index,
 		return of_property_read_string_index(ofnode_to_np(node),
 						     property, index, outp);
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		int len;
 
 		*outp = fdt_stringlist_get(gd->fdt_blob, ofnode_to_offset(node),
@@ -337,6 +380,9 @@ int ofnode_read_string_index(ofnode node, const char *property, int index,
 		if (len < 0)
 			return -EINVAL;
 		return 0;
+#else
+		return -EINVAL;
+#endif
 	}
 }
 
@@ -345,11 +391,16 @@ int ofnode_read_string_count(ofnode node, const char *property)
 	if (ofnode_is_np(node)) {
 		return of_property_count_strings(ofnode_to_np(node), property);
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_stringlist_count(gd->fdt_blob,
 					    ofnode_to_offset(node), property);
+#else
+		return -FDT_ERR_NOTFOUND;
+#endif
 	}
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 static void ofnode_from_fdtdec_phandle_args(struct fdtdec_phandle_args *in,
 					    struct ofnode_phandle_args *out)
 {
@@ -358,6 +409,7 @@ static void ofnode_from_fdtdec_phandle_args(struct fdtdec_phandle_args *in,
 	out->args_count = in->args_count;
 	memcpy(out->args, in->args, sizeof(out->args));
 }
+#endif
 
 static void ofnode_from_of_phandle_args(struct of_phandle_args *in,
 					struct ofnode_phandle_args *out)
@@ -384,6 +436,7 @@ int ofnode_parse_phandle_with_args(ofnode node, const char *list_name,
 			return ret;
 		ofnode_from_of_phandle_args(&args, out_args);
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		struct fdtdec_phandle_args args;
 		int ret;
 
@@ -394,6 +447,9 @@ int ofnode_parse_phandle_with_args(ofnode node, const char *list_name,
 		if (ret)
 			return ret;
 		ofnode_from_fdtdec_phandle_args(&args, out_args);
+#else
+		return -ENOENT;
+#endif
 	}
 
 	return 0;
@@ -406,9 +462,13 @@ int ofnode_count_phandle_with_args(ofnode node, const char *list_name,
 		return of_count_phandle_with_args(ofnode_to_np(node),
 				list_name, cells_name);
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdtdec_parse_phandle_with_args(gd->fdt_blob,
 				ofnode_to_offset(node), list_name, cells_name,
 				0, -1, NULL);
+#else
+		return -ENOENT;
+#endif
 }
 
 ofnode ofnode_path(const char *path)
@@ -416,7 +476,11 @@ ofnode ofnode_path(const char *path)
 	if (of_live_active())
 		return np_to_ofnode(of_find_node_by_path(path));
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return offset_to_ofnode(fdt_path_offset(gd->fdt_blob, path));
+#else
+		return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 }
 
 const char *ofnode_get_chosen_prop(const char *name)
@@ -532,8 +596,12 @@ const void *ofnode_get_property(ofnode node, const char *propname, int *lenp)
 	if (ofnode_is_np(node))
 		return of_get_property(ofnode_to_np(node), propname, lenp);
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_getprop(gd->fdt_blob, ofnode_to_offset(node),
 				   propname, lenp);
+#else
+		return NULL;
+#endif
 }
 
 bool ofnode_is_available(ofnode node)
@@ -541,8 +609,12 @@ bool ofnode_is_available(ofnode node)
 	if (ofnode_is_np(node))
 		return of_device_is_available(ofnode_to_np(node));
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdtdec_get_is_enabled(gd->fdt_blob,
 					     ofnode_to_offset(node));
+#else
+		return 0;
+#endif
 }
 
 fdt_addr_t ofnode_get_addr_size(ofnode node, const char *property,
@@ -565,9 +637,13 @@ fdt_addr_t ofnode_get_addr_size(ofnode node, const char *property,
 		else
 			return of_read_number(prop, na);
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdtdec_get_addr_size(gd->fdt_blob,
 					    ofnode_to_offset(node), property,
 					    sizep);
+#else
+		return FDT_ADDR_T_NONE;
+#endif
 	}
 }
 
@@ -584,8 +660,12 @@ const uint8_t *ofnode_read_u8_array_ptr(ofnode node, const char *propname,
 		return (uint8_t *)prop;
 
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdtdec_locate_byte_array(gd->fdt_blob,
 				ofnode_to_offset(node), propname, sz);
+#else
+		return NULL;
+#endif
 	}
 }
 
@@ -684,7 +764,11 @@ int ofnode_read_addr_cells(ofnode node)
 	if (ofnode_is_np(node))
 		return of_n_addr_cells(ofnode_to_np(node));
 	else  /* NOTE: this call should walk up the parent stack */
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_address_cells(gd->fdt_blob, ofnode_to_offset(node));
+#else
+		return -FDT_ERR_BADNCELLS;
+#endif
 }
 
 int ofnode_read_size_cells(ofnode node)
@@ -692,7 +776,11 @@ int ofnode_read_size_cells(ofnode node)
 	if (ofnode_is_np(node))
 		return of_n_size_cells(ofnode_to_np(node));
 	else  /* NOTE: this call should walk up the parent stack */
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_size_cells(gd->fdt_blob, ofnode_to_offset(node));
+#else
+		return -FDT_ERR_BADNCELLS;
+#endif
 }
 
 int ofnode_read_simple_addr_cells(ofnode node)
@@ -700,7 +788,11 @@ int ofnode_read_simple_addr_cells(ofnode node)
 	if (ofnode_is_np(node))
 		return of_simple_addr_cells(ofnode_to_np(node));
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_address_cells(gd->fdt_blob, ofnode_to_offset(node));
+#else
+		return -FDT_ERR_BADNCELLS;
+#endif
 }
 
 int ofnode_read_simple_size_cells(ofnode node)
@@ -708,7 +800,11 @@ int ofnode_read_simple_size_cells(ofnode node)
 	if (ofnode_is_np(node))
 		return of_simple_size_cells(ofnode_to_np(node));
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_size_cells(gd->fdt_blob, ofnode_to_offset(node));
+#else
+		return -FDT_ERR_BADNCELLS;
+#endif
 }
 
 bool ofnode_pre_reloc(ofnode node)
@@ -742,6 +838,7 @@ int ofnode_read_resource(ofnode node, uint index, struct resource *res)
 	if (ofnode_is_np(node)) {
 		return of_address_to_resource(ofnode_to_np(node), index, res);
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		struct fdt_resource fres;
 		int ret;
 
@@ -754,6 +851,9 @@ int ofnode_read_resource(ofnode node, uint index, struct resource *res)
 		res->end = fres.end;
 
 		return 0;
+#else
+		return -EINVAL;
+#endif
 	}
 }
 
@@ -774,7 +874,11 @@ u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr)
 	if (ofnode_is_np(node))
 		return of_translate_address(ofnode_to_np(node), in_addr);
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_translate_address(gd->fdt_blob, ofnode_to_offset(node), in_addr);
+#else
+		return OF_BAD_ADDR;
+#endif
 }
 
 u64 ofnode_translate_dma_address(ofnode node, const fdt32_t *in_addr)
@@ -782,7 +886,11 @@ u64 ofnode_translate_dma_address(ofnode node, const fdt32_t *in_addr)
 	if (ofnode_is_np(node))
 		return of_translate_dma_address(ofnode_to_np(node), in_addr);
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return fdt_translate_dma_address(gd->fdt_blob, ofnode_to_offset(node), in_addr);
+#else
+		return OF_BAD_ADDR;
+#endif
 }
 
 int ofnode_device_is_compatible(ofnode node, const char *compat)
@@ -791,9 +899,13 @@ int ofnode_device_is_compatible(ofnode node, const char *compat)
 		return of_device_is_compatible(ofnode_to_np(node), compat,
 					       NULL, NULL);
 	else
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return !fdt_node_check_compatible(gd->fdt_blob,
 						  ofnode_to_offset(node),
 						  compat);
+#else
+		return !-FDT_ERR_NOTFOUND;
+#endif
 }
 
 ofnode ofnode_by_compatible(ofnode from, const char *compat)
@@ -803,8 +915,12 @@ ofnode ofnode_by_compatible(ofnode from, const char *compat)
 			(struct device_node *)ofnode_to_np(from), NULL,
 			compat));
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return offset_to_ofnode(fdt_node_offset_by_compatible(
 				gd->fdt_blob, ofnode_to_offset(from), compat));
+#else
+		return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 	}
 }
 
@@ -816,9 +932,13 @@ ofnode ofnode_by_prop_value(ofnode from, const char *propname,
 			(struct device_node *)ofnode_to_np(from), propname,
 			propval, proplen));
 	} else {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 		return offset_to_ofnode(fdt_node_offset_by_prop_value(
 				gd->fdt_blob, ofnode_to_offset(from),
 				propname, propval, proplen));
+#else
+		return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 	}
 }
 
diff --git a/include/dm/read.h b/include/dm/read.h
index d37fcb504d..53ad523371 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -40,7 +40,11 @@ static inline ofnode dev_ofnode(struct udevice *dev)
 
 static inline bool dev_of_valid(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_valid(dev_ofnode(dev));
+#else
+	return false;
+#endif
 }
 
 #ifndef CONFIG_DM_DEV_READ_INLINE
@@ -607,13 +611,21 @@ int dev_read_alias_highest_id(const char *stem);
 static inline int dev_read_u32(struct udevice *dev,
 			       const char *propname, u32 *outp)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_u32(dev_ofnode(dev), propname, outp);
+#else
+	return -EINVAL;
+#endif
 }
 
 static inline int dev_read_u32_default(struct udevice *dev,
 				       const char *propname, int def)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_u32_default(dev_ofnode(dev), propname, def);
+#else
+	return def;
+#endif
 }
 
 static inline int dev_read_s32(struct udevice *dev,
@@ -625,7 +637,11 @@ static inline int dev_read_s32(struct udevice *dev,
 static inline int dev_read_s32_default(struct udevice *dev,
 				       const char *propname, int def)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_s32_default(dev_ofnode(dev), propname, def);
+#else
+	return def;
+#endif
 }
 
 static inline int dev_read_u32u(struct udevice *dev,
@@ -634,7 +650,11 @@ static inline int dev_read_u32u(struct udevice *dev,
 	u32 val;
 	int ret;
 
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	ret = ofnode_read_u32(dev_ofnode(dev), propname, &val);
+#else
+	return -EINVAL;
+#endif
 	if (ret)
 		return ret;
 	*outp = val;
@@ -645,35 +665,59 @@ static inline int dev_read_u32u(struct udevice *dev,
 static inline int dev_read_u64(struct udevice *dev,
 			       const char *propname, u64 *outp)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_u64(dev_ofnode(dev), propname, outp);
+#else
+	return -EINVAL;
+#endif
 }
 
 static inline u64 dev_read_u64_default(struct udevice *dev,
 				       const char *propname, u64 def)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_u64_default(dev_ofnode(dev), propname, def);
+#else
+	return def;
+#endif
 }
 
 static inline const char *dev_read_string(struct udevice *dev,
 					  const char *propname)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_string(dev_ofnode(dev), propname);
+#else
+	return NULL;
+#endif
 }
 
 static inline bool dev_read_bool(struct udevice *dev, const char *propname)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_bool(dev_ofnode(dev), propname);
+#else
+	return -EINVAL;
+#endif
 }
 
 static inline ofnode dev_read_subnode(struct udevice *dev,
 				      const char *subbnode_name)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_find_subnode(dev_ofnode(dev), subbnode_name);
+#else
+	return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 }
 
 static inline int dev_read_size(struct udevice *dev, const char *propname)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_size(dev_ofnode(dev), propname);
+#else
+	return -EINVAL;
+#endif
 }
 
 static inline fdt_addr_t dev_read_addr_index(struct udevice *dev, int index)
@@ -735,142 +779,239 @@ static inline fdt_addr_t dev_read_addr_size(struct udevice *dev,
 					    const char *propname,
 					    fdt_size_t *sizep)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_get_addr_size(dev_ofnode(dev), propname, sizep);
+#else
+	return FDT_ADDR_T_NONE;
+#endif
 }
 
 static inline const char *dev_read_name(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_get_name(dev_ofnode(dev));
+#else
+	return NULL;
+#endif
 }
 
 static inline int dev_read_stringlist_search(struct udevice *dev,
 					     const char *propname,
 					     const char *string)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_stringlist_search(dev_ofnode(dev), propname, string);
+#else
+	return -ENODATA;
+#endif
 }
 
 static inline int dev_read_string_index(struct udevice *dev,
 					const char *propname, int index,
 					const char **outp)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_string_index(dev_ofnode(dev), propname, index, outp);
+#else
+	return -EINVAL;
+#endif
 }
 
 static inline int dev_read_string_count(struct udevice *dev,
 					const char *propname)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_string_count(dev_ofnode(dev), propname);
+#else
+	return -FDT_ERR_NOTFOUND;
+#endif
 }
 
 static inline int dev_read_phandle_with_args(struct udevice *dev,
 		const char *list_name, const char *cells_name, int cell_count,
 		int index, struct ofnode_phandle_args *out_args)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_parse_phandle_with_args(dev_ofnode(dev), list_name,
 					      cells_name, cell_count, index,
 					      out_args);
+#else
+	return -ENOENT;
+#endif
 }
 
 static inline int dev_count_phandle_with_args(struct udevice *dev,
 		const char *list_name, const char *cells_name)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_count_phandle_with_args(dev_ofnode(dev), list_name,
 					      cells_name);
+#else
+	return -ENOENT;
+#endif
 }
 
 static inline int dev_read_addr_cells(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	/* NOTE: this call should walk up the parent stack */
 	return fdt_address_cells(gd->fdt_blob, dev_of_offset(dev));
+#else
+	return 2;
+#endif
 }
 
 static inline int dev_read_size_cells(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	/* NOTE: this call should walk up the parent stack */
 	return fdt_size_cells(gd->fdt_blob, dev_of_offset(dev));
+#else
+	return 2;
+#endif
 }
 
 static inline int dev_read_simple_addr_cells(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return fdt_address_cells(gd->fdt_blob, dev_of_offset(dev));
+#else
+	return 2;
+#endif
 }
 
 static inline int dev_read_simple_size_cells(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return fdt_size_cells(gd->fdt_blob, dev_of_offset(dev));
+#else
+	return 2;
+#endif
 }
 
 static inline int dev_read_phandle(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return fdt_get_phandle(gd->fdt_blob, dev_of_offset(dev));
+#else
+	return 0;
+#endif
 }
 
 static inline const void *dev_read_prop(struct udevice *dev,
 					const char *propname, int *lenp)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_get_property(dev_ofnode(dev), propname, lenp);
+#else
+	return NULL;
+#endif
 }
 
 static inline int dev_read_alias_seq(struct udevice *dev, int *devnump)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return fdtdec_get_alias_seq(gd->fdt_blob, dev->uclass->uc_drv->name,
 				    dev_of_offset(dev), devnump);
+#else
+	return -ENOENT;
+#endif
 }
 
 static inline int dev_read_u32_array(struct udevice *dev, const char *propname,
 				     u32 *out_values, size_t sz)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_u32_array(dev_ofnode(dev), propname, out_values, sz);
+#else
+	return -FDT_ERR_NOTFOUND;
+#endif
 }
 
 static inline ofnode dev_read_first_subnode(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_first_subnode(dev_ofnode(dev));
+#else
+	return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 }
 
 static inline ofnode dev_read_next_subnode(ofnode node)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_next_subnode(node);
+#else
+	return offset_to_ofnode(-FDT_ERR_NOTFOUND);
+#endif
 }
 
 static inline const uint8_t *dev_read_u8_array_ptr(struct udevice *dev,
 					const char *propname, size_t sz)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_u8_array_ptr(dev_ofnode(dev), propname, sz);
+#else
+	return NULL;
+#endif
 }
 
 static inline int dev_read_enabled(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return fdtdec_get_is_enabled(gd->fdt_blob, dev_of_offset(dev));
+#else
+	return 0;
+#endif
 }
 
 static inline int dev_read_resource(struct udevice *dev, uint index,
 				    struct resource *res)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_resource(dev_ofnode(dev), index, res);
+#else
+	return -EINVAL;
+#endif
 }
 
 static inline int dev_read_resource_byname(struct udevice *dev,
 					   const char *name,
 					   struct resource *res)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_read_resource_byname(dev_ofnode(dev), name, res);
+#else
+	return -EINVAL;
+#endif
 }
 
 static inline u64 dev_translate_address(struct udevice *dev, const fdt32_t *in_addr)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_translate_address(dev_ofnode(dev), in_addr);
+#else
+	return OF_BAD_ADDR;
+#endif
 }
 
 static inline u64 dev_translate_dma_address(struct udevice *dev, const fdt32_t *in_addr)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return ofnode_translate_dma_address(dev_ofnode(dev), in_addr);
+#else
+	return OF_BAD_ADDR;
+#endif
 }
 
 static inline int dev_read_alias_highest_id(const char *stem)
 {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 	return fdtdec_get_alias_highest_id(gd->fdt_blob, stem);
+#else
+	return -1;
+#endif
+
 }
 
 #endif /* CONFIG_DM_DEV_READ_INLINE */
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-05 15:26 [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary Walter Lozano
@ 2019-11-05 16:56 ` Ezequiel Garcia
  2019-11-05 18:12   ` Walter Lozano
  2019-11-07 18:27   ` Tom Rini
  0 siblings, 2 replies; 9+ messages in thread
From: Ezequiel Garcia @ 2019-11-05 16:56 UTC (permalink / raw)
  To: u-boot

Hello Walter,

Thanks for the patch.

On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
>
> The support of libfdt should only be needed when OF_CONTROL
> is enabled and OF_PLATDATA is not, as in other cases there is no
> DT file to query.
>
> This patch fixes this dependency allowing to save some space.
>

Can you add some more information about the space saving?
The ./scripts/bloat-o-meter will help you get some info
on static footprint.

> Signed-off-by: Walter Lozano <walter.lozano@collabora.com>
> ---
>  drivers/core/ofnode.c | 132 +++++++++++++++++++++++++++++++++++++--
>  include/dm/read.h     | 141 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 267 insertions(+), 6 deletions(-)
>

You should try to avoid adding #ifdefery to the code. Normally,
the way to ifdef code is by having this in a header:

#ifdef CONFIG_FOO
int foo_feature_optional(struct foo *priv);
#else
static inline int foo_feature_optional(struct foo *priv)
{
        return 0;
}
#endif

The user of foo_feature_optional shouldn't be bothered with
FOO being enabled or not. Pushing ifdefs away from .c to .h
is a common pattern, well described in a classic old article:

http://www.literateprogramming.com/ifdefs.pdf

Do you think you can try to rework the patch to reduce its impact
as much as possible?

Thanks,
Ezequiel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-05 16:56 ` Ezequiel Garcia
@ 2019-11-05 18:12   ` Walter Lozano
  2019-11-05 23:30     ` Ezequiel Garcia
  2019-11-07 18:27   ` Tom Rini
  1 sibling, 1 reply; 9+ messages in thread
From: Walter Lozano @ 2019-11-05 18:12 UTC (permalink / raw)
  To: u-boot

Hi Ezequiel,

On 5/11/19 13:56, Ezequiel Garcia wrote:
> Hello Walter,
>
> Thanks for the patch.
>
> On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
>> The support of libfdt should only be needed when OF_CONTROL
>> is enabled and OF_PLATDATA is not, as in other cases there is no
>> DT file to query.
>>
>> This patch fixes this dependency allowing to save some space.
>>
> Can you add some more information about the space saving?


Sure, I will add some additional information about the static footprint. 
However according to my understanding the impact depends on how well 
different drivers supports features like OF_PLATDATA. For instance, in 
my current configuration it saves 2 KB.


> The ./scripts/bloat-o-meter will help you get some info
> on static footprint.


Thanks for your suggestion. I think you are pointing to a script found 
in the Linux kernel, right? I also think it could be useful to have a 
deep understanding on how the static footprint of some .o files changes, 
but it won't give us much information about the end result of the binary 
image, which is affected by the dependencies and compiler/linker 
optimizations. Is this correct?


>> Signed-off-by: Walter Lozano <walter.lozano@collabora.com>
>> ---
>>   drivers/core/ofnode.c | 132 +++++++++++++++++++++++++++++++++++++--
>>   include/dm/read.h     | 141 ++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 267 insertions(+), 6 deletions(-)
>>
> You should try to avoid adding #ifdefery to the code. Normally,
> the way to ifdef code is by having this in a header:
>
> #ifdef CONFIG_FOO
> int foo_feature_optional(struct foo *priv);
> #else
> static inline int foo_feature_optional(struct foo *priv)
> {
>          return 0;
> }
> #endif
>
> The user of foo_feature_optional shouldn't be bothered with
> FOO being enabled or not. Pushing ifdefs away from .c to .h
> is a common pattern, well described in a classic old article:
>
> http://www.literateprogramming.com/ifdefs.pdf
>
> Do you think you can try to rework the patch to reduce its impact
> as much as possible?


Thanks for your review and your suggestions, I will be happy to rework 
this to improve it.


The intention of this RFC is to get some feedback about if this is 
something worth to be added and if this is the right approach. The use 
of OF_PLATDATA is quite limited as it needs drivers to support it, which 
will probably be a long process. Enabling it but having some drivers to 
query DT properties has no sense and requires additional dependencies, 
like libfdt.

Also I think it should be possible to remove some extra components but 
it will require extra work to avoid break some configurations.


> Thanks,
> Ezequiel


Thanks,

Walter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-05 18:12   ` Walter Lozano
@ 2019-11-05 23:30     ` Ezequiel Garcia
  2019-11-07 13:30       ` Walter Lozano
  0 siblings, 1 reply; 9+ messages in thread
From: Ezequiel Garcia @ 2019-11-05 23:30 UTC (permalink / raw)
  To: u-boot

On Tue, 5 Nov 2019 at 15:12, Walter Lozano <walter.lozano@collabora.com> wrote:
>
> Hi Ezequiel,
>
> On 5/11/19 13:56, Ezequiel Garcia wrote:
> > Hello Walter,
> >
> > Thanks for the patch.
> >
> > On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
> >> The support of libfdt should only be needed when OF_CONTROL
> >> is enabled and OF_PLATDATA is not, as in other cases there is no
> >> DT file to query.
> >>
> >> This patch fixes this dependency allowing to save some space.
> >>
> > Can you add some more information about the space saving?
>
>
> Sure, I will add some additional information about the static footprint.
> However according to my understanding the impact depends on how well
> different drivers supports features like OF_PLATDATA. For instance, in
> my current configuration it saves 2 KB.
>

Not sure I follow you. This patch adds a condition which adds/removes code
based on some conditions. So, it should depend on the arch, but otherwise
the reduction is an invariant as it just depend on the size of the
code that is being
added/removed. Or am I missing something?

>
> > The ./scripts/bloat-o-meter will help you get some info
> > on static footprint.
>
>
> Thanks for your suggestion. I think you are pointing to a script found
> in the Linux kernel, right?

Ah, yes. I have this script imported on my U-Boot repo, so I assumed
it was here as well.

> I also think it could be useful to have a
> deep understanding on how the static footprint of some .o files changes,
> but it won't give us much information about the end result of the binary
> image, which is affected by the dependencies and compiler/linker
> optimizations. Is this correct?
>

Normally, bloat-o-meter gives you a rough idea and allows you to
weigh code complexity vs. size reduction.

>
> >> Signed-off-by: Walter Lozano <walter.lozano@collabora.com>
> >> ---
> >>   drivers/core/ofnode.c | 132 +++++++++++++++++++++++++++++++++++++--
> >>   include/dm/read.h     | 141 ++++++++++++++++++++++++++++++++++++++++++
> >>   2 files changed, 267 insertions(+), 6 deletions(-)
> >>
> > You should try to avoid adding #ifdefery to the code. Normally,
> > the way to ifdef code is by having this in a header:
> >
> > #ifdef CONFIG_FOO
> > int foo_feature_optional(struct foo *priv);
> > #else
> > static inline int foo_feature_optional(struct foo *priv)
> > {
> >          return 0;
> > }
> > #endif
> >
> > The user of foo_feature_optional shouldn't be bothered with
> > FOO being enabled or not. Pushing ifdefs away from .c to .h
> > is a common pattern, well described in a classic old article:
> >
> > http://www.literateprogramming.com/ifdefs.pdf
> >
> > Do you think you can try to rework the patch to reduce its impact
> > as much as possible?
>
>
> Thanks for your review and your suggestions, I will be happy to rework
> this to improve it.
>
>
> The intention of this RFC is to get some feedback about if this is
> something worth to be added and if this is the right approach. The use
> of OF_PLATDATA is quite limited as it needs drivers to support it, which
> will probably be a long process. Enabling it but having some drivers to
> query DT properties has no sense and requires additional dependencies,
> like libfdt.
>
> Also I think it should be possible to remove some extra components but
> it will require extra work to avoid break some configurations.
>

It's always good to shave off bytes, specially in SPL -- not so much
in proper U-Boot, right?

BTW, what is the motivation of this patch: which platform are you
working on, and
how is it size constrained?

Thanks,
Ezequiel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-05 23:30     ` Ezequiel Garcia
@ 2019-11-07 13:30       ` Walter Lozano
  2019-11-07 16:42         ` Simon Glass
  0 siblings, 1 reply; 9+ messages in thread
From: Walter Lozano @ 2019-11-07 13:30 UTC (permalink / raw)
  To: u-boot

Hi Ezequiel,

On 5/11/19 20:30, Ezequiel Garcia wrote:
> On Tue, 5 Nov 2019 at 15:12, Walter Lozano <walter.lozano@collabora.com> wrote:
>> Hi Ezequiel,
>>
>> On 5/11/19 13:56, Ezequiel Garcia wrote:
>>> Hello Walter,
>>>
>>> Thanks for the patch.
>>>
>>> On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
>>>> The support of libfdt should only be needed when OF_CONTROL
>>>> is enabled and OF_PLATDATA is not, as in other cases there is no
>>>> DT file to query.
>>>>
>>>> This patch fixes this dependency allowing to save some space.
>>>>
>>> Can you add some more information about the space saving?
>>
>> Sure, I will add some additional information about the static footprint.
>> However according to my understanding the impact depends on how well
>> different drivers supports features like OF_PLATDATA. For instance, in
>> my current configuration it saves 2 KB.
>>
> Not sure I follow you. This patch adds a condition which adds/removes code
> based on some conditions. So, it should depend on the arch, but otherwise
> the reduction is an invariant as it just depend on the size of the
> code that is being
> added/removed. Or am I missing something?


The idea behind this patch is to break the dependency of libfdt when it 
is not needed. A specific example of this is found when building SPL 
using OF_PLATDATA, which basically removes DT so there is no sense of 
having libfdt present in SPL. Unfortunately as OF_PLATDATA has little 
support, drivers tend to assume that there is a DT and try to query 
different properties, which of course return no data. In this context, 
the idea is to keep the same behavior but reducing the SPL size, while 
trying to improve drivers.


>
>>> The ./scripts/bloat-o-meter will help you get some info
>>> on static footprint.
>>
>> Thanks for your suggestion. I think you are pointing to a script found
>> in the Linux kernel, right?
> Ah, yes. I have this script imported on my U-Boot repo, so I assumed
> it was here as well.


Thanks for confirm this.


>> I also think it could be useful to have a
>> deep understanding on how the static footprint of some .o files changes,
>> but it won't give us much information about the end result of the binary
>> image, which is affected by the dependencies and compiler/linker
>> optimizations. Is this correct?
>>
> Normally, bloat-o-meter gives you a rough idea and allows you to
> weigh code complexity vs. size reduction.


I see, however, in this context the reduction is due to the remove of 
dependencies.


>
>>>> Signed-off-by: Walter Lozano <walter.lozano@collabora.com>
>>>> ---
>>>>    drivers/core/ofnode.c | 132 +++++++++++++++++++++++++++++++++++++--
>>>>    include/dm/read.h     | 141 ++++++++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 267 insertions(+), 6 deletions(-)
>>>>
>>> You should try to avoid adding #ifdefery to the code. Normally,
>>> the way to ifdef code is by having this in a header:
>>>
>>> #ifdef CONFIG_FOO
>>> int foo_feature_optional(struct foo *priv);
>>> #else
>>> static inline int foo_feature_optional(struct foo *priv)
>>> {
>>>           return 0;
>>> }
>>> #endif
>>>
>>> The user of foo_feature_optional shouldn't be bothered with
>>> FOO being enabled or not. Pushing ifdefs away from .c to .h
>>> is a common pattern, well described in a classic old article:
>>>
>>> http://www.literateprogramming.com/ifdefs.pdf
>>>
>>> Do you think you can try to rework the patch to reduce its impact
>>> as much as possible?
>>
>> Thanks for your review and your suggestions, I will be happy to rework
>> this to improve it.
>>
>>
>> The intention of this RFC is to get some feedback about if this is
>> something worth to be added and if this is the right approach. The use
>> of OF_PLATDATA is quite limited as it needs drivers to support it, which
>> will probably be a long process. Enabling it but having some drivers to
>> query DT properties has no sense and requires additional dependencies,
>> like libfdt.
>>
>> Also I think it should be possible to remove some extra components but
>> it will require extra work to avoid break some configurations.
>>
> It's always good to shave off bytes, specially in SPL -- not so much
> in proper U-Boot, right?


Yes, this is specially interesting for SPL where the size constrain is 
more problematic.


>
> BTW, what is the motivation of this patch: which platform are you
> working on, and
> how is it size constrained?


I'm currently working on iMX6 where typically SPL size should be less 
than 68 KB. That I think this approach (or other in the same direction) 
could be useful in many other platforms with more severe restrictions.


>
> Thanks,
> Ezequiel


Thanks again Ezequiel for you review. I wil work in a better approach.


Regards,

Walter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-07 13:30       ` Walter Lozano
@ 2019-11-07 16:42         ` Simon Glass
  2019-11-07 19:20           ` Walter Lozano
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Glass @ 2019-11-07 16:42 UTC (permalink / raw)
  To: u-boot

Hi Walter,

On Thu, 7 Nov 2019 at 06:30, Walter Lozano <walter.lozano@collabora.com> wrote:
>
> Hi Ezequiel,
>
> On 5/11/19 20:30, Ezequiel Garcia wrote:
> > On Tue, 5 Nov 2019 at 15:12, Walter Lozano <walter.lozano@collabora.com> wrote:
> >> Hi Ezequiel,
> >>
> >> On 5/11/19 13:56, Ezequiel Garcia wrote:
> >>> Hello Walter,
> >>>
> >>> Thanks for the patch.
> >>>
> >>> On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
> >>>> The support of libfdt should only be needed when OF_CONTROL
> >>>> is enabled and OF_PLATDATA is not, as in other cases there is no
> >>>> DT file to query.
> >>>>
> >>>> This patch fixes this dependency allowing to save some space.
> >>>>
> >>> Can you add some more information about the space saving?
> >>
> >> Sure, I will add some additional information about the static footprint.
> >> However according to my understanding the impact depends on how well
> >> different drivers supports features like OF_PLATDATA. For instance, in
> >> my current configuration it saves 2 KB.
> >>
> > Not sure I follow you. This patch adds a condition which adds/removes code
> > based on some conditions. So, it should depend on the arch, but otherwise
> > the reduction is an invariant as it just depend on the size of the
> > code that is being
> > added/removed. Or am I missing something?
>
>
> The idea behind this patch is to break the dependency of libfdt when it
> is not needed. A specific example of this is found when building SPL
> using OF_PLATDATA, which basically removes DT so there is no sense of
> having libfdt present in SPL. Unfortunately as OF_PLATDATA has little
> support, drivers tend to assume that there is a DT and try to query
> different properties, which of course return no data. In this context,
> the idea is to keep the same behavior but reducing the SPL size, while
> trying to improve drivers.

I actually sent a similar patch in September but never fixed the
problems it caused. I just resent it as a series:

http://patchwork.ozlabs.org/project/uboot/list/?series=141416

[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-05 16:56 ` Ezequiel Garcia
  2019-11-05 18:12   ` Walter Lozano
@ 2019-11-07 18:27   ` Tom Rini
  2019-11-08  3:31     ` Walter Lozano
  1 sibling, 1 reply; 9+ messages in thread
From: Tom Rini @ 2019-11-07 18:27 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 05, 2019 at 01:56:00PM -0300, Ezequiel Garcia wrote:
> Hello Walter,
> 
> Thanks for the patch.
> 
> On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
> >
> > The support of libfdt should only be needed when OF_CONTROL
> > is enabled and OF_PLATDATA is not, as in other cases there is no
> > DT file to query.
> >
> > This patch fixes this dependency allowing to save some space.
> >
> 
> Can you add some more information about the space saving?
> The ./scripts/bloat-o-meter will help you get some info
> on static footprint.

Note that in U-Boot, we can as good or better information via buildman.
The --bloat flag gives a lot of useful info about what functions
grew/shrunk in addition to a section size summary.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191107/048774ab/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-07 16:42         ` Simon Glass
@ 2019-11-07 19:20           ` Walter Lozano
  0 siblings, 0 replies; 9+ messages in thread
From: Walter Lozano @ 2019-11-07 19:20 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 7/11/19 13:42, Simon Glass wrote:
> Hi Walter,
>
> On Thu, 7 Nov 2019 at 06:30, Walter Lozano <walter.lozano@collabora.com> wrote:
>> Hi Ezequiel,
>>
>> On 5/11/19 20:30, Ezequiel Garcia wrote:
>>> On Tue, 5 Nov 2019 at 15:12, Walter Lozano <walter.lozano@collabora.com> wrote:
>>>> Hi Ezequiel,
>>>>
>>>> On 5/11/19 13:56, Ezequiel Garcia wrote:
>>>>> Hello Walter,
>>>>>
>>>>> Thanks for the patch.
>>>>>
>>>>> On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
>>>>>> The support of libfdt should only be needed when OF_CONTROL
>>>>>> is enabled and OF_PLATDATA is not, as in other cases there is no
>>>>>> DT file to query.
>>>>>>
>>>>>> This patch fixes this dependency allowing to save some space.
>>>>>>
>>>>> Can you add some more information about the space saving?
>>>> Sure, I will add some additional information about the static footprint.
>>>> However according to my understanding the impact depends on how well
>>>> different drivers supports features like OF_PLATDATA. For instance, in
>>>> my current configuration it saves 2 KB.
>>>>
>>> Not sure I follow you. This patch adds a condition which adds/removes code
>>> based on some conditions. So, it should depend on the arch, but otherwise
>>> the reduction is an invariant as it just depend on the size of the
>>> code that is being
>>> added/removed. Or am I missing something?
>>
>> The idea behind this patch is to break the dependency of libfdt when it
>> is not needed. A specific example of this is found when building SPL
>> using OF_PLATDATA, which basically removes DT so there is no sense of
>> having libfdt present in SPL. Unfortunately as OF_PLATDATA has little
>> support, drivers tend to assume that there is a DT and try to query
>> different properties, which of course return no data. In this context,
>> the idea is to keep the same behavior but reducing the SPL size, while
>> trying to improve drivers.
> I actually sent a similar patch in September but never fixed the
> problems it caused. I just resent it as a series:
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=141416


Thanks for pointing to this patchset, it looks like the approach I was 
looking for to overcome the the issue with libfdt and other dependencies 
in a proper way. I'll try to use your same approach in other drivers.


>
> [..]
>
> Regards,
> Simon


Regards,


Walter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary
  2019-11-07 18:27   ` Tom Rini
@ 2019-11-08  3:31     ` Walter Lozano
  0 siblings, 0 replies; 9+ messages in thread
From: Walter Lozano @ 2019-11-08  3:31 UTC (permalink / raw)
  To: u-boot

Hi Tom

On 7/11/19 15:27, Tom Rini wrote:
> On Tue, Nov 05, 2019 at 01:56:00PM -0300, Ezequiel Garcia wrote:
>> Hello Walter,
>>
>> Thanks for the patch.
>>
>> On Tue, 5 Nov 2019 at 12:27, Walter Lozano <walter.lozano@collabora.com> wrote:
>>> The support of libfdt should only be needed when OF_CONTROL
>>> is enabled and OF_PLATDATA is not, as in other cases there is no
>>> DT file to query.
>>>
>>> This patch fixes this dependency allowing to save some space.
>>>
>> Can you add some more information about the space saving?
>> The ./scripts/bloat-o-meter will help you get some info
>> on static footprint.
> Note that in U-Boot, we can as good or better information via buildman.
> The --bloat flag gives a lot of useful info about what functions
> grew/shrunk in addition to a section size summary.
>
Thanks for sharing this information. I will check it.


Regards,


Walter

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-11-08  3:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 15:26 [U-Boot] [RFC PATCH] dm: core: Remove libfdt dependency when unnecessary Walter Lozano
2019-11-05 16:56 ` Ezequiel Garcia
2019-11-05 18:12   ` Walter Lozano
2019-11-05 23:30     ` Ezequiel Garcia
2019-11-07 13:30       ` Walter Lozano
2019-11-07 16:42         ` Simon Glass
2019-11-07 19:20           ` Walter Lozano
2019-11-07 18:27   ` Tom Rini
2019-11-08  3:31     ` Walter Lozano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.