On 23.01.2018 07:28, Chee, Tien Fong wrote:
On Mon, 2018-01-22 at 12:41 +0100, Simon Goldschmidt wrote:
On 22.01.2018 09:08, Chee, Tien Fong wrote:
On Thu, 2018-01-18 at 06:57 +0100, Simon Goldschmidt wrote:
On 27.12.2017 06:04, tien.fong.c...@intel.com wrote:
From: Tien Fong Chee <tien.fong.c...@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.c...@intel.com>
---
common/Makefile | 1 +
common/fs_loader.c | 309
+++++++++++++++++++++++++++++++++++++++++++++
doc/README.firmware_loader | 76 +++++++++++
include/fs_loader.h | 28 ++++
4 files changed, 414 insertions(+)
create mode 100644 common/fs_loader.c
create mode 100644 doc/README.firmware_loader
create mode 100644 include/fs_loader.h
<snip>
+#if defined(CONFIG_SPL_MMC_SUPPORT) &&
defined(CONFIG_SPL_BUILD)
+static int init_mmc(void)
+{
+ /* Just for the case MMC is not yet initialized */
+ struct mmc *mmc = NULL;
+ int err;
+
+ spl_mmc_find_device(&mmc, spl_boot_device());
+
+ err = mmc_init(mmc);
+ if (err) {
+ printf("spl: mmc init failed with error:
%d\n",
err);
+ return err;
+ }
+
+ return err;
+}
I see two problems here: First, you're ignoring the return value
of
spl_mmc_find_device() and initialize 'mmc' to NULL instead.
Wouldn't
it
be better to let 'mmc' be uninitialized and return the error code
returned by spl_mmc_find_device() if there is one?
Yeah, you are right, i should add the check on the return value. I
think that would better to initialize NULL to mmc, because there is
no
checking on the mmc in spl_mmc_find_device(), so that is possible
memory access violation/abort can be happended if unknown value in
mmc
pointer.
Second, using spl_boot_device() would prevent making the loader
work
on
mach-socfpga when spl is not loaded from mmc, right?
E.g. for the case I'm currently trying to fix (boot from qspi),
this
loader would not work although there's technically no reason
since
the
platform only has one mmc. The call to spl_boot_device() could be
replaced by the exact value here for platforms that only have one
mmc. I
don't know how to fix that, though.
The main purpose here is to initialize the mmc driver. So which
storage
user wants to load the file is totally depend what storage such as
mmc
user defines in location->name. Loader would init the storage based
on
the storage defined in location->name before accessing it. Since
the
loader only support file system at this moment, i would suggest FAT
fs
for mmc and ubi fs for qspi.
What I meant to say is this: at least on mach-socfpga, from reading
the
code, I cannot load a file from mmc when booting from qspi, as
'spl_boot_device' returns 'BOOT_DEVICE_SPI' in that case, although I
need to pass 'BOOT_DEVICE_MMC1' to 'spl_mmc_find_device'. Or am I
wrong
here?
Okay, i got you.
Yeah, you are right for use case if you need to load the file from mmc
during SPL boot and SPL is not loaded from mmc.
Since the spl_boot_device is platform dependent, you may try to add
some state machine so that this function can return correct device
type.
Secondly, you can use this function in U-Boot instead of SPL.
You're right on that. Thinking about it, I would probably use it from
U-Boot, not from SPL...
Lastly, i can declare __Weak to init_mmc, so that user can define their
own implementation.
At least somehow allowing to override which device is passed to
'spl_mmc_find_device' might be good, instead of hardcoding it to
'spl_boot_device'...
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
