[OpenBIOS] [PATCH 3/3] Don't assume that pointer and cell size are identical

[Blue Swirl]

On Mon, Oct 25, 2010 at 9:08 PM, Andreas Färber <andreas.faerber at web.de> wrote:
> Am 20.10.2010 um 21:04 schrieb Blue Swirl:
>
>> On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber <andreas.faerber at web.de>
>> wrote:
>>>
>>> Am 20.10.2010 um 18:19 schrieb Blue Swirl:
>>>
>>>> On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber
>>>> <andreas.faerber at web.de>
>>>> wrote:
>>>>>
>>>>> On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct
>>>>> casts
>>>>> result in warnings, treated as errors.
>>>>>
>>>>> Use [u]intptr_t cast or cell2pointer and pointer2cell macros as
>>>>> necessary.
>>>>
>>>>> -int uart_init(uint64_t port, unsigned long speed)
>>>>> +int uart_init(uintptr_t port, unsigned long speed)
>>>>
>>>> This is not correct. The physical addresses on Sparc32 are really 36
>>>> bits wide, so uintptr_t is not wide enough.
>>>
>>> Okay, then I'll revert this one. The patch may be mechanical to _review_
>>> but
>>> it's pretty invasive, so I'd rather have some parts of it committed
>>> before
>>> running into larger merge conflicts.
>>>
>>>> For that, we'd need a
>>>> physical address type. We could reuse for example QEMU name,
>>>> target_phys_addr_t.
>>>
>>> Sounds good. Should we go ahead with my patch first and do the uint64_t
>>> ->
>>> target_phys_addr_t change as a follow-up patch? Or should I strip all
>>> changes to drivers/ from my patch?
>>
>> The cell2pointer etc. changes look OK.
>
> Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer()
> fix in mac-parts.c that I forgot to mention.
>
> Attached is what I have not applied from the original patch. I'm thinking we
> should better avoid the native-bitwidth-equals-host-bitwidth path on ppc64
> than fixing the symptoms of using it. For the rest, target_phys_addr_t would
> seem a nice solution. Additionally, the double-dereference of "myself" and
> the trampoline issue still need proper fixes.

In some cases, instead of casts, the structure fields (in adb_kbd.c:
dev->state) should be widened to uintptr_t or pointer.