Re: [PATCH v4 4/5] block: add support for partition table defined in OF

[Andy Shevchenko]

On Mon, Sep 30, 2024 at 01:30:11PM +0200, Christian Marangi wrote:
> Add support for partition table defined in Device Tree. Similar to how
> it's done with MTD, add support for defining a fixed partition table in
> device tree.
> 
> A common scenario for this is fixed block (eMMC) embedded devices that
> have no MBR or GPT partition table to save storage space. Bootloader
> access the block device with absolute address of data.
> 
> This is to complete the functionality with an equivalent implementation
> with providing partition table with bootargs, for case where the booargs
> can't be modified and tweaking the Device Tree is the only solution to
> have an usabe partition table.
> 
> The implementation follow the fixed-partitions parser used on MTD
> devices where a "partitions" node is expected to be declared with
> "fixed-partitions" compatible in the OF node of the disk device
> (mmc-card for eMMC for example) and each child node declare a label
> and a reg with offset and size. If label is not declared, the node name
> is used as fallback. Eventually is also possible to declare the read-only
> property to flag the partition as read-only.
> 
> For eMMC block, driver scan the disk name and check if it's suffixed with
> "boot0" or "boot1".
> This is to handle the additional disk provided by eMMC as supported in
> JEDEC 4.4+. If this suffix is detected, "partitions-boot0" or
> "partitions-boot1" are used instead of the generic "partitions" for the
> relevant disk.

...

> +	strscpy(info->volname, partname, sizeof(info->volname));

We have 2-arguments strscpy(), please use that.

> +	strlcat(state->pp_buf, tmp, PAGE_SIZE);

In new code we should not use strl*(). They are subject to remove.
And actually why? You have used strscpy() a few lines above...

...

> +	for_each_child_of_node(partitions_np, np) {

Use _scoped() variant.

> +		if (validate_of_partition(np, slot)) {
> +			of_node_put(np);
> +			ret = -1;
> +			goto exit;
> +		}
> +
> +		slot++;
> +	}

...

> +	for_each_child_of_node(partitions_np, np) {

Ditto.

> +		if (slot >= state->limit) {
> +			of_node_put(np);
> +			break;
> +		}
> +
> +		add_of_partition(state, slot, np);
> +
> +		slot++;
> +	}

...

> +	strlcat(state->pp_buf, "\n", PAGE_SIZE);

Why strl*()?

-- 
With Best Regards,
Andy Shevchenko