[OpenWrt-Devel] [PATCH] apm821xx: for Meraki MR24 modify BLOCKSIZE to reduce LEB over-allocation

[Russell Senior]

On Fri, Sep 20, 2019 at 1:35 PM Christian Lamparter <chunkeey at gmail.com>
wrote:

> Hello,
>
> On Tuesday, September 17, 2019 6:59:28 AM CEST Russell Senior wrote:
> > On Meraki MR24, the BLOCKSIZE variable is used to allocate space for the
> > kernel blob. The LEB size on MR24 is 15.5k (15872 bytes). In the
> > particular instance observed, it was found that reducing blocksize to
> > 512 bytes resulted in 3 fewer LEBs being allocated to the kernel ubi
> > volume, with no ill effects.
> >
> > Signed-off-by: Russell Senior <russell at personaltelco.net>
> > ---
> >  target/linux/apm821xx/image/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/linux/apm821xx/image/Makefile
> b/target/linux/apm821xx/image/Makefile
> > index 8203de39c5..1aa4e0dad3 100644
> > --- a/target/linux/apm821xx/image/Makefile
> > +++ b/target/linux/apm821xx/image/Makefile
> > @@ -127,7 +127,7 @@ define Device/meraki_mr24
> >    DEVICE_PACKAGES := kmod-spi-gpio -swconfig
> >    BOARD_NAME := mr24
> >    DEVICE_DTS := meraki-mr24
> > -  BLOCKSIZE := 63k
> > +  BLOCKSIZE := 512
> >    IMAGES := sysupgrade.bin
> >    DTB_SIZE := 64512
> >    IMAGE_SIZE := 8191k
>
> The value looks odd, since UBI volumes are always a multiple of the LEB
> size
> and the documentation we have
> <https://openwrt.org/docs/techref/flash.layout#discovery_how_to_find_out>
> states that "The erasesize is the block size of the flash [...]".
> so in that regard BLOCKSIZE could be 15872 or 16 KiB (if we would stick to
> the real, raw value of the flash).
>

The only thing BLOCKSIZE is used for (afaict, for MR24) is in
bs=$(BLOCKSIZE) while generating the kernel blob (which includes a header
and a device tree binary as well). The DTB_SIZE is to align the device tree
and kernel at particular addresses. The sysupgrade infrastructure
(package/base-files/files/lib/upgrade/nand.sh) on the device deals with ubi
volume creation, and it's going to allocate LEBs to volumes to minimally
accommodate the file that is provided to occupy that volume. We could just
concatenate the kernel with no blocking at all, and it would work fine.
The section you are referring to is talking about the mtd layer, not ubi.


>
> But I don't think a generated image with these parameters would boot
> anymore.
>

Which parameters? I've tested 512 and it boots fine.


> As the problem here is hidden in "MerakiAdd-dtb" step that is used
> generate the
> KERNEL (for sysupgrade) relies on that  BLOCKSIZE value being a integer
> divisible
> of the 63KiB value. Since this magic value (63KiB) is needed to place the
> DTB+kernel at the correct offsets for mkmerakifw.
>

No, the device tree and kernel are placed in the "right spots" (the ones
expected by u-boot) because of the DTB_SIZE variable. There is a header,
the device tree at offset 1k, and the kernel starts at offset 64k.

define Build/MerakiAdd-dtb
        $(call Image/BuildDTB,../dts/$(DEVICE_DTS).dts,$@.dtb)
        ( \
                dd if=$@.dtb bs=$(DTB_SIZE) conv=sync; \
                dd if=$@ bs=$(BLOCKSIZE) conv=sync; \
        ) > $@.new
        @mv $@.new $@
endef

That second dd could be "cat $@" and I think it would work (but haven't
tested), because the blocking of the kernel is not important.


>
> so, what to do? Because it's possible to get rid of the whole logic as
> well as the
> MR24 portion of the mkmerakifw by refactoring the u-boot boot commands to
> just load
> and boot a multi-file image. The framework is all there since the
> initramfs image is
> already utilizing "MuImage-initramfs". This would save even more since
> this makes it
> possible to also shrink down the DTB as well. As the "raw" information in
> there is
> just around 10k-15k and the rest of this is fluff. (Some of this fluff is
> needed by
> u-boot though. As it needs some space (probably less than 128 bytes) of
> this area to
> "add" in values for frequencies and ranges. So it can't be completely
> removed like
> with newer u-boots).
>

It seems like the current u-boot-env variables won't support that and it's
not clear how to guarantee the u-boot-env is set up before you flash the
new image style. It seems much safer to just stick with the current layout
for now.


>
> So, Would you be willing to do this?
>
> Regards,
> Christian
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20190920/909295c2/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel