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

[Blue Swirl]

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.