Re: [Buildroot] [PATCH v3 3/4] configs/rock5b: use the arm64 rootfs partition-type-uuid

[Niklas Cassel via buildroot <buildroot@buildroot.org>]

On Tue, Oct 22, 2024 at 10:49:45PM +0200, Thomas Petazzoni wrote:
> Hello Niklas,
> 
> On Wed, 16 Oct 2024 15:24:42 +0200
> Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:
> 
> > diff --git a/board/radxa/rock5b/genimage.cfg b/board/radxa/rock5b/genimage.cfg
> > index 43bb65bdd9..8c56251135 100644
> > --- a/board/radxa/rock5b/genimage.cfg
> > +++ b/board/radxa/rock5b/genimage.cfg
> > @@ -12,7 +12,7 @@ image sdcard.img {
> >  	}
> >  
> >  	partition rootfs {
> > -		partition-type-uuid = L
> > +		partition-type-uuid = root-arm64
> 
> In principle, this looks fine, but this "breaks" the pattern that we
> have document in the Buildroot manual at
> https://buildroot.org/downloads/manual/manual.html#writing-genimage-cfg:
> 
>    For GPT partitions, the partition-type-uuid value must be U for the
>    EFI System Partition (expanded to
>    c12a7328-f81f-11d2-ba4b-00a0c93ec93b by genimage), F for a FAT
>    partition (expanded to ebd0a0a2-b9e5-4433-87c0-68b6b72699c7 by
>    genimage) or L for the root filesystem or other filesystems
>    (expanded to 0fc63daf-8483-4772-8e79-3d69d8477de4 by genimage). Even
>    though L is the default value of genimage, we prefer to have it
>    explicitly specified in our genimage.cfg files. Finally, these
>    shortcuts should be used without double quotes, e.g
>    partition-type-uuid = U. If an explicit GUID is specified,
>    lower-case letters should be used. 
> 
> That being said, we already have a few genimage.cfg files that use
> b921b045-1df0-41c3-af44-4c6f280d3fae which is root-arm64:
> 
> board/aarch64-efi/genimage-efi.cfg:             partition-type-uuid = b921b045-1df0-41c3-af44-4c6f280d3fae
> board/arm/fvp-ebbr/genimage.cfg:                partition-type-uuid = b921b045-1df0-41c3-af44-4c6f280d3fae
> board/pine64/rockpro64/genimage.cfg:            partition-type-uuid = b921b045-1df0-41c3-af44-4c6f280d3fae
> board/qemu/aarch64-ebbr/genimage.cfg:           partition-type-uuid = b921b045-1df0-41c3-af44-4c6f280d3fae
> board/qemu/aarch64-sbsa/genimage.cfg:           partition-type-uuid = b921b045-1df0-41c3-af44-4c6f280d3fae
> 
> My point is that we want *consistency*. So either we use "L" for all
> rootfs partitions. Or we use root-${ARCH} for all rootfs partitions.
> But we don't use "L" for some, "root-${ARCH}" for some others, and a
> complete UUID in the matching a known root-${ARCH} for yet some others.
> I don't have a strong opinion either way between L or root-${ARCH}, all
> I'm expecting is consistency, between the defconfigs, and with the
> manual.
> 
> Do you want to tackle this? :-)

I don't feel comfortable to change it for platforms that I don't have
access too.

The reason why I saw this was because u-boot has support for searching
for a suitable rootfs (if CONFIG_PARTITION_TYPE_GUID).

and then specifying the type using type=
in e.g. PARTS_DEFAULT.

However:
1) rock5b does not have the uboot CONFIG_PARTITION_TYPE_GUID Kconfig enabled.
2) rockchip is incorrectly specifying the arm64 partition type GUID:
https://github.com/u-boot/u-boot/blob/master/include/configs/rockchip-common.h#L19
using uuid= and not type=.

So currently a uboot built for rockchip will only automatically find the rootfs
partition if the rootfs partition (incorrectly) has set the partition UUID to
the arm64 partition type UUID.
Rock5b in buildroot does not rely on this automatic finding of rootfs anyway,
so all good.

However, there might be other uboot configurations in uboot that actually has
set the type= a specific partition type GUID in PARTS_DEFAULT, and I don't
want to risk breaking any platform.

As written in the commit log, the patch in $subject does not introduce any
functional change, so it is completely fine to ignore this patch.


Kind regards,
Niklas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot