[RFC] FDT fix for 64 bit platforms
Rafal Jaworowski
raj at semihalf.com
Sat Oct 15 17:12:16 UTC 2011
On 2011-10-15, at 18:48, Marcel Moolenaar wrote:
>
> On Oct 15, 2011, at 9:33 AM, Jayachandran C. wrote:
>
>> On Sat, Oct 15, 2011 at 8:43 PM, Nathan Whitehorn
>> <nwhitehorn at freebsd.org> wrote:
>>> On 10/15/11 01:12, Jayachandran C. wrote:
>>>>
>>>> On Sat, Oct 15, 2011 at 2:01 AM, Nathan Whitehorn
>>>> <nwhitehorn at freebsd.org> wrote:
>>>>>
>>>>> On 10/14/11 14:10, Jayachandran C. wrote:
>>>>>>
>>>>>> I'm planning commit this -CURRENT if there an no objections.
>>>>>>
>>>>>> In the current implementation, phandle is used to store a pointer to
>>>>>> the location inside the device tree. Since phandle_t is u32, this
>>>>>> will not work on 64 bit platforms. With this fix, the phandle is the
>>>>>> offset from the start of device tree pointer 'fdtp', which will be 32
>>>>>> bit.
>>>>>>
>>>>>> Review or testing from device tree users will be welcome.
>>>>>>
>>>>>> JC.
>>>>>
>>>>> Why not use offsets into the FDT rather than full pointers? I believe
>>>>> having
>>>>> phandles greater than 32 bits violates the FDT spec, and declaring that
>>>>> the
>>>>> FDT can't itself be larger than 4 GB seems reasonable.
>>>>
>>>> I am actually using the offset from the beginning of FDT (fdtp) as
>>>> phandle. I cannot use the usual fdt offset (after off_dt_struct) as
>>>> phandle, because in that case offset of 0 is valid, but phandle 0
>>>> should not be valid.
>>>
>>> Why shouldn't phandle 0 be valid? The invalid phandle is -1. This is one of
>>> the problems with our existing FDT code -- it makes all kinds of wrong
>>> assumptions like this about IEEE 1275.
>>
>> Well, the existing FDT code returns 0 as the invalid handle and I do
>> not want to change that in this commit.
>>
>> If the return value is really wrong, we will need a bigger exercise to
>> change the return value and fix any callers which are affected by that
>> change.
>
> It should be fairly easy to change the base from fdtp to the "usual"
> fdt offset, so let me propose the following:
>
> 1. JC commits what he has and based on the current code.
> 2. We get all the facts on the table. I say this because I
> read different and contradictory things (0 being an
> invalid phandle in OF, negative phandles exist, etc).
> 3. We change the implementation, if such is warranted, in
> a separate effort.
>
> The point really is that 0 is an invalid phandle right now,
> right or wrong, and JCs changes are based on that. I see no
> problem proceeding on the path we're on, while we discuss
> what's the correct implementation and whether or not we
> should have a course change...
>
> Thoughts?
The patch looks fine to me, but we didn't have a chance yet to test it on any PPC/ARM system, have you, Marcel? Regarding the phandle validity I need to recall the context as this was a while back and I don't quite remember all constraints and motivations.
Rafal
More information about the freebsd-current
mailing list