[U-Boot] [PATCH v3 10/17] fs: Convert fs_read/write to take buffer instead of address

[Alexander Graf]

On 15.06.18 16:24, Simon Glass wrote:
> Hi Alex,
> 
> On 15 June 2018 at 06:42, Alexander Graf <agraf at suse.de> wrote:
>> The fs_read() and fs_write() functions are internal interfaces that
>> naturally want to get pointers as arguments. Most users so far even
>> have pointers and explicitly cast them into integers just to be able
>> to pass them into the function.
>>
>> Convert them over to instead take a pointer argument for the buffer.
>> That way any sandbox mapping gets greatly simplified and users of
>> the API intuitively know what to do.
>>
>> Signed-off-by: Alexander Graf <agraf at suse.de>
>> ---
>>  board/BuR/common/common.c             |  2 +-
>>  board/gdsys/p1022/controlcenterd-id.c | 10 +++++-----
>>  cmd/mvebu/bubt.c                      |  4 ++--
>>  common/splash_source.c                |  4 +++-
>>  drivers/bootcount/bootcount_ext.c     | 12 ++++++------
>>  drivers/fpga/zynqpl.c                 |  8 +++++---
>>  fs/fs.c                               | 20 ++++++++++----------
>>  include/fs.h                          | 12 ++++++------
>>  lib/efi_loader/efi_file.c             |  6 ++----
>>  9 files changed, 40 insertions(+), 38 deletions(-)
> 
> U-Boot uses addresses for loading and managing images. I don't see a
> good reason to change that. We expect all logging to emit an address
> rather than a pointer, for example. See for example all the FIT and
> legacy image stuff, bootm, all the commands, etc.

Yes, usually the point of transition from address is when you go from
command line to internal infrastructure.

If you for example take a look at struct fstype_info in fs.c, you can
see that the read and write callbacks already use pointers.

The block equivalent of fs_read() is blk_dread() which also takes a buffer.

fs_read()/fs_write() really are the odd ones out in the API. And if you
take a look at all users of the API, I'd say 90% of them already get it
"wrong" (read: they assume a 1:1 map and just randomly cast pointers to
addresses).

So IMHO this patch really is just streamlining the API to adjust it to
the rest of the U-Boot infrastructure.


Alex