Change in ...coreboot[master]: device_tree: Update comment style to C89 - coreboot-gerrit

18 May 2019

Hello Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32875
to review the following change.
Change subject: device_tree: Update comment style to C89
......................................................................
device_tree: Update comment style to C89
This code was copied from depthcharge which uses C99 comment style, but
coreboot uses C89 comment style. Update to match coreboot.
Change-Id: Ib67bb9ff17b7688826071453ab58894a0835ce10
Signed-off-by: Julius Werner jwerner@chromium.org
---
M src/include/device_tree.h
M src/lib/device_tree.c
M src/lib/fit.c
3 files changed, 119 insertions(+), 109 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/32875/1

diff --git a/src/include/device_tree.h b/src/include/device_tree.h
index 8d86bc5..56a9361 100644
--- a/src/include/device_tree.h
+++ b/src/include/device_tree.h
@@ -74,9 +74,9 @@
    const char *name;
    uint32_t phandle;
-	// List of struct device_tree_property-s.
+	/* List of struct device_tree_property-s. */
    struct list_node properties;
-	// List of struct device_tree_nodes.
+	/* List of struct device_tree_nodes. */
    struct list_node children;
struct list_node list_node;
@@ -108,18 +108,18 @@
  * which were consumed reading the requested value.
  */
-// Read the property, if any, at offset offset.
+/* Read the property, if any, at offset offset. */
 int fdt_next_property(const void *blob, uint32_t offset,
    	      struct fdt_property *prop);
-// Read the name of the node, if any, at offset offset.
+/* Read the name of the node, if any, at offset offset. */
 int fdt_node_name(const void *blob, uint32_t offset, const char **name);
void fdt_print_node(const void *blob, uint32_t offset);
 int fdt_skip_node(const void *blob, uint32_t offset);
-// Read a flattened device tree into a heirarchical structure which refers to
-// the contents of the flattened tree in place. Modifying the flat tree
-// invalidates the unflattened one.
+/* Read a flattened device tree into a heirarchical structure which refers to
+   the contents of the flattened tree in place. Modifying the flat tree
+   invalidates the unflattened one. */
 struct device_tree *fdt_unflatten(const void *blob);
@@ -128,42 +128,42 @@
  * Unflattened device tree functions.
  */
-// Figure out how big a device tree would be if it were flattened.
+/* Figure out how big a device tree would be if it were flattened. */
 uint32_t dt_flat_size(const struct device_tree *tree);
-// Flatten a device tree into the buffer pointed to by dest.
+/* Flatten a device tree into the buffer pointed to by dest. */
 void dt_flatten(const struct device_tree *tree, void *dest);
 void dt_print_node(const struct device_tree_node *node);
-// Read #address-cells and #size-cells properties from a node.
+/* Read #address-cells and #size-cells properties from a node. */
 void dt_read_cell_props(const struct device_tree_node *node, u32 *addrcp,
    		u32 *sizecp);
-// Look up or create a node relative to a parent node, through its path
-// represented as an array of strings.
+/* Look up or create a node relative to a parent node, through its path
+   represented as an array of strings. */
 struct device_tree_node *dt_find_node(struct device_tree_node *parent, const char **path,
    		     u32 *addrcp, u32 *sizecp, int create);
 struct device_tree_node *dt_find_node_by_phandle(struct device_tree_node *root,
    					 uint32_t phandle);
-// Look up or create a node in the tree, through its path
-// represented as a string of '/' separated node names.
+/* Look up or create a node in the tree, through its path
+   represented as a string of '/' separated node names. */
 struct device_tree_node *dt_find_node_by_path(struct device_tree *tree,
    		const char *path, u32 *addrcp, u32 *sizecp, int create);
-// Look up a node through an alias.
+/* Look up a node through an alias. */
 struct device_tree_node *dt_find_node_by_alias(struct device_tree *tree,
    				       const char *alias);
-// Look up a node relative to a parent node, through its compatible string.
+/* Look up a node relative to a parent node, through its compatible string. */
 struct device_tree_node *dt_find_compat(struct device_tree_node *parent, const char *compatible);
-// Look up the next child of a parent node, through its compatible string. It
-// uses child pointer as the marker to find next.
+/* Look up the next child of a parent node, through its compatible string. It
+   uses child pointer as the marker to find next. */
 struct device_tree_node *dt_find_next_compat_child(struct device_tree_node *parent,
    				  struct device_tree_node *child,
    				  const char *compat);
-// Look up a node relative to a parent node, through its property value.
+/* Look up a node relative to a parent node, through its property value. */
 struct device_tree_node *dt_find_prop_value(struct device_tree_node *parent, const char *name,
    			   void *data, size_t size);
-// Write src into *dest as a 'length'-byte big-endian integer.
+/* Write src into *dest as a 'length'-byte big-endian integer. */
 void dt_write_int(u8 *dest, u64 src, size_t length);
-// Delete a property
+/* Delete a property */
 void dt_delete_prop(struct device_tree_node *node, const char *name);
-// Add different kinds of properties to a node, or update existing ones.
+/* Add different kinds of properties to a node, or update existing ones. */
 void dt_add_bin_prop(struct device_tree_node *node, const char *name,
    	     void *data, size_t size);
 void dt_add_string_prop(struct device_tree_node *node, const char *name,
diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c
index 05f2ea9..338ac45 100644
--- a/src/lib/device_tree.c
+++ b/src/lib/device_tree.c
@@ -133,7 +133,7 @@
    	offset += size;
    }
-	printk(BIOS_DEBUG, "\n");	// empty line between props and nodes
+	printk(BIOS_DEBUG, "\n");	/* empty line between props and nodes */
while ((size = print_flat_node(blob, offset, depth + 1)))
    	offset += size;
@@ -277,9 +277,9 @@
    uint32_t min_offset = 0;
    min_offset = MIN(struct_offset, strings_offset);
    min_offset = MIN(min_offset, reserve_offset);
-	// Assume everything up to the first non-header component is part of
-	// the header and needs to be preserved. This will protect us against
-	// new elements being added in the future.
+	/* Assume everything up to the first non-header component is part of
+	   the header and needs to be preserved. This will protect us against
+	   new elements being added in the future. */
    tree->header_size = min_offset;
struct device_tree_reserve_map_entry *entry;
@@ -307,25 +307,25 @@
 static void dt_flat_prop_size(struct device_tree_property *prop,
    		      uint32_t *struct_size, uint32_t *strings_size)
 {
-	// Starting token.
+	/* Starting token. */
    *struct_size += sizeof(uint32_t);
-	// Size.
+	/* Size. */
    *struct_size += sizeof(uint32_t);
-	// Name offset.
+	/* Name offset. */
    *struct_size += sizeof(uint32_t);
-	// Property value.
+	/* Property value. */
    *struct_size += ALIGN_UP(prop->prop.size, sizeof(uint32_t));
-	// Property name.
+	/* Property name. */
    *strings_size += strlen(prop->prop.name) + 1;
 }
static void dt_flat_node_size(struct device_tree_node *node,
    		      uint32_t *struct_size, uint32_t *strings_size)
 {
-	// Starting token.
+	/* Starting token. */
    *struct_size += sizeof(uint32_t);
-	// Node name.
+	/* Node name. */
    *struct_size += ALIGN_UP(strlen(node->name) + 1, sizeof(uint32_t));
struct device_tree_property *prop;
@@ -336,7 +336,7 @@
    list_for_each(child, node->children, list_node)
    	dt_flat_node_size(child, struct_size, strings_size);
-	// End token.
+	/* End token. */
    *struct_size += sizeof(uint32_t);
 }
@@ -353,7 +353,7 @@
    dt_flat_node_size(tree->root, &struct_size, &strings_size);
size += struct_size;
-	// End token.
+	/* End token. */
    size += sizeof(uint32_t);
size += strings_size;
@@ -478,7 +478,7 @@
 static void print_node(const struct device_tree_node *node, int depth)
 {
    print_indent(depth);
-	if (depth == 0)		// root node has no name, print a starting slash
+	if (depth == 0)	/* root node has no name, print a starting slash */
    	printk(BIOS_DEBUG, "/");
    printk(BIOS_DEBUG, "%s {\n", node->name);
@@ -486,7 +486,7 @@
    list_for_each(prop, node->properties, list_node)
    	print_property(&prop->prop, depth + 1);
-	printk(BIOS_DEBUG, "\n");	// empty line between props and nodes
+	printk(BIOS_DEBUG, "\n");	/* empty line between props and nodes */
struct device_tree_node *child;
    list_for_each(child, node->children, list_node)
@@ -546,13 +546,13 @@
 {
    struct device_tree_node *node, *found = NULL;
-	// Update #address-cells and #size-cells for this level.
+	/* Update #address-cells and #size-cells for this level. */
    dt_read_cell_props(parent, addrcp, sizecp);
if (!*path)
    	return parent;
-	// Find the next node in the path, if it exists.
+	/* Find the next node in the path, if it exists. */
    list_for_each(node, parent->children, list_node) {
    	if (!strcmp(node->name, *path)) {
    		found = node;
@@ -560,7 +560,7 @@
    	}
    }
-	// Otherwise create it or return NULL.
+	/* Otherwise create it or return NULL. */
    if (!found) {
    	if (!create)
    		return NULL;
@@ -607,8 +607,8 @@
    int i;
    struct device_tree_node *node = NULL;
-	if (path[0] == '/') { // regular path
-		if (path[1] == '\0') {	// special case: "/" is root node
+	if (path[0] == '/') { /* regular path */
+		if (path[1] == '\0') {	/* special case: "/" is root node */
    		dt_read_cell_props(tree->root, addrcp, sizecp);
    		return tree->root;
    	}
@@ -618,7 +618,7 @@
    		return NULL;
parent = tree->root;
-	} else { // alias
+	} else { /* alias */
    	char *alias;
alias = duped_str = strdup(path);
@@ -639,7 +639,7 @@
    	}
if (!sub_path) {
-			// it's just the alias, no sub-path
+			/* it's just the alias, no sub-path */
    		free(duped_str);
    		return parent;
    	}
@@ -754,7 +754,7 @@
 struct device_tree_node *dt_find_compat(struct device_tree_node *parent,
    				const char *compat)
 {
-	// Check if the parent node itself is compatible.
+	/* Check if the parent node itself is compatible. */
    if (dt_check_compat_match(parent, compat))
    	return parent;
@@ -1089,11 +1089,11 @@
    if (!reserved)
    	return NULL;
-	// Binding doc says this should have the same #{address,size}-cells as
-	// the root.
+	/* Binding doc says this should have the same #{address,size}-cells as
+	   the root. */
    dt_add_u32_prop(reserved, "#address-cells", addr);
    dt_add_u32_prop(reserved, "#size-cells", size);
-	// Binding doc says this should be empty (i.e., 1:1 mapping from root).
+	/* Binding doc says this should be empty (1:1 mapping from root). */
    dt_add_bin_prop(reserved, "ranges", NULL, 0);
return reserved;
@@ -1139,7 +1139,7 @@
 static uint32_t dt_adjust_all_phandles(struct device_tree_node *node,
    			       uint32_t base)
 {
-	uint32_t new_max = MAX(base, 1);  // make sure we don't return 0
+	uint32_t new_max = MAX(base, 1);  /* make sure we don't return 0 */
    struct device_tree_property *prop;
    struct device_tree_node *child;
@@ -1152,7 +1152,7 @@
    		if (!node->phandle)
    			return 0;
    		new_max = MAX(new_max, node->phandle);
-		}  // no break -- can have more than one phandle prop
+		}  /* no break -- can have more than one phandle prop */
list_for_each(child, node->children, list_node)
    	new_max = MAX(new_max, dt_adjust_all_phandles(child, base));
@@ -1178,11 +1178,13 @@
    struct device_tree_node *fixup_child;
    int i;
-	// For local fixups the /__local_fixup__ subtree contains the same node
-	// hierarchy as the main tree we're fixing up. Each property contains
-	// the fixup offsets for the respective property in the main tree. For
-	// each property in the fixup node, find the corresponding property in
-	// the base node and apply fixups to all offsets it specifies.
+	/*
+	 * For local fixups the /__local_fixup__ subtree contains the same node
+	 * hierarchy as the main tree we're fixing up. Each property contains
+	 * the fixup offsets for the respective property in the main tree. For
+	 * each property in the fixup node, find the corresponding property in
+	 * the base node and apply fixups to all offsets it specifies.
+	 */
    list_for_each(fixup_prop, fixup->properties, list_node) {
    	struct device_tree_property *base_prop = NULL;
    	list_for_each(prop, node->properties, list_node)
@@ -1191,8 +1193,8 @@
    			break;
    		}
-		// We should always find a corresponding base prop for a fixup,
-		// and fixup props contain a list of 32-bit fixup offsets.
+		/* We should always find a corresponding base prop for a fixup,
+		   and fixup props contain a list of 32-bit fixup offsets. */
    	if (!base_prop || fixup_prop->prop.size % sizeof(uint32_t))
    		return -1;
@@ -1202,8 +1204,8 @@
    			return -1;
    }
-	// Now recursively descend both the base tree and the /__local_fixups__
-	// subtree in sync to apply all fixups.
+	/* Now recursively descend both the base tree and the /__local_fixups__
+	   subtree in sync to apply all fixups. */
    list_for_each(fixup_child, fixup->children, list_node) {
    	struct device_tree_node *base_child = NULL;
    	list_for_each(child, node->children, list_node)
@@ -1212,7 +1214,7 @@
    			break;
    		}
-		// All fixup nodes should have a corresponding base node.
+		/* All fixup nodes should have a corresponding base node. */
    	if (!base_child)
    		return -1;
@@ -1236,10 +1238,10 @@
    		   const char *base_path)
 {
    struct device_tree_property *prop;
-	char buf[512];		// Should be enough for maximum DT path length?
-	char node_path[64];	// easily enough for /fragment@XXXX/__overlay__
+	char buf[512]; /* Should be enough for maximum DT path length? */
+	char node_path[64]; /* easily enough for /fragment@XXXX/__overlay__ */
-	if (!symbols)	// If the overlay has no /__symbols__ node, we're done!
+	if (!symbols) /* If the overlay has no /__symbols__ node, we're done! */
    	return;
int len = snprintf(node_path, sizeof(node_path), "/%s/__overlay__",
@@ -1273,10 +1275,10 @@
 {
    struct device_tree_property *prop;
-	// External fixup properties are encoded as "<path>:<prop>:<offset>".
+	/* External fixup properties are encoded as "<path>:<prop>:<offset>". */
    char *entry = fixup->prop.data;
    while ((void *)entry < fixup->prop.data + fixup->prop.size) {
-		// okay to destroy fixup property value, won't be needed again
+		/* okay to destroy fixup property value, won't need it again */
    	char *node_path = entry;
    	entry = strchr(node_path, ':');
    	if (!entry)
@@ -1301,18 +1303,18 @@
    			break;
    		}
-		// Move entry to first char after number, must be a '\0'.
+		/* Move entry to first char after number, must be a '\0'. */
    	uint32_t offset = skip_atoi((const char **)&entry);
    	if (!ovl_prop || offset + 4 > ovl_prop->prop.size || entry[0])
    		return -1;
-		entry++;  // jump over '\0' to potential next fixup
+		entry++;  /* jump over '\0' to potential next fixup */
be32enc(ovl_prop->prop.data + offset, phandle);
-		// If this is a /fragment@X:target property, update
-		// references to this fragment in the overlay __symbols__ now.
+		/* If this is a /fragment@X:target property, update references
+		   to this fragment in the overlay __symbols__ now. */
    	if (offset == 0 && !strcmp(prop_name, "target") &&
-		    !strchr(node_path + 1, '/')) // only toplevel nodes
+		    !strchr(node_path + 1, '/')) /* only toplevel nodes */
    		dt_fix_symbols(overlay_symbols, ovl_node, base_path);
    }
@@ -1339,27 +1341,29 @@
 {
    struct device_tree_property *fix;
-	// If we have any external fixups, the base tree must have /__symbols__.
+	/* If we have any external fixups, base tree must have /__symbols__. */
    if (!symbols)
    	return -1;
-	// Unlike /__local_fixups__, /__fixups__ is not a whole subtree that
-	// mirrors the node hierarchy. It's just a directory of fixup properties
-	// that each directly contain all information necessary to apply them.
+	/*
+	 * Unlike /__local_fixups__, /__fixups__ is not a whole subtree that
+	 * mirrors the node hierarchy. It's just a directory of fixup properties
+	 * that each directly contain all information necessary to apply them.
+	 */
    list_for_each(fix, fixups->properties, list_node) {
-		// The name of a fixup property is the label of the node we want
-		// a property to phandle-reference. Look it up in /__symbols__.
+		/* The name of a fixup property is the label of the node we want
+		   a property to phandle-reference. Look up in /__symbols__. */
    	const char *path = dt_find_string_prop(symbols, fix->prop.name);
    	if (!path)
    		return -1;
-		// Find the node the label pointed to to figure out its phandle.
+		/* Find node the label pointed to to figure out its phandle. */
    	struct device_tree_node *node = dt_find_node_by_path(tree, path,
    		NULL, NULL, 0);
    	if (!node)
    		return -1;
-		// Write it into the overlay property(s) pointing to that node.
+		/* Write into the overlay property(s) pointing to that node. */
    	if (dt_fixup_external(overlay, fix, node->phandle,
    			      path, overlay_symbols) < 0)
    		return -1;
@@ -1446,23 +1450,23 @@
    		      struct device_tree_node *fragment,
    		      struct device_tree_node *overlay_symbols)
 {
-	// The actually overlaid nodes/props are in an __overlay__ child node.
+	/* The actual overlaid nodes/props are in an __overlay__ child node. */
    static const char *overlay_path[] = { "__overlay__", NULL };
    struct device_tree_node *overlay = dt_find_node(fragment, overlay_path,
    						NULL, NULL, 0);
-	// If it doesn't have an __overlay__ child, it's not a fragment.
+	/* If it doesn't have an __overlay__ child, it's not a fragment. */
    if (!overlay)
    	return 0;
-	// The target node of the fragment can be given by path or by phandle.
+	/* Target node of the fragment can be given by path or by phandle. */
    struct device_tree_property *prop;
    struct device_tree_property *phandle = NULL;
    struct device_tree_property *path = NULL;
    list_for_each(prop, fragment->properties, list_node) {
    	if (!strcmp(prop->prop.name, "target")) {
    		phandle = prop;
-			break; // phandle target has priority, stop looking here
+			break; /* phandle target has priority, stop looking */
    	}
    	if (!strcmp(prop->prop.name, "target-path"))
    		path = prop;
@@ -1474,7 +1478,7 @@
    		return -1;
    	target = dt_find_node_by_phandle(tree->root,
    					 be32dec(phandle->prop.data));
-		// Symbols already updated as part of dt_fixup_external(target).
+		/* Symbols already updated as part of dt_fixup_external(). */
    } else if (path) {
    	target = dt_find_node_by_path(tree, path->prop.data,
    				      NULL, NULL, 0);
@@ -1499,10 +1503,12 @@
  */
 int dt_apply_overlay(struct device_tree *tree, struct device_tree *overlay)
 {
-	// First, we need to make sure phandles inside the overlay don't clash
-	// with those in the base tree. We just define the highest phandle value
-	// in the base tree as the "phandle offset" for this overlay and
-	// increment all phandles in it by that value.
+	/*
+	 * First, we need to make sure phandles inside the overlay don't clash
+	 * with those in the base tree. We just define the highest phandle value
+	 * in the base tree as the "phandle offset" for this overlay and
+	 * increment all phandles in it by that value.
+	 */
    uint32_t phandle_base = tree->max_phandle;
    uint32_t new_max = dt_adjust_all_phandles(overlay->root, phandle_base);
    if (!new_max) {
@@ -1511,8 +1517,8 @@
    }
    tree->max_phandle = new_max;
-	// Now that we changed phandles in the overlay, we need to update any
-	// nodes referring to them. Those are listed in /__local_fixups__.
+	/* Now that we changed phandles in the overlay, we need to update any
+	   nodes referring to them. Those are listed in /__local_fixups__. */
    struct device_tree_node *local_fixups = dt_find_node_by_path(overlay,
    				"/__local_fixups__", NULL, NULL, 0);
    if (local_fixups && dt_fixup_locals(overlay->root, local_fixups,
@@ -1521,16 +1527,18 @@
    	return -1;
    }
-	// Besides local phandle references (from nodes within the overlay to
-	// other nodes within the overlay), the overlay may also contain phandle
-	// references to the base tree. These are stored with invalid values and
-	// must be updated now. /__symbols__ contains a list of all labels in
-	// the base tree, and /__fixups__ describes all nodes in the overlay
-	// that contain external phandle references.
-	// We also take this opportunity to update all /fragment@X/__overlay__/
-	// prefixes in the overlay's /__symbols__ node to the correct path that
-	// the fragment will be placed in later, since this is the only step
-	// where we have all necessary information for that easily available.
+	/*
+	 * Besides local phandle references (from nodes within the overlay to
+	 * other nodes within the overlay), the overlay may also contain phandle
+	 * references to the base tree. These are stored with invalid values and
+	 * must be updated now. /__symbols__ contains a list of all labels in
+	 * the base tree, and /__fixups__ describes all nodes in the overlay
+	 * that contain external phandle references.
+	 * We also take this opportunity to update all /fragment@X/__overlay__/
+	 * prefixes in the overlay's /__symbols__ node to the correct path that
+	 * the fragment will be placed in later, since this is the only step
+	 * where we have all necessary information for that easily available.
+	 */
    struct device_tree_node *symbols = dt_find_node_by_path(tree,
    	"/__symbols__", NULL, NULL, 0);
    struct device_tree_node *fixups = dt_find_node_by_path(overlay,
@@ -1544,8 +1552,8 @@
    	return -1;
    }
-	// After all this fixing up, we can finally merge the overlay into the
-	// tree (one fragment at a time, because for some reason it's split up).
+	/* After all this fixing up, we can finally merge overlay into the tree
+	   (one fragment at a time, because for some reason it's split up). */
    struct device_tree_node *fragment;
    list_for_each(fragment, overlay->root->children, list_node)
    	if (dt_import_fragment(tree, fragment, overlay_symbols) < 0) {
@@ -1554,11 +1562,13 @@
    		return -1;
    	}
-	// We need to also update /__symbols__ to include labels from this
-	// overlay, in case we want to load further overlays with external
-	// phandle references to it. If the base tree already has a /__symbols__
-	// we merge them together, otherwise we just insert the overlay's
-	// /__symbols__ node into the base tree root.
+	/*
+	 * We need to also update /__symbols__ to include labels from this
+	 * overlay, in case we want to load further overlays with external
+	 * phandle references to it. If the base tree already has a /__symbols__
+	 * we merge them together, otherwise we just insert the overlay's
+	 * /__symbols__ node into the base tree root.
+	 */
    if (overlay_symbols) {
    	if (symbols)
    		dt_copy_subtree(symbols, overlay_symbols, 0);
diff --git a/src/lib/fit.c b/src/lib/fit.c
index 0d3cb2d..be243d7 100644
--- a/src/lib/fit.c
+++ b/src/lib/fit.c
@@ -419,8 +419,8 @@
  */
 static int fit_update_compat(struct fit_config_node *config)
 {
-	// If there was no "compatible" property in config node, this is a
-	// legacy FIT image. Must extract compat prop from FDT itself.
+	/* If there was no "compatible" property in config node, this is a
+	   legacy FIT image. Must extract compat prop from FDT itself. */
    if (!config->compat.name) {
    	void *fdt_blob = config->fdt->data;
    	const struct fdt_header *fdt_header = fdt_blob;
@@ -434,7 +434,7 @@
    		return -1;
    	}
-		// FDT overlays are not supported in legacy FIT images.
+		/* FDT overlays are not supported in legacy FIT images. */
    	if (config->overlays.next) {
    		printk(BIOS_ERR,
    		       "ERROR: config %s has overlay but no compat!\n",
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/32875
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib67bb9ff17b7688826071453ab58894a0835ce10
Gerrit-Change-Number: 32875
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner jwerner@chromium.org
Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com
Gerrit-MessageType: newchange



    