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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]

On 2011-12-10 17:26, Blue Swirl wrote:
> On Sat, Dec 10, 2011 at 16:03, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2011-12-10 16:54, Blue Swirl wrote:
>>> On Sat, Dec 10, 2011 at 15:51, Jan Kiszka <jan.kiszka@web.de> 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.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]