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

[Andreas Färber]

Am 26.10.2010 um 20:53 schrieb 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.

I was rather thinking of phys_addr_t.

Andreas