| Message ID | 20240123-offsetted-uimage-splitter-v1-1-dc0812b43c97@linaro.org |
|---|---|
| State | Rejected, archived |
| Delegated to: | Linus Walleij |
| Headers | show |
| Series | mtdsplit_uimage: Split also after offsetted uImage | expand |
The simple past tense of offset is... offset :) Suggested: Split also after offset uImage BTW: was this what I tested in your factory and sysimages for the XG6846? If so can you roll a fresh one with this patch? Then I can give you a more recent Tested-By: On 2024-01-23 23:17, Linus Walleij wrote: > The uImage splitter recognizes a rootfs either: > > 1. Right after the uImage if it comes first in the > partition or > 2. Before the uImage if it is located at an offset > inside the partition. > > Add a third case: > > 3. After the uImage also at an offset inside the > partition, if and only if 1 and 2 fails. > > The reason why this is needed is because on the > BCM6328-based Inteno XG6846 we need to put a small > U-Boot binary first in the partition, then the uImage, > then the rootfs. Suggest rewording to: This is necessary because the BCM6328-based Inteno XG6846 won't boot an image without a small U-Boot binary first in the partition followed by the uImage, then the rootfs. Then it doesn't depend on our needs :) > > The U-Boot binary that comes first cannot be split off > into its own partition in this case because it needs > to be part of the bigger "firmware" partition. Which > we use for installation and upgrades. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > .../files/drivers/mtd/mtdsplit/mtdsplit_uimage.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c > index a3e55fb1fe38..de043fb9f702 100644 > --- a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c > +++ b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c > @@ -217,11 +217,22 @@ static int __mtdsplit_parse_uimage(struct mtd_info *master, > if (ret) { > pr_debug("no rootfs before uImage in \"%s\"\n", > master->name); > - goto err_free_buf; > - } > > - rootfs_offset = 0; > - rootfs_size = uimage_offset; > + /* Try after the uImage */ > + ret = mtd_find_rootfs_from(master, uimage_offset + uimage_size, > + master->size, &rootfs_offset, &type); > + if (ret) { > + pr_debug("no rootfs after uImage either in \"%s\"\n", > + master->name); > + goto err_free_buf; > + } > + > + rootfs_size = master->size - rootfs_offset; > + uimage_size = rootfs_offset - uimage_offset; > + } else { > + rootfs_offset = 0; > + rootfs_size = uimage_offset; > + } > } > > if (rootfs_size == 0) { > > --- > base-commit: 1b7e62b20b1735fcdc498a35e005afcd775abcf4 > change-id: 20240123-offsetted-uimage-splitter-f5b65f0df2ed > > Best regards,
On Fri, Jan 26, 2024 at 12:21 AM Paul D <newtwen@gmail.com> wrote: > BTW: was this what I tested in your factory and sysimages for the XG6846? > > If so can you roll a fresh one with this patch? It has been used in all of them, but I made a new one on top of the OpenWrt main branch! https://dflund.se/~triad/krad/inteno-xg6846/ > > The reason why this is needed is because on the > > BCM6328-based Inteno XG6846 we need to put a small > > U-Boot binary first in the partition, then the uImage, > > then the rootfs. > > Suggest rewording to: > > This is necessary because the BCM6328-based Inteno XG6846 won't boot an > image without a small U-Boot binary first in the partition followed by > the uImage, then the rootfs. OK changed it. Yours, Linus Walleij
Hello Linus,
This is just a question: doesn't it work with "openwrt,offset"
property of mtdsplit_uimage parser instead of modifying that parser?
I'm thinking like the following:
- dts:
partition@10000 {
compatible = "openwrt,uimage", "denx,uimage";
reg = <0x010000 0xfe0000>;
openwrt,offset = <0x20000>;
label = "firmware";
};
- image/Makefile:
IMAGE/sysupgrade.bin := cfe-bin-uboot | pad-to 128k | \
append-kernel | pad-to $$$$$$$$(($$(BLOCKSIZE))) | \
append-rootfs | append-metadata
Thanks,
Hiroshi
On 2024/01/24 7:17, Linus Walleij wrote:
> The uImage splitter recognizes a rootfs either:
>
> 1. Right after the uImage if it comes first in the
> partition or
> 2. Before the uImage if it is located at an offset
> inside the partition.
>
> Add a third case:
>
> 3. After the uImage also at an offset inside the
> partition, if and only if 1 and 2 fails.
>
> The reason why this is needed is because on the
> BCM6328-based Inteno XG6846 we need to put a small
> U-Boot binary first in the partition, then the uImage,
> then the rootfs.
>
> The U-Boot binary that comes first cannot be split off
> into its own partition in this case because it needs
> to be part of the bigger "firmware" partition. Which
> we use for installation and upgrades.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> .../files/drivers/mtd/mtdsplit/mtdsplit_uimage.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> index a3e55fb1fe38..de043fb9f702 100644
> --- a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> +++ b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> @@ -217,11 +217,22 @@ static int __mtdsplit_parse_uimage(struct mtd_info *master,
> if (ret) {
> pr_debug("no rootfs before uImage in \"%s\"\n",
> master->name);
> - goto err_free_buf;
> - }
>
> - rootfs_offset = 0;
> - rootfs_size = uimage_offset;
> + /* Try after the uImage */
> + ret = mtd_find_rootfs_from(master, uimage_offset + uimage_size,
> + master->size, &rootfs_offset, &type);
> + if (ret) {
> + pr_debug("no rootfs after uImage either in \"%s\"\n",
> + master->name);
> + goto err_free_buf;
> + }
> +
> + rootfs_size = master->size - rootfs_offset;
> + uimage_size = rootfs_offset - uimage_offset;
> + } else {
> + rootfs_offset = 0;
> + rootfs_size = uimage_offset;
> + }
> }
>
> if (rootfs_size == 0) {
>
> ---
> base-commit: 1b7e62b20b1735fcdc498a35e005afcd775abcf4
> change-id: 20240123-offsetted-uimage-splitter-f5b65f0df2ed
>
> Best regards,
On Fri, Jan 26, 2024 at 9:56 AM INAGAKI Hiroshi <musashino.open@gmail.com> wrote: > This is just a question: doesn't it work with "openwrt,offset" > property of mtdsplit_uimage parser instead of modifying that parser? It works actually... The U-Boot takes up 3 64k erase blocks so if I just set offset to 0x30000 it works like a charm. I first thought that maybe it's not as flexible (for example if U-Boot becomes bigger and pass a 64K boundary) but it doesn't really affect this system as I control this pretty tightly anyway. Thanks, I'll drop this patch! Yours, Linus Walleij
diff --git a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c index a3e55fb1fe38..de043fb9f702 100644 --- a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c +++ b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c @@ -217,11 +217,22 @@ static int __mtdsplit_parse_uimage(struct mtd_info *master, if (ret) { pr_debug("no rootfs before uImage in \"%s\"\n", master->name); - goto err_free_buf; - } - rootfs_offset = 0; - rootfs_size = uimage_offset; + /* Try after the uImage */ + ret = mtd_find_rootfs_from(master, uimage_offset + uimage_size, + master->size, &rootfs_offset, &type); + if (ret) { + pr_debug("no rootfs after uImage either in \"%s\"\n", + master->name); + goto err_free_buf; + } + + rootfs_size = master->size - rootfs_offset; + uimage_size = rootfs_offset - uimage_offset; + } else { + rootfs_offset = 0; + rootfs_size = uimage_offset; + } } if (rootfs_size == 0) {
The uImage splitter recognizes a rootfs either: 1. Right after the uImage if it comes first in the partition or 2. Before the uImage if it is located at an offset inside the partition. Add a third case: 3. After the uImage also at an offset inside the partition, if and only if 1 and 2 fails. The reason why this is needed is because on the BCM6328-based Inteno XG6846 we need to put a small U-Boot binary first in the partition, then the uImage, then the rootfs. The U-Boot binary that comes first cannot be split off into its own partition in this case because it needs to be part of the bigger "firmware" partition. Which we use for installation and upgrades. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- .../files/drivers/mtd/mtdsplit/mtdsplit_uimage.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) --- base-commit: 1b7e62b20b1735fcdc498a35e005afcd775abcf4 change-id: 20240123-offsetted-uimage-splitter-f5b65f0df2ed Best regards,