[PATCH 00/15] nand: Add sandbox tests
Dario Binacchi
dario.binacchi at amarulasolutions.com
Thu Nov 9 09:23:05 CET 2023
Sean, All
On Sat, Nov 4, 2023 at 8:46 PM Sean Anderson <seanga2 at gmail.com> wrote:
>
> On 11/2/23 10:18, Dario Binacchi wrote:
> > On Thu, Nov 2, 2023 at 3:13 PM Sean Anderson <seanga2 at gmail.com> wrote:
> >>
> >> On 11/2/23 10:08, Dario Binacchi wrote:
> >>> On Thu, Nov 2, 2023 at 3:06 PM Sean Anderson <seanga2 at gmail.com> wrote:
> >>>>
> >>>> On 11/2/23 10:01, Dario Binacchi wrote:
> >>>>> Sean, All,
> >>>>>
> >>>>> On Sun, Oct 29, 2023 at 4:48 AM Sean Anderson <seanga2 at gmail.com> wrote:
> >>>>>>
> >>>>>> This series tests raw nand flash in sandbox and fixes various bugs discovered in
> >>>>>> the process. I've tried to do things in a contemporary manner, avoiding the
> >>>>>> (numerous) variations present on only a few boards. The test is pretty minimal.
> >>>>>> Future work could test the rest of the nand API as well as the MTD API.
> >>>>>>
> >>>>>> Bloat at [1] (for boards with SPL_NAND_SUPPORT enabled). Almost
> >>>>>> everything grows by a few bytes due to nand_page_size. A few boards grow more,
> >>>>>> mostly those using nand_spl_loaders.c.
> >>>>>>
> >>>>>> [1] https://gist.github.com/Forty-Bot/9694f3401893c9e706ccc374922de6c2
> >>>>>>
> >>>>>>
> >>>>>> Sean Anderson (15):
> >>>>>> spl: nand: Fix NULL-pointer dereference
> >>>>>> nand: Don't dereference NULL manufacturer_desc
> >>>>>> nand: Calculate SYS_NAND_PAGE_COUNT automatically
> >>>>>> nand: spl_loaders: Only read enough pages to load the image
> >>>>>> spl: legacy: Honor bl_len when decompressing
> >>>>>> spl: nand: Set bl_len to page size
> >>>>>> cmd: nand: Map memory before accessing it
> >>>>>> spl: nand: Map memory before accessing it
> >>>>>> mtd: Rename SPL_MTD_SUPPORT to SPL_MTD
> >>>>>> mtd: Add some fallbacks for add/del_mtd_device
> >>>>>> nand: Add function to unregister NAND devices
> >>>>>> nand: Allow reinitialization
> >>>>>> arch: sandbox: Add function to create temporary files
> >>>>>> nand: Add sandbox driver
> >>>>>> test: spl: Add a test for NAND
> >>>>>>
> >>>>>> README | 9 +-
> >>>>>> arch/sandbox/cpu/os.c | 17 +
> >>>>>> arch/sandbox/dts/test.dts | 67 ++
> >>>>>> arch/sandbox/include/asm/spl.h | 1 +
> >>>>>> cmd/nand.c | 26 +-
> >>>>>> common/spl/Kconfig | 2 +-
> >>>>>> common/spl/spl_legacy.c | 18 +-
> >>>>>> common/spl/spl_nand.c | 22 +-
> >>>>>> configs/am335x_baltos_defconfig | 3 +-
> >>>>>> configs/am335x_evm_defconfig | 3 +-
> >>>>>> configs/am335x_evm_spiboot_defconfig | 2 +-
> >>>>>> configs/am335x_guardian_defconfig | 1 -
> >>>>>> configs/am335x_hs_evm_defconfig | 2 +-
> >>>>>> configs/am335x_hs_evm_uart_defconfig | 2 +-
> >>>>>> configs/am335x_igep003x_defconfig | 3 +-
> >>>>>> configs/am335x_sl50_defconfig | 2 +-
> >>>>>> configs/am3517_evm_defconfig | 3 +-
> >>>>>> configs/am43xx_evm_defconfig | 3 +-
> >>>>>> configs/am43xx_evm_rtconly_defconfig | 3 +-
> >>>>>> configs/am43xx_evm_usbhost_boot_defconfig | 3 +-
> >>>>>> configs/am43xx_hs_evm_defconfig | 3 +-
> >>>>>> configs/am62ax_evm_r5_defconfig | 2 +-
> >>>>>> configs/am65x_evm_a53_defconfig | 2 +-
> >>>>>> configs/axm_defconfig | 1 -
> >>>>>> configs/chiliboard_defconfig | 1 -
> >>>>>> configs/cm_t43_defconfig | 2 +-
> >>>>>> configs/corvus_defconfig | 1 -
> >>>>>> configs/da850evm_nand_defconfig | 1 -
> >>>>>> configs/devkit3250_defconfig | 1 -
> >>>>>> configs/devkit8000_defconfig | 1 -
> >>>>>> configs/dra7xx_evm_defconfig | 1 -
> >>>>>> configs/draco_defconfig | 1 -
> >>>>>> configs/etamin_defconfig | 1 -
> >>>>>> .../gardena-smart-gateway-at91sam_defconfig | 1 -
> >>>>>> configs/igep00x0_defconfig | 3 +-
> >>>>>> configs/imx6ulz_smm_m2_defconfig | 2 +-
> >>>>>> configs/imx8mn_bsh_smm_s2_defconfig | 2 +-
> >>>>>> configs/j7200_evm_a72_defconfig | 2 +-
> >>>>>> configs/j7200_evm_r5_defconfig | 2 +-
> >>>>>> configs/j721e_evm_a72_defconfig | 2 +-
> >>>>>> configs/j721e_evm_r5_defconfig | 2 +-
> >>>>>> configs/j721s2_evm_a72_defconfig | 2 +-
> >>>>>> configs/j721s2_evm_r5_defconfig | 2 +-
> >>>>>> configs/m53menlo_defconfig | 1 -
> >>>>>> configs/omap35_logic_defconfig | 3 +-
> >>>>>> configs/omap35_logic_somlv_defconfig | 3 +-
> >>>>>> configs/omap3_beagle_defconfig | 3 +-
> >>>>>> configs/omap3_evm_defconfig | 3 +-
> >>>>>> configs/omap3_logic_defconfig | 3 +-
> >>>>>> configs/omap3_logic_somlv_defconfig | 3 +-
> >>>>>> configs/omapl138_lcdk_defconfig | 1 -
> >>>>>> configs/phycore-am335x-r2-regor_defconfig | 3 +-
> >>>>>> configs/phycore-am335x-r2-wega_defconfig | 3 +-
> >>>>>> configs/pxm2_defconfig | 1 -
> >>>>>> configs/rastaban_defconfig | 1 -
> >>>>>> configs/rut_defconfig | 1 -
> >>>>>> configs/sama5d3_xplained_nandflash_defconfig | 1 -
> >>>>>> configs/sama5d3xek_nandflash_defconfig | 1 -
> >>>>>> configs/sama5d4_xplained_nandflash_defconfig | 1 -
> >>>>>> configs/sama5d4ek_nandflash_defconfig | 1 -
> >>>>>> configs/sandbox64_defconfig | 10 +-
> >>>>>> configs/sandbox_defconfig | 9 +
> >>>>>> configs/sandbox_noinst_defconfig | 21 +-
> >>>>>> configs/smartweb_defconfig | 1 -
> >>>>>> configs/socfpga_secu1_defconfig | 2 +-
> >>>>>> configs/stm32746g-eval_spl_defconfig | 2 +-
> >>>>>> configs/stm32f746-disco_spl_defconfig | 2 +-
> >>>>>> configs/stm32f769-disco_spl_defconfig | 2 +-
> >>>>>> configs/stm32mp15_basic_defconfig | 2 +-
> >>>>>> configs/stm32mp15_dhcom_basic_defconfig | 2 +-
> >>>>>> configs/stm32mp15_dhcor_basic_defconfig | 2 +-
> >>>>>> configs/taurus_defconfig | 1 -
> >>>>>> configs/thuban_defconfig | 1 -
> >>>>>> drivers/mtd/Makefile | 2 +-
> >>>>>> drivers/mtd/nand/raw/Kconfig | 27 +-
> >>>>>> drivers/mtd/nand/raw/Makefile | 1 +
> >>>>>> drivers/mtd/nand/raw/am335x_spl_bch.c | 8 +-
> >>>>>> drivers/mtd/nand/raw/atmel_nand.c | 10 +-
> >>>>>> drivers/mtd/nand/raw/denali_spl.c | 5 +
> >>>>>> drivers/mtd/nand/raw/fsl_ifc_spl.c | 8 +
> >>>>>> drivers/mtd/nand/raw/lpc32xx_nand_mlc.c | 5 +
> >>>>>> drivers/mtd/nand/raw/mt7621_nand_spl.c | 5 +
> >>>>>> drivers/mtd/nand/raw/mxc_nand_spl.c | 10 +-
> >>>>>> drivers/mtd/nand/raw/mxs_nand_spl.c | 5 +
> >>>>>> drivers/mtd/nand/raw/nand.c | 66 +-
> >>>>>> drivers/mtd/nand/raw/nand_base.c | 7 +-
> >>>>>> drivers/mtd/nand/raw/nand_spl_loaders.c | 5 +-
> >>>>>> drivers/mtd/nand/raw/nand_spl_simple.c | 10 +-
> >>>>>> drivers/mtd/nand/raw/omap_gpmc.c | 3 +-
> >>>>>> drivers/mtd/nand/raw/sand_nand.c | 711 ++++++++++++++++++
> >>>>>> drivers/mtd/nand/raw/sunxi_nand_spl.c | 8 +-
> >>>>>> drivers/mtd/onenand/onenand_uboot.c | 2 -
> >>>>>> include/linux/mtd/mtd.h | 12 +
> >>>>>> include/mtd/cfi_flash.h | 2 +-
> >>>>>> include/nand.h | 3 +
> >>>>>> include/os.h | 13 +
> >>>>>> include/system-constants.h | 3 +
> >>>>>> test/dm/Makefile | 1 +
> >>>>>> test/dm/nand.c | 106 +++
> >>>>>> test/image/Kconfig | 9 +
> >>>>>> test/image/Makefile | 1 +
> >>>>>> test/image/spl_load_nand.c | 54 ++
> >>>>>> 102 files changed, 1269 insertions(+), 153 deletions(-)
> >>>>>> create mode 100644 drivers/mtd/nand/raw/sand_nand.c
> >>>>>> create mode 100644 test/dm/nand.c
> >>>>>> create mode 100644 test/image/spl_load_nand.c
> >>>>>>
> >>>>>> --
> >>>>>> 2.37.1
> >>>>>>
> >>>>>
> >>>>> The CI pipeline fails:
> >>>>> https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/pipelines/18409
> >>>>>
> >>>>> It seems like the problem is only for the clang tests (sandbox64 with
> >>>>> clang test.py and sandbox with clang test.py)
> >>>>
> >>>> I can't view that link. Can you post the error somewhere else?
> >>>>
> >>>> I saw a clang error [1] when running CI before sending this out, but I thought I had fixed it.
> >>>>
> >>>> --Sean
> >>>>
> >>>> [1] https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/18355
> >>>>
> >>>> sandbox: + sandbox
> >>>> +drivers/mtd/nand/raw/sand_nand.c:386:2: error: variable 'src' is used uninitialized whenever switch default is taken [-Werror,-Wsometimes-uninitialized]
> >>>> + default:
> >>>> + ^~~~~~~
> >>>> +drivers/mtd/nand/raw/sand_nand.c:395:14: note: uninitialized use occurs here
> >>>> + memcpy(buf, src + chip->column, to_copy);
> >>>> + ^~~
> >>>> +drivers/mtd/nand/raw/sand_nand.c:362:6: error: variable 'src' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> >>>> + if (!chip->selected)
> >>>> + ^~~~~~~~~~~~~~~
> >>>> +drivers/mtd/nand/raw/sand_nand.c:362:2: note: remove the 'if' if its condition is always false
> >>>> + ^~~~~~~~~~~~~~~~~~~~
> >>>> +drivers/mtd/nand/raw/sand_nand.c:360:15: note: initialize the variable 'src' to silence this warning
> >>>> + const u8 *src;
> >>>> + ^
> >>>> + = NULL
> >>>> +2 errors generated.
> >>>> +make[5]: *** [scripts/Makefile.build:256: drivers/mtd/nand/raw/sand_nand.o] Error 1
> >>>> +make[4]: *** [scripts/Makefile.build:397: drivers/mtd/nand/raw] Error 2
> >>>> +make[3]: *** [scripts/Makefile.build:397: drivers/mtd/nand] Error 2
> >>>> +make[2]: *** [scripts/Makefile.build:397: drivers/mtd] Error 2
> >>>> +make[1]: *** [Makefile:1858: drivers] Error 2
> >>>> +make: *** [Makefile:177: sub-make] Error 2
> >>>>
> >>>>
> >>>
> >>> +test/dm/nand.c:83:2: error: expression result unused [-Werror,-Wunused-value]
> >>> + ut_assertok(nand_erase_opts(mtd, &opts));
> >>> + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> +include/test/ut.h:309:27: note: expanded from macro 'ut_assertok'
> >>> +#define ut_assertok(cond) ut_asserteq(0, cond)
> >>> + ^~~~~~~~~~~~~~~~~~~~
> >>> +include/test/ut.h:162:2: note: expanded from macro 'ut_asserteq'
> >>> + __ret; \
> >>> + ^~~~~
> >>> +1 error generated.
> >>> +make[3]: *** [scripts/Makefile.build:257: test/dm/nand.o] Error 1
> >>> +make[2]: *** [scripts/Makefile.build:397: test/dm] Error 2
> >>> +make[1]: *** [Makefile:1858: test] Error 2
> >>> +make: *** [Makefile:177: sub-make] Error 2
> >>
> >> ...well that's a new one. Wonder why it doesn't complain about all the other uses of
> >> ut_assertok (which, invariably, discard the value in the same way).
> >
> > That's what I thought as well. It seems to be related to CLANG. Could it be the
> > 'unused-value' flag? I can't figure out why it causes the problem at
> > that point,
> > given that the 'ut_assertok()' is called even before the one that
> > generates the error.
>
> It was because of the comma instead of a semicolon on the previous line.
Great!
5022c5112
< + opts.length = mtd->erasesize * 2,
---
> + opts.length = mtd->erasesize * 2;
Just before the ut_assertok() pointed by the test failure message.
And I confirm to you that the CI pipeline has now finished successfully.
Many thanks!
Regards
Dario
>
> --Sean
--
Dario Binacchi
Senior Embedded Linux Developer
dario.binacchi at amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info at amarulasolutions.com
www.amarulasolutions.com
More information about the U-Boot
mailing list