Re: [RFC] FDT fix for 64 bit platforms

On Oct 15, 2011, at 9:33 AM, Jayachandran C. wrote:

> On Sat, Oct 15, 2011 at 8:43 PM, Nathan Whitehorn
> <nwhitehorn@freebsd.org> wrote:
>> On 10/15/11 01:12, Jayachandran C. wrote:
>>>=20
>>> On Sat, Oct 15, 2011 at 2:01 AM, Nathan Whitehorn
>>> <nwhitehorn@freebsd.org>  wrote:
>>>>=20
>>>> On 10/14/11 14:10, Jayachandran C. wrote:
>>>>>=20
>>>>> I'm planning commit this -CURRENT if there an no objections.
>>>>>=20
>>>>> 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.
>>>>>=20
>>>>> Review or testing from device tree users will be welcome.
>>>>>=20
>>>>> JC.
>>>>=20
>>>> 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.
>>>=20
>>> 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.
>>=20
>> 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.
>=20
> Well, the existing FDT code returns 0 as the invalid handle and I do
> not want to change that in this commit.
>=20
> 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?

--=20
Marcel Moolenaar
marcel@xcllnt.net

_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"