| Message ID | 20240613080604.3828083-1-d-gole@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | board: beagleplay: Fix the bootpart to pick from root partition | expand |
Hi Dhruva, On 13/06/24 13:36, Dhruva Gole wrote: > The Kernel Image and DTB files are supposed to be picked from the rootfs > of the SD Card, this fails in legacy boot flow because bootpart is set > to 1:1. Fix it. > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* defconfigs and env file") > Signed-off-by: Dhruva Gole <d-gole@ti.com> Acked-by: Chirag Shilwant <c-shilwant@ti.com> > --- > board/beagle/beagleplay/beagleplay.env | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env > index bbf6b925d02c..190181c8ad0c 100644 > --- a/board/beagle/beagleplay/beagleplay.env > +++ b/board/beagle/beagleplay/beagleplay.env > @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; > led led-2 on; led led-3 off; led led-4 on > boot=mmc > mmcdev=1 > -bootpart=1:1 > +bootpart=1:2 > bootdir=/boot > boot_targets=mmc1 mmc0 > bootmeths=script extlinux efi pxe
On Thu, Jun 13, 2024 at 3:06 AM Dhruva Gole <d-gole@ti.com> wrote: > > The Kernel Image and DTB files are supposed to be picked from the rootfs > of the SD Card, this fails in legacy boot flow because bootpart is set > to 1:1. Fix it. and where is that rule defined? Production boards ship with Kernel Image and DTB's in the 1st fat partition.. Regards,
On Thu, Jun 13, 2024 at 4:47 PM Robert Nelson <robertcnelson@gmail.com> wrote: > > On Thu, Jun 13, 2024 at 3:06 AM Dhruva Gole <d-gole@ti.com> wrote: > > > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > of the SD Card, this fails in legacy boot flow because bootpart is set > > to 1:1. Fix it. > > and where is that rule defined? > > Production boards ship with Kernel Image and DTB's in the 1st fat partition.. Besides, u-boot should scan for extlinux.conf on "any" partition. Regards,
Robert, On Jun 13, 2024 at 16:51:24 -0500, Robert Nelson wrote: > On Thu, Jun 13, 2024 at 4:47 PM Robert Nelson <robertcnelson@gmail.com> wrote: > > > > On Thu, Jun 13, 2024 at 3:06 AM Dhruva Gole <d-gole@ti.com> wrote: > > > > > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > > of the SD Card, this fails in legacy boot flow because bootpart is set > > > to 1:1. Fix it. > > > > and where is that rule defined? > > > > Production boards ship with Kernel Image and DTB's in the 1st fat partition.. > > Besides, u-boot should scan for extlinux.conf on "any" partition. I can see where you come from, and I agree on the above points when it comes to stdboot. However when a user wishes to use legacy boot, and in the case there isn't extlinux.conf in such a case u-boot will rely on information in the env file (please correct me if my understanding is wrong). The env file says: bootdir=/boot Thus u-boot ends up looking for Image and dtb in /boot/Image under BOOT partition, and ultimately fails, because the /boot/Image path is generally found inside the root partition ie. 1:2 The idea of this patch is to keep support for such legacy boot alive, if you feel there's another way to do this feel free to suggest. -- Dhruva
On June 13, 2024 thus sayeth Dhruva Gole: > The Kernel Image and DTB files are supposed to be picked from the rootfs > of the SD Card, this fails in legacy boot flow because bootpart is set > to 1:1. Fix it. > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* > defconfigs and env file") > Signed-off-by: Dhruva Gole <d-gole@ti.com> > --- > board/beagle/beagleplay/beagleplay.env | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env > index bbf6b925d02c..190181c8ad0c 100644 > --- a/board/beagle/beagleplay/beagleplay.env > +++ b/board/beagle/beagleplay/beagleplay.env > @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; > led led-2 on; led led-3 off; led led-4 on > boot=mmc > mmcdev=1 > -bootpart=1:1 > +bootpart=1:2 > bootdir=/boot > boot_targets=mmc1 mmc0 > bootmeths=script extlinux efi pxe Shouldn't the fix be to just drop 'script' from bootmeths. These TI scripts needed to go away years ago. ~Bryan
On Sat, Jun 15, 2024 at 8:55 PM Bryan Brattlof <bb@ti.com> wrote: > > On June 13, 2024 thus sayeth Dhruva Gole: > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > of the SD Card, this fails in legacy boot flow because bootpart is set > > to 1:1. Fix it. > > > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* > > defconfigs and env file") > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > > --- > > board/beagle/beagleplay/beagleplay.env | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env > > index bbf6b925d02c..190181c8ad0c 100644 > > --- a/board/beagle/beagleplay/beagleplay.env > > +++ b/board/beagle/beagleplay/beagleplay.env > > @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; > > led led-2 on; led led-3 off; led led-4 on > > boot=mmc > > mmcdev=1 > > -bootpart=1:1 > > +bootpart=1:2 > > bootdir=/boot > > boot_targets=mmc1 mmc0 > > bootmeths=script extlinux efi pxe > > Shouldn't the fix be to just drop 'script' from bootmeths. These TI > scripts needed to go away years ago. I agree! nuke the non bootmeth's. ;) Regards,
On Fri, Jun 14, 2024 at 1:04 AM Dhruva Gole <d-gole@ti.com> wrote: > > Robert, > > On Jun 13, 2024 at 16:51:24 -0500, Robert Nelson wrote: > > On Thu, Jun 13, 2024 at 4:47 PM Robert Nelson <robertcnelson@gmail.com> wrote: > > > > > > On Thu, Jun 13, 2024 at 3:06 AM Dhruva Gole <d-gole@ti.com> wrote: > > > > > > > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > > > of the SD Card, this fails in legacy boot flow because bootpart is set > > > > to 1:1. Fix it. > > > > > > and where is that rule defined? > > > > > > Production boards ship with Kernel Image and DTB's in the 1st fat partition.. > > > > Besides, u-boot should scan for extlinux.conf on "any" partition. > > I can see where you come from, and I agree on the above points when it > comes to stdboot. > However when a user wishes to use legacy boot, and in the case there > isn't extlinux.conf in such a case u-boot will rely on information in > the env file (please correct me if my understanding is wrong). > > The env file says: > bootdir=/boot > > Thus u-boot ends up looking for Image and dtb in /boot/Image under BOOT > partition, and ultimately fails, because the /boot/Image path is > generally found inside the root partition ie. 1:2 > > The idea of this patch is to keep support for such legacy boot alive, if > you feel there's another way to do this feel free to suggest. For BeagleBoard.org we never used the legacy boot path on this target (or BeagleBone-AI64), this was put in place mostly for TI's u-boot scripts. ;) Regards,
Hi Robert and Bryan, On Jun 15, 2024 at 21:30:55 -0500, Robert Nelson wrote: > On Sat, Jun 15, 2024 at 8:55 PM Bryan Brattlof <bb@ti.com> wrote: > > > > On June 13, 2024 thus sayeth Dhruva Gole: > > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > > of the SD Card, this fails in legacy boot flow because bootpart is set > > > to 1:1. Fix it. > > > > > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* > > > defconfigs and env file") > > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > > > --- > > > board/beagle/beagleplay/beagleplay.env | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env > > > index bbf6b925d02c..190181c8ad0c 100644 > > > --- a/board/beagle/beagleplay/beagleplay.env > > > +++ b/board/beagle/beagleplay/beagleplay.env > > > @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; > > > led led-2 on; led led-3 off; led led-4 on > > > boot=mmc > > > mmcdev=1 > > > -bootpart=1:1 > > > +bootpart=1:2 > > > bootdir=/boot > > > boot_targets=mmc1 mmc0 > > > bootmeths=script extlinux efi pxe > > > > Shouldn't the fix be to just drop 'script' from bootmeths. These TI > > scripts needed to go away years ago. > > I agree! nuke the non bootmeth's. ;) People are still using legacy bootmethods out there. I think the cleanup will still take time till everyone is on the same page and I feel like beagle being a community platform, doesn't mandate to use a "fixed" boot flow atleast until stdboot is fully at feature parity with legacy boot. Some users may still care for the legacy boot method. From the discussions so far it seems evident to me that the BeagleBone debian images are anyway going to be unaffected by this change so that removes the question of it causing any regressions on that end. People who do want to use legacy boot and pick kernel image and DTB from their root partitions will benefit from this change for sure. So I don't really see the need/argument here to not go ahead with this patch? We can definitely have separate discussion around whether we need to remove legacy flow bits from upstream U-Boot, but no reason to keep this small fix blocked till we have those discussions is what I feel.
Hi Dhruva, On Thu, 13 Jun 2024 at 02:06, Dhruva Gole <d-gole@ti.com> wrote: > > The Kernel Image and DTB files are supposed to be picked from the rootfs > of the SD Card, this fails in legacy boot flow because bootpart is set > to 1:1. Fix it. > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* defconfigs and env file") > Signed-off-by: Dhruva Gole <d-gole@ti.com> > --- > board/beagle/beagleplay/beagleplay.env | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > What is the legacy boot flow? > diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env > index bbf6b925d02c..190181c8ad0c 100644 > --- a/board/beagle/beagleplay/beagleplay.env > +++ b/board/beagle/beagleplay/beagleplay.env > @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; > led led-2 on; led led-3 off; led led-4 on > boot=mmc > mmcdev=1 > -bootpart=1:1 > +bootpart=1:2 > bootdir=/boot > boot_targets=mmc1 mmc0 > bootmeths=script extlinux efi pxe > -- > 2.34.1 > Regards, Simon
On Mon, Jun 17, 2024 at 8:53 AM Simon Glass <sjg@chromium.org> wrote: > > Hi Dhruva, > > On Thu, 13 Jun 2024 at 02:06, Dhruva Gole <d-gole@ti.com> wrote: > > > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > of the SD Card, this fails in legacy boot flow because bootpart is set > > to 1:1. Fix it. > > > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* defconfigs and env file") > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > > --- > > board/beagle/beagleplay/beagleplay.env | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > What is the legacy boot flow? Here's the fun part. From BeagleBoard.org, all BeaglePlay's manufactured by Seeed has the eMMC booting via extlinux.conf. All BeagleBoard.org images use extlinux.conf by default for this hardware. Dhruva's just looking to make sure it works with TI's currently released sdk's. (So last year, and this quarter's releases..) Regards,
On June 17, 2024 thus sayeth Dhruva Gole: > Hi Robert and Bryan, > > On Jun 15, 2024 at 21:30:55 -0500, Robert Nelson wrote: > > On Sat, Jun 15, 2024 at 8:55 PM Bryan Brattlof <bb@ti.com> wrote: > > > > > > On June 13, 2024 thus sayeth Dhruva Gole: > > > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > > > of the SD Card, this fails in legacy boot flow because bootpart is set > > > > to 1:1. Fix it. > > > > > > > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* > > > > defconfigs and env file") > > > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > > > > --- > > > > board/beagle/beagleplay/beagleplay.env | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env > > > > index bbf6b925d02c..190181c8ad0c 100644 > > > > --- a/board/beagle/beagleplay/beagleplay.env > > > > +++ b/board/beagle/beagleplay/beagleplay.env > > > > @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; > > > > led led-2 on; led led-3 off; led led-4 on > > > > boot=mmc > > > > mmcdev=1 > > > > -bootpart=1:1 > > > > +bootpart=1:2 > > > > bootdir=/boot > > > > boot_targets=mmc1 mmc0 > > > > bootmeths=script extlinux efi pxe > > > > > > Shouldn't the fix be to just drop 'script' from bootmeths. These TI > > > scripts needed to go away years ago. > > > > I agree! nuke the non bootmeth's. ;) > > People are still using legacy bootmethods out there. I think the cleanup > will still take time till everyone is on the same page and I feel like > beagle being a community platform, doesn't mandate to use a "fixed" boot > flow atleast until stdboot is fully at feature parity with legacy boot. > Some users may still care for the legacy boot method. > > From the discussions so far it seems evident to me that the BeagleBone > debian images are anyway going to be unaffected by this change so that > removes the question of it causing any regressions on that end. > > People who do want to use legacy boot and pick kernel image and DTB from > their root partitions will benefit from this change for sure. > > So I don't really see the need/argument here to not go ahead with this patch? > > We can definitely have separate discussion around whether we need to remove > legacy flow bits from upstream U-Boot, but no reason to keep this small fix > blocked till we have those discussions is what I feel. config_distro_bootcmd.h was introduced in 2014. If we or these distributions cannot update their packaging in a decade we cannot support them. Simply hiding their inability to update their builds and shifting the work to U-Boot is not acceptable. As you have just demonstrated these legacy boot scripts have no way of being tested. You've linked to a fundamental boot issue since the very introduction of the beagleplay support. Obviously no one is using them. a200f428b5b2 ("oard: ti: am62x: Add am62x_beagleplay_* ... Fixing these scripts continue to signal to these distributions that relying on custom scripting to boot their distribution rather than packing a EFI stub or extlinux.conf file is acceptable. The reason I'm trying to block this is because we will never be able to remove them if we keep fixing them. I'm fine if we want to fix our reference boards booting Arago, but we shouldn't block BeaglePlay from cleaning up their codebase because of our reference distributions. ~Bryan
On 11:55-20240617, Dhruva Gole wrote: > Hi Robert and Bryan, > > On Jun 15, 2024 at 21:30:55 -0500, Robert Nelson wrote: > > On Sat, Jun 15, 2024 at 8:55 PM Bryan Brattlof <bb@ti.com> wrote: > > > > > > On June 13, 2024 thus sayeth Dhruva Gole: > > > > The Kernel Image and DTB files are supposed to be picked from the rootfs > > > > of the SD Card, this fails in legacy boot flow because bootpart is set > > > > to 1:1. Fix it. > > > > > > > > Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* > > > > defconfigs and env file") > > > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > > > > --- > > > > board/beagle/beagleplay/beagleplay.env | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env > > > > index bbf6b925d02c..190181c8ad0c 100644 > > > > --- a/board/beagle/beagleplay/beagleplay.env > > > > +++ b/board/beagle/beagleplay/beagleplay.env > > > > @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; > > > > led led-2 on; led led-3 off; led led-4 on > > > > boot=mmc > > > > mmcdev=1 > > > > -bootpart=1:1 > > > > +bootpart=1:2 > > > > bootdir=/boot > > > > boot_targets=mmc1 mmc0 > > > > bootmeths=script extlinux efi pxe > > > > > > Shouldn't the fix be to just drop 'script' from bootmeths. These TI > > > scripts needed to go away years ago. > > > > I agree! nuke the non bootmeth's. ;) > > People are still using legacy bootmethods out there. I think the cleanup > will still take time till everyone is on the same page and I feel like > beagle being a community platform, doesn't mandate to use a "fixed" boot > flow atleast until stdboot is fully at feature parity with legacy boot. > Some users may still care for the legacy boot method. > > From the discussions so far it seems evident to me that the BeagleBone > debian images are anyway going to be unaffected by this change so that > removes the question of it causing any regressions on that end. > > People who do want to use legacy boot and pick kernel image and DTB from > their root partitions will benefit from this change for sure. > > So I don't really see the need/argument here to not go ahead with this patch? > > We can definitely have separate discussion around whether we need to remove > legacy flow bits from upstream U-Boot, but no reason to keep this small fix > blocked till we have those discussions is what I feel. Based on an offline discussion, while it is in the ineterest of dropping legacy boot, we are currently supporting both set of users at the moment. The fix is valid for legacy boot folks.. so, for this patch, at this point in time: Reviewed-by: Nishanth Menon <nm@ti.com>
diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env index bbf6b925d02c..190181c8ad0c 100644 --- a/board/beagle/beagleplay/beagleplay.env +++ b/board/beagle/beagleplay/beagleplay.env @@ -11,7 +11,7 @@ set_led_state_start_load=led led-0 on; led led-1 off; led led-2 on; led led-3 off; led led-4 on boot=mmc mmcdev=1 -bootpart=1:1 +bootpart=1:2 bootdir=/boot boot_targets=mmc1 mmc0 bootmeths=script extlinux efi pxe
The Kernel Image and DTB files are supposed to be picked from the rootfs of the SD Card, this fails in legacy boot flow because bootpart is set to 1:1. Fix it. Fixes: a200f428b5b21 ("board: ti: am62x: Add am62x_beagleplay_* defconfigs and env file") Signed-off-by: Dhruva Gole <d-gole@ti.com> --- board/beagle/beagleplay/beagleplay.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)