Re: [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 
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 
> > ---
> >  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
> 
> 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
>
>
>
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

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

[Christian Lamparter]

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 
> ---
>  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

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).

But I don't think a generated image with these parameters would boot anymore.
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.

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).

So, Would you be willing to do this? 

Regards,
Christian



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

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

[Russell Senior]

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 
---
 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
-- 
2.23.0


-- 
Russell Senior, President
russ...@personaltelco.net

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel