Hi Tien Fong,

On 27 June 2018 at 01:32, Chee, Tien Fong <tien.fong.c...@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.c...@intel.com> wrote:
>> >
>> > From: Tien Fong Chee <tien.fong.c...@intel.com>
>> >
>> > Add a document to describe file system firmware loader binding
>> > information.
>> >
>> > Signed-off-by: Tien Fong Chee <tien.fong.c...@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@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@0 {
>> This should be :
>>
>> >
>> > +       fs_loader0: fs-loader@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@0 {
>> > +               u-boot,dm-pre-reloc;
>> > +               compatible = "fs_loader";
>> > +               storage_device = "mmc";
>> > +               devpart = "0:1";
>> > +       };
>> > +
>> > +       Example for usb:
>> > +       fs_loader1: fs_loader@1 {
>> > +               u-boot,dm-pre-reloc;
>> > +               compatible = "fs_loader";
>> > +               storage_device = "usb";
>> > +               devpart = "0:1";
>> > +       };
>> > +
>> > +       Example for sata:
>> > +       fs_loader2: fs_loader@2 {
>> > +               u-boot,dm-pre-reloc;
>> > +               compatible = "fs_loader";
>> > +               storage_device = "sata";
>> > +               devpart = "0:1";
>> > +       };
>> > +
>> > +       Example for ubi:
>> > +       fs_loader3: fs_loader@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to