Re: [Qemu-devel] [PATCH 06/10] vmmouse: convert to qdev

[Blue Swirl]

On Tue, Feb 15, 2011 at 7:22 PM, Blue Swirl <address@hidden> wrote:
> On Tue, Feb 15, 2011 at 12:07 PM, Markus Armbruster <address@hidden> wrote:
>> Anthony Liguori <address@hidden> writes:
>>
>>> On 02/12/2011 11:03 AM, Markus Armbruster wrote:
>>>> Blue Swirl<address@hidden>  writes:
>>>>
>>>>
>>>>> Convert to qdev, also add a proper reset function.
>> [...]
>>>> Pointer properties are for dirty hacks only.  Is there really no better
>>>> solution?  Why does it have to be a property?
>>>>
>>>
>>> vmmouse is really just an extension to the PS2 Mouse.  It's definitely
>>> not an ISA device.
>>>
>>> In terms of qdev enablement, I would just make it a boolean option to
>>> the PS2Mouse and not expose it as a top level device at all.  It
>>> cannot exist without a PS2Mouse.
>>
>> Which means making it a separate qdev is wrong.  That wrongness gave
>> rise to the dirty pointer property.  Pointer property serves as canary
>> again.
>>
>> What now?
>
> I don't find pointer property use so dirty, but I'll try to combine
> the devices to see whether that makes sense.

ps2.c is actually a library which implements core parts of PS/2 mouse
and keyboard. It is used by pckbd.c (i8042) and pl050.c, so if we want
to get rid of it, all three should be merged. Perhaps instead the file
should be just renamed to libps2.c. As a side note, Makefile
dependencies are not optimal, the file should only be compiled when
either CONFIG_PCKBD is set (most architectures) or the target is ARM
(pl050).

vmport.c is only needed by vmmouse.c. It still implements some
unrelated functions. vmmouse.c does not register any ISA ports by
itself, so keeping it as a separate ISADevice does not make much
sense. Merging would let us get rid of the useless registration,
vmport.c is also compiled now even though it may be unused if there is
no vmmouse.

Merging pckbd.c with vmmouse.c: one problem is that pckbd.c is
compiled in hwlib, but vmmouse via vmport needs to access CPU
registers. Actually the interface between them is quite slim, only
i8042_isa_mouse_fake_event(). Maybe this can be replaced with a
qemu_irq, so we get rid of the pointer property.