Hi Mario, On 5 December 2016 at 10:32, Mario Six <mariosix1...@gmail.com> wrote: > Hi Simon, > > On Mon, Dec 5, 2016 at 7:24 AM, Simon Glass <s...@chromium.org> wrote: >> Hi, >> >> On 1 December 2016 at 01:39, Stefan Roese <s...@denx.de> wrote: >>> (Adding Simon and Maxim to Cc) >>> >>> On 23.11.2016 16:12, Mario Six wrote: >>>> >>>> Certain boards come in different variations by way of utilizing daughter >>>> boards, for example. These boards might contain additional chips, which >>>> are added to the main board's busses, e.g. I2C. >>>> >>>> The device tree support for such boards would either, quite naturally, >>>> employ the overlay mechanism to add such chips to the tree, or would use >>>> one large default device tree, and delete the devices that are actually >>>> not present. >>>> >>>> Regardless of approach, even on the U-Boot level, a modification of the >>>> device tree is a prerequisite to have such modular families of boards >>>> supported properly. >>>> >>>> Therefore, we add an option to make the U-Boot device tree (the actual >>>> copy later used by the driver model) writeable, and add a callback >>>> method that allows boards to modify the device tree at an early stage, >>>> at which, hopefully, also the application of device tree overlays will >>>> be possible. >>>> >>>> Signed-off-by: Mario Six <mario....@gdsys.cc> >>> >>> >>> I didn't follow DT overlay lately closely especially not in U-Boot. >>> Simon, Maxim could you please take a look at this patch and comment >>> on its necessity? >>> >>> >>>> --- >>>> common/board_f.c | 3 +++ >>>> dts/Kconfig | 10 ++++++++++ >>>> include/asm-generic/global_data.h | 4 ++++ >>>> include/common.h | 1 + >>>> 4 files changed, 18 insertions(+) >>>> >>>> diff --git a/common/board_f.c b/common/board_f.c >>>> index 4b74835..cda5aae 100644 >>>> --- a/common/board_f.c >>>> +++ b/common/board_f.c >>>> @@ -1034,6 +1034,9 @@ static init_fnc_t init_sequence_f[] = { >>>> #ifdef CONFIG_SYS_EXTBDINFO >>>> setup_board_extra, >>>> #endif >>>> +#ifdef CONFIG_OF_BOARD_FIXUP >>>> + board_fix_fdt, >>>> +#endif >> >> Can you add documentation for this somewhere? E.g. in the driver model >> readme? >> > > OK, I'll document the feature in v2. > >>>> INIT_FUNC_WATCHDOG_RESET >>>> reloc_fdt, >>>> setup_reloc, >>>> diff --git a/dts/Kconfig b/dts/Kconfig >>>> index 4b7d8b1..3f64eda 100644 >>>> --- a/dts/Kconfig >>>> +++ b/dts/Kconfig >>>> @@ -14,6 +14,16 @@ config OF_CONTROL >>>> This feature provides for run-time configuration of U-Boot >>>> via a flattened device tree. >>>> >>>> +config OF_BOARD_FIXUP >>>> + bool "Board-specific manipulation of Device Tree" >>>> + help >>>> + In certain circumstances it is necessary to be able to modify >>>> + U-Boot's device tree (e.g. to delete device from it). This >>>> option >>>> + make the Device Tree writeable and provides a board-specific >>>> + "board_fix_fdt" callback (called during pre-relocation time), >>>> which >>>> + enables the board initialization to modifiy the Device Tree. The >>>> + modified copy is subsequently used by U-Boot after relocation. >>>> + >>>> config SPL_OF_CONTROL >>>> bool "Enable run-time configuration via Device Tree in SPL" >>>> depends on SPL && OF_CONTROL >>>> diff --git a/include/asm-generic/global_data.h >>>> b/include/asm-generic/global_data.h >>>> index e02863d..f566186 100644 >>>> --- a/include/asm-generic/global_data.h >>>> +++ b/include/asm-generic/global_data.h >>>> @@ -69,7 +69,11 @@ typedef struct global_data { >>>> struct udevice *timer; /* Timer instance for Driver Model >>>> */ >>>> #endif >>>> >>>> +#ifdef CONFIG_OF_BOARD_FIXUP >>>> + void *fdt_blob; /* Our device tree, NULL if none >>>> */ >>>> +#else >>>> const void *fdt_blob; /* Our device tree, NULL if none >>>> */ >>>> +#endif >> >> Can we keep it as const and just use a cast when it needs to change? >> You could add a function which returns a writable pointer. >> > > Having the pointer globally writable has the advantage that you can get a > writable pointer wherever you need it and don't have to pass it around, but > separating the parts where the pointer is writable from the ones where it is > not is probably a good idea. Will fix in v2. > >> >>>> void *new_fdt; /* Relocated FDT */ >>>> unsigned long fdt_size; /* Space reserved for relocated >>>> FDT */ >>>> struct jt_funcs *jt; /* jump table */ >>>> diff --git a/include/common.h b/include/common.h >>>> index a8d833b..293ce23 100644 >>>> --- a/include/common.h >>>> +++ b/include/common.h >>>> @@ -502,6 +502,7 @@ extern ssize_t spi_write (uchar *, int, uchar *, int); >>>> >>>> /* $(BOARD)/$(BOARD).c */ >>>> int board_early_init_f (void); >>>> +int board_fix_fdt (void); >> >> Comment please. Perhaps it should pass a writable fdt pointer? >> > > I'll add a comment in v2. > >>>> int board_late_init (void); >>>> int board_postclk_init (void); /* after clocks/timebase, before >>>> env/serial */ >>>> int board_early_init_r (void); >> >> There have been discussions about moving to a live tree (hierarchical >> format) in U-Boot post-relocation. What do you think? It might make >> these changes easier or more efficient. >> > > Yes, that would definitely make things easier. The approach of modifying the > tree before the main U-Boot "sees it" that I'm taking here seems a bit hacky. > The other approach I've been experimenting with consisted of starting the > board > with a base device tree, which only has the devices in it that are guaranteed > to exists, gather all needed data to decide which overlays to apply, then tear > down the DM (using dm_uninit), make the modifications to the tree, and restart > the DM (with dm_init_and_scan). While this worked more or less fine, it seems > very wasteful, and a genuinely modifiable tree would be a much nicer solution. > > Also, we need a solution to this problem one way or another; the way our > hardware is constructed, we need to check which devices actually exist and > which do not by querying the hardware itself. And the most natural thing to do > here would be to have a modifiable device tree.
Creating a live tree is pretty easy - perhaps 500 lines of code. Trickier is to adjust the existing fdtdec API so that you can pass a node pointer instead of a blob and offset. I suppose we need to support the existing API for a while. I think ideally we should have a flat tree before relocation (to save time and space) and a live tree afterwards. So your approach of making the flip just before relocation seems right to me. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot