On Thu, 2018-01-18 at 08:18 -0500, Tom Rini wrote:
On Thu, Jan 18, 2018 at 03:42:14AM +0000, Chee, Tien Fong wrote:
On Tue, 2018-01-16 at 09:35 -0500, Tom Rini wrote:
On Tue, Jan 16, 2018 at 07:58:00AM +0000, Chee, Tien Fong wrote:
On Mon, 2018-01-15 at 11:36 -0500, Tom Rini wrote:
On Wed, Dec 27, 2017 at 01:04:38PM +0800, tien.fong.chee@inte l.co m wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
This is file system generic loader which can be used to load the file image from the storage into target such as memory. The consumer driver would then use this loader to program whatever, ie. the FPGA device.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Please add Lothar's Reviewed-by for v7. There's a number of minor checkpatch.pl issues that checkpatch.pl can in turn fixup itself, please correct them.
I have ran the checkpatch.pl on this patch, i didn't see any error.
When I ran it, it was pointing out cases where you have: if (foo->bar != NULL) when you can just use: if (foo->bar)
It's weird for checkpatch.pl not showing the same. I would fix the code with above example.
That is odd. Are you running it from within the U-Boot tree?
Yes.
[snip]
diff --git a/common/fs_loader.c b/common/fs_loader.c new file mode 100644 index 0000000..56d29b6 --- /dev/null +++ b/common/fs_loader.c @@ -0,0 +1,309 @@ +/*
- Copyright (C) 2017 Intel Corporation <www.intel.com>
- SPDX-License-Identifier: GPL-2.0
- */
+#include <common.h> +#include <errno.h> +#include <fs.h> +#include <fs_loader.h> +#include <nand.h> +#include <sata.h> +#include <spi.h> +#include <spi_flash.h> +#include <spl.h>
This wants <asm/spl.h> which is not globally available, so you need to come up with something here. At least making this Kconfig- enabled will be a start and perhaps OK for now.
I can enable the Kconfig, and put the caution about dependency on <asm/spl.h> in document.
Well, you need to make that part of the code depend on CONFIG_SPL as SPL support requires <asm/spl.h> to exist. Perhaps part of the code needs to be refactored to more easily deal with SPL not always being present?
How about i just move the whole enum { } from line 17 to line 35 in asm/spl.h to fs.h, and then enum{} above replaced with #include <fs.h>?
asm/spl.h
#if defined(CONFIG_ARCH_OMAP2PLUS) \ || defined(CONFIG_EXYNOS4) || defined(CONFIG_EXYNOS5) \ || defined(CONFIG_EXYNOS4210) /* Platform-specific defines */ #include <asm/arch/spl.h>
#else #include <fs.h> <-- replacement #endif [...]
No, because that's still arch specific code and will blow up some platforms that do not have asm/spl.h at all and is still non-portable (look at the various asm/spl.h files that do exist for how BOOT_DEVICE_xxx is handled in different places).
Ok, you are right.
It's OK to restructure your code to have: #ifdef CONFIG_SPL #include <spl.h> ... SPL specific functions #endif
Okay, noted.