[U-Boot] [PATCH v3 2/3] doc: dtbinding: Add file system firmware loader binding document

[Chee, Tien Fong <tien.fong.chee@intel.com>]

On Fri, 2018-06-29 at 21:19 -0700, Simon Glass wrote:
> Hi Tien Fong,
> 
> On 28 June 2018 at 01:58, Chee, Tien Fong <tien.fong.chee@intel.com>
> wrote:
> > 
> > On Thu, 2018-06-28 at 01:04 -0700, Chee, Tien Fong wrote:
> > > 
> > > On Wed, 2018-06-27 at 14:03 -0700, Simon Glass wrote:
> > > > 
> > > > 
> > > > Hi Tien Fong,
> > > > 
> > > > On 27 June 2018 at 01:32, Chee, Tien Fong <tien.fong.chee@intel
> > > > .com
> > > > > 
> > > > > 
> > > > wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On Mon, 2018-06-25 at 21:58 -0600, Simon Glass wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > On 25 June 2018 at 07:28,  <tien.fong.chee@intel.com>
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > 
> > > > > > > Add a document to describe file system firmware loader
> > > > > > > binding
> > > > > > > information.
> > > > > > > 
> > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > ---
> > > > > > >  doc/device-tree-bindings/chosen.txt         | 22
> > > > > > > ++++++++++++
> > > > > > >  doc/device-tree-bindings/misc/fs_loader.txt | 52
> > > > > > > +++++++++++++++++++++++++++++
> > > > > > >  2 files changed, 74 insertions(+)
> > > > > > >  create mode 100644 doc/device-tree-
> > > > > > > bindings/misc/fs_loader.txt
> > > > > > > 
> > > > > > > diff --git a/doc/device-tree-bindings/chosen.txt
> > > > > > > b/doc/device-
> > > > > > > tree-
> > > > > > > bindings/chosen.txt
> > > > > > > index c96b8f7..738673c 100644
> > > > > > > --- a/doc/device-tree-bindings/chosen.txt
> > > > > > > +++ b/doc/device-tree-bindings/chosen.txt
> > > > > > > @@ -73,3 +73,25 @@ Example
> > > > > > >                 u-boot,spl-boot-order = "same-as-spl",
> > > > > > > &sdmmc,
> > > > > > > "/sd
> > > > > > > hci at fe330000";
> > > > > > >         };
> > > > > > >  };
> > > > > > > +
> > > > > > > +firmware-loader property
> > > > > > > +------------------------
> > > > > > > +Multiple file system firmware loader nodes could be
> > > > > > > defined
> > > > > > > in
> > > > > > > device trees for
> > > > > > > +multiple storage type and their default partition, then
> > > > > > > a
> > > > > > > property
> > > > > > > +"firmware-loader" can be used to pass default firmware
> > > > > > > loader
> > > > > > > +node(default storage type) to the firmware loader
> > > > > > > driver.
> > > > > > > +
> > > > > > > +Example
> > > > > > > +-------
> > > > > > > +/ {
> > > > > > > +       chosen {
> > > > > > > +               firmware-loader = &fs_loader0;
> > > > > > > +       };
> > > > > > > +
> > > > > > > +       fs_loader0: fs_loader at 0 {
> > > > > > This should be :
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > +       fs_loader0: fs-loader at 0 {
> > > > > > since hyphen is used for node and property names.
> > > > > > Underscore is
> > > > > > used
> > > > > > for phandles (so you have that correct).
> > > > > > 
> > > > > Okay.
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > +               u-boot,dm-pre-reloc;
> > > > > > > +               compatible = "fs_loader";
> > > > > > How about u-boot,fs-loader since this is U-Boot specific I
> > > > > > think.
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > +               storage_device = "mmc";
> > > > > > storage-device
> > > > > > 
> > > > > Okay
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > +               devpart = "0:1";
> > > > > > I think you should use a phandle to the node containing the
> > > > > > mmc
> > > > > > device to use.
> > > > > > 
> > > > > >    storage-device = <&mmc_3 1>;
> > > > > > 
> > > > > > for example (meaning use that MMC, partition 1.
> > > > > > 
> > > > > How to get device number based on node of a storage?
> > > > > This could make design more complicated because this driver
> > > > > is
> > > > > designed
> > > > > to support both fdt and without fdt(U-boot env and hard
> > > > > coding in
> > > > > driver).
> > > > I think it is fine to support the environment as a way of
> > > > providing
> > > > this info, but there is no need to support non-DM. Everything
> > > > should
> > > > be converting to DM now.
> > > > 
> > > Ok,i would keep the DM and environment support.
> > > 
> > > > 
> > > > 
> > > > We have:
> > > > 
> > > > uclass_get_device_by_phandle_id()
> > > > device_get_global_by_of_offset()
> > > > 
> > > > I think we need to create a function called
> > > > 
> > > > device_get_global_by_phandle_id()
> > > > 
> > > > which can be used to obtain the device based on a phandle.
> > > > 
> > > I means the device number in the struct blk_desc, block device. I
> > > don't
> > >  know how the device number is built up during driver model init.
> > > Is
> > > there a function to retrive the device number?
> > After roughly checking the blk-uclass.c, i think we can get the
> > device
> > number for blok device and also mainstain the consistent interface
> > with
> > UBI(non block device), we need two parameters in FDT,
> > 1) storage-interface:
> > block device -
> >         "ide"
> >         "scsi"
> >         "atapi"
> >         "usb"
> >         "doc"
> >         "mmc"
> >         "sd"
> >         "sata"
> >         "host"
> >         "nvme"
> >         "efi"
> Do you need this? Can it not be obtained from uclass_get_name() using
> the phandle (below)?
> 
The purpose of above parameter is used to differentiate the block
devices and non block devices(like UBI). This function would be called
if it is block device.

Is the name is standard defined as above block devices string?
> > 
> > 
> > non block device
> >         "ubi"
> > 
> > 2) phandle for physical storage node(parent node) which contain the
> > uclass support as listed below for block devices:
> >         UCLASS_IDE
> >         UCLASS_SCSI
> >         UCLASS_INVALID
> >         UCLASS_MASS_STORAGE
> >         UCLASS_INVALID
> >         UCLASS_MMC
> >         UCLASS_INVALID
> >         UCLASS_AHCI
> >         UCLASS_ROOT
> >         UCLASS_NVME
> >         UCLASS_EFI
> > Above nodes must contain the child node UCLASS_BLK(this node
> > contains
> > device number)
> > 
> > No node required for UBI interface.
> > 
> > I can create a new function, which is specific for getting the
> > device
> > number from the parent node(block device) which contains the child
> > node
> > UCLASS_BLK. May be that function can be put into fs.c.
> > 
> > I'm not sure would this work as expected, any ideas/thoughts on my
> > proposal?
> Sounds right to me.
> 
Is that possible one parent device(like MMC) contains multiple child
devices in UCLASS_BLK?
> [...]
> 
> Regards,
> Simon