Hi Michal,

On Wed, Jul 4, 2012 at 7:43 AM, Michal Simek monstr@monstr.eu wrote:

Hi Simon,

I have create another thread because this can be discuss separately.

In board file there is this code (We discussed fdtcontroladdr separately).

#ifdef CONFIG_OF_EMBED /* Get a pointer to the FDT */ gd->fdt_blob = _binary_dt_dtb_start; #elif defined CONFIG_OF_SEPARATE /* FDT is at end of image */ gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE); #endif /* Allow the early environment to override the fdt address */ gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16, (uintptr_t)gd->fdt_blob);

Is there any reason to use CONFIG_ options here? Because what I think will be the best is to remove CONFIG_ options and create priorities and checking in at run time.

Priority should be:

1. variable
2. appended blob
3. compiled in blob

Because compiled in blob can be also used as safe blob for cases where fdtcontroladdr points to nor flash which is broken. For this case current u-boot will fail because fdt_prepare won't find out valid DTB.

Here is just skeleton to show you what design I would prefer.

int fdtdec_prepare_fdt(void *dtb) { if (((uintptr_t)gd->fdt_blob & 3) || fdt_check_header(dtb)) { return -1; } return 0; }

void * fdt_addr = (void *)getenv_ulong("fdtcontroladdr", 16, (uintptr_t)gd->fdt_blob); if(!fdt_prepare(fdt_addr)) { gd->fdt_blob = fdt_addr; } else { fdt_addr = (void *)(_end_ofs + _TEXT_BASE); if(!fdt_prepare(fdt_addr)) { gd->fdt_blob = fdt_addr;

This much is probably reasonable, although I am not 100% comfortable with fallbacks. We should perhaps provide at least some debug() stuff to indicate where the fdt came from.

    } else {
            fdt_addr = _binary_dt_dtb_start; // FIXME special section

for it

What if this doesn't exist? Are you saying that we should have a built-in fdt as a backup for when one is not supplied? I worry that this is going to be incorrect (e.g. it might be for default hardware) so would just create a hard-to-find bug.

Also what if _binary_dt_dtb_start doesn't exist?

            if(!fdt_prepare(fdt_addr)) {
                    gd->fdt_blob = fdt_addr;
            }
    }

}

Thanks, Michal

-- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian

Regards, Simon