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

[Simon Glass]

Hi Tien Fong,

On 27 June 2018 at 01:32, Chee, Tien Fong <tien.fong.chee at 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 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).

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.

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.

>> >
>> > +       };
>> > +};
>> > 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