From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chee, Tien Fong Date: Mon, 9 Jul 2018 06:50:45 +0000 Subject: [U-Boot] [PATCH v3 2/3] doc: dtbinding: Add file system firmware loader binding document In-Reply-To: References: <1529933338-11010-1-git-send-email-tien.fong.chee@intel.com> <1529933338-11010-3-git-send-email-tien.fong.chee@intel.com> <1530088347.9999.11.camel@intel.com> <1530173080.10114.9.camel@intel.com> <1530176335.10114.24.camel@intel.com> <1530519981.9877.13.camel@intel.com> Message-ID: <1531119044.9974.5.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Sun, 2018-07-08 at 20:39 -0600, Simon Glass wrote: > Hi Tien Fong, > > On 2 July 2018 at 01:26, Chee, Tien Fong > wrote: > > > > On Fri, 2018-06-29 at 21:19 -0700, Simon Glass wrote: > > > > > > Hi Tien Fong, > > > > > > On 28 June 2018 at 01:58, Chee, Tien Fong > > om> > > > wrote: > > > > > > > > > > > > On Thu, 2018-06-28 at 01:04 -0700, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > 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 > > > > > ntel > > > > > > .com > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, 2018-06-25 at 21:58 -0600, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > On 25 June 2018 at 07:28,   > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee > > > > > > > > > > > > > > > > > > Add a document to describe file system firmware > > > > > > > > > loader > > > > > > > > > binding > > > > > > > > > information. > > > > > > > > > > > > > > > > > > Signed-off-by: Tien Fong Chee > > > > > > > > om> > > > > > > > > > --- > > > > > > > > >  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? > > > > After roughly checking the blk-uclass.c, i think we can get the > > > > device > > > > number for blok device and also mainstain the consistent > > > > interface > > > > with > > > > UBI(non block device), we need two parameters in FDT, > > > > 1) storage-interface: > > > > block device - > > > >         "ide" > > > >         "scsi" > > > >         "atapi" > > > >         "usb" > > > >         "doc" > > > >         "mmc" > > > >         "sd" > > > >         "sata" > > > >         "host" > > > >         "nvme" > > > >         "efi" > > > Do you need this? Can it not be obtained from uclass_get_name() > > > using > > > the phandle (below)? > > > > > The purpose of above parameter is used to differentiate the block > > devices and non block devices(like UBI). This function would be > > called > > if it is block device. > Did you see the strings in the uclass drivers? E.g.: > > UCLASS_DRIVER(mmc) = { >         .id             = UCLASS_MMC, >         .name           = "mmc", >         .flags          = DM_UC_FLAG_SEQ_ALIAS, >         .per_device_auto_alloc_size = sizeof(struct mmc_uclass_priv), > }; > Yeah, but i'm not sure all uclass drivers having same string as above interface string. Anyway, i found alternate way using blk structure instead of the interface string. You can check the v[4] patchset i have submitted from last week. > > > > > > Is the name is standard defined as above block devices string? > I don't understand this. The same is defined for all uclasses that > can > have a block device as a child. > > > > > > > > > > > > > > > > > > > > > > non block device > > > >         "ubi" > > > > > > > > 2) phandle for physical storage node(parent node) which contain > > > > the > > > > uclass support as listed below for block devices: > > > >         UCLASS_IDE > > > >         UCLASS_SCSI > > > >         UCLASS_INVALID > > > >         UCLASS_MASS_STORAGE > > > >         UCLASS_INVALID > > > >         UCLASS_MMC > > > >         UCLASS_INVALID > > > >         UCLASS_AHCI > > > >         UCLASS_ROOT > > > >         UCLASS_NVME > > > >         UCLASS_EFI > > > > Above nodes must contain the child node UCLASS_BLK(this node > > > > contains > > > > device number) > > > > > > > > No node required for UBI interface. > > > > > > > > I can create a new function, which is specific for getting the > > > > device > > > > number from the parent node(block device) which contains the > > > > child > > > > node > > > > UCLASS_BLK. May be that function can be put into fs.c. > > > > > > > > I'm not sure would this work as expected, any ideas/thoughts on > > > > my > > > > proposal? > > > Sounds right to me. > > > > > Is that possible one parent device(like MMC) contains multiple > > child > > devices in UCLASS_BLK? > No, only one is supported. > Noted. > Regards, > Simon