lkml.org 
[lkml]   [2018]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] of: Kconfig: OF_OVERLAY, select OF_EARLY_FLATTREE
From
Date
On 02/28/18 12:26, Rob Herring wrote:
> Sorry for the delay, been away for a few days.
>
> On Mon, Feb 19, 2018 at 12:24 AM, Frank Rowand <frowand.list@gmail.com> wrote:
>>
>> On 02/18/18 17:46, Rob Herring wrote:
>>> On Sun, Feb 18, 2018 at 6:29 PM, <frowand.list@gmail.com> wrote:
>>>> From: Frank Rowand <frank.rowand@sony.com>
>>>>
>>>> kbuild test robot reported a new warning for a recent patch:
>>>>>> drivers/of/overlay.c:832:2: error: implicit declaration of function 'of_fdt_unflatten_tree' [-Werror=implicit-function-declaration]
>>>> of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
>>>>
>>>> The cause is that the prototype for of_fdt_unflatten_tree() in
>>>> include/linux/of_fdt.c is guarded by OF_EARLY_FLATTREE.
>
> Umm, this is not true. of_fdt_unflatten_tree is outside the ifdef for
> OF_EARLY_FLATTREE.

Brown paper bag. :-( As you note below, CONFIG_OF_FLATTREE is the
relevant option.


>>>>
>>>> This was a pre-existing problem for any overlay related caller of
>>>> of_fdt_unflatten_device_tree(), who was then going to pass the
>>>> unflattened tree to of_overlay_apply(). After the patch that triggered
>>>> this warning, all other overlay callers of of_fdt_unflatten_device_tree()
>>>> no longer exist, so adding the select to OF_OVERLAY is a sufficient fix.
>>>>
>>>> To reproduce the warning:
>>>> Use the .config attached to https://lkml.org/lkml/2018/2/17/268
>>>> make ARCH=i386 olddefconfig
>>>> make ARCH=i386 CC=gcc-7 drivers/of/overlay.o
>>>>
>>>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>>>> ---
>>>> drivers/of/Kconfig | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>>> index 783e0870bd22..00a6abfaaec7 100644
>>>> --- a/drivers/of/Kconfig
>>>> +++ b/drivers/of/Kconfig
>>>> @@ -92,6 +92,7 @@ config OF_RESOLVE
>>>> config OF_OVERLAY
>>>> bool "Device Tree overlays"
>>>> select OF_DYNAMIC
>>>> + select OF_EARLY_FLATTREE
>
> This should really have been OF_FLATTREE...

Yes. This only fixed the warning because OF_EARLY_FLATTREE selects
OF_FLATTREE.


>>> If we do this, we might as well kill OF_EARLY_FLATTREE. What platform
>>> really boots from not FDT, but uses DT without overlays?
>>
>> Making sure I'm understanding... So you want to remove OF_EARLY_FLATTREE
>> and convert the current users of it to OF_FLATTREE?
>
> Right. OF_EARLY_FLATTREE just controls everything that's in __init
> section anyway. So the question is it worth a kconfig option to
> control code that's in the __init section and will get discarded
> anyway?
>
>
>> I don't see any way to directly configure OF_FLATTREE and I don't see anyk
>> Kconfig file selecting it, other than drivers/of/Kconfig which selects
>> OF_FLATTREE from OF_EARLY_FLATTREE. So as far as I can tell, the two
>> config options are essentially a single config option. Meaning that
>> either one could be replaced by the other.
>>
>> Changing all to OF_FLATTREE will touch more files and thus will be a
>> bit more obtrusive. It looks like it would take two releases to avoid
>> a flag day change.
>>
>> Changing all to OF_EARLY_FLATTREE can be done in a single release.
>
> I would get rid of OF_EARLY_FLATREE within the DT core code making it
> do nothing more than select OF_FLATTREE. Then the arches can later
> just select OF_FLATTREE directly.

OK, will do.


> But this is all orthogonal to your issue. I think we need to keep the
> overlay dependencies on FDT separate. In theory we could support
> non-FDT sources or some more complex use that needs to unflatten FDT
> themselves. I don't really want to add another user visible kconfig
> option (OF_FDT_OVERLAY), so I think as long as we structure things to
> support that when needed we're fine.

OK, at the "theoretical, what if, keep our options open" level.

I'll add the desire to allow future support of non-FDT sources to
my general overlay requirements as I propose changes. So far, my
proposed patches isolate the FDT source dependency in a single small
function, so it seems all is OK from that angle. But all use of overlays
for many previous releases requires manually selecting OF_FLATTREE, given
that FDTs have been the overlay source. So it seems to make sense for
OF_OVERLAY to select OF_FLATTREE (not OF_EARLY_FLATTREE as I mistakenly
chose) now. And if future non-FDT overlays are implemented in the
future, re-visit how to select OF_FLATTREE in a more granular manner.

\
 
 \ /
  Last update: 2018-02-28 22:46    [W:2.080 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site