Re: [Qemu-devel] [PATCH 2/2] i8254: Rework & fix interaction with HPET in legacy mode

[Blue Swirl]

On Sat, Dec 10, 2011 at 16:29, Jan Kiszka <address@hidden> wrote:
> On 2011-12-10 17:26, Blue Swirl wrote:
>> On Sat, Dec 10, 2011 at 16:03, Jan Kiszka <address@hidden> wrote:
>>> On 2011-12-10 16:54, Blue Swirl wrote:
>>>> On Sat, Dec 10, 2011 at 15:51, Jan Kiszka <address@hidden> wrote:
>>>>> On 2011-12-10 16:49, Blue Swirl wrote:
>>>>>>>
>>>>>>> +ISADevice *pit_init(int base, qemu_irq irq)
>>>>>>
>>>>>> Please retain this function in pc.h, or even better, introduce i8254.h.
>>>>>
>>>>> No concerns about i8254.h, but this function does not qualify for static
>>>>> inline.
>>>>
>>>> The function is static inline in a header file not for performance
>>>> reasons, but to keep the instantiation separate from device internals.
>>>
>>> Not performance, footprint and header dependencies. You need to pull in
>>> all the stuff the inline function needs for everyone including the
>>> header that contains this function. That's messy.
>>
>> There's only ISA and qdev stuff, that's not messy since both are
>> needed in any case.
>>
>>> Even if the instantiation helper should not poke into the device model
>>> internals (and I don't want this to change as well), it belongs to the
>>> module that implements the device. We do the same with other fabric
>>> functions.
>>
>> In this case, the callers have the same needs and there are several of
>> them. In general this need not be true at all, if for example some
>> part of instantiation would have to be skipped, the functions may need
>> to be manually inlined to the board level anyway. The instantiation
>> definitely does not belong to the implementer but to the creator.
>> Ideally file implementing the device contains only static functions
>> and instantiation is either in a header file or at the board. This is
>> true for example for several Sparc32 devices.
>
> The helper is wrapping the property base API into a proper function call
> - nothing that is board-specific.

Not in this case, but in general boards could need to pass different
sets of properties or avoid passing something at all.