[U-Boot] [PATCH v3 06/20] common: Generic firmware loader for file system

[Marek Vasut]

On 10/28/2017 11:43 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 10/27/2017 12:35 PM, Lukasz Majewski wrote:
>> [...]
>>>>>>>>>  common/Makefile   |   2 +
>>>>>>>>>  common/load_fs.c  | 163
>>>>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>  include/load_fs.h |  40 ++++++++++++++
>>>>>>>>>  3 files changed, 205 insertions(+)
>>>>>>>>>  create mode 100644 common/load_fs.c
>>>>>>>>>  create mode 100644 include/load_fs.h
>>>>>>>> There is alot of change here and the commit message doesn't
>>>>>>>> tell
>>>>>>>> me anything! Please describe, in detail, what your patch is
>>>>>>>> doing.
>>>>>>>>
>>>>>>>> Also you need to include more people in the review path for
>>>>>>>> this
>>>>>>>> patch.
>>>>>> These are the code factored out from splash loader, contains some
>>>>>> common functions which can be used by other file system loader
>>>>>> such as
>>>>>> fpga loadfs.
>>>>> Would it be possible to provide ./doc entry to explain how one can
>>>>> use
>>>>> this set of tools (splash/loadfs loaders) ?
>>>>>
>>>> Sure. I will provide a./doc or comment in next version. Basically,
>>>> the idea is factoring out the common code which specific handlle
>>>> image in file format loading from flash to target(memory/device)
>>>> between splash loader and fpga loadfs. So, you will see i have
>>>> declared a few weak functions, which is used for defined speficic
>>>> handling algorithm such as get_file, and fs_loading.
>>>>
>>>> Initially, my plan is creating a more generic function name and
>>>> geneirc file name, then replacing those splash_loader fs at
>>>> separate patch set.
>>>>
>>>> Now, i am working directly on splash loader. Anyway, i also like
>>>> more discussion and good comments while i am working on it.
>>>
>>> I've asked for a documentation, since I do have one idea in the
>>> back of my head.
>>>
>>> I'm wondering if other SoCs could benefit from this solution? For
>>> example when we treat the FPGA as a DSP processor which needs to
>>> have bitstream ( or better firmware ) loaded to some physical
>>> address. I'm also wondering if your work would allow for start/stop
>>> of the code execution?
>>
>> This is supposed to be a firmware loader (kind-of like the firmware
>> loader in Linux),
> 
> So in principle I should be able to load any bitstream (firmware)

Yes, using this framework.

> to
> any FPGA/SoC/whatever.

No, this part is up to you to implement.

> I do have in mind the ADI's SHARC DSP cores...

The part which calls the firmware loader API and pushes it into those
cores is up to you to implement.

>> so I have no idea what you mean by "start/stop"
>> execution.
> 
> Ok. This can be done in a separate driver. No issues with that.

IMO it indeed should.

>>> It would be best to have some kind of common code and extensions per
>>> soc/architecture.
>>
>> I can't see a usecase for that.
> 
> The use case would be to reuse/tweak this code to be able to load
> firmware for the SHARC DSP processors.

I think your driver will use the firmware loader indeed, but this
functionality you described would be part of some driver for that
hardware, not the FW loader.

-- 
Best regards,
Marek Vasut