fit: Use the libfdt subnode iterator

Replace fdt_next_node() with depth tracking with fdt_for_each_subnode()
which has been available for some time.

This also fixes a latent bug where the default configuration was being
read from the wrong node offset. It happened to work before because
noffset ended up at the right value after the images loop.

Series-to: concept
Series-cc: heinrich
Cover-letter:
fit: Improve and test the code to print FIT info
The code for printing information about FITs is fairly old and not that
easy to maintain. It also lacks tests.

This series adds some tests, moves the code into its own file and then
adds a series of helpers to deal with the intricacies of printing each
item.

This provides a binary-size reduction of about 320 bytes on aarch64.
END

Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>

boot/fit_print.c

@@ -15,10 +15,10 @@
 #include <time.h>
 #include <linux/libfdt.h>
 #else
+#include <linux/compiler.h>
 #include <log.h>
 #include <malloc.h>
 #include <mapmem.h>
-#include <linux/compiler.h>
 #endif
 
 #include <image.h>
@@ -35,7 +35,7 @@ void fit_print_init(struct fit_print_ctx *ctx, const void *fit)
 {
 	ctx->fit = fit;
 	ctx->indent = IMAGE_INDENT;
-	ctx->tab = 17 + IMAGE_INDENT;
+	ctx->tab = 20;
 }
 
 /**

drivers/core/Makefile

@@ -16,7 +16,6 @@ ifndef CONFIG_DM_DEV_READ_INLINE
 obj-$(CONFIG_OF_CONTROL) += read.o
 endif
 obj-$(CONFIG_$(PHASE_)OF_PLATDATA) += read.o
-obj-$(CONFIG_OF_CONTROL) += of_extra.o read_extra.o
-obj-$(CONFIG_$(PHASE_)OF_REAL) += ofnode.o ofnode_graph.o
+obj-$(CONFIG_OF_CONTROL) += of_extra.o ofnode.o read_extra.o ofnode_graph.o
 
 ccflags-$(CONFIG_DM_DEBUG) += -DDEBUG

include/dm/ofnode.h

@@ -140,7 +140,6 @@ static inline void oftree_dispose(oftree tree) {}
 
 #endif /* OFNODE_MULTI_TREE */
 
-#if CONFIG_IS_ENABLED(OF_REAL)
 /**
  * oftree_new() - Create a new, empty tree
  *
@@ -161,17 +160,6 @@ int oftree_new(oftree *treep);
  * error
  */
 int oftree_to_fdt(oftree tree, struct abuf *buf);
-#else
-static inline int oftree_new(oftree *treep)
-{
-	return -ENOSYS;
-}
-
-static inline int oftree_to_fdt(oftree tree, struct abuf *buf)
-{
-	return -ENOSYS;
-}
-#endif /* OF_REAL */
 
 /**
  * ofnode_to_np() - convert an ofnode to a live DT node pointer
@@ -399,7 +387,6 @@ static inline oftree oftree_from_np(struct device_node *root)
 	return tree;
 }
 
-#if CONFIG_IS_ENABLED(OF_REAL)
 /**
  * ofnode_name_eq() - Check a node name ignoring its unit address
  *
@@ -496,56 +483,6 @@ int ofnode_read_u32_index(ofnode node, const char *propname, int index,
  */
 int ofnode_read_u64_index(ofnode node, const char *propname, int index,
 			  u64 *outp);
-#else
-static inline bool ofnode_name_eq(ofnode node, const char *name)
-{
-	return false;
-}
-
-static inline bool ofnode_name_eq_unit(ofnode node, const char *name)
-{
-	return false;
-}
-
-static inline int ofnode_read_u8(ofnode node, const char *propname, u8 *outp)
-{
-	return -EINVAL;
-}
-
-static inline u8 ofnode_read_u8_default(ofnode node, const char *propname,
-					u8 def)
-{
-	return def;
-}
-
-static inline int ofnode_read_u16(ofnode node, const char *propname, u16 *outp)
-{
-	return -EINVAL;
-}
-
-static inline u16 ofnode_read_u16_default(ofnode node, const char *propname,
-					  u16 def)
-{
-	return def;
-}
-
-static inline int ofnode_read_u32(ofnode node, const char *propname, u32 *outp)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_u32_index(ofnode node, const char *propname,
-					int index, u32 *outp)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_u64_index(ofnode node, const char *propname,
-					int index, u64 *outp)
-{
-	return -EINVAL;
-}
-#endif /* OF_REAL */
 
 /**
  * ofnode_read_s32() - Read a 32-bit integer from a property
@@ -561,7 +498,6 @@ static inline int ofnode_read_s32(ofnode node, const char *propname,
 	return ofnode_read_u32(node, propname, (u32 *)outp);
 }
 
-#if CONFIG_IS_ENABLED(OF_REAL)
 /**
  * ofnode_read_u32_default() - Read a 32-bit integer from a property
  *
@@ -684,72 +620,8 @@ ofnode ofnode_find_subnode(ofnode node, const char *subnode_name);
  * subnode)
  */
 ofnode ofnode_find_subnode_unit(ofnode node, const char *subnode_name);
-#else
-static inline u32 ofnode_read_u32_default(ofnode node, const char *propname,
-					  u32 def)
-{
-	return def;
-}
 
-static inline u32 ofnode_read_u32_index_default(ofnode node,
-						const char *propname,
-						int index, u32 def)
-{
-	return def;
-}
-
-static inline int ofnode_read_s32_default(ofnode node, const char *propname,
-					  s32 def)
-{
-	return def;
-}
-
-static inline int ofnode_read_u64(ofnode node, const char *propname, u64 *outp)
-{
-	return -EINVAL;
-}
-
-static inline u64 ofnode_read_u64_default(ofnode node, const char *propname,
-					  u64 def)
-{
-	return def;
-}
-
-static inline const void *ofnode_read_prop(ofnode node, const char *propname,
-					   int *sizep)
-{
-	return NULL;
-}
-
-static inline const char *ofnode_read_string(ofnode node, const char *propname)
-{
-	return NULL;
-}
-
-static inline int ofnode_read_u32_array(ofnode node, const char *propname,
-					u32 *out_values, size_t sz)
-{
-	return -EINVAL;
-}
-
-static inline bool ofnode_read_bool(ofnode node, const char *propname)
-{
-	return false;
-}
-
-static inline ofnode ofnode_find_subnode(ofnode node, const char *subnode_name)
-{
-	return ofnode_null();
-}
-
-static inline ofnode ofnode_find_subnode_unit(ofnode node,
-					      const char *subnode_name)
-{
-	return ofnode_null();
-}
-#endif
-
-#if CONFIG_IS_ENABLED(OF_REAL) && CONFIG_IS_ENABLED(DM_INLINE_OFNODE)
+#if CONFIG_IS_ENABLED(DM_INLINE_OFNODE)
 #include <asm/global_data.h>
 
 static inline bool ofnode_is_enabled(ofnode node)
@@ -781,7 +653,7 @@ static inline ofnode ofnode_next_subnode(ofnode node)
 	return offset_to_ofnode(
 		fdt_next_subnode(gd->fdt_blob, ofnode_to_offset(node)));
 }
-#elif CONFIG_IS_ENABLED(OF_REAL)
+#else
 /**
  * ofnode_is_enabled() - Checks whether a node is enabled.
  * This looks for a 'status' property. If this exists, then returns true if
@@ -811,47 +683,8 @@ ofnode ofnode_first_subnode(ofnode node);
  * has no more siblings)
  */
 ofnode ofnode_next_subnode(ofnode node);
-#else
-static inline bool ofnode_is_enabled(ofnode node)
-{
-	return false;
-}
+#endif /* DM_INLINE_OFNODE */
 
-static inline ofnode ofnode_first_subnode(ofnode node)
-{
-	return ofnode_null();
-}
-
-static inline ofnode ofnode_next_subnode(ofnode node)
-{
-	return ofnode_null();
-}
-#endif
-
-/**
- * ofnode_for_each_subnode() - iterate over all subnodes of a parent
- *
- * @node:       child node (ofnode, lvalue)
- * @parent:     parent node (ofnode)
- *
- * This is a wrapper around a for loop and is used like so::
- *
- *   ofnode node;
- *   ofnode_for_each_subnode(node, parent) {
- *       Use node
- *       ...
- *   }
- *
- * Note that this is implemented as a macro and @node is used as
- * iterator in the loop. The parent variable can be a constant or even a
- * literal.
- */
-#define ofnode_for_each_subnode(node, parent) \
-	for (node = ofnode_first_subnode(parent); \
-	     ofnode_valid(node); \
-	     node = ofnode_next_subnode(node))
-
-#if CONFIG_IS_ENABLED(OF_REAL)
 /**
  * ofnode_get_parent() - get the ofnode's parent (enclosing ofnode)
  *
@@ -1607,6 +1440,29 @@ ofnode ofnode_by_compatible(ofnode from, const char *compat);
 ofnode ofnode_by_prop_value(ofnode from, const char *propname,
 			    const void *propval, int proplen);
 
+/**
+ * ofnode_for_each_subnode() - iterate over all subnodes of a parent
+ *
+ * @node:       child node (ofnode, lvalue)
+ * @parent:     parent node (ofnode)
+ *
+ * This is a wrapper around a for loop and is used like so::
+ *
+ *   ofnode node;
+ *   ofnode_for_each_subnode(node, parent) {
+ *       Use node
+ *       ...
+ *   }
+ *
+ * Note that this is implemented as a macro and @node is used as
+ * iterator in the loop. The parent variable can be a constant or even a
+ * literal.
+ */
+#define ofnode_for_each_subnode(node, parent) \
+	for (node = ofnode_first_subnode(parent); \
+	     ofnode_valid(node); \
+	     node = ofnode_next_subnode(node))
+
 /**
  * ofnode_for_each_compatible_node() - iterate over all nodes with a given
  *				       compatible string
@@ -1807,376 +1663,8 @@ ofnode ofnode_get_phy_node(ofnode eth_node);
  *	   error
  */
 phy_interface_t ofnode_read_phy_mode(ofnode mac_node);
-#else
-static inline ofnode ofnode_get_parent(ofnode node)
-{
-	return ofnode_null();
-}
 
-static inline const char *ofnode_get_name(ofnode node)
-{
-	return NULL;
-}
-
-static inline int ofnode_get_path(ofnode node, char *buf, int buflen)
-{
-	return -EINVAL;
-}
-
-static inline ofnode ofnode_get_by_phandle(uint phandle)
-{
-	return ofnode_null();
-}
-
-static inline ofnode oftree_get_by_phandle(oftree tree, uint phandle)
-{
-	return ofnode_null();
-}
-
-static inline int ofnode_read_size(ofnode node, const char *propname)
-{
-	return -EINVAL;
-}
-
-static inline fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index,
-						    fdt_size_t *size)
-{
-	return FDT_ADDR_T_NONE;
-}
-
-static inline fdt_addr_t ofnode_get_addr_size_index_notrans(ofnode node,
-							    int index,
-							    fdt_size_t *size)
-{
-	return FDT_ADDR_T_NONE;
-}
-
-static inline fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
-{
-	return FDT_ADDR_T_NONE;
-}
-
-static inline fdt_addr_t ofnode_get_addr(ofnode node)
-{
-	return FDT_ADDR_T_NONE;
-}
-
-static inline fdt_size_t ofnode_get_size(ofnode node)
-{
-	return FDT_SIZE_T_NONE;
-}
-
-static inline int ofnode_stringlist_search(ofnode node, const char *propname,
-					   const char *string)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_string_index(ofnode node, const char *propname,
-					   int index, const char **outp)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_string_count(ofnode node, const char *property)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_string_list(ofnode node, const char *property,
-					  const char ***listp)
-{
-	return -EINVAL;
-}
-
-static inline ofnode ofnode_parse_phandle(ofnode node, const char *phandle_name,
-					  int index)
-{
-	return ofnode_null();
-}
-
-static inline int ofnode_parse_phandle_with_args(ofnode node,
-						 const char *list_name,
-						 const char *cells_name,
-						 int cell_count, int index,
-						 struct ofnode_phandle_args *out_args)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_count_phandle_with_args(ofnode node,
-						 const char *list_name,
-						 const char *cells_name,
-						 int cell_count)
-{
-	return -EINVAL;
-}
-
-static inline ofnode oftree_parse_phandle(oftree tree, ofnode node,
-					  const char *phandle_name, int index)
-{
-	return ofnode_null();
-}
-
-static inline int oftree_parse_phandle_with_args(oftree tree, ofnode node,
-						 const char *list_name,
-						 const char *cells_name,
-						 int cell_count, int index,
-						 struct ofnode_phandle_args *out_args)
-{
-	return -EINVAL;
-}
-
-static inline int oftree_count_phandle_with_args(oftree tree, ofnode node,
-						 const char *list_name,
-						 const char *cells_name,
-						 int cell_count)
-{
-	return -EINVAL;
-}
-
-static inline ofnode ofnode_path(const char *path)
-{
-	return ofnode_null();
-}
-
-static inline ofnode oftree_path(oftree tree, const char *path)
-{
-	return ofnode_null();
-}
-
-static inline ofnode oftree_root(oftree tree)
-{
-	return ofnode_null();
-}
-
-static inline const void *ofnode_read_chosen_prop(const char *propname,
-						  int *sizep)
-{
-	return NULL;
-}
-
-static inline const char *ofnode_read_chosen_string(const char *propname)
-{
-	return NULL;
-}
-
-static inline ofnode ofnode_get_chosen_node(const char *propname)
-{
-	return ofnode_null();
-}
-
-static inline int ofnode_read_baud(void)
-{
-	return -EINVAL;
-}
-
-static inline const void *ofnode_read_aliases_prop(const char *propname,
-						   int *sizep)
-{
-	return NULL;
-}
-
-static inline ofnode ofnode_get_aliases_node(const char *propname)
-{
-	return ofnode_null();
-}
-
-static inline int ofnode_decode_display_timing(ofnode node, int index,
-					       struct display_timing *config)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_decode_panel_timing(ofnode node,
-					     struct display_timing *config)
-{
-	return -EINVAL;
-}
-
-static inline const void *ofnode_get_property(ofnode node, const char *propname,
-					      int *lenp)
-{
-	return NULL;
-}
-
-static inline bool ofnode_has_property(ofnode node, const char *propname)
-{
-	return false;
-}
-
-static inline int ofnode_first_property(ofnode node, struct ofprop *prop)
-{
-	return -FDT_ERR_NOTFOUND;
-}
-
-static inline int ofnode_next_property(struct ofprop *prop)
-{
-	return -FDT_ERR_NOTFOUND;
-}
-
-static inline const void *ofprop_get_property(const struct ofprop *prop,
-					      const char **propname, int *lenp)
-{
-	return NULL;
-}
-
-static inline fdt_addr_t ofnode_get_addr_size(ofnode node, const char *propname,
-					      fdt_size_t *sizep)
-{
-	return FDT_ADDR_T_NONE;
-}
-
-static inline const uint8_t *ofnode_read_u8_array_ptr(ofnode node,
-						      const char *propname,
-						      size_t sz)
-{
-	return NULL;
-}
-
-static inline int ofnode_read_pci_addr(ofnode node, enum fdt_pci_space type,
-				       const char *propname,
-				       struct fdt_pci_addr *addr,
-				       fdt_size_t *size)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_eth_phy_id(ofnode node, u16 *vendor, u16 *device)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_addr_cells(ofnode node)
-{
-	return 0;
-}
-
-static inline int ofnode_read_size_cells(ofnode node)
-{
-	return 0;
-}
-
-static inline int ofnode_read_simple_addr_cells(ofnode node)
-{
-	return 0;
-}
-
-static inline int ofnode_read_simple_size_cells(ofnode node)
-{
-	return 0;
-}
-
-static inline bool ofnode_pre_reloc(ofnode node)
-{
-	return false;
-}
-
-static inline int ofnode_read_resource(ofnode node, uint index,
-				       struct resource *res)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_resource_byname(ofnode node, const char *name,
-					      struct resource *res)
-{
-	return -EINVAL;
-}
-
-static inline ofnode ofnode_by_compatible(ofnode from, const char *compat)
-{
-	return ofnode_null();
-}
-
-static inline ofnode ofnode_by_prop_value(ofnode from, const char *propname,
-					  const void *propval, int proplen)
-{
-	return ofnode_null();
-}
-
-static inline int ofnode_get_child_count(ofnode parent)
-{
-	return 0;
-}
-
-static inline u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr)
-{
-	return OF_BAD_ADDR;
-}
-
-static inline u64 ofnode_translate_dma_address(ofnode node,
-					       const fdt32_t *in_addr)
-{
-	return OF_BAD_ADDR;
-}
-
-static inline int ofnode_get_dma_range(ofnode node, phys_addr_t *cpu,
-				       dma_addr_t *bus, u64 *size)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_device_is_compatible(ofnode node, const char *compat)
-{
-	return 0;
-}
-
-static inline int ofnode_write_prop(ofnode node, const char *propname,
-				    const void *value, int len, bool copy)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_write_string(ofnode node, const char *propname,
-				      const char *value)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_write_u32(ofnode node, const char *propname, u32 value)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_write_u64(ofnode node, const char *propname, u64 value)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_write_bool(ofnode node, const char *propname,
-				    bool value)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_delete_prop(ofnode node, const char *propname)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_set_enabled(ofnode node, bool value)
-{
-	return -ENOSYS;
-}
-
-static inline ofnode ofnode_get_phy_node(ofnode eth_node)
-{
-	return ofnode_null();
-}
-
-static inline phy_interface_t ofnode_read_phy_mode(ofnode mac_node)
-{
-	return PHY_INTERFACE_MODE_NA;
-}
-#endif /* OF_REAL */
-
-#if CONFIG_IS_ENABLED(OF_REAL)
+#if CONFIG_IS_ENABLED(DM)
 /**
  * ofnode_conf_read_bool() - Read a boolean value from the U-Boot config
  *
@@ -2314,7 +1802,8 @@ int ofnode_read_bootscript_address(u64 *bootscr_address, u64 *bootscr_offset);
  */
 int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset,
 				 u64 *bootscr_flash_size);
-#else
+
+#else /* CONFIG_DM */
 static inline bool ofnode_conf_read_bool(const char *prop_name)
 {
 	return false;
@@ -2330,29 +1819,7 @@ static inline const char *ofnode_conf_read_str(const char *prop_name)
 	return NULL;
 }
 
-static inline bool ofnode_options_read_bool(const char *prop_name)
-{
-	return false;
-}
-
-static inline int ofnode_options_read_int(const char *prop_name, int default_val)
-{
-	return default_val;
-}
-
-static inline const char *ofnode_options_read_str(const char *prop_name)
-{
-	return NULL;
-}
-
-static inline int ofnode_options_get_by_phandle(const char *prop_name,
-						ofnode *nodep)
-{
-	return -EINVAL;
-}
-
-static inline int ofnode_read_bootscript_address(u64 *bootscr_address,
-						 u64 *bootscr_offset)
+static inline int ofnode_read_bootscript_address(u64 *bootscr_address, u64 *bootscr_offset)
 {
 	return -EINVAL;
 }
@@ -2362,9 +1829,9 @@ static inline int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset,
 {
 	return -EINVAL;
 }
-#endif /* OF_REAL */
 
-#if CONFIG_IS_ENABLED(OF_REAL)
+#endif /* CONFIG_DM */
+
 /**
  * of_add_subnode() - add a new subnode to a node
  *
@@ -2420,28 +1887,5 @@ int ofnode_copy_node(ofnode dst_parent, const char *name, ofnode src,
  *
  */
 int ofnode_delete(ofnode *nodep);
-#else
-static inline int ofnode_add_subnode(ofnode parent, const char *name,
-				     ofnode *nodep)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_copy_props(ofnode dst, ofnode src)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_copy_node(ofnode dst_parent, const char *name,
-				   ofnode src, ofnode *nodep)
-{
-	return -ENOSYS;
-}
-
-static inline int ofnode_delete(ofnode *nodep)
-{
-	return -ENOSYS;
-}
-#endif /* OF_REAL */
 
 #endif

include/image.h

@@ -1213,14 +1213,12 @@ struct fit_print_ctx {
 	int tab;
 };
 
-#if CONFIG_IS_ENABLED(FIT_PRINT)
-
 /**
  * fit_print_init() - initialize FIT print context
  * @ctx: pointer to FIT print context to initialize
  * @fit: pointer to the FIT format image header
  *
- * This inits a fit_print_ctx structure with the given FIT image.
+ * This initializes a fit_print_ctx structure with the given FIT image.
  */
 void fit_print_init(struct fit_print_ctx *ctx, const void *fit);
 
@@ -1228,6 +1226,7 @@ void fit_print_init(struct fit_print_ctx *ctx, const void *fit);
  * fit_print() - prints out the contents of the FIT format image
  * @ctx: pointer to FIT print context
  *
+ * fit_print() formats a multi line FIT image contents description.
  * The routine prints out FIT image properties (root node level) followed by
  * the details of each component image.
  *
@@ -1237,9 +1236,23 @@ void fit_print_init(struct fit_print_ctx *ctx, const void *fit);
 void fit_print(struct fit_print_ctx *ctx);
 
 /**
- * fit_image_print() - prints out the FIT component image details
+ * fit_print_contents() - prints out the contents of the FIT format image
+ * @fit: pointer to the FIT format image header
+ * @p: pointer to prefix string
+ *
+ * fit_print_contents() formats a multi line FIT image contents description.
+ * The routine prints out FIT image properties (root node level) followed by
+ * the details of each component image.
+ *
+ * returns:
+ *     no returned results
+ */
+void fit_print_contents(const void *fit);
+
+/**
+ * fit_image_print - prints out the FIT component image details
  * @ctx: pointer to FIT print context
- * @noffset: offset of the component image node
+ * @image_noffset: offset of the component image node
  *
  * fit_image_print() lists all mandatory properties for the processed component
  * image. If present, hash nodes are printed out as well. Load
@@ -1252,31 +1265,6 @@ void fit_print(struct fit_print_ctx *ctx);
  */
 void fit_image_print(struct fit_print_ctx *ctx, int noffset);
 
-/**
- * fit_print_contents() - prints out the contents of the FIT format image
- * @fit: pointer to the FIT format image header
- * @p: pointer to prefix string
- *
- * This formats a multi line FIT image contents description.
- * The routine prints out FIT image properties (root node level) followed by
- * the details of each component image.
- *
- * returns:
- *     no returned results
- */
-void fit_print_contents(const void *fit);
-
-#else /* !FIT_PRINT */
-
-static inline void fit_print_init(struct fit_print_ctx *ctx, const void *fit)
-{
-}
-static inline void fit_print(const void *fit) {}
-static inline void fit_image_print(const void *fit, int noffset) {}
-static inline void fit_print_contents(const void *fit) {}
-
-#endif
-
 /**
  * fit_get_end - get FIT image size
  * @fit: pointer to the FIT format image header

test/boot/fit_print.c

@@ -2,8 +2,8 @@
 /*
  * Test for FIT image printing
  *
- * Copyright 2025 Canonical Ltd
- * Written by Simon Glass <simon.glass@canonical.com>
+ * Copyright 2025 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
  */
 
 #include <image.h>
@@ -14,7 +14,7 @@
 #include "bootstd_common.h"
 
 /* Test fit_print_contents() output */
-static int test_fit_print_norun(struct unit_test_state *uts)
+static int test_fit_print(struct unit_test_state *uts)
 {
 	char fname[256];
 	void *fit;
@@ -157,10 +157,10 @@ static int test_fit_print_norun(struct unit_test_state *uts)
 
 	return 0;
 }
-BOOTSTD_TEST(test_fit_print_norun, UTF_CONSOLE | UTF_MANUAL);
+BOOTSTD_TEST(test_fit_print, UTF_CONSOLE);
 
 /* Test fit_print_contents() with missing FIT description */
-static int test_fit_print_no_desc_norun(struct unit_test_state *uts)
+static int test_fit_print_no_desc(struct unit_test_state *uts)
 {
 	char fname[256];
 	void *fit;
@@ -168,9 +168,8 @@ static int test_fit_print_no_desc_norun(struct unit_test_state *uts)
 	ulong addr;
 	int size;
 
-	/* Load the FIT created by the Python test (with deleted description) */
-	ut_assertok(os_persistent_file(fname, sizeof(fname),
-				       "test-fit-nodesc.fit"));
+	/* Load the FIT created by the Python test (which deleted description) */
+	ut_assertok(os_persistent_file(fname, sizeof(fname), "test-fit-nodesc.fit"));
 	ut_assertok(os_read_file(fname, &buf, &size));
 
 	/* Copy to address 0x10000 and print from there */
@@ -189,4 +188,4 @@ static int test_fit_print_no_desc_norun(struct unit_test_state *uts)
 
 	return 0;
 }
-BOOTSTD_TEST(test_fit_print_no_desc_norun, UTF_CONSOLE | UTF_MANUAL);
+BOOTSTD_TEST(test_fit_print_no_desc, UTF_CONSOLE);

test/py/tests/test_fit_print.py

@@ -1,17 +1,13 @@
 # SPDX-License-Identifier: GPL-2.0+
-# Copyright 2025 Canonical Ltd
-# Written by Simon Glass <simon.glass@canonical.com>
+# Copyright (c) 2025, Google Inc.
 
 """Test for FIT image printing"""
 
 import os
-import re
-import time
-
 import pytest
-
-import fit_util
+import shutil
 import utils
+import fit_util
 
 # ITS for testing FIT printing with hashes, ramdisk, and multiple configs
 PRINT_ITS = '''
@@ -136,14 +132,13 @@ PRINT_ITS = '''
 };
 '''
 
-def build_test_fit(ubman, fit):
-    """Build a test FIT image with all components
-
-    Args:
-        ubman (ConsoleBase): U-Boot manager object
-        fit (str): Path where the FIT file should be created
-    """
-    # pylint: disable=too-many-locals
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('fit')
+@pytest.mark.buildconfigspec('fit_print')
+@pytest.mark.requiredtool('dtc')
+@pytest.mark.requiredtool('openssl')
+def test_fit_print(ubman):
+    """Test fit_print_contents() via C unit test"""
     mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage')
 
     # Create test files (make kernel ~6.3K)
@@ -193,6 +188,7 @@ def build_test_fit(ubman, fit):
     }
     env = os.environ.copy()
     env['SOURCE_DATE_EPOCH'] = '1234567890'  # 2009-02-13 23:31:30 UTC
+    fit = fit_util.make_fname(ubman, 'test-fit.fit')
     its = fit_util.make_its(ubman, PRINT_ITS, params)
     utils.run_and_log(ubman, [mkimage, '-f', its, fit], env=env)
 
@@ -229,7 +225,7 @@ NlRJpZjsHNRPd0WFVxXnJRzZxkStoTwL2BhPtG3Xx1ReIkNVCxlu1Dk0rLLKl1nj
 S0n8gbs0Ht/ZckLk8mPclbk=
 -----END PRIVATE KEY-----'''
 
-    with open(tmpdir + 'test-key.key', 'w', encoding='utf-8') as f:
+    with open(tmpdir + 'test-key.key', 'w') as f:
         f.write(key_pem)
 
     utils.run_and_log(ubman,
@@ -243,189 +239,32 @@ S0n8gbs0Ht/ZckLk8mPclbk=
 
     # Sign the FIT configuration (use env for reproducible timestamp)
     utils.run_and_log(ubman, [mkimage, '-F', '-k', tmpdir, '-K', dtb,
-                              '-r', fit, '-c', 'Configuration signing'],
-                      env=env)
+                              '-r', fit, '-c', 'Configuration signing'], env=env)
 
-    # Delete the algo property from the hash-1 node to test invalid/unsupported
-    utils.run_and_log(ubman, ['fdtput', '-d', fit, '/images/script/hash-1',
-                              'algo'])
+    # Delete the algo property from script's hash-1 node to test invalid/unsupported algo
+    utils.run_and_log(ubman, ['fdtput', '-t', 's', fit, '/images/script/hash-1', 'algo', ''])
+    utils.run_and_log(ubman, ['fdtput', '-d', fit, '/images/script/hash-1', 'algo'])
 
-
-@pytest.mark.boardspec('sandbox')
-@pytest.mark.buildconfigspec('fit_print')
-@pytest.mark.requiredtool('dtc')
-@pytest.mark.requiredtool('openssl')
-def test_fit_print(ubman):
-    """Test fit_print_contents() via C unit test"""
-    fit = os.path.join(ubman.config.persistent_data_dir, 'test-fit.fit')
-    build_test_fit(ubman, fit)
+    # Copy to persistent directory with known name
+    persistent_fit = os.path.join(ubman.config.persistent_data_dir,
+                                  'test-fit.fit')
+    shutil.copy(fit, persistent_fit)
 
     # Run the C test which will load and verify this FIT
-    ubman.run_command('ut -f bootstd test_fit_print_norun')
+    ubman.run_command('ut -f bootstd test_fit_print')
     result = ubman.run_command('echo $?')
     assert '0' == result
 
+    # Test missing FIT description by deleting it and checking the output
+    fit_no_desc = fit_util.make_fname(ubman, 'test-fit-nodesc.fit')
+    shutil.copy(fit, fit_no_desc)
+    utils.run_and_log(ubman, ['fdtput', '-d', fit_no_desc, '/', 'description'])
 
-@pytest.mark.boardspec('sandbox')
-@pytest.mark.buildconfigspec('fit_print')
-@pytest.mark.requiredtool('dtc')
-@pytest.mark.requiredtool('openssl')
-def test_fit_print_no_desc(ubman):
-    """Test fit_print_contents() with missing FIT description"""
-    fit = os.path.join(ubman.config.persistent_data_dir, 'test-fit-nodesc.fit')
-    build_test_fit(ubman, fit)
+    persistent_fit_no_desc = os.path.join(ubman.config.persistent_data_dir,
+                                          'test-fit-nodesc.fit')
+    shutil.copy(fit_no_desc, persistent_fit_no_desc)
 
-    # Delete the description property
-    utils.run_and_log(ubman, ['fdtput', '-d', fit, '/', 'description'])
-
-    # Run the C test to check the missing description
-    ubman.run_command('ut -f bootstd test_fit_print_no_desc_norun')
+    # Run a second C test to check the missing description
+    ubman.run_command('ut -f bootstd test_fit_print_no_desc')
     result = ubman.run_command('echo $?')
     assert '0' == result
-
-@pytest.mark.boardspec('sandbox')
-@pytest.mark.buildconfigspec('fit_print')
-@pytest.mark.requiredtool('dtc')
-@pytest.mark.requiredtool('openssl')
-def test_fit_print_mkimage(ubman):
-    """Test 'mkimage -l' output on FIT image"""
-    mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage')
-    fit = fit_util.make_fname(ubman, 'test-fit-mkimage.fit')
-    build_test_fit(ubman, fit)
-
-    # Run mkimage -l and capture output
-    output = utils.run_and_log(ubman, [mkimage, '-l', fit])
-
-    # Extract the actual timestamp from mkimage output to avoid timezone issues
-    # mkimage uses localtime() which can vary based on system timezone
-    match = re.search(r'Created:\s+(.+)', output)
-    if not match:
-        raise ValueError("Could not find Created: line in mkimage output")
-    timestamp_str = match.group(1).strip()
-
-    expected_timestamp = 1234567890
-    # Validate timestamp is reasonable (SOURCE_DATE_EPOCH)
-    parsed_time = time.strptime(timestamp_str, '%a %b %d %H:%M:%S %Y')
-    parsed_timestamp = time.mktime(parsed_time)
-    time_diff = abs(parsed_timestamp - expected_timestamp)
-
-    # Check it is within 24 hours (86400 seconds)
-    assert time_diff < 86400, \
-        f"Timestamp {timestamp_str} is more than 24 hours from expected"
-
-    # Expected output (complete output from mkimage -l)
-    expected = f'''
-FIT description: Test FIT image for printing
-Created:         {timestamp_str}
- Image 0 (kernel)
-  Description:   Test kernel
-  Created:       {timestamp_str}
-  Type:          Kernel Image
-  Compression:   gzip compressed
-  Data Size:     327 Bytes = 0.32 KiB = 0.00 MiB
-  Architecture:  Sandbox
-  OS:            Linux
-  Load Address:  0x01000000
-  Entry Point:   0x01000000
-  Hash algo:     sha256
-  Hash value:    fad998b94ef12fdac0c347915d8b9b6069a4011399e1a2097638a2cb33244cee
- Image 1 (ramdisk)
-  Description:   Test ramdisk
-  Created:       {timestamp_str}
-  Type:          RAMDisk Image
-  Compression:   uncompressed
-  Data Size:     301 Bytes = 0.29 KiB = 0.00 MiB
-  Architecture:  Sandbox
-  OS:            Linux
-  Load Address:  0x02000000
-  Entry Point:   unavailable
-  Hash algo:     sha256
-  Hash value:    53e2a65d92ad890dcd89d83a1f95ad6b8206e0e4889548b035062fc494e7f655
- Image 2 (fdt-1)
-  Description:   Test FDT 1
-  Created:       {timestamp_str}
-  Type:          Flat Device Tree
-  Compression:   uncompressed
-  Data Size:     161 Bytes = 0.16 KiB = 0.00 MiB
-  Architecture:  Sandbox
-  Hash algo:     sha256
-  Hash value:    1264bc4619a1162736fdca8e63e44a1b009fbeaaa259c356b555b91186257ffb
- Image 3 (fdt-2)
-  Description:   Test FDT 2
-  Created:       {timestamp_str}
-  Type:          Flat Device Tree
-  Compression:   uncompressed
-  Data Size:     161 Bytes = 0.16 KiB = 0.00 MiB
-  Architecture:  Sandbox
-  Hash algo:     sha256
-  Hash value:    3a07e37c76dd48c2a17927981f0959758ac6fd0d649e2032143c5afeea9a98a4
- Image 4 (firmware-1)
-  Description:   Test Firmware 1
-  Created:       {timestamp_str}
-  Type:          Firmware
-  Compression:   uncompressed
-  Data Size:     3891 Bytes = 3.80 KiB = 0.00 MiB
-  Architecture:  Sandbox
-  OS:            Unknown OS
-  Load Address:  unavailable
-  Hash algo:     sha256
-  Hash value:    53f1358540a556282764ceaf2912e701d2e25902a6b069b329e57e3c59148414
- Image 5 (firmware-2)
-  Description:   Test Firmware 2
-  Created:       {timestamp_str}
-  Type:          Firmware
-  Compression:   uncompressed
-  Data Size:     3891 Bytes = 3.80 KiB = 0.00 MiB
-  Architecture:  Sandbox
-  OS:            Unknown OS
-  Load Address:  unavailable
-  Hash algo:     sha256
-  Hash value:    6a12ac2283f3c9605113b5c2287e983da5671d8d0015381009d75169526676f1
- Image 6 (fpga)
-  Description:   Test FPGA
-  Created:       {timestamp_str}
-  Type:          FPGA Image
-  Compression:   uncompressed
-  Data Size:     4291 Bytes = 4.19 KiB = 0.00 MiB
-  Load Address:  unavailable
-  Hash algo:     sha256
-  Hash value:    2f588e50e95abc7f9d6afd1d5b3f2bf285cccd55efcf52f47a975dbff3265622
- Image 7 (script)
-  Description:   unavailable
-  Created:       {timestamp_str}
-  Type:          Script
-  Compression:   uncompressed
-  Data Size:     3791 Bytes = 3.70 KiB = 0.00 MiB
-  Hash algo:     invalid/unsupported
- Default Configuration: 'conf-1'
- Configuration 0 (conf-1)
-  Description:   Test configuration
-  Kernel:        kernel
-  Init Ramdisk:  ramdisk
-  FDT:           fdt-1
-  Compatible:    vendor,board-1.0
-                 vendor,board
-  Sign algo:     sha256,rsa2048:test-key
-  Sign padding:  pkcs-1.5
-  Sign value:    c20f64d9bf79ddb0b1a69293b2375ad88e70536684705a9577f2156e6da4df6d
-  Timestamp:     {timestamp_str}
- Configuration 1 (conf-2)
-  Description:   Alternate configuration
-  Kernel:        kernel
-  FDT:           fdt-1
-                 fdt-2
-  FPGA:          fpga
-  Loadables:     firmware-1
-                 firmware-2
-  Compatible:    vendor,board-2.0
- Configuration 2 (conf-3)
-  Description:   unavailable
-  Kernel:        unavailable
-  Loadables:     script
-'''.strip().split('\n')
-
-    lines = output.split('\n')
-    for seq, (expected, line) in enumerate(zip(expected, lines)):
-        exp = expected[:80]
-        act = line[:80]
-        assert exp == act, f"line {seq + 1}: expect '{exp}' got '{act}'"

tools/Makefile

@@ -72,7 +72,7 @@ hostprogs-y += file2include
 endif
 
 FIT_OBJS-y := fit_common.o fit_image.o image-host.o generated/boot/image-fit.o
-FIT_OBJS-y += generated/boot/fit_print.o
+FIT_OBJS-$(CONFIG_FIT_PRINT) += generated/boot/fit_print.o
 FIT_SIG_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := image-sig-host.o generated/boot/image-fit-sig.o
 FIT_CIPHER_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := generated/boot/image-cipher.o