On Sun, Mar 6, 2011 at 5:39 PM, Jan Kiszka <jan.kis...@web.de> wrote:
> On 2011-03-06 16:35, Blue Swirl wrote:
>> On Sun, Mar 6, 2011 at 5:10 PM, Jan Kiszka <jan.kis...@web.de> wrote:
>>> On 2011-02-13 22:10, Blue Swirl wrote:
>>>> Convert to qdev. Don't expose PITState.
>>>>
>>>
>>> ...
>>>
>>>> diff --git a/hw/pc.h b/hw/pc.h
>>>> index 60f8c42..feb8a7a 100644
>>>> --- a/hw/pc.h
>>>> +++ b/hw/pc.h
>>>> @@ -82,14 +82,23 @@ void isa_irq_handler(void *opaque, int n, int level);
>>>>
>>>> #define PIT_FREQ 1193182
>>>>
>>>> -typedef struct PITState PITState;
>>>> +static inline ISADevice *pit_init(int base, int irq)
>>>> +{
>>>> + ISADevice *dev;
>>>> +
>>>> + dev = isa_create("isa-pit");
>>>> + qdev_prop_set_uint32(&dev->qdev, "iobase", base);
>>>> + qdev_prop_set_uint32(&dev->qdev, "irq", irq);
>>>> + qdev_init_nofail(&dev->qdev);
>>>> +
>>>> + return dev;
>>>> +}
>>>
>>> This should be moved to i8254.c. We cannot compile the PIT out, and the
>>> above contains no board information. So I see no reason for this inlining.
>>
>> I see no reason for moving this back. In fact, if this was only used
>> by PC, it could be moved to pc.c.
>
> To my understanding, it's a factory helper for the PIT, avoiding
> boilerplate code at the creator site. And if we get >1 users, this
> cleanup would definitely pay off.
It should be possible to leave PIT out, with small changes to pcspk
(which obviously has a hard dependency to PIT) and HPET. Then this
makes even more sense.
