Re: [PATCH] mtd: parsers: trx: parse "firmware" MTD partitions only

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Rafał,

Rafał Miłecki <zajec5@gmail.com> wrote on Tue, 13 Apr 2021 18:37:00
+0200:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Parsing every partition with "compatible" set to "brcm,trx" results in
> parsing both: firmware partition and failsafe partition on devices that
> implement failsafe booting. This affects e.g. Linksys EA9500 which has:
> 
> partition@200000 {
> 	reg = <0x0200000 0x01d00000>;
> 	compatible = "linksys,ns-firmware", "brcm,trx";
> };
> 
> partition@1f00000 {
> 	reg = <0x01f00000 0x01d00000>;
> 	compatible = "linksys,ns-firmware", "brcm,trx";
> };
> 
> Check for MTD partition name "firmware" before parsing. Recently added
> ofpart_linksys_ns.c creates "firmware" and "failsafe" depending on
> bootloader setup.

I don't like very much the matching against a partition name, this is
and should probably remain free text. Why not matching against the
entire linksys,ns-firmware compatible instead?

> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> Vivek has recently reported this problem to me and this is soltuion I
> came up with.
> 
> One alternative I thought of could be marking "failsafe" MTD partition
> node as disabled (using of_update_property() + "status" + "disabled") and
> then using of_device_is_available() in the parser_trx.c.
> 
> Let me know if you prefer the other (or any other) solution over this
> patch.
> ---
>  drivers/mtd/parsers/parser_trx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mtd/parsers/parser_trx.c b/drivers/mtd/parsers/parser_trx.c
> index 8541182134d4..0063791e164d 100644
> --- a/drivers/mtd/parsers/parser_trx.c
> +++ b/drivers/mtd/parsers/parser_trx.c
> @@ -58,6 +58,10 @@ static int parser_trx_parse(struct mtd_info *mtd,
>  	uint8_t curr_part = 0, i = 0;
>  	int err;
>  
> +	/* Don't parse any failsafe / backup partitions */
> +	if (strcmp(mtd->name, "firmware"))
> +		return -EINVAL;
> +
>  	parts = kcalloc(TRX_PARSER_MAX_PARTS, sizeof(struct mtd_partition),
>  			GFP_KERNEL);
>  	if (!parts)




Thanks,
Miquèl