[U-Boot] [PATCH v2 3/3] common: Generic loader for file system

[Marek Vasut]

On 06/07/2018 12:11 PM, Chee, Tien Fong wrote:
> On Thu, 2018-06-07 at 11:50 +0200, Marek Vasut wrote:
>> On 06/07/2018 11:45 AM, Chee, Tien Fong wrote:
>>>
>>> On Thu, 2018-06-07 at 10:41 +0200, Marek Vasut wrote:
>>>>
>>>> On 06/07/2018 10:36 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Thu, 2018-06-07 at 08:51 +0200, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 06/07/2018 06:04 AM, Chee, Tien Fong wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, 2018-06-06 at 10:39 +0200, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 05/24/2018 07:04 AM, tien.fong.chee at intel.com wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> From: Tien Fong Chee <tien.fong.chee at 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.chee at intel.com
>>>>>>>>>>
>>>>>>>>> ---
>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> +static int fs_loader_probe(struct udevice *dev)
>>>>>>>>> +{
>>>>>>>>> +	return 0;
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> +static const struct udevice_id fs_loader_ids[] = {
>>>>>>>>> +	{ .compatible = "fs_loader"},
>>>>>>>> Why exactly is there a DT compatible for a firmware
>>>>>>>> loader?
>>>>>>>>
>>>>>>> Correct me if i'm wrong, this is required to look the
>>>>>>> platform
>>>>>>> data
>>>>>>> from DTS, right? Details of DTS in patch 2.
>>>>>> How so ? The FW loader should behave as a library for other
>>>>>> drivers
>>>>>> to
>>>>>> use, not like a driver.
>>>>>>
>>>>> The fs_loader node in DTS just provide a way for user to tell
>>>>> the
>>>>> firmware loader what storage device and default partition to
>>>>> load
>>>>> data
>>>>> from. Default partition can be overriden through the variable
>>>>> environment.
>>>> So that's sitting in the chosen node ? But why do you need to
>>>> match
>>>> on it ?
>>>>
>>> This is new device tree binding for firmware loader called
>>> fs_loader
>>> node. firmware loader need to know where is the storage device it
>>> need
>>> to load from. Those storage device would be defined in fs_loader
>>> node.
>> This is a configuration. You do not need (new) a DT compatible for
>> that.
>> So why is the DT compatible even needed in the FW loader at all ?
>>
> I thought DT compatible is used by driver to find the fs_loader node in
> DTS. May be i am wrong.

There should be no FW loader in the DTS. Why would there be one ?

>>>
>>>>
>>>>>
>>>>> Caller/other drivers can create different firmware loader
>>>>> instance
>>>>> based on different fs_loader node(different storage device) for
>>>>> their
>>>>> loading purpose.
>>>> They can, but that could be done even without the DT compatible.
>>>>
>>> Yes, once you get dev, you can set the storage type and partiiton
>>> attributes through accessing dev->platdata.
>> Can you show me the DT patch needed for this to work ?
>>
> I can send you the patches separately, but this driver is designed to
> work with/without DT.

This is not a driver though, it should be a library (or so) for drivers
to use.

>>>
>>>>
>>>>>
>>>>> Why this cannot be used by other driver?
>>>> I don't understand the question.
>>>>
>>> I means why u say this firmware loader cannot be used by other
>>> drivers.
>>> I have tested with FPGA Manager driver(as caller).
>> It must be used by other drivers, that is the point.
>>
> yes, it can be used by FPGA manager driver.
> 


-- 
Best regards,
Marek Vasut