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

Andy Shevchenko
2024-09-30T12:49:21Z

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