On Mon, Aug 7, 2017 at 2:19 PM, Brüns, Stefan <stefan.bru...@rwth-aachen.de> wrote: > On Freitag, 4. August 2017 21:31:43 CEST Rob Clark wrote: >> Needed to support efi file protocol. The fallback.efi loader wants >> to be able to read the contents of the /EFI directory to find an OS >> to boot. >> >> For reference, the expected EFI semantics are described in (v2.7 of UEFI >> spec) in section 13.5 (page 609). Or for convenience, see: >> >> http://wiki.phoenix.com/wiki/index.php/EFI_FILE_PROTOCOL#Read.28.29 >> >> The EFI level semantics are implemented in a later patch, so they are >> not too important to the understanding of this patch. >> >> Signed-off-by: Rob Clark <robdcl...@gmail.com> >> --- >> fs/fs.c | 25 +++++++++++++++++++++++++ >> include/fs.h | 21 +++++++++++++++++++++ >> 2 files changed, 46 insertions(+) > > Still, the commit message is in no way helpful when trying to understand what > your changes are actually doing.
You were the one that wanted a reference to the relevant EFI protocol, even though it is not terribly useful for understanding this patch ;-) > You introduce an arbitrary new API in the filesystem level (you neither expose > an existing API, nor are you implementing the API requested by EFI, nor > anything roughly resembling it). I am exposing the API needed to implement the EFI API. I am not sure why you describe it as arbitrary. I would describe it as posix readdir() ported to fs's stateless design. Ie. not quite the same, neither are any of fs's other APIs. > The API you expose adds an index-based directory lookup, while EFI wants an > POSIX-like directory stream. After reading through both the EFI spec and U- > Boots file system code, its clear you want to have some matching layer between > the mostly stateless U-Boot filesystem layer and the stateful EFI API. What EFI wants and the way the u-boot filesystem API works are two completely different things. The u-boot fs APIs are stateless. EFI is not, not just for directories but also for file read/write. Please see patch 16/20. > Please provide a thorough description why you create this new API in the fs > layer, state that it is a hack to achieve what you want. If sometime later > someone else wants to clean this up (both the FAT implementation, and the > API), she/he should not have to go through all the code. The fat implementation is a hack, but the API is not. Ie. it does exactly what the comment in fs.h describes. (I might do it differently if u-boot had a concept of file handles that were stateful. But that is a bit of a departure from how u-boot's fs works.) BR, -R _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
