On 03/07/18 17:14, Rob Herring wrote:
> On Wed, Mar 7, 2018 at 5:59 PM, Frank Rowand <frowand.list@xxxxxxxxx> wrote:
>> On 03/07/18 14:21, Rob Herring wrote:
>>> On Wed, Mar 7, 2018 at 4:14 PM, Tyrel Datwyler
>>> <tyreld@xxxxxxxxxxxxxxxxxx> wrote:
>>>> On 03/07/2018 12:38 PM, Rob Herring wrote:
>>>>> On Fri, Mar 02, 2018 at 08:46:44PM -0500, Tyrel Datwyler wrote:
>>>>>> There are a couple places in drivers/of where the root node is found
>>>>>> with a of_find_node_by_path("/") call. This call just returns
>>>>>> of_node_get(of_root) when the the path "/" is passed as a parameter.
>>>>>> So, lets fixup these instances to just do that instead.
>>>>>>
>>>>>> Signed-off-by: Tyrel Datwyler <tyreld@xxxxxxxxxxxxxxxxxx>
>>>>>> ---
>>>>>> drivers/of/base.c | 2 +-
>>>>>> drivers/of/platform.c | 4 ++--
>>>>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>>>> index ad28de9..94c1b4d 100644
>>>>>> --- a/drivers/of/base.c
>>>>>> +++ b/drivers/of/base.c
>>>>>> @@ -455,7 +455,7 @@ int of_machine_is_compatible(const char *compat)
>>>>>> struct device_node *root;
>>>>>> int rc = 0;
>>>>>>
>>>>>> - root = of_find_node_by_path("/");
>>>>>> + root = of_node_get(of_root);
>>>>>
>>>>> The former seems more readable to me. I'll wait and see if Frank has any
>>>>> comments on this change.>>
>>>> Fair enough. I find them equally readable now that we have a well
>>>> named/defined global, namely "of_root", pointing at the device tree
>>>> root. My primary goal was saving a function call, but I wasn't sure
>>>> if anybody really cares enough. A quick check shows ~77 uses of
>>>> "of_find_node_by_path("/")" in the kernel.
>>
>> That is an interesting result.
>>
>>>> Figured I'd start here
>>>> and see what the consensus was.
>>>
>>> As far as outside drivers/of/ is concerned, we certainly don't want to
>>> expose global variables. But for most uses, you shouldn't need the
>>> root node pointer because most iterating or searching functions treat
>>> NULL as root.
>>>
>>> Rob
>>
>> For a simple answer, I'll go along with Rob here.
>>
>>
>> "Here, hold my beer and watch this!"
>>
>> Things one should never say. :-)
>>
>> Please don't run with this following idea yet. This could open a can
>> of worms that I've been trying to leave closed until I have some time
>> scheduled to handle the fall out.
>
> Now there are worms everywhere!
>
>> The core devicetree code was not written with the concept of overlays
>> in mind. When the first portions of the overlay code were accepted,
>> the architectural issues of the extending existing core code were not
>> first addressed. I don't even know if they were considered or
>> discussed because I was not involved at the time.
>>
>> Some key things that were not updated to account for overlays:
>>
>> 1) locking (protection of the live devicetree data structures)
>>
>> 2) pointers to live devicetree data structures being handed out
>> to kernel code outside the core devicetree code
>
> 3) the possibility of multiple independent roots not tied into any
> base DT. Think of USB FTDI type chips with things hanging off of
> UARTs, I2C, GPIO, etc.
>
>> One result of this is that after an overlay is applied, we can never
>> free the memory containing the overlay FDT and the overlay expanded
>> tree, because we do not know if any other part of the kernel has a
>> pointer into the overlay data.
>>
>> I haven't dug into this in any depth yet, but it seems like it should
>> be possible to create a new set of devicetree APIS, called by drivers
>> and other parts of the kernel that retrieve information from the
>> devicetree. This API can not return pointers into the core devicetree
>> data structures. Thus no node pointers, no property pointers, no
>> property value pointers.
>
> Or it could be just opaque pointers (aka handles). Or could be split
> into public and private structs/pointers. Returning raw property data
> and length will still be needed for example.
Yes, my naive assumption, without having thought about things very
hard, is that opaque objects are going to be involved.
For property data, I'm hoping that we can allocate memory to copy
the property into and return the copy to the caller. Then the
memory returned to the caller is owned by the caller and they
are responsible for freeing it when they are done with it.
>> We may find some easy solution for this, but I suspect not.
>>
>> That's a long winded way of saying that maybe we should not spend
>> effort on cleaning up non-critical issues with the way the
>> current API is being used, _if_ the current API is going to
>> be replaced long term.
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
