Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage

[Mitsyanko Igor <i.mitsyanko@samsung.com>]

On 02/22/2012 04:48 PM, andrzej zaborowski wrote:
> On 22 February 2012 13:26, Mitsyanko Igor<i.mitsyanko@samsung.com>  wrote:
>> On 02/22/2012 03:36 PM, andrzej zaborowski wrote:
>>> Why's uint32_t more correct though?  The purpose of using a named type
>>> across qemu is to mark fields as memory addresses (similar to size_t
>>> being used for sizes, etc.), uint32_t conveys less information -- only
>>> the size.
>>
>> It's obviously more informative, but I thought it's main purpose is to be
>> used with code that could be executed for a different targets (with
>> different address bus width).
>
> In some case for sure, but I believe not in most cases.
>
>>
>>
>>
>>> It's a safe hack, but I don't see the rationale.
>>
>>
>> I don't consider this a hack, we are trying to emulate real hardware, and
>> pxa lcd and dma controllers are intended to work with 32-bit bus. We should
>> not have a possibility to use them with 64-bit targets.
>>
>>
>>> If it's because VMSTATE_UINT32 requires that specific type than a less
>>> ugly hack would be to make a pxa specific memory address type.
>>>
>>
>> Introducing new type doesn't look pretty to me,
>
> Why?

Peter already answered, this fields should be exactly 32-bit wide 
(hardware is implemented this way) and we already have a type that is 
exactly 32-bit wide. Implementing each device without any assumptions 
about other parts of emulated system seems like a right approach to me.
Doing something like typedef uint32_t pxalcd_phys_addr_t is fun, but 
then we could end up introducing typedefs like this for every device in hw/.
Also, currently we can't save custom types in VMSTATE.

>> maybe just rename variables
>> to source_addr, dest_addr e.t.c?
>
> Wouldn't it be analogous to changing pointer typed variables to void *
> and adding the actual type in their names?  The result is that at
> language level they'll all be the same type even though they are not.
>
> (or changing le32 and be32 to uint32 in Linux)
>

Yes, but you got to admit they would be more informative for human :)


-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com