[PATCH 1/2] mtd: bcm47xxpart: move TRX parsing code to separated function

[Marek Vasut]

On 01/10/2017 12:50 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal at milecki.pl>
> 
> This change simplifies main parsing loop logic a bit. In future it may
> be useful for moving TRX support to separated module / parser (if we
> implement support for them at some point).
> Finally parsing TRX at the end puts us in a better position as we have
> better flash layout knowledge. It may be useful e.g. if it appears there
> is more than 1 TRX partition.
> 
> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
> ---
>  drivers/mtd/bcm47xxpart.c | 124 ++++++++++++++++++++++++++++------------------
>  1 file changed, 77 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/bcm47xxpart.c
> index 283ff7e..d7d1b6e 100644
> --- a/drivers/mtd/bcm47xxpart.c
> +++ b/drivers/mtd/bcm47xxpart.c
> @@ -83,6 +83,70 @@ static const char *bcm47xxpart_trx_data_part_name(struct mtd_info *master,
>  	return "rootfs";
>  }
>  
> +static int bcm47xxpart_parse_trx(struct mtd_info *master,
> +				 struct mtd_partition *trx,
> +				 struct mtd_partition *parts,
> +				 size_t parts_len)
> +{
> +	struct mtd_partition *part;
> +	struct trx_header header;
> +	size_t bytes_read;
> +	int curr_part = 0;
> +	int i, err;
> +
> +	if (parts_len < 3) {
> +		pr_warn("No enough space to add TRX partitions!\n");
> +		return -ENOMEM;
> +	}
> +
> +	err = mtd_read(master, trx->offset, sizeof(header), &bytes_read,
> +		       (uint8_t *)&header);
> +	if (err && !mtd_is_bitflip(err)) {
> +		pr_err("mtd_read error while reading TRX header: %d\n", err);
> +		return err;
> +	}
> +
> +	i = 0;
> +
> +	/* We have LZMA loader if offset[2] points to sth */
> +	if (header.offset[2]) {
> +		part = &parts[curr_part++];
> +		part->name = "loader";
> +		part->offset = trx->offset + header.offset[i];
> +		i++;
> +	}
> +
> +	if (header.offset[i]) {
> +		part = &parts[curr_part++];
> +		part->name = "linux";
> +		part->offset = trx->offset + header.offset[i];
> +		i++;
> +	}
> +
> +	if (header.offset[i]) {
> +		size_t offset = trx->offset + header.offset[i];
> +
> +		part = &parts[curr_part++];
> +		part->name = bcm47xxpart_trx_data_part_name(master, offset);
> +		part->offset = offset;

Why don't you still use bcm47xxpart_add_part() here ?

> +		i++;
> +	}
> +
> +	/*
> +	 * Assume that every partition ends at the beginning of the one it is
> +	 * followed by.
> +	 */
> +	for (i = 0; i < curr_part; i++) {
> +		u64 next_part_offset = (i < curr_part - 1) ?
> +					parts[i + 1].offset :
> +					trx->offset + trx->size;
> +
> +		parts[i].size = next_part_offset - parts[i].offset;
> +	}
> +
> +	return curr_part;
> +}
> +
>  static int bcm47xxpart_parse(struct mtd_info *master,
>  			     const struct mtd_partition **pparts,
>  			     struct mtd_part_parser_data *data)
> @@ -93,9 +157,7 @@ static int bcm47xxpart_parse(struct mtd_info *master,
>  	size_t bytes_read;
>  	uint32_t offset;
>  	uint32_t blocksize = master->erasesize;
> -	struct trx_header *trx;
>  	int trx_part = -1;
> -	int last_trx_part = -1;
>  	int possible_nvram_sizes[] = { 0x8000, 0xF000, 0x10000, };
>  	int err;
>  
> @@ -182,54 +244,14 @@ static int bcm47xxpart_parse(struct mtd_info *master,
>  
>  		/* TRX */
>  		if (buf[0x000 / 4] == TRX_MAGIC) {
> -			if (BCM47XXPART_MAX_PARTS - curr_part < 4) {
> -				pr_warn("Not enough partitions left to register trx, scanning stopped!\n");
> -				break;
> -			}
> -
> -			trx = (struct trx_header *)buf;
> +			struct trx_header *trx;
>  
>  			trx_part = curr_part;
>  			bcm47xxpart_add_part(&parts[curr_part++], "firmware",
>  					     offset, 0);
>  
> -			i = 0;
> -			/* We have LZMA loader if offset[2] points to sth */
> -			if (trx->offset[2]) {
> -				bcm47xxpart_add_part(&parts[curr_part++],
> -						     "loader",
> -						     offset + trx->offset[i],
> -						     0);
> -				i++;
> -			}
> -
> -			if (trx->offset[i]) {
> -				bcm47xxpart_add_part(&parts[curr_part++],
> -						     "linux",
> -						     offset + trx->offset[i],
> -						     0);
> -				i++;
> -			}
> -
> -			/*
> -			 * Pure rootfs size is known and can be calculated as:
> -			 * trx->length - trx->offset[i]. We don't fill it as
> -			 * we want to have jffs2 (overlay) in the same mtd.
> -			 */
> -			if (trx->offset[i]) {
> -				const char *name;
> -
> -				name = bcm47xxpart_trx_data_part_name(master, offset + trx->offset[i]);
> -				bcm47xxpart_add_part(&parts[curr_part++],
> -						     name,
> -						     offset + trx->offset[i],
> -						     0);
> -				i++;
> -			}
> -
> -			last_trx_part = curr_part - 1;
> -
>  			/* Jump to the end of TRX */
> +			trx = (struct trx_header *)buf;
>  			offset = roundup(offset + trx->length, blocksize);
>  			/* Next loop iteration will increase the offset */
>  			offset -= blocksize;
> @@ -307,9 +329,17 @@ static int bcm47xxpart_parse(struct mtd_info *master,
>  				       parts[i + 1].offset : master->size;
>  
>  		parts[i].size = next_part_offset - parts[i].offset;
> -		if (i == last_trx_part && trx_part >= 0)
> -			parts[trx_part].size = next_part_offset -
> -					       parts[trx_part].offset;
> +	}
> +
> +	/* If there was TRX parse it now */
> +	if (trx_part >= 0) {
> +		int num_parts;
> +
> +		num_parts = bcm47xxpart_parse_trx(master, &parts[trx_part],
> +						  parts + curr_part,
> +						  BCM47XXPART_MAX_PARTS - curr_part);
> +		if (num_parts > 0)
> +			curr_part += num_parts;
>  	}
>  
>  	*pparts = parts;
> 


-- 
Best regards,
Marek Vasut