[PATCH v1 1/3] fdt: validate/fix cells count on mtdpart fixup

[Marek Vasut]

On 1/13/23 19:45, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini at 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.
> 
> 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.
> 
> In general board should not generally need nor use this functionality
> and should be just deprecated, passing mtdparts= in the kernel command
> line is the preferred way according to Linux MTD subsystem maintainer.
> 
> Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/
> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
> Cc: Marek Vasut <marex at denx.de>
> Cc: Miquel Raynal <miquel.raynal at bootlin.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini at toradex.com>
> ---
>   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 */
>   			}
>   		}

Can you please include the resulting gpmi node content with this fixup 
applied in the commit message , so it can be validated ?