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

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

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?
> > 
> > > 
> > > > 
> > > > 
> > > > +       };
> > > > +};
> > > > diff --git a/doc/device-tree-bindings/misc/fs_loader.txt
> > > > b/doc/device-tree-bindings/misc/fs_loader.txt
> > > > new file mode 100644
> > > > index 0000000..78bea66
> > > > --- /dev/null
> > > > +++ b/doc/device-tree-bindings/misc/fs_loader.txt
> > > > @@ -0,0 +1,52 @@
> > > > +* File system firmware loader
> > > > +
> > > > +Required properties:
> > > > +--------------------
> > > > +
> > > > +- compatible: should contain "fs_loader"
> > > > +- storage_device: which storage device loading from, could be:
> > > > +                 - mmc, usb, sata, and ubi.
> > > > +- devpart: which storage device and partition the image
> > > > loading
> > > > from,
> > > > +          this property is required for mmc, usb and sata.
> > > > +- mdtpart: which partition of ubi the image loading from, this
> > > > property is
> > > > +          required for ubi.
> > > > +- ubivol: which volume of ubi the image loading from, this
> > > > proprety is required
> > > > +         for ubi.
> > > > +
> > > > +Example of storage device and partition search set for mmc,
> > > > usb,
> > > > sata and
> > > > +ubi in device tree source as shown in below:
> > > > +
> > > > +       Example of storage type and device partition search set
> > > > for
> > > > mmc, usb,
> > > > +       sata and ubi as shown in below:
> > > > +       Example for mmc:
> > > > +       fs_loader0: fs_loader at 0 {
> > > > +               u-boot,dm-pre-reloc;
> > > > +               compatible = "fs_loader";
> > > > +               storage_device = "mmc";
> > > > +               devpart = "0:1";
> > > > +       };
> > > > +
> > > > +       Example for usb:
> > > > +       fs_loader1: fs_loader at 1 {
> > > > +               u-boot,dm-pre-reloc;
> > > > +               compatible = "fs_loader";
> > > > +               storage_device = "usb";
> > > > +               devpart = "0:1";
> > > > +       };
> > > > +
> > > > +       Example for sata:
> > > > +       fs_loader2: fs_loader at 2 {
> > > > +               u-boot,dm-pre-reloc;
> > > > +               compatible = "fs_loader";
> > > > +               storage_device = "sata";
> > > > +               devpart = "0:1";
> > > > +       };
> > > > +
> > > > +       Example for ubi:
> > > > +       fs_loader3: fs_loader at 3 {
> > > > +               u-boot,dm-pre-reloc;
> > > > +               compatible = "fs_loader";
> > > > +               storage_device = "ubi";
> > > > +               mtdpart = "UBI",
> > > > +               ubivol = "ubi0";
> > > I'm not sure about ubi. It needs to refer to a particular device
> > > I
> > > suppose. How do we know the mapping from ubi to device?
> > > 
> > Actually this design is based on file system implementation which
> > is
> > abstract from physical device, storage_device is used as interface
> > of
> > file system, so nand and qspi may having the same ubi interface.
> > 
> > May be i can change the storage_device into storage_interface to
> > avoid
> > confusing.
> OK I see. Well I suppose this is OK then, at least for now. I'm not
> have any great ideas on how to specify which ubi storage device to
> use. Let's assume we only have one for now.
> 
> Regards,
> Simon