| Message ID | 20220422182748.2309992-4-sean.anderson@seco.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | spl: Use common function for loading/parsing images | expand |
On Fri, Apr 22, 2022 at 02:27:41PM -0400, Sean Anderson wrote: > This converts the fat loader to use spl_load. > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > Reviewed-by: Stefan Roese <sr@denx.de> On am335x_evm_defconfig, and booting from a FAT SD card I see: Trying to boot from MMC1 spl_load_image_fat: error reading image u-boot.img, err - 0 SPL: failed to boot from all boot devices ### ERROR ### Please RESET the board ### Note that while not _this_ patch (but likely the one for raw) breaks booting from my mx6cuboxi via SD card with: Trying to boot from MMC1 [hang]
Hi Tom, On 5/4/22 8:06 PM, Tom Rini wrote: > On Fri, Apr 22, 2022 at 02:27:41PM -0400, Sean Anderson wrote: > >> This converts the fat loader to use spl_load. >> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> Reviewed-by: Stefan Roese <sr@denx.de> > > On am335x_evm_defconfig, and booting from a FAT SD card I see: > Trying to boot from MMC1 > spl_load_image_fat: error reading image u-boot.img, err - 0 > SPL: failed to boot from all boot devices > ### ERROR ### Please RESET the board ### Looks like spl_load_image_fat was expecting bytes read and not an errno (and so turned your success into a failure). Will fix. > Note that while not _this_ patch (but likely the one for raw) breaks > booting from my mx6cuboxi via SD card with: > Trying to boot from MMC1 > [hang] > Hm, if there's a problem you should at least see "mmc_load_image_raw_sector: mmc block read error\n" Could you enable DEBUG in common/spl/spl.c and common/spl/spl_mmc.c? --Sean
On Thu, May 05, 2022 at 10:51:39AM -0400, Sean Anderson wrote: > Hi Tom, > > On 5/4/22 8:06 PM, Tom Rini wrote: > > On Fri, Apr 22, 2022 at 02:27:41PM -0400, Sean Anderson wrote: > > > >> This converts the fat loader to use spl_load. > >> > >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> > >> Reviewed-by: Stefan Roese <sr@denx.de> > > > > On am335x_evm_defconfig, and booting from a FAT SD card I see: > > Trying to boot from MMC1 > > spl_load_image_fat: error reading image u-boot.img, err - 0 > > SPL: failed to boot from all boot devices > > ### ERROR ### Please RESET the board ### > > Looks like spl_load_image_fat was expecting bytes read and > not an errno (and so turned your success into a failure). Will fix. OK. > > Note that while not _this_ patch (but likely the one for raw) breaks > > booting from my mx6cuboxi via SD card with: > > Trying to boot from MMC1 > > [hang] > > > > Hm, if there's a problem you should at least see > "mmc_load_image_raw_sector: mmc block read error\n" > > Could you enable DEBUG in common/spl/spl.c and common/spl/spl_mmc.c? U-Boot SPL 2022.07-rc1-00267-g064334e21dde-dirty (May 05 2022 - 11:33:38 -0400) >>SPL: board_init_r() spl_init Trying to boot from MMC1 spl: mmc boot mode: raw hdr read sector 8a, count=1
On 5/5/22 11:38 AM, Tom Rini wrote: > On Thu, May 05, 2022 at 10:51:39AM -0400, Sean Anderson wrote: >> Hi Tom, >> >> On 5/4/22 8:06 PM, Tom Rini wrote: >> > On Fri, Apr 22, 2022 at 02:27:41PM -0400, Sean Anderson wrote: >> > >> >> This converts the fat loader to use spl_load. >> >> >> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> >> Reviewed-by: Stefan Roese <sr@denx.de> >> > >> > On am335x_evm_defconfig, and booting from a FAT SD card I see: >> > Trying to boot from MMC1 >> > spl_load_image_fat: error reading image u-boot.img, err - 0 >> > SPL: failed to boot from all boot devices >> > ### ERROR ### Please RESET the board ### >> >> Looks like spl_load_image_fat was expecting bytes read and >> not an errno (and so turned your success into a failure). Will fix. > > OK. > >> > Note that while not _this_ patch (but likely the one for raw) breaks >> > booting from my mx6cuboxi via SD card with: >> > Trying to boot from MMC1 >> > [hang] >> > >> >> Hm, if there's a problem you should at least see >> "mmc_load_image_raw_sector: mmc block read error\n" >> >> Could you enable DEBUG in common/spl/spl.c and common/spl/spl_mmc.c? > > U-Boot SPL 2022.07-rc1-00267-g064334e21dde-dirty (May 05 2022 - 11:33:38 -0400) >>>SPL: board_init_r() > spl_init > Trying to boot from MMC1 > spl: mmc boot mode: raw > hdr read sector 8a, count=1 > Looks like there's a bug in spl_simple_read size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len; should be size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : info->bl_len; The funny thing is that this kind of bug results in a -Wuninitialized warning on godbolt [1] but *not* from my installed gcc 9.4... Not sure what's going on there. In other bugs, there was also a problem calculating bl_shift (ffs instead of fls). I probably should have tested this the first two times around, but I now have a "raw" MMC working. I expect that there are probably some similar bugs lurking in spi/net/nor. I will try and test net, but I don't think I have any spi/nor boards I can test with. --Sean [1] https://godbolt.org/z/fnaGKvTMr
On Thu, May 05, 2022 at 02:53:55PM -0400, Sean Anderson wrote: > > > On 5/5/22 11:38 AM, Tom Rini wrote: > > On Thu, May 05, 2022 at 10:51:39AM -0400, Sean Anderson wrote: > >> Hi Tom, > >> > >> On 5/4/22 8:06 PM, Tom Rini wrote: > >> > On Fri, Apr 22, 2022 at 02:27:41PM -0400, Sean Anderson wrote: > >> > > >> >> This converts the fat loader to use spl_load. > >> >> > >> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> > >> >> Reviewed-by: Stefan Roese <sr@denx.de> > >> > > >> > On am335x_evm_defconfig, and booting from a FAT SD card I see: > >> > Trying to boot from MMC1 > >> > spl_load_image_fat: error reading image u-boot.img, err - 0 > >> > SPL: failed to boot from all boot devices > >> > ### ERROR ### Please RESET the board ### > >> > >> Looks like spl_load_image_fat was expecting bytes read and > >> not an errno (and so turned your success into a failure). Will fix. > > > > OK. > > > >> > Note that while not _this_ patch (but likely the one for raw) breaks > >> > booting from my mx6cuboxi via SD card with: > >> > Trying to boot from MMC1 > >> > [hang] > >> > > >> > >> Hm, if there's a problem you should at least see > >> "mmc_load_image_raw_sector: mmc block read error\n" > >> > >> Could you enable DEBUG in common/spl/spl.c and common/spl/spl_mmc.c? > > > > U-Boot SPL 2022.07-rc1-00267-g064334e21dde-dirty (May 05 2022 - 11:33:38 -0400) > >>>SPL: board_init_r() > > spl_init > > Trying to boot from MMC1 > > spl: mmc boot mode: raw > > hdr read sector 8a, count=1 > > > > Looks like there's a bug in spl_simple_read > > size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len; > > should be > > size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : info->bl_len; > > The funny thing is that this kind of bug results in a -Wuninitialized warning > on godbolt [1] but *not* from my installed gcc 9.4... Not sure what's going on > there. > > In other bugs, there was also a problem calculating bl_shift (ffs instead of fls). > I probably should have tested this the first two times around, but I now have a "raw" > MMC working. I expect that there are probably some similar bugs lurking in spi/net/nor. > I will try and test net, but I don't think I have any spi/nor boards I can test with. OK. When you're reasonably confident SPI should work I'll go fight around with the am335x_evm and check my notes on how to make it SPI boot. Looking at am335x_evm_spiboot_defconfig it would cover what you're wanting to check, yes?
On 5/5/22 2:58 PM, Tom Rini wrote: > On Thu, May 05, 2022 at 02:53:55PM -0400, Sean Anderson wrote: >> >> >> On 5/5/22 11:38 AM, Tom Rini wrote: >> > On Thu, May 05, 2022 at 10:51:39AM -0400, Sean Anderson wrote: >> >> Hi Tom, >> >> >> >> On 5/4/22 8:06 PM, Tom Rini wrote: >> >> > On Fri, Apr 22, 2022 at 02:27:41PM -0400, Sean Anderson wrote: >> >> > >> >> >> This converts the fat loader to use spl_load. >> >> >> >> >> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> >> >> Reviewed-by: Stefan Roese <sr@denx.de> >> >> > >> >> > On am335x_evm_defconfig, and booting from a FAT SD card I see: >> >> > Trying to boot from MMC1 >> >> > spl_load_image_fat: error reading image u-boot.img, err - 0 >> >> > SPL: failed to boot from all boot devices >> >> > ### ERROR ### Please RESET the board ### >> >> >> >> Looks like spl_load_image_fat was expecting bytes read and >> >> not an errno (and so turned your success into a failure). Will fix. >> > >> > OK. >> > >> >> > Note that while not _this_ patch (but likely the one for raw) breaks >> >> > booting from my mx6cuboxi via SD card with: >> >> > Trying to boot from MMC1 >> >> > [hang] >> >> > >> >> >> >> Hm, if there's a problem you should at least see >> >> "mmc_load_image_raw_sector: mmc block read error\n" >> >> >> >> Could you enable DEBUG in common/spl/spl.c and common/spl/spl_mmc.c? >> > >> > U-Boot SPL 2022.07-rc1-00267-g064334e21dde-dirty (May 05 2022 - 11:33:38 -0400) >> >>>SPL: board_init_r() >> > spl_init >> > Trying to boot from MMC1 >> > spl: mmc boot mode: raw >> > hdr read sector 8a, count=1 >> > >> >> Looks like there's a bug in spl_simple_read >> >> size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len; >> >> should be >> >> size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : info->bl_len; >> >> The funny thing is that this kind of bug results in a -Wuninitialized warning >> on godbolt [1] but *not* from my installed gcc 9.4... Not sure what's going on >> there. >> >> In other bugs, there was also a problem calculating bl_shift (ffs instead of fls). >> I probably should have tested this the first two times around, but I now have a "raw" >> MMC working. I expect that there are probably some similar bugs lurking in spi/net/nor. >> I will try and test net, but I don't think I have any spi/nor boards I can test with. > > OK. When you're reasonably confident SPI should work I'll go fight > around with the am335x_evm and check my notes on how to make it SPI > boot. Looking at am335x_evm_spiboot_defconfig it would cover what > you're wanting to check, yes? > I'll send v3 soon and that should be good to test with. Falcon mode SPI boot also needs to be tested, which seems to be rather unpopular: git grep -l SPI_LOAD $(git grep -l OS_BOOT configs/) configs/am57xx_evm_defconfig configs/display5_defconfig configs/display5_factory_defconfig configs/dra7xx_evm_defconfig configs/imx28_xea_defconfig configs/xilinx_zynq_virt_defconfig configs/xilinx_zynqmp_virt_defconfig I'm not sure whether the zynq stuff is virtual as in qemu or something else. --Sean
On Thu, May 05, 2022 at 03:05:06PM -0400, Sean Anderson wrote: > > > On 5/5/22 2:58 PM, Tom Rini wrote: > > On Thu, May 05, 2022 at 02:53:55PM -0400, Sean Anderson wrote: > >> > >> > >> On 5/5/22 11:38 AM, Tom Rini wrote: > >> > On Thu, May 05, 2022 at 10:51:39AM -0400, Sean Anderson wrote: > >> >> Hi Tom, > >> >> > >> >> On 5/4/22 8:06 PM, Tom Rini wrote: > >> >> > On Fri, Apr 22, 2022 at 02:27:41PM -0400, Sean Anderson wrote: > >> >> > > >> >> >> This converts the fat loader to use spl_load. > >> >> >> > >> >> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> > >> >> >> Reviewed-by: Stefan Roese <sr@denx.de> > >> >> > > >> >> > On am335x_evm_defconfig, and booting from a FAT SD card I see: > >> >> > Trying to boot from MMC1 > >> >> > spl_load_image_fat: error reading image u-boot.img, err - 0 > >> >> > SPL: failed to boot from all boot devices > >> >> > ### ERROR ### Please RESET the board ### > >> >> > >> >> Looks like spl_load_image_fat was expecting bytes read and > >> >> not an errno (and so turned your success into a failure). Will fix. > >> > > >> > OK. > >> > > >> >> > Note that while not _this_ patch (but likely the one for raw) breaks > >> >> > booting from my mx6cuboxi via SD card with: > >> >> > Trying to boot from MMC1 > >> >> > [hang] > >> >> > > >> >> > >> >> Hm, if there's a problem you should at least see > >> >> "mmc_load_image_raw_sector: mmc block read error\n" > >> >> > >> >> Could you enable DEBUG in common/spl/spl.c and common/spl/spl_mmc.c? > >> > > >> > U-Boot SPL 2022.07-rc1-00267-g064334e21dde-dirty (May 05 2022 - 11:33:38 -0400) > >> >>>SPL: board_init_r() > >> > spl_init > >> > Trying to boot from MMC1 > >> > spl: mmc boot mode: raw > >> > hdr read sector 8a, count=1 > >> > > >> > >> Looks like there's a bug in spl_simple_read > >> > >> size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len; > >> > >> should be > >> > >> size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : info->bl_len; > >> > >> The funny thing is that this kind of bug results in a -Wuninitialized warning > >> on godbolt [1] but *not* from my installed gcc 9.4... Not sure what's going on > >> there. > >> > >> In other bugs, there was also a problem calculating bl_shift (ffs instead of fls). > >> I probably should have tested this the first two times around, but I now have a "raw" > >> MMC working. I expect that there are probably some similar bugs lurking in spi/net/nor. > >> I will try and test net, but I don't think I have any spi/nor boards I can test with. > > > > OK. When you're reasonably confident SPI should work I'll go fight > > around with the am335x_evm and check my notes on how to make it SPI > > boot. Looking at am335x_evm_spiboot_defconfig it would cover what > > you're wanting to check, yes? > > > > I'll send v3 soon and that should be good to test with. Falcon mode SPI boot also needs > to be tested, which seems to be rather unpopular: > > git grep -l SPI_LOAD $(git grep -l OS_BOOT configs/) > configs/am57xx_evm_defconfig > configs/display5_defconfig > configs/display5_factory_defconfig > configs/dra7xx_evm_defconfig > configs/imx28_xea_defconfig > configs/xilinx_zynq_virt_defconfig > configs/xilinx_zynqmp_virt_defconfig > > I'm not sure whether the zynq stuff is virtual as in qemu or something else. > I believe it's QEMU. But ah, right, dra7xx_evm has SPI too. I see I have SPI writing notes for that, so that's probably the best bet for a board for me to test all of this on.
diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c index 5b270541fc..c092eb3481 100644 --- a/common/spl/spl_fat.c +++ b/common/spl/spl_fat.c @@ -61,6 +61,11 @@ int spl_load_image_fat(struct spl_image_info *spl_image, { int err; struct image_header *header; + struct spl_load_info load = { + .read = spl_fit_read, + .bl_len = 1, + .filename = filename, + }; err = spl_register_fat_device(block_dev, partition); if (err) @@ -72,36 +77,7 @@ int spl_load_image_fat(struct spl_image_info *spl_image, if (err <= 0) goto end; - if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) && - image_get_magic(header) == FDT_MAGIC) { - err = file_fat_read(filename, (void *)CONFIG_SYS_LOAD_ADDR, 0); - if (err <= 0) - goto end; - err = spl_parse_image_header(spl_image, bootdev, - (struct image_header *)CONFIG_SYS_LOAD_ADDR); - if (err == -EAGAIN) - return err; - if (err == 0) - err = 1; - } else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) && - image_get_magic(header) == FDT_MAGIC) { - struct spl_load_info load; - - debug("Found FIT\n"); - load.read = spl_fit_read; - load.bl_len = 1; - load.filename = (void *)filename; - load.priv = NULL; - - return spl_load_simple_fit(spl_image, &load, 0, header); - } else { - err = spl_parse_image_header(spl_image, bootdev, header); - if (err) - goto end; - - err = file_fat_read(filename, - (u8 *)(uintptr_t)spl_image->load_addr, 0); - } + err = spl_load(spl_image, bootdev, &load, header, err, 0); end: #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT