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

[Chee, Tien Fong]

On Mon, 2018-06-25 at 21:58 -0600, Simon Glass wrote:
> Hi,
> 
> On 25 June 2018 at 07:28,  <tien.fong.chee at intel.com> wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee at intel.com>
> > 
> > Add a document to describe file system firmware loader binding
> > information.
> > 
> > Signed-off-by: Tien Fong Chee <tien.fong.chee at 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).
> > 
> > +       };
> > +};
> > 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. 
> > 
> > +       };
> > --
> > 2.2.0
> > 
> Regards,
> Simon