On Thu, 2018-09-20 at 21:42 -0700, Chee, Tien Fong wrote:
> >
If everybody agree with current framework, then the next version i will
include the fixes:
1. Adding DM_FLAG_PRE_RELOC, this would allow runtime to choose the
right fs_loader if the chosen node u-boot, fs-loader is not defined.
2. Let driver model handles all memory allocation
3. Using local variable instead of messy casts.
Thanks.
> On Tue, 2018-07-31 at 08:22 +0200, Michal Simek wrote:
> >
> > On 30.7.2018 18:05, Simon Glass wrote:
> > >
> > >
> > > Hi Michal,
> > >
> > > On 30 July 2018 at 07:30, Michal Simek <michal.si...@xilinx.com>
> > > wrote:
> > > >
> > > >
> > > > On 30.7.2018 15:26, Simon Glass wrote:
> > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > On 27 July 2018 at 02:40, Chee, Tien Fong <tien.fong.chee@int
> > > > > el
> > > > > .com> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Thu, 2018-07-26 at 11:03 +0200, Michal Simek wrote:
> > > > > > >
> > > > > > >
> > > > > > > On 25.7.2018 18:03, Tom Rini wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On Wed, Jul 25, 2018 at 09:47:17AM -0600, Simon Glass
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > On 25 July 2018 at 03:48, Michal Simek <michal.simek@
> > > > > > > > > xi
> > > > > > > > > linx.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On 25.7.2018 08:31, Chee, Tien Fong wrote:
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Wed, 2018-07-18 at 16:48 +0200, Michal Simek
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > On 6.7.2018 10:28, tien.fong.c...@intel.com
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.c...@intel.co
> > > > > > > > > > > > > m>
> > > > > > > > > > > > >
> > > > > > > > > [...]
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Also that DT binding is quite weird and I don't
> > > > > > > > > > > > think you
> > > > > > > > > > > > will get
> > > > > > > > > > > > ACK
> > > > > > > > > > > > for this from device tree community at all. I
> > > > > > > > > > > > think that
> > > > > > > > > > > > calling via
> > > > > > > > > > > > platdata and avoid DT nodes would be better way
> > > > > > > > > > > > to go.
> > > > > > > > > > > Why do you think DT binding is weird? The DT is
> > > > > > > > > > > designed
> > > > > > > > > > > based on Simon
> > > > > > > > > > > proposal, and i believe following the rules in
> > > > > > > > > > > DTS
> > > > > > > > > > > spec.
> > > > > > > > > > > There are some DT benefits with current design, i
> > > > > > > > > > > think
> > > > > > > > > > > someone may be
> > > > > > > > > > > maintainer need to made the final decision on the
> > > > > > > > > > > design.
> > > > > > > > > > It is software configuration in file which should
> > > > > > > > > > mostly
> > > > > > > > > > describe
> > > > > > > > > > hardware and state for hardware configuration.
> > > > > > > > > >
> > > > > > > > > > Your fs_loader node is purely describe sw
> > > > > > > > > > configuration which
> > > > > > > > > > shouldn't
> > > > > > > > > > be here.
> > > > > > > > > > You have there run time configuration via
> > > > > > > > > > variables.
> > > > > > > > > > I think
> > > > > > > > > > using only
> > > > > > > > > > this way is enough. Default variables will match
> > > > > > > > > > what
> > > > > > > > > > you would
> > > > > > > > > > want to
> > > > > > > > > > add to DT.
> > > > > > > > > I think DT makes sense in the U-Boot context.
> > > > > > > > >
> > > > > > > > > We don't have a user space to handle policy
> > > > > > > > > decisions,
> > > > > > > > > and the
> > > > > > > > > 'chosen' node is a good place to configure these
> > > > > > > > > common
> > > > > > > > > features.
> > > > > > > > >
> > > > > > > > > While you can argue that the partition or filesystem
> > > > > > > > > where an
> > > > > > > > > image
> > > > > > > > > comes from is a software config, it is something that
> > > > > > > > > has to be
> > > > > > > > > configured. It has impact on hardware too, since the
> > > > > > > > > FPGA has to
> > > > > > > > > get
> > > > > > > > > its firmware from somewhere. We use the chosen node
> > > > > > > > > to
> > > > > > > > > specify
> > > > > > > > > the
> > > > > > > > > UART to use, and this is no different. Again, we
> > > > > > > > > don't
> > > > > > > > > have user-
> > > > > > > > > space
> > > > > > > > > config files in U-Boot.
> > > > > > > > >
> > > > > > > > > This argument comes up from time to time and I'd
> > > > > > > > > really
> > > > > > > > > like to
> > > > > > > > > put it
> > > > > > > > > to bed for U-Boot. I understand that Linux has its
> > > > > > > > > own
> > > > > > > > > approach
> > > > > > > > > and
> > > > > > > > > rules, but in some cases they serve U-Boot poorly.
> > > > > > > > I want to second this as well. So long as we're using
> > > > > > > > our prefix
> > > > > > > > and
> > > > > > > > we've thought through and discussed what we're trying
> > > > > > > > to
> > > > > > > > do here,
> > > > > > > > it's
> > > > > > > > OK to do things that might not be accepted for Linux.
> > > > > > > >
> > > > > > > I have not a problem with using chosen node with u-boot
> > > > > > > prefix
> > > > > > > properties and my colleague hopefully with finish work
> > > > > > > about moving
> > > > > > > u-boot,dm-pre-reloc; to chosen node where it should be
> > > > > > > (because
> > > > > > > current
> > > > > > > solution has also problem with ordering).
> > > > > > >
> > > > > > > In this loader case doc is saying that you can rewrite it
> > > > > > > with
> > > > > > > variables
> > > > > > > on the prompt (or via script).
> > > > > > > For cases that you want to autodetect platform and
> > > > > > > pass/load correct
> > > > > > > dtb
> > > > > > > which setup u-boot this can be problematic and using DT
> > > > > > > is
> > > > > > > could be
> > > > > > > considered as easier for use.
> > > > > > >
> > > > > > > In this case this is what was proposed:
> > > > > > >
> > > > > > > + fs_loader0: fs-loader@0 {
> > > > > > > + u-boot,dm-pre-reloc;
> > > > > > > + compatible = "u-boot,fs-loader";
> > > > > > > + phandlepart = <&mmc 1>;
> > > > > > > + };
> > > > > > >
> > > > > > > + fs_loader1: fs-loader@1 {
> > > > > > > + u-boot,dm-pre-reloc;
> > > > > > > + compatible = "u-boot,fs-loader";
> > > > > > > + mtdpart = "UBI",
> > > > > > > + ubivol = "ubi0";
> > > > > > > + };
> > > > > > >
> > > > > > > u-boot,dm-pre-reloc; requires DM_FLAG_PRE_RELOC which is
> > > > > > > not setup
> > > > > > > for
> > > > > > > this driver - it means this should be here.
> > > > > > You are right, i missed this one. The intention of design
> > > > > > enables user
> > > > > > to call any loader with default storage through the
> > > > > > sequence
> > > > > > number if
> > > > > > fs loader is not defined in chosen. For example, there is a
> > > > > > case where
> > > > > > system loading the file from SDMMC, NAND and QSPI.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > compatible = "u-boot,fs-loader"; - bind and probe are
> > > > > > > empty
> > > > > > > that's
> > > > > > > why
> > > > > > > this is only used for filling platdata but driver has no
> > > > > > > user that's
> > > > > > > why
> > > > > > > this is unused till someone calls that functions.
> > > > > > >
> > > > > > > phandlepart/mtdpart/ubivol is just for setup.
> > > > > > There are some benefits with driver model:
> > > > > > 1. Saving space, calling when need.
> > > > > > 2. Handle memory allocation and deallocation automatically.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > For the first case you can just use in chosen node:
> > > > > > > u-boot,fs-loader = <&mmc 1>;
> > > > > > >
> > > > > > > And for UBIfs. I have never played with that but I expect
> > > > > > > it
> > > > > > > shouldn't
> > > > > > > be big problem to describe it differently too (something
> > > > > > > like)
> > > > > > > u-boot,fs-loader = <0 ubi0>;
> > > > > > Need consider description for UBIFS, using fs-loader seems
> > > > > > not working
> > > > > > for UBIFS, since more arguments such as mtdpartition and
> > > > > > mtd
> > > > > > volume
> > > > > > need passing into driver. In order to avoid messing,
> > > > > > fs_loader can act
> > > > > > the pointer to the chosen.
> > > > > >
> > > > > > Anyway, i have no strong opinion with driver designed via
> > > > > > platdata or
> > > > > > driver model if we can resolve the problem for UBIFS and
> > > > > > maintainers
> > > > > > agree with it.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Then this driver/interface can stay in DT where it should
> > > > > > > stay. The
> > > > > > > only
> > > > > > > thing is how this should be initialized because there is
> > > > > > > no
> > > > > > > compatible
> > > > > > > string. But you can do that via platdata for platforms
> > > > > > > which want to
> > > > > > > use
> > > > > > > this.
> > > > > We should add a compatible string then :-)
> > > > Isn't driver name used in case of platdata initialization?
> > > If the node is in /chosen and has a compatible string, it will be
> > > bound automatically. Manually binding a device is really just a
> > > fallback for particular situations (e.g. buses like PCI where we
> > > often
> > > rely on probing to find out what is on the bus).
> > up2you guys. I just have different opinion how this should be done.
> >
> > Thanks,
> > Michal
> Sorry for late reply, because rushing the project.
> So, are you guys OK with current implementation because we have no
> solution for the UBIFS setting with other solution than using DT?
> >
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
