All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
To: u-boot@lists.denx.de
Subject: [PATCH v2 2/8] spl: fit: Factor out FIT parsing and use a context struct
Date: Tue, 22 Dec 2020 17:54:43 -0600	[thread overview]
Message-ID: <20201222235449.460100-3-mr.nuke.me@gmail.com> (raw)
In-Reply-To: <20201216000944.2832585-1-mr.nuke.me@gmail.com>

The logical steps in spl_load_simple_fit() are difficult to follow.
I think the long comments, ifdefs, and ungodly number of variables
seriously affect the readability. In particular, it violates section 6
of the coding style, paragraphs (3), and (4).

The purpose of this patch is to improve the situation by
  - Factoring out initialization and parsing to separate functions
  - Reduce the number of variables by using a context structure
This change introduces no functional changes.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 common/spl/spl_fit.c | 90 +++++++++++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 30 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 1b4a7f6b15..bcc943b0b8 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -26,6 +26,12 @@ DECLARE_GLOBAL_DATA_PTR;
 #define CONFIG_SYS_BOOTM_LEN	(64 << 20)
 #endif
 
+struct spl_fit_info {
+	const void *fit;	/* Pointer to a valid FIT blob */
+	size_t ext_data_offset;	/* Offset to FIT external data (end of FIT) */
+	int images_node;	/* FDT offset to "/images" node */
+};
+
 __weak void board_spl_fit_post_load(const void *fit)
 {
 }
@@ -521,28 +527,22 @@ __weak bool spl_load_simple_fit_skip_processing(void)
 	return false;
 }
 
-int spl_load_simple_fit(struct spl_image_info *spl_image,
-			struct spl_load_info *info, ulong sector, void *fit)
+static int spl_simple_fit_read(struct spl_fit_info *ctx,
+			       struct spl_load_info *info, ulong sector,
+			       const void *fit_header)
 {
+	unsigned long count, size;
 	int sectors;
-	ulong size, hsize;
-	unsigned long count;
-	struct spl_image_info image_info;
-	int node = -1;
-	int images, ret;
-	int base_offset;
-	int index = 0;
-	int firmware_node;
+	void *buf;
 
 	/*
 	 * For FIT with external data, figure out where the external images
 	 * start. This is the base for the data-offset properties in each
 	 * image.
 	 */
-	size = fdt_totalsize(fit);
-	size = (size + 3) & ~3;
+	size = ALIGN(fdt_totalsize(fit_header), 4);
 	size = board_spl_fit_size_align(size);
-	base_offset = (size + 3) & ~3;
+	ctx->ext_data_offset = ALIGN(size, 4);
 
 	/*
 	 * So far we only have one block of data from the FIT. Read the entire
@@ -552,36 +552,66 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 	 * For FIT with external data, data is not loaded in this step.
 	 */
 	sectors = get_aligned_image_size(info, size, 0);
-	hsize = sectors * info->bl_len;
-	fit = spl_get_fit_load_buffer(hsize);
-	count = info->read(info, sector, sectors, fit);
-	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu, size=0x%lx\n",
-	      sector, sectors, fit, count, size);
+	buf = spl_get_fit_load_buffer(sectors * info->bl_len);
 
-	if (count == 0)
-		return -EIO;
+	count = info->read(info, sector, sectors, buf);
+	ctx->fit = buf;
+	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu, size=0x%lx\n",
+	      sector, sectors, buf, count, size);
 
-	/* skip further processing if requested to enable load-only use cases */
-	if (spl_load_simple_fit_skip_processing())
-		return 0;
+	return (count == 0) ? -EIO : 0;
+}
 
+static int spl_simple_fit_parse(struct spl_fit_info *ctx)
+{
 	if (IS_ENABLED(CONFIG_SPL_FIT_SIGNATURE)) {
-		int conf_offset = fit_find_config_node(fit);
+		int conf_offset = fit_find_config_node(ctx->fit);
 
 		printf("## Checking hash(es) for config %s ... ",
-		       fit_get_name(fit, conf_offset, NULL));
-		if (fit_config_verify(fit, conf_offset))
+		       fit_get_name(ctx->fit, conf_offset, NULL));
+		if (fit_config_verify(ctx->fit, conf_offset))
 			return -EPERM;
 		puts("OK\n");
 	}
 
 	/* find the node holding the images information */
-	images = fdt_path_offset(fit, FIT_IMAGES_PATH);
-	if (images < 0) {
-		debug("%s: Cannot find /images node: %d\n", __func__, images);
-		return -1;
+	ctx->images_node = fdt_path_offset(ctx->fit, FIT_IMAGES_PATH);
+	if (ctx->images_node < 0) {
+		debug("%s: Cannot find /images node: %d\n", __func__,
+		      ctx->images_node);
+		return -EINVAL;
 	}
 
+	return 0;
+}
+
+int spl_load_simple_fit(struct spl_image_info *spl_image,
+			struct spl_load_info *info, ulong sector, void *fit)
+{
+	struct spl_image_info image_info;
+	struct spl_fit_info ctx;
+	int node = -1;
+	int images, ret;
+	int base_offset;
+	int index = 0;
+	int firmware_node;
+
+	ret = spl_simple_fit_read(&ctx, info, sector, fit);
+	if (ret < 0)
+		return ret;
+
+	/* skip further processing if requested to enable load-only use cases */
+	if (spl_load_simple_fit_skip_processing())
+		return 0;
+
+	ret = spl_simple_fit_parse(&ctx);
+	if (ret < 0)
+		return ret;
+
+	images = ctx.images_node;
+	fit = (void *)ctx.fit;
+	base_offset = ctx.ext_data_offset;
+
 #ifdef CONFIG_SPL_FPGA
 	node = spl_fit_get_image_node(fit, images, "fpga", 0);
 	if (node >= 0) {
-- 
2.26.2

  parent reply	other threads:[~2020-12-22 23:54 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16  0:09 [PATCH 0/8] spl: fit: Play nicely with OP-TEE and Linux Alexandru Gagniuc
2020-12-16  0:09 ` [PATCH 1/8] spl: fit: Drop 'length' argument to board_spl_fit_post_load() Alexandru Gagniuc
2020-12-16  7:13   ` Peng Fan
2020-12-16 14:32     ` Alex G.
2020-12-19  2:28   ` Simon Glass
2020-12-16  0:09 ` [PATCH 2/8] spl: fit: Factor out FIT parsing and use a context struct Alexandru Gagniuc
2020-12-19  2:28   ` Simon Glass
2020-12-21 19:28     ` Alex G.
2020-12-21 20:23       ` Simon Glass
2020-12-21 22:24         ` Alex G.
2020-12-29  3:33           ` Simon Glass
2020-12-30  0:07             ` Alex G.
2020-12-30 14:15               ` Tom Rini
2020-12-31  2:57               ` Simon Glass
2021-01-31  3:45                 ` Simon Glass
2020-12-16  0:09 ` [PATCH 3/8] spl: fit: Pass FIT context via a structure pointer Alexandru Gagniuc
2020-12-19  2:28   ` Simon Glass
2020-12-16  0:09 ` [PATCH 4/8] spl: fit: Remove useless loop in spl_fit_get_image_name() Alexandru Gagniuc
2020-12-19  2:29   ` Simon Glass
2020-12-16  0:09 ` [PATCH 5/8] spl: fit: Only look up FIT configuration node once Alexandru Gagniuc
2020-12-19  2:29   ` Simon Glass
2020-12-16  0:09 ` [PATCH 6/8] image: Do not #if guard board_fit_config_name_match() prototype Alexandru Gagniuc
2020-12-19  2:29   ` Simon Glass
2020-12-16  0:09 ` [PATCH 7/8] spl: fit: Replace #ifdef blocks with more readable constructs Alexandru Gagniuc
2020-12-19  2:29   ` Simon Glass
2020-12-21 17:43     ` Alex G.
2020-12-21 20:23       ` Simon Glass
2020-12-16  0:09 ` [PATCH 8/8] spl: fit: Load devicetree when a Linux payload is found Alexandru Gagniuc
2020-12-16 17:26   ` Alex G.
2020-12-22 23:54 ` [PATCH v2 0/8] spl: fit: Play nicely with OP-TEE and Linux Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 " Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 1/8] spl: fit: Drop 'length' argument to board_spl_fit_post_load() Alexandru Gagniuc
2021-01-16  2:33     ` Tom Rini
2021-01-18 16:28       ` Alex G.
2021-01-18 16:59         ` Tom Rini
2020-12-23 14:44   ` [PATCH v3 2/8] spl: fit: Factor out FIT parsing and use a context struct Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 3/8] spl: fit: Pass FIT context via a structure pointer Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 4/8] spl: fit: Remove useless loop in spl_fit_get_image_name() Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 5/8] spl: fit: Only look up FIT configuration node once Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 6/8] image: Do not #if guard board_fit_config_name_match() prototype Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 7/8] spl: fit: Replace #ifdef blocks with more readable constructs Alexandru Gagniuc
2020-12-23 14:44   ` [PATCH v3 8/8] spl: fit: Load devicetree when a Linux payload is found Alexandru Gagniuc
2020-12-22 23:54 ` [PATCH v2 1/8] spl: fit: Drop 'length' argument to board_spl_fit_post_load() Alexandru Gagniuc
2020-12-22 23:54 ` Alexandru Gagniuc [this message]
2020-12-29  3:32   ` [PATCH v2 2/8] spl: fit: Factor out FIT parsing and use a context struct Simon Glass
2020-12-22 23:54 ` [PATCH v2 3/8] spl: fit: Pass FIT context via a structure pointer Alexandru Gagniuc
2020-12-22 23:54 ` [PATCH v2 4/8] spl: fit: Remove useless loop in spl_fit_get_image_name() Alexandru Gagniuc
2020-12-22 23:54 ` [PATCH v2 5/8] spl: fit: Only look up FIT configuration node once Alexandru Gagniuc
2020-12-22 23:54 ` [PATCH v2 6/8] image: Do not #if guard board_fit_config_name_match() prototype Alexandru Gagniuc
2020-12-22 23:54 ` [PATCH v2 7/8] spl: fit: Replace #ifdef blocks with more readable constructs Alexandru Gagniuc
2020-12-22 23:54 ` [PATCH v2 8/8] spl: fit: Load devicetree when a Linux payload is found Alexandru Gagniuc

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201222235449.460100-3-mr.nuke.me@gmail.com \
    --to=mr.nuke.me@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.