Re: [U-Boot] [RFC 09/11] armv7R: dts: k3: am654: Update for loading SYSFW from MMC

2019-05-21 Thread dannenb...@ti.com
Hi TF,

On Tue, May 21, 2019 at 05:40:19AM +, Chee, Tien Fong wrote:
> > +   fs_loader0: fs_loader@0 {
> > +   u-boot,dm-pre-reloc;
> > +   compatible = "u-boot,fs-loader";
> 
> Why not using phandlepart = < 1>, this would help to avoid mmc init
> duplication in a few places such as patch [05/11].
> 

See comment on earlier patch, this path was chosen to allow for
flexibility during runtime. When using FS loader as you intended it to
get used it works very well (we are actually using it for other purposes
on another device).

Hence my argument that the approach I chose with the original [1] SYSFW
loader series of opening up the SPL loader framework in a "lean & mean"
fashion and the FS loader driver as it is today are really things that
are complementary and can co-exist when one starts looking under the hood.

--
Andreas Dannenberg
Texas Instruments Inc

[1] https://lists.denx.de/pipermail/u-boot/2019-May/thread.html#368461

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [RFC 05/11] arm: K3: Introduce System Firmware loader framework

2019-05-21 Thread dannenb...@ti.com
Hi TF,

On Tue, May 21, 2019 at 05:21:15AM +, Chee, Tien Fong wrote:
> On Thu, 2019-05-16 at 15:54 -0500, Andreas Dannenberg wrote:
> > +   if (mmc_dev < 0) {
> > +   pr_err("%s: Getting MMC device index failed (%d)\n",
> > __func__,
> > +      mmc_dev);
> > +   return mmc_dev;
> > +   }
> > +
> > +   ret = uclass_get_device(UCLASS_MMC, mmc_dev, );
> > +   if (ret < 0) {
> > +   pr_err("%s: Getting MMC device failed (%d)\n",
> > __func__, ret);
> > +   return ret;
> > +   }
> > +
> > +   ret = mmc_initialize(NULL);
> > +   if (ret < 0) {
> > +   pr_err("%s: Initializing MMC device failed (%d)\n",
> > __func__,
> > +      ret);
> > +   return ret;
> > +   }
> > +
> > +   mmc = mmc_get_mmc_dev(mmcdev);
> > +   if (!mmc) {
> > +   pr_err("%s: Getting underlying MMC device failed\n",
> > __func__);
> > +   return -ENODEV;
> > +   }
> > +
> > +   ret = mmc_init(mmc);
> > +   if (ret) {
> > +   printf("%s: mmc init failed with error: %d\n",
> > __func__, ret);
> > +   return ret;
> > +   }
> > +
> 
> The probe function in fs loader already support the block init, and the
> block init would init the mmc.

Yes I'm sure this could have gotten simplified if I were to use the DTS
to describe the boot device. But as explained earlier I was exploring a
way to make it work without DTS for added flexibility so I can pick the
boot device at runtime.

--
Andreas Dannenberg
Texas Instruments Inc
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [RFC 04/11] misc: fs_loader: Allow initializing blkdev using platform data

2019-05-21 Thread dannenb...@ti.com
Hi TF,
first, thanks for taking time to continue having the discussion around
this patch series.

On Tue, May 21, 2019 at 05:40:59AM +, Chee, Tien Fong wrote:
> On Thu, 2019-05-16 at 15:54 -0500, Andreas Dannenberg wrote:
> > To give us more flexibility using the FS loader eliminate the need of
> > always having to use the ENV to configure the block device but rather
> > allow the respective block device and partition to be setup through
> > platform data.
> > 
> > Signed-off-by: Andreas Dannenberg 
> 
> Why not using DT method?

We would like to use a single U-Boot build to support all of the
different boot modes (boot media) a given device supports. In case of
this RFC series this is really distilled down to only mmc0 and mmc1 but
this already illustrates the point. The patch 05/11 shows how
spl_boot_device() is used to make the MMC probing dependent on the boot
device mmc0 vs mmc1. So hard-coding in DT will lose this flexibility. Of
course there would be ways out such as dynamically updating DTS or to
extend the FS loader probe function to select from multiple DTS nodes
based on boot device, but all would come at a cost of additional memory
consumption.


--
Andreas Dannenberg
Texas Instruments Inc
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 00/13] System Firmware Loader for TI K3 family SoCs

2019-05-15 Thread dannenb...@ti.com
Hi TF,

On Mon, May 13, 2019 at 01:37:22PM +, Chee, Tien Fong wrote:
> On Wed, 2019-05-08 at 13:43 -0500, dannenb...@ti.com wrote:
> > Hi TF,
> > thanks for chiming in. Comments inlined...
> > 
> > On Wed, May 08, 2019 at 04:31:35AM +, Chee, Tien Fong wrote:
> > > 
> > > On Tue, 2019-05-07 at 22:00 +0200, Simon Goldschmidt wrote:
> > > > 
> > > > 
> > > > On 07.05.19 19:25, Andreas Dannenberg wrote:
> > > > > 
> > > > > 
> > > [...]
> > > > 
> > > > > 
> > > > > 
> > > > > While I also have a working solution based on the existing FS
> > > > > loader
> > > > > framework this has its own challenges, namely by its very
> > > > > nature
> > > > > only
> > > > > addressing a subset of our use cases (no eMMC/SD RAW boot
> > > > > support
> > > > > for
> > > > > example), 
> > > IMO, it's actually not that hard to enhance RAW support, i think
> > > minimal changes are required. I have attached the patches about an
> > > example of loading RAW from QSPI that i have done locally last few
> > > week
> > > ago.
> > As your patches show, no it's not hard, it's more or less taking
> > pieces
> > from the SPL loader framework and refactoring them into the FS
> > loader,
> > creating a good and universal solution usable across SPL and U-Boot
> > in
> > environments that are not tightly constrained in terms of memory.
> > 
> > What I was going after is finding a way to load from different media
> > "pre-relocation" SPL (board_init_f), with almost no memory available,
> > where I have to agonize over every single KB available.
> 
> This is just a simple "loader", provide user flexibility of loading
> stuff in anywhere, from SPL to U-Boot. As long as DM is supported by
> the time running at "pre-relocation" SPL, then FS loader should be able
> to work.

Understood. FS loader also relies on the peripheral being brought up.
For MMC, from my SPL board_init_f() user code I had to do a sequence of
mmc_initialize(), mmc_get_mmc_dev(), and mmc_init() before I could use
the FS loader functionality - just like the U-Boot SPL loader is already
doing this. Hence my concern with code duplication for certain use
cases. Anyways I'll post my use of the FS loader as an RFC here soon
so you can have a look at the whole thing if you like.

> > > > > being heavier on resource usage (needing to use ENV to pass
> > > > > parameters),
> > > ENV is optional, you can use DTS.
> > Is it? I had to update the FS loader framework when I experimented
> > with
> > it, please see attached patch. I had refactored it such that I can
> > pass
> > in all relevant data via platform data for the intial boot mode I was
> > going after, so that I can dynamically configure it on the fly from
> > early SPL board_init_f() based on boot media / boot mode, etc.
> 
> Yes, you can tie up loader with target HW node for destination loading.
> For example, tie up with FPGA manager node, loading bistream file from
> MMC to FPGA manager.
> 
> Here is an example, but i put the fs loader phandle under chosen node
> because most files and images are stored in the same storage.
> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=0a42a132a4b8
> 46031df2c4a7d04692240ed34843

Thanks for pointing to this example for DTS-based loading, I think I had
seen this earlier when I looked for implementations using the FS loader
framework.

> > > 
> > > For example loading FPGA bitstream from QSPI RAW:
> > > 
> > > /* DTS */
> > > / {
> > > + aliases {
> > > + spi0 = 
> > > + };
> > > +
> > > + fs_loader0: fs-loader {
> > > + u-boot,dm-pre-reloc;
> > > + compatible = "u-boot,fs-loader";
> > > + sfconfig = <0 0 1 3>;
> > > + };
> > > +};
> > > +
> > > +_mgr {
> > > + u-boot,dm-pre-reloc;
> > > + firmware-loader = <_loader0>;
> > > + altr,bitstream = "30";
> > > +};
> > The above hard-codes and duplicates information that is already known
> > to
> > U-Boot (CONFIG_SF_DEFAULT_*), and will do more of the same as this is
> > being extended. How does one keep this consistent?
> 
> Current fs loader not support RAW loading yet, i'm not sure whether it
> should support it by adding more specific storage API(much more messy),
>

Re: [U-Boot] [PATCH 00/13] System Firmware Loader for TI K3 family SoCs

2019-05-08 Thread dannenb...@ti.com
Hi TF,
thanks for chiming in. Comments inlined...

On Wed, May 08, 2019 at 04:31:35AM +, Chee, Tien Fong wrote:
> On Tue, 2019-05-07 at 22:00 +0200, Simon Goldschmidt wrote:
> > 
> > On 07.05.19 19:25, Andreas Dannenberg wrote:
> > > 
> [...]
> > > 
> > > While I also have a working solution based on the existing FS
> > > loader
> > > framework this has its own challenges, namely by its very nature
> > > only
> > > addressing a subset of our use cases (no eMMC/SD RAW boot support
> > > for
> > > example), 
> 
> IMO, it's actually not that hard to enhance RAW support, i think
> minimal changes are required. I have attached the patches about an
> example of loading RAW from QSPI that i have done locally last few week
> ago.

As your patches show, no it's not hard, it's more or less taking pieces
from the SPL loader framework and refactoring them into the FS loader,
creating a good and universal solution usable across SPL and U-Boot in
environments that are not tightly constrained in terms of memory.

What I was going after is finding a way to load from different media
"pre-relocation" SPL (board_init_f), with almost no memory available,
where I have to agonize over every single KB available.

> > > being heavier on resource usage (needing to use ENV to pass
> > > parameters),
> 
> ENV is optional, you can use DTS.

Is it? I had to update the FS loader framework when I experimented with
it, please see attached patch. I had refactored it such that I can pass
in all relevant data via platform data for the intial boot mode I was
going after, so that I can dynamically configure it on the fly from
early SPL board_init_f() based on boot media / boot mode, etc.

> For example loading FPGA bitstream from QSPI RAW:
> 
> /* DTS */
> / {
> + aliases {
> + spi0 = 
> + };
> +
> + fs_loader0: fs-loader {
> + u-boot,dm-pre-reloc;
> + compatible = "u-boot,fs-loader";
> + sfconfig = <0 0 1 3>;
> + };
> +};
> +
> +_mgr {
> + u-boot,dm-pre-reloc;
> + firmware-loader = <_loader0>;
> + altr,bitstream = "30";
> +};

The above hard-codes and duplicates information that is already known to
U-Boot (CONFIG_SF_DEFAULT_*), and will do more of the same as this is
being extended. How does one keep this consistent?

And how does this scale to support like 5 different boot modes using a
single DTB? I guess one  could populate 5 nodes, and then pick one based
on boot mode during SPL execution, by extending the probe function
accordingly.

> 
> > > and not addressing the need to probe the boot peripheral.
> 
> You can add more different probing method in function called
> "fs_loader_probe". Current fs_loader supports block(sdmmc, emmc, etc...) 
> probing, and with
> the patches attached support QSPI probing.
> 
> Another idea come to mind, we can use fs_loader for loading FIT boot
> image into RAM, and boot from RAM with existing SPL loader framework,
> but i'm not sure this implementation fit to your use case?

Unfortunately for what I'm working on there is no space for this. It
would be nice to be able to load our "System Firmware" alongside the
next stage of U-Boot in a single FIT, and then just extact that firmware
image similar to what CONFIG_SPL_FPGA_SUPPORT does in spl_fit.c. 
However I must load SYSFW and the U-Boot next stage as two steps
(while bringing up DDR in-between).

As I tried to explain in my earlier email to Simon, I almost see the
minimally-intrusive extensions I did to the SPL loader framework via
"spl: Make image loader infrastructure more universal" and the FS loader
framework to be elements that are orthogonal and can both exist. Maybe
FS loader can partially build upon the SPL loader foundation as well?

--
Andreas Dannenberg
Texas Instruments Inc


> 
> > > This particular framework works well for use cases requiring to
> > > load
> > > firmware from FS-based media once DDR is up and U-Boot is in a more
> > > "initialized" state but it is not a one-fits all solution for very
> > > early use in SPL board_init_f() accross different boot modes.
> > And would it be an option to improve the loader (maybe dropping the
> > "fs" 
> > from its name)? I think it's an "fs" loader because its idea has
> > been 
> > copied from Linux. I think in U-Boot, it's more common to have things
> > at 
> > a raw offset instead of a file system. Just thinking...
> 
> Current fs_loader only support filesystem, and i agree that it made
> sense to remove the "fs" once it supports the RAW offset as well.
> 
> Thanks.
> 
> Regards,
> TF
> 
> > 
> > And the current state of that fs_loader is like it is because it
> > fits 
> > its single user (socfpga stratix 10), I think.
> > 
> > Anyway, even if you do need yet another loader, would it make sense
> > to 
> > create a common file instead of adding this in your arch/mach?
> > 
> > Regards,
> > Simon
> > 
> > > 
> > > 
> > > Andreas Dannenberg (10):
> > >    mmc: k3_arasan: Allow driver to probe 

Re: [U-Boot] [PATCH] verified-boot: Minimal support for booting U-Boot proper from SPL

2016-06-09 Thread dannenb...@ti.com
On Thu, Jun 09, 2016 at 09:36:18AM -0700, Teddy Reed wrote:
> On Wed, Jun 8, 2016 at 11:45 PM, Sumit Garg <sumit.g...@nxp.com> wrote:
> > Hi Teddy,
> >
> > Can you please rebase this patch on upstream?
> 
> Sure! If there are any changes needed I can send a new patch version
> by EOD and will CC you.

Hi Teddy,
Yes it needs rebasing, it doesn't apply cleanly anymore to the latest
master. Otherwise it looks pretty good. I still wish we could get by
without CONFIG_SPL_DM but let's take one step at a time ;)

Acked-by: Andreas Dannenberg <dannenb...@ti.com>

Thanks,
Andreas
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot