On 09/28/2017 05:14 PM, Chee, Tien Fong wrote:
On Rab, 2017-09-27 at 11:23 +0200, Marek Vasut wrote:
On 09/27/2017 11:13 AM, Chee, Tien Fong wrote:
On Sel, 2017-09-26 at 12:39 +0200, Marek Vasut wrote:
On 09/26/2017 11:52 AM, Chee, Tien Fong wrote:
On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote:
On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > > > > From: Tien Fong Chee tien.fong.chee@intel.com > > These drivers handle FPGA program operation from flash > loading > RBF to memory and then to program FPGA. > > Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
[...]
> > > +const char *get_cff_devpart(const void *fdt, int *len) > +{ > + const char *cff_devpart = NULL; > + const char *cell; > + int nodeoffset; > + nodeoffset = fdtdec_next_compatible(fdt, 0, > + COMPAT_ALTERA_SOCFPGA_FPGA0); > + > + cell = fdt_getprop(fdt, nodeoffset, > "bitstream_devpart", > len); > + > + if (cell) > + cff_devpart = cell; > + > + return cff_devpart; > +} Take a look at splash*.c , I believe that can be reworked into generic firmware loader , which you could then use here.
the devpart is hard coded in splash*.c. The function here is getting devpart info from DTS. So, is there any similar function in splash*.c? May be you can share more about your idea.
The generic loader could use some work of course ...
Sorry, i am still confusing. Allow me to ask you more:
- Is the generic firmware loader already exists in splash*.c?
No
- Are you talking about get_cff_devpart or whole fpga laodfs?
- You want me integrate get_cff_devpart function into splash*.c?
- Are you means to hard code the devpart instead providing dynamic
devpart described in DTS?
I am talking about factoring out generic firmware loader from splash*c , since it already contains most of the parts for such a thing.
Current implementation are located in spl_board_init func (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, nand and QSPI, then reading some info from DTS, setting dev and partition with generic fs functions, and reading with generic fs function before programming RBF into FPGA. All these are in patch 19.
That's what splash*c also does, so adding separate parallel implementation of the same functionality is a no-no.
After reading through splash*c, i found there are two functions bear a close similarity to. 1st function --> In /common/splash.c : static struct splash_location default_splash_locations[] = { { .name = "sf", .storage = SPLASH_STORAGE_SF, .flags = SPLASH_STORAGE_RAW, .offset = 0x0, }, { .name = "mmc_fs", .storage = SPLASH_STORAGE_MMC, .flags = SPLASH_STORAGE_FS, .devpart = "0:1", }, { .name = "usb_fs", .storage = SPLASH_STORAGE_USB, .flags = SPLASH_STORAGE_FS, .devpart = "0:1", }, { .name = "sata_fs", .storage = SPLASH_STORAGE_SATA, .flags = SPLASH_STORAGE_FS, .devpart = "0:1", }, };
In my /arch/arm/mach-socfpga/spl.c (spl_board_init(void)) bootdev.boot_device = spl_boot_device();
if (BOOT_DEVICE_MMC1 == bootdev.boot_device) { struct mmc *mmc = NULL; int err = 0;
spl_mmc_find_device(&mmc, bootdev.boot_device); err = mmc_init(mmc); if (err) {#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: mmc init failed with error: %d\n", err); #endif }
fpga_fsinfo.dev_part = (char *)get_cff_devpart(gd-fdt_blob,
&len); fpga_fsinfo.filename = (char *)get_cff_filename(gd-fdt_blob,
&len, PERIPH_RBF);
fpga_fsinfo.interface = "mmc"; fpga_fsinfo.fstype = FS_TYPE_FAT;} else { printf("Invalid boot device!\n"); return; }
/* Program peripheral RBF */ if (fpga_fsinfo.filename && fpga_fsinfo.dev_part && (len > 0)) rval = fpga_fsload(0, buffer, BSIZE, &fpga_fsinfo);
In /common/splash.c, dev_Part and flash type everything are hard coded in struct splash_location. In my spl.c, flash type are determined on run time and dev_part are retrived from DTS, and then assigned to struct fpga_fsinfo. Please note that this is in SPL, mmc need to be initialized 1st before loading raw file into memory. In SPL, raw file are coppied to OCRAM chunk by chunk, but In u-boot it would normally done in one big chunk to DRAM. This would be handled by fpga loadfs.
So, you want me hard code everthing like in splash.c?
No, I need you to play around with this and come up with generic firmware loader that's flexible enough.
2nd function --> In /common/splash_source.c static int splash_select_fs_dev(struct splash_location *location) { int res;
switch (location->storage) { case SPLASH_STORAGE_MMC: res = fs_set_blk_dev("mmc", location->devpart, FS_TYPE_ANY); break; case SPLASH_STORAGE_USB: res = fs_set_blk_dev("usb", location->devpart, FS_TYPE_ANY); break; case SPLASH_STORAGE_SATA: res = fs_set_blk_dev("sata", location->devpart, FS_TYPE_ANY); break; case SPLASH_STORAGE_NAND: if (location->ubivol != NULL) res = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS); else res = -ENODEV; break; default: printf("Error: unsupported location storage.\n"); return -ENODEV; }
if (res) printf("Error: could not access storage.\n");
return res; }
In my /drivers/fpga/socfpga_arria10.c static int flash_read(struct flash_info *flashinfo, u32 size_read, u32 *buffer_ptr) { size_t ret = EEXIST; loff_t actread = 0;
if (fs_set_blk_dev(flashinfo->interface, flashinfo->dev_part, flashinfo->fstype)) return FPGA_FAIL;
ret = fs_read(flashinfo->filename, (u32) buffer_ptr, flashinfo->flash_offset, size_read, &actread);
if (ret || actread != size_read) { printf("Failed to read %s from flash %d ", flashinfo->filename, ret); printf("!= %d.\n", size_read); return -EPERM; } else ret = actread;
return ret; }
Some attributes like flash type is determined on run time and and dev_part is retrieved from DTS, so every infos driver need to know are assinged into struct flashinfo and passed to fs_set_blk_dev as arguments. I found that function in splash_source.c some like flash type are getting from env variable, but we are still in SPL phase, those env variable is not set up yet. So, i think that is very ineffcient to factor out them as common.
If you want me create a generic firmware loader which is generic enough loading content for all components like flashes, FPGA, splash ....etc, i don't think that is effient enough, as fpga loadfs has different handling in both SPL and U-boot like copying raw into memory.
Duplicating code is nonsense and I believe generic firmware loader would be the way to go here.
It would be good you can direct point me out which functions have similirity and how to factor them out as common.
Thanks a lot.
Thanks.
[...]