linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Francesco Dolcini <francesco@dolcini.it>
To: Simon Glass <sjg@chromium.org>,
	u-boot@lists.denx.de,
	Marcel Ziswiler <marcel.ziswiler@toradex.com>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>,
	Marek Vasut <marex@denx.de>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	linux-mtd@lists.infradead.org
Subject: [PATCH v2 1/3] fdt: validate/fix cells count on mtdpart fixup
Date: Mon,  6 Feb 2023 23:48:36 +0100	[thread overview]
Message-ID: <20230206224838.75963-2-francesco@dolcini.it> (raw)
In-Reply-To: <20230206224838.75963-1-francesco@dolcini.it>

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Fixup #size-cells value when updating the MTD partitions, this is
required to prevent issues in case the MTD parent set #size-cells to
zero.
This could happen for example in the legacy case in which the partitions
are created as direct child of the mtd node and that specific node has
no children. Recent clean-up on Linux device tree files created a boot
regression on colibri-imx7 [0].

This fixup has the limitation to assume 32-bit (#size-cells=1)
addressing, therefore it will not work with device bigger than 4GiB.

This change also enforce #address-cells to be the same as #size-cells,
this was already silently enforced by fdt_node_set_part_info(), now this
is checked explicitly and partition fixup will just fail in such case.

When the partition list is static the preferred way to pass the mtd
partition list to the kernel is to either define those in the source DTS
file or use mtdparts= in the command line.
Tweaking the DT from U-Boot should be avoided, unless some dynamic
changes are required, since it proved to be problematic when not
following the evolution of the "standard".

Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/
Cc: Marek Vasut <marex@denx.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: improved commit message
---
 common/fdt_support.c | 45 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index dbceec6f2dcc..3aee826e60cf 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -877,27 +877,20 @@ static int fdt_del_partitions(void *blob, int parent_offset)
 	return 0;
 }
 
-static int fdt_node_set_part_info(void *blob, int parent_offset,
+/* This expects #address-cells and #size-cells to have same value */
+static int fdt_node_set_part_info(void *blob, int parent_offset, int sizecell,
 				  struct mtd_device *dev)
 {
 	struct list_head *pentry;
 	struct part_info *part;
 	int off, ndepth = 0;
 	int part_num, ret;
-	int sizecell;
 	char buf[64];
 
 	ret = fdt_del_partitions(blob, parent_offset);
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * Check if size/address is 1 or 2 cells.
-	 * We assume #address-cells and #size-cells have same value.
-	 */
-	sizecell = fdt_getprop_u32_default_node(blob, parent_offset,
-						0, "#size-cells", 1);
-
 	/*
 	 * Check if it is nand {}; subnode, adjust
 	 * the offset in this case
@@ -992,6 +985,31 @@ err_prop:
 	return ret;
 }
 
+static int fdt_mtdparts_cell_cnt(void *fdt, int off)
+{
+	int sizecell, addrcell;
+
+	sizecell = fdt_getprop_u32_default_node(fdt, off, 0, "#size-cells", 0);
+	if (sizecell != 1 && sizecell != 2) {
+		printf("%s: Invalid or missing #size-cells %d value, assuming 1\n",
+		       __func__, sizecell);
+
+		sizecell = 1;
+		if (fdt_setprop_u32(fdt, off, "#size-cells", sizecell))
+			return -1;
+	}
+
+	addrcell = fdt_getprop_u32_default_node(fdt, off, 0,
+						"#address-cells", 0);
+	if (addrcell != sizecell) {
+		printf("%s: Invalid #address-cells %d != #size-cells %d, aborting\n",
+		       __func__, addrcell, sizecell);
+		return -1;
+	}
+
+	return sizecell;
+}
+
 /*
  * Update partitions in nor/nand nodes using info from
  * mtdparts environment variable. The nodes to update are
@@ -1037,12 +1055,19 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info,
 
 			dev = device_find(node_info[i].type, idx++);
 			if (dev) {
+				int cell;
+
 				parts = fdt_subnode_offset(blob, noff,
 							   "partitions");
 				if (parts < 0)
 					parts = noff;
 
-				if (fdt_node_set_part_info(blob, parts, dev))
+				cell = fdt_mtdparts_cell_cnt(blob, parts);
+				if (cell < 0)
+					return;
+
+				if (fdt_node_set_part_info(blob, parts,
+							   cell, dev))
 					return; /* return on error */
 			}
 		}
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

      reply	other threads:[~2023-02-06 22:49 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-06 22:48 [PATCH v2 0/3] fdt: Fix mtparts fixup Francesco Dolcini
2023-02-06 22:48 ` Francesco Dolcini [this message]

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=20230206224838.75963-2-francesco@dolcini.it \
    --to=francesco@dolcini.it \
    --cc=francesco.dolcini@toradex.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marcel.ziswiler@toradex.com \
    --cc=marex@denx.de \
    --cc=miquel.raynal@bootlin.com \
    --cc=sjg@chromium.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).