On Mon, Aug 7, 2017 at 2:19 PM, Brüns, Stefan Stefan.Bruens@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 robdclark@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