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

[Simon Glass]

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, "/sdhci 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).

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

> +               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.

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

> +       };
> --
> 2.2.0
>

Regards,
Simon