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

[Chee, Tien Fong]

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.
> > 
> > > 
> > > > 
> > > > 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.
> > 
> > > 
> > > > 
> > > > 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.